* [PATCH v3 01/12] dt-bindings: iio: dac: ad5696: add reset/ldac/gain support
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar,
Conor Dooley
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add GPIO property for RESET, LDAC and GAIN pin. RESET is active-low, LDAC
is used to load DAC channels with values from input registers and GAIN
can double the voltage in output channels. The gain-gpios property is
not available to all supported parts. The adi,range-double property
indicates that GAIN pin is hardwired to high in case gain-gpios is not
set, otherwise it sets the initial value for the gain setting.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
.../devicetree/bindings/iio/dac/adi,ad5696.yaml | 41 +++++++++++++++++++++-
1 file changed, 40 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
index b5a88b03dc2f..c55158c464fd 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
@@ -37,14 +37,52 @@ properties:
description: |
The regulator supply for DAC reference voltage.
+ reset-gpios:
+ description: Active-low RESET pin to reset the device.
+ maxItems: 1
+
+ ldac-gpios:
+ description:
+ Active-low LDAC pin used to asynchronously update the DAC channels.
+ maxItems: 1
+
+ gain-gpios:
+ description:
+ GAIN pin that sets a multiplier for the DAC output voltage. When high,
+ the DAC output voltage is multiplied by 2, otherwise it is unchanged.
+ maxItems: 1
+
+ adi,range-double:
+ description:
+ Sets the initial voltage output range from 0 to 2xVREF. On devices that
+ have a GAIN pin and no gain-gpios property is set, this indicates the pin
+ is hardwired high.
+ type: boolean
+
required:
- compatible
- reg
-additionalProperties: false
+allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ anyOf:
+ - const: adi,ad5311r
+ - const: adi,ad5691r
+ - const: adi,ad5692r
+ - const: adi,ad5693
+ - const: adi,ad5693r
+ then:
+ properties:
+ gain-gpios: false
+
+unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
i2c {
#address-cells = <1>;
#size-cells = <0>;
@@ -53,6 +91,7 @@ examples:
compatible = "adi,ad5696";
reg = <0>;
vcc-supply = <&dac_vref>;
+ ldac-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
};
};
...
--
2.43.0
^ permalink raw reply related
* [PATCH v3 02/12] dt-bindings: iio: dac: ad5696: rework on power supplies
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar,
Conor Dooley
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add supplies for VDD, VLOGIC and VREF input voltage pins. The vcc-supply
property is deprecated, once it does not really exist as none of the
devices describe any power input with that name. VCC is also misleading as
it sounds like the input power supply, but it is being used as an external
voltage reference, which should be called VREF. Certain devices require
vref-supply to be available once an internal reference voltage is absent.
For correct operation vdd and vlogic supplies are required.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
.../devicetree/bindings/iio/dac/adi,ad5696.yaml | 34 ++++++++++++++++++++--
1 file changed, 31 insertions(+), 3 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
index c55158c464fd..7b936824917e 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
@@ -33,9 +33,22 @@ properties:
reg:
maxItems: 1
+ vdd-supply:
+ description: Input power supply.
+
+ vlogic-supply:
+ description:
+ Digital power supply. On some tiny package variants for single-channel
+ devices, this supply is internally connected to vdd; in that case, specify
+ this property with the same regulator as vdd.
+
+ vref-supply:
+ description:
+ Reference voltage supply. If not supplied the internal reference is used.
+
vcc-supply:
- description: |
- The regulator supply for DAC reference voltage.
+ deprecated: true
+ description: Use vref-supply instead.
reset-gpios:
description: Active-low RESET pin to reset the device.
@@ -62,8 +75,21 @@ properties:
required:
- compatible
- reg
+ - vdd-supply
+ - vlogic-supply
allOf:
+ - if:
+ properties:
+ compatible:
+ contains:
+ anyOf:
+ - const: adi,ad5693
+ - const: adi,ad5694
+ - const: adi,ad5696
+ then:
+ required:
+ - vref-supply
- if:
properties:
compatible:
@@ -90,7 +116,9 @@ examples:
ad5696: dac@0 {
compatible = "adi,ad5696";
reg = <0>;
- vcc-supply = <&dac_vref>;
+ vdd-supply = <&dac_vdd>;
+ vlogic-supply = <&dac_vlogic>;
+ vref-supply = <&dac_vref>;
ldac-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
};
};
--
2.43.0
^ permalink raw reply related
* [PATCH v3 00/12] New features for the AD5686 IIO driver
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar,
Conor Dooley, Maxwell Doose, Joshua Crofts
This is the second series of three on updating the AD5686 driver.
Initially, a big patch series was sent:
https://lore.kernel.org/r/20260422-ad5313r-iio-support-v1-0-ed7dca001d1b@analog.com
Then, the first patch series added fixes and cleanups:
https://lore.kernel.org/linux-iio/20260524-ad5686-fixes-v7-0-b6bf395d08bd@analog.com/
This one is introducing new features:
- Consume optional reset and correct power supplies;
- LDAC GPIO handling (active-low, held low when unused);
- SPI bus sync() implementation for batching multiple transfers;
- Triggered buffer support, leveraging LDAC and sync() to flush
all channel writes atomically;
- Gain control support through the scale property.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
Changes in v3:
- Add range-double property.
- Acquire reset control after power-up delay.
- Include cleanup.h and use guard(mutex)() in read_raw().
- Link to v2: https://lore.kernel.org/r/20260609-ad5686-new-features-v2-0-70b423f5c76d@analog.com
Changes in v2:
- Get reset control deasserted.
- Update entire spi_transfer struct rather than individual fields.
- Replace udelay() for fsleep() in probe().
- Minor changes addressing further feedback.
- Link to v1: https://lore.kernel.org/r/20260602-ad5686-new-features-v1-0-691e01883d27@analog.com
---
Rodrigo Alencar (12):
dt-bindings: iio: dac: ad5696: add reset/ldac/gain support
dt-bindings: iio: dac: ad5696: rework on power supplies
dt-bindings: iio: dac: ad5686: add reset/ldac/gain support
dt-bindings: iio: dac: ad5686: rework on power supplies
iio: dac: ad5686: add support for missing power supplies
iio: dac: ad5686: consume optional reset signal
iio: dac: ad5686: add ldac gpio
iio: dac: ad5686: introduce sync operation
iio: dac: ad5686: implement new sync() op for the spi bus
iio: dac: ad5686: add triggered buffer support
iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()
iio: dac: ad5686: add gain control support
.../devicetree/bindings/iio/dac/adi,ad5686.yaml | 72 +++++-
.../devicetree/bindings/iio/dac/adi,ad5696.yaml | 75 ++++++-
drivers/iio/dac/Kconfig | 2 +
drivers/iio/dac/ad5686-spi.c | 121 +++++++---
drivers/iio/dac/ad5686.c | 249 +++++++++++++++++++--
drivers/iio/dac/ad5686.h | 33 ++-
drivers/iio/dac/ad5696-i2c.c | 2 +-
7 files changed, 492 insertions(+), 62 deletions(-)
---
base-commit: ae696dfa47c30016cd429b9db5e70b259b8f509e
change-id: 20260602-ad5686-new-features-e116c04bddb9
Best regards,
--
Rodrigo Alencar <rodrigo.alencar@analog.com>
^ permalink raw reply
* [PATCH v3 03/12] dt-bindings: iio: dac: ad5686: add reset/ldac/gain support
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar,
Conor Dooley
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add GPIO property for RESET, LDAC and GAIN pin. RESET is active-low, LDAC
is used to load DAC channels with values from input registers and GAIN
can double the voltage in output channels. The gain-gpios property is
not available to all supported parts. The adi,range-double property
indicates that GAIN pin is hardwired to high in case gain-gpios is not
set, otherwise it sets the initial value for the gain setting.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
.../devicetree/bindings/iio/dac/adi,ad5686.yaml | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
index 713f535bb33a..7567de0ead4a 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
@@ -35,17 +35,53 @@ properties:
vcc-supply:
description: If not supplied the internal reference is used.
+ reset-gpios:
+ description: Active-low RESET pin to reset the device.
+ maxItems: 1
+
+ ldac-gpios:
+ description:
+ Active-low LDAC pin used to asynchronously update the DAC channels.
+ maxItems: 1
+
+ gain-gpios:
+ description:
+ GAIN pin that sets a multiplier for the DAC output voltage. When high,
+ the DAC output voltage is multiplied by 2, otherwise it is unchanged.
+ maxItems: 1
+
+ adi,range-double:
+ description:
+ Sets the initial voltage output range from 0 to 2xVREF. On devices that
+ have a GAIN pin and no gain-gpios property is set, this indicates the pin
+ is hardwired high.
+ type: boolean
+
required:
- compatible
- reg
allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ anyOf:
+ - const: adi,ad5310r
+ - const: adi,ad5681r
+ - const: adi,ad5682r
+ - const: adi,ad5683
+ - const: adi,ad5683r
+ then:
+ properties:
+ gain-gpios: false
unevaluatedProperties: false
examples:
- |
+ #include <dt-bindings/gpio/gpio.h>
spi {
#address-cells = <1>;
#size-cells = <0>;
@@ -53,6 +89,8 @@ examples:
reg = <0>;
compatible = "adi,ad5310r";
vcc-supply = <&dac_vref0>;
+ reset-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
+ ldac-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
};
};
...
--
2.43.0
^ permalink raw reply related
* [PATCH v3 04/12] dt-bindings: iio: dac: ad5686: rework on power supplies
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar,
Conor Dooley
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add supplies for VDD, VLOGIC and VREF input voltage pins. The vcc-supply
property is deprecated, once it does not really exist as none of the
devices describe any power input with that name. VCC is also misleading as
it sounds like the input power supply, but it is being used as an external
voltage reference, which should be called VREF. Certain devices require
vref-supply to be available once an internal reference voltage is absent.
For correct operation vdd and vlogic supplies are required.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
.../devicetree/bindings/iio/dac/adi,ad5686.yaml | 34 ++++++++++++++++++++--
1 file changed, 32 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
index 7567de0ead4a..bdc6bc425b48 100644
--- a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
+++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
@@ -32,8 +32,22 @@ properties:
reg:
maxItems: 1
+ vdd-supply:
+ description: Input power supply.
+
+ vlogic-supply:
+ description:
+ Digital power supply. On some tiny package variants for single-channel
+ devices, this supply is internally connected to vdd; in that case, specify
+ this property with the same regulator as vdd.
+
+ vref-supply:
+ description:
+ Reference voltage supply. If not supplied the internal reference is used.
+
vcc-supply:
- description: If not supplied the internal reference is used.
+ deprecated: true
+ description: Use vref-supply instead.
reset-gpios:
description: Active-low RESET pin to reset the device.
@@ -60,9 +74,23 @@ properties:
required:
- compatible
- reg
+ - vdd-supply
+ - vlogic-supply
allOf:
- $ref: /schemas/spi/spi-peripheral-props.yaml#
+ - if:
+ properties:
+ compatible:
+ contains:
+ anyOf:
+ - const: adi,ad5676
+ - const: adi,ad5683
+ - const: adi,ad5684
+ - const: adi,ad5686
+ then:
+ required:
+ - vref-supply
- if:
properties:
compatible:
@@ -88,7 +116,9 @@ examples:
dac@0 {
reg = <0>;
compatible = "adi,ad5310r";
- vcc-supply = <&dac_vref0>;
+ vdd-supply = <&dac_vdd>;
+ vlogic-supply = <&dac_vlogic>;
+ vref-supply = <&dac_vref>;
reset-gpios = <&gpio0 0 GPIO_ACTIVE_LOW>;
ldac-gpios = <&gpio0 1 GPIO_ACTIVE_LOW>;
};
--
2.43.0
^ permalink raw reply related
* [PATCH v3 06/12] iio: dac: ad5686: consume optional reset signal
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add RESET pin GPIO support through an optional reset control, which is
local to the probe function. A reset pulse is manually generated after
the device is powered up.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/ad5686.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index fc3863274b29..a48acedff51c 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -15,6 +15,7 @@
#include <linux/kstrtox.h>
#include <linux/module.h>
#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
#include <linux/sysfs.h>
#include <linux/wordpart.h>
@@ -472,6 +473,7 @@ int ad5686_probe(struct device *dev,
const struct ad5686_chip_info *chip_info,
const char *name, const struct ad5686_bus_ops *ops)
{
+ struct reset_control *rstc;
struct ad5686_state *st;
struct iio_dev *indio_dev;
int ret, i;
@@ -508,6 +510,15 @@ int ad5686_probe(struct device *dev,
fsleep(5); /* power-up time */
+ rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
+ if (IS_ERR(rstc))
+ return dev_err_probe(dev, PTR_ERR(rstc),
+ "Failed to get reset control\n");
+
+ reset_control_assert(rstc);
+ fsleep(1); /* reset pulse: comfortably bigger than the spec */
+ reset_control_deassert(rstc);
+
/* Initialize masks to all ones */
st->pwr_down_mask = ~0;
st->pwr_down_mode = ~0;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 05/12] iio: dac: ad5686: add support for missing power supplies
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Get and enable regulators for vdd, vlogic and vref input power pins. Vdd
is the input power supply, while vlogic powers the digital side. vref is
replacing vcc, which is being deprecated, but still supported. The value
of vref_mv is checked so that a device without internal voltage reference
cannot proceed without an explicit supply. For correct operation, vdd and
vlogic are required, then devm_regulator_get_enable() is used so the
driver can still work without them by using the stub/dummy regulators.
Error report uses dev_err_probe(), which helps debugging an init issue.
A small delay is added after the regulators are enabled to consider for
the power-up time (4.5 us).
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/ad5686.c | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 5840fda4b011..fc3863274b29 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -8,6 +8,8 @@
#include <linux/array_size.h>
#include <linux/bitfield.h>
#include <linux/bitops.h>
+#include <linux/delay.h>
+#include <linux/dev_printk.h>
#include <linux/errno.h>
#include <linux/export.h>
#include <linux/kstrtox.h>
@@ -484,12 +486,27 @@ int ad5686_probe(struct device *dev,
st->ops = ops;
st->chip_info = chip_info;
- ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
+ ret = devm_regulator_get_enable(dev, "vdd");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to enable vdd supply\n");
+
+ ret = devm_regulator_get_enable(dev, "vlogic");
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to enable vlogic supply\n");
+
+ ret = devm_regulator_get_enable_read_voltage(dev, "vref");
+ if (ret == -ENODEV) /* vcc-supply is deprecated, but supported still */
+ ret = devm_regulator_get_enable_read_voltage(dev, "vcc");
if (ret < 0 && ret != -ENODEV)
- return ret;
+ return dev_err_probe(dev, ret, "failed to read vref voltage\n");
st->use_internal_vref = ret == -ENODEV;
st->vref_mv = st->use_internal_vref ? st->chip_info->int_vref_mv : ret / 1000;
+ if (!st->vref_mv)
+ return dev_err_probe(dev, -EINVAL,
+ "invalid or not provided vref voltage\n");
+
+ fsleep(5); /* power-up time */
/* Initialize masks to all ones */
st->pwr_down_mask = ~0;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 07/12] iio: dac: ad5686: add ldac gpio
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
If wired LDAC, should be held low when unused (pin is active-low), which
allows for synchronous DAC updates. This will be used to update all the
channels at the same time when adding buffer support.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/ad5686.c | 6 ++++++
drivers/iio/dac/ad5686.h | 3 +++
2 files changed, 9 insertions(+)
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index a48acedff51c..73105c197964 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -12,6 +12,7 @@
#include <linux/dev_printk.h>
#include <linux/errno.h>
#include <linux/export.h>
+#include <linux/gpio/consumer.h>
#include <linux/kstrtox.h>
#include <linux/module.h>
#include <linux/regulator/consumer.h>
@@ -515,6 +516,11 @@ int ad5686_probe(struct device *dev,
return dev_err_probe(dev, PTR_ERR(rstc),
"Failed to get reset control\n");
+ st->ldac_gpio = devm_gpiod_get_optional(dev, "ldac", GPIOD_OUT_HIGH);
+ if (IS_ERR(st->ldac_gpio))
+ return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
+ "Failed to get LDAC GPIO\n");
+
reset_control_assert(rstc);
fsleep(1); /* reset pulse: comfortably bigger than the spec */
reset_control_deassert(rstc);
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index a06fe7d89305..f3110410d89b 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -60,6 +60,7 @@ enum ad5686_regmap_type {
AD5686_REGMAP,
};
+struct gpio_desc;
struct ad5686_state;
/**
@@ -119,6 +120,7 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
* @dev: device instance
* @chip_info: chip model specific constants, available modes etc
* @ops: bus specific operations
+ * @ldac_gpio: LDAC pin GPIO descriptor
* @vref_mv: actual reference voltage used
* @pwr_down_mask: power down mask
* @pwr_down_mode: current power down mode
@@ -131,6 +133,7 @@ struct ad5686_state {
struct device *dev;
const struct ad5686_chip_info *chip_info;
const struct ad5686_bus_ops *ops;
+ struct gpio_desc *ldac_gpio;
unsigned short vref_mv;
unsigned int pwr_down_mask;
unsigned int pwr_down_mode;
--
2.43.0
^ permalink raw reply related
* [PATCH v3 08/12] iio: dac: ad5686: introduce sync operation
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Add sync() to operation to ad5686_bus_ops, which can be used to flush
multiple pending data transfers at once. This is going to be used when
implementing triggered buffer support.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/ad5686.h | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index f3110410d89b..1da5f1b4d52a 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -67,10 +67,12 @@ struct ad5686_state;
* struct ad5686_bus_ops - bus specific read/write operations
* @read: read a register value at the given address
* @write: write a command, address and value to the device
+ * @sync: ensure the completion of the write operation (optional)
*/
struct ad5686_bus_ops {
int (*read)(struct ad5686_state *st, u8 addr);
int (*write)(struct ad5686_state *st, u8 cmd, u8 addr, u16 val);
+ int (*sync)(struct ad5686_state *st);
};
/**
@@ -159,7 +161,13 @@ int ad5686_probe(struct device *dev,
static inline int ad5686_write(struct ad5686_state *st, u8 cmd, u8 addr, u16 val)
{
- return st->ops->write(st, cmd, addr, val);
+ int ret;
+
+ ret = st->ops->write(st, cmd, addr, val);
+ if (ret)
+ return ret;
+
+ return st->ops->sync ? st->ops->sync(st) : 0;
}
static inline int ad5686_read(struct ad5686_state *st, u8 addr)
--
2.43.0
^ permalink raw reply related
* [PATCH v3 10/12] iio: dac: ad5686: add triggered buffer support
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Implement trigger handler by leveraging the LDAC gpio to update all DAC
channels at once when it is available. Also, the multiple channel writes
can be flushed at once with the sync() operation.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/Kconfig | 2 ++
drivers/iio/dac/ad5686.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 68 insertions(+)
diff --git a/drivers/iio/dac/Kconfig b/drivers/iio/dac/Kconfig
index 657c68e75542..5f14fcd780e2 100644
--- a/drivers/iio/dac/Kconfig
+++ b/drivers/iio/dac/Kconfig
@@ -240,6 +240,8 @@ config LTC2688
config AD5686
tristate
+ select IIO_BUFFER
+ select IIO_TRIGGERED_BUFFER
config AD5686_SPI
tristate "Analog Devices AD5686 and similar multi-channel DACs (SPI)"
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 34011cbef903..9e8a014e5ac3 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -20,7 +20,11 @@
#include <linux/sysfs.h>
#include <linux/wordpart.h>
+#include <linux/iio/buffer.h>
#include <linux/iio/iio.h>
+#include <linux/iio/trigger.h>
+#include <linux/iio/trigger_consumer.h>
+#include <linux/iio/triggered_buffer.h>
#include "ad5686.h"
@@ -246,6 +250,7 @@ static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
.address = addr, \
+ .scan_index = chan, \
.scan_type = { \
.sign = 'u', \
.realbits = (bits), \
@@ -470,6 +475,60 @@ const struct ad5686_chip_info ad5679r_chip_info = {
};
EXPORT_SYMBOL_NS_GPL(ad5679r_chip_info, "IIO_AD5686");
+static irqreturn_t ad5686_trigger_handler(int irq, void *p)
+{
+ struct iio_poll_func *pf = p;
+ struct iio_dev *indio_dev = pf->indio_dev;
+ struct iio_buffer *buffer = indio_dev->buffer;
+ struct ad5686_state *st = iio_priv(indio_dev);
+ u16 val[AD5686_MAX_CHANNELS] = { };
+ unsigned int scan_count;
+ int ret, ch, i = 0;
+ bool async_update;
+ u8 cmd;
+
+ ret = iio_pop_from_buffer(buffer, val);
+ if (ret)
+ goto out_notify_done;
+
+ mutex_lock(&st->lock);
+
+ scan_count = bitmap_weight(indio_dev->active_scan_mask,
+ iio_get_masklength(indio_dev));
+ async_update = st->ldac_gpio && scan_count > 1;
+ if (async_update) {
+ /* use LDAC to update all channels simultaneously */
+ cmd = AD5686_CMD_WRITE_INPUT_N;
+ gpiod_set_value_cansleep(st->ldac_gpio, 0);
+ } else {
+ cmd = AD5686_CMD_WRITE_INPUT_N_UPDATE_N;
+ }
+
+ iio_for_each_active_channel(indio_dev, ch) {
+ ret = st->ops->write(st, cmd, indio_dev->channels[ch].address, val[i++]);
+ if (ret)
+ break;
+ }
+
+ /*
+ * If sync() is available, it is called here regardless of write
+ * failure to allow bus implementation to reset. In that case, partial
+ * writes are unlikely as the write operations would just queue up
+ * the transfers.
+ */
+ if (st->ops->sync)
+ ret = st->ops->sync(st); /* flush all pending transfers */
+
+ if (async_update)
+ gpiod_set_value_cansleep(st->ldac_gpio, 1);
+
+ mutex_unlock(&st->lock);
+out_notify_done:
+ iio_trigger_notify_done(indio_dev->trig);
+
+ return IRQ_HANDLED;
+}
+
int ad5686_probe(struct device *dev,
const struct ad5686_chip_info *chip_info,
const char *name, const struct ad5686_bus_ops *ops,
@@ -570,6 +629,13 @@ int ad5686_probe(struct device *dev,
return -EINVAL;
}
+ ret = devm_iio_triggered_buffer_setup_ext(dev, indio_dev, NULL,
+ &ad5686_trigger_handler,
+ IIO_BUFFER_DIRECTION_OUT,
+ NULL, NULL);
+ if (ret)
+ return ret;
+
return devm_iio_device_register(dev, indio_dev);
}
EXPORT_SYMBOL_NS_GPL(ad5686_probe, "IIO_AD5686");
--
2.43.0
^ permalink raw reply related
* [PATCH v3 09/12] iio: dac: ad5686: implement new sync() op for the spi bus
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Use of local SPI bus data to manage a collection of SPI transfers and
flush them to the SPI platform driver with the sync() operation. This
allows for faster handling of multiple channel DAC writes, avoiding kernel
overhead per spi_sync() call, which will be helpful when enabling
triggered buffer support.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/ad5686-spi.c | 121 ++++++++++++++++++++++++++++++++-----------
drivers/iio/dac/ad5686.c | 4 +-
drivers/iio/dac/ad5686.h | 8 ++-
drivers/iio/dac/ad5696-i2c.c | 2 +-
4 files changed, 100 insertions(+), 35 deletions(-)
diff --git a/drivers/iio/dac/ad5686-spi.c b/drivers/iio/dac/ad5686-spi.c
index 6b6ef1d7071f..77c86674d178 100644
--- a/drivers/iio/dac/ad5686-spi.c
+++ b/drivers/iio/dac/ad5686-spi.c
@@ -12,59 +12,91 @@
#include <linux/errno.h>
#include <linux/mod_devicetable.h>
#include <linux/module.h>
+#include <linux/overflow.h>
#include <linux/spi/spi.h>
#include <asm/byteorder.h>
#include "ad5686.h"
+/**
+ * struct ad5686_spi_data - SPI bus specific data
+ * @msg: SPI message used for transfers
+ * @size: number of transfers currently in the message
+ * @capacity: maximum number of transfers that can be added to the message
+ * @xfers: array of SPI transfers, allocated with the provided capacity
+ */
+struct ad5686_spi_data {
+ struct spi_message msg;
+ unsigned int size;
+ unsigned int capacity;
+ struct spi_transfer xfers[] __counted_by(capacity);
+};
+
static int ad5686_spi_write(struct ad5686_state *st,
u8 cmd, u8 addr, u16 val)
{
- struct spi_device *spi = to_spi_device(st->dev);
- u8 tx_len, *buf;
+ struct ad5686_spi_data *bus_data = st->bus_data;
+ struct spi_transfer *xfer;
+ if (bus_data->size >= bus_data->capacity)
+ return -E2BIG;
+
+ if (bus_data->size)
+ bus_data->xfers[bus_data->size - 1].cs_change = 1;
+ else
+ spi_message_init(&bus_data->msg);
+
+ xfer = &bus_data->xfers[bus_data->size];
switch (st->chip_info->regmap_type) {
case AD5310_REGMAP:
- st->data[0].d16 = cpu_to_be16(AD5310_CMD(cmd) |
- val);
- buf = &st->data[0].d8[0];
- tx_len = 2;
+ st->data[bus_data->size].d16 =
+ cpu_to_be16(AD5310_CMD(cmd) | val);
+ *xfer = (struct spi_transfer) {
+ .tx_buf = &st->data[bus_data->size].d16,
+ .len = sizeof(st->data[bus_data->size].d16),
+ };
break;
case AD5683_REGMAP:
- st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
- AD5683_DATA(val));
- buf = &st->data[0].d8[1];
- tx_len = 3;
+ st->data[bus_data->size].d32 =
+ cpu_to_be32(AD5686_CMD(cmd) | AD5683_DATA(val));
+ *xfer = (struct spi_transfer) {
+ .tx_buf = &st->data[bus_data->size].d8[1],
+ .len = sizeof(st->data[bus_data->size].d32) - 1,
+ };
break;
case AD5686_REGMAP:
- st->data[0].d32 = cpu_to_be32(AD5686_CMD(cmd) |
- AD5686_ADDR(addr) |
- val);
- buf = &st->data[0].d8[1];
- tx_len = 3;
+ st->data[bus_data->size].d32 =
+ cpu_to_be32(AD5686_CMD(cmd) | AD5686_ADDR(addr) | val);
+ *xfer = (struct spi_transfer) {
+ .tx_buf = &st->data[bus_data->size].d8[1],
+ .len = sizeof(st->data[bus_data->size].d32) - 1,
+ };
break;
default:
return -EINVAL;
}
- return spi_write(spi, buf, tx_len);
+ spi_message_add_tail(xfer, &bus_data->msg);
+ bus_data->size++;
+
+ return 0;
+}
+
+static int ad5686_spi_sync(struct ad5686_state *st)
+{
+ struct spi_device *spi = to_spi_device(st->dev);
+ struct ad5686_spi_data *bus_data = st->bus_data;
+
+ bus_data->size = 0; /* always reset, even on sync failure */
+ return spi_sync(spi, &bus_data->msg);
}
static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
{
- struct spi_transfer t[] = {
- {
- .tx_buf = &st->data[0].d8[1],
- .len = 3,
- .cs_change = 1,
- }, {
- .tx_buf = &st->data[1].d8[1],
- .rx_buf = &st->data[2].d8[1],
- .len = 3,
- },
- };
struct spi_device *spi = to_spi_device(st->dev);
+ struct ad5686_spi_data *bus_data = st->bus_data;
+ struct spi_transfer *xfer = &bus_data->xfers[0];
u8 cmd = 0;
int ret;
@@ -85,8 +117,21 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
AD5686_ADDR(addr));
st->data[1].d32 = cpu_to_be32(AD5686_CMD(AD5686_CMD_NOOP));
- ret = spi_sync_transfer(spi, t, ARRAY_SIZE(t));
- if (ret < 0)
+ xfer[0] = (struct spi_transfer) {
+ .tx_buf = &st->data[0].d8[1],
+ .len = sizeof(st->data[0].d32) - 1,
+ .cs_change = 1,
+ };
+ xfer[1] = (struct spi_transfer) {
+ .tx_buf = &st->data[1].d8[1],
+ .rx_buf = &st->data[2].d8[1],
+ .len = sizeof(st->data[1].d32) - 1,
+ };
+
+ spi_message_init_with_transfers(&bus_data->msg, xfer, 2);
+
+ ret = spi_sync(spi, &bus_data->msg);
+ if (ret)
return ret;
return be32_to_cpu(st->data[2].d32);
@@ -95,12 +140,26 @@ static int ad5686_spi_read(struct ad5686_state *st, u8 addr)
static const struct ad5686_bus_ops ad5686_spi_ops = {
.write = ad5686_spi_write,
.read = ad5686_spi_read,
+ .sync = ad5686_spi_sync,
};
static int ad5686_spi_probe(struct spi_device *spi)
{
- return ad5686_probe(&spi->dev, spi_get_device_match_data(spi),
- spi->modalias, &ad5686_spi_ops);
+ const struct ad5686_chip_info *info = spi_get_device_match_data(spi);
+ struct ad5686_spi_data *bus_data;
+ struct device *dev = &spi->dev;
+ unsigned int capacity;
+
+ /* read operation requires at least 2 transfers */
+ capacity = max(info->num_channels, 2);
+ bus_data = devm_kzalloc(dev, struct_size(bus_data, xfers, capacity),
+ GFP_KERNEL);
+ if (!bus_data)
+ return -ENOMEM;
+
+ bus_data->capacity = capacity;
+
+ return ad5686_probe(dev, info, spi->modalias, &ad5686_spi_ops, bus_data);
}
static const struct spi_device_id ad5686_spi_id[] = {
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 73105c197964..34011cbef903 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -472,7 +472,8 @@ EXPORT_SYMBOL_NS_GPL(ad5679r_chip_info, "IIO_AD5686");
int ad5686_probe(struct device *dev,
const struct ad5686_chip_info *chip_info,
- const char *name, const struct ad5686_bus_ops *ops)
+ const char *name, const struct ad5686_bus_ops *ops,
+ void *bus_data)
{
struct reset_control *rstc;
struct ad5686_state *st;
@@ -487,6 +488,7 @@ int ad5686_probe(struct device *dev,
st->dev = dev;
st->ops = ops;
+ st->bus_data = bus_data;
st->chip_info = chip_info;
ret = devm_regulator_get_enable(dev, "vdd");
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index 1da5f1b4d52a..6f47493906d4 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -23,6 +23,7 @@
#define AD5686_ADDR_DAC(chan) (0x1 << (chan))
#define AD5686_ADDR_ALL_DAC 0xF
+#define AD5686_MAX_CHANNELS 16
#define AD5686_CMD_NOOP 0x0
#define AD5686_CMD_WRITE_INPUT_N 0x1
@@ -129,6 +130,7 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
* @use_internal_vref: set to true if the internal reference voltage is used
* @lock: lock to protect access to state fields, which includes
* the data buffer during regmap ops
+ * @bus_data: bus specific data
* @data: transfer buffers
*/
struct ad5686_state {
@@ -141,6 +143,7 @@ struct ad5686_state {
unsigned int pwr_down_mode;
bool use_internal_vref;
struct mutex lock;
+ void *bus_data;
/*
* DMA (thus cache coherency maintenance) may require the
@@ -151,13 +154,14 @@ struct ad5686_state {
__be32 d32;
__be16 d16;
u8 d8[4];
- } data[3] __aligned(IIO_DMA_MINALIGN);
+ } data[AD5686_MAX_CHANNELS] __aligned(IIO_DMA_MINALIGN);
};
int ad5686_probe(struct device *dev,
const struct ad5686_chip_info *chip_info,
- const char *name, const struct ad5686_bus_ops *ops);
+ const char *name, const struct ad5686_bus_ops *ops,
+ void *bus_data);
static inline int ad5686_write(struct ad5686_state *st, u8 cmd, u8 addr, u16 val)
{
diff --git a/drivers/iio/dac/ad5696-i2c.c b/drivers/iio/dac/ad5696-i2c.c
index 279309329b64..28c97ded43ce 100644
--- a/drivers/iio/dac/ad5696-i2c.c
+++ b/drivers/iio/dac/ad5696-i2c.c
@@ -70,7 +70,7 @@ static const struct ad5686_bus_ops ad5686_i2c_ops = {
static int ad5686_i2c_probe(struct i2c_client *i2c)
{
return ad5686_probe(&i2c->dev, i2c_get_match_data(i2c),
- i2c->name, &ad5686_i2c_ops);
+ i2c->name, &ad5686_i2c_ops, NULL);
}
static const struct i2c_device_id ad5686_i2c_id[] = {
--
2.43.0
^ permalink raw reply related
* [PATCH v3 11/12] iio: dac: ad5686: read_raw/write_raw: use guard(mutex)()
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar,
Maxwell Doose, Joshua Crofts
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Use guarded mutex lock to facilitate code review when adding new
attributes. This will allow for early returns, avoiding error-prone
locking and unlocking in error paths. This also adds missing include
linux/cleanup.h. Gain-control support will allow the scale attribute
to be configurable.
Reviewed-by: Maxwell Doose <m32285159@gmail.com>
Reviewed-by: Joshua Crofts <joshua.crofts1@gmail.com>
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/ad5686.c | 19 ++++++++-----------
1 file changed, 8 insertions(+), 11 deletions(-)
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 9e8a014e5ac3..6ae788f665b4 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -8,6 +8,7 @@
#include <linux/array_size.h>
#include <linux/bitfield.h>
#include <linux/bitops.h>
+#include <linux/cleanup.h>
#include <linux/delay.h>
#include <linux/dev_printk.h>
#include <linux/errno.h>
@@ -181,11 +182,11 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
struct ad5686_state *st = iio_priv(indio_dev);
int ret;
+ guard(mutex)(&st->lock);
+
switch (m) {
case IIO_CHAN_INFO_RAW:
- mutex_lock(&st->lock);
ret = ad5686_read(st, chan->address);
- mutex_unlock(&st->lock);
if (ret < 0)
return ret;
*val = (ret >> chan->scan_type.shift) &
@@ -206,23 +207,19 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct ad5686_state *st = iio_priv(indio_dev);
- int ret;
+
+ guard(mutex)(&st->lock);
switch (mask) {
case IIO_CHAN_INFO_RAW:
if (val >= (1 << chan->scan_type.realbits) || val < 0)
return -EINVAL;
- mutex_lock(&st->lock);
- ret = ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
- chan->address, val << chan->scan_type.shift);
- mutex_unlock(&st->lock);
- break;
+ return ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
+ chan->address, val << chan->scan_type.shift);
default:
- ret = -EINVAL;
+ return -EINVAL;
}
-
- return ret;
}
static const struct iio_info ad5686_info = {
--
2.43.0
^ permalink raw reply related
* [PATCH v3 12/12] iio: dac: ad5686: add gain control support
From: Rodrigo Alencar via B4 Relay @ 2026-06-16 8:21 UTC (permalink / raw)
To: Michael Auchter, linux, linux-iio, devicetree, linux-kernel,
linux-hardening
Cc: Michael Hennerich, Jonathan Cameron, David Lechner,
Andy Shevchenko, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Philipp Zabel, Kees Cook, Gustavo A. R. Silva, Rodrigo Alencar
In-Reply-To: <20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com>
From: Rodrigo Alencar <rodrigo.alencar@analog.com>
Most of the supported devices rely on a GAIN pin to control a 2x
multiplier applied to the output voltage. Other devices, e.g. the
single-channel ones, provides a gain control through a bit field in
the control register. Some designs might have the GAIN pin hardwired
to VDD/VLOGIC or GND, which would have no "gain-gpios" device property,
being able to set "adi,range-double" if it is hardwired to VDD. The
vref_mv field is moved down in the struct ad5686_state, so that the
overall size increase is reduced.
Signed-off-by: Rodrigo Alencar <rodrigo.alencar@analog.com>
---
drivers/iio/dac/ad5686.c | 122 +++++++++++++++++++++++++++++++++++++++++++++--
drivers/iio/dac/ad5686.h | 12 ++++-
2 files changed, 127 insertions(+), 7 deletions(-)
diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
index 6ae788f665b4..c186213a46f6 100644
--- a/drivers/iio/dac/ad5686.c
+++ b/drivers/iio/dac/ad5686.c
@@ -15,10 +15,13 @@
#include <linux/export.h>
#include <linux/gpio/consumer.h>
#include <linux/kstrtox.h>
+#include <linux/math64.h>
#include <linux/module.h>
+#include <linux/property.h>
#include <linux/regulator/consumer.h>
#include <linux/reset.h>
#include <linux/sysfs.h>
+#include <linux/units.h>
#include <linux/wordpart.h>
#include <linux/iio/buffer.h>
@@ -41,7 +44,8 @@ static int ad5310_control_sync(struct ad5686_state *st)
return ad5686_write(st, AD5686_CMD_CONTROL_REG, 0,
FIELD_PREP(AD5310_PD_MSK, pd_val & AD5686_PD_MSK) |
- FIELD_PREP(AD5310_REF_BIT_MSK, st->use_internal_vref ? 0 : 1));
+ FIELD_PREP(AD5310_REF_BIT_MSK, st->use_internal_vref ? 0 : 1) |
+ FIELD_PREP(AD5310_GAIN_BIT_MSK, st->double_scale ? 1 : 0));
}
static int ad5683_control_sync(struct ad5686_state *st)
@@ -50,7 +54,8 @@ static int ad5683_control_sync(struct ad5686_state *st)
return ad5686_write(st, AD5686_CMD_CONTROL_REG, 0,
FIELD_PREP(AD5683_PD_MSK, pd_val & AD5686_PD_MSK) |
- FIELD_PREP(AD5683_REF_BIT_MSK, st->use_internal_vref ? 0 : 1));
+ FIELD_PREP(AD5683_REF_BIT_MSK, st->use_internal_vref ? 0 : 1) |
+ FIELD_PREP(AD5683_GAIN_BIT_MSK, st->double_scale ? 1 : 0));
}
static inline unsigned int ad5686_pd_mask_shift(const struct iio_chan_spec *chan)
@@ -193,9 +198,14 @@ static int ad5686_read_raw(struct iio_dev *indio_dev,
GENMASK(chan->scan_type.realbits - 1, 0);
return IIO_VAL_INT;
case IIO_CHAN_INFO_SCALE:
- *val = st->vref_mv;
- *val2 = chan->scan_type.realbits;
- return IIO_VAL_FRACTIONAL_LOG2;
+ if (st->double_scale) {
+ *val = st->scale_avail[2];
+ *val2 = st->scale_avail[3];
+ } else {
+ *val = st->scale_avail[0];
+ *val2 = st->scale_avail[1];
+ }
+ return IIO_VAL_INT_PLUS_NANO;
}
return -EINVAL;
}
@@ -207,6 +217,8 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
long mask)
{
struct ad5686_state *st = iio_priv(indio_dev);
+ bool double_scale;
+ int ret;
guard(mutex)(&st->lock);
@@ -217,6 +229,84 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
return ad5686_write(st, AD5686_CMD_WRITE_INPUT_N_UPDATE_N,
chan->address, val << chan->scan_type.shift);
+ case IIO_CHAN_INFO_SCALE:
+ if (val == st->scale_avail[0] && val2 == st->scale_avail[1])
+ double_scale = false;
+ else if (val == st->scale_avail[2] && val2 == st->scale_avail[3])
+ double_scale = true;
+ else
+ return -EINVAL;
+
+ if (st->double_scale == double_scale)
+ return 0; /* no change */
+
+ st->double_scale = double_scale;
+ switch (st->chip_info->regmap_type) {
+ case AD5310_REGMAP:
+ ret = ad5310_control_sync(st);
+ break;
+ case AD5683_REGMAP:
+ ret = ad5683_control_sync(st);
+ break;
+ case AD5686_REGMAP:
+ if (!st->gain_gpio) {
+ ret = -EINVAL;
+ break;
+ }
+
+ ret = gpiod_set_value_cansleep(st->gain_gpio,
+ st->double_scale ? 1 : 0);
+ break;
+ default:
+ ret = -EINVAL;
+ }
+ if (ret)
+ st->double_scale = !double_scale; /* revert on failure */
+ return ret;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad5686_write_raw_get_fmt(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ long mask)
+{
+ switch (mask) {
+ case IIO_CHAN_INFO_RAW:
+ return IIO_VAL_INT;
+ case IIO_CHAN_INFO_SCALE:
+ return IIO_VAL_INT_PLUS_NANO;
+ default:
+ return -EINVAL;
+ }
+}
+
+static int ad5686_read_avail(struct iio_dev *indio_dev,
+ struct iio_chan_spec const *chan,
+ const int **vals, int *type, int *length,
+ long mask)
+{
+ struct ad5686_state *st = iio_priv(indio_dev);
+
+ switch (mask) {
+ case IIO_CHAN_INFO_SCALE:
+ *type = IIO_VAL_INT_PLUS_NANO;
+
+ if (st->chip_info->regmap_type == AD5686_REGMAP && !st->gain_gpio) {
+ /*
+ * GAIN pin is board-strapped, so only the current
+ * scale is available.
+ */
+ *vals = st->double_scale ? &st->scale_avail[2] :
+ &st->scale_avail[0];
+ *length = 2;
+ return IIO_AVAIL_LIST;
+ }
+
+ *vals = st->scale_avail;
+ *length = ARRAY_SIZE(st->scale_avail);
+ return IIO_AVAIL_LIST;
default:
return -EINVAL;
}
@@ -225,6 +315,8 @@ static int ad5686_write_raw(struct iio_dev *indio_dev,
static const struct iio_info ad5686_info = {
.read_raw = ad5686_read_raw,
.write_raw = ad5686_write_raw,
+ .write_raw_get_fmt = ad5686_write_raw_get_fmt,
+ .read_avail = ad5686_read_avail,
};
static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
@@ -246,6 +338,7 @@ static const struct iio_chan_spec_ext_info ad5686_ext_info[] = {
.channel = chan, \
.info_mask_separate = BIT(IIO_CHAN_INFO_RAW), \
.info_mask_shared_by_type = BIT(IIO_CHAN_INFO_SCALE),\
+ .info_mask_shared_by_type_available = BIT(IIO_CHAN_INFO_SCALE),\
.address = addr, \
.scan_index = chan, \
.scan_type = { \
@@ -472,6 +565,15 @@ const struct ad5686_chip_info ad5679r_chip_info = {
};
EXPORT_SYMBOL_NS_GPL(ad5679r_chip_info, "IIO_AD5686");
+static void ad5686_init_scale_avail(struct ad5686_state *st)
+{
+ int realbits = st->chip_info->channels[0].scan_type.realbits;
+ s64 tmp = 2ULL * st->vref_mv * NANO >> realbits;
+
+ st->scale_avail[2] = div_s64_rem(tmp, NANO, &st->scale_avail[3]);
+ st->scale_avail[0] = div_s64_rem(tmp >> 1, NANO, &st->scale_avail[1]);
+}
+
static irqreturn_t ad5686_trigger_handler(int irq, void *p)
{
struct iio_poll_func *pf = p;
@@ -579,6 +681,14 @@ int ad5686_probe(struct device *dev,
return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
"Failed to get LDAC GPIO\n");
+ st->gain_gpio = devm_gpiod_get_optional(dev, "gain", GPIOD_OUT_LOW);
+ if (IS_ERR(st->gain_gpio))
+ return dev_err_probe(dev, PTR_ERR(st->gain_gpio),
+ "Failed to get GAIN GPIO\n");
+
+ st->double_scale = device_property_read_bool(dev, "adi,range-double");
+ ad5686_init_scale_avail(st);
+
reset_control_assert(rstc);
fsleep(1); /* reset pulse: comfortably bigger than the spec */
reset_control_deassert(rstc);
@@ -621,6 +731,8 @@ int ad5686_probe(struct device *dev,
st->use_internal_vref ? 0 : AD5686_REF_BIT_MSK);
if (ret)
return ret;
+
+ gpiod_set_value_cansleep(st->gain_gpio, st->double_scale ? 1 : 0);
break;
default:
return -EINVAL;
diff --git a/drivers/iio/dac/ad5686.h b/drivers/iio/dac/ad5686.h
index 6f47493906d4..b74a641d8fa4 100644
--- a/drivers/iio/dac/ad5686.h
+++ b/drivers/iio/dac/ad5686.h
@@ -39,9 +39,11 @@
#define AD5686_CMD_CONTROL_REG 0x4
#define AD5686_CMD_READBACK_ENABLE_V2 0x5
+#define AD5310_GAIN_BIT_MSK BIT(7)
#define AD5310_REF_BIT_MSK BIT(8)
#define AD5310_PD_MSK GENMASK(10, 9)
+#define AD5683_GAIN_BIT_MSK BIT(11)
#define AD5683_REF_BIT_MSK BIT(12)
#define AD5683_PD_MSK GENMASK(14, 13)
@@ -124,9 +126,12 @@ extern const struct ad5686_chip_info ad5679r_chip_info;
* @chip_info: chip model specific constants, available modes etc
* @ops: bus specific operations
* @ldac_gpio: LDAC pin GPIO descriptor
- * @vref_mv: actual reference voltage used
+ * @gain_gpio: GAIN pin GPIO descriptor
* @pwr_down_mask: power down mask
* @pwr_down_mode: current power down mode
+ * @scale_avail: pre-calculated available scale values
+ * @vref_mv: actual reference voltage used
+ * @double_scale: flag to indicate the gain multiplier is applied
* @use_internal_vref: set to true if the internal reference voltage is used
* @lock: lock to protect access to state fields, which includes
* the data buffer during regmap ops
@@ -138,9 +143,12 @@ struct ad5686_state {
const struct ad5686_chip_info *chip_info;
const struct ad5686_bus_ops *ops;
struct gpio_desc *ldac_gpio;
- unsigned short vref_mv;
+ struct gpio_desc *gain_gpio;
unsigned int pwr_down_mask;
unsigned int pwr_down_mode;
+ int scale_avail[4];
+ unsigned short vref_mv;
+ bool double_scale;
bool use_internal_vref;
struct mutex lock;
void *bus_data;
--
2.43.0
^ permalink raw reply related
* Re: [PATCH RFC v7 0/9] firmware: arm_scmi: vendors: Qualcomm Generic Vendor Extensions
From: Sudeep Holla @ 2026-06-16 8:27 UTC (permalink / raw)
To: Pragnesh Papaniya
Cc: Cristian Marussi, Rob Herring, Krzysztof Kozlowski, Sudeep Holla,
Conor Dooley, Sibi Sankar, MyungJoo Ham, Kyungmin Park,
Chanwoo Choi, Dmitry Osipenko, Thierry Reding, Jonathan Hunter,
Bjorn Andersson, Konrad Dybcio, Rajendra Nayak, Pankaj Patil,
linux-arm-msm, linux-kernel, arm-scmi, linux-arm-kernel,
devicetree, linux-pm, linux-tegra, Amir Vajid,
Ramakrishna Gottimukkula
In-Reply-To: <20260610-rfc_v7_scmi_memlat-v7-0-f3f68c608f25@oss.qualcomm.com>
On Wed, Jun 10, 2026 at 02:21:27PM +0530, Pragnesh Papaniya wrote:
> The QCOM SCMI vendor protocol provides a generic way of exposing a number of
> Qualcomm SoC specific features (like memory bus scaling) through a mixture of
> pre-determined algorithm strings and param_id pairs hosted on the SCMI
> controller. On Qualcomm Glymur and Hamoa SoCs, the memlat governor and the
> mechanism to control the various caches and RAM is hosted on the CPU Control
> Processor (CPUCP) and the method to tweak and start the governor is exposed
> through the QCOM SCMI Generic Extension Protocol.
>
> This series introduces the devfreq SCMI client driver that uses the MEMLAT
> algorithm string hosted on the QCOM SCMI Generic Extension Protocol to detect
> memory latency workloads and control frequency/level of the various memory
> buses (DDR/LLCC/DDR_QOS). DDR/LLCC/DDR_QOS are modelled as devfreq devices
> using the remote devfreq governor. This provides basic insight into device
> operation via trans_stat and lets userspace further tweak the parameters of
> the remote governor.
>
> trans_stat data for DDR/LLCC/DDR_QOS is now available with this series:
>
> From : To
> 315000000 479000000 545000000 725000000 840000000 959000000 1090000000 1211000000 time(ms)
> 315000000: 0 3 6 6 6 7 0 30 143956
> 479000000: 2 0 7 1 1 1 0 3 356
> 545000000: 7 6 0 5 5 0 0 10 1200
> 725000000: 3 0 5 0 6 1 0 6 2172
> 840000000: 8 2 3 2 0 4 0 12 1188
> 959000000: 3 0 1 2 2 0 0 13 272
> 1090000000: 0 0 0 0 0 0 0 0 0
> 1211000000: 35 4 11 5 11 8 0 0 21684
> Total transition : 253
>
> QCOM SCMI Generic Vendor protocol background:
> A lot of the vendor protocol numbers used internally were for
> debug/internal development purposes that were either highly SoC-specific
> or had to be disabled because some features were fused out during
> production. This led to a large number of vendor protocol numbers being
> quickly consumed and never released. Using a single generic vendor
> protocol with functionality abstracted behind algorithm strings gives us
> the flexibility of letting such functionality exist during initial
> development/debugging while still being able to expose mature features
> (like MEMLAT) once they have stabilised. The param_ids are expected to
> act as ABI for algorithm strings like MEMLAT.
>
Not sure if it was discussed in the previous versions or not, it would be
good if you can capture why some of bus scaling doesn't work with the existing
SCMI performance protocol and the monitors don't fit the MPAM mode.
Please capture them in 1/9 as a motivation for this vendor protocol. It will
then help to understand it better as I am still struggling to. Sorry for that.
--
Regards,
Sudeep
^ permalink raw reply
* Re: [PATCH v3 06/12] iio: dac: ad5686: consume optional reset signal
From: sashiko-bot @ 2026-06-16 8:30 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260616-ad5686-new-features-v3-6-f829fb7e9262@analog.com>
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing return value checks for `reset_control_assert()` and `reset_control_deassert()`.
- [High] The reset pin is left in a deasserted (driven) state on probe failure or device unbind, leading to potential back-powering of the DAC.
- [Medium] Acquiring the reset control after enabling regulators causes unnecessary power toggling on `-EPROBE_DEFER` and leaves the reset pin floating during power-up.
- [Medium] Missing recovery delay after deasserting the hardware reset may cause subsequent device communications to fail on fast CPUs.
--
commit c3eb5fda0704f698d1f8b2f76769857300b80132
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>
iio: dac: ad5686: consume optional reset signal
This commit adds GPIO support for the RESET pin using an optional
reset control local to the probe function. It manually generates
a reset pulse after the device is powered up.
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index fc3863274b291..a48acedff51c1 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
[ ... ]
> @@ -508,6 +510,15 @@ int ad5686_probe(struct device *dev,
>
> fsleep(5); /* power-up time */
>
> + rstc = devm_reset_control_get_optional_exclusive(dev, NULL);
[Severity: Medium]
Is it intentional to fetch the reset control after the regulators are
enabled and the 5µs power-up delay has occurred?
If devm_reset_control_get_optional_exclusive() returns -EPROBE_DEFER,
the driver will disable the regulators and have to retry the sequence
later. Fetching the reset control before enabling regulators might also
prevent the reset pin from floating during power-up.
> + if (IS_ERR(rstc))
> + return dev_err_probe(dev, PTR_ERR(rstc),
> + "Failed to get reset control\n");
> +
> + reset_control_assert(rstc);
[Severity: High]
Do we need to check the return value of reset_control_assert() and
reset_control_deassert()?
These functions can fail and return error codes, particularly if the
reset controller is behind a slow bus. Proceeding without checking
could cause the driver to communicate with a device that is still held
in reset.
> + fsleep(1); /* reset pulse: comfortably bigger than the spec */
> + reset_control_deassert(rstc);
[Severity: Medium]
Does the device need a guaranteed recovery delay after the reset is
deasserted?
Exiting the hardware reset initiates an internal power-on reset
sequence. On faster CPUs, the execution time before the first SPI/I2C
command might be shorter than the device's required reset recovery time.
[Severity: High]
Should there be a registered devm action to assert the reset pin on
driver removal or probe failure?
Currently, the reset pin is deasserted here but never asserted again on
cleanup. Since devm_regulator_get_enable() automatically disables the
VDD and VLOGIC regulators during cleanup, leaving this pin driven HIGH
while the device is unpowered could forward-bias ESD protection diodes
and back-power or damage the DAC.
> +
> /* Initialize masks to all ones */
> st->pwr_down_mask = ~0;
> st->pwr_down_mode = ~0;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=6
^ permalink raw reply
* Re: [PATCH v3 01/12] dt-bindings: iio: dac: ad5696: add reset/ldac/gain support
From: sashiko-bot @ 2026-06-16 8:32 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260616-ad5686-new-features-v3-1-f829fb7e9262@analog.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Incorrect use of `unevaluatedProperties: false` without an external schema `$ref`.
--
commit cd7d8addb3fad4751f6c895cad5073c1197387e2
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>
dt-bindings: iio: dac: ad5696: add reset/ldac/gain support
This commit adds GPIO properties for RESET, LDAC, and GAIN pins to the
adi,ad5696 DAC device tree binding. It introduces conditionally restricted
support for gain-gpios and adi,range-double based on the compatible string.
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> index b5a88b03dc2f0..c55158c464fde 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5696.yaml
[ ... ]
> required:
> - compatible
> - reg
>
> -additionalProperties: false
> +allOf:
> + - if:
> + properties:
> + compatible:
> + contains:
> + anyOf:
> + - const: adi,ad5311r
> + - const: adi,ad5691r
> + - const: adi,ad5692r
> + - const: adi,ad5693
> + - const: adi,ad5693r
> + then:
> + properties:
> + gain-gpios: false
> +
> +unevaluatedProperties: false
[Severity: Low]
This isn't a bug, but is it necessary to change additionalProperties to
unevaluatedProperties here? Because this schema does not reference any
external schemas via a $ref, and the conditionally restricted gain-gpios
property is already explicitly declared in the top-level properties block,
additionalProperties: false is sufficient and usually preferred when no
external schema references are present.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=1
^ permalink raw reply
* Re: [PATCH v3 04/12] dt-bindings: iio: dac: ad5686: rework on power supplies
From: sashiko-bot @ 2026-06-16 8:33 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay; +Cc: conor+dt, robh, devicetree
In-Reply-To: <20260616-ad5686-new-features-v3-4-f829fb7e9262@analog.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Making new power supply properties strictly required breaks backward compatibility for existing devicetrees.
--
commit 37c22112eb8071cacd8ef295c9d3096b1ae584af
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>
dt-bindings: iio: dac: ad5686: rework on power supplies
This commit introduces vdd-supply, vlogic-supply, and vref-supply properties
to the ad5686 binding while deprecating the misleading vcc-supply property.
The new power supply properties are added to the required lists to better
reflect the physical hardware requirements.
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
> index 7567de0ead4a3..bdc6bc425b489 100644
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
[ ... ]
> @@ -60,9 +74,23 @@ properties:
> required:
> - compatible
> - reg
> + - vdd-supply
> + - vlogic-supply
[Severity: High]
Does making these new properties unconditionally required break backward
compatibility for existing device trees? Older device trees that were valid
under the previous schema will now fail dtbs_check because they lack
vdd-supply and vlogic-supply.
> allOf:
> - $ref: /schemas/spi/spi-peripheral-props.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + anyOf:
> + - const: adi,ad5676
> + - const: adi,ad5683
> + - const: adi,ad5684
> + - const: adi,ad5686
> + then:
> + required:
> + - vref-supply
[Severity: High]
Will this strict requirement cause older device trees that rely on the now
deprecated vcc-supply to fail validation? Is there a way to allow either
vref-supply or the deprecated vcc-supply in an anyOf block so older dtb
files still pass validation while enforcing the requirement moving forward?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=4
^ permalink raw reply
* Re: [PATCH v3 12/12] iio: dac: ad5686: add gain control support
From: sashiko-bot @ 2026-06-16 8:34 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260616-ad5686-new-features-v3-12-f829fb7e9262@analog.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The GAIN GPIO is unconditionally requested as driven low, causing a potential DAC output glitch during probe.
--
commit 2f56ef5391a1b46c707191ed6f4562758e4e5acc
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>
iio: dac: ad5686: add gain control support
This commit adds gain control support to the AD5686 DAC driver. It handles
gain multiplication either via an external hardware GAIN pin or through
internal control register bits, initializing the default scale based on
device properties.
> diff --git a/drivers/iio/dac/ad5686.c b/drivers/iio/dac/ad5686.c
> index 6ae788f665b4d..c186213a46f6d 100644
> --- a/drivers/iio/dac/ad5686.c
> +++ b/drivers/iio/dac/ad5686.c
[ ... ]
> @@ -579,6 +681,14 @@ int ad5686_probe(struct device *dev,
> return dev_err_probe(dev, PTR_ERR(st->ldac_gpio),
> "Failed to get LDAC GPIO\n");
>
> + st->gain_gpio = devm_gpiod_get_optional(dev, "gain", GPIOD_OUT_LOW);
> + if (IS_ERR(st->gain_gpio))
> + return dev_err_probe(dev, PTR_ERR(st->gain_gpio),
> + "Failed to get GAIN GPIO\n");
> +
> + st->double_scale = device_property_read_bool(dev, "adi,range-double");
[Severity: Medium]
Could this cause an unwanted glitch on the analog output during probe?
If the system is warm-booted and the DAC is already outputting voltage with
a 2x gain, unconditionally forcing the gain pin low here will abruptly halve
the output voltage before the device is reset or the pin is corrected later in
the probe sequence.
Would it be safer to read the adi,range-double property before requesting the
GPIO, so that the pin can be initialized to its correct target state without
the temporary drop to 1x gain?
> + ad5686_init_scale_avail(st);
> +
> reset_control_assert(rstc);
> fsleep(1); /* reset pulse: comfortably bigger than the spec */
> reset_control_deassert(rstc);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=12
^ permalink raw reply
* Re: [PATCH v3 03/12] dt-bindings: iio: dac: ad5686: add reset/ldac/gain support
From: sashiko-bot @ 2026-06-16 8:35 UTC (permalink / raw)
To: Rodrigo Alencar via B4 Relay; +Cc: conor+dt, devicetree, robh
In-Reply-To: <20260616-ad5686-new-features-v3-3-f829fb7e9262@analog.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The conditional block restricts `gain-gpios` for single-channel variants but omits several multi-channel variants (e.g., `adi,ad5672r`, `adi,ad5676`, `adi,ad5674r`, `adi,ad5679r`) that lack a physical GAIN pin and internal multiplier. It also fails to restrict the `adi,range-double` property for these multi-channel variants.
--
commit aa16b66ecffaca7347b3cea2e8c47bd2390db0b1
Author: Rodrigo Alencar <rodrigo.alencar@analog.com>
dt-bindings: iio: dac: ad5686: add reset/ldac/gain support
This commit adds GPIO properties for RESET, LDAC, and GAIN pins to the
ad5686 device tree binding. It also adds an adi,range-double property to
indicate when the GAIN pin is hardwired high.
> diff --git a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
> --- a/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
> +++ b/Documentation/devicetree/bindings/iio/dac/adi,ad5686.yaml
[ ... ]
> allOf:
> - $ref: /schemas/spi/spi-peripheral-props.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + anyOf:
> + - const: adi,ad5310r
> + - const: adi,ad5681r
> + - const: adi,ad5682r
> + - const: adi,ad5683
> + - const: adi,ad5683r
> + then:
> + properties:
> + gain-gpios: false
>
> unevaluatedProperties: false
[Severity: Medium]
Does this conditional block also need to restrict gain-gpios and
adi,range-double for multi-channel variants that lack a physical GAIN pin?
Variants like adi,ad5672r, adi,ad5676, adi,ad5674r, and adi,ad5679r also
seem to lack this hardware pin and an internal multiplier. If omitted from
this list, the binding allows these properties to pass validation for
incompatible hardware.
Furthermore, if adi,range-double is provided for these multi-channel
variants, the driver might initialize with double_scale = true and
permanently report an incorrect 2x output voltage scale to userspace.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616-ad5686-new-features-v3-0-f829fb7e9262@analog.com?part=3
^ permalink raw reply
* Re: [PATCH v2 1/6] dt-bindings: clock: qcom: gcc-sm8450: Add missing power-domains property
From: Krzysztof Kozlowski @ 2026-06-16 8:36 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vinod Koul,
Abel Vesa, Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-1-6b5752dd4747@linaro.org>
On 15/06/2026 18:57, Neil Armstrong wrote:
> In order for the GCC votes on the GDSCs it provides to be propagated
> to CX, CX needs to be declared as power domain of the GCC.
I would be happier to see here short mentioning what is the effect of
missing votes - do things work? - as justification why required
property, thus ABI change, is justified.
The DTS patches gave me that answer, but should be here as well the best.
Well, don't want to stall anything here, so anyway:
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
>
> Document the missing power-domains property to that purpose.
>
> Fixes: 72a0ca203ca7 ("dt-bindings: clock: Add SM8450 GCC clock bindings")
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/6] dt-bindings: clock: qcom: sm8550-gcc: Add missing power-domains property
From: Krzysztof Kozlowski @ 2026-06-16 8:36 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vinod Koul,
Abel Vesa, Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-2-6b5752dd4747@linaro.org>
On 15/06/2026 18:57, Neil Armstrong wrote:
> In order for the GCC votes on the GDSCs it provides to be propagated
> to CX, CX needs to be declared as power domain of the GCC.
>
> Document the missing power-domains property to that purpose.
>
> Fixes: 47ba9c50bbeb ("dt-bindings: clock: Add SM8550 GCC clocks")
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
> Documentation/devicetree/bindings/clock/qcom,sm8550-gcc.yaml | 6 ++++++
> 1 file changed, 6 insertions(+)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 3/6] dt-bindings: clock: qcom: sm8650-gcc: Add missing power-domains property
From: Krzysztof Kozlowski @ 2026-06-16 8:36 UTC (permalink / raw)
To: Neil Armstrong, Bjorn Andersson, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Vinod Koul,
Abel Vesa, Konrad Dybcio, Sai Prakash Ranjan, Brian Masney
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <20260615-topic-sm8x50-tie-gcc-to-cx-v2-3-6b5752dd4747@linaro.org>
On 15/06/2026 18:57, Neil Armstrong wrote:
> In order for the GCC votes on the GDSCs it provides to be propagated
> to CX, CX needs to be declared as power domain of the GCC.
>
> Document the missing power-domains property to that purpose.
>
> Fixes: b69d932154dc ("dt-bindings: clock: qcom: document the SM8650 General Clock Controller")
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support
From: Andy Shevchenko @ 2026-06-16 8:36 UTC (permalink / raw)
To: David Lechner (TI)
Cc: Jonathan Cameron, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Kurt Borja, Nguyen Minh Tien,
linux-iio, devicetree, linux-kernel
In-Reply-To: <20260615-iio-adc-ti-ads122c14-v1-4-e6bdadf7cb2b@baylibre.com>
On Mon, Jun 15, 2026 at 05:00:02PM -0500, David Lechner (TI) wrote:
> Add support for parsing devicetree properties for measurement channels
> and doing direct reads on these.
>
> There are quite a lot of conditions that have to be met for each
> measurement to be made, so quite a bit of state and algorithms are
> required to handle it.
>
> Channels are created dynamically since the number of possibilities is
> unreasonably large.
...
> +struct ads112c14_measurement {
> + const char *label;
> + u32 vref_source;
> + u32 idac_current_uA;
> + u8 iadc_count;
> + u8 idac1_mux;
> + u8 idac2_mux;
> + bool current_chop;
> + bool bipolar;
> + u8 gain_val;
Hmm... Can we group u8:s for a better visual?
> + int scale_available[ARRAY_SIZE(ads112c14_pga_gains_x10)][2];
> +};
...
> struct ads112c14_data {
> const struct ads112c14_chip_info *chip_info;
> struct i2c_client *client;
> struct regmap *regmap;
> + u32 avdd_uV;
> + u32 ext_ref_uV;
> + bool refp_is_avdd;
> + bool refn_is_gnd;
> + u32 ext_ref_ohms;
> + struct ads112c14_measurement *measurements;
> + u32 num_measurements;
> u8 sys_mon_chan_short_gain_val;
If you run `pahole` you will see a gap that can be easily avoided.
> int sys_mon_chan_short_scale_available[ARRAY_SIZE(ads112c14_pga_gains_x10)][2];
> };
...
> +static int ads112c14_prepare_measurement_channel(struct ads112c14_data *data,
> + const struct iio_chan_spec *chan)
> +{
> + struct ads112c14_measurement *measurement = &data->measurements[chan->scan_index];
> + bool iunit = measurement->idac_current_uA > 100;
> + u32 idac1_val = measurement->iadc_count > 0 ?
> + measurement->idac_current_uA / (iunit ? 100 : 10) : 0;
> + u32 idac2_val = measurement->iadc_count > 1 ? idac1_val : 0;
These three will look much better when decoupled from the definitions.
> + u32 refp_buf_en, refn_buf_en;
> + u32 ref_val, ref_sel;
> + int ret;
iunit = measurement->idac_current_uA > 100;
if (measurement->iadc_count > 0)
idac1_val = measurement->idac_current_uA / (iunit ? 100 : 10);
else
idac1_val = 0;
idac2_val = measurement->iadc_count > 1 ? idac1_val : 0;
> + ret = regmap_write(data->regmap, ADS112C14_REG_MUX_CFG,
> + FIELD_PREP(ADS112C14_MUX_CFG_AINP, chan->channel) |
> + FIELD_PREP(ADS112C14_MUX_CFG_AINN, chan->channel2));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(data->regmap, ADS112C14_REG_DIGITAL_CFG,
> + ADS112C14_DIGITAL_CFG_CODING,
> + FIELD_PREP(ADS112C14_DIGITAL_CFG_CODING,
> + measurement->bipolar ? 0 : 1));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(data->regmap, ADS112C14_REG_GAIN_CFG,
> + ADS112C14_GAIN_CFG_SYS_MON | ADS112C14_GAIN_CFG_GAIN,
> + FIELD_PREP(ADS112C14_GAIN_CFG_SYS_MON, 0) |
> + FIELD_PREP(ADS112C14_GAIN_CFG_GAIN,
> + measurement->gain_val));
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, ADS112C14_REG_IDAC_MAG_CFG,
> + FIELD_PREP(ADS112C14_IDAC_MAG_CFG_I2MAG, idac2_val) |
> + FIELD_PREP(ADS112C14_IDAC_MAG_CFG_I1MAG, idac1_val));
> + if (ret)
> + return ret;
> +
> + ret = regmap_write(data->regmap, ADS112C14_REG_IDAC_MUX_CFG,
> + (iunit ? ADS112C14_IDAC_MUX_CFG_IUNIT : 0) |
> + FIELD_PREP(ADS112C14_IDAC_MUX_CFG_I2MUX,
> + measurement->idac2_mux) |
> + FIELD_PREP(ADS112C14_IDAC_MUX_CFG_I1MUX,
> + measurement->idac1_mux));
> + if (ret)
> + return ret;
> +
> + ret = regmap_update_bits(data->regmap, ADS112C14_REG_DATA_RATE_CFG,
> + ADS112C14_DATA_RATE_CFG_GC_EN,
> + FIELD_PREP(ADS112C14_DATA_RATE_CFG_GC_EN,
> + measurement->current_chop));
> + if (ret)
> + return ret;
> +
> + refp_buf_en = !data->refp_is_avdd &&
> + measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL;
> + refn_buf_en = !data->refn_is_gnd &&
> + measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL;
> +
> + ref_val = measurement->vref_source == ADS112C14_VREF_SOURCE_INTERNAL_2_5V;
> +
> + switch (measurement->vref_source) {
> + case ADS112C14_VREF_SOURCE_AVDD:
> + ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_AVDD;
> + break;
> + case ADS112C14_VREF_SOURCE_EXTERNAL:
> + ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_EXTERNAL;
> + break;
> + default:
> + ref_sel = ADS112C14_REFERENCE_CFG_REF_SEL_INTERNAL;
> + break;
> + }
> +
> + return regmap_update_bits(data->regmap, ADS112C14_REG_REFERENCE_CFG,
> + ADS112C14_REFERENCE_CFG_REFP_BUF_EN |
> + ADS112C14_REFERENCE_CFG_REFN_BUF_EN |
> + ADS112C14_REFERENCE_CFG_REF_VAL |
> + ADS112C14_REFERENCE_CFG_REF_SEL,
> + FIELD_PREP(ADS112C14_REFERENCE_CFG_REFP_BUF_EN,
> + refp_buf_en) |
> + FIELD_PREP(ADS112C14_REFERENCE_CFG_REFN_BUF_EN,
> + refn_buf_en) |
> + FIELD_PREP(ADS112C14_REFERENCE_CFG_REF_VAL,
> + ref_val) |
> + FIELD_PREP(ADS112C14_REFERENCE_CFG_REF_SEL,
> + ref_sel));
> +}
...
> - if (chan->channel >= 100) {
> - ret = ads112c14_prepare_sys_mon_channel(data, chan);
> + if (chan->channel < 100) {
> + ret = ads112c14_prepare_measurement_channel(data, chan);
> if (ret)
> return ret;
> } else {
> - /* Not implemented yet. */
> - return -EINVAL;
> + ret = ads112c14_prepare_sys_mon_channel(data, chan);
> + if (ret)
> + return ret;
> }
Why ping-ponging this? Leave the condition as is. Or start with the < 100 in
the previous patch.
...
> + if (chan->channel < 100) {
For the consistency's sake keep it the similar to the above.
> + measurement = &data->measurements[chan->scan_index];
> + fsr_bits = data->chip_info->resolution_bits - measurement->bipolar;
> + } else {
> + /* All SYS_MON channels are using signed coding. */
> + fsr_bits = data->chip_info->resolution_bits - 1;
> + }
...
> if (chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT) {
> - int *scale_avail = data->sys_mon_chan_short_scale_available[
> + scale_avail = data->sys_mon_chan_short_scale_available[
> data->sys_mon_chan_short_gain_val
> ];
Oh, can we simply move the index to the temporary variable and make this look
better?
...
> - for (u32 i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
> - if (val == data->sys_mon_chan_short_scale_available[i][0] &&
> - val2 == data->sys_mon_chan_short_scale_available[i][1]) {
> - data->sys_mon_chan_short_gain_val = i;
> - return 0;
> - }
> + for (u32 i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
> + if (val == scale_avail[i][0] && val2 == scale_avail[i][1]) {
> + *gain_val = i;
> + return 0;
> }
> - return -EINVAL;
> - }
Can this be written in a better form initially to make less churn here?
...
> + /* measurement channels */
> + if (chan->channel < 100) {
> + struct ads112c14_measurement *measurement =
> + &data->measurements[chan->scan_index];
> + if (!measurement->label)
> + return -EINVAL;
Hmm... Can it be true?
> + return sysfs_emit(label, "%s\n", measurement->label);
> + }
...
> +static int ads112c14_parse_channels(struct iio_dev *indio_dev,
> + bool *need_avdd_ref, bool *need_ext_ref)
> +{
> + struct ads112c14_data *data = iio_priv(indio_dev);
> + struct device *dev = &data->client->dev;
> + struct iio_chan_spec *channels;
> + u32 pair[2];
> + int i = 0;
Why signed? Also decouple assignment from definition.
> + int ret;
> +
> + *need_avdd_ref = false;
> + *need_ext_ref = false;
> +
> + data->num_measurements = device_get_named_child_node_count(dev, "channel");
> + data->measurements = devm_kcalloc(dev, data->num_measurements,
> + sizeof(*data->measurements), GFP_KERNEL);
> + if (!data->measurements)
> + return -ENOMEM;
> +
> + data->num_measurements += ARRAY_SIZE(ads112c14_sys_mon_channels);
> +
> + channels = devm_kcalloc(dev, data->num_measurements +
> + ARRAY_SIZE(ads112c14_sys_mon_channels),
size_add()? But here you add it twice, why?
> + sizeof(*channels), GFP_KERNEL);
> + if (!channels)
> + return -ENOMEM;
> +
> + device_for_each_named_child_node_scoped(dev, child, "channel") {
> + struct ads112c14_measurement *measurement = &data->measurements[i];
> + struct iio_chan_spec *spec = &channels[i];
> +
> + if (!fwnode_device_is_available(child))
> + continue;
> +
> + spec->type = IIO_VOLTAGE;
> + spec->indexed = 1;
> + spec->scan_index = i;
> + measurement->gain_val = 1;
> +
> + fwnode_property_read_string(child, "label", &measurement->label);
> + if (fwnode_property_present(child, "single-channel")) {
> + ret = fwnode_property_read_u32(child, "single-channel", &spec->channel);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read single-channel property\n");
> +
> + if (spec->channel >= 8)
> + return dev_err_probe(dev, -EINVAL,
> + "single-channel value must be between 0 and 7\n");
> + } else if (fwnode_property_present(child, "diff-channels")) {
> + ret = fwnode_property_read_u32_array(child, "diff-channels", pair, ARRAY_SIZE(pair));
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read diff-channels property\n");
> +
> + if (pair[0] >= 8 || pair[1] >= 8)
> + return dev_err_probe(dev, -EINVAL,
> + "diff-channels values must be between 0 and 7\n");
> +
> + spec->channel = pair[0];
> + spec->channel2 = pair[1];
> + spec->differential = 1;
> + } else {
> + return dev_err_probe(dev, -EINVAL,
> + "channel node missing channel type property\n");
> + }
Looking how it's going to spread (I mean the above pattern), perhaps it's a time to introduce bunch of
fwnode_property_read_*_optional()
and the respective device_property_read_*_optional()?
Let's start from u32 case only, as it will be most used anyway.
> + if (fwnode_property_present(child, "excitation-channels")) {
> + ret = fwnode_property_count_u32(child, "excitation-channels");
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "failed to read excitation-channels property\n");
> +
> + if (ret < 1 || ret > 2)
> + return dev_err_probe(dev, -EINVAL,
> + "excitation-channels property must have 1 or 2 values\n");
> +
> + measurement->iadc_count = ret;
> + pair[1] = 0;
> +
> + ret = fwnode_property_read_u32_array(child, "excitation-channels", pair, measurement->iadc_count);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read excitation-channels property\n");
> +
> + if (pair[0] >= 8 || pair[1] >= 8)
> + return dev_err_probe(dev, -EINVAL,
> + "excitation-channels values must be between 0 and 7\n");
> +
> + measurement->idac1_mux = pair[0];
> + measurement->idac2_mux = measurement->iadc_count > 1 ? pair[1] : 0;
> +
> + ret = fwnode_property_read_u32(child, "excitation-current-microamp",
> + &measurement->idac_current_uA);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read excitation-current-microamp property\n");
> +
> + measurement->current_chop = fwnode_property_read_bool(child, "current-chopping");
> + }
> +
> + measurement->bipolar = fwnode_property_read_bool(child, "bipolar");
> +
> + fwnode_property_read_u32(child, "ti,vref-source", &measurement->vref_source);
> + if (measurement->vref_source > ADS112C14_VREF_SOURCE_AVDD)
> + return dev_err_probe(dev, -EINVAL,
> + "invalid vref-source value\n");
> +
> + if (measurement->vref_source == ADS112C14_VREF_SOURCE_AVDD)
> + *need_avdd_ref = true;
> + if (measurement->vref_source == ADS112C14_VREF_SOURCE_EXTERNAL)
> + *need_ext_ref = true;
> +
> + spec->info_mask_separate = BIT(IIO_CHAN_INFO_RAW) | BIT(IIO_CHAN_INFO_SCALE);
> + spec->info_mask_separate_available = BIT(IIO_CHAN_INFO_SCALE);
> +
> + i++;
> + }
> +
> + memcpy(channels + i, ads112c14_sys_mon_channels, sizeof(ads112c14_sys_mon_channels));
> +
> + indio_dev->channels = channels;
> + indio_dev->num_channels = i + ARRAY_SIZE(ads112c14_sys_mon_channels);
> +
> + return 0;
> +}
...
> +static void ads112c14_populate_scale_available(int scale_avail[][2],
> + u32 vref_uV, u32 fsr_bits)
> +{
> + for (u32 i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
> + int *entry = scale_avail[i];
> + u32 gain_x10 = ads112c14_pga_gains_x10[i];
> +
> + entry[0] = div_u64_rem(div64_u64((u64)(NANO * 10 /
> + (MICRO / MILLI)) * vref_uV,
> + (u64)gain_x10 * BIT(fsr_bits)),
Hmm... This differs from the previous implementation. Why?
> + NANO, &entry[1]);
> + }
> +}
...
> + if (device_property_present(dev, "refp-refn-resistor-ohms")) {
> + if (refp_uV != 0 || refn_uV != 0)
> + return dev_err_probe(dev, -EINVAL,
> + "refp-refn-resistor-ohms property should not be present when refp-supply or refn-supply is present\n");
> +
> + ret = device_property_read_u32(dev, "refp-refn-resistor-ohms",
> + &data->ext_ref_ohms);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read refp-refn-resistor-ohms property\n");
Using
const char *propname;
...
propname = "refp-refn-resistor-ohms";
makes this
if (device_property_present(dev, propname)) {
if (refp_uV != 0 || refn_uV != 0)
return dev_err_probe(dev, -EINVAL,
"%s property should not be present when refp-supply or refn-supply is present\n",
propname);
ret = device_property_read_u32(dev, propname, &data->ext_ref_ohms);
if (ret)
return dev_err_probe(dev, ret, "failed to read %s property\n", propname);
Also the rest can be improved in the similar way.
> + } else {
> + if (need_ext_ref && data->ext_ref_uV == 0)
> + return dev_err_probe(dev, -EINVAL,
> + "external reference measurements require either refp-supply or refp-refn-resistor-ohms property\n");
> + }
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v2 1/6] dt-bindings: clock: qcom: gcc-sm8450: Add missing power-domains property
From: Neil Armstrong @ 2026-06-16 8:38 UTC (permalink / raw)
To: Krzysztof Kozlowski, Bjorn Andersson, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vinod Koul, Abel Vesa, Konrad Dybcio, Sai Prakash Ranjan,
Brian Masney
Cc: linux-arm-msm, linux-clk, devicetree, linux-kernel
In-Reply-To: <1d4f3e4e-ec9e-44d0-9e4a-651b6920015c@kernel.org>
On 6/16/26 10:36, Krzysztof Kozlowski wrote:
> On 15/06/2026 18:57, Neil Armstrong wrote:
>> In order for the GCC votes on the GDSCs it provides to be propagated
>> to CX, CX needs to be declared as power domain of the GCC.
>
> I would be happier to see here short mentioning what is the effect of
> missing votes - do things work? - as justification why required
> property, thus ABI change, is justified.
>
> The DTS patches gave me that answer, but should be here as well the best.
>
> Well, don't want to stall anything here, so anyway:
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Thanks I'll update the commit message with the same explanation as
the DT changes.
Thanks,
Neil
>
>>
>> Document the missing power-domains property to that purpose.
>>
>> Fixes: 72a0ca203ca7 ("dt-bindings: clock: Add SM8450 GCC clock bindings")
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>
>
>
>
> Best regards,
> Krzysztof
^ permalink raw reply
* Re: [PATCH v2] dt-bindings: display: Add Solomon SSD1351 OLED controller
From: Javier Martinez Canillas @ 2026-06-16 8:39 UTC (permalink / raw)
To: Amit Barzilai, robh, krzk+dt, conor+dt
Cc: devicetree, dri-devel, linux-kernel, airlied, maarten.lankhorst,
mripard, simona, tzimmermann, Amit Barzilai
In-Reply-To: <20260615175620.88828-1-amit.barzilai22@gmail.com>
Amit Barzilai <amit.barzilai22@gmail.com> writes:
Hello Amit,
> Add a device tree binding for the Solomon SSD1351, a 128x128 65k-color
> RGB OLED display controller driven over a 4-wire SPI bus. The binding
> builds on the shared solomon,ssd-common.yaml properties already used by
> the other Solomon display controllers.
>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
> ---
> Changes since v1:
> - Drop solomon,width / solomon,height: both are deducible from the
> compatible and are already declared (as optional) by the referenced
> solomon,ssd-common.yaml, so a local override is unnecessary.
> - Drop the rotation property: it has no consumer (rotation is being removed from the driver).
> - Use dt-bindings/gpio/gpio.h flag defines in the example
> (reset-gpios active-low, dc-gpios active-high).
>
> .../bindings/display/solomon,ssd1351.yaml | 41 +++++++++++++++++++
> 1 file changed, 41 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
>
> diff --git a/Documentation/devicetree/bindings/display/solomon,ssd1351.yaml b/Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
> new file mode 100644
> index 000000000000..39622d74a41d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/solomon,ssd1351.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/solomon,ssd1351.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Solomon SSD1351 OLED Display Controller
> +
> +maintainers:
> + - Amit Barzilai <amit.barzilai22@gmail.com>
> +
Feel free to add me as a maintainer of this binding too since I
maintain the ssd130x driver.
It looks good to me.
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
--
Best regards,
Javier Martinez Canillas
Core Platforms
Red Hat
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox