* Re: [PATCH v2 2/5] dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths
From: sashiko-bot @ 2026-06-21 21:51 UTC (permalink / raw)
To: Yuanshen Cao
Cc: robh, vkoul, dmaengine, Frank.Li, linux-sunxi, conor+dt,
devicetree
In-Reply-To: <20260621-sun60i-a733-dma-v2-2-340f205891cc@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Use-After-Free during error path cleanup in sun6i_dma_prep_slave_sg and sun6i_dma_prep_dma_cyclic.
--
commit 9bdc9020fe709f46ed0cba2bea4a233b8e23c603
Author: Yuanshen Cao <alex.caoys@gmail.com>
dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths
This commit refactors the sun6i DMA driver by moving the address setting logic
into a function pointer within the configuration structure. This allows
handling of different hardware variations, specifically adding support for
the A733's 32G high-address bits while preserving legacy behavior for
older generations.
> diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
> index ef3052c4ab363..9984b9033cbb9 100644
> --- a/drivers/dma/sun6i-dma.c
> +++ b/drivers/dma/sun6i-dma.c
> @@ -138,6 +139,8 @@ struct sun6i_dma_config {
> void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
> void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
> void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
> + void (*set_addr)(struct sun6i_dma_dev *sdev, struct sun6i_dma_lli *v_lli,
> + dma_addr_t src, dma_addr_t dst);
> void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
[ ... ]
[Severity: High]
This is a pre-existing issue, but I noticed a potential use-after-free
in the error cleanup paths.
Looking at sun6i_dma_prep_slave_sg():
err_lli_free:
for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
dma_pool_free(sdev->pool, v_lli, p_lli);
Does this loop cause a use-after-free? The dma_pool_free() call frees
v_lli, but the loop increment step then dereferences it via
v_lli = v_lli->v_lli_next.
This exact same pattern also appears in sun6i_dma_prep_dma_cyclic():
err_lli_free:
for (p_lli = txd->p_lli, v_lli = txd->v_lli; v_lli;
p_lli = v_lli->p_lli_next, v_lli = v_lli->v_lli_next)
dma_pool_free(sdev->pool, v_lli, p_lli);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com?part=2
^ permalink raw reply
* [PATCH v2 5/5] dmaengine: sun6i-dma: Implement support for Allwinner A733 DMA controller
From: Yuanshen Cao @ 2026-06-21 21:40 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
In-Reply-To: <20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com>
This patch implements the actual support for the Allwinner A733 DMA
controller. It defines the new register offsets and bitfield mappings
required for the A733, which slightly differs from the older `sun6i`
series.
Changes:
- New register macros for A733 interrupt enable `DMA_IRQ_EN_A733` and
status `DMA_IRQ_STAT_A733`.
- New `SRC_HIGH_ADDR_32G` and `DST_HIGH_ADDR_32G` macro to handle the
32G high-address field in the LLI.
- Implemented `sun6i_dma_set_addr_a733` and A733-specific interrupt
register accessors.
- Added `sun60i_a733_dma_config`, which ties all the refactored
functionality together for this specific hardware.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
drivers/dma/sun6i-dma.c | 87 +++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 87 insertions(+)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 196a0d73b221..4808015934cc 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -52,6 +52,15 @@
#define SUNXI_H3_SECURE_REG 0x20
#define SUNXI_H3_DMA_GATE 0x28
#define SUNXI_H3_DMA_GATE_ENABLE 0x4
+
+/*
+ * sun60i specific registers
+ */
+#define DMA_IRQ_EN_A733(x) ((x) * 0x40 + 0x134)
+#define DMA_IRQ_STAT_A733(x) ((x) * 0x40 + 0x138)
+
+#define DMA_IRQ_CHAN_NR_A733 1
+
/*
* Channels specific registers
*/
@@ -100,6 +109,8 @@
*/
#define SRC_HIGH_ADDR(x) (((x) & 0x3U) << 16)
#define DST_HIGH_ADDR(x) (((x) & 0x3U) << 18)
+#define SRC_HIGH_ADDR_32G(x) (((x) & 0x7U) << 11)
+#define DST_HIGH_ADDR_32G(x) (((x) & 0x7U) << 15)
/*
* Various hardware related defines
@@ -257,6 +268,23 @@ static inline void sun6i_dma_dump_com_regs(struct sun6i_dma_dev *sdev)
DMA_STAT, readl(sdev->base + DMA_STAT));
}
+static inline void sun6i_dma_dump_com_regs_a733(struct sun6i_dma_dev *sdev)
+{
+ int i;
+
+ for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
+ dev_dbg(sdev->slave.dev, "Common register:\n"
+ "chan num %d\n"
+ "\tmask(%04x): 0x%08x\n"
+ "\tpend(%04x): 0x%08x\n"
+ "\tstats(%04x): 0x%08x\n",
+ i,
+ DMA_IRQ_EN_A733(i), readl(sdev->base + DMA_IRQ_EN_A733(i)),
+ DMA_IRQ_STAT_A733(i), readl(sdev->base + DMA_IRQ_STAT_A733(i)),
+ DMA_STAT, readl(sdev->base + DMA_STAT));
+ }
+}
+
static inline void sun6i_dma_dump_chan_regs(struct sun6i_dma_dev *sdev,
struct sun6i_pchan *pchan)
{
@@ -360,21 +388,41 @@ static u32 sun6i_read_irq_en(struct sun6i_dma_dev *sdev, u32 irq_reg)
return readl(sdev->base + DMA_IRQ_EN(irq_reg));
}
+static u32 sun6i_read_irq_en_a733(struct sun6i_dma_dev *sdev, u32 irq_reg)
+{
+ return readl(sdev->base + DMA_IRQ_EN_A733(irq_reg));
+}
+
static void sun6i_write_irq_en(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val)
{
writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
}
+static void sun6i_write_irq_en_a733(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val)
+{
+ writel(irq_val, sdev->base + DMA_IRQ_EN_A733(irq_reg));
+}
+
static u32 sun6i_read_irq_stat(struct sun6i_dma_dev *sdev, u32 irq_reg)
{
return readl(sdev->base + DMA_IRQ_STAT(irq_reg));
}
+static u32 sun6i_read_irq_stat_a733(struct sun6i_dma_dev *sdev, u32 irq_reg)
+{
+ return readl(sdev->base + DMA_IRQ_STAT_A733(irq_reg));
+}
+
static void sun6i_write_irq_stat(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 status)
{
writel(status, sdev->base + DMA_IRQ_STAT(irq_reg));
}
+static void sun6i_write_irq_stat_a733(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 status)
+{
+ writel(status, sdev->base + DMA_IRQ_STAT_A733(irq_reg));
+}
+
static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
{
struct sun6i_desc *txd = pchan->desc;
@@ -695,6 +743,17 @@ static void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
DST_HIGH_ADDR(upper_32_bits(dst));
}
+static void sun6i_dma_set_addr_a733(struct sun6i_dma_dev *sdev,
+ struct sun6i_dma_lli *v_lli,
+ dma_addr_t src, dma_addr_t dst)
+{
+ v_lli->src = lower_32_bits(src);
+ v_lli->dst = lower_32_bits(dst);
+
+ v_lli->para |= SRC_HIGH_ADDR_32G(upper_32_bits(src)) |
+ DST_HIGH_ADDR_32G(upper_32_bits(dst));
+}
+
static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
struct sun6i_dma_lli *v_lli,
dma_addr_t src, dma_addr_t dst)
@@ -1339,6 +1398,33 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
+/*
+ * The A733 binding uses the number of dma channels from the
+ * device tree node.
+ */
+static struct sun6i_dma_config sun60i_a733_dma_cfg = {
+ .clock_autogate_enable = sun6i_enable_clock_autogate_h3,
+ .set_burst_length = sun6i_set_burst_length_h3,
+ .set_drq = sun6i_set_drq_h6,
+ .set_mode = sun6i_set_mode_h6,
+ .set_addr = sun6i_dma_set_addr_a733,
+ .dump_com_regs = sun6i_dma_dump_com_regs_a733,
+ .read_irq_en = sun6i_read_irq_en_a733,
+ .write_irq_en = sun6i_write_irq_en_a733,
+ .read_irq_stat = sun6i_read_irq_stat_a733,
+ .write_irq_stat = sun6i_write_irq_stat_a733,
+ .src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
+ .dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
+ .src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
+ BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
+ BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ .dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
+ BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
+ BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR_A733,
+ .has_mbus_clk = true,
+};
+
/*
* The V3s have only 8 physical channels, a maximum DRQ port id of 23,
* and a total of 24 usable source and destination endpoints.
@@ -1375,6 +1461,7 @@ static const struct of_device_id sun6i_dma_match[] = {
{ .compatible = "allwinner,sun50i-a64-dma", .data = &sun50i_a64_dma_cfg },
{ .compatible = "allwinner,sun50i-a100-dma", .data = &sun50i_a100_dma_cfg },
{ .compatible = "allwinner,sun50i-h6-dma", .data = &sun50i_h6_dma_cfg },
+ { .compatible = "allwinner,sun60i-a733-dma", .data = &sun60i_a733_dma_cfg },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, sun6i_dma_match);
--
2.54.0
^ permalink raw reply related
* [PATCH v2 4/5] dt-bindings: dma: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
From: Yuanshen Cao @ 2026-06-21 21:40 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
In-Reply-To: <20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com>
Add `allwinner,sun60i-a733-dma` to the list of compatible strings for the
`sun50i-a64-dma` dtbinding documentation.
While the A733 DMA controller shares many similarities with the sun50i-a64
DMA controller, it requires a specific configuration due to differences in:
- Interrupt register layout and mapping.
- Number of channels per interrupt register.
- Support for higher (32G) address widths in LLI parameters.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 ++
1 file changed, 2 insertions(+)
diff --git a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
index c3e14eb6cfff..1cc3304b7414 100644
--- a/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
+++ b/Documentation/devicetree/bindings/dma/allwinner,sun50i-a64-dma.yaml
@@ -25,6 +25,7 @@ properties:
- allwinner,sun50i-a64-dma
- allwinner,sun50i-a100-dma
- allwinner,sun50i-h6-dma
+ - allwinner,sun60i-a733-dma
- items:
- const: allwinner,sun8i-r40-dma
- const: allwinner,sun50i-a64-dma
@@ -70,6 +71,7 @@ if:
- allwinner,sun20i-d1-dma
- allwinner,sun50i-a100-dma
- allwinner,sun50i-h6-dma
+ - allwinner,sun60i-a733-dma
then:
properties:
--
2.54.0
^ permalink raw reply related
* [PATCH v2 3/5] dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
From: Yuanshen Cao @ 2026-06-21 21:40 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
In-Reply-To: <20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com>
The previous implementation of `sun6i-dma` had some implicit assumptions
about the number of channels per interrupt register. Specifically,
functions like `sun6i_kill_tasklet` were hardcoded to only disable
interrupts for IRQ 0 and 1. `DMA_MAX_CHANNELS` is also not in used in
the past, and the old SoCs never has more than 16 channels.
The A733 has a different interrupt structure where the number of
channels per register may differ. This patch introduces
`num_channels_per_reg` to the `sun6i_dma_config`, similar to BSP, to
make the interrupt handling logic hardware-agnostic. It also sets
`DMA_MAX_CHANNELS` to 16 to align with the new BSP code and ensure loops
over interrupts are correctly bounded.
Changes:
- Change `DMA_MAX_CHANNELS` definition to 16.
- Added `num_channels_per_reg` to `struct sun6i_dma_config`.
- Replaced hardcoded IRQ register calculations with values from
`sdev->cfg->num_channels_per_reg`.
- Updated `sun6i_kill_tasklet` to loop through all possible interrupt
registers based on `DMA_MAX_CHANNELS` and the configuration.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
drivers/dma/sun6i-dma.c | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index 9984b9033cbb..196a0d73b221 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -41,7 +41,7 @@
#define DMA_STAT 0x30
/* Offset between DMA_IRQ_EN and DMA_IRQ_STAT limits number of channels */
-#define DMA_MAX_CHANNELS (DMA_IRQ_CHAN_NR * 0x10 / 4)
+#define DMA_MAX_CHANNELS 16
/*
* sun8i specific registers
@@ -151,6 +151,7 @@ struct sun6i_dma_config {
u32 src_addr_widths;
u32 dst_addr_widths;
bool has_mbus_clk;
+ u32 num_channels_per_reg;
};
/*
@@ -482,8 +483,8 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
sun6i_dma_dump_lli(vchan, pchan->desc->v_lli, pchan->desc->p_lli);
- irq_reg = pchan->idx / DMA_IRQ_CHAN_NR;
- irq_offset = pchan->idx % DMA_IRQ_CHAN_NR;
+ irq_reg = pchan->idx / sdev->cfg->num_channels_per_reg;
+ irq_offset = pchan->idx % sdev->cfg->num_channels_per_reg;
vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
@@ -575,7 +576,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
int i, j, ret = IRQ_NONE;
u32 status;
- for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
+ for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
status = sdev->cfg->read_irq_stat(sdev, i);
if (!status)
continue;
@@ -585,7 +586,7 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
sdev->cfg->write_irq_stat(sdev, i, status);
- for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
+ for (j = 0; (j < sdev->cfg->num_channels_per_reg) && status; j++) {
pchan = sdev->pchans + j;
vchan = pchan->vchan;
if (vchan && (status & vchan->irq_type)) {
@@ -1116,9 +1117,11 @@ static struct dma_chan *sun6i_dma_of_xlate(struct of_phandle_args *dma_spec,
static inline void sun6i_kill_tasklet(struct sun6i_dma_dev *sdev)
{
+ int i;
+
/* Disable all interrupts from DMA */
- writel(0, sdev->base + DMA_IRQ_EN(0));
- writel(0, sdev->base + DMA_IRQ_EN(1));
+ for (i = 0; i < DMA_MAX_CHANNELS / sdev->cfg->num_channels_per_reg; i++)
+ sdev->cfg->write_irq_en(sdev, i, 0);
/* Prevent spurious interrupts from scheduling the tasklet */
atomic_inc(&sdev->tasklet_shutdown);
@@ -1181,6 +1184,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1206,6 +1210,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1226,6 +1231,7 @@ static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1255,6 +1261,7 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1278,6 +1285,7 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1301,6 +1309,7 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR,
.has_mbus_clk = true,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1325,6 +1334,7 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR,
.has_mbus_clk = true,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1351,6 +1361,7 @@ static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ .num_channels_per_reg = DMA_IRQ_CHAN_NR,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/5] dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths
From: Yuanshen Cao @ 2026-06-21 21:40 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
In-Reply-To: <20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com>
The A733 DMA controller supports higher address (up to 32G) compared to
previous generations. The existing `sun6i_dma_set_addr` function uses a
hardcoded logic for setting the high-address bits in the LLI parameters.
By moving `set_addr` into the `sun6i_dma_config` structure, we can
provide specialized implementations for different hardware. This allows
the A733 to use a version of `set_addr` that correctly handles its
specific `SRC_HIGH_ADDR_32G` and `DST_HIGH_ADDR_32G` in the `set_addr`
register later in the series.
Changes:
- Added `set_addr` function pointer to `struct sun6i_dma_config`.
- Refactored `sun6i_dma_set_addr` and introduced
`sun6i_dma_set_addr_a31/a100` (keeping the logic for previous
generations).
- Updated all existing configuration structs to include the new
`set_addr` pointer.
- Removed `has_high_addr` since the logic is replaced by
`sun6i_dma_set_addr_a100`.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
drivers/dma/sun6i-dma.c | 35 +++++++++++++++++++++++++++++------
1 file changed, 29 insertions(+), 6 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index ef3052c4ab36..9984b9033cbb 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -112,6 +112,7 @@
/* forward declaration */
struct sun6i_dma_dev;
+struct sun6i_dma_lli;
/*
* Hardware channels / ports representation
@@ -138,6 +139,8 @@ struct sun6i_dma_config {
void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
+ void (*set_addr)(struct sun6i_dma_dev *sdev, struct sun6i_dma_lli *v_lli,
+ dma_addr_t src, dma_addr_t dst);
void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 irq_reg);
void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val);
@@ -147,7 +150,6 @@ struct sun6i_dma_config {
u32 dst_burst_lengths;
u32 src_addr_widths;
u32 dst_addr_widths;
- bool has_high_addr;
bool has_mbus_clk;
};
@@ -673,16 +675,30 @@ static int set_config(struct sun6i_dma_dev *sdev,
return 0;
}
-static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
+static void sun6i_dma_set_addr_a31(struct sun6i_dma_dev *sdev,
+ struct sun6i_dma_lli *v_lli,
+ dma_addr_t src, dma_addr_t dst)
+{
+ v_lli->src = lower_32_bits(src);
+ v_lli->dst = lower_32_bits(dst);
+}
+
+static void sun6i_dma_set_addr_a100(struct sun6i_dma_dev *sdev,
struct sun6i_dma_lli *v_lli,
dma_addr_t src, dma_addr_t dst)
{
v_lli->src = lower_32_bits(src);
v_lli->dst = lower_32_bits(dst);
- if (sdev->cfg->has_high_addr)
- v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
- DST_HIGH_ADDR(upper_32_bits(dst));
+ v_lli->para |= SRC_HIGH_ADDR(upper_32_bits(src)) |
+ DST_HIGH_ADDR(upper_32_bits(dst));
+}
+
+static inline void sun6i_dma_set_addr(struct sun6i_dma_dev *sdev,
+ struct sun6i_dma_lli *v_lli,
+ dma_addr_t src, dma_addr_t dst)
+{
+ sdev->cfg->set_addr(sdev, v_lli, src, dst);
}
static struct dma_async_tx_descriptor *sun6i_dma_prep_dma_memcpy(
@@ -1156,6 +1172,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
.set_burst_length = sun6i_set_burst_length_a31,
.set_drq = sun6i_set_drq_a31,
.set_mode = sun6i_set_mode_a31,
+ .set_addr = sun6i_dma_set_addr_a31,
.src_burst_lengths = BIT(1) | BIT(8),
.dst_burst_lengths = BIT(1) | BIT(8),
.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1180,6 +1197,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
.set_burst_length = sun6i_set_burst_length_a31,
.set_drq = sun6i_set_drq_a31,
.set_mode = sun6i_set_mode_a31,
+ .set_addr = sun6i_dma_set_addr_a31,
.src_burst_lengths = BIT(1) | BIT(8),
.dst_burst_lengths = BIT(1) | BIT(8),
.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1199,6 +1217,7 @@ static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
.set_burst_length = sun6i_set_burst_length_a31,
.set_drq = sun6i_set_drq_a31,
.set_mode = sun6i_set_mode_a31,
+ .set_addr = sun6i_dma_set_addr_a31,
.src_burst_lengths = BIT(1) | BIT(8),
.dst_burst_lengths = BIT(1) | BIT(8),
.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1225,6 +1244,7 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
.set_burst_length = sun6i_set_burst_length_h3,
.set_drq = sun6i_set_drq_a31,
.set_mode = sun6i_set_mode_a31,
+ .set_addr = sun6i_dma_set_addr_a31,
.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1247,6 +1267,7 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg = {
.set_burst_length = sun6i_set_burst_length_h3,
.set_drq = sun6i_set_drq_a31,
.set_mode = sun6i_set_mode_a31,
+ .set_addr = sun6i_dma_set_addr_a31,
.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1269,6 +1290,7 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
.set_burst_length = sun6i_set_burst_length_h3,
.set_drq = sun6i_set_drq_h6,
.set_mode = sun6i_set_mode_h6,
+ .set_addr = sun6i_dma_set_addr_a100,
.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1279,7 +1301,6 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
- .has_high_addr = true,
.has_mbus_clk = true,
SUN6I_DMA_IRQ_A31_COMMON_OPS
};
@@ -1293,6 +1314,7 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
.set_burst_length = sun6i_set_burst_length_h3,
.set_drq = sun6i_set_drq_h6,
.set_mode = sun6i_set_mode_h6,
+ .set_addr = sun6i_dma_set_addr_a31,
.src_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
.dst_burst_lengths = BIT(1) | BIT(4) | BIT(8) | BIT(16),
.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
@@ -1320,6 +1342,7 @@ static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
.set_burst_length = sun6i_set_burst_length_a31,
.set_drq = sun6i_set_drq_a31,
.set_mode = sun6i_set_mode_a31,
+ .set_addr = sun6i_dma_set_addr_a31,
.src_burst_lengths = BIT(1) | BIT(8),
.dst_burst_lengths = BIT(1) | BIT(8),
.src_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
--
2.54.0
^ permalink raw reply related
* [PATCH v2 1/5] dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling
From: Yuanshen Cao @ 2026-06-21 21:40 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
In-Reply-To: <20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com>
Refactor to support the Allwinner A733 DMA controller. Currently, the
`sun6i-dma` driver has several functions related to interrupt handling
(reading/writing interrupt enable and status registers) and register
dumping that are hardcoded.
To support the A733, which has different register layouts and interrupt
handling logic, these functions are being moved into the
`sun6i_dma_config` structure as function pointers. This allows the
driver to use a polymorphic approach where the specific implementation
is determined by the hardware configuration assigned during device
probing.
Changes:
- Added function pointers to `struct sun6i_dma_config` for:
- `dump_com_regs`
- `read_irq_en`
- `write_irq_en`
- `read_irq_stat`
- `write_irq_stat`
- Implemented generic `sun6i_read/write_irq_*` functions for existing
hardware.
- Added a macro and updated existing `sun6i_dma_config` instances (A31,
A23, H3, A64, A100, H6, V3S) to use these new function pointers.
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
drivers/dma/sun6i-dma.c | 50 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 45 insertions(+), 5 deletions(-)
diff --git a/drivers/dma/sun6i-dma.c b/drivers/dma/sun6i-dma.c
index a9a254dbf8cb..ef3052c4ab36 100644
--- a/drivers/dma/sun6i-dma.c
+++ b/drivers/dma/sun6i-dma.c
@@ -138,6 +138,11 @@ struct sun6i_dma_config {
void (*set_burst_length)(u32 *p_cfg, s8 src_burst, s8 dst_burst);
void (*set_drq)(u32 *p_cfg, s8 src_drq, s8 dst_drq);
void (*set_mode)(u32 *p_cfg, s8 src_mode, s8 dst_mode);
+ void (*dump_com_regs)(struct sun6i_dma_dev *sdev);
+ u32 (*read_irq_en)(struct sun6i_dma_dev *sdev, u32 irq_reg);
+ void (*write_irq_en)(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val);
+ u32 (*read_irq_stat)(struct sun6i_dma_dev *sdev, u32 irq_reg);
+ void (*write_irq_stat)(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 status);
u32 src_burst_lengths;
u32 dst_burst_lengths;
u32 src_addr_widths;
@@ -347,6 +352,26 @@ static void sun6i_set_mode_h6(u32 *p_cfg, s8 src_mode, s8 dst_mode)
DMA_CHAN_CFG_DST_MODE_H6(dst_mode);
}
+static u32 sun6i_read_irq_en(struct sun6i_dma_dev *sdev, u32 irq_reg)
+{
+ return readl(sdev->base + DMA_IRQ_EN(irq_reg));
+}
+
+static void sun6i_write_irq_en(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 irq_val)
+{
+ writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
+}
+
+static u32 sun6i_read_irq_stat(struct sun6i_dma_dev *sdev, u32 irq_reg)
+{
+ return readl(sdev->base + DMA_IRQ_STAT(irq_reg));
+}
+
+static void sun6i_write_irq_stat(struct sun6i_dma_dev *sdev, u32 irq_reg, u32 status)
+{
+ writel(status, sdev->base + DMA_IRQ_STAT(irq_reg));
+}
+
static size_t sun6i_get_chan_size(struct sun6i_pchan *pchan)
{
struct sun6i_desc *txd = pchan->desc;
@@ -460,16 +485,16 @@ static int sun6i_dma_start_desc(struct sun6i_vchan *vchan)
vchan->irq_type = vchan->cyclic ? DMA_IRQ_PKG : DMA_IRQ_QUEUE;
- irq_val = readl(sdev->base + DMA_IRQ_EN(irq_reg));
+ irq_val = sdev->cfg->read_irq_en(sdev, irq_reg);
irq_val &= ~((DMA_IRQ_HALF | DMA_IRQ_PKG | DMA_IRQ_QUEUE) <<
(irq_offset * DMA_IRQ_CHAN_WIDTH));
irq_val |= vchan->irq_type << (irq_offset * DMA_IRQ_CHAN_WIDTH);
- writel(irq_val, sdev->base + DMA_IRQ_EN(irq_reg));
+ sdev->cfg->write_irq_en(sdev, irq_reg, irq_val);
writel(pchan->desc->p_lli, pchan->base + DMA_CHAN_LLI_ADDR);
writel(DMA_CHAN_ENABLE_START, pchan->base + DMA_CHAN_ENABLE);
- sun6i_dma_dump_com_regs(sdev);
+ sdev->cfg->dump_com_regs(sdev);
sun6i_dma_dump_chan_regs(sdev, pchan);
return 0;
@@ -549,14 +574,14 @@ static irqreturn_t sun6i_dma_interrupt(int irq, void *dev_id)
u32 status;
for (i = 0; i < sdev->num_pchans / DMA_IRQ_CHAN_NR; i++) {
- status = readl(sdev->base + DMA_IRQ_STAT(i));
+ status = sdev->cfg->read_irq_stat(sdev, i);
if (!status)
continue;
dev_dbg(sdev->slave.dev, "DMA irq status %s: 0x%x\n",
str_high_low(i), status);
- writel(status, sdev->base + DMA_IRQ_STAT(i));
+ sdev->cfg->write_irq_stat(sdev, i, status);
for (j = 0; (j < DMA_IRQ_CHAN_NR) && status; j++) {
pchan = sdev->pchans + j;
@@ -1101,6 +1126,13 @@ static inline void sun6i_dma_free(struct sun6i_dma_dev *sdev)
}
}
+#define SUN6I_DMA_IRQ_A31_COMMON_OPS \
+ .dump_com_regs = sun6i_dma_dump_com_regs, \
+ .read_irq_en = sun6i_read_irq_en, \
+ .write_irq_en = sun6i_write_irq_en, \
+ .read_irq_stat = sun6i_read_irq_stat, \
+ .write_irq_stat = sun6i_write_irq_stat,
+
/*
* For A31:
*
@@ -1132,6 +1164,7 @@ static struct sun6i_dma_config sun6i_a31_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_OPS
};
/*
@@ -1155,6 +1188,7 @@ static struct sun6i_dma_config sun8i_a23_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_OPS
};
static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
@@ -1173,6 +1207,7 @@ static struct sun6i_dma_config sun8i_a83t_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_OPS
};
/*
@@ -1200,6 +1235,7 @@ static struct sun6i_dma_config sun8i_h3_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_OPS
};
/*
@@ -1221,6 +1257,7 @@ static struct sun6i_dma_config sun50i_a64_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_OPS
};
/*
@@ -1244,6 +1281,7 @@ static struct sun6i_dma_config sun50i_a100_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
.has_high_addr = true,
.has_mbus_clk = true,
+ SUN6I_DMA_IRQ_A31_COMMON_OPS
};
/*
@@ -1266,6 +1304,7 @@ static struct sun6i_dma_config sun50i_h6_dma_cfg = {
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_8_BYTES),
.has_mbus_clk = true,
+ SUN6I_DMA_IRQ_A31_COMMON_OPS
};
/*
@@ -1289,6 +1328,7 @@ static struct sun6i_dma_config sun8i_v3s_dma_cfg = {
.dst_addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
BIT(DMA_SLAVE_BUSWIDTH_4_BYTES),
+ SUN6I_DMA_IRQ_A31_COMMON_OPS
};
static const struct of_device_id sun6i_dma_match[] = {
--
2.54.0
^ permalink raw reply related
* [PATCH v2 0/5] dmaengine: sun6i-dma: Add support for Allwinner A733 DMA controller
From: Yuanshen Cao @ 2026-06-21 21:40 UTC (permalink / raw)
To: Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard
Cc: Yuanshen Cao, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
Hi everyone,
This patch series introduces support for the Allwinner A733 DMA
controller in the `sun6i-dma` driver.
The A733 DMA controller differs from previous generations in several key
ways:
1. It supports higher address (up to 32G).
2. It uses a different interrupt register layout and mapping.
3. It has a different number of channels per interrupt register.
To support these differences without introducing complex conditional
logic throughout the driver, this series first refactors the
`sun6i_dma_config` structure. By moving interrupt handling, register
dumping, and address configuration into function pointers within the
configuration structure. This allows the driver to support the A733
and future hardware revisions. It also aligns with the DMA drivers in
Radxa BSP Package[1].
The series is organized as follows:
- Refactors the configuration structure to include function pointers for
interrupt and register operations.
- Moves address setting logic into the configuration structure to handle
varying address widths.
- Adds support for variable channels per interrupt register.
- Updates the device tree bindings documentation.
- Implements the A733-specific configuration and register mappings.
Tested on Radxa Cubie A7Z.
[1] https://github.com/radxa/allwinner-bsp/blob/cubie-aiot-v1.4.8/drivers/dma/sunxi-dma.c
Thanks!
Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
---
Changes in v2:
- Implement SUN6I_DMA_IRQ_A31_COMMON_OPS macro to avoid duplicate.
- Move set_addr into helper function and revert back sun6i_dma_set_addr.
- Rename chan_num to irq_req to avoid misleading name as suggested by
sashiko.
- Reorder and reword the dtbinding patch for more clarity.
- Link to v1: https://patch.msgid.link/20260619-sun60i-a733-dma-v1-0-da4b649fc72a@gmail.com
To: Vinod Koul <vkoul@kernel.org>
To: Frank Li <Frank.Li@kernel.org>
To: Chen-Yu Tsai <wens@kernel.org>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Samuel Holland <samuel@sholland.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Maxime Ripard <mripard@kernel.org>
Cc: dmaengine@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Cc: linux-kernel@vger.kernel.org
Cc: devicetree@vger.kernel.org
---
Yuanshen Cao (5):
dmaengine: sun6i-dma: Refactor to support A733 interrupt and register handling
dmaengine: sun6i-dma: Add set_addr function pointer for variable address widths
dmaengine: sun6i-dma: Add num_channels_per_reg for flexible interrupt mapping
dt-bindings: dma: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
dmaengine: sun6i-dma: Implement support for Allwinner A733 DMA controller
.../bindings/dma/allwinner,sun50i-a64-dma.yaml | 2 +
drivers/dma/sun6i-dma.c | 197 +++++++++++++++++++--
2 files changed, 181 insertions(+), 18 deletions(-)
---
base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
change-id: 20260619-sun60i-a733-dma-c2455149165d
Best regards,
--
Yuanshen Cao <alex.caoys@gmail.com>
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
From: David Lechner @ 2026-06-21 21:14 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Kurt Borja, Nguyen Minh Tien, linux-iio, devicetree,
linux-kernel
In-Reply-To: <20260621194102.08d7fdd6@jic23-huawei>
On 6/21/26 1:41 PM, Jonathan Cameron wrote:
> On Mon, 15 Jun 2026 16:59:59 -0500
> "David Lechner (TI)" <dlechner@baylibre.com> wrote:
>
>> Add new bindings for ti,ads122c14 and similar devices.
>>
>> This is an ADC that is primarily intended for use with temperature
>> sensors. There are a few unusual properties because of this. In
>> particular, the reference voltage source and current output requirements
>> can be different for each measurement, so these are included in the
>> channel bindings.
>>
>> The REFP/REFN reference voltage is usually just connected to a resistor
>> that is being driven by the ADC's current outputs, so there is special
>> property for this case rather than requiring a regulator to be defined
>> to represent that.
>>
>> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
>> have preferred an enum of strings).
>>
>> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
>
> A few queries inline though I'm only just starting to get my head
> around this device...
>
> Thanks
>
> Jonathan
>
>> ---
>> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
>> MAINTAINERS | 7 +
>> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
>> 3 files changed, 242 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
>> new file mode 100644
>> index 000000000000..dc7f37cad772
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
>> @@ -0,0 +1,224 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/iio/adc/ti,ads112c14.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Texas Instruments' ADS112C14 and similar ADC chips
>> +
>> +description: |
>> + Supports the following Texas Instruments' ADC chips:
>> + - ADS112C14 (16-bit)
>> + - ADS122C14 (24-bit)
>> +
>> + https://www.ti.com/lit/ds/symlink/ads122c14.pdf
>> +
>> + These chips are primarily designed for use with temperature sensors such as
>> + RTDs and thermocouples. The channel bindings reflect this in that each channel
>> + represents the conditions required to make a measurement rather than strictly
>> + just the physical input channels.
>> +
>> +maintainers:
>> + - David Lechner <dlechner@baylibre.com>
>> +
>> +unevaluatedProperties: false
>> +
>> +properties:
>> + compatible:
>> + enum:
>> + - ti,ads112c14
>> + - ti,ads122c14
>> +
>> + reg:
>> + items:
>> + - minimum: 0x40
>> + maximum: 0x47
>> +
>> + clocks:
>> + maxItems: 1
>> + description: Optional external clock connected to GPIO3 pin.
>> +
>> + avdd-supply: true
>> + dvdd-supply: true
>> +
>> + refp-supply: true
>> + refn-supply: true
>> +
>> + refp-refn-resistor-ohms:
>> + description:
>> + The resistance of the external resistor between REFP and REFN when using
>> + resistor bridge driven by current outputs for RTD measurements.
>> +
>> + interrupts:
>> + minItems: 1
>> + items:
>> + - description: FAULT interrupt (GPIO2 pin)
>> + - description: DRDY interrupt (GPIO3 pin)
>> +
>> + interrupt-names:
>> + minItems: 1
>> + maxItems: 2
>> + items:
>> + enum: [fault, drdy]
>> +
>> + gpio-controller: true
>> + '#gpio-cells':
>> + const: 2
>> +
>> + '#address-cells':
>> + const: 1
>> +
>> + '#size-cells':
>> + const: 0
>> +
>> +patternProperties:
>> + ^channel@[0-7]$:
>> + $ref: adc.yaml
>> +
>> + unevaluatedProperties: false
>> +
>> + properties:
>> + reg:
>> + maximum: 16 # arbitrary limit, channel@ can be any combination of AIN0-AIN7
>> +
>> + single-channel:
>> + maximum: 7
>> +
>> + diff-channels:
>> + items:
>> + maximum: 7
>> +
>> + bipolar:
>> + description:
>> + Set this flag if the differential input can be negative.
>
> I'd leave that description to adc.yaml Maybe that doc could be improved though
> given it basically says bipolar == bipolar mode ;)
It seems not always obvious to me which properties from adc.yaml apply
and which ones don't to a given ADC that makes use of it. So I was
hoping to have some way of saying that bipolar is applicable to this
chip.
>
>> +
>> + excitation-channels:
>> + description: AINx pins used as current output.
>> + $ref: /schemas/types.yaml#/definitions/uint32-array
>> + minItems: 1
>> + maxItems: 2
>> + items:
>> + maximum: 7
>> +
>> + excitation-current-microamp:
>
> There seem to be separate controls. Are their usecases where this needs
> to be in array?
I'll have to ask, but probably yes since there are separate controls
so `maxItems: 2` would be appropriate.
>
>> + description: The current output of the excitation channels in microamps.
>> + minimum: 1
>> + maximum: 1000
>> +
>> + current-chopping:
>> + $ref: /schemas/types.yaml#/definitions/flag
>> + description:
>> + If provided, the two excitation channels are to be used with current
>> + chopping enabled.
>
> Can I have a reference for that? My initial read suggests it's the input channels
No. :-)
I must have got two ideas mixed together in my head to come up with
this. Clearly this should be `input-channel-rotation` or something like
that (we discussed in another thread already). Also curious if you thing
any of these properties are common enough to promote to adc.yaml or if we
should just make them e.g. `ti,input-channel-rotation` (you might not have
had time to read the threads on that yet).
> that are chopped. For GC_EN
> "When enabled, the device automatically swaps
> the analog inputs and takes the average of two consecutive conversions to
> cancel the internal offset voltage"
>
>
>> +
>> + ti,vref-source:
>> + description: |
>> + Indicates the source for the reference voltage for this channel.
>> + 0 - Internal 2.5V reference
>> + 1 - Internal 1.25V reference
>> + 2 - External reference (REFP-REFN)
>> + 3 - AVDD as reference
>> +
>> + For convenience, macros for these values are available in
>> + dt-bindings/iio/adc/ti,ads112c14.h.
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + maximum: 3
>> + default: 0
>> +
>> + dependencies:
>> + excitation-channels: [ excitation-current-microamp ]
>> + excitation-current-microamp: [ excitation-channels ]
>> + current-chopping: [ excitation-channels ]
>> +
>> + oneOf:
>> + - required: [ single-channel ]
>> + - required: [ diff-channels ]
>
>> +examples:
>> + - |
>> + #include <dt-bindings/iio/adc/ti,ads112c14.h>
>> +
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + adc@40 {
>> + compatible = "ti,ads112c14";
>> + reg = <0x40>;
>> +
>> + avdd-supply = <&avdd>;
>> + dvdd-supply = <&dvdd>;
>> +
>> + /* 3-Wire RTD: Two IDACs, One Measurement (AIN1-AIN2) */
>> +
>> + refp-refn-resistor-ohms = <500>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + channel@0 {
>> + reg = <0>;
>> + diff-channels = <1>, <2>;
>> + excitation-channels = <0>, <3>;
>> + excitation-current-microamp = <500>;
>> + current-chopping;
>> + ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
>> + label = "rtd";
>> + };
>> + };
>> + };
>> + - |
>> + #include <dt-bindings/iio/adc/ti,ads112c14.h>
>> +
>> + i2c {
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + adc@40 {
>> + compatible = "ti,ads112c14";
>> + reg = <0x40>;
>> +
>> + avdd-supply = <&avdd>;
>> + dvdd-supply = <&dvdd>;
>> +
>> + /* Resistive Bridge Measurement With a Thermistor for Temperature Compensation*/
>> +
>> + refp-supply = <&avdd>;
>> +
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + channel@0 {
>> + reg = <0>;
>> + diff-channels = <6>, <7>;
>> + bipolar;
>> + ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
>> + label = "bridge";
>> + };
>> +
>> + channel@1 {
>> + reg = <1>;
>> + diff-channels = <1>, <2>;
>> + ti,vref-source = <ADS112C14_VREF_SOURCE_INTERNAL_2_5V>;
>> + label = "thermistor";
>
> Hmm. I'm interested to see where this goes, but generally when we have
> a thermistor we attempt to ultimately convert it to a temperature
> channel and I'm not seeing info here to allow us to do that.
Since the hardware doesn't have any special features for handling
specific sensor types, it seems like a case of the driver trying to
do things that the hardware doesn't do, which we generally try to
avoid in the kernel.
For cases where we want a quick and easy (and not necessarily accurate)
temperature conversion done in the kernel, we could make a generic thermistor
analog front end binding and driver like we already have for RTDs and
(linear) temperature transducers. This seems more sensible to me rather
than having to re-implement such a thing in each ADC that could be used
with a thermistor.
>
>> + };
>> + };
>> + };
>
>
^ permalink raw reply
* Re: [PATCH] arm64: dts: broadcom: bcm2712: Remove non-functional EL2 virtual timer
From: Daniel Drake @ 2026-06-21 20:58 UTC (permalink / raw)
To: Florian Fainelli, Marc Zyngier
Cc: robh, krzk+dt, conor+dt, bcm-kernel-feedback-list, devicetree,
linux-rpi-kernel, linux-arm-kernel, m.szyprowski, andrea.porta
In-Reply-To: <223cd514-41b3-45b9-8617-b54d379d5091@broadcom.com>
On 21/06/2026 21:03, Florian Fainelli wrote:
> Daniel, do you happen to know which 2712 SoC revision you have, whether
> this is a C0 or D0 stepping?
>
> We have an internal bug tracker item pertaining exactly to the virtual
> timer interrupt connection however it affected a sister chip (77122) and
> not 2712 AFAICT, now checking with the design team whether the same
> happened on 2712.
Thanks for looking into this! I am using Raspberry Pi 500 with D0 stepping.
Daniel
^ permalink raw reply
* Re: [PATCH] arm64: dts: broadcom: bcm2712: Remove non-functional EL2 virtual timer
From: Florian Fainelli @ 2026-06-21 20:03 UTC (permalink / raw)
To: Marc Zyngier, Daniel Drake
Cc: robh, krzk+dt, conor+dt, bcm-kernel-feedback-list, devicetree,
linux-rpi-kernel, linux-arm-kernel, m.szyprowski, andrea.porta
In-Reply-To: <878q898ulx.wl-maz@kernel.org>
On 6/20/2026 9:49 AM, Marc Zyngier wrote:
> Hi Daniel,
>
> Thanks for posting this.
>
> On Fri, 19 Jun 2026 21:48:32 +0100,
> Daniel Drake <dan@reactivated.net> wrote:
>>
>> Commit d87773de9efe1 ("clocksource/drivers/arm_arch_timer: Default to
>> EL2 virtual timer when running VHE") causes boot to hang on
>> Raspberry Pi 5. The newly-selected EL2 virtual timer does not generate
>> any interrupts, even though the GIC_DIST_ENABLE_SET flag has been
>> confirmed set via readback.
>>
>> The reasons for this failure are unknown, however it is likely that
>> this timer was never tested. Raspberry Pi's original devicetree did
>
> The timer is part of the CPU, and there are enough A76 implementations
> around to prove that it actually works. The same can be said for the
> GIC400 this is (supposedly) attached to.
> >> not include this timer interrupt; it was only introduced via a
>> suggestion[1] made in code review as part of the upstreaming process.
>> (Current RPi firmware versions do include this timer, but only because
>> they rebased on top of the upstreamed devicetree starting with
>> Linux 6.12)
>>
>> Until more is known about this non-firing timer interrupt, remove
>> the devicetree entry to enable RPi5 devices to boot.
>
> I'd like to understand the reason why the timer interrupt isn't being
> delivered *before* we paper over it, and not the other way
> around. Each of the CPUs definitely have an EL2 virtual timer, the GIC
> has a per-CPU interrupt, but somehow the two don't seem to be linked.
>
> Since DT is supposed to describe the HW, I'd expect someone from
> Broadcom or RPi to shine a light on this issue. Integration mistakes
> happen, and we work around them (see the handful of Samsung SoCs where
> the timer interrupt was simply not wired). But we absolutely need to
> know what we are dealing with beforehand.
>
> Finally, just hacking the DT is not enough. Assuming that the timer is
> indeed unusable, we need to cope with the fact that there are DTs
> describing it in the wild, as nobody should be forced to upgrade their
> DT in lockstep with the kernel. For that, you'd also need something
> like the patch below (untested, and in need of a proper commit
> message, which I expect the SoC vendor to provide).
Daniel, do you happen to know which 2712 SoC revision you have, whether
this is a C0 or D0 stepping?
We have an internal bug tracker item pertaining exactly to the virtual
timer interrupt connection however it affected a sister chip (77122) and
not 2712 AFAICT, now checking with the design team whether the same
happened on 2712.
Thanks!
--
Florian
^ permalink raw reply
* Re: [PATCH 0/4] iio: adc: new ti-ads112c14 driver
From: Jonathan Cameron @ 2026-06-21 19:14 UTC (permalink / raw)
To: David Lechner
Cc: Kurt Borja, Nuno Sá, Andy Shevchenko, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Nguyen Minh Tien, linux-iio,
devicetree, linux-kernel
In-Reply-To: <9b8d5cfc-e392-45aa-9adc-867c364dd36e@baylibre.com>
On Tue, 16 Jun 2026 13:16:46 -0500
David Lechner <dlechner@baylibre.com> wrote:
> On 6/16/26 12:26 PM, Kurt Borja wrote:
> > On Tue Jun 16, 2026 at 10:21 AM -05, David Lechner wrote:
> >> On 6/15/26 7:18 PM, Kurt Borja wrote:
> >>> On Mon Jun 15, 2026 at 4:59 PM -05, David Lechner (TI) wrote:
>
> ...
>
> >>>> All of these chips have in common that they are designed for use with
> >>>> RTDs and thermocouples and so they look very similar to each other in
> >>>> terms of wiring and feature set, even if the register maps are
> >>>> different. They are in the gray area where we could either keep them
> >>>> separate because they are just different enough, or we could do like
> >>>> we've done before with ad_sigma_delta and have a bit of an abstraction
> >>>> layer for the register differences and otherwise try to share as much
> >>>> code as possible. Normally, I would lean towards keeping them separate,
> >>>> but in this case, I'm considering trying to share code because the
> >>>> devicetree bindings for the inputs is complex and is going to be mostly
> >>>> the same across all of these chips.
> >>>
> >>> The channel configuration is indeed very similar for the three chips.
> >>> All three have IDAC, BOC and VREF configurations.
> >>
> >> Hmm... I forgot to include the burnout current in the DT bindings. Following
> >> the channel = "conditions for measurement" pattern that I have set out here
> >> I guess that would mean that we would need to have the same inputs twice
> >> when using the burnout. One "channel" would be the one used to do a "precision"
> >> measurement and the other would be the one to do open/short circuit detection.
> >>
> >>
> >> i2c {
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> adc@40 {
> >> compatible = "ti,ads112c14";
> >> reg = <0x40>;
> >>
> >> avdd-supply = <&avdd>;
> >> dvdd-supply = <&dvdd>;
> >>
> >> refp-supply = <&avdd>;
> >>
> >> #address-cells = <1>;
> >> #size-cells = <0>;
> >>
> >> channel@0 {
> >> reg = <0>;
> >> diff-channels = <1>, <2>;
> >> excitation-channels = <0>, <3>;
> >> excitation-current-microamp = <500>;
> >> current-chopping;
> >> ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> >> label = "rtd-precision";
> >> };
> >>
> >> channel@1 {
> >> reg = <0>;
> >> diff-channels = <1>, <2>;
> >> excitation-channels = <0>, <3>;
> >> excitation-current-microamp = <500>;
Maybe use an example with more stuff changing? Do we want same excitation
for burn out? I've no idea.
> >> burnout-current-nanoamp = <1000>;
> >> ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> >> label = "rtd-diagnostic";
> >> };
> >
> > This would mean we wouldn't be able to use iio_chan_spec .channel and
> > .channel2 to describe inputs because of duplicate sysfs attributes, no?
> >
>
> Yes, that is a bit unfortunate. At least there the labels to tell them
> apart. I guess we would just need to use consecutive channel and channel2
> when dynamically allocating the channels to avoid conflict.
From a very initial look, maybe do something similar to the folk have
been looking at for the more complex DDS devices where we have lots
of channels that are on the same 'wires'. Basically add a numbering
scheme to keep them reasonably separate - channel numbers are cheap.
Maybe first channel is 10->1f, second 20-2f etc. They are differential
so it will get ugly. Perhaps have a play around and see if there is
a reasonable channel naming scheme for this 'same inputs, different thing
being measured case'
I'm not yet sure I'm convinced that a separate channel model makes sense.
Even less in the DT given these are different settings for one channel.
That doesn't mean we don't split them up in the driver if channels
are the best implementation / ABI to userspace.
Basically nothing actually says diff-channels numbers match the
userspace ABI, so break that link.
>
> >>>> This makes things more flexible, but does make the driver a bit more
> >>>> complex. For example, knowing when the current output needs to be
> >>>> enabled or disabled. For now, I have chosen a lazy-enable where they
> >>>> are not turned on until the first measurement is taken that requires
> >>>> them, but then they stay on until another measurement is taken that
> >>>> doesn't require them. This can lead to some oddness with the diagnostic
> >>>> channels that may be measuring something that indirectly requires the
> >>>> current output (i.e. the external reference voltage when it is connected
> >>>> to a resistor rather than a power supply). This means you need to take
> >>>> a measurement that requires the current output to be enabled before the
> >>>> diagnostic channels will give accurate readings.
> >>>
> >>> This is the same approach I took around the BOC, it feels kinda hacky
> >>> but it makes sense. Just an idea I thought about just now: What if we
> >>> have an additional write-only "_enable" sysfs attribute for these
> >>> channels?
> >>
> >> I would not want to make a write-only attribute, we always want to be
> >> able to read back what the current state is.
> >
> > Yeah, I don't know why I said WO. Reading would be fine too.
> >
> >>
> >> Do you mean an _enable for just the BOC? I think I would do it like I
> >> suggested above instead.
> >
> > No, no just the BOC. The BOC, IDAC and rest of side effects. Thinking
> > about it some more, it would be a bit redundant but clearer if proper
> > documentation is provided.
> >
> I would be interested to see what Jonathan has to say about this too.
> Generally, his advice has been to avoid attributes that power things
> on and off if we can help it.
>
This is again a bit similar to the DDS case, but there we have more than
one on at a time (as controlling different parts of the signal - freq
/ phase / amplitude). Here we at least avoid that complexity.
I didn't really like that solution but we didn't come up with any other
way to support the complex stuff going on. Hence I'm not necessarily
suggesting to go that way here.
I think this may be a case of laying out different options in an ABI
doc then reviewing that to make sure we spot corner cases etc.
Thanks,
Jonathan
p.s. Sometimes it feels like the world just keeps getting more complex!
^ permalink raw reply
* Re: [PATCH 4/4] iio: adc: ti-ads112c14: add measurement channel support
From: Jonathan Cameron @ 2026-06-21 19:08 UTC (permalink / raw)
To: David Lechner (TI)
Cc: 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, 15 Jun 2026 17:00:02 -0500
"David Lechner (TI)" <dlechner@baylibre.com> 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.
>
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
Trivial stuff. Seems the bulk of discussion here is in the dt vs
channel mappings area. I'll probably reply to that in a few mins.
> @@ -449,25 +599,31 @@ static int ads112c14_write_raw(struct iio_dev *indio_dev,
> int val2, long mask)
> {
> struct ads112c14_data *data = iio_priv(indio_dev);
> + const int (*scale_avail)[2];
> + u8 *gain_val;
> +
> + if (chan->channel == ADS112C14_SYS_MON_CHANNEL_SHORT) {
> + scale_avail = data->sys_mon_chan_short_scale_available;
> + gain_val = &data->sys_mon_chan_short_gain_val;
> + } else if (chan->channel < 100) {
> + scale_avail = data->measurements[chan->scan_index].scale_available;
> + gain_val = &data->measurements[chan->scan_index].gain_val;
> + } else {
> + return -EINVAL;
> + }
>
> - 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;
> + IIO_DEV_ACQUIRE_DIRECT_MODE(indio_dev, claim);
> + if (IIO_DEV_ACQUIRE_FAILED(claim))
> + return -EBUSY;
Anything stop you doing this in the earlier patch?
>
> - 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;
> - }
> - default:
> - return -EINVAL;
> }
> +
> + return -EINVAL;
> }
>
>
> static int ads112c14_probe(struct i2c_client *client)
> @@ -547,6 +876,9 @@ static int ads112c14_probe(struct i2c_client *client)
> const struct ads112c14_chip_info *info;
> struct iio_dev *indio_dev;
> struct ads112c14_data *data;
> + bool need_avdd_ref, need_ext_ref;
> + u32 refp_uV = 0;
> + u32 refn_uV = 0;
> u32 reg_val;
> int ret;
>
>
> + if (device_property_present(dev, "refp-supply")) {
> + ret = devm_regulator_get_enable_read_voltage(&client->dev, "refp");
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get refp voltage\n");
> +
> + refp_uV = ret;
> +
> + struct fwnode_handle *refp_fwnode __free(fwnode_handle) =
> + fwnode_find_reference(dev->fwnode, "refp-supply", 0);
> + if (IS_ERR(refp_fwnode))
> + return dev_err_probe(dev, PTR_ERR(refp_fwnode),
> + "failed to get refp fwnode\n");
> +
> + struct fwnode_handle *avdd_fwnode __free(fwnode_handle) =
> + fwnode_find_reference(dev->fwnode, "avdd-supply", 0);
> + if (IS_ERR(avdd_fwnode))
> + return dev_err_probe(dev, PTR_ERR(avdd_fwnode),
> + "failed to get avdd fwnode\n");
> +
> + data->refp_is_avdd = refp_fwnode == avdd_fwnode;
Add a comment somewhere on why we care. I wonder how common this is. Maybe
a generic helper? Even if it isn't common lets have a helper here!
fwnode_same_reference() or something like that.
> + }
> +
> + if (device_property_present(dev, "refn-supply")) {
> + ret = devm_regulator_get_enable_read_voltage(&client->dev, "refn");
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to get refn voltage\n");
> +
> + refn_uV = ret;
> + } else {
> + data->refn_is_gnd = true;
> + }
> +
> + data->ext_ref_uV = refp_uV - refn_uV;
> +
> + 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");
> + } 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");
> + }
^ permalink raw reply
* Re: [PATCH RFC v4 01/12] dt-bindings: clk: zte: Add zx297520v3 top clock and reset bindings
From: Conor Dooley @ 2026-06-21 18:58 UTC (permalink / raw)
To: Stefan Dösinger
Cc: Michael Turquette, Stephen Boyd, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Brian Masney, linux-clk, devicetree,
linux-kernel, linux-arm-kernel
In-Reply-To: <vYm1twErR8mp-Fjgbvf-MQ@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3772 bytes --]
On Sat, Jun 20, 2026 at 08:28:03PM +0300, Stefan Dösinger wrote:
> Hi Conor,
> Am Donnerstag, 18. Juni 2026, 22:54:53 Ostafrikanische Zeit schrieb Conor
> Dooley:
>
> I think I get the gist of your suggestions. I have a few follow-up questions
> to make sure I understand things right:
>
> > I think aux bus makes perfect sense when you have a clock/reset
> > controller, but once you start expanding past that and you have reboot
> > or hwmon or hwspinlock then mfd starts to make sense.
>
> At what point does it make sense to move the bindings from bindings/clock to
> bindings/mfd? The controllers are still very clock-heavy. allwinner,*-
> prcm.yaml look like clock, reset, misc controllers in mfd/ whereas
> ingenic,cgu.yaml, sprd,sc9863a-clk.yaml and da8xx-cfgchip.txt are clock + misc
> drivers in clock/.
Yeah, to bindings/mfd or bindings/soc/<vendor>. Which I think is mostly
a judgement call. Two of your devices have at least three functions,
which I think is enough to make the claim that it's not just a clock
controller.
> Likewise for the node names: syscon@ or clock-controller@?
If you have syscon in the compatible, then I think it should be syscon
in the node name as it's more general and makes it clear the device
isn't just a clock controller.
> > You'd then have topclock that is a syscon + simple-mfd, matrixclk that is
> > a syscon and lsp that's using the aux bus. The topclock and matrixclock
> > would have dedicated and trivial drivers somewhere that have the mfd_cells
> > and call mfd_add_devices().
>
> Do I even need simple-mfd? It seems I can add the syscon-reboot node via
> mfd_cells too by setting .of_compatible. It seems once it has a driver (even a
> very short one) simple-mfd is misplaced.
If you don't need child nodes in dt, you don't need simple-mfd.
Whether setting of_compatible is a correct thing to do, I do not know,
sorry.
> What about syscon? Topclk needs it for syscon-reboot and the watchdog
> controls. For the other two I only want a regmap. Afaiu device_node_to_regmap
> works without a "syscon" compatible. There's also regmap_init_mmio, but afaics
> I only want this when my driver is the only one using the regmap.
If it is a miscellaneous system register region, then it should be a
syscon. These devices that perform multiple functions like hwspinlock,
clocks and resets fit that bill. Whether or not you "need" it for linux to
work, if it is a correct description of your hardware you need to use
that compatible.
>
> > Probably the compatibles you've chosen start to make less sense at this
> > point though, but probably "topclk" and "matrixclk" are not what the
> > documentation for this device calls these register regions?
>
> Yeah I'll rename them top topcrm / matrixcrm / lspcrm. I just stuck to the old
> names for this email.
>
> > I think the priority is having something that reflects the hardware
> > accurately, I wouldn't compromise on that just to have the same design
> > for all three drivers.
>
> As far as I can see the primary difference between mfd_add_devices and simple-
> mfd + child nodes is that the latter makes the MFD composition visible in the
> device tree and the former keeps it a driver implementation detail. My sense
> is that the latter is preferred unless a subcomponent of the MFD might be
> reused in other components - e.g. an ADC is used in PMIC-abc and PMIC-xyz and
> thus the driver can be reused as well.
Correct. The other reason for doing it the devicetree way is if there
are conflicting property requirements. E.g. two of the same class of
device, like a pair of pin control functions or devices that use
different #address-cells to one another.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 2/4] iio: adc: add ti-ads112c14 driver
From: Jonathan Cameron @ 2026-06-21 18:52 UTC (permalink / raw)
To: David Lechner (TI)
Cc: 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, 15 Jun 2026 17:00:00 -0500
"David Lechner (TI)" <dlechner@baylibre.com> 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.
>
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
> ---
>
> A few other notes for review that didn't seem worth putting in the
> commit message:
> * I intentionally did not use bulk regmap because later we may need to
> get the voltage of the avdd supply.
> * I left some comments in the code where the code might look funny (e.g.
> to reduce future diff) or does not exactly match the datasheet, in
> which case later changes will address that.
A few really small things inline.
> diff --git a/drivers/iio/adc/ti-ads112c14.c b/drivers/iio/adc/ti-ads112c14.c
> new file mode 100644
> index 000000000000..97097ae2a487
> --- /dev/null
> +++ b/drivers/iio/adc/ti-ads112c14.c
> @@ -0,0 +1,536 @@
> +
> +#define ADS112C14_REG_IDAC_MUX_CFG 0x0E
> +#define ADS112C14_IDAC_MUX_CFG_IUNIT BIT(7)
> +#define ADS112C14_IDAC_MUX_CFG_I2MUX GENMASK(6, 4)
> +#define ADS112C14_IDAC_MUX_CFG_I1MUX GENMASK(2, 0)
Maybe tweak the indent so the register vs fields is slightly easier
to see.
#define ADS112C14_REG_IDAC_MUX_CFG 0x0E
#define ADS112C14_IDAC_MUX_CFG_IUNIT BIT(7)
#define ADS112C14_IDAC_MUX_CFG_I2MUX GENMASK(6, 4)
#define ADS112C14_IDAC_MUX_CFG_I1MUX GENMASK(2, 0)
And deeper still for values where appropriate.
> +static const struct reg_default ads112c14_reg_defaults[] = {
> + { ADS112C14_REG_DEVICE_CFG, 0x00 },
> + { ADS112C14_REG_DATA_RATE_CFG, 0x00 },
> + { ADS112C14_REG_MUX_CFG, 0x00 },
> + { ADS112C14_REG_GAIN_CFG, 0x01 },
> + { ADS112C14_REG_REFERENCE_CFG, 0x00 },
> + { ADS112C14_REG_DIGITAL_CFG, 0x00 },
> + { ADS112C14_REG_GPIO_CFG, 0x00 },
> + { ADS112C14_REG_GPIO_DATA_OUTPUT, 0x00 },
> + { ADS112C14_REG_IDAC_MAG_CFG, 0x00 },
> + { ADS112C14_REG_IDAC_MUX_CFG, 0x10 },
If it is plausible to define these in terms of fields that make them
up I'd prefer that. It isn't always sensible to do so though as
they can get very messy.
> +};
>
> +
> +static int ads112c14_read_raw(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan, int *val,
> + int *val2, long mask)
> +{
> + struct ads112c14_data *data = iio_priv(indio_dev);
> + u32 vref_uV, fsr_bits;
> +
> + /* Selecting V_REF source is not implemented yet. */
> + vref_uV = ads112c14_internal_ref_uV[ADS112C14_REFERENCE_CFG_REF_VAL_2_5V];
> +
> + /* Currently, everything is using signed data. */
> + fsr_bits = data->chip_info->resolution_bits - 1;
> +
> + switch (mask) {
> + case IIO_CHAN_INFO_RAW: {
> + u8 buf[3];
> + int ret;
> +
> + if (!iio_device_claim_direct(indio_dev))
> + return -EBUSY;
> +
> + ret = ads112c14_single_conversion(data, chan, buf);
> + iio_device_release_direct(indio_dev);
> + if (ret < 0)
> + return ret;
> +
> + switch (data->chip_info->resolution_bits) {
> + case 16:
> + *val = get_unaligned_be16(buf);
> + break;
> + case 24:
> + *val = get_unaligned_be24(buf);
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + *val = sign_extend32(*val, fsr_bits);
> +
> + return IIO_VAL_INT;
> + }
> + case IIO_CHAN_INFO_SCALE:
> + if (chan->type == IIO_TEMP) {
> + /* TS_TC (typical) = 405 uV/°C */
> + *val = MILLI * vref_uV / 405;
> + *val2 = fsr_bits;
> + return IIO_VAL_FRACTIONAL_LOG2;
> + }
> +
> + *val = vref_uV / (MICRO / MILLI);
> + /*
> + * Last 3 SYS_MON channels (ext ref, AVDD, DVDD) need to be
> + * multiplied by 8 to account for internal attenuation of / 8.
> + */
> + *val2 = fsr_bits - (chan->address >= 3 ? 3 : 0);
> + return IIO_VAL_FRACTIONAL_LOG2;
> + case IIO_CHAN_INFO_OFFSET:
> + /* Only the temperature channel has an offset. */
> + if (chan->type != IIO_TEMP)
> + return -EINVAL;
> + /* Die temperature [°C] = 25°C + (Measured voltage – TS_Offset) / TS_TC */
> + /* TS_TC (typical) = 405 uV/°C */
> + /* TS_Offset (typical) = 119.5 mV */
Trivial but make that a multiline comment block given it is all related stuff.
> + *val = div_s64((s64)(25 * 405 - 119500) * BIT(fsr_bits), vref_uV);
> + return IIO_VAL_INT;
> + default:
> + return -EINVAL;
> + }
> +}
> +
> +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);
We currently (I think) still need to protect against a manual driver
override and that being NULL. Just error out if that happens as we
don't care about trying to support that.
> +
> + indio_dev = devm_iio_device_alloc(&client->dev, sizeof(*data));
> + if (!indio_dev)
^ permalink raw reply
* Re: [PATCH 1/4] dt-bindings: iio: adc: add ti,ads122c14
From: Jonathan Cameron @ 2026-06-21 18:41 UTC (permalink / raw)
To: David Lechner (TI)
Cc: 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-1-e6bdadf7cb2b@baylibre.com>
On Mon, 15 Jun 2026 16:59:59 -0500
"David Lechner (TI)" <dlechner@baylibre.com> wrote:
> Add new bindings for ti,ads122c14 and similar devices.
>
> This is an ADC that is primarily intended for use with temperature
> sensors. There are a few unusual properties because of this. In
> particular, the reference voltage source and current output requirements
> can be different for each measurement, so these are included in the
> channel bindings.
>
> The REFP/REFN reference voltage is usually just connected to a resistor
> that is being driven by the ADC's current outputs, so there is special
> property for this case rather than requiring a regulator to be defined
> to represent that.
>
> ti,vref-source is reused from ti,tlv320adcx140.yaml (otherwise might
> have preferred an enum of strings).
>
> Signed-off-by: David Lechner (TI) <dlechner@baylibre.com>
A few queries inline though I'm only just starting to get my head
around this device...
Thanks
Jonathan
> ---
> .../devicetree/bindings/iio/adc/ti,ads112c14.yaml | 224 +++++++++++++++++++++
> MAINTAINERS | 7 +
> include/dt-bindings/iio/adc/ti,ads112c14.h | 11 +
> 3 files changed, 242 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> new file mode 100644
> index 000000000000..dc7f37cad772
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/iio/adc/ti,ads112c14.yaml
> @@ -0,0 +1,224 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/iio/adc/ti,ads112c14.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Texas Instruments' ADS112C14 and similar ADC chips
> +
> +description: |
> + Supports the following Texas Instruments' ADC chips:
> + - ADS112C14 (16-bit)
> + - ADS122C14 (24-bit)
> +
> + https://www.ti.com/lit/ds/symlink/ads122c14.pdf
> +
> + These chips are primarily designed for use with temperature sensors such as
> + RTDs and thermocouples. The channel bindings reflect this in that each channel
> + represents the conditions required to make a measurement rather than strictly
> + just the physical input channels.
> +
> +maintainers:
> + - David Lechner <dlechner@baylibre.com>
> +
> +unevaluatedProperties: false
> +
> +properties:
> + compatible:
> + enum:
> + - ti,ads112c14
> + - ti,ads122c14
> +
> + reg:
> + items:
> + - minimum: 0x40
> + maximum: 0x47
> +
> + clocks:
> + maxItems: 1
> + description: Optional external clock connected to GPIO3 pin.
> +
> + avdd-supply: true
> + dvdd-supply: true
> +
> + refp-supply: true
> + refn-supply: true
> +
> + refp-refn-resistor-ohms:
> + description:
> + The resistance of the external resistor between REFP and REFN when using
> + resistor bridge driven by current outputs for RTD measurements.
> +
> + interrupts:
> + minItems: 1
> + items:
> + - description: FAULT interrupt (GPIO2 pin)
> + - description: DRDY interrupt (GPIO3 pin)
> +
> + interrupt-names:
> + minItems: 1
> + maxItems: 2
> + items:
> + enum: [fault, drdy]
> +
> + gpio-controller: true
> + '#gpio-cells':
> + const: 2
> +
> + '#address-cells':
> + const: 1
> +
> + '#size-cells':
> + const: 0
> +
> +patternProperties:
> + ^channel@[0-7]$:
> + $ref: adc.yaml
> +
> + unevaluatedProperties: false
> +
> + properties:
> + reg:
> + maximum: 16 # arbitrary limit, channel@ can be any combination of AIN0-AIN7
> +
> + single-channel:
> + maximum: 7
> +
> + diff-channels:
> + items:
> + maximum: 7
> +
> + bipolar:
> + description:
> + Set this flag if the differential input can be negative.
I'd leave that description to adc.yaml Maybe that doc could be improved though
given it basically says bipolar == bipolar mode ;)
> +
> + excitation-channels:
> + description: AINx pins used as current output.
> + $ref: /schemas/types.yaml#/definitions/uint32-array
> + minItems: 1
> + maxItems: 2
> + items:
> + maximum: 7
> +
> + excitation-current-microamp:
There seem to be separate controls. Are their usecases where this needs
to be in array?
> + description: The current output of the excitation channels in microamps.
> + minimum: 1
> + maximum: 1000
> +
> + current-chopping:
> + $ref: /schemas/types.yaml#/definitions/flag
> + description:
> + If provided, the two excitation channels are to be used with current
> + chopping enabled.
Can I have a reference for that? My initial read suggests it's the input channels
that are chopped. For GC_EN
"When enabled, the device automatically swaps
the analog inputs and takes the average of two consecutive conversions to
cancel the internal offset voltage"
> +
> + ti,vref-source:
> + description: |
> + Indicates the source for the reference voltage for this channel.
> + 0 - Internal 2.5V reference
> + 1 - Internal 1.25V reference
> + 2 - External reference (REFP-REFN)
> + 3 - AVDD as reference
> +
> + For convenience, macros for these values are available in
> + dt-bindings/iio/adc/ti,ads112c14.h.
> + $ref: /schemas/types.yaml#/definitions/uint32
> + maximum: 3
> + default: 0
> +
> + dependencies:
> + excitation-channels: [ excitation-current-microamp ]
> + excitation-current-microamp: [ excitation-channels ]
> + current-chopping: [ excitation-channels ]
> +
> + oneOf:
> + - required: [ single-channel ]
> + - required: [ diff-channels ]
> +examples:
> + - |
> + #include <dt-bindings/iio/adc/ti,ads112c14.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@40 {
> + compatible = "ti,ads112c14";
> + reg = <0x40>;
> +
> + avdd-supply = <&avdd>;
> + dvdd-supply = <&dvdd>;
> +
> + /* 3-Wire RTD: Two IDACs, One Measurement (AIN1-AIN2) */
> +
> + refp-refn-resistor-ohms = <500>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@0 {
> + reg = <0>;
> + diff-channels = <1>, <2>;
> + excitation-channels = <0>, <3>;
> + excitation-current-microamp = <500>;
> + current-chopping;
> + ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> + label = "rtd";
> + };
> + };
> + };
> + - |
> + #include <dt-bindings/iio/adc/ti,ads112c14.h>
> +
> + i2c {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + adc@40 {
> + compatible = "ti,ads112c14";
> + reg = <0x40>;
> +
> + avdd-supply = <&avdd>;
> + dvdd-supply = <&dvdd>;
> +
> + /* Resistive Bridge Measurement With a Thermistor for Temperature Compensation*/
> +
> + refp-supply = <&avdd>;
> +
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + channel@0 {
> + reg = <0>;
> + diff-channels = <6>, <7>;
> + bipolar;
> + ti,vref-source = <ADS112C14_VREF_SOURCE_EXTERNAL>;
> + label = "bridge";
> + };
> +
> + channel@1 {
> + reg = <1>;
> + diff-channels = <1>, <2>;
> + ti,vref-source = <ADS112C14_VREF_SOURCE_INTERNAL_2_5V>;
> + label = "thermistor";
Hmm. I'm interested to see where this goes, but generally when we have
a thermistor we attempt to ultimately convert it to a temperature
channel and I'm not seeing info here to allow us to do that.
> + };
> + };
> + };
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Conor Dooley @ 2026-06-21 18:35 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Nuno Sá, Janani Sunil, Rodrigo Alencar, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, David Lechner,
Nuno Sá, Andy Shevchenko, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-kernel, linux-doc, Mark Brown
In-Reply-To: <20260621153330.79b6600c@jic23-huawei>
[-- Attachment #1: Type: text/plain, Size: 15710 bytes --]
On Sun, Jun 21, 2026 at 03:33:40PM +0100, Jonathan Cameron wrote:
> On Fri, 19 Jun 2026 16:54:11 +0100
> Nuno Sá <noname.nuno@gmail.com> wrote:
>
> > On Fri, Jun 19, 2026 at 03:12:07PM +0100, Conor Dooley wrote:
> > > On Fri, Jun 19, 2026 at 02:01:08PM +0100, Nuno Sá wrote:
> > > > On Fri, Jun 19, 2026 at 12:40:54PM +0100, Conor Dooley wrote:
> > > > > On Fri, Jun 19, 2026 at 12:36:55PM +0100, Conor Dooley wrote:
> > > > > > On Fri, Jun 19, 2026 at 12:33:11PM +0200, Janani Sunil wrote:
> > > > > > >
> > > > > > > On 6/14/26 21:44, Jonathan Cameron wrote:
> > > > > > > > On Tue, 9 Jun 2026 16:47:23 +0200
> > > > > > > > Janani Sunil <jan.sun97@gmail.com> wrote:
> > > > > > > >
> > > > > > > > > On 5/26/26 15:11, Rodrigo Alencar wrote:
> > > > > > > > > > On 26/05/19 05:42PM, Janani Sunil wrote:
> > > > > > > > > > > Devicetree bindings for AD5529R 16 channel 12/16 bit high voltage,
> > > > > > > > > > > buffered voltage output digital-to-analog converter (DAC) with an
> > > > > > > > > > > integrated precision reference.
> > > > > > > > > > ...
> > > > > > > > > > Probably others may comment on that, but...
> > > > > > > > > >
> > > > > > > > > > This parent node may support device addressing for multi-device support through
> > > > > > > > > > those ID pins. I suppose that each device may have its own power supplies or
> > > > > > > > > > other resources like the toggle pins or reset and enable.
> > > > > > > > > >
> > > > > > > > > > That way I suppose that an example would look like...
> > > > > > > > > > > +
> > > > > > > > > > > +patternProperties:
> > > > > > > > > > > + "^channel@([0-9]|1[0-5])$":
> > > > > > > > > > > + type: object
> > > > > > > > > > > + description: Child nodes for individual channel configuration
> > > > > > > > > > > +
> > > > > > > > > > > + properties:
> > > > > > > > > > > + reg:
> > > > > > > > > > > + description: Channel number.
> > > > > > > > > > > + minimum: 0
> > > > > > > > > > > + maximum: 15
> > > > > > > > > > > +
> > > > > > > > > > > + adi,output-range-microvolt:
> > > > > > > > > > > + description: |
> > > > > > > > > > > + Output voltage range for this channel as [min, max] in microvolts.
> > > > > > > > > > > + If not specified, defaults to 0V to 5V range.
> > > > > > > > > > > + oneOf:
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: 0
> > > > > > > > > > > + - enum: [5000000, 10000000, 20000000, 40000000]
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: -5000000
> > > > > > > > > > > + - const: 5000000
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: -10000000
> > > > > > > > > > > + - const: 10000000
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: -15000000
> > > > > > > > > > > + - const: 15000000
> > > > > > > > > > > + - items:
> > > > > > > > > > > + - const: -20000000
> > > > > > > > > > > + - const: 20000000
> > > > > > > > > > > +
> > > > > > > > > > > + required:
> > > > > > > > > > > + - reg
> > > > > > > > > > > +
> > > > > > > > > > > + additionalProperties: false
> > > > > > > > > > > +
> > > > > > > > > > > +required:
> > > > > > > > > > > + - compatible
> > > > > > > > > > > + - reg
> > > > > > > > > > > + - vdd-supply
> > > > > > > > > > > + - avdd-supply
> > > > > > > > > > > + - hvdd-supply
> > > > > > > > > > > +
> > > > > > > > > > > +dependencies:
> > > > > > > > > > > + spi-cpha: [ spi-cpol ]
> > > > > > > > > > > + spi-cpol: [ spi-cpha ]
> > > > > > > > > > > +
> > > > > > > > > > > +allOf:
> > > > > > > > > > > + - $ref: /schemas/spi/spi-peripheral-props.yaml#
> > > > > > > > > > > +
> > > > > > > > > > > +unevaluatedProperties: false
> > > > > > > > > > > +
> > > > > > > > > > > +examples:
> > > > > > > > > > > + - |
> > > > > > > > > > > + #include <dt-bindings/gpio/gpio.h>
> > > > > > > > > > > +
> > > > > > > > > > > + spi {
> > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > +
> > > > > > > > > > > + dac@0 {
> > > > > > > > > > > + compatible = "adi,ad5529r-16";
> > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > + spi-max-frequency = <25000000>;
> > > > > > > > > > > +
> > > > > > > > > > > + vdd-supply = <&vdd_regulator>;
> > > > > > > > > > > + avdd-supply = <&avdd_regulator>;
> > > > > > > > > > > + hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > > + hvss-supply = <&hvss_regulator>;
> > > > > > > > > > > +
> > > > > > > > > > > + reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > > > +
> > > > > > > > > > > + #address-cells = <1>;
> > > > > > > > > > > + #size-cells = <0>;
> > > > > > > > > > > +
> > > > > > > > > > > + channel@0 {
> > > > > > > > > > > + reg = <0>;
> > > > > > > > > > > + adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > > + };
> > > > > > > > > > > +
> > > > > > > > > > > + channel@1 {
> > > > > > > > > > > + reg = <1>;
> > > > > > > > > > > + adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > > + };
> > > > > > > > > > > +
> > > > > > > > > > > + channel@2 {
> > > > > > > > > > > + reg = <2>;
> > > > > > > > > > > + adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > > + };
> > > > > > > > > > > + };
> > > > > > > > > > > + };
> > > > > > > > > > ...
> > > > > > > > > >
> > > > > > > > > > spi {
> > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > #size-cells = <0>;
> > > > > > > > > >
> > > > > > > > > > multi-dac@0 {
> > > > > > > > > > compatible = "adi,ad5529r-16";
> > > > > > > > > > reg = <0>;
> > > > > > > > > > spi-max-frequency = <25000000>;
> > > > > > > > > >
> > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > #size-cells = <0>;
> > > > > > > > > >
> > > > > > > > > > dac@0 {
> > > > > > > > > > reg = <0>;
> > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > >
> > > > > > > > > > reset-gpios = <&gpio0 87 GPIO_ACTIVE_LOW>;
> > > > > > > > > >
> > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > #size-cells = <0>;
> > > > > > > > > >
> > > > > > > > > > channel@0 {
> > > > > > > > > > reg = <0>;
> > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > channel@1 {
> > > > > > > > > > reg = <1>;
> > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > channel@2 {
> > > > > > > > > > reg = <2>;
> > > > > > > > > > adi,output-range-microvolt = <0 40000000>;
> > > > > > > > > > };
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > dac@1 {
> > > > > > > > > > reg = <1>;
> > > > > > > > > > vdd-supply = <&vdd_regulator>;
> > > > > > > > > > avdd-supply = <&avdd_regulator>;
> > > > > > > > > > hvdd-supply = <&hvdd_regulator>;
> > > > > > > > > > hvss-supply = <&hvss_regulator>;
> > > > > > > > > >
> > > > > > > > > > reset-gpios = <&gpio0 88 GPIO_ACTIVE_LOW>;
> > > > > > > > > >
> > > > > > > > > > #address-cells = <1>;
> > > > > > > > > > #size-cells = <0>;
> > > > > > > > > >
> > > > > > > > > > channel@0 {
> > > > > > > > > > reg = <0>;
> > > > > > > > > > adi,output-range-microvolt = <0 5000000>;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > channel@1 {
> > > > > > > > > > reg = <1>;
> > > > > > > > > > adi,output-range-microvolt = <(-10000000) 10000000>;
> > > > > > > > > > };
> > > > > > > > > > }
> > > > > > > > > > };
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > then you might need something like:
> > > > > > > > > >
> > > > > > > > > > patternProperties:
> > > > > > > > > > "^dac@[0-3]$":
> > > > > > > > > >
> > > > > > > > > > and put most of the things under this node pattern.
> > > > > > > > > >
> > > > > > > > > > So the main driver that you're putting together might need to handle up to four instances.
> > > > > > > > > > Even if your current driver cannot handle this, the dt-bindings might need cover that.
> > > > > > > > > >
> > > > > > > > > > Need to double check if each dac node needs a separate compatible, so you would maybe populate
> > > > > > > > > > a platform data to be shared with the child nodes, which would be a separate driver.
> > > > > > > > > > (not sure if it would make sense to mix and match ad5529r-16 and ad5529r-12).
> > > > > > > > > Hi Rodrigo,
> > > > > > > > >
> > > > > > > > > Thank you for looking at this.
> > > > > > > > >
> > > > > > > > > For now, I would prefer to keep the binding scoped to a single AD5529R device instance. The current
> > > > > > > > > hardware/use case we have only needs one device node and the driver is written around that model as well.
> > > > > > > > > While the device addressing pins could allow multi-device topology, we do not have an actual platform using
> > > > > > > > > that configuration at the moment, so I would prefer not to introduce an extra parent/child binding structure
> > > > > > > > > speculatively without a validating use case.
> > > > > > > > Interesting feature - kind of similar to address control on a typical i2c bus device, or
> > > > > > > > looking at it another way a kind of distributed SPI mux.
> > > > > > > >
> > > > > > > > Challenge of a binding is we need to anticipate the future. So I think we do need something
> > > > > > > > like Rodrigo is suggesting even if we only (for now) support a single instance in the driver.
> > > > > > > > That would leave the path open to supporting the addressing at a later date.
> > > > > > > > An alternative might be to look at it like a chained device setup. In those we pretend there
> > > > > > > > is just one device with a lot of channels etc. The snag is that here things are more loosely
> > > > > > > > coupled whereas for those devices it tends to be you have to read / write the same register
> > > > > > > > in all devices in the chain as one big SPI message.
> > > > > > > >
> > > > > > > > +CC Mark Brown as he may know of some precedence for this feature. For his reference..
> > > > > > > > - Each of these device has 2 ID pins. The SPI transfers have to contain the 2 bit
> > > > > > > > value that matches that or they are ignored. Thus a single bus + 1 chip select can
> > > > > > > > be used to talk to 4 devices. Question is what that looks like in device tree + I guess
> > > > > > > > longer term how to support it cleanly in SPI.
> > > > > >
> > > > > > I'd swear I have seen this before, from some Microchip devices. Let me
> > > > > > see if I can find what I am thinking of...
> > > > >
> > > > >
> > > > > microchip,mcp3911 and microchip,mcp3564 both seem to do this with
> > > > > slightly different properties.
> > > > >
> > > > > microchip,device-addr:
> > > > > description: Device address when multiple MCP3911 chips are present on the same SPI bus.
> > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > enum: [0, 1, 2, 3]
> > > > > default: 0
> > > > >
> > > > > and
> > > > >
> > > > >
> > > > > microchip,hw-device-address:
> > > > > $ref: /schemas/types.yaml#/definitions/uint32
> > > > > minimum: 0
> > > > > maximum: 3
> > > > > description:
> > > > > The address is set on a per-device basis by fuses in the factory,
> > > > > configured on request. If not requested, the fuses are set for 0x1.
> > > > > The device address is part of the device markings to avoid
> > > > > potential confusion. This address is coded on two bits, so four possible
> > > > > addresses are available when multiple devices are present on the same
> > > > > SPI bus with only one Chip Select line for all devices.
> > > > > Each device communication starts by a CS falling edge, followed by the
> > > > > clocking of the device address (BITS[7:6] - top two bits of COMMAND BYTE
> > > > > which is first one on the wire).
> > > > >
> > > > > This sounds exactly like the sort of feature that you're dealing with
> > > > > here?
> > > > >
> > > >
> > > > The core idea yes but for this chip, things are a bit more annoying (but
> > > > Janani can correct me if I'm wrong). Here, each device can, in theory,
> > > > have it's own supplies, pins and at the very least, channels with maybe
> > > > different scales. That is why Janani is proposing dac nodes. Given I
> > > > honestly don't like much of that "adi,ad5529r-bus" compatible I wondered
> > > > about solving this at the spi level.
> > > >
> > > > Ah and to make it more annoying, we can also mix 12 and 16 bits variants
> > > > together in the same bus.
> > >
> > > I'm definitely missing something, because that property for the
> > > microchip devices is not impacted what else is on the bus. AFAICT, you
> > > could have an mcp3911 and an mcp3564 on the same bus even though both
> > > are completely different devices with different drivers. They have
> > > individual device nodes and their own supplies etc etc. These aren't
> > > per-channel properties on an adc or dac, they're per child device on a
> > > spi bus.
> >
> > Maybe I'm the one missing something :). IIRC, spi would not allow two
> > devices on the same CS right? Because for this chip we would need
> > something like:
> >
> > spi {
> > dac@0 {
> > reg = <0>;
> > adi,pin-id = <0>;
> > };
> >
> > dac@1 {
> > reg = <0>; // which seems already problematic?
> > adi,pin-id <1>;
> > };
> >
> > ...
> >
> > //up to 4
> > };
> Yeah. It's not clear to me how that works for the microchip devices
> (I suspect it doesn't!)
>
> Just thinking as I type, but could we do something a bit nasty with
> a gpio mux that doesn't actually switch but represents the GPIO being
> shared? Given this is all tied to the spi bus that should all happen
> under serializing locks.
>
> Agreed though that this would be nicer as an SPI thing that let
> us specify that a single CS is share by multiple devices and their
> is some other signal acting to select which one we are talking to.
Whether it works or not, I think it is the more correct approach. Messing
with gpio muxes seems completely wrong, given the chip select may not be
a gpio at all.
Why do you think the microchip devices won't work? Does the spi core
reject multiple devices with the same chip select being registered or
something like that?
Certainly seems like an opportunity though to do something common
property wise, if both ADI and Microchip are trying to do the same thing
here with multiple users of the same chip select.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3] dt-bindings: misc: add binding for Xilinx AXI-Stream FIFO
From: Krzysztof Kozlowski @ 2026-06-21 18:33 UTC (permalink / raw)
To: Aditya Chari, robh, krzk+dt, conor+dt, gregkh
Cc: jacobsfeder, devicetree, linux-staging, linux-kernel
In-Reply-To: <20260621094312.53655-1-adi25charis@gmail.com>
On 21/06/2026 11:43, Aditya Chari wrote:
> The axis-fifo driver's compatible strings were undocumented, flagged
> by checkpatch.pl as UNDOCUMENTED_DT_STRING. Add a YAML devicetree
> binding document for drivers/staging/axis-fifo, converted from and
> replacing the existing free-form text binding (axis-fifo.txt), which
> this patch removes.
>
> Constrain xlnx,tx-fifo-depth to a minimum of 4, since the driver
> subtracts 4 from this value in its transmit bounds check and a
> smaller value would underflow that check.
>
> Signed-off-by: Aditya Chari <adi25charis@gmail.com>
> ---
>
> Changes since v2:
> - Added $ref: /schemas/types.yaml#/definitions/string to the three
> AXI-Stream protocol enum properties (xlnx,axi-str-rxd-protocol,
> xlnx,axi-str-txd-protocol, xlnx,axi-str-txc-protocol) for explicit
> type consistency with the rest of the schema.
> - Added minimum: 4 to xlnx,tx-fifo-depth, since the driver subtracts
> 4 from this value in its transmit bounds check
> (axis_fifo_write()) and a smaller configured value would underflow
> that unsigned check, bypassing the oversized-packet guard.
>
> Changes since v1:
> - Fixed xlnx,rx/tx-fifo-depth: depth is in 32-bit words, not bytes,
> matching the driver's overflow check in axis_fifo_write() and the
> wording of the original text binding.
> - Restored the full set of hardware-generated properties (interrupt-
> names, AXI-Stream protocol/width properties, has-axis-t* feature
> flags, fifo threshold properties, etc.) so that additionalProperties:
> false does not reject valid device trees generated for real hardware.
> - Removed the now-superseded axis-fifo.txt text binding.
Please slow down. Three versions within 1 hour! Why sending something
and immediately sending fixes to it?
>
> .../bindings/misc/xlnx,axi-fifo-mm-s.yaml | 227 ++++++++++++++++++
> drivers/staging/axis-fifo/axis-fifo.txt | 96 --------
Why are you touching staging binding?
https://lore.kernel.org/all/?q=dfn%3Aaxis-fifo.txt
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 2/2] iio: temperature: Add STS30 temperature sensor driver
From: Joshua Crofts @ 2026-06-21 18:33 UTC (permalink / raw)
To: Maxwell Doose
Cc: Jonathan Cameron, David Lechner, Nuno Sá, Andy Shevchenko,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
open list:IIO SUBSYSTEM AND DRIVERS,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <20260621004626.66629-3-m32285159@gmail.com>
On Sat, 20 Jun 2026 19:46:24 -0500
Maxwell Doose <m32285159@gmail.com> wrote:
> +#include <linux/array_size.h>
> +#include <linux/bits.h>
> +#include <linux/cleanup.h>
> +#include <linux/crc8.h>
> +#include <linux/delay.h>
> +#include <linux/device.h>
> +#include <linux/errno.h>
> +#include <linux/export.h>
> +#include <linux/i2c.h>
I am a numpty as I also forgot to mention a missing
mod_devicetable.h header.
--
Kind regards
CJD
^ permalink raw reply
* Re: [PATCH 1/3] dt-bindings: arm: qcom: Add HP EliteBook X G2q 14 AI
From: Krzysztof Kozlowski @ 2026-06-21 18:29 UTC (permalink / raw)
To: Jason Pettit, Bjorn Andersson, Konrad Dybcio, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Akhil P Oommen,
Mahadevan P, Sibi Sankar, Jingyi Wang, Ananthu C V
In-Reply-To: <20260620-glymur-send-v1-1-fc4a2cfd107c@oss.qualcomm.com>
On 21/06/2026 06:50, Jason Pettit wrote:
> The HP EliteBook X G2q 14 AI is a Snapdragon X2 Elite (Glymur) laptop.
> Document its top-level "hp,elitebook-x-g2q" compatible, falling back to
> the existing "qcom,glymur" SoC compatible.
Drop last sentence. It's pointless to explain what the diff is doing.
>
> Signed-off-by: Jason Pettit <jason.pettit@oss.qualcomm.com>
> Assisted-by: Claude:claude-opus-4-8
Your tag is supposed to be the last one. And really, making such commit
with Claude feels odd. This is a single one liner.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH V13 7/9] iio: imu: inv_icm42607: Add Accelerometer for icm42607
From: Jonathan Cameron @ 2026-06-21 17:36 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko, Chris Morgan
In-Reply-To: <20260615172554.160910-8-macroalpha82@gmail.com>
On Mon, 15 Jun 2026 12:25:50 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add icm42607 accelerometer sensor for icm42607.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
FWIW I think this has the same issue as the gyro patch around
management of the cache of what is enabled, but
this one sashiko gave a clean bill of health...
:)
^ permalink raw reply
* Re: [PATCH V13 8/9] iio: imu: inv_icm42607: Add Gyroscope to icm42607
From: Jonathan Cameron @ 2026-06-21 17:34 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko, Chris Morgan
In-Reply-To: <20260615172554.160910-9-macroalpha82@gmail.com>
On Mon, 15 Jun 2026 12:25:51 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add gyroscope functions to the icm42607 driver.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
There is another bit of sashiko stuff in here. Please
take a look
https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com
I think it is correct about there being a path in which the
gyro ends up always enabled along side anything else after
the first read.
In general there seems to be a bit of mix on whether the caller
or the power management function should be responsible for the caching
of state.
I know you look at Sashiko so I could just have waited a bit, but
I was reviewing anyway so took a look and having done that might
as well highlight some stuff!
Jonathan
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index eb239987a1ce..23ca7529825c 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -218,6 +218,49 @@ int inv_icm42607_set_accel_conf(struct inv_icm42607_state *st,
> st->conf.temp_en, sleep_ms);
> }
>
> +int inv_icm42607_set_gyro_conf(struct inv_icm42607_state *st,
> + struct inv_icm42607_sensor_conf *conf,
> + unsigned int *sleep_ms)
> +{
> + struct inv_icm42607_sensor_conf *oldconf = &st->conf.gyro;
> + unsigned int val;
> + int ret;
> +
> + if (conf->mode < 0)
> + conf->mode = oldconf->mode;
> + if (conf->fs < 0)
> + conf->fs = oldconf->fs;
> + if (conf->odr < 0)
> + conf->odr = oldconf->odr;
> + if (conf->filter < 0)
> + conf->filter = oldconf->filter;
> +
> + if (conf->fs != oldconf->fs || conf->odr != oldconf->odr) {
> + val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_FS_SEL_MASK,
> + conf->fs);
> + val |= FIELD_PREP(INV_ICM42607_GYRO_CONFIG0_ODR_MASK,
> + conf->odr);
> + ret = regmap_write(st->map, INV_ICM42607_REG_GYRO_CONFIG0, val);
> + if (ret)
> + return ret;
> + oldconf->fs = conf->fs;
> + oldconf->odr = conf->odr;
> + }
> +
> + if (conf->filter != oldconf->filter) {
> + val = FIELD_PREP(INV_ICM42607_GYRO_CONFIG1_FILTER_MASK,
> + conf->filter);
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_GYRO_CONFIG1,
> + INV_ICM42607_GYRO_CONFIG1_FILTER_MASK, val);
> + if (ret)
> + return ret;
> + oldconf->filter = conf->filter;
> + }
> +
> + return inv_icm42607_set_pwr_mgmt0(st, conf->mode, st->conf.accel.mode,
> + st->conf.temp_en, sleep_ms);
> +}
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> new file mode 100644
> index 000000000000..ef73560b39d7
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_gyro.c
> +static int inv_icm42607_gyro_read_sensor(struct iio_dev *indio_dev,
> + struct iio_chan_spec const *chan,
> + s16 *val)
> +{
> + struct inv_icm42607_sensor_conf conf = INV_ICM42607_SENSOR_CONF_INIT;
> + struct inv_icm42607_state *st = iio_device_get_drvdata(indio_dev);
> + struct inv_icm42607_sensor_state *gyro_st = iio_priv(indio_dev);
> + struct device *dev = regmap_get_device(st->map);
> + unsigned int reg;
> + u8 data[2];
> + int ret;
> +
> + if (chan->type != IIO_ANGL_VEL)
> + return -EINVAL;
> +
> + switch (chan->channel2) {
> + case IIO_MOD_X:
> + reg = INV_ICM42607_REG_GYRO_DATA_X1;
> + break;
> + case IIO_MOD_Y:
> + reg = INV_ICM42607_REG_GYRO_DATA_Y1;
> + break;
> + case IIO_MOD_Z:
> + reg = INV_ICM42607_REG_GYRO_DATA_Z1;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&st->lock);
> +
> + /* enable gyro sensor */
> + conf.mode = gyro_st->power_mode;
> + conf.filter = gyro_st->filter;
> + ret = inv_icm42607_set_gyro_conf(st, &conf, NULL);
The sashiko report is basically:
This turns it on, and runtime pm will turn it off but the
state cached ends up such that it is turned on again for
any runtime pm resume.
> + if (ret)
> + return ret;
> +
> + /* read gyro register data */
> + ret = regmap_bulk_read(st->map, reg, data, sizeof(data));
> + if (ret)
> + return ret;
> +
> + *val = get_unaligned_be16(data);
> + if (*val == INV_ICM42607_DATA_INVALID)
> + return -EINVAL;
> +
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH V13 6/9] iio: imu: inv_icm42607: Add Temp Support in icm42607
From: Jonathan Cameron @ 2026-06-21 17:26 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko, Chris Morgan
In-Reply-To: <20260615172554.160910-7-macroalpha82@gmail.com>
On Mon, 15 Jun 2026 12:25:49 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add functions for reading temperature sensor data.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
Another sashiko reported thing. I'd definitely have missed
this one and I think it is correct.
When I get caught up I'll post a thread to see if people
feel we should generally just ask for Sashiko to reply on
list.
Jonathan
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 64f5d263de4f..644cd7f821b9 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -162,6 +162,24 @@ static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> return 0;
> }
>
> +int inv_icm42607_set_temp_conf(struct inv_icm42607_state *st, bool enable,
> + unsigned int *sleep_ms)
> +{
> + unsigned int val;
> + int ret;
> +
> + val = FIELD_PREP(INV_ICM42607_TEMP_CONFIG0_FILTER_MASK,
> + INV_ICM42607_FILTER_BW_34HZ);
> + ret = regmap_update_bits(st->map, INV_ICM42607_REG_TEMP_CONFIG0,
> + INV_ICM42607_TEMP_CONFIG0_FILTER_MASK, val);
> + if (ret)
> + return ret;
> +
> + return inv_icm42607_set_pwr_mgmt0(st, st->conf.gyro.mode,
> + st->conf.accel.mode, enable,
> + sleep_ms);
> +}
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> new file mode 100644
> index 000000000000..9a60e1a478b0
> --- /dev/null
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_temp.c
> +static int inv_icm42607_temp_read(struct inv_icm42607_state *st, s16 *temp)
> +{
> + struct device *dev = regmap_get_device(st->map);
> + u8 raw[2];
> + int ret;
> +
> + PM_RUNTIME_ACQUIRE_AUTOSUSPEND(dev, pm);
> + ret = PM_RUNTIME_ACQUIRE_ERR(&pm);
> + if (ret)
> + return ret;
> +
> + guard(mutex)(&st->lock);
> +
> + st->conf.temp_en = true;
Sashiko points out (seems right) that this sets the internal state
before the power management routine expects it to be set. So why
is this here as opposed to just passing true into the
function that follows?
> + ret = inv_icm42607_set_temp_conf(st, st->conf.temp_en, NULL);
> + st->conf.temp_en = false;
> + if (ret)
> + return ret;
> +
> + ret = regmap_bulk_read(st->map, INV_ICM42607_REG_TEMP_DATA1,
> + raw, sizeof(raw));
> + if (ret)
> + return ret;
> +
> + *temp = get_unaligned_be16(raw);
> + if (*temp == INV_ICM42607_DATA_INVALID)
> + return -EINVAL;
> +
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH V13 5/9] iio: imu: inv_icm42607: Add PM support for icm42607
From: Jonathan Cameron @ 2026-06-21 17:19 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko, Chris Morgan
In-Reply-To: <20260615172554.160910-6-macroalpha82@gmail.com>
On Mon, 15 Jun 2026 12:25:48 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add power management support for the ICM42607 device driver.
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
A few things from taking a look at the sashiko report:
https://sashiko.dev/#/patchset/20260615172554.160910-1-macroalpha82%40gmail.com
> ---
> drivers/iio/imu/inv_icm42607/inv_icm42607.h | 18 +++
> .../iio/imu/inv_icm42607/inv_icm42607_core.c | 139 ++++++++++++++++++
> .../iio/imu/inv_icm42607/inv_icm42607_i2c.c | 1 +
> .../iio/imu/inv_icm42607/inv_icm42607_spi.c | 1 +
> 4 files changed, 159 insertions(+)
>
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607.h b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> index a6a58571935f..4f4f541027dc 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607.h
> @@ -334,11 +345,18 @@ struct inv_icm42607_state {
> #define INV_ICM42607_GYRO_STOP_TIME_MS 45
> #define INV_ICM42607_TEMP_STARTUP_TIME_MS 77
>
> +/*
> + * Suspend delay assumed from other icm42600 series device, not
> + * documented in datasheet.
> + */
> +#define INV_ICM42607_SUSPEND_DELAY_MS (2 * USEC_PER_MSEC)
Sashiko had a valid comment on this. MSEC_PER_SEC seems more
appropriate given this is 2 seconds in milli seconds.
> +
> typedef int (*inv_icm42607_bus_setup)(struct inv_icm42607_state *);
>
> extern const struct regmap_config inv_icm42607_regmap_config;
> extern const struct inv_icm42607_hw inv_icm42607_hw_data;
> extern const struct inv_icm42607_hw inv_icm42607p_hw_data;
> +extern const struct dev_pm_ops inv_icm42607_pm_ops;
>
> int inv_icm42607_core_probe(struct regmap *regmap,
> const struct inv_icm42607_hw *hw,
> diff --git a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> index 4b8e19091786..64f5d263de4f 100644
> --- a/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> +++ b/drivers/iio/imu/inv_icm42607/inv_icm42607_core.c
> @@ -4,6 +4,7 @@
> */
>
> #include <linux/bitfield.h>
> +#include <linux/cleanup.h>
> #include <linux/delay.h>
> #include <linux/dev_printk.h>
> #include <linux/device/devres.h>
> @@ -11,6 +12,7 @@
> #include <linux/iio/iio.h>
> #include <linux/module.h>
> #include <linux/mutex.h>
> +#include <linux/pm_runtime.h>
> #include <linux/regmap.h>
> #include <linux/regulator/consumer.h>
> #include <linux/time.h>
> @@ -103,6 +105,63 @@ const struct inv_icm42607_hw inv_icm42607p_hw_data = {
> };
> EXPORT_SYMBOL_NS_GPL(inv_icm42607p_hw_data, "IIO_ICM42607");
>
> +static int inv_icm42607_set_pwr_mgmt0(struct inv_icm42607_state *st,
> + enum inv_icm42607_sensor_mode gyro,
> + enum inv_icm42607_sensor_mode accel,
> + bool temp, unsigned int *sleep_ms)
> +{
> + enum inv_icm42607_sensor_mode oldaccel = st->conf.accel.mode;
> + enum inv_icm42607_sensor_mode oldgyro = st->conf.gyro.mode;
> + bool oldtemp = st->conf.temp_en;
> + unsigned int sleepval_ms;
> + unsigned int val;
> + int ret;
> +
> + if (gyro == oldgyro && accel == oldaccel && temp == oldtemp)
> + return 0;
> +
> + /*
> + * Datasheet on page 14.26 says we need to ensure the gyro sensor is on
> + * for a minimum of 45ms. So if we transition from an on state to an
> + * off state wait 45ms to ensure a sufficient pause before power off.
Sashiko commented on this.. I think what we could do with adding to the
comment is what the path is that didn't pass through this function which would
ensure we have been on for 30 of this msecs already.
> + */
> + if (!gyro && oldgyro)
> + fsleep(INV_ICM42607_GYRO_STOP_TIME_MS * USEC_PER_MSEC);
> +
> + val = FIELD_PREP(INV_ICM42607_PWR_MGMT0_GYRO_MODE_MASK, gyro);
> + val |= FIELD_PREP(INV_ICM42607_PWR_MGMT0_ACCEL_MODE_MASK, accel);
> + ret = regmap_write(st->map, INV_ICM42607_REG_PWR_MGMT0, val);
> + if (ret)
> + return ret;
> +
> + st->conf.gyro.mode = gyro;
> + st->conf.accel.mode = accel;
> + st->conf.temp_en = temp;
> +
> + /*
> + * If a state change occurs from off to on, sleep for the startup
> + * time of the sensor, unless a sleep_ms is specified. Since more
> + * than one sensor can be transitioned from off to on, select the
> + * maximum time from each of the sensors changing from off to on.
> + */
> + sleepval_ms = 0;
> + if (temp && !oldtemp)
> + sleepval_ms = max(sleepval_ms, INV_ICM42607_TEMP_STARTUP_TIME_MS);
> +
> + if (accel && !oldaccel)
> + sleepval_ms = max(sleepval_ms, INV_ICM42607_ACCEL_STARTUP_TIME_MS);
> +
> + if (gyro && !oldgyro)
> + sleepval_ms = max(sleepval_ms, INV_ICM42607_GYRO_STARTUP_TIME_MS);
> +
> + if (sleep_ms)
> + *sleep_ms = sleepval_ms;
> + else if (sleepval_ms)
> + fsleep(sleepval_ms * USEC_PER_MSEC);
> +
> + return 0;
> +}
>
> int inv_icm42607_core_probe(struct regmap *regmap,
> @@ -236,6 +305,8 @@ int inv_icm42607_core_probe(struct regmap *regmap,
> if (!st)
> return -ENOMEM;
>
> + dev_set_drvdata(dev, st);
> +
> ret = devm_mutex_init(dev, &st->lock);
> if (ret)
> return ret;
> @@ -271,10 +342,78 @@ int inv_icm42607_core_probe(struct regmap *regmap,
> if (ret)
> return ret;
>
> + ret = devm_pm_runtime_set_active_enabled(dev);
> + if (ret)
> + return ret;
> +
> + pm_runtime_set_autosuspend_delay(dev, INV_ICM42607_SUSPEND_DELAY_MS);
> + pm_runtime_use_autosuspend(dev);
Sashiko does put out some stuff here. Please take a look and work out or
test if it is right (I think not but haven't checked that carefully!)
From a quick look I think that the auto disabling of autosuspend does a
rpm_idle() that should result in it suspending...
> +
> return 0;
> }
> EXPORT_SYMBOL_NS_GPL(inv_icm42607_core_probe, "IIO_ICM42607");
^ permalink raw reply
* Re: [PATCH V13 2/9] dt-bindings: iio: imu: icm42600: Add icm42607
From: Jonathan Cameron @ 2026-06-21 17:18 UTC (permalink / raw)
To: Chris Morgan
Cc: linux-iio, andy, nuno.sa, dlechner, jean-baptiste.maneyrol,
linux-rockchip, devicetree, heiko, conor+dt, krzk+dt, robh,
andriy.shevchenko, Chris Morgan, Krzysztof Kozlowski
In-Reply-To: <20260615172554.160910-3-macroalpha82@gmail.com>
On Mon, 15 Jun 2026 12:25:45 -0500
Chris Morgan <macroalpha82@gmail.com> wrote:
> From: Chris Morgan <macromorgan@hotmail.com>
>
> Add the ICM42607 and ICM42607P inertial measurement unit.
>
> This device is functionally very similar to the icm42600 series with a
> very different register layout. The driver does not require an
> interrupt for these specific chip revisions.
This talks about how they are different from other parts already
supported, but I think we need something on how they are different
from each other.
I'm kind of assuming one is a subset of the other? If that is the
case the ideal might be the more complex part falling back to the
simpler one.
Thanks,
Jonathan
>
> Signed-off-by: Chris Morgan <macromorgan@hotmail.com>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> ---
> .../bindings/iio/imu/invensense,icm42600.yaml | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> index 9b2af104f186..81b6e85decd5 100644
> --- a/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> +++ b/Documentation/devicetree/bindings/iio/imu/invensense,icm42600.yaml
> @@ -30,6 +30,8 @@ properties:
> - invensense,icm42600
> - invensense,icm42602
> - invensense,icm42605
> + - invensense,icm42607
> + - invensense,icm42607p
> - invensense,icm42622
> - invensense,icm42631
> - invensense,icm42686
> @@ -67,10 +69,24 @@ properties:
> required:
> - compatible
> - reg
> - - interrupts
>
> allOf:
> - $ref: /schemas/spi/spi-peripheral-props.yaml#
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - invensense,icm42600
> + - invensense,icm42602
> + - invensense,icm42605
> + - invensense,icm42622
> + - invensense,icm42631
> + - invensense,icm42686
> + - invensense,icm42688
> + then:
> + required:
> + - interrupts
>
> unevaluatedProperties: false
>
^ permalink raw reply
* Re: [PATCH v2 2/2] arm64: dts: qcom: sdm845-oneplus: Update compatible to include model
From: David Heidelberg @ 2026-06-21 17:11 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Jason A. Donenfeld, Matthias Schiffer, Vincent Huang,
Bjorn Andersson, Konrad Dybcio, linux-input, devicetree,
linux-kernel, linux-arm-msm, phone-devel, Krzysztof Kozlowski,
Konrad Dybcio
In-Reply-To: <1d0e7e31-f808-4347-955a-7246dea208f5@ixit.cz>
On 28/05/2026 00:13, David Heidelberg wrote:
> On 27/05/2026 23:56, Dmitry Torokhov wrote:
>> Hi David,
>>
>> On Sat, May 23, 2026 at 11:45:35AM +0200, David Heidelberg via B4 Relay wrote:
>>> From: David Heidelberg <david@ixit.cz>
>>>
>>> We know the driver is reporting s3706b, introduce the compatible so we
>>> can more easily introduce quirks for weird touchscreen replacements in
>>> followup series.
>>>
>>> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>>> Signed-off-by: David Heidelberg <david@ixit.cz>
>>> ---
>>> arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi b/arch/
>>> arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> index 6b7378cf4d493..148164d456a5a 100644
>>> --- a/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> +++ b/arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi
>>> @@ -475,17 +475,17 @@ bq27441_fg: bq27441-battery@55 {
>>> };
>>> };
>>> &i2c12 {
>>> status = "okay";
>>> clock-frequency = <400000>;
>>> synaptics-rmi4-i2c@20 {
>>> - compatible = "syna,rmi4-i2c";
>>> + compatible = "syna,rmi4-s3706b", "syna,rmi4-i2c";
>>
>> So I believe we established that this device (s3706b) does not in fact
>> implement rmi4 protocol properly. Why do we have "syna,rmi4-i2c" as a
>> fallback? Shouldn't it be just "syna,rmi4-s3706b"?
>
> The vendor supplies s3706b which does implement the RMI4 properly.
>
> The 3rd party replacement impersonating original parts may not implement it
> properly, but I don't address this issue in this initial submission.
>
> With this compatible we know which original part is used by the vendor and
> installed in the phones, so later we can deduct specific sequences for the
> replacement aftermarket parts to keep phone touchscreen working same as they do
> on Android without affecting other devices.
Hello Dmitry.
May I ask what is currently preventing this series from moving forward?
The first version was posted in 2023 [1]. I picked it up again in 2025 [2] and
am now on the 9th iteration (this patchset). At this point, the series has been
under discussion for well over a year, with relatively little feedback and
increasingly long gaps between review rounds.
The current approach is based on the guidance I have received so far, including
suggestions from the device-tree maintainers. When concerns were raised, I tried
to address them and rework the series accordingly.
What I am struggling with is understanding what specific issue still needs to be
resolved before these patches can be accepted. If there are remaining
requirements, objections to the approach, or technical concerns that I have not
addressed, I would appreciate having them stated explicitly so I can work on them.
I also split out the straightforward, self-contained changes in the hope that at
least those could progress independently while I continued working on any
follow-up requirements. However, even those patches do not appear to be moving
forward.
Could you please clarify what outcome you would like to see from this series,
and what concrete changes would be required to get it accepted?
Thank you,
David
[1]
https://lore.kernel.org/all/20230929-caleb-rmi4-quirks-v1-0-cc3c703f022d@linaro.org/
[2] https://lore.kernel.org/all/20250402-synaptics-rmi4-v4-0-1bb95959e564@ixit.cz/
>
> David
>
>>
>> Thanks.
>>
>
--
David Heidelberg
^ 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