Re: [PATCH] iio: imu: st_lsm6dsx: simplify data ready pin parsing

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> On 09/04/17 19:03, Lorenzo Bianconi wrote:
>> Simplify st_lsm6dsx_of_get_drdy_pin routine since of_property_read_u32
>> error conditions are already managed in st_lsm6dsx_get_drdy_reg()
>>
>> Fixes: dba329048ee5 (iio: imu: st_lsm6dsx: add possibility to select drdy pin)
> Not really a fix that I can see.  Adding this tag encourages people to pick this
> up for stable branches which isn't appropriate for a cleanup like this.

Ack, it is just a cleanup patch.

>> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@xxxxxx>
>> ---
>>  drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c | 10 +---------
>>  1 file changed, 1 insertion(+), 9 deletions(-)
>>
>> diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> index 98b51d7..462a27b 100644
>> --- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> +++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_core.c
>> @@ -559,19 +559,11 @@ static const unsigned long st_lsm6dsx_available_scan_masks[] = {0x7, 0x0};
>>  static int st_lsm6dsx_of_get_drdy_pin(struct st_lsm6dsx_hw *hw, int *drdy_pin)
>>  {
>>       struct device_node *np = hw->dev->of_node;
>> -     int err;
>>
>>       if (!np)
>>               return -EINVAL;
>>
>> -     err = of_property_read_u32(np, "st,drdy-int-pin", drdy_pin);
>> -     if (err == -ENODATA) {
>> -             /* if the property has not been specified use default value */
>> -             *drdy_pin = 1;
>> -             err = 0;
>> -     }
>> -
>> -     return err;
>> +     return of_property_read_u32(np, "st,drdy-int-pin", drdy_pin);
> Does this not result in problems if the pin isn't specified?
> There may be devicetrees out there relying on defaulting to 1.
>

According to documentation of_property_read_u32 returns -EINVAL if the
property does not exist, -ENODATA if property does not have a value
and -EOVERFLOW if the property data isn't large enough.
So I think it is cleaner to manage all error conditions in a same
place (st_lsm6dsx_get_drdy_reg in this case). If the property is not
specified in device tree and if drdy pin is not passed to the driver
through platform_data the default value (pin 1) will be used. What do
you think? Should I send a v2?

> Jonathan
>
>
>>  }
>>
>>  static int st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw, u8 *drdy_reg)
>>
>

Regards,
Lorenzo

-- 
UNIX is Sexy: who | grep -i blonde | talk; cd ~; wine; talk; touch;
unzip; touch; strip; gasp; finger; gasp; mount; fsck; more; yes; gasp;
umount; make clean; sleep
--
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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux