[PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Fabien Proriol]

 From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
From: Fabien Proriol <fabien.proriol@jdsu.com>
Date: Tue, 4 Nov 2014 17:05:59 +0100
Subject: [PATCH] xilinx-xadc-core: Fix voltage offset

When xilinx-xadc is used with hwmon driver to read voltage, offset used 
for temperature is always apply.
This patch change the return for IIO_CHAN_INFO_OFFSET to -EINVAL except 
for temperature to avoid offset.

Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>
---
  drivers/iio/adc/xilinx-xadc-core.c | 11 ++++++++---
  1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/iio/adc/xilinx-xadc-core.c 
b/drivers/iio/adc/xilinx-xadc-core.c
index 41d3a5e..6064d26 100644
--- a/drivers/iio/adc/xilinx-xadc-core.c
+++ b/drivers/iio/adc/xilinx-xadc-core.c
@@ -877,9 +877,14 @@ static int xadc_read_raw(struct iio_dev *indio_dev,
                         return -EINVAL;
                 }
         case IIO_CHAN_INFO_OFFSET:
-               /* Only the temperature channel has an offset */
-               *val = -((273150 << 12) / 503975);
-               return IIO_VAL_INT;
+               switch (chan->type) {
+               case IIO_TEMP:
+                       /* Only the temperature channel has an offset */
+                       *val = -((273150 << 12) / 503975);
+                       return IIO_VAL_INT;
+               default:
+                       return -EINVAL;
+               }
         case IIO_CHAN_INFO_SAMP_FREQ:
                 ret = xadc_read_adc_reg(xadc, XADC_REG_CONF2, &val16);
                 if (ret)
-- 
2.0.4

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Lars-Peter Clausen]

On 11/06/2014 04:33 PM, Fabien Proriol wrote:
>   From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
> From: Fabien Proriol <fabien.proriol@jdsu.com>
> Date: Tue, 4 Nov 2014 17:05:59 +0100
> Subject: [PATCH] xilinx-xadc-core: Fix voltage offset
>
> When xilinx-xadc is used with hwmon driver to read voltage, offset used
> for temperature is always apply.
> This patch change the return for IIO_CHAN_INFO_OFFSET to -EINVAL except
> for temperature to avoid offset.

I think we should rather fix iio_channel_read() to check if the channel 
supports the property that we try to read. Other drivers are likely to 
suffer from the same issue and fixing it in a central place fixes them all.

- Lars

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Fabien Proriol]

On 06/11/2014 16:44, Lars-Peter Clausen wrote:
> On 11/06/2014 04:33 PM, Fabien Proriol wrote:
>>   From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
>> From: Fabien Proriol <fabien.proriol@jdsu.com>
>> Date: Tue, 4 Nov 2014 17:05:59 +0100
>> Subject: [PATCH] xilinx-xadc-core: Fix voltage offset
>>
>> When xilinx-xadc is used with hwmon driver to read voltage, offset used
>> for temperature is always apply.
>> This patch change the return for IIO_CHAN_INFO_OFFSET to -EINVAL except
>> for temperature to avoid offset.
>
> I think we should rather fix iio_channel_read() to check if the 
> channel supports the property that we try to read. Other drivers are 
> likely to suffer from the same issue and fixing it in a central place 
> fixes them all.
>
> - Lars
>
>

Ok, I can propose this following patch.
With my xilinx-xadc driver, it fix also the same problem...

Fabien


 From d605db8de19687b7271e722aa1fa6028f1472a7b Mon Sep 17 00:00:00 2001
From: Fabien Proriol <fabien.proriol@jdsu.com>
Date: Thu, 6 Nov 2014 17:12:27 +0100
Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info

iio_channel_read must return an error to avoid offset for channel
without IIO_CHAN_INFO_OFFSET property

Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>
---
  drivers/iio/inkern.c | 10 +++++++++-
  1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 1e8e94d..04cb23f 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -419,12 +419,20 @@ static int iio_channel_read(struct iio_channel 
*chan, int *val, int *val2,
         enum iio_chan_info_enum info)
  {
         int unused;
+       int ret;

         if (val2 == NULL)
                 val2 = &unused;

-       return chan->indio_dev->info->read_raw(chan->indio_dev, 
chan->channel,
+       if (!iio_channel_has_info(chan->channel, info)) {
+               ret = -EINVAL;
+               goto err;
+       }
+
+       ret = chan->indio_dev->info->read_raw(chan->indio_dev, 
chan->channel,
                                                 val, val2, info);
+err:
+       return ret;
  }

  int iio_read_channel_raw(struct iio_channel *chan, int *val)
-- 
2.0.4

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Lars-Peter Clausen]

