linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/1] mfd: intel_quark_i2c_gpio: don't crash if !DMI
@ 2015-03-25 18:03 Andy Shevchenko
  2015-03-26  8:29 ` Lee Jones
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2015-03-25 18:03 UTC (permalink / raw)
  To: Lee Jones, linux-kernel; +Cc: 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 <andriy.shevchenko@linux.intel.com>
---
 drivers/mfd/intel_quark_i2c_gpio.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
index 006f2a1..35291c2 100644
--- a/drivers/mfd/intel_quark_i2c_gpio.c
+++ b/drivers/mfd/intel_quark_i2c_gpio.c
@@ -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);
-- 
2.1.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] mfd: intel_quark_i2c_gpio: don't crash if !DMI
  2015-03-25 18:03 [PATCH 1/1] mfd: intel_quark_i2c_gpio: don't crash if !DMI Andy Shevchenko
@ 2015-03-26  8:29 ` Lee Jones
  2015-03-26 11:08   ` Andy Shevchenko
  0 siblings, 1 reply; 4+ messages in thread
From: Lee Jones @ 2015-03-26  8:29 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel

On Wed, 25 Mar 2015, Andy Shevchenko wrote:

> 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 <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/mfd/intel_quark_i2c_gpio.c | 23 +++++++++++++++--------
>  1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> index 006f2a1..35291c2 100644
> --- a/drivers/mfd/intel_quark_i2c_gpio.c
> +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> @@ -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.

>  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);

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] mfd: intel_quark_i2c_gpio: don't crash if !DMI
  2015-03-26  8:29 ` Lee Jones
@ 2015-03-26 11:08   ` Andy Shevchenko
  2015-03-26 13:56     ` Lee Jones
  0 siblings, 1 reply; 4+ messages in thread
From: Andy Shevchenko @ 2015-03-26 11:08 UTC (permalink / raw)
  To: Lee Jones; +Cc: linux-kernel

On Thu, 2015-03-26 at 08:29 +0000, Lee Jones wrote:
> On Wed, 25 Mar 2015, Andy Shevchenko wrote:
> 
> > 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 <andriy.shevchenko@linux.intel.com>
> > ---
> >  drivers/mfd/intel_quark_i2c_gpio.c | 23 +++++++++++++++--------
> >  1 file changed, 15 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> > index 006f2a1..35291c2 100644
> > --- a/drivers/mfd/intel_quark_i2c_gpio.c
> > +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> > @@ -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.

> 
> I'll apply the patch and change this.
> 
> Applied, thanks.
> 
> >  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);
> 


-- 
Andy Shevchenko <andriy.shevchenko@intel.com>
Intel Finland Oy


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 1/1] mfd: intel_quark_i2c_gpio: don't crash if !DMI
  2015-03-26 11:08   ` Andy Shevchenko
@ 2015-03-26 13:56     ` Lee Jones
  0 siblings, 0 replies; 4+ messages in thread
From: Lee Jones @ 2015-03-26 13:56 UTC (permalink / raw)
  To: Andy Shevchenko; +Cc: linux-kernel

On Thu, 26 Mar 2015, Andy Shevchenko wrote:

> On Thu, 2015-03-26 at 08:29 +0000, Lee Jones wrote:
> > On Wed, 25 Mar 2015, Andy Shevchenko wrote:
> > 
> > > 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 <andriy.shevchenko@linux.intel.com>
> > > ---
> > >  drivers/mfd/intel_quark_i2c_gpio.c | 23 +++++++++++++++--------
> > >  1 file changed, 15 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/mfd/intel_quark_i2c_gpio.c b/drivers/mfd/intel_quark_i2c_gpio.c
> > > index 006f2a1..35291c2 100644
> > > --- a/drivers/mfd/intel_quark_i2c_gpio.c
> > > +++ b/drivers/mfd/intel_quark_i2c_gpio.c
> > > @@ -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.

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.

> > I'll apply the patch and change this.
> > 
> > Applied, thanks.
> > 
> > >  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);
> > 
> 
> 

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

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2015-03-26 13:56 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-25 18:03 [PATCH 1/1] mfd: intel_quark_i2c_gpio: don't crash if !DMI Andy Shevchenko
2015-03-26  8:29 ` Lee Jones
2015-03-26 11:08   ` Andy Shevchenko
2015-03-26 13:56     ` Lee Jones

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).