This series adds the following improvements over the current AD7606's
driver implementation:
- Fix wrong usage of gpio array
- Fix standby that was documented as ACTIVE_LOW but handled in the
driver as if it was ACTIVE_HIGH
- Improve dt-bindings documentation
- Switch mutex lock to scoped guard
Signed-off-by: Guillaume Stols <redacted>
---
Changes in v2:
- Change scoped guard to guard(mutex)(&st->lock). This was tested with
Rob's bot condition, and seems not to generate warning anymore.
- Reorder the commits for avoiding bisection issues and respect commit
priority rules.
- Add vdrive-supply to required properties.
- Separate cosmetic changes from content ones in dt-binding
descriptions.
- Move maxItems changes (and plural in descriptions) to the commit that
adds conditions.
- Link to v1: https://lore.kernel.org/r/20240618-cleanup-ad7606-v1-0-f1854d5c779d@baylibre.com
---
Guillaume Stols (10):
dt-bindings: iio: adc: adi,ad7606: add missing datasheet link
dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names
dt-bindings: iio: adc: adi,ad7606: normalize textwidth
dt-bindings: iio: adc: adi,ad7606: improve descriptions
dt-bindings: iio: adc: adi,ad7606: add supply properties
dt-bindings: iio: adc: adi,ad7606: fix example
dt-bindings: iio: adc: adi,ad7606: add conditions
iio: adc: ad7606: fix oversampling gpio array
iio: adc: ad7606: fix standby gpio state to match the documentation
iio: adc: ad7606: switch mutexes to scoped_guard
.../devicetree/bindings/iio/adc/adi,ad7606.yaml | 129 ++++++++++++++++-----
drivers/iio/adc/ad7606.c | 68 +++++------
drivers/iio/adc/ad7606_spi.c | 5 +-
3 files changed, 131 insertions(+), 71 deletions(-)
---
base-commit: 07d4d0bb4a8ddcc463ed599b22f510d5926c2495
change-id: 20240416-cleanup-ad7606-161e2ed9818b
Best regards,
--
Guillaume Stols [off-list ref]
AD7606-8 is referred to as AD7606 by Analog Devices. This comment aims
to avoid confusion. Also the compatible names were not sorted by
alphabetical order.
Signed-off-by: Guillaume Stols <redacted>
---
Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
@@ -20,9 +20,9 @@ properties:compatible:enum:-adi,ad7605-4--adi,ad7606-8--adi,ad7606-6-adi,ad7606-4+-adi,ad7606-6+-adi,ad7606-8# Referred to as AD7606 (without -8) in the datasheet-adi,ad7606b-adi,ad7616
@@ -40,60 +40,58 @@ properties:adi,conversion-start-gpios:description:-Must be the device tree identifier of the CONVST pin.-This logic input is used to initiate conversions on the analog-input channels. As the line is active high, it should be marked-GPIO_ACTIVE_HIGH.+Must be the device tree identifier of the CONVST pin. This logic input is+used to initiate conversions on the analog input channels. As the line is+active high, it should be marked GPIO_ACTIVE_HIGH.maxItems:1reset-gpios:description:-Must be the device tree identifier of the RESET pin. If specified,-it will be asserted during driver probe. As the line is active high,-it should be marked GPIO_ACTIVE_HIGH.+Must be the device tree identifier of the RESET pin. If specified, it will+be asserted during driver probe. As the line is active high, it should be+marked GPIO_ACTIVE_HIGH.maxItems:1standby-gpios:description:-Must be the device tree identifier of the STBY pin. This pin is used-to place the AD7606 into one of two power-down modes, Standby mode or+Must be the device tree identifier of the STBY pin. This pin is used to+place the AD7606 into one of two power-down modes, Standby mode orShutdown mode. As the line is active low, it should be markedGPIO_ACTIVE_LOW.maxItems:1adi,first-data-gpios:description:-Must be the device tree identifier of the FRSTDATA pin.-The FRSTDATA output indicates when the first channel, V1, is-being read back on either the parallel, byte or serial interface.-As the line is active high, it should be marked GPIO_ACTIVE_HIGH.+Must be the device tree identifier of the FRSTDATA pin. The FRSTDATA+output indicates when the first channel, V1, is being read back on either+the parallel, byte or serial interface. As the line is active high, it+should be marked GPIO_ACTIVE_HIGH.maxItems:1adi,range-gpios:description:-Must be the device tree identifier of the RANGE pin. The polarity on-this pin determines the input range of the analog input channels. If-this pin is tied to a logic high, the analog input range is ±10V for-all channels. If this pin is tied to a logic low, the analog input range-is ±5V for all channels. As the line is active high, it should be marked+Must be the device tree identifier of the RANGE pin. The polarity on this+pin determines the input range of the analog input channels. If this pin+is tied to a logic high, the analog input range is ±10V for all channels.+If this pin is tied to a logic low, the analog input range is ±5V for all+channels. As the line is active high, it should be markedGPIO_ACTIVE_HIGH.maxItems:1adi,oversampling-ratio-gpios:description:-Must be the device tree identifier of the over-sampling-mode pins. As the line is active high, it should be marked-GPIO_ACTIVE_HIGH.+Must be the device tree identifier of the over-sampling mode pins. As the+line is active high, it should be marked GPIO_ACTIVE_HIGH.maxItems:3adi,sw-mode:description:Software mode of operation, so far available only for ad7616 and ad7606b.-It is enabled when all three oversampling mode pins are connected to-high level. The device is configured by the corresponding registers. If the-adi,oversampling-ratio-gpios property is defined, then the driver will set the-oversampling gpios to high. Otherwise, it is assumed that the pins are hardwired-to VDD.+It is enabled when all three oversampling mode pins are connected to high+level. The device is configured by the corresponding registers. If the+adi,oversampling-ratio-gpios property is defined, then the driver will set+the oversampling gpios to high. Otherwise, it is assumed that the pins are+hardwired to VDD.type:booleanrequired:
Reword a few descriptions, and normalize the text width to 80 characters.
Signed-off-by: Guillaume Stols <redacted>
---
.../devicetree/bindings/iio/adc/adi,ad7606.yaml | 33 +++++++++++++---------
1 file changed, 20 insertions(+), 13 deletions(-)
@@ -36,6 +36,9 @@ properties:avcc-supply:trueinterrupts:+description:+The BUSY pin falling edge indicates that the conversion is over, and thus+new data is available.maxItems:1adi,conversion-start-gpios:
@@ -48,8 +51,9 @@ properties:reset-gpios:description:Must be the device tree identifier of the RESET pin. If specified, it will-be asserted during driver probe. As the line is active high, it should be-marked GPIO_ACTIVE_HIGH.+be asserted during driver probe. On the AD7606x, as the line is active+high, it should be marked GPIO_ACTIVE_HIGH. On the AD7616, as the line is+active low, it should be marked GPIO_ACTIVE_LOW.maxItems:1standby-gpios:
@@ -62,36 +66,39 @@ properties:adi,first-data-gpios:description:-Must be the device tree identifier of the FRSTDATA pin. The FRSTDATA+Must be the device tree identifier of the FRSTDATA pin. The FRSTDATAoutput indicates when the first channel, V1, is being read back on either-the parallel, byte or serial interface. As the line is active high, it+the parallel, byte or serial interface. As the line is active high, itshould be marked GPIO_ACTIVE_HIGH.maxItems:1adi,range-gpios:description:-Must be the device tree identifier of the RANGE pin. The polarity on this+Must be the device tree identifier of the RANGE pin. The state on thispin determines the input range of the analog input channels. If this pinis tied to a logic high, the analog input range is ±10V for all channels.-If this pin is tied to a logic low, the analog input range is ±5V for all-channels. As the line is active high, it should be marked+On the AD760X, if this pin is tied to a logic low, the analog input range+is ±5V for all channels. As the line is active high, it should be markedGPIO_ACTIVE_HIGH.maxItems:1adi,oversampling-ratio-gpios:description:Must be the device tree identifier of the over-sampling mode pins. As the-line is active high, it should be marked GPIO_ACTIVE_HIGH.+line is active high, it should be marked GPIO_ACTIVE_HIGH. On the AD7606X+parts that support it, if all 3 pins are tied to a logic high, software+mode is enabled.maxItems:3adi,sw-mode:description:-Software mode of operation, so far available only for ad7616 and ad7606b.+Software mode of operation, so far available only for AD7616 and AD7606b.It is enabled when all three oversampling mode pins are connected to high-level. The device is configured by the corresponding registers. If the-adi,oversampling-ratio-gpios property is defined, then the driver will set-the oversampling gpios to high. Otherwise, it is assumed that the pins are-hardwired to VDD.+level for the AD7606B, or all two range selection pins are connected to+high level for the AD7616. The device is configured by the corresponding+registers. If the adi,oversampling-ratio-gpios property is defined, then+the driver will set the oversampling gpios to high. Otherwise, it is+assumed that the pins are hardwired to VDD.type:booleanrequired:
Example uses adi,ad7606-8 as compatible, but adi,sw-mode is not
available for it. So remove this property from example.
Signed-off-by: Guillaume Stols <redacted>
---
Documentation/devicetree/bindings/iio/adc/adi,ad7606.yaml | 1 -
1 file changed, 1 deletion(-)
@@ -35,6 +35,15 @@ properties:avcc-supply:true+vdrive-supply:+description:+Determines the voltage level at which the interface logic pins will+operate.++refin-supply:+description:+The voltage supply for optional external reference voltage.+interrupts:description:The BUSY pin falling edge indicates that the conversion is over, and thus
Since the driver supports several parts that present differences in
their layout and behaviour, it is necessary to describe the differences
from one chip to another.
Signed-off-by: Guillaume Stols <redacted>
---
.../devicetree/bindings/iio/adc/adi,ad7606.yaml | 60 +++++++++++++++++++---
1 file changed, 54 insertions(+), 6 deletions(-)
@@ -52,10 +52,11 @@ properties:adi,conversion-start-gpios:description:-Must be the device tree identifier of the CONVST pin. This logic input is-used to initiate conversions on the analog input channels. As the line is-active high, it should be marked GPIO_ACTIVE_HIGH.-maxItems:1+Must be the device tree identifier of the CONVST pin(s). This logic input+is used to initiate conversions on the analog input channels. As the line+is active high, it should be marked GPIO_ACTIVE_HIGH.+minItems:1+maxItems:2reset-gpios:description:
@@ -88,8 +89,11 @@ properties:is tied to a logic high, the analog input range is ±10V for all channels.On the AD760X, if this pin is tied to a logic low, the analog input rangeis ±5V for all channels. As the line is active high, it should be marked-GPIO_ACTIVE_HIGH.-maxItems:1+GPIO_ACTIVE_HIGH. On the AD7616, there are 2 pins, and if the 2 pins are+tied to a logic high, software mode is enabled, otherwise one of the 3+possible range values is selected.+minItems:1+maxItems:2adi,oversampling-ratio-gpios:description:
gpiod_set_array_value was misused here: the implementation relied on the
assumption that an unsigned long was required for each gpio, while the
function expects a bit array stored in "as much unsigned long as needed
for storing one bit per GPIO", i.e it is using a bit field.
Fixes: d2a415c86c6b ("iio: adc: ad7606: Add support for AD7606B ADC")
Signed-off-by: Guillaume Stols <redacted>
---
drivers/iio/adc/ad7606.c | 4 ++--
drivers/iio/adc/ad7606_spi.c | 5 +++--
2 files changed, 5 insertions(+), 4 deletions(-)
@@ -236,9 +236,9 @@ static int ad7606_write_os_hw(struct iio_dev *indio_dev, int val)structad7606_state*st=iio_priv(indio_dev);DECLARE_BITMAP(values,3);-values[0]=val;+values[0]=val&GENMASK(2,0);-gpiod_set_array_value(ARRAY_SIZE(values),st->gpio_os->desc,+gpiod_set_array_value(st->gpio_os->ndescs,st->gpio_os->desc,st->gpio_os->info,values);/* AD7616 requires a reset to update value */
@@ -249,8 +249,9 @@ static int ad7616_sw_mode_config(struct iio_dev *indio_dev)staticintad7606B_sw_mode_config(structiio_dev*indio_dev){structad7606_state*st=iio_priv(indio_dev);-unsignedlongos[3]={1};+DECLARE_BITMAP(os,3);+bitmap_fill(os,3);/**Softwaremodeisenabledwhenallthreeoversampling*pinsaresettohigh.Ifoversamplinggpiosaredefined
@@ -258,7 +259,7 @@ static int ad7606B_sw_mode_config(struct iio_dev *indio_dev)*otherwise,theymustbehardwiredtoVDD*/if(st->gpio_os){-gpiod_set_array_value(ARRAY_SIZE(os),+gpiod_set_array_value(st->gpio_os->ndescs,st->gpio_os->desc,st->gpio_os->info,os);}/* OS of 128 and 256 are available only in software mode */
The binding's documentation specifies that "As the line is active low, it
should be marked GPIO_ACTIVE_LOW". However, in the driver, it was handled
the opposite way. This commit sets the driver's behaviour in sync with the
documentation
Fixes: 722407a4e8c0 ("staging:iio:ad7606: Use GPIO descriptor API")
Signed-off-by: Guillaume Stols <redacted>
---
drivers/iio/adc/ad7606.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Switching to scoped_guard simplifies the code and avoids to take care to
unlock the mutex in case of premature return.
Signed-off-by: Guillaume Stols <redacted>
---
drivers/iio/adc/ad7606.c | 60 ++++++++++++++++++++++--------------------------
1 file changed, 27 insertions(+), 33 deletions(-)
@@ -69,19 +69,17 @@ static int ad7606_reg_access(struct iio_dev *indio_dev,structad7606_state*st=iio_priv(indio_dev);intret;-mutex_lock(&st->lock);+guard(mutex)(&st->lock);+if(readval){ret=st->bops->reg_read(st,reg);if(ret<0)-gotoerr_unlock;+returnret;*readval=ret;-ret=0;+return0;}else{-ret=st->bops->reg_write(st,reg,writeval);+returnst->bops->reg_write(st,reg,writeval);}-err_unlock:-mutex_unlock(&st->lock);-returnret;}staticintad7606_read_samples(structad7606_state*st)
@@ -124,18 +122,18 @@ static irqreturn_t ad7606_trigger_handler(int irq, void *p)structad7606_state*st=iio_priv(indio_dev);intret;-mutex_lock(&st->lock);+scoped_guard(mutex,&st->lock){+ret=ad7606_read_samples(st);+if(ret)+gotoerror_ret;-ret=ad7606_read_samples(st);-if(ret==0)iio_push_to_buffers_with_timestamp(indio_dev,st->data,iio_get_time_ns(indio_dev));--iio_trigger_notify_done(indio_dev->trig);-/* The rising edge of the CONVST signal starts a new conversion. */-gpiod_set_value(st->gpio_convst,1);--mutex_unlock(&st->lock);+error_ret:+iio_trigger_notify_done(indio_dev->trig);+/* The rising edge of the CONVST signal starts a new conversion. */+gpiod_set_value(st->gpio_convst,1);+}returnIRQ_HANDLED;}
@@ -259,17 +257,15 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,switch(mask){caseIIO_CHAN_INFO_SCALE:-mutex_lock(&st->lock);-i=find_closest(val2,st->scale_avail,st->num_scales);-if(st->sw_mode_en)-ch=chan->address;-ret=st->write_scale(indio_dev,ch,i);-if(ret<0){-mutex_unlock(&st->lock);-returnret;+scoped_guard(mutex,&st->lock){+i=find_closest(val2,st->scale_avail,st->num_scales);+if(st->sw_mode_en)+ch=chan->address;+ret=st->write_scale(indio_dev,ch,i);+if(ret<0)+returnret;+st->range[ch]=i;}-st->range[ch]=i;-mutex_unlock(&st->lock);return0;caseIIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -277,14 +273,12 @@ static int ad7606_write_raw(struct iio_dev *indio_dev,return-EINVAL;i=find_closest(val,st->oversampling_avail,st->num_os_ratios);-mutex_lock(&st->lock);-ret=st->write_os(indio_dev,i);-if(ret<0){-mutex_unlock(&st->lock);-returnret;+scoped_guard(mutex,&st->lock){+ret=st->write_os(indio_dev,i);+if(ret<0)+returnret;+st->oversampling=st->oversampling_avail[i];}-st->oversampling=st->oversampling_avail[i];-mutex_unlock(&st->lock);return0;default:
On Fri, Jun 28, 2024 at 02:48:25PM +0000, Guillaume Stols wrote:
Since the driver supports several parts that present differences in
their layout and behaviour, it is necessary to describe the differences
from one chip to another.
Signed-off-by: Guillaume Stols <redacted>
On Fri, Jun 28, 2024 at 02:48:18PM +0000, Guillaume Stols wrote:
This series adds the following improvements over the current AD7606's
driver implementation:
- Fix wrong usage of gpio array
- Fix standby that was documented as ACTIVE_LOW but handled in the
driver as if it was ACTIVE_HIGH
- Improve dt-bindings documentation
- Switch mutex lock to scoped guard
Signed-off-by: Guillaume Stols <redacted>
On Fri, Jun 28, 2024 at 04:53:50PM +0100, Conor Dooley wrote:
On Fri, Jun 28, 2024 at 02:48:18PM +0000, Guillaume Stols wrote:
quoted
This series adds the following improvements over the current AD7606's
driver implementation:
- Fix wrong usage of gpio array
- Fix standby that was documented as ACTIVE_LOW but handled in the
driver as if it was ACTIVE_HIGH
- Improve dt-bindings documentation
- Switch mutex lock to scoped guard
Signed-off-by: Guillaume Stols <redacted>
You need to pick up all the tags given on earlier versions.
Rob acked this one for example on Thursday and you sent this update
on Friday so should have seen that to pick it up.
If there are substantial changes and it is appropriate to drop a tag,
say so below the ---
Applied with tag from Rob put in place.
Thanks,
Jonathan
From: Jonathan Cameron <jic23@kernel.org> Date: 2024-06-29 15:22:33
On Fri, 28 Jun 2024 14:48:20 +0000
Guillaume Stols [off-list ref] wrote:
AD7606-8 is referred to as AD7606 by Analog Devices. This comment aims
to avoid confusion. Also the compatible names were not sorted by
alphabetical order.
Signed-off-by: Guillaume Stols <redacted>
So b4 interestingly picked up both acks from Rob and Conor on this
one but I can't figure out where Conor's one came from so I've dropped
it.
Applied.
@@ -20,9 +20,9 @@ properties:compatible:enum:-adi,ad7605-4--adi,ad7606-8--adi,ad7606-6-adi,ad7606-4+-adi,ad7606-6+-adi,ad7606-8# Referred to as AD7606 (without -8) in the datasheet-adi,ad7606b-adi,ad7616
@@ -36,6 +36,9 @@ properties:avcc-supply:trueinterrupts:+description:+The BUSY pin falling edge indicates that the conversion is over, and thus+new data is available.maxItems:1adi,conversion-start-gpios:
@@ -48,8 +51,9 @@ properties:reset-gpios:description:Must be the device tree identifier of the RESET pin. If specified, it will-be asserted during driver probe. As the line is active high, it should be-marked GPIO_ACTIVE_HIGH.+be asserted during driver probe. On the AD7606x, as the line is active+high, it should be marked GPIO_ACTIVE_HIGH. On the AD7616, as the line is+active low, it should be marked GPIO_ACTIVE_LOW.maxItems:1standby-gpios:
@@ -62,36 +66,39 @@ properties:adi,first-data-gpios:description:-Must be the device tree identifier of the FRSTDATA pin. The FRSTDATA+Must be the device tree identifier of the FRSTDATA pin. The FRSTDATAoutput indicates when the first channel, V1, is being read back on either-the parallel, byte or serial interface. As the line is active high, it+the parallel, byte or serial interface. As the line is active high, itshould be marked GPIO_ACTIVE_HIGH.maxItems:1adi,range-gpios:description:-Must be the device tree identifier of the RANGE pin. The polarity on this+Must be the device tree identifier of the RANGE pin. The state on thispin determines the input range of the analog input channels. If this pinis tied to a logic high, the analog input range is ±10V for all channels.-If this pin is tied to a logic low, the analog input range is ±5V for all-channels. As the line is active high, it should be marked+On the AD760X, if this pin is tied to a logic low, the analog input range+is ±5V for all channels. As the line is active high, it should be markedGPIO_ACTIVE_HIGH.maxItems:1adi,oversampling-ratio-gpios:description:Must be the device tree identifier of the over-sampling mode pins. As the-line is active high, it should be marked GPIO_ACTIVE_HIGH.+line is active high, it should be marked GPIO_ACTIVE_HIGH. On the AD7606X+parts that support it, if all 3 pins are tied to a logic high, software+mode is enabled.maxItems:3adi,sw-mode:description:-Software mode of operation, so far available only for ad7616 and ad7606b.+Software mode of operation, so far available only for AD7616 and AD7606b.It is enabled when all three oversampling mode pins are connected to high-level. The device is configured by the corresponding registers. If the-adi,oversampling-ratio-gpios property is defined, then the driver will set-the oversampling gpios to high. Otherwise, it is assumed that the pins are-hardwired to VDD.+level for the AD7606B, or all two range selection pins are connected to
If rolling again for some reason
level for the AD7606B, or both the range selections are connected to high
level for the AD7616.
+ high level for the AD7616. The device is configured by the corresponding
+ registers. If the adi,oversampling-ratio-gpios property is defined, then
+ the driver will set the oversampling gpios to high. Otherwise, it is
+ assumed that the pins are hardwired to VDD.
type: boolean
required:
@@ -35,6 +35,15 @@ properties:avcc-supply:true+vdrive-supply:+description:+Determines the voltage level at which the interface logic pins will+operate.++refin-supply:+description:+The voltage supply for optional external reference voltage.+interrupts:description:The BUSY pin falling edge indicates that the conversion is over, and thus
From: Jonathan Cameron <jic23@kernel.org> Date: 2024-06-29 15:37:09
On Fri, 28 Jun 2024 14:48:28 +0000
Guillaume Stols [off-list ref] wrote:
quoted hunk
Switching to scoped_guard simplifies the code and avoids to take care to
unlock the mutex in case of premature return.
Signed-off-by: Guillaume Stols <redacted>
---
drivers/iio/adc/ad7606.c | 60 ++++++++++++++++++++++--------------------------
1 file changed, 27 insertions(+), 33 deletions(-)
Why not simply a guard(mutex)(&st->lock) ?
Then we avoid the somewhat nasty label in an nested block of code.
quoted hunk
+ scoped_guard(mutex, &st->lock) {
+ ret = ad7606_read_samples(st);
+ if (ret)
+ goto error_ret;
- ret = ad7606_read_samples(st);
- if (ret == 0)
iio_push_to_buffers_with_timestamp(indio_dev, st->data,
iio_get_time_ns(indio_dev));
-
- iio_trigger_notify_done(indio_dev->trig);
- /* The rising edge of the CONVST signal starts a new conversion. */
- gpiod_set_value(st->gpio_convst, 1);
-
- mutex_unlock(&st->lock);
+error_ret:
+ iio_trigger_notify_done(indio_dev->trig);
+ /* The rising edge of the CONVST signal starts a new conversion. */
+ gpiod_set_value(st->gpio_convst, 1);
+ }
return IRQ_HANDLED;
}
@@ -259,17 +257,15 @@ static int ad7606_write_raw(struct iio_dev *indio_dev, switch (mask) { case IIO_CHAN_INFO_SCALE:- mutex_lock(&st->lock);- i = find_closest(val2, st->scale_avail, st->num_scales);- if (st->sw_mode_en)- ch = chan->address;- ret = st->write_scale(indio_dev, ch, i);- if (ret < 0) {- mutex_unlock(&st->lock);- return ret;+ scoped_guard(mutex, &st->lock) {
The mutex is grabbed in all paths that actually do anything.
Pull it out of the switch and use
guard(mutex)(&st->lock);
That will reduce the changes needed and give the same cleanups.
I doubt we care about potentially slowing down the path that returns an error
as we are writing something unwriteable.
quoted hunk
+ i = find_closest(val2, st->scale_avail, st->num_scales);
+ if (st->sw_mode_en)
+ ch = chan->address;
+ ret = st->write_scale(indio_dev, ch, i);
+ if (ret < 0)
+ return ret;
+ st->range[ch] = i;
}
- st->range[ch] = i;
- mutex_unlock(&st->lock);
return 0;
case IIO_CHAN_INFO_OVERSAMPLING_RATIO:
@@ -277,14 +273,12 @@ static int ad7606_write_raw(struct iio_dev *indio_dev, return -EINVAL; i = find_closest(val, st->oversampling_avail, st->num_os_ratios);- mutex_lock(&st->lock);- ret = st->write_os(indio_dev, i);- if (ret < 0) {- mutex_unlock(&st->lock);- return ret;+ scoped_guard(mutex, &st->lock) {+ ret = st->write_os(indio_dev, i);+ if (ret < 0)+ return ret;+ st->oversampling = st->oversampling_avail[i]; }- st->oversampling = st->oversampling_avail[i];- mutex_unlock(&st->lock); return 0; default:
From: Jonathan Cameron <jic23@kernel.org> Date: 2024-06-29 15:38:55
On Fri, 28 Jun 2024 16:55:37 +0100
Conor Dooley [off-list ref] wrote:
On Fri, Jun 28, 2024 at 04:53:50PM +0100, Conor Dooley wrote:
quoted
On Fri, Jun 28, 2024 at 02:48:18PM +0000, Guillaume Stols wrote:
quoted
This series adds the following improvements over the current AD7606's
driver implementation:
- Fix wrong usage of gpio array
- Fix standby that was documented as ACTIVE_LOW but handled in the
driver as if it was ACTIVE_HIGH
- Improve dt-bindings documentation
- Switch mutex lock to scoped guard
Signed-off-by: Guillaume Stols <redacted>
I picked up the first 2 with the acks scraped from v1.
There are enough minor changes that I've requested in the other patches
that I'd like a v3 fixing those. Obviously make sure to gather up appropriate
acks. You may want to wait a few days first though as there are a couple
of DT patches in here that need tags.
Jonathan
On Sat, Jun 29, 2024 at 04:22:23PM +0100, Jonathan Cameron wrote:
On Fri, 28 Jun 2024 14:48:20 +0000
Guillaume Stols [off-list ref] wrote:
quoted
AD7606-8 is referred to as AD7606 by Analog Devices. This comment aims
to avoid confusion. Also the compatible names were not sorted by
alphabetical order.
Signed-off-by: Guillaume Stols <redacted>
So b4 interestingly picked up both acks from Rob and Conor on this
one but I can't figure out where Conor's one came from so I've dropped
it.
My copy of b4 (0.14-dev-d4707) doesn't create one for me:
/stuff/b4/b4.sh shazam -s -S -t shazam 20240628-cleanup-ad7606-v2-2-96e02f90256d@baylibre.com
Grabbing thread from lore.kernel.org/all/20240628-cleanup-ad7606-v2-2-96e02f90256d@baylibre.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 20 messages in the thread
Looking for additional code-review trailers on lore.kernel.org
Checking attestation on all messages, may take a moment...
---
✓ [PATCH v2 1/10] dt-bindings: iio: adc: adi,ad7606: add missing datasheet link
+ Acked-by: Rob Herring (Arm) [off-list ref] (✓ DKIM/kernel.org)
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 2/10] dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names
+ Acked-by: Rob Herring (Arm) [off-list ref] (✓ DKIM/kernel.org)
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 3/10] dt-bindings: iio: adc: adi,ad7606: normalize textwidth
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 4/10] dt-bindings: iio: adc: adi,ad7606: improve descriptions
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 5/10] dt-bindings: iio: adc: adi,ad7606: add supply properties
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 6/10] dt-bindings: iio: adc: adi,ad7606: fix example
+ Acked-by: Conor Dooley [off-list ref] (✓ DKIM/kernel.org)
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 7/10] dt-bindings: iio: adc: adi,ad7606: add conditions
+ Reviewed-by: Conor Dooley [off-list ref] (✓ DKIM/kernel.org)
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 8/10] iio: adc: ad7606: fix oversampling gpio array
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 9/10] iio: adc: ad7606: fix standby gpio state to match the documentation
+ Signed-off-by: Conor Dooley [off-list ref]
✓ [PATCH v2 10/10] iio: adc: ad7606: switch mutexes to scoped_guard
+ Signed-off-by: Conor Dooley [off-list ref]
---
✓ Signed: DKIM/baylibre-com.20230601.gappssmtp.com (From: gstols@baylibre.com)
---
Total patches: 10
---
Base: using specified base-commit 07d4d0bb4a8ddcc463ed599b22f510d5926c2495
Applying: dt-bindings: iio: adc: adi,ad7606: add missing datasheet link
Applying: dt-bindings: iio: adc: adi,ad7606: comment and sort the compatible names
Applying: dt-bindings: iio: adc: adi,ad7606: normalize textwidth
Applying: dt-bindings: iio: adc: adi,ad7606: improve descriptions
Applying: dt-bindings: iio: adc: adi,ad7606: add supply properties
Applying: dt-bindings: iio: adc: adi,ad7606: fix example
Applying: dt-bindings: iio: adc: adi,ad7606: add conditions
Applying: iio: adc: ad7606: fix oversampling gpio array
Applying: iio: adc: ad7606: fix standby gpio state to match the documentation
Applying: iio: adc: ad7606: switch mutexes to scoped_guard
tbh, I'm not actually sure why I didn't ack those patches on v1, they
were all pretty trivial...
On Fri, 28 Jun 2024 14:48:22 +0000, Guillaume Stols wrote:
Reword a few descriptions, and normalize the text width to 80 characters.
Signed-off-by: Guillaume Stols <redacted>
---
.../devicetree/bindings/iio/adc/adi,ad7606.yaml | 33 +++++++++++++---------
1 file changed, 20 insertions(+), 13 deletions(-)