On 11/06/2014 05:18 PM, Fabien Proriol wrote:
> On 06/11/2014 16:44, Lars-Peter Clausen wrote:
>> On 11/06/2014 04:33 PM, Fabien Proriol wrote:
>>>    From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
>>> From: Fabien Proriol <fabien.proriol@jdsu.com>
>>> Date: Tue, 4 Nov 2014 17:05:59 +0100
>>> Subject: [PATCH] xilinx-xadc-core: Fix voltage offset
>>>
>>> When xilinx-xadc is used with hwmon driver to read voltage, offset used
>>> for temperature is always apply.
>>> This patch change the return for IIO_CHAN_INFO_OFFSET to -EINVAL except
>>> for temperature to avoid offset.
>>
>> I think we should rather fix iio_channel_read() to check if the
>> channel supports the property that we try to read. Other drivers are
>> likely to suffer from the same issue and fixing it in a central place
>> fixes them all.
>>
>> - Lars
>>
>>
>
> Ok, I can propose this following patch.
> With my xilinx-xadc driver, it fix also the same problem...
>

Yep, that looks better, thanks. It looks like your mail client screwed up 
the indenting and line wrapping in the patch. Can you try to re-send so it 
can be applied properly? One minor comment about the patch itself inline.

- Lars

> Fabien
>
>
>   From d605db8de19687b7271e722aa1fa6028f1472a7b Mon Sep 17 00:00:00 2001
> From: Fabien Proriol <fabien.proriol@jdsu.com>
> Date: Thu, 6 Nov 2014 17:12:27 +0100
> Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info
>
> iio_channel_read must return an error to avoid offset for channel
> without IIO_CHAN_INFO_OFFSET property
>
> Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>
> ---
>    drivers/iio/inkern.c | 10 +++++++++-
>    1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 1e8e94d..04cb23f 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -419,12 +419,20 @@ static int iio_channel_read(struct iio_channel
> *chan, int *val, int *val2,
>           enum iio_chan_info_enum info)
>    {
>           int unused;
> +       int ret;
>
>           if (val2 == NULL)
>                   val2 = &unused;
>
> -       return chan->indio_dev->info->read_raw(chan->indio_dev,
> chan->channel,
> +       if (!iio_channel_has_info(chan->channel, info)) {

Just return -EINVAL, no need for the goto.

> +               ret = -EINVAL;
> +               goto err;
> +       }
> +
> +       ret = chan->indio_dev->info->read_raw(chan->indio_dev,
> chan->channel,
>                                                   val, val2, info);
> +err:
> +       return ret;
>    }
>
>    int iio_read_channel_raw(struct iio_channel *chan, int *val)
>

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Jonathan Cameron]

On 06/11/14 16:25, Lars-Peter Clausen wrote:
> On 11/06/2014 05:18 PM, Fabien Proriol wrote:
>> On 06/11/2014 16:44, Lars-Peter Clausen wrote:
>>> On 11/06/2014 04:33 PM, Fabien Proriol wrote:
>>>>    From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
>>>> From: Fabien Proriol <fabien.proriol@jdsu.com>
>>>> Date: Tue, 4 Nov 2014 17:05:59 +0100
>>>> Subject: [PATCH] xilinx-xadc-core: Fix voltage offset
>>>>
>>>> When xilinx-xadc is used with hwmon driver to read voltage, offset used
>>>> for temperature is always apply.
>>>> This patch change the return for IIO_CHAN_INFO_OFFSET to -EINVAL except
>>>> for temperature to avoid offset.
>>>
>>> I think we should rather fix iio_channel_read() to check if the
>>> channel supports the property that we try to read. Other drivers are
>>> likely to suffer from the same issue and fixing it in a central place
>>> fixes them all.
>>>
>>> - Lars
>>>
>>>
>>
>> Ok, I can propose this following patch.
>> With my xilinx-xadc driver, it fix also the same problem...
>>
> 
> Yep, that looks better, thanks. It looks like your mail client screwed up the indenting and line wrapping in the patch. Can you try to re-send so it can be applied properly? One minor comment about the patch itself inline.
> 
Hi Fabien,

Did you get a chance to  make the minor tweak Lars suggested?

Thanks,

