* 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
* [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
* [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 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 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 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 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 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 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 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 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 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 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 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
* Re: [PATCH v3 1/2] cpufreq: spacemit: Add K1 cpufreq driver
From: Viresh Kumar @ 2026-06-16 8:19 UTC (permalink / raw)
To: Shuwei Wu
Cc: Rafael J. Wysocki, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
Yixun Lan, linux-pm, linux-kernel, linux-riscv, spacemit,
devicetree
In-Reply-To: <DJ9M2IDYE6XZ.20RN5AFBYNB9Y@mailbox.org>
On 15-06-26, 20:11, Shuwei Wu wrote:
> With two cpufreq-dt policies (one per cluster) and these ranges, neither cluster
> blocks the other. Tested on BPI-F3 and OrangePi Rv2 boards, works as expected.
>
> Does this look good to you, or would you prefer a different approach?
Looks correct.
--
viresh
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: mfd: s2mu005-pmic: reorder reg and interrupts properties
From: Krzysztof Kozlowski @ 2026-06-16 8:05 UTC (permalink / raw)
To: Kaustabh Chakraborty, André Draszik, Lee Jones, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Pavel Machek
Cc: linux-kernel, linux-samsung-soc, devicetree, linux-leds
In-Reply-To: <DJA938R30SZG.7WGSHHZHP07X@disroot.org>
On 16/06/2026 08:13, Kaustabh Chakraborty wrote:
> On 2026-06-16 06:14 +02:00, Krzysztof Kozlowski wrote:
>> On 15/06/2026 22:26, Kaustabh Chakraborty wrote:
>>> As per convention, and as also reiterated by maintainers [1], the
>>> properties in schema is to be ordered similar to how its done in
>>> devicetree sources; starting from compatible and reg. Re-order the
>>> properties in this schema accordingly.
>>>
>>> Link: https://lore.kernel.org/all/0240eb13-6c56-4879-8db7-b990a220a78f@kernel.org [1]
>>> Suggested-by: Krzysztof Kozlowski <krzk@kernel.org>
>>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>>
>> Honestly, nah... I commented on v6 so you change the patch. But you were
>> posting this huge patchset faster than we can review (v6 and v7 posted
>> on the same day!), so v7 got applied where you did not implement the
>> comments. One small posting per 24h. One big posting per 2-3 days, not
>> more often.
>
> Fair, there were a lot of sashiko reviews, so I quickly addressed most
> of them and send a v7. It is indeed a failure on my part. :(
>
>> There is little benefit in fixing this single file.
>
> Fine, I drop the series. I'd assumed it'd be at least fine to have it
> before a stable release.
I would ack such patch if you do it rather for multiple files in MFD
bindings, e.g. all Samsung PMICs or even all files. Otherwise it feels a
bit too much of a churn doing this file by file.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 3/4] iio: adc: ti-ads112c14: implement gain on internal short SYS_MON channel
From: Andy Shevchenko @ 2026-06-16 7:58 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-3-e6bdadf7cb2b@baylibre.com>
On Mon, Jun 15, 2026 at 05:00:01PM -0500, David Lechner (TI) wrote:
> Implement support for the programmable gain amplifier on the internal
> short SYS_MON channel. This channel is used for calibration, so it is
> useful to be able to set the PGA to the same gain as the external
> channels. The gain setting is implemented via the `_scale` attribute.
>
> In the future, we may want to support different reference voltages for
> this channel, so the scale_available table is populated during probe
> rather than being a static table.
...
> + switch (chan->channel) {
> + case ADS112C14_SYS_MON_CHANNEL_SHORT: {
> + IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> + if (IIO_DEV_ACQUIRE_FAILED(claim))
> + return -EBUSY;
> +
> + for (u32 i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
u32 here looks confusing. the entire loop can be made readable with a temporary
for the array item I believe.
> + 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 (size_t i = 0; i < ARRAY_SIZE(data->sys_mon_chan_short_scale_available); i++) {
const int *sa = data->sys_mon_chan_short_scale_available[i];
if (val == sa[0] && val2 == sa[1]) {
data->sys_mon_chan_short_gain_val = i;
return 0;
}
}
> + return -EINVAL;
> + }
> + default:
> + return -EINVAL;
> + }
> +}
...
> +static void ads112c14_populate_tables(struct ads112c14_data *data)
> +{
> + u32 vref_uV, fsr_bits;
> + int i;
size_t? unsigned int?
> + /* For now, assuming we are using 2.5V reference. */
> + vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
> + fsr_bits = data->chip_info->resolution_bits - 1;
> +
> + for (i = 0; i < ARRAY_SIZE(ads112c14_pga_gains_x10); i++) {
> + int *scale_avail = &data->sys_mon_chan_short_scale_available[i][0];
> + u32 gain_x10 = ads112c14_pga_gains_x10[i];
> +
> + /* NB: slightly odd arrangement to avoid overflow. */
> + scale_avail[0] = div_u64_rem(div_u64((u64)NANO * 10 /
> + (MICRO / MILLI) * vref_uV /
> + gain_x10,
> + BIT(fsr_bits)),
> + NANO, &scale_avail[1]);
Oh, what about temporary variable for the inner division? Also note one trick
to avoid casting (and making it shorter).
u64 foo;
foo = div_u64(10ULL * NANO / (MICRO / MILLI) * vref_uV / gain_x10, BIT(fsr_bits));
/* NB: slightly odd arrangement to avoid overflow. */
scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);
Now, with much more readability, it's visible that the first division is just a right shift.
u64 foo;
/* ...a comment to explain voodoo calculations... */
foo = (10ULL * NANO / (MICRO / MILLI) * vref_uV / gain_x10) >> fsr_bits;
scale_avail[0] = div_u64_rem(foo, NANO, &scale_avail[1]);
> + }
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v3 2/2] clk: amlogic: Add A9 peripherals clock controller driver
From: Jerome Brunet @ 2026-06-16 7:51 UTC (permalink / raw)
To: Jian Hu
Cc: Jian Hu via B4 Relay, Neil Armstrong, Michael Turquette,
Stephen Boyd, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Xianwei Zhao, Kevin Hilman, Martin Blumenstingl, linux-amlogic,
linux-clk, devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <5601fe65-777b-4db0-a6e5-8d2cdcde7e53@amlogic.com>
On mar. 16 juin 2026 at 14:12, Jian Hu <jian.hu@amlogic.com> wrote:
>>>
>>> If you think splitting it further into separate helper macros would improve
>>> readability.
>> One clock per macro please. Hidding 2 declaration is recipe for
>> disaster. For ex, here the first one is static, the 2nd is not
>
>
> I'll split it into separate helper macros so that each macro expands to a
> single clock definition.
>
> They are defined as follows: (Excluding struct clk_regmap)
>
> #define A9_VCLK_GATE(_name, _reg, _bit, _parent) \
> .data = &(struct clk_regmap_gate_data){ \
> .offset = _reg, \
> .bit_idx = _bit, \
> }, \
> .hw.init = &(struct clk_init_data) { \
> .name = #_name "_en", \
> .ops = &clk_regmap_gate_ops, \
> .parent_hws = (const struct clk_hw *[]) { _parent }, \
> .num_parents = 1, \
> .flags = CLK_SET_RATE_PARENT, \
> },
>
> #define A9_VCLK_DIV(_name, _reg, _div) \
>
> ....
>
> static struct clk_regmap a9_vclk_div2_en = {
> A9_VCLK_GATE(vclk_div2, VID_CLK_CTRL, 1, &a9_vclk.hw),
> };
>
>
> static struct clk_regmap a9_vclk_div2 = {
> A9_VCLK_DIV(vclk_div2, VID_CLK_CTRL, 2),
> };
>
> My understanding is that you would prefer helper macros to cover only the
> repeated initializer fields,
> while keeping the actual clock declarations explicit.
I do not have a definitive preference over this but I do want things to be
consistent, at least within the driver, globaly whenever possible.
Look at the other macros you have already defined in your driver and do
the same thing, including the way you declare the variable. Apart from
this, it seems fine.
>
> If that's not what you had in mind, please let me know.
>>> I can do that as well.
>>>
--
Jerome
^ permalink raw reply
* Re: [PATCH v7 3/5] iio: adc: versal-sysmon: add I2C driver
From: Andy Shevchenko @ 2026-06-16 7:45 UTC (permalink / raw)
To: Erim, Salih
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <15d14eee-8384-4605-ad83-5aad774541ac@amd.com>
On Mon, Jun 15, 2026 at 04:42:43PM +0100, Erim, Salih wrote:
> On 15/06/2026 15:30, Andy Shevchenko wrote:
> > On Mon, Jun 15, 2026 at 12:37:20AM +0100, Salih Erim wrote:
...
> > > +static const struct regmap_config sysmon_i2c_regmap_config = {
> > > + .reg_bits = 32,
> > > + .val_bits = 32,
> > > + .reg_stride = SYSMON_REG_STRIDE,
> > > + .max_register = SYSMON_MAX_REG,
> > > + .reg_read = sysmon_i2c_reg_read,
> > > + .reg_write = sysmon_i2c_reg_write,
> > > +};
> >
> > No cache?
>
> No, the registers are live ADC readings and interrupt status.
> Caching would return stale voltage and temperature data.
So, basically what you are saying is this:
"All registers are volatile in this HW."
Or alternatively:
"Almost all registers are volatile in this HW. The rest is not being accessed
too often to cache."
Choose the one that fits and add on top of this regmap_config initialiser.
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH v7 2/5] iio: adc: add Versal SysMon driver
From: Andy Shevchenko @ 2026-06-16 7:43 UTC (permalink / raw)
To: Erim, Salih
Cc: jic23, andy, dlechner, nuno.sa, robh, krzk+dt, conor+dt,
conall.ogriofa, michal.simek, linux, erimsalih, linux-iio,
devicetree, linux-kernel
In-Reply-To: <2998a9fc-4b9f-49bd-8020-697081401f85@amd.com>
On Mon, Jun 15, 2026 at 04:41:12PM +0100, Erim, Salih wrote:
> On 15/06/2026 15:22, Andy Shevchenko wrote:
> > On Mon, Jun 15, 2026 at 12:37:19AM +0100, Salih Erim wrote:
...
> > > +/**
> > > + * sysmon_core_probe() - Initialize Versal SysMon core
> >
> > It is managed, please name it accordingly: devm_sysmon_core_probe().
>
> Will rename to devm_sysmon_core_probe() and update callers.
I believe you also want to have a proper namespace. The sysmon is to broad.
The easiest solution is to name it as devm_versal_sysmon_core_probe().
> > > + * @dev: Parent device
> > > + * @regmap: Register map for hardware access
> > > + *
> > > + * Return: 0 on success, negative errno on failure.
> > > + */
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v5 6/6] iio: osf: register IIO devices from capabilities
From: sashiko-bot @ 2026-06-16 7:38 UTC (permalink / raw)
To: Jinseob Kim; +Cc: robh, devicetree, conor+dt
In-Reply-To: <20260616072242.3942-7-kimjinseob88@gmail.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing synchronization between asynchronous IIO buffer push and buffer enable/disable operations, leading to list corruption.
- [High] Uninitialized padding bytes in the IIO bounce buffer leak kernel heap memory to user-space.
--
commit e42cf74aa62314d9b92e389435a75cfcd58b0820
Author: Jinseob Kim <kimjinseob88@gmail.com>
iio: osf: register IIO devices from capabilities
Register IIO devices for supported Open Sensor Fusion capability entries
and push received samples into IIO buffers when enabled.
> diff --git a/drivers/iio/opensensorfusion/osf_iio.c b/drivers/iio/opensensorfusion/osf_iio.c
> new file mode 100644
> index 0000000000000..862a797f4fb84
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_iio.c
[ ... ]
> +int osf_iio_push_sample(struct iio_dev *indio_dev, const s32 *values,
> + unsigned int channel_count)
> +{
> + struct osf_iio_state *state = iio_priv(indio_dev);
> + s64 timestamp;
> +
> + if (channel_count != state->spec->channel_count)
> + return -EPROTO;
> +
> + /* This is only a fast path; IIO rechecks buffer state while pushing. */
> + if (!iio_buffer_enabled(indio_dev))
[Severity: High]
Is there a race condition between this asynchronous IIO buffer push and
buffer enable/disable operations?
The IIO core's iio_push_to_buffers() function iterates over the buffer_list
without locks, expecting the driver to guarantee no pushes occur concurrently
with buffer state changes.
Since this driver doesn't implement IIO setup_ops (like predisable) to stop
incoming frames, or use a software lock, could a concurrent write to the
sysfs enable attribute invoke iio_disable_buffers() and modify the
buffer_list while the UART RX path is traversing it, causing list corruption?
> + return 0;
> +
> + timestamp = iio_get_time_ns(indio_dev);
> +
> + return iio_push_to_buffers_with_ts_unaligned(indio_dev, values,
> + channel_count * sizeof(*values),
> + timestamp);
[Severity: High]
Does this leak uninitialized kernel heap memory to user-space through the
bounce buffer padding?
The data payload size passed here is exactly the size of the channels
without padding. The IIO core computes scan_bytes to include padding for the
64-bit timestamp to ensure alignment, and allocates the internal bounce
buffer using devm_krealloc() without the __GFP_ZERO flag.
Because the IIO core only copies the exact data payload size and inserts the
timestamp at the aligned offset, it leaves the intermediate padding bytes
completely uninitialized. When this bounce buffer is pushed into the KFIFO,
will it leak uninitialized memory with every sample?
Would it be safer to assemble the sample in a zero-initialized padded struct
on the stack and use iio_push_to_buffers_with_timestamp() instead?
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616072242.3942-1-kimjinseob88@gmail.com?part=6
^ permalink raw reply
* Re: [PATCH RFC v5 4/6] iio: osf: add stream parser
From: sashiko-bot @ 2026-06-16 7:38 UTC (permalink / raw)
To: Jinseob Kim; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260616072242.3942-5-kimjinseob88@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The stream parser discards up to `frame_len` bytes of unauthenticated data on protocol errors (`-EPROTO`), allowing malformed or random data to cause prolonged desynchronization and data loss of up to 4096 bytes of valid frames.
--
commit 9d1c984a937aba83c2b56bd0cda78b4b642ec15a
Author: Jinseob Kim <kimjinseob88@gmail.com>
iio: osf: add stream parser
Add a byte-stream parser that resynchronizes on OSF frame magic, validates
complete frames, and forwards decoded frames to the OSF core.
> diff --git a/drivers/iio/opensensorfusion/osf_stream.c b/drivers/iio/opensensorfusion/osf_stream.c
> new file mode 100644
> index 0000000000000..957f737160f42
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_stream.c
[ ... ]
> +static int osf_stream_process(struct osf_stream *stream)
> +{
[ ... ]
> + ret = osf_core_receive_frame(stream->osf, stream->buf, frame_len);
> + if (ret) {
> + if (ret == -EBADMSG) {
> + stream->stats.bad_crc_frames++;
> + stream->stats.dropped_bytes++;
> + osf_stream_drop_invalid_head(stream);
> + } else {
> + osf_stream_discard(stream, frame_len);
> + }
[Severity: High]
If osf_core_receive_frame() returns a non-CRC error like -EPROTO, is it safe
to discard frame_len bytes here?
It looks like osf_protocol_decode_frame() returns -EPROTO for framing errors
before validating the CRC:
osf_protocol_decode_frame() {
...
major = buf[4];
if (major != OSF_PROTOCOL_MAJOR)
return -EPROTO;
...
}
Because the CRC hasn't been verified yet when this error is returned,
frame_len could be an unauthenticated garbage value if line noise happened to
match the magic bytes.
Could this blindly discard up to 4096 bytes of potentially valid frames and
cause prolonged desynchronization, instead of dropping just the invalid head
to quickly resynchronize?
> + if (!first_err)
> + first_err = ret;
> + continue;
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616072242.3942-1-kimjinseob88@gmail.com?part=4
^ permalink raw reply
* Re: [PATCH RFC v5 5/6] iio: osf: add UART transport
From: sashiko-bot @ 2026-06-16 7:37 UTC (permalink / raw)
To: Jinseob Kim; +Cc: devicetree, robh, conor+dt
In-Reply-To: <20260616072242.3942-6-kimjinseob88@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The frame decoder validates header fields before verifying the frame CRC, returning `-EPROTO` for structural errors and causing the stream parser to drop unauthenticated payload lengths.
--
commit ca765586fd1687d99b4b8cd4c1d18456513a142b
Author: Jinseob Kim <kimjinseob88@gmail.com>
iio: osf: add UART transport
Add the serdev UART transport and the initial OSF core receive path.
Enable the required vcc regulator with devm_regulator_get_enable()
before opening the UART, keeping power handling limited to the simple
probe-time requirement for this RFC.
> diff --git a/drivers/iio/opensensorfusion/osf_core.c b/drivers/iio/opensensorfusion/osf_core.c
> new file mode 100644
> index 0000000000000..137fb7166febc
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_core.c
> @@ -0,0 +1,99 @@
[ ... ]
> +int osf_core_receive_frame(struct osf_device *osf, const u8 *buf, size_t len)
> +{
> + struct osf_frame frame;
> + size_t frame_len;
> + int ret;
> +
> + if (!osf || !buf)
> + return -EINVAL;
> +
> + ret = osf_protocol_decode_frame(buf, len, &frame, &frame_len);
[Severity: High]
Could this error handling cause a regression where data is lost during parsing?
osf_protocol_decode_frame() validates header fields (like the major version
and header length) before checking the CRC, returning -EPROTO on structural
errors:
drivers/iio/opensensorfusion/osf_protocol.c:osf_protocol_decode_frame() {
...
major = buf[4];
if (major != OSF_PROTOCOL_MAJOR)
return -EPROTO;
if (get_unaligned_le16(buf + 6) != OSF_FRAME_HEADER_LEN)
return -EPROTO;
...
}
If -EPROTO is returned here, the stream parser discards the unauthenticated
frame_len number of bytes instead of dropping 1 byte for resynchronization:
drivers/iio/opensensorfusion/osf_stream.c:osf_stream_process() {
...
ret = osf_core_receive_frame(stream->osf, stream->buf, frame_len);
if (ret) {
if (ret == -EBADMSG) {
stream->stats.bad_crc_frames++;
stream->stats.dropped_bytes++;
osf_stream_drop_invalid_head(stream);
} else {
osf_stream_discard(stream, frame_len);
}
...
}
Because the CRC wasn't evaluated yet, frame_len is derived from an
unauthenticated payload_len, which could be a large, corrupted value. This
regression might cause the stream parser to skip legitimate frames hidden in
the incoming buffer under noisy conditions.
> + if (ret)
> + return ret;
> +
> + if (frame_len != len)
> + return -EMSGSIZE;
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616072242.3942-1-kimjinseob88@gmail.com?part=5
^ permalink raw reply
* Re: [PATCH 2/4] iio: adc: add ti-ads112c14 driver
From: Andy Shevchenko @ 2026-06-16 7:32 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-2-e6bdadf7cb2b@baylibre.com>
On Mon, Jun 15, 2026 at 05:00:00PM -0500, David Lechner (TI) wrote:
> Add a new driver for the TI ADS112C14/ADS122C14 ADC chips.
>
> This first step is adding a very basic driver that only supports power
> on/reset and reading the system monitor channels.
>
> ADS112C14_SYS_MON_CHANNEL_SHORT is the last channel rather than being in
> logical order by address to keep the voltage channels together and in
> case we find we need to add variants of this channel with different
> voltage reference later.
...
> +#include <linux/bitfield.h>
+ bitops.h // BIT(), GENMASK(), sign_extend32()
> +#include <linux/delay.h>
> +#include <linux/dev_printk.h>
> +#include <linux/i2c.h>
> +#include <linux/iio/iio.h>
> +#include <linux/iio/types.h>
I believe we discussed that (with Jonathan) already and seems the trend is to
imply that iio/types.h is always included whenever iio/iio.h is included.
> +#include <linux/math64.h>
> +#include <linux/mod_devicetable.h>
> +#include <linux/module.h>
> +#include <linux/property.h>
> +#include <linux/regmap.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/sysfs.h>
> +#include <linux/time64.h>
> +#include <linux/types.h>
> +#include <linux/unaligned.h>
> +#include <linux/units.h>
...
> +/* Datasheet t_d(RST) - time to wait after reset before next I2C use. */
> +#define ADS112C14_DELAY_RESET_us 500
For µs (and second related units) we traditionally use _US suffix
(yeah, I know...). Of course, we can start a SI schema in new code
but it will look quite strange and probably more confusing.
...
> +struct ads112c14_data {
> + const struct ads112c14_chip_info *chip_info;
> + struct i2c_client *client;
No need to keep it, it's one time use and can be derived from regmap.
regmap --> dev --> i2c_client.
> + struct regmap *regmap;
> +};
...
> +static int ads112c14_read_label(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, char *label)
> +{
> + const char *label_source;
I don't see the need of having this. Can't be returned directly?
> + /* System monitor channels. */
> + switch (chan->channel) {
> + case ADS112C14_SYS_MON_CHANNEL_TEMP:
> + label_source = "Internal temperature sensor";
> + break;
> + case ADS112C14_SYS_MON_CHANNEL_EXT_REF:
> + label_source = "External reference";
> + break;
> + case ADS112C14_SYS_MON_CHANNEL_AVDD:
> + label_source = "AVDD";
> + break;
> + case ADS112C14_SYS_MON_CHANNEL_DVDD:
> + label_source = "DVDD";
> + break;
> + case ADS112C14_SYS_MON_CHANNEL_SHORT:
> + label_source = "Internal short";
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + return sysfs_emit(label, "%s\n", label_source);
> +}
...
> +static int ads112c14_probe(struct i2c_client *client)
> +{
> + struct device *dev = &client->dev;
> + const struct ads112c14_chip_info *info;
> + struct iio_dev *indio_dev;
> + struct ads112c14_data *data;
> + u32 reg_val;
> + int ret;
> +
> + info = i2c_get_match_data(client);
NULL check (yeah) due to driver_override issue.
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
Here, and below, use 'dev'.
> + if (!indio_dev)
> + return -ENOMEM;
> +
> + data = iio_priv(indio_dev);
> + data->chip_info = info;
> + data->client = client;
> +
> + ret = devm_regulator_get_enable(&client->dev, "dvdd");
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get dvdd regulator\n");
> +
> + ret = devm_regulator_get_enable(&client->dev, "avdd");
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get avdd regulator\n");
> +
> + data->regmap = devm_regmap_init_i2c(client, &ads112c14_regmap_config);
> + if (IS_ERR(data->regmap))
> + return dev_err_probe(dev, PTR_ERR(data->regmap),
> + "failed to init regmap\n");
> +
> + /* Write magic reset value (0x16) to ensure known state.*/
> + ret = regmap_write(data->regmap, ADS112C14_REG_CONVERSION_CTRL,
> + FIELD_PREP(ADS112C14_CONVERSION_CTRL_RESET, 0x16));
> + /*
> + * The reset may cause an -EREMOTEIO error because of failing to get the
> + * I2C ACK at the end of the message. The device still gets reset.
> + */
> + if (ret != -EREMOTEIO)
> + return ret;
I would do it separately as
if (ret == -EREMOTEIO)
/* ...big comment here... */
return 0;
if (ret) // which is regular pattern and doesn't need any comment.
return ret;
> + fsleep(ADS112C14_DELAY_RESET_us);
> +
> + ret = regmap_read(data->regmap, ADS112C14_REG_STATUS_MSB, ®_val);
> + if (ret)
> + return ret;
> +
> + if (FIELD_GET(ADS112C14_STATUS_MSB_RESETN, reg_val))
> + return dev_err_probe(dev, -EIO, "reset failed\n");
> +
> + /*
> + * Clear reset bit to prepare for next probe. And clear AVDD fault since
> + * that happens on every reset.
> + */
> + ret = regmap_write(data->regmap, ADS112C14_REG_STATUS_MSB,
> + ADS112C14_STATUS_MSB_RESETN |
> + ADS112C14_STATUS_MSB_AVDD_UVN);
> + if (ret)
> + return ret;
> +
> + /* Place in single-shot conversion mode to make ready for raw read. */
> + ret = regmap_set_bits(data->regmap, ADS112C14_REG_DEVICE_CFG,
> + ADS112C14_DEVICE_CFG_CONV_MODE);
> + if (ret)
> + return ret;
> +
> + indio_dev->name = info->name;
> + indio_dev->modes = INDIO_DIRECT_MODE;
> + indio_dev->channels = ads112c14_sys_mon_channels;
> + indio_dev->num_channels = ARRAY_SIZE(ads112c14_sys_mon_channels);
> + indio_dev->info = &ads112c14_info;
> +
> + return devm_iio_device_register(&client->dev, indio_dev);
> +}
--
With Best Regards,
Andy Shevchenko
^ permalink raw reply
* Re: [PATCH RFC v5 1/6] dt-bindings: iio: add Open Sensor Fusion device
From: sashiko-bot @ 2026-06-16 7:31 UTC (permalink / raw)
To: Jinseob Kim; +Cc: robh, conor+dt, devicetree
In-Reply-To: <20260616072242.3942-2-kimjinseob88@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The binding uses a generic software protocol as the compatible string instead of describing specific hardware.
--
commit d9858298f5a88a8d0d1665f79878488487e06393
Author: Jinseob Kim <kimjinseob88@gmail.com>
dt-bindings: iio: add Open Sensor Fusion device
Adds the generic Open Sensor Fusion device binding for a serdev-attached IIO
sensor aggregation hub and documents the vendor prefix.
> The opensensorfusion,osf compatible describes the generic Open Sensor
> Fusion host interface. OSF GREEN is not the Linux compatible identity.
> Likewise, OSF0 is the current wire magic and a wire-format detail, not
> the Linux driver identity.
[Severity: Medium]
Does this approach align with Device Tree design principles?
Device Tree is typically intended to describe specific hardware components
(such as vendor,chip-model), rather than software interfaces or generic wire
protocols.
> diff --git a/Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml b/Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml
> new file mode 100644
> index 0000000000000..012a07fd69fe4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/opensensorfusion,osf.yaml
[ ... ]
> +properties:
> + compatible:
> + const: opensensorfusion,osf
> +
> + vcc-supply:
> + description:
> + Regulator supplying power to the Open Sensor Fusion device.
> +
> +required:
> + - compatible
> + - vcc-supply
[Severity: Medium]
Is it appropriate to constrain the compatible string to a generic software
protocol identifier here instead of a hardware-specific string?
Because this generic compatible string also requires specific hardware
constraints like vcc-supply, will future hardware variants that speak the
same OSF protocol but have different electrical needs (such as needing a
vddio-supply or reset GPIOs) be able to distinguish themselves without
clashing with these generic constraints?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260616072242.3942-1-kimjinseob88@gmail.com?part=1
^ 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