* [PATCH] iio: imu: st_lsm6dsx: simplify data ready pin parsing
@ 2017-04-09 18:03 Lorenzo Bianconi
2017-04-14 14:07 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Bianconi @ 2017-04-09 18:03 UTC (permalink / raw)
To: jic23; +Cc: linux-iio, lorenzo.bianconi
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)
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);
}
static int st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw, u8 *drdy_reg)
--
2.9.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: simplify data ready pin parsing
2017-04-09 18:03 [PATCH] iio: imu: st_lsm6dsx: simplify data ready pin parsing Lorenzo Bianconi
@ 2017-04-14 14:07 ` Jonathan Cameron
2017-04-14 14:18 ` Lorenzo Bianconi
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2017-04-14 14:07 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, lorenzo.bianconi
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.
> 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.
Jonathan
> }
>
> static int st_lsm6dsx_get_drdy_reg(struct st_lsm6dsx_hw *hw, u8 *drdy_reg)
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: simplify data ready pin parsing
2017-04-14 14:07 ` Jonathan Cameron
@ 2017-04-14 14:18 ` Lorenzo Bianconi
2017-04-14 14:34 ` Jonathan Cameron
0 siblings, 1 reply; 5+ messages in thread
From: Lorenzo Bianconi @ 2017-04-14 14:18 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Lorenzo BIANCONI
> 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?
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: simplify data ready pin parsing
2017-04-14 14:18 ` Lorenzo Bianconi
@ 2017-04-14 14:34 ` Jonathan Cameron
2017-04-14 14:37 ` Lorenzo Bianconi
0 siblings, 1 reply; 5+ messages in thread
From: Jonathan Cameron @ 2017-04-14 14:34 UTC (permalink / raw)
To: Lorenzo Bianconi; +Cc: linux-iio, Lorenzo BIANCONI
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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] iio: imu: st_lsm6dsx: simplify data ready pin parsing
2017-04-14 14:34 ` Jonathan Cameron
@ 2017-04-14 14:37 ` Lorenzo Bianconi
0 siblings, 0 replies; 5+ messages in thread
From: Lorenzo Bianconi @ 2017-04-14 14:37 UTC (permalink / raw)
To: Jonathan Cameron; +Cc: linux-iio, Lorenzo BIANCONI
> 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.
>
Ack, thx.
Regards,
Lorenzo
> 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
>>
>
--
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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-04-14 14:37 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-09 18:03 [PATCH] iio: imu: st_lsm6dsx: simplify data ready pin parsing Lorenzo Bianconi
2017-04-14 14:07 ` Jonathan Cameron
2017-04-14 14:18 ` Lorenzo Bianconi
2017-04-14 14:34 ` Jonathan Cameron
2017-04-14 14:37 ` Lorenzo Bianconi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.