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-iio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html