Jonathan
> - Lars
> 
>> Fabien
>>
>>
>>   From d605db8de19687b7271e722aa1fa6028f1472a7b Mon Sep 17 00:00:00 2001
>> From: Fabien Proriol <fabien.proriol@jdsu.com>
>> Date: Thu, 6 Nov 2014 17:12:27 +0100
>> Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info
>>
>> iio_channel_read must return an error to avoid offset for channel
>> without IIO_CHAN_INFO_OFFSET property
>>
>> Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>
>> ---
>>    drivers/iio/inkern.c | 10 +++++++++-
>>    1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>> index 1e8e94d..04cb23f 100644
>> --- a/drivers/iio/inkern.c
>> +++ b/drivers/iio/inkern.c
>> @@ -419,12 +419,20 @@ static int iio_channel_read(struct iio_channel
>> *chan, int *val, int *val2,
>>           enum iio_chan_info_enum info)
>>    {
>>           int unused;
>> +       int ret;
>>
>>           if (val2 == NULL)
>>                   val2 = &unused;
>>
>> -       return chan->indio_dev->info->read_raw(chan->indio_dev,
>> chan->channel,
>> +       if (!iio_channel_has_info(chan->channel, info)) {
> 
> Just return -EINVAL, no need for the goto.
> 
>> +               ret = -EINVAL;
>> +               goto err;
>> +       }
>> +
>> +       ret = chan->indio_dev->info->read_raw(chan->indio_dev,
>> chan->channel,
>>                                                   val, val2, info);
>> +err:
>> +       return ret;
>>    }
>>
>>    int iio_read_channel_raw(struct iio_channel *chan, int *val)
>>
> 
> -- 
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Lars-Peter Clausen]

On 11/16/2014 09:29 PM, Jonathan Cameron wrote:
> On 06/11/14 16:25, Lars-Peter Clausen wrote:
>> On 11/06/2014 05:18 PM, Fabien Proriol wrote:
>>> On 06/11/2014 16:44, Lars-Peter Clausen wrote:
>>>> On 11/06/2014 04:33 PM, Fabien Proriol wrote:
>>>>>     From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
>>>>> From: Fabien Proriol <fabien.proriol@jdsu.com>
>>>>> Date: Tue, 4 Nov 2014 17:05:59 +0100
>>>>> Subject: [PATCH] xilinx-xadc-core: Fix voltage offset
>>>>>
>>>>> When xilinx-xadc is used with hwmon driver to read voltage, offset used
>>>>> for temperature is always apply.
>>>>> This patch change the return for IIO_CHAN_INFO_OFFSET to -EINVAL except
>>>>> for temperature to avoid offset.
>>>>
>>>> I think we should rather fix iio_channel_read() to check if the
>>>> channel supports the property that we try to read. Other drivers are
>>>> likely to suffer from the same issue and fixing it in a central place
>>>> fixes them all.
>>>>
>>>> - Lars
>>>>
>>>>
>>>
>>> Ok, I can propose this following patch.
>>> With my xilinx-xadc driver, it fix also the same problem...
>>>
>>
>> Yep, that looks better, thanks. It looks like your mail client screwed up the indenting and line wrapping in the patch. Can you try to re-send so it can be applied properly? One minor comment about the patch itself inline.
>>
> Hi Fabien,
>
> Did you get a chance to  make the minor tweak Lars suggested?

He did, but I just noticed he only sent it to me instead of the ml.

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Fabien Proriol]

>From bdeab2d72a00587eac15eb91ec0820ff6d11bfe8 Mon Sep 17 00:00:00 2001
From: Fabien Proriol <fabien.proriol@jdsu.com>
Date: Mon, 17 Nov 2014 09:46:02 +0100
Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info

iio_channel_read must return an error to avoid offset for channel
without IIO_CHAN_INFO_OFFSET property

Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>
---
 drivers/iio/inkern.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index 1e8e94d..917eb00 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -423,6 +423,9 @@ static int iio_channel_read(struct iio_channel *chan, i=
nt *val, int *val2,
 	if (val2 =3D=3D NULL)
 		val2 =3D &unused;
=20
+	if(!iio_channel_has_info(chan->channel, info))
+		return -EINVAL;
+
 	return chan->indio_dev->info->read_raw(chan->indio_dev, chan->channel,
 						val, val2, info);
 }
--=20
2.0.4

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Lars-Peter Clausen]

On 11/17/2014 09:53 AM, Fabien Proriol wrote:
>  From bdeab2d72a00587eac15eb91ec0820ff6d11bfe8 Mon Sep 17 00:00:00 2001
> From: Fabien Proriol <fabien.proriol@jdsu.com>
> Date: Mon, 17 Nov 2014 09:46:02 +0100
> Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info
>
> iio_channel_read must return an error to avoid offset for channel
> without IIO_CHAN_INFO_OFFSET property
>
> Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>

Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>

Thanks.

> ---
>   drivers/iio/inkern.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index 1e8e94d..917eb00 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -423,6 +423,9 @@ static int iio_channel_read(struct iio_channel *chan, int *val, int *val2,
>   	if (val2 == NULL)
>   		val2 = &unused;
>
> +	if(!iio_channel_has_info(chan->channel, info))
> +		return -EINVAL;
> +
>   	return chan->indio_dev->info->read_raw(chan->indio_dev, chan->channel,
>   						val, val2, info);
>   }
>

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Jonathan Cameron]

