On 02/09/14 11:30, Laurentiu Palcu wrote:
> 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.
Agreed. Don't rename the driver. Now, if companies went with naming based on
the interface it might be a good idea. In reality it is a nightmare so much
better to keep to a name that covers one part and as you say, make it clear
in the Kconfig help what other parts are supported...
Jonathan
>
> 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
