Skip to contentSkip to conversation

0001-xilinx-xadc-core-Fix-voltage-offset.patch

PATCH In review Fabien Proriol 3 participants 11 messages Nov 6, 2014 – Jan 1, 2015 2 messages not in the archive

Subsystems (2)
IIO Arm/Zynq architecture
lore
Share & tools
lore

Fabien Proriol

author
#0 lore raw

PATCH 1/1 0001-xilinx-xadc-core-Fix-voltage-offset.patch

From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
From: Fabien Proriol <[email protected]>
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 <[email protected]>
---
  drivers/iio/adc/xilinx-xadc-core.c | 11 ++++++++---
  1 file changed, 8 insertions(+), 3 deletions(-)
(quoted diff) +0 -0

b/drivers/iio/adc/xilinx-xadc-core.c
index 41d3a5e..6064d26 100644

drivers/iio/adc/xilinx-xadc-core.c +8 -3
@@ -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)
Signature · 1 line
2.0.4
#1 lore raw
Quoted 9 lines · Fabien Proriol

From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
From: Fabien Proriol <[email protected]>
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

author
#2 lore raw
Quoted 11 lines · Lars-Peter Clausen
Quoted 9 lines · Fabien Proriol

From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001
From: Fabien Proriol <[email protected]>
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.

Quoted 8 lines

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 <[email protected]>
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 <[email protected]>
---
  drivers/iio/inkern.c | 10 +++++++++-
  1 file changed, 9 insertions(+), 1 deletion(-)
drivers/iio/inkern.c +0 -0
index 1e8e94d..04cb23f 100644
@@ -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)

Signature · 1 line
2.0.4
#3 lore raw
Quoted 17 lines · Fabien Proriol
Quoted 16 lines · Lars-Peter Clausen
Quoted 9 lines · Fabien Proriol
   From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001

From: Fabien Proriol <[email protected]>
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.

Quoted 7 lines
Quoted 3 lines

- 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

Quoted 23 lines

Fabien

From d605db8de19687b7271e722aa1fa6028f1472a7b Mon Sep 17 00:00:00 2001
From: Fabien Proriol <[email protected]>
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 <[email protected]>
---
   drivers/iio/inkern.c | 10 +++++++++-
   1 file changed, 9 insertions(+), 1 deletion(-)
drivers/iio/inkern.c +0 -0
index 1e8e94d..04cb23f 100644
@@ -419,12 +419,20 @@ static int iio_channel_read(struct iio_channel
 *chan, int *val, int *val2,
          enum iio_chan_info_enum info)