On 17/11/14 08:55, Lars-Peter Clausen wrote:
> On 11/17/2014 09:53 AM, Fabien Proriol wrote:
>>  From bdeab2d72a00587eac15eb91ec0820ff6d11bfe8 Mon Sep 17 00:00:00 2001
>> From: Fabien Proriol <fabien.proriol@jdsu.com>
>> Date: Mon, 17 Nov 2014 09:46:02 +0100
>> Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info
>>
>> iio_channel_read must return an error to avoid offset for channel
>> without IIO_CHAN_INFO_OFFSET property
>>
>> Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>
> 
> Reviewed-by: Lars-Peter Clausen <lars@metafoo.de>
> 
> Thanks.
Hi

This patch appears to be against a relatively ancient kernel.
Would you mind rebasing it on linus' current tree?

Thanks,

Jonathan

> 
>> ---
>>   drivers/iio/inkern.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
>> index 1e8e94d..917eb00 100644
>> --- a/drivers/iio/inkern.c
>> +++ b/drivers/iio/inkern.c
>> @@ -423,6 +423,9 @@ static int iio_channel_read(struct iio_channel *chan, int *val, int *val2,
>>       if (val2 == NULL)
>>           val2 = &unused;
>>
>> +    if(!iio_channel_has_info(chan->channel, info))
Space after if.
>> +        return -EINVAL;
>> +
>>       return chan->indio_dev->info->read_raw(chan->indio_dev, chan->channel,
>>                           val, val2, info);
>>   }
>>
> 

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Fabien Proriol]

On 22/11/2014 12:44, Jonathan Cameron wrote:
> Hi
>
> This patch appears to be against a relatively ancient kernel.
> Would you mind rebasing it on linus' current tree?
>
> Thanks,
>
> Jonathan

Ok, this is the patch rebase on Linus Trovald master


>From 8833373032c2c4a48569137e0d10804131b98f1d Mon Sep 17 00:00:00 2001
From: Fabien Proriol <fabien.proriol@jdsu.com>
Date: Mon, 17 Nov 2014 09:46:02 +0100
Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info

iio_channel_read must return an error to avoid offset for channel
without IIO_CHAN_INFO_OFFSET property

Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>
---
 drivers/iio/inkern.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
index f084610..d33590e 100644
--- a/drivers/iio/inkern.c
+++ b/drivers/iio/inkern.c
@@ -426,6 +426,9 @@ static int iio_channel_read(struct iio_channel *chan, i=
nt *val, int *val2,
        if (val2 =3D=3D NULL)
                val2 =3D &unused;
=20
+       if(!iio_channel_has_info(chan->channel, info))
+               return -EINVAL;
+
        if (chan->indio_dev->info->read_raw_multi) {
                ret =3D chan->indio_dev->info->read_raw_multi(chan->indio_d=
ev,
                                        chan->channel, INDIO_MAX_RAW_ELEMEN=
TS,
--=20
2.0.4

Re: [PATCH] 0001-xilinx-xadc-core-Fix-voltage-offset.patch

[Jonathan Cameron]

On 05/12/14 16:12, Fabien Proriol wrote:
> On 22/11/2014 12:44, Jonathan Cameron wrote:
>> Hi
>>
>> This patch appears to be against a relatively ancient kernel.
>> Would you mind rebasing it on linus' current tree?
>>
>> Thanks,
>>
>> Jonathan
> 
> Ok, this is the patch rebase on Linus Trovald master
> 
> 
> From 8833373032c2c4a48569137e0d10804131b98f1d Mon Sep 17 00:00:00 2001
> From: Fabien Proriol <fabien.proriol@jdsu.com>
> Date: Mon, 17 Nov 2014 09:46:02 +0100
> Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info
> 
> iio_channel_read must return an error to avoid offset for channel
> without IIO_CHAN_INFO_OFFSET property
> 
> Signed-off-by: Fabien Proriol <fabien.proriol@jdsu.com>
Applied - by hand as something odd happened to this email to the
fixes-togreg branch of iio.git

Thanks,

Jonathan
> ---
>  drivers/iio/inkern.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/inkern.c b/drivers/iio/inkern.c
> index f084610..d33590e 100644
> --- a/drivers/iio/inkern.c
> +++ b/drivers/iio/inkern.c
> @@ -426,6 +426,9 @@ static int iio_channel_read(struct iio_channel *chan, int *val, int *val2,
>         if (val2 == NULL)
>                 val2 = &unused;
>  
> +       if(!iio_channel_has_info(chan->channel, info))
> +               return -EINVAL;
> +
>         if (chan->indio_dev->info->read_raw_multi) {
>                 ret = chan->indio_dev->info->read_raw_multi(chan->indio_dev,
>                                         chan->channel, INDIO_MAX_RAW_ELEMENTS,
>