mfd: intel_quark_i2c_gpio: don't crash if !DMI

PATCH Applied Andy Shevchenko 2 participants 4 messages Mar 25, 2015 – Mar 26, 2015 Multifunction devices (MFD)

Share & tools

Andy Shevchenko

author

2015-03-25 18:04 UTC #0 lore raw

PATCH 1/1 mfd: intel_quark_i2c_gpio: don't crash if !DMI

dmi_get_system_info() may return NULL either when CONFIG_DMI is not set or when board has an old firmware. The patch prevents a crash and changes the default frequency to be in align with older board.

Signed-off-by Andy Shevchenko <[email protected]>

---
 drivers/mfd/intel_quark_i2c_gpio.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

drivers/mfd/intel_quark_i2c_gpio.c +15 -8

index 006f2a1..35291c2 100644
@@ -70,6 +70,9 @@ static const struct i2c_mode_info platform_i2c_mode_info[] = {
 		.name = "GalileoGen2",
 		.i2c_scl_freq = 400000,
 	},
+	{
+		/* Sentinel */
+	}
 };
 
 static struct resource intel_quark_i2c_res[] = {

@@ -153,10 +156,10 @@ static void intel_quark_unregister_i2c_clk(struct pci_dev *pdev)
 static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 {
 	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
+	const struct i2c_mode_info *info;
 	struct dw_i2c_platform_data *pdata;
 	struct resource *res = (struct resource *)cell->resources;
 	struct device *dev = &pdev->dev;
-	unsigned int i;
 
 	res[INTEL_QUARK_IORES_MEM].start =
 		pci_resource_start(pdev, MFD_I2C_BAR);

@@ -170,13 +173,17 @@ static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 	if (!pdata)
 		return -ENOMEM;
 
-	/* Fast mode by default */
-	pdata->i2c_scl_freq = 400000;
-
-	for (i = 0; i < ARRAY_SIZE(platform_i2c_mode_info); i++)
-		if (!strcmp(board_name, platform_i2c_mode_info[i].name))
-			pdata->i2c_scl_freq
-				= platform_i2c_mode_info[i].i2c_scl_freq;
+	/* Normal mode by default */
+	pdata->i2c_scl_freq = 100000;
+
+	if (board_name) {
+		for (info = platform_i2c_mode_info; info->name; info++) {
+			if (!strcmp(board_name, info->name)) {
+				pdata->i2c_scl_freq = info->i2c_scl_freq;
+				break;
+			}
+		}
+	}
 
 	cell->platform_data = pdata;
 	cell->pdata_size = sizeof(*pdata);

Signature · 1 line

2.1.4

Lee Jones

Applied

2015-03-26 08:30 UTC #1 lore raw

Quoted 14 lines · Andy Shevchenko

dmi_get_system_info() may return NULL either when CONFIG_DMI is not set or when board has an old firmware. The patch prevents a crash and changes the default frequency to be in align with older board.
Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)
drivers/mfd/intel_quark_i2c_gpio.c +0 -0
index 006f2a1..35291c2 100644

drivers/mfd/intel_quark_i2c_gpio.c static const struct i2c_mode_info platform_i2c_mode_info[] = { · line 76

Quoted 7 lines

drivers/mfd/intel_quark_i2c_gpio.c +3 -0

@@ -70,6 +70,9 @@ static const struct i2c_mode_info platform_i2c_mode_info[] = {
 		.name = "GalileoGen2",
 		.i2c_scl_freq = 400000,
 	},
+	{
+		/* Sentinel */
+	}
 };

There's no need for this overt NULL entry.

I'll apply the patch and change this.

Applied, thanks.

Quoted 38 lines

drivers/mfd/intel_quark_i2c_gpio.c +12 -8

 static struct resource intel_quark_i2c_res[] = {

@@ -153,10 +156,10 @@ static void intel_quark_unregister_i2c_clk(struct pci_dev *pdev)
 static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 {
 	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
+	const struct i2c_mode_info *info;
 	struct dw_i2c_platform_data *pdata;
 	struct resource *res = (struct resource *)cell->resources;
 	struct device *dev = &pdev->dev;
-	unsigned int i;
 
 	res[INTEL_QUARK_IORES_MEM].start =
 		pci_resource_start(pdev, MFD_I2C_BAR);

@@ -170,13 +173,17 @@ static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 	if (!pdata)
 		return -ENOMEM;
 
-	/* Fast mode by default */
-	pdata->i2c_scl_freq = 400000;
-
-	for (i = 0; i < ARRAY_SIZE(platform_i2c_mode_info); i++)
-		if (!strcmp(board_name, platform_i2c_mode_info[i].name))
-			pdata->i2c_scl_freq
-				= platform_i2c_mode_info[i].i2c_scl_freq;
+	/* Normal mode by default */
+	pdata->i2c_scl_freq = 100000;
+
+	if (board_name) {
+		for (info = platform_i2c_mode_info; info->name; info++) {
+			if (!strcmp(board_name, info->name)) {
+				pdata->i2c_scl_freq = info->i2c_scl_freq;
+				break;
+			}
+		}
+	}
 
 	cell->platform_data = pdata;
 	cell->pdata_size = sizeof(*pdata);

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Andy Shevchenko

author

2015-03-26 11:08 UTC #2 lore raw

Quoted 11 lines · Lee Jones

Quoted 9 lines · Andy Shevchenko
dmi_get_system_info() may return NULL either when CONFIG_DMI is not set or when board has an old firmware. The patch prevents a crash and changes the default frequency to be in align with older board.
Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

Quoted 14 lines

drivers/mfd/intel_quark_i2c_gpio.c static const struct i2c_mode_info platform_i2c_mode_info[] = { · line 76
Quoted 12 lines
drivers/mfd/intel_quark_i2c_gpio.c +3 -0
index 006f2a1..35291c2 100644
@@ -70,6 +70,9 @@ static const struct i2c_mode_info platform_i2c_mode_info[] = {
 		.name = "GalileoGen2",
 		.i2c_scl_freq = 400000,
 	},
+	{
+		/* Sentinel */
+	}
 };
There's no need for this overt NULL entry.

I had changed the loop condition, thus this additional item is appeared. I can't see how to prevent crash here if ID is matched but board name is not.

Quoted 44 lines

I'll apply the patch and change this.

Applied, thanks.

Quoted 38 lines

drivers/mfd/intel_quark_i2c_gpio.c +12 -8

 static struct resource intel_quark_i2c_res[] = {

@@ -153,10 +156,10 @@ static void intel_quark_unregister_i2c_clk(struct pci_dev *pdev)
 static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 {
 	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
+	const struct i2c_mode_info *info;
 	struct dw_i2c_platform_data *pdata;
 	struct resource *res = (struct resource *)cell->resources;
 	struct device *dev = &pdev->dev;
-	unsigned int i;
 
 	res[INTEL_QUARK_IORES_MEM].start =
 		pci_resource_start(pdev, MFD_I2C_BAR);

@@ -170,13 +173,17 @@ static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 	if (!pdata)
 		return -ENOMEM;
 
-	/* Fast mode by default */
-	pdata->i2c_scl_freq = 400000;
-
-	for (i = 0; i < ARRAY_SIZE(platform_i2c_mode_info); i++)
-		if (!strcmp(board_name, platform_i2c_mode_info[i].name))
-			pdata->i2c_scl_freq
-				= platform_i2c_mode_info[i].i2c_scl_freq;
+	/* Normal mode by default */
+	pdata->i2c_scl_freq = 100000;
+
+	if (board_name) {
+		for (info = platform_i2c_mode_info; info->name; info++) {
+			if (!strcmp(board_name, info->name)) {
+				pdata->i2c_scl_freq = info->i2c_scl_freq;
+				break;
+			}
+		}
+	}
 
 	cell->platform_data = pdata;
 	cell->pdata_size = sizeof(*pdata);

Signature · 2 lines

Andy Shevchenko <[email protected]>
Intel Finland Oy

Lee Jones

2015-03-26 13:56 UTC #3 lore raw

Quoted 27 lines · Andy Shevchenko

Quoted 11 lines · Lee Jones
Quoted 9 lines · Andy Shevchenko
dmi_get_system_info() may return NULL either when CONFIG_DMI is not set or when board has an old firmware. The patch prevents a crash and changes the default frequency to be in align with older board.
Signed-off-by Andy Shevchenko <[email protected]>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)
Quoted 14 lines
drivers/mfd/intel_quark_i2c_gpio.c static const struct i2c_mode_info platform_i2c_mode_info[] = { · line 76
Quoted 12 lines
drivers/mfd/intel_quark_i2c_gpio.c +3 -0
index 006f2a1..35291c2 100644
@@ -70,6 +70,9 @@ static const struct i2c_mode_info platform_i2c_mode_info[] = {
 		.name = "GalileoGen2",
 		.i2c_scl_freq = 400000,
 	},
+	{
+		/* Sentinel */
+	}
 };
There's no need for this overt NULL entry.

Quoted 3 lines

I had changed the loop condition, thus this additional item is appeared. I can't see how to prevent crash here if ID is matched but board name is not.

Sorry if my point was not clear.

The NULL entry is required. It taking up 3 lines with a comment in the middle is overkill.

Quoted 45 lines

Quoted 43 lines

I'll apply the patch and change this.

Applied, thanks.

Quoted 38 lines

drivers/mfd/intel_quark_i2c_gpio.c +12 -8

 static struct resource intel_quark_i2c_res[] = {

@@ -153,10 +156,10 @@ static void intel_quark_unregister_i2c_clk(struct pci_dev *pdev)
 static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 {
 	const char *board_name = dmi_get_system_info(DMI_BOARD_NAME);
+	const struct i2c_mode_info *info;
 	struct dw_i2c_platform_data *pdata;
 	struct resource *res = (struct resource *)cell->resources;
 	struct device *dev = &pdev->dev;
-	unsigned int i;
 
 	res[INTEL_QUARK_IORES_MEM].start =
 		pci_resource_start(pdev, MFD_I2C_BAR);

@@ -170,13 +173,17 @@ static int intel_quark_i2c_setup(struct pci_dev *pdev, struct mfd_cell *cell)
 	if (!pdata)
 		return -ENOMEM;
 
-	/* Fast mode by default */
-	pdata->i2c_scl_freq = 400000;
-
-	for (i = 0; i < ARRAY_SIZE(platform_i2c_mode_info); i++)
-		if (!strcmp(board_name, platform_i2c_mode_info[i].name))
-			pdata->i2c_scl_freq
-				= platform_i2c_mode_info[i].i2c_scl_freq;
+	/* Normal mode by default */
+	pdata->i2c_scl_freq = 100000;
+
+	if (board_name) {
+		for (info = platform_i2c_mode_info; info->name; info++) {
+			if (!strcmp(board_name, info->name)) {
+				pdata->i2c_scl_freq = info->i2c_scl_freq;
+				break;
+			}
+		}
+	}
 
 	cell->platform_data = pdata;
 	cell->pdata_size = sizeof(*pdata);

Signature · 4 lines

Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog