[PATCH] iio: adc: ad7380: fix event threshold shift

David Lechner posted 1 patch 10 months, 1 week ago
drivers/iio/adc/ad7380.c | 25 +++++++++++++++++++------
1 file changed, 19 insertions(+), 6 deletions(-)
[PATCH] iio: adc: ad7380: fix event threshold shift
Posted by David Lechner 10 months, 1 week ago
Add required bit shift to the event threshold read function to get
correct scaling.

When alert support was added, the write function correctly included the
required shift needed to convert the threshold register value to the
same scale as the raw ADC value. However, the shift got missed in the
read function.

Fixes: 27d1a4dbe1e1 ("iio: adc: ad7380: add alert support")
Signed-off-by: David Lechner <dlechner@baylibre.com>
---
 drivers/iio/adc/ad7380.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
index 4fcb49fdf56639784098f0147a9faef8dcb6b0f6..f3962a45e1e5b88cebf712cc867fbb576d3ca058 100644
--- a/drivers/iio/adc/ad7380.c
+++ b/drivers/iio/adc/ad7380.c
@@ -1611,11 +1611,25 @@ static int ad7380_write_event_config(struct iio_dev *indio_dev,
 	return ret;
 }
 
-static int ad7380_get_alert_th(struct ad7380_state *st,
+static int ad7380_get_alert_th(struct iio_dev *indio_dev,
+			       const struct iio_chan_spec *chan,
 			       enum iio_event_direction dir,
 			       int *val)
 {
-	int ret, tmp;
+	struct ad7380_state *st = iio_priv(indio_dev);
+	const struct iio_scan_type *scan_type;
+	int ret, tmp, shift;
+
+	scan_type = iio_get_current_scan_type(indio_dev, chan);
+	if (IS_ERR(scan_type))
+		return PTR_ERR(scan_type);
+
+	/*
+	 * The register value is 12-bits and is compared to the most significant
+	 * bits of raw value, therefore a shift is required to convert this to
+	 * the same scale as the raw value.
+	 */
+	shift = scan_type->realbits - 12;
 
 	switch (dir) {
 	case IIO_EV_DIR_RISING:
@@ -1625,7 +1639,7 @@ static int ad7380_get_alert_th(struct ad7380_state *st,
 		if (ret)
 			return ret;
 
-		*val = FIELD_GET(AD7380_ALERT_HIGH_TH, tmp);
+		*val = FIELD_GET(AD7380_ALERT_HIGH_TH, tmp) << shift;
 		return IIO_VAL_INT;
 	case IIO_EV_DIR_FALLING:
 		ret = regmap_read(st->regmap,
@@ -1634,7 +1648,7 @@ static int ad7380_get_alert_th(struct ad7380_state *st,
 		if (ret)
 			return ret;
 
-		*val = FIELD_GET(AD7380_ALERT_LOW_TH, tmp);
+		*val = FIELD_GET(AD7380_ALERT_LOW_TH, tmp) << shift;
 		return IIO_VAL_INT;
 	default:
 		return -EINVAL;
@@ -1648,7 +1662,6 @@ static int ad7380_read_event_value(struct iio_dev *indio_dev,
 				   enum iio_event_info info,
 				   int *val, int *val2)
 {
-	struct ad7380_state *st = iio_priv(indio_dev);
 	int ret;
 
 	switch (info) {
@@ -1656,7 +1669,7 @@ static int ad7380_read_event_value(struct iio_dev *indio_dev,
 		if (!iio_device_claim_direct(indio_dev))
 			return -EBUSY;
 
-		ret = ad7380_get_alert_th(st, dir, val);
+		ret = ad7380_get_alert_th(indio_dev, chan, dir, val);
 
 		iio_device_release_direct(indio_dev);
 		return ret;

---
base-commit: f8ffc92ae9052e6615896052f0c5b808bfc17520
change-id: 20250402-iio-adc-ad7380-fix-event-threshold-shift-b614db1a307f

Best regards,
-- 
David Lechner <dlechner@baylibre.com>
Re: [PATCH] iio: adc: ad7380: fix event threshold shift
Posted by Julien Stephan 10 months, 1 week ago
Le jeu. 3 avr. 2025 à 01:56, David Lechner <dlechner@baylibre.com> a écrit :
>
> Add required bit shift to the event threshold read function to get
> correct scaling.
>
> When alert support was added, the write function correctly included the
> required shift needed to convert the threshold register value to the
> same scale as the raw ADC value. However, the shift got missed in the
> read function.

Hi David,

Thank you for fixing that. LGTM

Reviewed-by: Julien Stephan <jstephan@baylibre.com>

>
> Fixes: 27d1a4dbe1e1 ("iio: adc: ad7380: add alert support")
> Signed-off-by: David Lechner <dlechner@baylibre.com>
> ---
>  drivers/iio/adc/ad7380.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> index 4fcb49fdf56639784098f0147a9faef8dcb6b0f6..f3962a45e1e5b88cebf712cc867fbb576d3ca058 100644
> --- a/drivers/iio/adc/ad7380.c
> +++ b/drivers/iio/adc/ad7380.c
> @@ -1611,11 +1611,25 @@ static int ad7380_write_event_config(struct iio_dev *indio_dev,
>         return ret;
>  }
>
> -static int ad7380_get_alert_th(struct ad7380_state *st,
> +static int ad7380_get_alert_th(struct iio_dev *indio_dev,
> +                              const struct iio_chan_spec *chan,
>                                enum iio_event_direction dir,
>                                int *val)
>  {
> -       int ret, tmp;
> +       struct ad7380_state *st = iio_priv(indio_dev);
> +       const struct iio_scan_type *scan_type;
> +       int ret, tmp, shift;
> +
> +       scan_type = iio_get_current_scan_type(indio_dev, chan);
> +       if (IS_ERR(scan_type))
> +               return PTR_ERR(scan_type);
> +
> +       /*
> +        * The register value is 12-bits and is compared to the most significant
> +        * bits of raw value, therefore a shift is required to convert this to
> +        * the same scale as the raw value.
> +        */
> +       shift = scan_type->realbits - 12;
>
>         switch (dir) {
>         case IIO_EV_DIR_RISING:
> @@ -1625,7 +1639,7 @@ static int ad7380_get_alert_th(struct ad7380_state *st,
>                 if (ret)
>                         return ret;
>
> -               *val = FIELD_GET(AD7380_ALERT_HIGH_TH, tmp);
> +               *val = FIELD_GET(AD7380_ALERT_HIGH_TH, tmp) << shift;
>                 return IIO_VAL_INT;
>         case IIO_EV_DIR_FALLING:
>                 ret = regmap_read(st->regmap,
> @@ -1634,7 +1648,7 @@ static int ad7380_get_alert_th(struct ad7380_state *st,
>                 if (ret)
>                         return ret;
>
> -               *val = FIELD_GET(AD7380_ALERT_LOW_TH, tmp);
> +               *val = FIELD_GET(AD7380_ALERT_LOW_TH, tmp) << shift;
>                 return IIO_VAL_INT;
>         default:
>                 return -EINVAL;
> @@ -1648,7 +1662,6 @@ static int ad7380_read_event_value(struct iio_dev *indio_dev,
>                                    enum iio_event_info info,
>                                    int *val, int *val2)
>  {
> -       struct ad7380_state *st = iio_priv(indio_dev);
>         int ret;
>
>         switch (info) {
> @@ -1656,7 +1669,7 @@ static int ad7380_read_event_value(struct iio_dev *indio_dev,
>                 if (!iio_device_claim_direct(indio_dev))
>                         return -EBUSY;
>
> -               ret = ad7380_get_alert_th(st, dir, val);
> +               ret = ad7380_get_alert_th(indio_dev, chan, dir, val);
>
>                 iio_device_release_direct(indio_dev);
>                 return ret;
>
> ---
> base-commit: f8ffc92ae9052e6615896052f0c5b808bfc17520
> change-id: 20250402-iio-adc-ad7380-fix-event-threshold-shift-b614db1a307f
>
> Best regards,
> --
> David Lechner <dlechner@baylibre.com>
>
Re: [PATCH] iio: adc: ad7380: fix event threshold shift
Posted by Jonathan Cameron 10 months, 1 week ago
On Thu, 3 Apr 2025 13:45:41 +0200
Julien Stephan <jstephan@baylibre.com> wrote:

> Le jeu. 3 avr. 2025 à 01:56, David Lechner <dlechner@baylibre.com> a écrit :
> >
> > Add required bit shift to the event threshold read function to get
> > correct scaling.
> >
> > When alert support was added, the write function correctly included the
> > required shift needed to convert the threshold register value to the
> > same scale as the raw ADC value. However, the shift got missed in the
> > read function.  
> 
> Hi David,
> 
> Thank you for fixing that. LGTM
> 
> Reviewed-by: Julien Stephan <jstephan@baylibre.com>
Applied to the fixes-togreg branch of iio.git

Thanks,

J
> 
> >
> > Fixes: 27d1a4dbe1e1 ("iio: adc: ad7380: add alert support")
> > Signed-off-by: David Lechner <dlechner@baylibre.com>
> > ---
> >  drivers/iio/adc/ad7380.c | 25 +++++++++++++++++++------
> >  1 file changed, 19 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/iio/adc/ad7380.c b/drivers/iio/adc/ad7380.c
> > index 4fcb49fdf56639784098f0147a9faef8dcb6b0f6..f3962a45e1e5b88cebf712cc867fbb576d3ca058 100644
> > --- a/drivers/iio/adc/ad7380.c
> > +++ b/drivers/iio/adc/ad7380.c
> > @@ -1611,11 +1611,25 @@ static int ad7380_write_event_config(struct iio_dev *indio_dev,
> >         return ret;
> >  }
> >
> > -static int ad7380_get_alert_th(struct ad7380_state *st,
> > +static int ad7380_get_alert_th(struct iio_dev *indio_dev,
> > +                              const struct iio_chan_spec *chan,
> >                                enum iio_event_direction dir,
> >                                int *val)
> >  {
> > -       int ret, tmp;
> > +       struct ad7380_state *st = iio_priv(indio_dev);
> > +       const struct iio_scan_type *scan_type;
> > +       int ret, tmp, shift;
> > +
> > +       scan_type = iio_get_current_scan_type(indio_dev, chan);
> > +       if (IS_ERR(scan_type))
> > +               return PTR_ERR(scan_type);
> > +
> > +       /*
> > +        * The register value is 12-bits and is compared to the most significant
> > +        * bits of raw value, therefore a shift is required to convert this to
> > +        * the same scale as the raw value.
> > +        */
> > +       shift = scan_type->realbits - 12;
> >
> >         switch (dir) {
> >         case IIO_EV_DIR_RISING:
> > @@ -1625,7 +1639,7 @@ static int ad7380_get_alert_th(struct ad7380_state *st,
> >                 if (ret)
> >                         return ret;
> >
> > -               *val = FIELD_GET(AD7380_ALERT_HIGH_TH, tmp);
> > +               *val = FIELD_GET(AD7380_ALERT_HIGH_TH, tmp) << shift;
> >                 return IIO_VAL_INT;
> >         case IIO_EV_DIR_FALLING:
> >                 ret = regmap_read(st->regmap,
> > @@ -1634,7 +1648,7 @@ static int ad7380_get_alert_th(struct ad7380_state *st,
> >                 if (ret)
> >                         return ret;
> >
> > -               *val = FIELD_GET(AD7380_ALERT_LOW_TH, tmp);
> > +               *val = FIELD_GET(AD7380_ALERT_LOW_TH, tmp) << shift;
> >                 return IIO_VAL_INT;
> >         default:
> >                 return -EINVAL;
> > @@ -1648,7 +1662,6 @@ static int ad7380_read_event_value(struct iio_dev *indio_dev,
> >                                    enum iio_event_info info,
> >                                    int *val, int *val2)
> >  {
> > -       struct ad7380_state *st = iio_priv(indio_dev);
> >         int ret;
> >
> >         switch (info) {
> > @@ -1656,7 +1669,7 @@ static int ad7380_read_event_value(struct iio_dev *indio_dev,
> >                 if (!iio_device_claim_direct(indio_dev))
> >                         return -EBUSY;
> >
> > -               ret = ad7380_get_alert_th(st, dir, val);
> > +               ret = ad7380_get_alert_th(indio_dev, chan, dir, val);
> >
> >                 iio_device_release_direct(indio_dev);
> >                 return ret;
> >
> > ---
> > base-commit: f8ffc92ae9052e6615896052f0c5b808bfc17520
> > change-id: 20250402-iio-adc-ad7380-fix-event-threshold-shift-b614db1a307f
> >
> > Best regards,
> > --
> > David Lechner <dlechner@baylibre.com>
> >