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

[Jonathan Cameron <jic23@kernel.org>]

On 14/04/17 15:18, Lorenzo Bianconi wrote:
>> 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@st.com>
>>> ---
>>>  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?
Ah.  I'd missed that we end up with that default anyway.

Don't worry about a v2. I'll just drop the fixes tag and apply.

Done. Applied to the togreg branch of iio.git and pushed out
as testing for the autobuilders to play with it.

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