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.shevche...@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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to