drivers/iio/inkern.c static int iio_channel_read(struct iio_channel · line 429

Quoted 10 lines
drivers/iio/inkern.c +2 -1
   {
          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.

Quoted 13 lines
drivers/iio/inkern.c +7 -0
+               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)

maintainer
#4 lore raw
Quoted 22 lines · Lars-Peter Clausen
Quoted 21 lines · Fabien Proriol
Quoted 19 lines · Lars-Peter Clausen
Quoted 9 lines · Fabien Proriol
   From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001

From: Fabien Proriol <[email protected]>
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

Quoted 6 lines
Quoted 3 lines

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

Quoted 56 lines

- Lars

Quoted 23 lines

Fabien

From d605db8de19687b7271e722aa1fa6028f1472a7b Mon Sep 17 00:00:00 2001
From: Fabien Proriol <[email protected]>
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 <[email protected]>
---
   drivers/iio/inkern.c | 10 +++++++++-
   1 file changed, 9 insertions(+), 1 deletion(-)
drivers/iio/inkern.c +0 -0
index 1e8e94d..04cb23f 100644
@@ -419,12 +419,20 @@ static int iio_channel_read(struct iio_channel
 *chan, int *val, int *val2,
          enum iio_chan_info_enum info)

drivers/iio/inkern.c static int iio_channel_read(struct iio_channel · line 429

Quoted 10 lines
drivers/iio/inkern.c +2 -1
   {
          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.

Quoted 13 lines
drivers/iio/inkern.c +7 -0
+               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)

Signature · 3 lines
To unsubscribe from this list: send the line "unsubscribe linux-iio" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#5 lore raw
Quoted 27 lines · Jonathan Cameron
Quoted 26 lines · Lars-Peter Clausen
Quoted 24 lines · Fabien Proriol
Quoted 11 lines · Lars-Peter Clausen
Quoted 9 lines · Fabien Proriol
    From 3f57e39e3c69476513c00cd5ec45703f58334972 Mon Sep 17 00:00:00 2001

From: Fabien Proriol <[email protected]>
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.

Quoted 8 lines

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...

Quoted 5 lines
Quoted 2 lines

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.

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#8 lore raw
Quoted 1 line

From bdeab2d72a00587eac15eb91ec0820ff6d11bfe8 Mon Sep 17 00:00:00 2001

From: Fabien Proriol <[email protected]>
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 <[email protected]>
---
 drivers/iio/inkern.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/iio/inkern.c +0 -0
index 1e8e94d..917eb00 100644
@@ -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

Reviewed-by
#9 lore raw
Quoted 9 lines · Fabien Proriol

From bdeab2d72a00587eac15eb91ec0820ff6d11bfe8 Mon Sep 17 00:00:00 2001
From: Fabien Proriol <[email protected]>
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 <[email protected]>
Reviewed-by Lars-Peter Clausen <[email protected]>

Thanks.

Quoted 19 lines
---
  drivers/iio/inkern.c | 3 +++
  1 file changed, 3 insertions(+)
drivers/iio/inkern.c +3 -0
index 1e8e94d..917eb00 100644
@@ -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);
  }
maintainer
#10 lore raw
Quoted 6 lines · Lars-Peter Clausen
Quoted 5 lines · Fabien Proriol

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

Quoted 8 lines
Quoted 4 lines

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

Signed-off-by Fabien Proriol <[email protected]>
Reviewed-by Lars-Peter Clausen <[email protected]>

Thanks.

Hi

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

Thanks,

Jonathan

Quoted 13 lines
Quoted 13 lines
---
  drivers/iio/inkern.c | 3 +++
  1 file changed, 3 insertions(+)
drivers/iio/inkern.c +0 -0
index 1e8e94d..917eb00 100644
@@ -423,6 +423,9 @@ static int iio_channel_read(struct iio_channel *chan, int *val, int *val2,
      if (val2 == NULL)
          val2 = &unused;
Quoted 1 line

drivers/iio/inkern.c static int iio_channel_read(struct iio_channel *chan, int *val, int *val2, · line 426

Quoted 1 line
drivers/iio/inkern.c +1 -0
+    if(!iio_channel_has_info(chan->channel, info))

Space after if.

Quoted 7 lines
Quoted 6 lines
drivers/iio/inkern.c +2 -0
+        return -EINVAL;
+
      return chan->indio_dev->info->read_raw(chan->indio_dev, chan->channel,
                          val, val2, info);
  }
author
#11 lore raw
Quoted 8 lines · Jonathan Cameron

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

Quoted 1 line

From 8833373032c2c4a48569137e0d10804131b98f1d Mon Sep 17 00:00:00 2001

From: Fabien Proriol <[email protected]>
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 <[email protected]>
---
 drivers/iio/inkern.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/iio/inkern.c +0 -0
index f084610..d33590e 100644
@@ -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

maintainer
#12 lore raw
Quoted 18 lines · Fabien Proriol
Quoted 8 lines · Jonathan Cameron

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 <[email protected]>
Date: Mon, 17 Nov 2014 09:46:02 +0100
Subject: [PATCH] iio: Fix iio_channel_read return if channel havn't info

Quoted 4 lines

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

Signed-off-by Fabien Proriol <[email protected]>

Applied - by hand as something odd happened to this email to the fixes-togreg branch of iio.git

Thanks,

Jonathan

Quoted 19 lines
---
 drivers/iio/inkern.c | 3 +++
 1 file changed, 3 insertions(+)
drivers/iio/inkern.c +3 -0
index f084610..d33590e 100644
@@ -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,