On Mon, Sep 01, 2014 at 08:51:36AM -0700, Srinivas Pandruvada wrote:
> On Mon, 2014-09-01 at 08:36 -0700, Joe Perches wrote:
> > On Mon, 2014-09-01 at 12:11 +0300, Laurentiu Palcu wrote:
> > > The following chips are either similar or have only the resolution
> > > different. Hence, change this driver to support these chips too:
> > > 
> > > BMI055  - combo chip (accelerometer part is identical to BMC150's)
> > > BMA255  - identical to BMC150's accelerometer
> > > BMA222E - 8 bit resolution
> > > BMA250E - 10 bit resolution
> > > BMA280  - 14 bit resolution
> > > 
> > > Additionally:
> > >  * add bmc150_accel_match_acpi_device() function to check that the device
> > >    has been enumerated through ACPI;
> > >  * rename bmc150_accel_acpi_gpio_probe() to bmc150_accel_gpio_probe()
> > >    since the ACPI matching has been moved to the new function.  Also, this
> > >    will allow for the GPIO matching to be done against a device tree too, 
> > > not only
> > >    ACPI tree;
> > []
> > > diff --git a/drivers/iio/accel/bmc150-accel.c 
> > > b/drivers/iio/accel/bmc150-accel.c
> > []
> > > @@ -647,12 +659,13 @@ static int bmc150_accel_read_raw(struct iio_dev 
> > > *indio_dev,
> > >           {
> > >                   int i;
> > >  
> > > -                 for (i = 0; i < ARRAY_SIZE(bmc150_accel_scale_table);
> > > -                                                                  ++i) {
> > > -                         if (bmc150_accel_scale_table[i].range ==
> > > +                 for (i = 0;
> > > +                      i < ARRAY_SIZE(data->chip_info->scale_table);
> > > +                      ++i) {
> > > +                         if (data->chip_info->scale_table[i].range ==
> > >                                                           data->range) {
> > >                                   *val2 =
> > > -                                 bmc150_accel_scale_table[i].scale;
> > > +                                 data->chip_info->scale_table[i].scale;
> > >                                   return IIO_VAL_INT_PLUS_MICRO;
> > >                           }
> > >                   }
> > 
> > This looks like it would read a lot better with
> > a temporary for data->chip_info->scale_table[i]
> > 
> > so these could become:
> > 
> >                     for (i = 0; i < etc; i++) {
> >                             type *temp = &data->chip_info->scale_table[i];
> >                             if (temp->range == data->range) {
> >                                     *val2 = temp->scale;
> >                                     return IIO_VAL_INT_PLUS_MICRO;
> >                             }
Rewrote this part in v5. Thanks for the suggestion.

> > 
> > Maybe all the bmc150_ variable names could be removed.
> > The prefixes don't seem to serve a purpose other than
> > to make the code longer.
> > 
> > The filename could be changed to be more generic.
> Then this will also require change in the CONFIG name to match. This
> will require all current users to change the config file once they
> update to new version of the driver, which they don't like to change
> once product config is finalized. Since the most of the chips will just
> differ by a number at the end and they may not be compatible to each
> other, finding a common name will be challenge.
> Instead the CONFIG description for this module should explicitly state
> the names of chips it is compatible to.
v5 contains the changed CONFIG description that includes the new added chips.

thanks,
laurentiu

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