* [PATCH v18 2/4] arm64: dts: renesas: r9a08g046: Add SDHI nodes for RZ/G3L SoC and SDHI1 pincontrol on SMARC EVK
From: Biju @ 2026-06-22 16:48 UTC (permalink / raw)
To: Geert Uytterhoeven, Magnus Damm, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Biju Das, linux-renesas-soc, devicetree, linux-kernel,
Prabhakar Mahadev Lad, Biju Das
In-Reply-To: <20260622164819.184674-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
Add device tree nodes for the three SDHI controllers (SDHI{0,1,2})
on the RZ/G3L SoC (r9a08g046) and enable SDHI1 on the RZ/G3L SMARC
EVK platform with pincontrol and GPIO-based voltage switching
regulator support.
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v17->v18:
* No change.
* This patch depend on [1]
[1] https://lore.kernel.org/all/20260622155610.184271-2-biju.das.jz@bp.renesas.com/
v1->v17:
* No change.
---
arch/arm64/boot/dts/renesas/r9a08g046.dtsi | 73 ++++++++++++++-
.../boot/dts/renesas/r9a08g046l48-smarc.dts | 88 +++++++++++++++++++
2 files changed, 160 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
index c63a857f0e5b..ff2de3f192b5 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g046.dtsi
@@ -762,9 +762,80 @@ dmac: dma-controller@11820000 {
dma-channels = <16>;
};
+ sdhi0: mmc@11c00000 {
+ compatible = "renesas,sdhi-r9a08g046";
+ reg = <0x0 0x11c00000 0 0x10000>;
+ interrupts = <GIC_SPI 130 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_SDHI0_IMCLK>,
+ <&cpg CPG_MOD R9A08G046_SDHI0_CLK_HS>,
+ <&cpg CPG_MOD R9A08G046_SDHI0_IMCLK2>,
+ <&cpg CPG_MOD R9A08G046_SDHI0_IACLKS>,
+ <&cpg CPG_MOD R9A08G046_SDHI0_IACLKM>;
+ clock-names = "core", "clkh", "cd", "aclk", "aclkm";
+ max-frequency = <150000000>;
+ resets = <&cpg R9A08G046_SDHI0_IXRST>,
+ <&cpg R9A08G046_SDHI0_IXRSTAXIM>,
+ <&cpg R9A08G046_SDHI0_IXRSTAXIS>;
+ reset-names = "rst", "axim", "axis";
+ power-domains = <&cpg>;
+ status = "disabled";
+ };
+
sdhi1: mmc@11c10000 {
+ compatible = "renesas,sdhi-r9a08g046";
reg = <0x0 0x11c10000 0 0x10000>;
- /* placeholder */
+ interrupts = <GIC_SPI 132 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 133 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_SDHI1_IMCLK>,
+ <&cpg CPG_MOD R9A08G046_SDHI1_CLK_HS>,
+ <&cpg CPG_MOD R9A08G046_SDHI1_IMCLK2>,
+ <&cpg CPG_MOD R9A08G046_SDHI1_IACLKS>,
+ <&cpg CPG_MOD R9A08G046_SDHI1_IACLKM>;
+ clock-names = "core", "clkh", "cd", "aclk", "aclkm";
+ max-frequency = <150000000>;
+ resets = <&cpg R9A08G046_SDHI1_IXRST>,
+ <&cpg R9A08G046_SDHI1_IXRSTAXIM>,
+ <&cpg R9A08G046_SDHI1_IXRSTAXIS>;
+ reset-names = "rst", "axim", "axis";
+ power-domains = <&cpg>;
+ status = "disabled";
+
+ sdhi1_vqmmc: vqmmc-regulator {
+ regulator-name = "SDHI1-VQMMC";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-ramp-delay = <1200>;
+ status = "disabled";
+ };
+ };
+
+ sdhi2: mmc@11c20000 {
+ compatible = "renesas,sdhi-r9a08g046";
+ reg = <0x0 0x11c20000 0 0x10000>;
+ interrupts = <GIC_SPI 134 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 135 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cpg CPG_MOD R9A08G046_SDHI2_IMCLK>,
+ <&cpg CPG_MOD R9A08G046_SDHI2_CLK_HS>,
+ <&cpg CPG_MOD R9A08G046_SDHI2_IMCLK2>,
+ <&cpg CPG_MOD R9A08G046_SDHI2_IACLKS>,
+ <&cpg CPG_MOD R9A08G046_SDHI2_IACLKM>;
+ clock-names = "core", "clkh", "cd", "aclk", "aclkm";
+ max-frequency = <150000000>;
+ resets = <&cpg R9A08G046_SDHI2_IXRST>,
+ <&cpg R9A08G046_SDHI2_IXRSTAXIM>,
+ <&cpg R9A08G046_SDHI2_IXRSTAXIS>;
+ reset-names = "rst", "axim", "axis";
+ power-domains = <&cpg>;
+ status = "disabled";
+
+ sdhi2_vqmmc: vqmmc-regulator {
+ regulator-name = "SDHI2-VQMMC";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ regulator-ramp-delay = <1200>;
+ status = "disabled";
+ };
};
eth0: ethernet@11c30000 {
diff --git a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
index 5289efd1a430..0b6b7e109200 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
+++ b/arch/arm64/boot/dts/renesas/r9a08g046l48-smarc.dts
@@ -14,6 +14,7 @@
#define SW_GPIO4 1
#define SW_I3C_EN 0
#define SW_SER0_PMOD 1
+#define SW_SDIO_M2E 0
#define PMOD_GPIO4 0
#define PMOD_GPIO6 0
@@ -38,6 +39,7 @@ / {
aliases {
i2c2 = &i2c2;
i2c3 = &i2c3;
+ mmc1 = &sdhi1;
serial0 = &rsci2;
serial1 = &rsci3;
serial2 = &rsci1;
@@ -69,6 +71,19 @@ codec_dai: codec {
};
};
#endif
+
+#if RZ_BOOT_MODE3
+ vqmmc_sd1_pvdd: regulator-vqmmc-sd1-pvdd {
+ compatible = "regulator-gpio";
+ regulator-name = "SD1_PVDD";
+ regulator-min-microvolt = <1800000>;
+ regulator-max-microvolt = <3300000>;
+ gpios = <&pinctrl RZG3L_GPIO(J, 1) GPIO_ACTIVE_HIGH>;
+ gpios-states = <0>;
+ states = <3300000 0>, <1800000 1>;
+ regulator-ramp-delay = <1200>;
+ };
+#endif
};
&i2c2 {
@@ -175,6 +190,68 @@ scif0_pins: scif0 {
power-source = <1800>;
};
+#if RZ_BOOT_MODE3
+ sd1-pwr-en-hog {
+ gpio-hog;
+ gpios = <RZG3L_GPIO(J, 2) GPIO_ACTIVE_HIGH>;
+ output-high;
+ line-name = "sd1_pwr_en";
+ };
+#endif
+
+ sdhi1_pins: sd1 {
+ sd1-cd {
+ pinmux = <RZG3L_PORT_PINMUX(J, 0, 8)>; /* SD1_CD */
+ };
+
+ sd1-clk {
+ pinmux = <RZG3L_PORT_PINMUX(G, 0, 1)>; /* SD1_CLK */
+ power-source = <3300>;
+ };
+
+ sd1-cmd {
+ pinmux = <RZG3L_PORT_PINMUX(G, 1, 1)>; /* SD1_CMD */
+ input-enable;
+ power-source = <3300>;
+ bias-pull-up;
+ };
+
+ sd1-data {
+ pinmux = <RZG3L_PORT_PINMUX(G, 2, 1)>, /* SD1_DAT0 */
+ <RZG3L_PORT_PINMUX(G, 3, 1)>, /* SD1_DAT1 */
+ <RZG3L_PORT_PINMUX(G, 4, 1)>, /* SD1_DAT2 */
+ <RZG3L_PORT_PINMUX(G, 5, 1)>; /* SD1_DAT3 */
+ input-enable;
+ power-source = <3300>;
+ };
+ };
+
+ sdhi1_uhs_pins: sd1-uhs {
+ sd1-cd {
+ pinmux = <RZG3L_PORT_PINMUX(J, 0, 8)>; /* SD1_CD */
+ };
+
+ sd1-clk {
+ pinmux = <RZG3L_PORT_PINMUX(G, 0, 1)>; /* SD1_CLK */
+ power-source = <1800>;
+ };
+
+ sd1-cmd {
+ pinmux = <RZG3L_PORT_PINMUX(G, 1, 1)>; /* SD1_CMD */
+ input-enable;
+ power-source = <1800>;
+ };
+
+ sd1-data {
+ pinmux = <RZG3L_PORT_PINMUX(G, 2, 1)>, /* SD1_DAT0 */
+ <RZG3L_PORT_PINMUX(G, 3, 1)>, /* SD1_DAT1 */
+ <RZG3L_PORT_PINMUX(G, 4, 1)>, /* SD1_DAT2 */
+ <RZG3L_PORT_PINMUX(G, 5, 1)>; /* SD1_DAT3 */
+ input-enable;
+ power-source = <1800>;
+ };
+ };
+
ssi0_pins: ssi0 {
pinmux = <RZG3L_PORT_PINMUX(H, 0, 9)>, /* SSIF0_RXD */
<RZG3L_PORT_PINMUX(H, 1, 9)>, /* SSIF0_BCK */
@@ -230,6 +307,17 @@ &scif0 {
pinctrl-names = "default";
};
+#if RZ_BOOT_MODE3
+&sdhi1 {
+ pinctrl-0 = <&sdhi1_pins>;
+ pinctrl-1 = <&sdhi1_uhs_pins>;
+ pinctrl-names = "default", "state_uhs";
+
+ vmmc-supply = <®_3p3v>;
+ vqmmc-supply = <&vqmmc_sd1_pvdd>;
+};
+#endif
+
#if !SW_SD2_EN
&ssi0 {
clocks = <&cpg CPG_MOD R9A08G046_SSI0_PCLK2>,
--
2.43.0
^ permalink raw reply related
* [PATCH v18 0/4] Add SDHI support for RZ/G3L SoC
From: Biju @ 2026-06-22 16:48 UTC (permalink / raw)
To: Geert Uytterhoeven, Linus Walleij, Magnus Damm, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Biju Das, linux-renesas-soc, linux-gpio, devicetree, linux-kernel,
Prabhakar Mahadev Lad, Biju Das
From: Biju Das <biju.das.jz@bp.renesas.com>
This series adds pin control and device tree support for the three
SDHI (SD/MMC host interface) controllers on the Renesas RZ/G3L SoC
(r9a08g046), and enables them on the RZ/G3L SMARC EVK platform.
This patch series depend on [1]
[1] https://lore.kernel.org/all/20260622155610.184271-1-biju.das.jz@bp.renesas.com/
v18:
* Split from patch series [2]
* Moved sd_ch2 variable near to sd_ch[].
[2] https://lore.kernel.org/all/20260603065731.93243-1-biju.das.jz@bp.renesas.com/
Biju Das (4):
pinctrl: renesas: rzg2l: Add SD channel POC support for RZ/G3L
arm64: dts: renesas: r9a08g046: Add SDHI nodes for RZ/G3L SoC and
SDHI1 pincontrol on SMARC EVK
arm64: dts: renesas: rzg3l-smarc-som: Enable SD/eMMC on SDHI0
arm64: dts: renesas: rzg3l-smarc-som: Enable SDHI2
arch/arm64/boot/dts/renesas/r9a08g046.dtsi | 73 ++++++-
.../boot/dts/renesas/r9a08g046l48-smarc.dts | 89 ++++++++
.../boot/dts/renesas/rzg3l-smarc-som.dtsi | 199 ++++++++++++++++++
drivers/pinctrl/renesas/pinctrl-rzg2l.c | 74 ++++---
4 files changed, 410 insertions(+), 25 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: thermal: amlogic: Correct 'reg' in the example
From: Conor Dooley @ 2026-06-22 16:45 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Guillaume La Roque, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Ronald Claveau, linux-pm, linux-amlogic, devicetree, linux-kernel
In-Reply-To: <20260622100231.438435-4-krzysztof.kozlowski@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 53 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: thermal: amlogic: Fix missing header in the example
From: Conor Dooley @ 2026-06-22 16:45 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Guillaume La Roque, Rafael J. Wysocki, Daniel Lezcano, Zhang Rui,
Lukasz Luba, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Ronald Claveau, linux-pm, linux-amlogic, devicetree, linux-kernel
In-Reply-To: <20260622100231.438435-3-krzysztof.kozlowski@oss.qualcomm.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH RFC v5 3/6] iio: osf: add protocol decoding
From: Jonathan Cameron @ 2026-06-22 16:43 UTC (permalink / raw)
To: Jinseob Kim
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
Nuno Sá, Andy Shevchenko, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-doc, linux-kernel
In-Reply-To: <20260616072242.3942-4-kimjinseob88@gmail.com>
On Tue, 16 Jun 2026 16:22:39 +0900
Jinseob Kim <kimjinseob88@gmail.com> wrote:
> Add helpers for validating and decoding Open Sensor Fusion frames and the
> message payloads used by the initial receive path.
>
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
A few things inline.
> ---
> drivers/iio/opensensorfusion/osf_protocol.c | 249 ++++++++++++++++++++
> drivers/iio/opensensorfusion/osf_protocol.h | 97 ++++++++
> 2 files changed, 346 insertions(+)
> create mode 100644 drivers/iio/opensensorfusion/osf_protocol.c
> create mode 100644 drivers/iio/opensensorfusion/osf_protocol.h
>
> diff --git a/drivers/iio/opensensorfusion/osf_protocol.c b/drivers/iio/opensensorfusion/osf_protocol.c
> new file mode 100644
> index 000000000..5bee545f3
> --- /dev/null
> +++ b/drivers/iio/opensensorfusion/osf_protocol.c
> +int osf_protocol_decode_frame(const u8 *buf, size_t len,
> + struct osf_frame *frame, size_t *frame_len)
> +{
> + u32 expected_crc;
> + u32 actual_crc;
> + u32 payload_len;
> + size_t total_len;
> + u8 major;
> +
> + if (!buf || !frame || !frame_len)
> + return -EINVAL;
> +
> + if (len < OSF_FRAME_MIN_LEN)
> + return -EMSGSIZE;
> +
> + if (get_unaligned_le32(buf) != OSF_FRAME_MAGIC)
> + return -EPROTO;
> +
> + major = buf[4];
> + if (major != OSF_PROTOCOL_MAJOR)
> + return -EPROTO;
> +
> + if (get_unaligned_le16(buf + 6) != OSF_FRAME_HEADER_LEN)
> + return -EPROTO;
> +
> + payload_len = get_unaligned_le32(buf + 10);
> + if (payload_len > len - OSF_FRAME_MIN_LEN)
> + return -EMSGSIZE;
> +
> + if (get_unaligned_le32(buf + 34))
> + return -EPROTO;
> +
> + total_len = OSF_FRAME_HEADER_LEN + payload_len + OSF_FRAME_CRC_LEN;
> + expected_crc = osf_crc32_ieee(buf, OSF_FRAME_HEADER_LEN + payload_len);
> + actual_crc = get_unaligned_le32(buf + OSF_FRAME_HEADER_LEN + payload_len);
> +
> + if (actual_crc != expected_crc)
> + return -EBADMSG;
> +
> + frame->protocol_minor = buf[5];
> + frame->message_type = get_unaligned_le16(buf + 8);
> + frame->payload_len = payload_len;
> + frame->sequence = get_unaligned_le64(buf + 14);
> + frame->timestamp_us = get_unaligned_le64(buf + 22);
> + frame->flags = get_unaligned_le32(buf + 30);
> + frame->payload = buf + OSF_FRAME_HEADER_LEN;
> + frame->crc = actual_crc;
Same as below wrt to to using designated initializers for these
structure fills
> + *frame_len = total_len;
> +
> + return 0;
> +}
> +
> +int osf_protocol_decode_sensor_sample(const struct osf_frame *frame,
> + struct osf_sensor_sample *sample)
> +{
> + u16 channel_count;
> + u16 sample_format;
> + u16 sensor_type;
> + size_t expected_len;
> + const u8 *payload;
> +
> + if (!frame || !sample || !frame->payload)
> + return -EINVAL;
> +
> + if (frame->message_type != OSF_MSG_SENSOR_SAMPLE)
> + return -EPROTO;
> +
> + if (frame->payload_len < OSF_SENSOR_SAMPLE_BASE_LEN)
> + return -EMSGSIZE;
> +
> + payload = frame->payload;
> + sensor_type = get_unaligned_le16(payload);
> + channel_count = get_unaligned_le16(payload + 4);
> + sample_format = get_unaligned_le16(payload + 6);
> +
> + if (!osf_sensor_type_valid(sensor_type))
> + return -EPROTO;
> +
> + if (!channel_count)
> + return -EPROTO;
> +
> + if (sample_format != OSF_SAMPLE_FORMAT_S32)
> + return -EPROTO;
> +
> + if (get_unaligned_le32(payload + 12))
> + return -EPROTO;
> +
> + if (channel_count > (SIZE_MAX - OSF_SENSOR_SAMPLE_BASE_LEN) / sizeof(s32))
> + return -EOVERFLOW;
> +
> + expected_len = OSF_SENSOR_SAMPLE_BASE_LEN + channel_count * sizeof(s32);
> + if (frame->payload_len != expected_len)
> + return -EMSGSIZE;
> +
> + sample->sensor_type = sensor_type;
> + sample->sensor_index = get_unaligned_le16(payload + 2);
> + sample->channel_count = channel_count;
> + sample->sample_format = sample_format;
> + sample->scale_nano = get_unaligned_le32(payload + 8);
> + sample->samples = payload + OSF_SENSOR_SAMPLE_BASE_LEN;
See below. Designated initializer would help readability a little here.
> +
> + return 0;
> +}
> +
> +int osf_protocol_sensor_sample_value(const struct osf_sensor_sample *sample,
> + unsigned int index, s32 *value)
Given channel count is a u16 and we can't be equal or bigger than it, perhaps
use a u16 for index as well.
> +{
> + if (!sample || !sample->samples || !value)
> + return -EINVAL;
> +
> + if (index >= sample->channel_count)
> + return -ERANGE;
> +
> + /* Samples are little-endian two's-complement signed values. */
> + *value = (s32)get_unaligned_le32(sample->samples + index * sizeof(s32));
sizeof(__le32) slightly more appropriate given that is what you are treating it
as.
> +
> + return 0;
> +}
> +
> +int osf_protocol_decode_device_status(const struct osf_frame *frame,
> + struct osf_device_status *status)
> +{
> + const u8 *payload;
> +
> + if (!frame || !status || !frame->payload)
> + return -EINVAL;
> +
> + if (frame->message_type != OSF_MSG_DEVICE_STATUS)
> + return -EPROTO;
> +
> + if (frame->payload_len != OSF_DEVICE_STATUS_LEN)
> + return -EMSGSIZE;
> +
> + payload = frame->payload;
> + if (get_unaligned_le32(payload + 16))
> + return -EPROTO;
> +
> + status->uptime_s = get_unaligned_le32(payload);
> + status->status_flags = get_unaligned_le32(payload + 4);
> + status->error_flags = get_unaligned_le32(payload + 8);
> + status->dropped_frames = get_unaligned_le32(payload + 12);
Similar to below. I'd use a designated initializer for status as it
is all written in one place.
> +
> + return 0;
> +}
> +
> +int osf_protocol_decode_capability_entry(const struct osf_capability_report *report,
> + unsigned int index,
> + struct osf_capability_entry *entry)
> +{
> + u16 sample_format;
> + u16 sensor_type;
> + u32 flags;
> + const u8 *payload;
> +
> + if (!report || !report->entries || !entry)
> + return -EINVAL;
> +
> + if (index >= report->capability_count)
Neater to size index to match capability_count. Not that
important though.
> + return -ERANGE;
> +
> + payload = report->entries + index * OSF_CAP_SENSOR_ENTRY_LEN;
> + sensor_type = get_unaligned_le16(payload);
> + sample_format = get_unaligned_le16(payload + 6);
> + flags = get_unaligned_le32(payload + 12);
> +
> + if (!osf_sensor_type_valid(sensor_type))
> + return -EPROTO;
> +
> + if (sample_format != OSF_SAMPLE_FORMAT_S32)
> + return -EPROTO;
> +
> + if (flags & ~OSF_CAPABILITY_FLAGS_MASK)
> + return -EPROTO;
> +
> + if (get_unaligned_le32(payload + 16))
> + return -EPROTO;
> +
> + entry->sensor_type = sensor_type;
> + entry->sensor_index = get_unaligned_le16(payload + 2);
> + entry->channel_count = get_unaligned_le16(payload + 4);
> + entry->sample_format = sample_format;
> + entry->scale_nano = get_unaligned_le32(payload + 8);
> + entry->flags = flags;
neater as designated initializer I think.
*entry = (struct osf_capability_entry) {
.sensor_type = sensor_type,
.sensor_index = get_unaligned_le16(payload + 2),
etc
};
> +
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH v2 4/5] dt-bindings: dma: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
From: Conor Dooley @ 2026-06-22 16:42 UTC (permalink / raw)
To: Frank Li
Cc: Yuanshen Cao, Vinod Koul, Frank Li, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Maxime Ripard, dmaengine, linux-arm-kernel, linux-sunxi,
linux-kernel, devicetree
In-Reply-To: <ajhjj7FLn136qMmt@SMW015318>
[-- Attachment #1: Type: text/plain, Size: 2123 bytes --]
On Sun, Jun 21, 2026 at 05:19:59PM -0500, Frank Li wrote:
> On Sun, Jun 21, 2026 at 09:40:57PM +0000, Yuanshen Cao wrote:
>
> subject dt-bindings: dmaengine: ....
>
> > 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>
> > ---
>
> After fix subject tags,
Do not change this unless you're respinning for another reason. dma v
dmaengine is not worth resubmission, especially since dma is far more
commonly used and is the directory name.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
>
> Reviewed-by: Frank Li <Frank.Li@nxp.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
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 2/5] iio: adc: Add ti-ads1262 driver
From: David Lechner @ 2026-06-22 16:42 UTC (permalink / raw)
To: Jonathan Cameron, Kurt Borja
Cc: Krzysztof Kozlowski, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Linus Walleij, Bartosz Golaszewski, Nuno Sá,
Andy Shevchenko, linux-iio, devicetree, linux-kernel, linux-gpio
In-Reply-To: <20260622104728.039a5ea2@jic23-huawei>
On 6/22/26 4:47 AM, Jonathan Cameron wrote:
> On Sun, 21 Jun 2026 19:18:33 -0500
> "Kurt Borja" <kuurtb@gmail.com> wrote:
>
>> On Sun Jun 21, 2026 at 9:33 AM -05, Jonathan Cameron wrote:
>>> On Mon, 15 Jun 2026 06:30:28 +0200
>>> Krzysztof Kozlowski <krzk@kernel.org> wrote:
>>>
>>>> On 14/06/2026 22:56, Kurt Borja wrote:
>>>>> On Sat Jun 13, 2026 at 1:59 PM -05, Krzysztof Kozlowski wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>> Functions used by probe() should be before probe(), not somewhere in the
>>>>>> middle of the code. IOW, entire probe is together.
>>>>>
>>>>> I they all are, it's just that regmap stuff takes a huge chunk. I'll
>>>>> check how to reorganize.
>>>>>
>>>>> [...]
>>>>>
>>>>>>> +static const struct of_device_id ads1262_of_match[] = {
>>>>>>> + { .compatible = "ti,ads1262" },
>>>>>>> + { .compatible = "ti,ads1263" },
>>>>>>
>>>>>> So devices are fully compatible? Then it should be expressed in the
>>>>>> binding and drop one entry here.
>>>>>
>>>>> Not fully compatible as Jonathan said. One is a subset of the other.
>>>>
>>>> This is THE meaning of compatible!
>>>
>>> This one I'm in agreement with. It is a strict subset, so should be
>>> using a fallback. If the fallback is used, you just get support of the
>>> stuff in the simpler chip (or if you can override it with a chip ID
>>> you might still 'upgrade' to the more complex driver support).
>>> If you do end up with properties that only apply to 'new' parts of
>>> the more complex chip then they should be verified as part of the
>>> binding (assuming you can do that without the verifier complaining
>>> - I haven't checked!)
>>
>> In v1 I had the "adc" subnode which was specific to ADS1263. Then I
>> agreed to drop the subnode but I'm having second thoughts...
>>
>> If we dropped it, then we would still have some specific stuff.
>> #io-channel-cells would be "const: 2" in ADS1263 chips. Also ADS1263's
>> channels would have an extra ti,vref-adc2 prop, for ADC2 voltage
>> reference selection. I should maybe also add a vref-adc2-supply.
>>
>> Maybe it's better to keep the subnode or, again, go for something like:
>>
>> spi {
>> multi-adc@0 {
>> adc@0 {
>> ...
>> vref-suppy = <&adc1-vref>;
>>
>> channel@0 {
>> ...
>> reference-source = <ADS1262_VREF_AIN0_AIN1>;
>> };
>> };
>> adc@1 {
>> ...
>> vref-suppy = <&adc2-vref>;
>>
>> channel@0 {
>> ...
>> reference-source = <ADS1262_VREF_AIN2_AIN3>;
>> };
>> };
>> };
>> };
>>
>> In this case we would have to kinda duplicate channel description, but I
>> don't think it's that bad.
>>
>> Jonathan, Krzysztof, David, thoughts?
>>
>> IMO the ADC2 specific voltage reference stuff is a strong argument for a
>> subnode or the above solution.
>
> Given you end up with channel specific stuff that differs I think it probably
> makes sense - though I do wonder a bit if that is real. What's the use case
> for using a different reference for the monitoring / debug than the main one?
> I could imagine some dynamic use where you want to sanity check against
> a wider reference range, but maybe that needs userspace control rather than
> in here?
I think is is going to mostly be the same, so could be simpler to just
add extra channel properties on an as-needed basis if things do actually
differ between ADC1 and ADC2 rather than having to define all channels
twice.
This seems pretty similar to the discussion of how to handle e.g. measuring
the same inputs with and without the burn-out current enabled in the
ti,ads112c14 series and I think you have convinced me that we should not
be having a separate channel in the devicetree for that either.
>
> Jonathan
>
>
>>
>>>
>>> The SLF3F discussion is about (to me) less obvious case of not a strict
>>> subset, but rather being detectable parts with different channel related
>>> properties. In that case the ID match is necessary for anything to work.
>>> Anyhow, that discussion is in a different thread and not really relevant
>>> here.
>>>
>>> Jonathan
>>>
>>>>
>>>>
>>>> Best regards,
>>>> Krzysztof
>>
>
^ permalink raw reply
* Re: [PATCH 2/2] arm64: dts: qcom: shikra: Add MDSP carveout memory and update APM DAIs memory regions
From: Mark Brown @ 2026-06-22 16:42 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Ajay Kumar Nandam, Srinivas Kandagatla, Liam Girdwood,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Bjorn Andersson,
Konrad Dybcio, linux-arm-msm, linux-sound, devicetree,
linux-kernel
In-Reply-To: <20260622-defiant-warthog-of-authority-d9d1ba@quoll>
[-- Attachment #1: Type: text/plain, Size: 447 bytes --]
On Mon, Jun 22, 2026 at 03:42:15PM +0200, Krzysztof Kozlowski wrote:
> On Thu, Jun 18, 2026 at 05:05:09PM +0530, Ajay Kumar Nandam wrote:
> > q6apmdai: dais {
> > compatible = "qcom,q6apm-dais";
> > - qcom,vmid = <QCOM_SCM_VMID_MSS_MSA>;
> There is no such line in next-20260619, which means this is some wrong
> base.
There's a pile of in flight stuff listed in the cover letter as
dependencies, likely it's one of those.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v2 4/5] dt-bindings: dma: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
From: Conor Dooley @ 2026-06-22 16:40 UTC (permalink / raw)
To: sashiko-reviews
Cc: Yuanshen Cao, linux-sunxi, devicetree, Frank.Li, vkoul, robh,
dmaengine, conor+dt
In-Reply-To: <20260621215235.13FF01F000E9@smtp.kernel.org>
[-- Attachment #1: Type: text/plain, Size: 2287 bytes --]
On Sun, Jun 21, 2026 at 09:52:34PM +0000, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The IRQ handler loop skips trailing DMA channels if the total number of physical channels is not an exact multiple of the channels per register.
> --
>
> commit 1a8550cc9da2ef939f555f682f0d5ea9606e1907
> Author: Yuanshen Cao <alex.caoys@gmail.com>
>
> dt-bindings: dma: sun50i-a64-dma: Add allwinner,sun60i-a733-dma compatible string
>
> 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.
>
> [Severity: High]
> This isn't a bug introduced by this patch, but could this truncating
> integer division skip trailing DMA channels if the total number of physical
> channels is not an exact multiple of the channels per register?
What has this got to do with the binding?
>
> In drivers/dma/sun6i-dma.c:sun6i_dma_interrupt(), the loop condition
> determines which interrupt registers to poll:
>
> for (i = 0; i < sdev->num_pchans / sdev->cfg->num_channels_per_reg; i++) {
>
> For a controller with 12 physical channels (e.g., sun8i_h3_dma_cfg sets
> nr_max_channels = 12) and 8 channels per register, 12 / 8 evaluates to 1.
>
> Consequently, the loop only iterates once (for i=0), checking interrupts
> for the first 8 channels and completely ignoring the remaining 4 channels.
> Any DMA transfers on channels 8-11 will never have their completion
> interrupts handled, leading to system hangs or driver timeouts.
>
> Could this be updated to use DIV_ROUND_UP() to ensure all applicable
> interrupt registers are processed?
>
> > - Support for higher (32G) address widths in LLI parameters.
> >
> > Signed-off-by: Yuanshen Cao <alex.caoys@gmail.com>
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260621-sun60i-a733-dma-v2-0-340f205891cc@gmail.com?part=4
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: iommu: Fix interrupt type in example
From: Conor Dooley @ 2026-06-22 16:38 UTC (permalink / raw)
To: Ashish Mhetre
Cc: joro, will, robin.murphy, robh, krzk+dt, conor+dt, jonathanh,
thierry.reding, nicolinc, iommu, devicetree, linux-tegra,
linux-kernel
In-Reply-To: <20260622065410.2780215-1-amhetre@nvidia.com>
[-- Attachment #1: Type: text/plain, Size: 75 bytes --]
Acked-by: Conor Dooley <conor.dooley@microchip.com>
pw-bot: not-applicable
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: hwmon: chipcap2: Add label property
From: Javier Carrasco @ 2026-06-22 16:29 UTC (permalink / raw)
To: Flaviu Nistor, Guenter Roeck, Javier Carrasco, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Jonathan Corbet, Shuah Khan
Cc: linux-hwmon, linux-kernel, devicetree, linux-doc
In-Reply-To: <20260622122200.14245-1-flaviu.nistor@gmail.com>
On Mon Jun 22, 2026 at 2:21 PM CEST, Flaviu Nistor wrote:
> Add support for an optional label property similar to other hwmon devices.
> This allows, in case of boards with multiple CHIPCAP2 sensors, to assign
> distinct names to each instance.
>
> Signed-off-by: Flaviu Nistor <flaviu.nistor@gmail.com>
> ---
> .../devicetree/bindings/hwmon/amphenol,chipcap2.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> index 17351fdbefce..f00b5a4b14dd 100644
> --- a/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> +++ b/Documentation/devicetree/bindings/hwmon/amphenol,chipcap2.yaml
> @@ -33,6 +33,10 @@ properties:
> reg:
> maxItems: 1
>
> + label:
> + description:
> + A descriptive name for this channel, like "ambient" or "psu".
> +
> interrupts:
> items:
> - description: measurement ready indicator
> @@ -72,6 +76,7 @@ examples:
> <5 IRQ_TYPE_EDGE_RISING>,
> <6 IRQ_TYPE_EDGE_RISING>;
> interrupt-names = "ready", "low", "high";
> + label = "somelabel";
> vdd-supply = <®_vdd>;
> };
> };
Hello Falviu, thank you for your patch.
Should we not add a reference to hwmon-common.yaml (with
unevelautedProperties instead of additionalProperties), as label is
defined there? I believe that Krzysztof Kozlowski did something similar
for the shunt-resistor-micro-ohms property. Could we follow suit here?
I am also not a big fan of a name like "somelabel", and a more
meaningful name from a "real" example would look better. I know that
some examples have already used "somelabel" as an example, but others
have used more meaningful names too.
Best regards,
Javier Carrasco
^ permalink raw reply
* Re: [PATCH 2/2] pwm: add Axiado AX3000 PWM driver
From: Uwe Kleine-König @ 2026-06-22 16:29 UTC (permalink / raw)
To: Petar Stepanovic
Cc: Akhila Kavi, Prasad Bolisetty, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Harshit Shah, linux-pwm, devicetree,
linux-arm-kernel, linux-kernel
In-Reply-To: <20260618-axiado-ax3000-pwm-v1-2-c9797a909414@axiado.com>
[-- Attachment #1: Type: text/plain, Size: 9646 bytes --]
Hello Petar,
Just a very high-level review:
On Thu, Jun 18, 2026 at 05:26:57AM -0700, Petar Stepanovic wrote:
> The Axiado AX3000 and AX3005 SoCs include PWM controllers that can be
> used to generate configurable PWM output signals.
>
> Add a PWM driver with support for configuring period, duty cycle, and
> enable state through the Linux PWM framework.
>
> Signed-off-by: Petar Stepanovic <pstepanovic@axiado.com>
> ---
> MAINTAINERS | 1 +
> drivers/pwm/Kconfig | 11 +++
> drivers/pwm/Makefile | 1 +
> drivers/pwm/pwm-axiado.c | 193 +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 206 insertions(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 394c4a3527e8..db93fc235c32 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4319,6 +4319,7 @@ M: Prasad Bolisetty <pbolisetty@axiado.com>
> L: linux-pwm@vger.kernel.org
> S: Maintained
> F: Documentation/devicetree/bindings/pwm/axiado,ax3000-pwm.yaml
> +F: drivers/pwm/pwm-axiado.c
>
> AXIS ARTPEC ARM64 SoC SUPPORT
> M: Jesper Nilsson <jesper.nilsson@axis.com>
> diff --git a/drivers/pwm/Kconfig b/drivers/pwm/Kconfig
> index 6f3147518376..76f6c04b0e23 100644
> --- a/drivers/pwm/Kconfig
> +++ b/drivers/pwm/Kconfig
> @@ -129,6 +129,17 @@ config PWM_ATMEL_TCB
> To compile this driver as a module, choose M here: the module
> will be called pwm-atmel-tcb.
>
> +config PWM_AXIADO
> + tristate "Axiado PWM support"
> + depends on ARCH_AXIADO || COMPILE_TEST
> + depends on HAS_IOMEM
> + help
> + PWM framework driver for the PWM controller found on Axiado
> + AX3000 and AX3005 SoCs.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called pwm-axiado.
> +
> config PWM_AXI_PWMGEN
> tristate "Analog Devices AXI PWM generator"
> depends on MICROBLAZE || NIOS2 || ARCH_ZYNQ || ARCH_ZYNQMP || ARCH_INTEL_SOCFPGA || COMPILE_TEST
> diff --git a/drivers/pwm/Makefile b/drivers/pwm/Makefile
> index 0dc0d2b69025..4466a29e780a 100644
> --- a/drivers/pwm/Makefile
> +++ b/drivers/pwm/Makefile
> @@ -8,6 +8,7 @@ obj-$(CONFIG_PWM_ARGON_FAN_HAT) += pwm-argon-fan-hat.o
> obj-$(CONFIG_PWM_ATMEL) += pwm-atmel.o
> obj-$(CONFIG_PWM_ATMEL_HLCDC_PWM) += pwm-atmel-hlcdc.o
> obj-$(CONFIG_PWM_ATMEL_TCB) += pwm-atmel-tcb.o
> +obj-$(CONFIG_PWM_AXIADO) += pwm-axiado.o
> obj-$(CONFIG_PWM_AXI_PWMGEN) += pwm-axi-pwmgen.o
> obj-$(CONFIG_PWM_BCM2835) += pwm-bcm2835.o
> obj-$(CONFIG_PWM_BCM_IPROC) += pwm-bcm-iproc.o
> diff --git a/drivers/pwm/pwm-axiado.c b/drivers/pwm/pwm-axiado.c
> new file mode 100644
> index 000000000000..db197886c5c4
> --- /dev/null
> +++ b/drivers/pwm/pwm-axiado.c
> @@ -0,0 +1,193 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (c) 2021-2026 Axiado Corporation.
Please add a Limitations paragraph here like the ones found in the newer
driver files. It should answer:
- Is a period completed on configuration change?
- Is a period completed on disable?
- How does the output behave when disabled? (Low? Inactive? Freeze? High-Z?)
Also mention special properties, like being unable to set 0% or 100%
relative duty.
> + */
> +
> +#include <linux/bits.h>
> +#include <linux/clk.h>
> +#include <linux/io.h>
> +#include <linux/math64.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/pwm.h>
> +
> +/* Register offsets */
> +#define AX_PWM_CNTRL_REG 0x0000
> +#define AX_PWM_PERIOD_REG 0x0004
> +#define AX_PWM_HIGH_REG 0x0008
> +
> +/* PWM channels */
> +#define AX_PWM_NUM 1
This is only used once, and having
chip = devm_pwmchip_alloc(dev, 1, sizeof(*axpwm));
below simplifies grepping for channel numbers.
> +
> +/* Period and duty cycle limits */
> +#define AX_PWM_PERIOD_MIN 2
> +#define AX_PWM_PERIOD_MAX 0xfffffffeU
> +#define AX_PWM_DUTY_MIN 1
> +#define AX_PWM_DUTY_MAX 0xfffffffdU
The U suffix is not needed for hex constants (AFAIK).
> +
> +/* Control register bits */
> +#define AX_PWM_CTRL_ENABLE BIT(0)
> +#define AX_PWM_CTRL_DISABLE 0x0
> +
> +struct axiado_pwm_chip {
> + struct clk *clk;
> + void __iomem *base;
> +};
If you use axiado_pwm_ as prefix for structs and functions, please use
AXIADO_PWM_ as prefix for #defines.
> +
> +static int axiado_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,
> + u64 duty_ns, u64 period_ns)
> +{
> + struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
> + unsigned long rate;
> + u64 period_cycles, duty_cycles;
> +
> + /*
> + * The hardware does not support a zero period, 0% duty cycle, or
> + * 100% duty cycle. The caller should handle 0% duty cycle by
> + * disabling the PWM.
> + */
> + if (!period_ns || !duty_ns || duty_ns >= period_ns)
> + return -EINVAL;
> +
> + rate = clk_get_rate(axpwm->clk);
> + if (!rate)
> + return -EINVAL;
> +
> + period_cycles = mul_u64_u64_div_u64(period_ns, rate, NSEC_PER_SEC);
> + if (period_cycles < AX_PWM_PERIOD_MIN ||
> + period_cycles > AX_PWM_PERIOD_MAX)
> + return -EINVAL;
> +
> + duty_cycles = mul_u64_u64_div_u64(duty_ns, rate, NSEC_PER_SEC);
> + if (duty_cycles < AX_PWM_DUTY_MIN ||
> + duty_cycles > AX_PWM_DUTY_MAX)
> + return -EINVAL;
> +
> + if (duty_cycles >= period_cycles)
> + return -EINVAL;
> +
> + writel((u32)period_cycles, axpwm->base + AX_PWM_PERIOD_REG);
> + writel((u32)duty_cycles, axpwm->base + AX_PWM_HIGH_REG);
> +
> + return 0;
> +}
> +
> +static int axiado_pwm_apply(struct pwm_chip *chip, struct pwm_device *pwm,
> + const struct pwm_state *state)
> +{
> + struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
> + int ret;
> +
> + if (state->polarity != PWM_POLARITY_NORMAL)
> + return -EINVAL;
> +
> + if (!state->enabled || !state->duty_cycle) {
> + if (pwm->state.enabled)
> + writel(AX_PWM_CTRL_DISABLE, axpwm->base + AX_PWM_CNTRL_REG);
> +
> + return 0;
> + }
> +
> + ret = axiado_pwm_config(chip, pwm, state->duty_cycle, state->period);
> + if (ret)
> + return ret;
> +
> + if (!pwm->state.enabled)
Ideally check hardware state and not PWM internal variables.
> + writel(AX_PWM_CTRL_ENABLE, axpwm->base + AX_PWM_CNTRL_REG);
> +
> + return 0;
> +}
> +
> +static int axiado_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> + struct pwm_state *state)
> +{
> + struct axiado_pwm_chip *axpwm = pwmchip_get_drvdata(chip);
> + unsigned long rate;
> + u32 period_cycles;
> + u32 duty_cycles;
> + u32 ctrl;
> +
> + rate = clk_get_rate(axpwm->clk);
> + if (!rate)
> + return -EINVAL;
> +
> + ctrl = readl(axpwm->base + AX_PWM_CNTRL_REG);
> + period_cycles = readl(axpwm->base + AX_PWM_PERIOD_REG);
> + duty_cycles = readl(axpwm->base + AX_PWM_HIGH_REG);
> +
> + state->enabled = !!(ctrl & AX_PWM_CTRL_ENABLE);
> + state->period = mul_u64_u64_div_u64(period_cycles, NSEC_PER_SEC, rate);
> + state->duty_cycle = mul_u64_u64_div_u64(duty_cycles, NSEC_PER_SEC, rate);
> + state->polarity = PWM_POLARITY_NORMAL;
Please test your driver with PWM_DEBUG enabled, the rounding is wrong
here.
> +
> + return 0;
> +}
> +
> +static const struct pwm_ops axiado_pwm_ops = {
> + .get_state = axiado_pwm_get_state,
> + .apply = axiado_pwm_apply,
Please implement the waveform callbacke instead of .get_state() and
.apply()
> +};
> +
> +static void axiado_pwm_disable(void *data)
> +{
> + struct axiado_pwm_chip *axpwm = data;
> +
> + writel(AX_PWM_CTRL_DISABLE, axpwm->base + AX_PWM_CNTRL_REG);
> +}
> +
> +static int axiado_pwm_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct axiado_pwm_chip *axpwm;
> + struct pwm_chip *chip;
> + int ret;
> +
> + chip = devm_pwmchip_alloc(dev, AX_PWM_NUM, sizeof(*axpwm));
> + if (IS_ERR(chip))
> + return PTR_ERR(chip);
> +
> + axpwm = pwmchip_get_drvdata(chip);
> +
> + axpwm->base = devm_platform_ioremap_resource(pdev, 0);
> + if (IS_ERR(axpwm->base))
> + return dev_err_probe(dev, PTR_ERR(axpwm->base),
> + "failed to map registers\n");
Start error messages with a capital letter please.
> +
> + ret = devm_add_action_or_reset(dev, axiado_pwm_disable, axpwm);
> + if (ret)
> + return ret;
This isn't supposed to happen. It's the responsibility of the consumer
to disable the PWM before it's freed.
> +
> +
Single empty line only.
> + axpwm->clk = devm_clk_get_enabled(dev, "pwm");
> + if (IS_ERR(axpwm->clk))
> + return dev_err_probe(dev, PTR_ERR(axpwm->clk),
> + "failed to get/enable clock\n");
Please ensure that the clk rate doesn't change while the PWM is enabled.
Then you can cache the clk rate and set chip->atomic.
> +
> + chip->ops = &axiado_pwm_ops;
> +
> + ret = devm_pwmchip_add(dev, chip);
> + if (ret)
> + return dev_err_probe(dev, ret, "failed to add PWM chip\n");
> +
> + return 0;
> +}
> +
> +static const struct of_device_id axiado_pwm_match[] = {
> + { .compatible = "axiado,ax3000-pwm" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, axiado_pwm_match);
> +
> +static struct platform_driver axiado_pwm_driver = {
> + .driver = {
> + .name = "axiado-pwm",
> + .of_match_table = axiado_pwm_match,
> + },
> + .probe = axiado_pwm_probe,
> +};
> +
> +module_platform_driver(axiado_pwm_driver);
No empty line between the driver struct and the module_platform helper
please.
> +
> +MODULE_AUTHOR("Axiado Corporation");
> +MODULE_DESCRIPTION("Axiado PWM driver");
> +MODULE_LICENSE("GPL");
Best regards
Uwe
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Jonathan Cameron @ 2026-06-22 16:29 UTC (permalink / raw)
To: Nuno Sá
Cc: Rodrigo Alencar, Conor Dooley, Janani Sunil, 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: <ajkMBh-R_7pYaoAn@nsa>
On Mon, 22 Jun 2026 11:29:38 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Mon, Jun 22, 2026 at 10:24:05AM +0100, Rodrigo Alencar wrote:
> > On 21/06/26 15:33, 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.
> > >
> >
> > If the device-addressing on the same chip-select is to be handled
> > by the spi framework, wouldn't we lose device-specific features?
> >
> > I understand that this multi-device feature is there mostly to extend the
> > channel count from 16 to 32, 48 or 64. I suppose the command:
> >
> > "MULTI DEVICE SW LDAC MODE"
> >
> > exists so that software can update channel values accross multiple devices.
>
> Right! You do have a point! I agree the main driver for a feature like
> this is likely to extend the channel count and effectively "aggregate"
> devices.
>
> But I would say that even with the spi solution the MULTI DEVICE stuff
> should be doable (as we still need a sort of adi,pin-id property).
>
> But yes, I do feel that the whole feature is for aggregation so seeing
> one device with 32 channels is the expectation here? Rather than seeing
> two devices with 16 channels.
Agreed - if we have messages that address both devices at once that needs
to be a unified driver and given they are about triggering simultaneous
update of all channels it needs to look like one big device.
This ends up similar to how we handle daisy chain devices.
The question of what to do on devices that don't have this feature
is rather different. Good thing you read the datasheet :)
Jonathan
>
> - Nuno Sá
>
> >
> > --
> > Kind regards,
> >
> > Rodrigo Alencar
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: Jonathan Cameron @ 2026-06-22 16:24 UTC (permalink / raw)
To: Nuno Sá
Cc: Conor Dooley, 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: <ajkILRPq_g24g4dH@nsa>
On Mon, 22 Jun 2026 11:17:56 +0100
Nuno Sá <noname.nuno@gmail.com> wrote:
> On Mon, Jun 22, 2026 at 10:27:22AM +0100, Jonathan Cameron wrote:
> > On Mon, 22 Jun 2026 10:07:01 +0100
> > Nuno Sá <noname.nuno@gmail.com> wrote:
> >
> > > On Sun, Jun 21, 2026 at 07:35:42PM +0100, Conor Dooley wrote:
> > > > 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?
> > >
> > > Not sure how things work atm. But I'm fairly sure it used to be like
> > > that. SPI would reject devices on the same controller and CS. Now that
> > > we support more than one CS per controller, not sure how things work.
> > We always supported more than one per CS per controller. I guess you mean
> > per device.
>
> Obviously :)
> >
> > >
> > > Janani, maybe you can give it a try?
> >
> > I think we'd need to get it to work with shared gpio proxy which maybe
> > will just get set up under the hood. This used to be opt in, but seems
> > that changed fairly recently so maybe some of us are working with out
> > of date knowledge! I haven't played with it yet, so might not be
> > that simple.
> >
>
> What I meant for Janani was basically testing two devices on the same CS
> as in my pseudo DT. For the GPIO, you mean having a way to select
> between devices on the same CS?
Nope. It is what you suggest - the implementation in the gpio layer
is to detect the reuse of the same GPIO and insert a proxy layer that
allows multiple consumers. I think that will provide different gpio
numbers (well descs really) to each of them but I haven't checked the details
that closely.
>
> For these devices the pin id numbers get's setted up as part of the spi message
> so my assumption is that all of them will receive the message but only one acks it.
Yup. As much as we have an ack on SPI. So with a write only message you'd never
know if anyone got it.
Jonathan
>
> - Nuno Sá
>
> > Jonathan
> >
> > >
> > > - Nuno Sá
> > >
> >
>
^ permalink raw reply
* Re: [PATCH RFC v5 2/6] Documentation: iio: add Open Sensor Fusion driver overview
From: Jonathan Cameron @ 2026-06-22 16:21 UTC (permalink / raw)
To: Jinseob Kim
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
Nuno Sá, Andy Shevchenko, Jonathan Corbet, Shuah Khan,
linux-iio, devicetree, linux-doc, linux-kernel
In-Reply-To: <20260616072242.3942-3-kimjinseob88@gmail.com>
On Tue, 16 Jun 2026 16:22:38 +0900
Jinseob Kim <kimjinseob88@gmail.com> wrote:
> Document the Linux IIO mapping for Open Sensor Fusion devices, including
> capability-driven IIO device registration and the initially supported
> receive path.
>
> Call out that OSF0 is a wire magic value, while protocol_major and
> protocol_minor carry protocol compatibility inside frames. The Linux
> compatible remains the generic Open Sensor Fusion host interface.
>
> Signed-off-by: Jinseob Kim <kimjinseob88@gmail.com>
Just one small thing inline.
Otherwise this look good to me.
Thanks,
Jonathan
> ---
> Documentation/iio/index.rst | 1 +
> Documentation/iio/open-sensor-fusion.rst | 71 ++++++++++++++++++++++++
> 2 files changed, 72 insertions(+)
> create mode 100644 Documentation/iio/open-sensor-fusion.rst
>
> diff --git a/Documentation/iio/index.rst b/Documentation/iio/index.rst
> index ba3e609c6..2713ec5e0 100644
> --- a/Documentation/iio/index.rst
> +++ b/Documentation/iio/index.rst
> @@ -38,4 +38,5 @@ Industrial I/O Kernel Drivers
> adxl345
> bno055
> ep93xx_adc
> + open-sensor-fusion
> opt4060
> diff --git a/Documentation/iio/open-sensor-fusion.rst b/Documentation/iio/open-sensor-fusion.rst
> new file mode 100644
> index 000000000..cf3bbd761
> --- /dev/null
> +++ b/Documentation/iio/open-sensor-fusion.rst
> @@ -0,0 +1,71 @@
> +.. SPDX-License-Identifier: GPL-2.0-only
> +
> +Open Sensor Fusion
> +==================
> +
> +Open Sensor Fusion is a sensor aggregation hub interface. The Linux IIO driver
> +receives OSF protocol frames from an attached device, discovers supported sensor
> +streams through capability reports, and registers matching IIO devices for the
> +sensor classes supported by the driver.
> +
> +This document is a driver-facing overview for the Linux IIO mapping. The full
> +wire protocol, firmware behavior, and hardware model details belong in the Open
> +Sensor Fusion project documentation.
> +
> +Device Model
> +------------
> +
> +An OSF device sends binary frames from the device to the host. The host driver
> +uses ``CAPABILITY_REPORT`` messages to discover which sensor streams are
> +available. Device Tree describes the attached OSF sensor aggregation hub; it does
> +not enumerate the individual sensors discovered at runtime.
> +
> +The currently supported Linux subset exposes:
> +
> +* accelerometer samples as ``IIO_ACCEL`` X/Y/Z channels,
> +* gyroscope samples as ``IIO_ANGL_VEL`` X/Y/Z channels,
> +* magnetometer samples as ``IIO_MAGN`` X/Y/Z channels, and
> +* temperature samples as ``IIO_TEMP``.
> +
> +Protocol Scope
> +---------------
> +
> +The driver supports OSF protocol major version 0 for the initial IIO receive
> +path. The current wire magic is ``OSF0``; that string is a wire-format detail and
> +is not the Linux driver identity. Device Tree keeps the generic
> +``opensensorfusion,osf`` compatible rather than naming a product such as OSF
> +GREEN or a wire magic value.
I'd remove anything that is documented elsewhere (i.e. the dt-binding)
and instead use a cross reference to it. That will reduce the chance of this getting
out of sync.
> +
> +Protocol versioning is carried by the ``protocol_major`` and ``protocol_minor``
> +fields at fixed offsets in the OSF frame header. The driver currently
> +supports ``protocol_major`` 0. ``protocol_minor`` changes within major version
> +0 are intended to remain backward-compatible within the fixed header layout.
> +Incompatible wire-format changes require a new ``protocol_major``. A future
> +device that cannot expose compatible version discovery through that fixed
> +header layout would need a different Device Tree compatible.
> +
> +The initial Linux driver handles device-to-host frames for:
> +
> +* ``SENSOR_SAMPLE`` buffered and direct-mode sample data,
> +* ``CAPABILITY_REPORT`` based IIO device registration, and
> +* ``DEVICE_STATUS`` cache updates.
> +
> +Vendor-private message types are ignored. Command transport, calibration
> +control ABI, fusion output ABI, and runtime capability removal are outside the
> +initial Linux IIO receive path.
> +
> +Timestamps
> +----------
> +
> +OSF frames include a device-side ``timestamp_us`` field. Buffered IIO samples use
> +an IIO timestamp captured on the host when samples are pushed to IIO buffers.
> +The initial driver does not correlate the device timestamp with the host IIO
> +clock.
> +
> +Compatibility Notes
> +-------------------
> +
> +The project protocol documentation should define the compatibility rules for
> +reserved fields, optional flags, and trailing extension data. Until those rules
> +are finalized, the Linux decoder keeps conservative bounds checks around the
> +currently supported message layouts.
^ permalink raw reply
* Re: [PATCH v18 01/12] dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC
From: sashiko-bot @ 2026-06-22 16:05 UTC (permalink / raw)
To: Biju; +Cc: robh, conor+dt, wsa+renesas, devicetree
In-Reply-To: <20260622155610.184271-2-biju.das.jz@bp.renesas.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Relaxing the global `maxItems` for `resets` and `reset-names` from 1 to 3 removes the strict constraint for legacy controllers, allowing them to incorrectly define up to 3 resets.
--
commit 5bd955e3c9833c72a7847cd9926c293d95e5c927
Author: Biju Das <biju.das.jz@bp.renesas.com>
dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC
Document the RZ/G3L (r9a08g046) SDHI controller. The RZ/G3L SDHI
controller is similar to RZ/G2L but has five clocks (core, clkh,
cd, aclk, aclkm) and three resets (rst, axim, axis), so update the
clocks/clock-names maximum to 5 and resets/reset-names maximum to 3.
It has an internal divider for all modes except HS400, and a 2048-bit
divider compared to 512 on others.
> diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -116,7 +117,12 @@ properties:
> maxItems: 1
>
> resets:
> - maxItems: 1
> + minItems: 1
> + maxItems: 3
> +
> + reset-names:
> + minItems: 1
> + maxItems: 3
[ ... ]
> @@ -155,60 +161,101 @@ allOf:
[ ... ]
> + else:
> + if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - renesas,rcar-gen2-sdhi
> + - renesas,rcar-gen3-sdhi
> + - renesas,rcar-gen4-sdhi
> + then:
> + properties:
> + clocks:
> + minItems: 1
> + maxItems: 3
> + clock-names:
> + minItems: 1
> + uniqueItems: true
> + items:
> + - const: core
> + - enum: [ clkh, cd ]
> + - const: cd
> + else:
> + properties:
> + clocks:
> + minItems: 1
> + maxItems: 2
> + clock-names:
> + minItems: 1
> + items:
> + - const: core
> + - const: cd
[Severity: Medium]
Since the top-level resets and reset-names maxItems were increased to 3, do
the conditional branches for R-Car Gen2/3/4 and the subsequent fallback branch
need to explicitly constrain resets and reset-names back to maxItems: 1?
Without overriding the top-level relaxation here, it appears legacy controllers
are now permitted to incorrectly define up to 3 resets.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260622155610.184271-1-biju.das.jz@bp.renesas.com?part=1
^ permalink raw reply
* [PATCH v18 01/12] dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC
From: Biju @ 2026-06-22 15:55 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Magnus Damm
Cc: Biju Das, Wolfram Sang, linux-mmc, devicetree, linux-kernel,
linux-renesas-soc, Prabhakar Mahadev Lad, Biju Das, Conor Dooley
In-Reply-To: <20260622155610.184271-1-biju.das.jz@bp.renesas.com>
From: Biju Das <biju.das.jz@bp.renesas.com>
Document the RZ/G3L (r9a08g046) SDHI controller. The RZ/G3L SDHI
controller is similar to RZ/G2L but has five clocks (core, clkh,
cd, aclk, aclkm) and three resets (rst, axim, axis), so update the
clocks/clock-names maximum to 5 and resets/reset-names maximum to 3.
It has an internal divider for all modes except HS400, and a 2048-bit
divider compared to 512 on others.
Acked-by: Conor Dooley <conor.dooley@microchip.com>
Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v17->v18:
* No change.
v1->v17:
* Collected tag.
---
.../devicetree/bindings/mmc/renesas,sdhi.yaml | 101 +++++++++++++-----
1 file changed, 75 insertions(+), 26 deletions(-)
diff --git a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
index 4d66966ce290..16cb395403f6 100644
--- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
+++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
@@ -18,6 +18,7 @@ properties:
- renesas,sdhi-r7s9210 # SH-Mobile AG5
- renesas,sdhi-r8a73a4 # R-Mobile APE6
- renesas,sdhi-r8a7740 # R-Mobile A1
+ - renesas,sdhi-r9a08g046 # RZ/G3L
- renesas,sdhi-r9a09g057 # RZ/V2H(P)
- renesas,sdhi-sh73a0 # R-Mobile APE6
- items:
@@ -86,11 +87,11 @@ properties:
clocks:
minItems: 1
- maxItems: 4
+ maxItems: 5
clock-names:
minItems: 1
- maxItems: 4
+ maxItems: 5
dmas:
minItems: 4
@@ -116,7 +117,12 @@ properties:
maxItems: 1
resets:
- maxItems: 1
+ minItems: 1
+ maxItems: 3
+
+ reset-names:
+ minItems: 1
+ maxItems: 3
pinctrl-0:
minItems: 1
@@ -155,60 +161,101 @@ allOf:
properties:
compatible:
contains:
- enum:
- - renesas,sdhi-r9a09g057
- - renesas,rzg2l-sdhi
+ const: renesas,sdhi-r9a08g046
then:
properties:
clocks:
items:
- description: IMCLK, SDHI channel main clock1.
- description: CLK_HS, SDHI channel High speed clock which operates
- 4 times that of SDHI channel main clock1.
+ 2 times that of SDHI channel main clock1.
- description: IMCLK2, SDHI channel main clock2. When this clock is
turned off, external SD card detection cannot be
detected.
- - description: ACLK, SDHI channel bus clock.
+ - description: ACLK/IACLKS, SDHI channel bus clock.
+ - description: IACLKM, SDHI channel bus clock m.
clock-names:
items:
- const: core
- const: clkh
- const: cd
- const: aclk
+ - const: aclkm
+ resets:
+ items:
+ - description: rst, Core reset.
+ - description: axim, SDHI axi bus reset m.
+ - description: axis, SDHI axi bus reset s.
+ reset-names:
+ items:
+ - const: rst
+ - const: axim
+ - const: axis
required:
- clock-names
- resets
+ - reset-names
else:
if:
properties:
compatible:
contains:
enum:
- - renesas,rcar-gen2-sdhi
- - renesas,rcar-gen3-sdhi
- - renesas,rcar-gen4-sdhi
+ - renesas,sdhi-r9a09g057
+ - renesas,rzg2l-sdhi
then:
properties:
clocks:
- minItems: 1
- maxItems: 3
- clock-names:
- minItems: 1
- uniqueItems: true
items:
- - const: core
- - enum: [ clkh, cd ]
- - const: cd
- else:
- properties:
- clocks:
- minItems: 1
- maxItems: 2
+ - description: IMCLK, SDHI channel main clock1.
+ - description: CLK_HS, SDHI channel High speed clock which operates
+ 4 times that of SDHI channel main clock1.
+ - description: IMCLK2, SDHI channel main clock2. When this clock is
+ turned off, external SD card detection cannot be
+ detected.
+ - description: ACLK, SDHI channel bus clock.
clock-names:
- minItems: 1
items:
- const: core
+ - const: clkh
- const: cd
+ - const: aclk
+ resets:
+ maxItems: 1
+ required:
+ - clock-names
+ - resets
+ else:
+ if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - renesas,rcar-gen2-sdhi
+ - renesas,rcar-gen3-sdhi
+ - renesas,rcar-gen4-sdhi
+ then:
+ properties:
+ clocks:
+ minItems: 1
+ maxItems: 3
+ clock-names:
+ minItems: 1
+ uniqueItems: true
+ items:
+ - const: core
+ - enum: [ clkh, cd ]
+ - const: cd
+ else:
+ properties:
+ clocks:
+ minItems: 1
+ maxItems: 2
+ clock-names:
+ minItems: 1
+ items:
+ - const: core
+ - const: cd
- if:
properties:
@@ -247,7 +294,9 @@ allOf:
properties:
compatible:
contains:
- const: renesas,sdhi-r9a09g057
+ enum:
+ - renesas,sdhi-r9a08g046
+ - renesas,sdhi-r9a09g057
then:
properties:
vqmmc-regulator:
--
2.43.0
^ permalink raw reply related
* [PATCH v18 00/12] Add Renesas RZ/G3L SD/eMMC support
From: Biju @ 2026-06-22 15:55 UTC (permalink / raw)
To: Ulf Hansson, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Geert Uytterhoeven, Philipp Zabel, Magnus Damm
Cc: Biju Das, Wolfram Sang, linux-mmc, devicetree, linux-kernel,
linux-renesas-soc, Prabhakar Mahadev Lad, Biju Das
From: Biju Das <biju.das.jz@bp.renesas.com>
RZ/G3L SoC has:
Channel 0 supports SD and eMMC (including HS400/HS400ES).
Channel 1 supports SD and eMMC (except for HS400).
Channel 2 supports SD.
The SoC supports a maximum frequency of 150 MHz. The SD0 interface does
not support IOVS and PWEN in the SDHI register (no internal regulator),
unlike SD1 and SD2. It has an internal divider for all modes except HS400.
It also has a 2048-bit divider compared to 512 on others. Moreover
RZ/G3L supports HS400 enhanced strobe mode.
v17->v18:
* Collected tag
* Merged patch #4 and #5 and updated commit description
* Annotated the empty sentinel entries in the OF match tables with a
"Sentinel." comment for clarity.
* Retained the tag as it is a trivial cleanup.
* New patches drop struct renesas_sdhi_hw_info, instead using
renesas_sdhi_of_data and tmio_mmc_data.
* Dropped clk, pinctrl, SoC, and board dtsi from this patch series;
will send later.
v1->v17:
* Collected tag for binding patch.
* Resending the series as there is an issue with patch threading from
patch #14.
Biju Das (12):
dt-bindings: mmc: renesas,sdhi: Document RZ/G3L (r9a08g046) SoC
mmc: renesas_sdhi: Fix whitespace alignment in struct
renesas_sdhi_of_data
mmc: renesas_sdhi: Add clk_mask field to support SoC-specific clock
divider widths
mmc: renesas_sdhi: Add max_divider field to support SoC-specific clock
divider ranges
mmc: renesas_sdhi: Add tuning delay support for RZ/G2L
mmc: renesas_sdhi: Add TMIO_MMC_INTERNAL_DIVIDER flag
mmc: renesas_sdhi: Add optional axis/axim reset controls
mmc: renesas_sdhi: Add RZ/G3L SDHI support
mmc: renesas_sdhi: Save and restore IOVS across suspend/resume
mmc: renesas_sdhi: Make HS400 OSEL bit configurable per SoC
mmc: renesas_sdhi: Add RZ/G3L HS400 support
mmc: renesas_sdhi: Add HS400 enhanced strobe support for RZ/G3L
.../devicetree/bindings/mmc/renesas,sdhi.yaml | 101 ++++++--
drivers/mmc/host/renesas_sdhi.h | 12 +-
drivers/mmc/host/renesas_sdhi_core.c | 239 ++++++++++++++----
drivers/mmc/host/renesas_sdhi_internal_dmac.c | 73 +++++-
drivers/mmc/host/renesas_sdhi_sys_dmac.c | 12 +-
include/linux/platform_data/tmio.h | 18 ++
6 files changed, 370 insertions(+), 85 deletions(-)
--
2.43.0
^ permalink raw reply
* Re: [PATCH v2 0/2] iio: temperature: Add support for the STS30 temperature sensor
From: Maxwell Doose @ 2026-06-22 15:51 UTC (permalink / raw)
To: David Lechner
Cc: Jonathan Cameron, 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: <614988b7-c77f-4f0e-b220-c0acf44bef27@baylibre.com>
On Mon, Jun 22, 2026 at 10:45 AM David Lechner <dlechner@baylibre.com> wrote:
>
> On 6/20/26 7:46 PM, Maxwell Doose wrote:
> > Hi all,
> >
> > This patch series adds support for the Sensirion STS30 temperature
> > sensor family. This driver currently supports non clock stretched single
> > shot measurements.
> >
> > Given there were very little issues found with the v1 submission, I've
> > decided to make this a regular patch series rather than an RFC patch.
>
> You should wait at least one week for feedback on a new driver before
> submitting the next revision.
>
> Given that you said in v1 that don't actually have the hardware, I am not
> going to review this. We are getting more patches than I can keep up with
> already.
>
Fair enough, I'll be away until 4 July anyways.
^ permalink raw reply
* Re: [PATCH v2 0/2] iio: temperature: Add support for the STS30 temperature sensor
From: David Lechner @ 2026-06-22 15:45 UTC (permalink / raw)
To: Maxwell Doose, Jonathan Cameron, 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-1-m32285159@gmail.com>
On 6/20/26 7:46 PM, Maxwell Doose wrote:
> Hi all,
>
> This patch series adds support for the Sensirion STS30 temperature
> sensor family. This driver currently supports non clock stretched single
> shot measurements.
>
> Given there were very little issues found with the v1 submission, I've
> decided to make this a regular patch series rather than an RFC patch.
You should wait at least one week for feedback on a new driver before
submitting the next revision.
Given that you said in v1 that don't actually have the hardware, I am not
going to review this. We are getting more patches than I can keep up with
already.
>
> Changes since v1:
> * whole series:
> - Squashed MAINTAINERS updates into both the dt-bindings commit and the
> driver commit.
>
^ permalink raw reply
* Re: [PATCH 2/8] power: sequencing: pcie-m2: Add PCI ID for NXP 88W9098 and AW693 Bluetooth
From: Manivannan Sadhasivam @ 2026-06-22 15:42 UTC (permalink / raw)
To: Sherry Sun (OSS)
Cc: robh, krzk+dt, conor+dt, Frank.Li, s.hauer, kernel, festevam,
amitkumar.karwar, neeraj.sanjaykale, marcel, luiz.dentz,
hongxing.zhu, l.stach, lpieralisi, kwilczynski, bhelgaas, brgl,
imx, linux-pci, linux-arm-kernel, devicetree, linux-kernel,
linux-bluetooth, linux-pm, sherry.sun
In-Reply-To: <20260618101047.4185497-3-sherry.sun@oss.nxp.com>
On Thu, Jun 18, 2026 at 06:10:41PM +0800, Sherry Sun (OSS) wrote:
> From: Sherry Sun <sherry.sun@nxp.com>
>
> 88W9098 is a NXP Wi-Fi/BT combo chip with PCI device ID 0x2b43 under
> Marvell Extended vendor ID. AW693 is a NXP Wi-Fi/BT combo chip with
> PCI device ID 0x3003 under NXP/Philips vendor ID.
>
> Add both chips to pwrseq_m2_pci_ids[] so that the pwrseq-pcie-m2 driver
> can create the Bluetooth serdev device when these cards are inserted into
> a PCIe M.2 Key E connector.
>
> Both chips use "nxp,88w8987-bt" as the serdev compatible string, which
> is the entry point for the btnxpuart driver. The driver identifies the
> actual chip variant at runtime via chip ID auto-detection and loads the
> appropriate firmware accordingly.
>
> Signed-off-by: Sherry Sun <sherry.sun@nxp.com>
Reviewed-by: Manivannan Sadhasivam <mani@kernel.org>
- Mani
> ---
> drivers/power/sequencing/pwrseq-pcie-m2.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/power/sequencing/pwrseq-pcie-m2.c b/drivers/power/sequencing/pwrseq-pcie-m2.c
> index 94c3f4b7ee36..9217ffcfa6e5 100644
> --- a/drivers/power/sequencing/pwrseq-pcie-m2.c
> +++ b/drivers/power/sequencing/pwrseq-pcie-m2.c
> @@ -186,6 +186,10 @@ static int pwrseq_pcie_m2_match(struct pwrseq_device *pwrseq,
> }
>
> static const struct pci_device_id pwrseq_m2_pci_ids[] = {
> + { PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x2b43),
> + .driver_data = (kernel_ulong_t)"nxp,88w8987-bt" },
> + { PCI_DEVICE(PCI_VENDOR_ID_PHILIPS, 0x3003),
> + .driver_data = (kernel_ulong_t)"nxp,88w8987-bt" },
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1107),
> .driver_data = (kernel_ulong_t)"qcom,wcn7850-bt" },
> { PCI_DEVICE(PCI_VENDOR_ID_QCOM, 0x1103),
> --
> 2.50.1
>
--
மணிவண்ணன் சதாசிவம்
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: iio: dac: Add AD5529R
From: David Lechner @ 2026-06-22 15:36 UTC (permalink / raw)
To: Nuno Sá, Rodrigo Alencar
Cc: Jonathan Cameron, Conor Dooley, Janani Sunil, Janani Sunil,
Lars-Peter Clausen, Michael Hennerich, 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: <ajklksIDLsj0BZul@nsa>
On 6/22/26 7:20 AM, Nuno Sá wrote:
> On Mon, Jun 22, 2026 at 12:51:20PM +0100, Rodrigo Alencar wrote:
>> On 22/06/26 11:29, Nuno Sá wrote:
>>> On Mon, Jun 22, 2026 at 10:24:05AM +0100, Rodrigo Alencar wrote:
>>>> On 21/06/26 15:33, 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.
>>>>>
>>>>
>>>> If the device-addressing on the same chip-select is to be handled
>>>> by the spi framework, wouldn't we lose device-specific features?
>>>>
>>>> I understand that this multi-device feature is there mostly to extend the
>>>> channel count from 16 to 32, 48 or 64. I suppose the command:
>>>>
>>>> "MULTI DEVICE SW LDAC MODE"
>>>>
>>>> exists so that software can update channel values accross multiple devices.
>>>
>>> Right! You do have a point! I agree the main driver for a feature like
>>> this is likely to extend the channel count and effectively "aggregate"
>>> devices.
>>>
>>> But I would say that even with the spi solution the MULTI DEVICE stuff
>>> should be doable (as we still need a sort of adi,pin-id property).
>>
>> I don't think we can have something like an IIO buffer shared by multiple
>> devices. Synchronizing separate devices would be doable with proper hardware
>> support for this (probably involving an FGPA).
>
> True!
>
>>
>>> But yes, I do feel that the whole feature is for aggregation so seeing
>>> one device with 32 channels is the expectation here? Rather than seeing
>>> two devices with 16 channels.
>>
>> Yes, I think aggregation is the whole point there... so that the IIO driver
>> is multi-device-aware.
>
> Which makes me feel that different pins per device might be possible
> from an HW point of view but does not make much sense. For example, for
> the buffer example I would expect LDAC to be shared between all the
> devices.
>
> - Nuno Sá
I think I mentioned this on a previous revision, but I still think the
simplest way to go about it would be to assume that all chips treated
as an aggregate device have everything wired in parallel and just add
support for per-chip wiring on an as-needed basis. This is how we have
handled daisy-chained devices so far.
^ permalink raw reply
* [PATCH v2 4/4] staging: fbtft: remove fb_ssd1351 driver
From: Amit Barzilai @ 2026-06-22 15:25 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller
Cc: azuddinadam, chintanlike, dri-devel, devicetree, linux-kernel,
linux-fbdev, linux-staging, Amit Barzilai
In-Reply-To: <20260622152506.78627-1-amit.barzilai22@gmail.com>
The SSD1351 support was added to the ssd130x DRM driver. To avoid
confusion and irrelevant updates, the staging fb_ssd1351 driver is
removed.
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
drivers/staging/fbtft/Kconfig | 5 -
drivers/staging/fbtft/Makefile | 1 -
drivers/staging/fbtft/fb_ssd1351.c | 240 -----------------------------
3 files changed, 246 deletions(-)
delete mode 100644 drivers/staging/fbtft/fb_ssd1351.c
diff --git a/drivers/staging/fbtft/Kconfig b/drivers/staging/fbtft/Kconfig
index 92943564cb91..c8ea1a0f3fb0 100644
--- a/drivers/staging/fbtft/Kconfig
+++ b/drivers/staging/fbtft/Kconfig
@@ -133,11 +133,6 @@ config FB_TFT_SSD1331
help
Framebuffer support for SSD1331
-config FB_TFT_SSD1351
- tristate "FB driver for the SSD1351 LCD Controller"
- help
- Framebuffer support for SSD1351
-
config FB_TFT_ST7735R
tristate "FB driver for the ST7735R LCD Controller"
help
diff --git a/drivers/staging/fbtft/Makefile b/drivers/staging/fbtft/Makefile
index e9cdf0f0a7da..c230bf008fc7 100644
--- a/drivers/staging/fbtft/Makefile
+++ b/drivers/staging/fbtft/Makefile
@@ -28,7 +28,6 @@ obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1305.o
obj-$(CONFIG_FB_TFT_SSD1306) += fb_ssd1306.o
obj-$(CONFIG_FB_TFT_SSD1305) += fb_ssd1325.o
obj-$(CONFIG_FB_TFT_SSD1331) += fb_ssd1331.o
-obj-$(CONFIG_FB_TFT_SSD1351) += fb_ssd1351.o
obj-$(CONFIG_FB_TFT_ST7735R) += fb_st7735r.o
obj-$(CONFIG_FB_TFT_ST7789V) += fb_st7789v.o
obj-$(CONFIG_FB_TFT_TINYLCD) += fb_tinylcd.o
diff --git a/drivers/staging/fbtft/fb_ssd1351.c b/drivers/staging/fbtft/fb_ssd1351.c
deleted file mode 100644
index 6736b09b2f45..000000000000
--- a/drivers/staging/fbtft/fb_ssd1351.c
+++ /dev/null
@@ -1,240 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-
-#include <linux/backlight.h>
-#include <linux/module.h>
-#include <linux/kernel.h>
-#include <linux/init.h>
-#include <linux/spi/spi.h>
-#include <linux/delay.h>
-#include <linux/string_choices.h>
-
-#include "fbtft.h"
-
-#define DRVNAME "fb_ssd1351"
-#define WIDTH 128
-#define HEIGHT 128
-#define GAMMA_NUM 1
-#define GAMMA_LEN 63
-#define DEFAULT_GAMMA "0 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2 2 " \
- "2 2 2 2 2 2 2" \
-
-static void register_onboard_backlight(struct fbtft_par *par);
-
-static int init_display(struct fbtft_par *par)
-{
- if (par->pdata &&
- par->pdata->display.backlight == FBTFT_ONBOARD_BACKLIGHT) {
- /* module uses onboard GPIO for panel power */
- par->fbtftops.register_backlight = register_onboard_backlight;
- }
-
- par->fbtftops.reset(par);
-
- write_reg(par, 0xfd, 0x12); /* Command Lock */
- write_reg(par, 0xfd, 0xb1); /* Command Lock */
- write_reg(par, 0xae); /* Display Off */
- write_reg(par, 0xb3, 0xf1); /* Front Clock Div */
- write_reg(par, 0xca, 0x7f); /* Set Mux Ratio */
- write_reg(par, 0x15, 0x00, 0x7f); /* Set Column Address */
- write_reg(par, 0x75, 0x00, 0x7f); /* Set Row Address */
- write_reg(par, 0xa1, 0x00); /* Set Display Start Line */
- write_reg(par, 0xa2, 0x00); /* Set Display Offset */
- write_reg(par, 0xb5, 0x00); /* Set GPIO */
- write_reg(par, 0xab, 0x01); /* Set Function Selection */
- write_reg(par, 0xb1, 0x32); /* Set Phase Length */
- write_reg(par, 0xb4, 0xa0, 0xb5, 0x55); /* Set Segment Low Voltage */
- write_reg(par, 0xbb, 0x17); /* Set Precharge Voltage */
- write_reg(par, 0xbe, 0x05); /* Set VComH Voltage */
- write_reg(par, 0xc1, 0xc8, 0x80, 0xc8); /* Set Contrast */
- write_reg(par, 0xc7, 0x0f); /* Set Master Contrast */
- write_reg(par, 0xb6, 0x01); /* Set Second Precharge Period */
- write_reg(par, 0xa6); /* Set Display Mode Reset */
- write_reg(par, 0xaf); /* Set Sleep Mode Display On */
-
- return 0;
-}
-
-static void set_addr_win(struct fbtft_par *par, int xs, int ys, int xe, int ye)
-{
- write_reg(par, 0x15, xs, xe);
- write_reg(par, 0x75, ys, ye);
- write_reg(par, 0x5c);
-}
-
-static int set_var(struct fbtft_par *par)
-{
- unsigned int remap;
-
- if (par->fbtftops.init_display != init_display) {
- /* don't risk messing up register A0h */
- return 0;
- }
-
- remap = 0x60 | (par->bgr << 2); /* Set Colour Depth */
-
- switch (par->info->var.rotate) {
- case 0:
- write_reg(par, 0xA0, remap | 0x00 | BIT(4));
- break;
- case 270:
- write_reg(par, 0xA0, remap | 0x03 | BIT(4));
- break;
- case 180:
- write_reg(par, 0xA0, remap | 0x02);
- break;
- case 90:
- write_reg(par, 0xA0, remap | 0x01);
- break;
- }
-
- return 0;
-}
-
-/*
- * Grayscale Lookup Table
- * GS1 - GS63
- * The driver Gamma curve contains the relative values between the entries
- * in the Lookup table.
- *
- * From datasheet:
- * 8.8 Gray Scale Decoder
- *
- * there are total 180 Gamma Settings (Setting 0 to Setting 180)
- * available for the Gray Scale table.
- *
- * The gray scale is defined in incremental way, with reference
- * to the length of previous table entry:
- * Setting of GS1 has to be >= 0
- * Setting of GS2 has to be > Setting of GS1 +1
- * Setting of GS3 has to be > Setting of GS2 +1
- * :
- * Setting of GS63 has to be > Setting of GS62 +1
- *
- */
-static int set_gamma(struct fbtft_par *par, u32 *curves)
-{
- unsigned long tmp[GAMMA_NUM * GAMMA_LEN];
- int i, acc = 0;
-
- for (i = 0; i < 63; i++) {
- if (i > 0 && curves[i] < 2) {
- dev_err(par->info->device,
- "Illegal value in Grayscale Lookup Table at index %d : %d. Must be greater than 1\n",
- i, curves[i]);
- return -EINVAL;
- }
- acc += curves[i];
- tmp[i] = acc;
- if (acc > 180) {
- dev_err(par->info->device,
- "Illegal value(s) in Grayscale Lookup Table. At index=%d : %d, the accumulated value has exceeded 180\n",
- i, acc);
- return -EINVAL;
- }
- }
-
- write_reg(par, 0xB8,
- tmp[0], tmp[1], tmp[2], tmp[3],
- tmp[4], tmp[5], tmp[6], tmp[7],
- tmp[8], tmp[9], tmp[10], tmp[11],
- tmp[12], tmp[13], tmp[14], tmp[15],
- tmp[16], tmp[17], tmp[18], tmp[19],
- tmp[20], tmp[21], tmp[22], tmp[23],
- tmp[24], tmp[25], tmp[26], tmp[27],
- tmp[28], tmp[29], tmp[30], tmp[31],
- tmp[32], tmp[33], tmp[34], tmp[35],
- tmp[36], tmp[37], tmp[38], tmp[39],
- tmp[40], tmp[41], tmp[42], tmp[43],
- tmp[44], tmp[45], tmp[46], tmp[47],
- tmp[48], tmp[49], tmp[50], tmp[51],
- tmp[52], tmp[53], tmp[54], tmp[55],
- tmp[56], tmp[57], tmp[58], tmp[59],
- tmp[60], tmp[61], tmp[62]);
-
- return 0;
-}
-
-static int blank(struct fbtft_par *par, bool on)
-{
- fbtft_par_dbg(DEBUG_BLANK, par, "(%s=%s)\n",
- __func__, str_true_false(on));
- if (on)
- write_reg(par, 0xAE);
- else
- write_reg(par, 0xAF);
- return 0;
-}
-
-static struct fbtft_display display = {
- .regwidth = 8,
- .width = WIDTH,
- .height = HEIGHT,
- .gamma_num = GAMMA_NUM,
- .gamma_len = GAMMA_LEN,
- .gamma = DEFAULT_GAMMA,
- .fbtftops = {
- .init_display = init_display,
- .set_addr_win = set_addr_win,
- .set_var = set_var,
- .set_gamma = set_gamma,
- .blank = blank,
- },
-};
-
-static int update_onboard_backlight(struct backlight_device *bd)
-{
- struct fbtft_par *par = bl_get_data(bd);
- bool on;
-
- fbtft_par_dbg(DEBUG_BACKLIGHT, par, "%s: power=%d\n", __func__, bd->props.power);
-
- on = !backlight_is_blank(bd);
- /* Onboard backlight connected to GPIO0 on SSD1351, GPIO1 unused */
- write_reg(par, 0xB5, on ? 0x03 : 0x02);
-
- return 0;
-}
-
-static const struct backlight_ops bl_ops = {
- .update_status = update_onboard_backlight,
-};
-
-static void register_onboard_backlight(struct fbtft_par *par)
-{
- struct backlight_device *bd;
- struct backlight_properties bl_props = { 0, };
-
- bl_props.type = BACKLIGHT_RAW;
- bl_props.power = BACKLIGHT_POWER_OFF;
-
- bd = backlight_device_register(dev_driver_string(par->info->device),
- par->info->device, par, &bl_ops,
- &bl_props);
- if (IS_ERR(bd)) {
- dev_err(par->info->device,
- "cannot register backlight device (%ld)\n",
- PTR_ERR(bd));
- return;
- }
- par->info->bl_dev = bd;
-
- if (!par->fbtftops.unregister_backlight)
- par->fbtftops.unregister_backlight = fbtft_unregister_backlight;
-}
-
-FBTFT_REGISTER_DRIVER(DRVNAME, "solomon,ssd1351", &display);
-
-MODULE_ALIAS("spi:" DRVNAME);
-MODULE_ALIAS("platform:" DRVNAME);
-MODULE_ALIAS("spi:ssd1351");
-MODULE_ALIAS("platform:ssd1351");
-
-MODULE_DESCRIPTION("SSD1351 OLED Driver");
-MODULE_AUTHOR("James Davies");
-MODULE_LICENSE("GPL");
--
2.54.0
^ permalink raw reply related
* [PATCH v2 3/4] drm/ssd130x: Add SSD135X_FAMILY and SSD1351 support
From: Amit Barzilai @ 2026-06-22 15:25 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller
Cc: azuddinadam, chintanlike, dri-devel, devicetree, linux-kernel,
linux-fbdev, linux-staging, Amit Barzilai
In-Reply-To: <20260622152506.78627-1-amit.barzilai22@gmail.com>
The Solomon SSD1351 is a 128x128 RGB color OLED controller. It shares
the SSD133X data path: a column/row addressing window followed by a bulk
RGB565 pixel write. Add it as a new SSD135X_FAMILY rather than a separate
driver, reusing the SSD133X plane, CRTC and blit/clear helpers.
The only data-path difference is that the SSD1351 requires an explicit
Write RAM command (0x5c) after the address window is programmed, before
pixel data is accepted, whereas the SSD133X enters data mode implicitly.
This is emitted from a shared ssd133x_write_pixels() helper so both the
damage-update and clear-screen paths cover it.
The SSD1351 also needs its own init sequence (ssd135x_init), dispatched
via ssd135x_encoder_atomic_enable, and a longer post-reset settle delay.
The re-map byte is fixed at 0 degrees, 65k color, COM split, BGR
sub-pixel order; rotation is not supported.
The SSD1351 is SPI-only, so only the SPI transport match tables gain an
entry; no new config symbol is needed.
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
drivers/gpu/drm/solomon/ssd130x-spi.c | 7 +
drivers/gpu/drm/solomon/ssd130x.c | 214 +++++++++++++++++++++-----
drivers/gpu/drm/solomon/ssd130x.h | 5 +-
3 files changed, 189 insertions(+), 37 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x-spi.c b/drivers/gpu/drm/solomon/ssd130x-spi.c
index b52f5fd592a1..6e0dd6e5a88d 100644
--- a/drivers/gpu/drm/solomon/ssd130x-spi.c
+++ b/drivers/gpu/drm/solomon/ssd130x-spi.c
@@ -146,6 +146,11 @@ static const struct of_device_id ssd130x_of_match[] = {
.compatible = "solomon,ssd1331",
.data = &ssd130x_variants[SSD1331_ID],
},
+ /* ssd135x family */
+ {
+ .compatible = "solomon,ssd1351",
+ .data = &ssd130x_variants[SSD1351_ID],
+ },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(of, ssd130x_of_match);
@@ -171,6 +176,8 @@ static const struct spi_device_id ssd130x_spi_id[] = {
{ "ssd1327", SSD1327_ID },
/* ssd133x family */
{ "ssd1331", SSD1331_ID },
+ /* ssd135x family */
+ { "ssd1351", SSD1351_ID },
{ /* sentinel */ }
};
MODULE_DEVICE_TABLE(spi, ssd130x_spi_id);
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 2b0a8218f529..e5a9428f91b8 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -146,6 +146,33 @@
#define SSD133X_COLOR_DEPTH_256 0x0
#define SSD133X_COLOR_DEPTH_65K 0x1
+/* ssd135x commands */
+#define SSD135X_SET_COL_RANGE 0x15
+#define SSD135X_WRITE_RAM 0x5c
+#define SSD135X_SET_ROW_RANGE 0x75
+#define SSD135X_SET_DISPLAY_START 0xa1
+#define SSD135X_SET_DISPLAY_OFFSET 0xa2
+#define SSD135X_SET_DISPLAY_NORMAL 0xa6
+#define SSD135X_SET_FUNCTION 0xab
+#define SSD135X_SET_PHASE_LENGTH 0xb1
+#define SSD135X_SET_CLOCK_FREQ 0xb3
+#define SSD135X_SET_VSL 0xb4
+#define SSD135X_SET_GPIO 0xb5
+#define SSD135X_SET_PRECHARGE2 0xb6
+#define SSD135X_SET_PRECHARGE 0xbb
+#define SSD135X_SET_VCOMH_VOLTAGE 0xbe
+#define SSD135X_SET_CONTRAST 0xc1
+#define SSD135X_SET_CONTRAST_MASTER 0xc7
+#define SSD135X_SET_MUX_RATIO 0xca
+#define SSD135X_SET_COMMAND_LOCK 0xfd
+
+/* ssd135x remap byte (data of SSD13XX_SET_SEG_REMAP) */
+#define SSD135X_SET_REMAP_COLUMN BIT(1)
+#define SSD135X_SET_REMAP_COLOR_BGR BIT(2)
+#define SSD135X_SET_REMAP_COM_SCAN BIT(4)
+#define SSD135X_SET_REMAP_COM_SPLIT BIT(5)
+#define SSD135X_SET_REMAP_65K BIT(6)
+
#define MAX_CONTRAST 255
const struct ssd130x_deviceinfo ssd130x_variants[] = {
@@ -214,6 +241,13 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
.default_height = 64,
.format_rgb565 = 1,
.family_id = SSD133X_FAMILY,
+ },
+ /* ssd135x family */
+ [SSD1351_ID] = {
+ .default_width = 128,
+ .default_height = 128,
+ .format_rgb565 = 1,
+ .family_id = SSD135X_FAMILY,
}
};
EXPORT_SYMBOL_NS_GPL(ssd130x_variants, "DRM_SSD130X");
@@ -248,47 +282,16 @@ static inline struct ssd130x_device *drm_to_ssd130x(struct drm_device *drm)
/*
* Helper to write data (SSD13XX_DATA) to the device.
*/
-static int ssd130x_write_data(struct ssd130x_device *ssd130x, u8 *values, int count)
+static int ssd130x_write_data(struct ssd130x_device *ssd130x, const u8 *values, int count)
{
return regmap_bulk_write(ssd130x->regmap, SSD13XX_DATA, values, count);
}
-/*
- * Helper to write command (SSD13XX_COMMAND). The fist variadic argument
- * is the command to write and the following are the command options.
- *
- * Note that the ssd13xx protocol requires each command and option to be
- * written as a SSD13XX_COMMAND device register value. That is why a call
- * to regmap_write(..., SSD13XX_COMMAND, ...) is done for each argument.
- */
-static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
- /* u8 cmd, u8 option, ... */...)
-{
- va_list ap;
- u8 value;
- int ret;
-
- va_start(ap, count);
-
- do {
- value = va_arg(ap, int);
- ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, value);
- if (ret)
- goto out_end;
- } while (--count);
-
-out_end:
- va_end(ap);
-
- return ret;
-}
-
/*
* Write a command byte sequence from a buffer.
*
- * Like ssd130x_write_cmd() but takes a pre-built byte array instead of
- * variadic arguments, handy when the command is already in an array or
- * when the caller wants to use sizeof() for the length.
+ * The first byte is the command opcode and the following bytes are its
+ * options/parameters.
*/
static int ssd130x_write_cmds(struct ssd130x_device *ssd130x, const u8 *cmd,
size_t len)
@@ -296,6 +299,22 @@ static int ssd130x_write_cmds(struct ssd130x_device *ssd130x, const u8 *cmd,
unsigned int i;
int ret;
+ /*
+ * The SSD135X family latches command parameters with D/C# HIGH (i.e.
+ * clocked in as data), unlike the other families where the opcode and
+ * all of its parameters are sent as commands (D/C# LOW). Send the
+ * opcode as a command and any following parameter bytes as data.
+ */
+ if (ssd130x->device_info->family_id == SSD135X_FAMILY) {
+ if (len == 0)
+ return 0;
+ ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, cmd[0]);
+ if (ret || len == 1)
+ return ret;
+
+ return ssd130x_write_data(ssd130x, cmd + 1, len - 1);
+ }
+
for (i = 0; i < len; i++) {
ret = regmap_write(ssd130x->regmap, SSD13XX_COMMAND, cmd[i]);
if (ret)
@@ -305,6 +324,28 @@ static int ssd130x_write_cmds(struct ssd130x_device *ssd130x, const u8 *cmd,
return 0;
}
+/*
+ * Variadic wrapper around ssd130x_write_cmds(). The first variadic argument is
+ * the command opcode and the following ones are its options/parameters.
+ */
+static int ssd130x_write_cmd(struct ssd130x_device *ssd130x, int count,
+ /* u8 cmd, u8 option, ... */...)
+{
+ u8 buf[8];
+ va_list ap;
+ int i;
+
+ if (count > ARRAY_SIZE(buf))
+ return -EINVAL;
+
+ va_start(ap, count);
+ for (i = 0; i < count; i++)
+ buf[i] = va_arg(ap, int);
+ va_end(ap);
+
+ return ssd130x_write_cmds(ssd130x, buf, count);
+}
+
/*
* Run a packed command sequence. The format is a flat byte array where each
* entry starts with a length byte followed by that many command bytes. A
@@ -628,6 +669,49 @@ static int ssd133x_init(struct ssd130x_device *ssd130x)
return ssd130x_run_cmd_seq(ssd130x, cmds);
}
+static int ssd135x_init(struct ssd130x_device *ssd130x)
+{
+ /*
+ * Horizontal address increment, COM split, reversed COM scan direction,
+ * BGR sub-pixel order and 65k (RGB565) color depth. Rotation is not
+ * supported, so the remap byte is fixed.
+ */
+ u8 remap = SSD135X_SET_REMAP_65K | SSD135X_SET_REMAP_COM_SPLIT |
+ SSD135X_SET_REMAP_COLOR_BGR | SSD135X_SET_REMAP_COM_SCAN;
+ const u8 cmds[] = {
+ 2, SSD135X_SET_COMMAND_LOCK, 0x12,
+ 2, SSD135X_SET_COMMAND_LOCK, 0xb1,
+ 1, SSD13XX_DISPLAY_OFF,
+ 2, SSD135X_SET_CLOCK_FREQ, 0xf1,
+ 2, SSD135X_SET_MUX_RATIO, ssd130x->height - 1,
+ 3, SSD135X_SET_COL_RANGE, 0x00, ssd130x->width - 1,
+ 3, SSD135X_SET_ROW_RANGE, 0x00, ssd130x->height - 1,
+ 2, SSD135X_SET_DISPLAY_START, 0x00,
+ 2, SSD135X_SET_DISPLAY_OFFSET, 0x00,
+ 2, SSD135X_SET_GPIO, 0x00,
+ 2, SSD135X_SET_FUNCTION, 0x01,
+ 2, SSD135X_SET_PHASE_LENGTH, 0x32,
+ 4, SSD135X_SET_VSL, 0xa0, 0xb5, 0x55,
+ 2, SSD135X_SET_PRECHARGE, 0x17,
+ 2, SSD135X_SET_VCOMH_VOLTAGE, 0x05,
+ 4, SSD135X_SET_CONTRAST, 0xc8, 0x80, 0xc8,
+ 2, SSD135X_SET_CONTRAST_MASTER, 0x0f,
+ 2, SSD135X_SET_PRECHARGE2, 0x01,
+ 1, SSD135X_SET_DISPLAY_NORMAL,
+ 2, SSD13XX_SET_SEG_REMAP, remap,
+ 0,
+ };
+
+ /*
+ * ssd130x_power_on() issues a short reset pulse, but the SSD1351 is not
+ * ready to accept commands immediately afterwards. Give the controller
+ * time to settle before sending the init sequence.
+ */
+ msleep(120);
+
+ return ssd130x_run_cmd_seq(ssd130x, cmds);
+}
+
static int ssd130x_update_rect(struct ssd130x_device *ssd130x,
struct drm_rect *rect, u8 *buf,
u8 *data_array)
@@ -790,6 +874,25 @@ static int ssd132x_update_rect(struct ssd130x_device *ssd130x,
return ret;
}
+/*
+ * Write a run of pixel data to the controller's display RAM. The SSD135X
+ * family requires an explicit Write RAM command once the address window has
+ * been set, before any pixel data is accepted; the SSD133X family enters data
+ * mode implicitly after the column/row range is programmed.
+ */
+static int ssd133x_write_pixels(struct ssd130x_device *ssd130x,
+ u8 *data_array, unsigned int count)
+{
+ if (ssd130x->device_info->family_id == SSD135X_FAMILY) {
+ int ret = ssd130x_write_cmd(ssd130x, 1, SSD135X_WRITE_RAM);
+
+ if (ret < 0)
+ return ret;
+ }
+
+ return ssd130x_write_data(ssd130x, data_array, count);
+}
+
static int ssd133x_update_rect(struct ssd130x_device *ssd130x,
struct drm_rect *rect, u8 *data_array,
unsigned int pitch)
@@ -832,7 +935,7 @@ static int ssd133x_update_rect(struct ssd130x_device *ssd130x,
return ret;
/* Write out update in one go since horizontal addressing mode is used */
- ret = ssd130x_write_data(ssd130x, data_array, pitch * rows);
+ ret = ssd133x_write_pixels(ssd130x, data_array, pitch * rows);
return ret;
}
@@ -917,7 +1020,7 @@ static void ssd133x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
memset(data_array, 0, pitch * ssd130x->height);
/* Write out update in one go since horizontal addressing mode is used */
- ssd130x_write_data(ssd130x, data_array, pitch * ssd130x->height);
+ ssd133x_write_pixels(ssd130x, data_array, pitch * ssd130x->height);
}
static int ssd130x_fb_blit_rect(struct drm_framebuffer *fb,
@@ -1380,6 +1483,12 @@ static const struct drm_plane_helper_funcs ssd130x_primary_plane_helper_funcs[]
.atomic_check = ssd133x_primary_plane_atomic_check,
.atomic_update = ssd133x_primary_plane_atomic_update,
.atomic_disable = ssd133x_primary_plane_atomic_disable,
+ },
+ [SSD135X_FAMILY] = {
+ DRM_GEM_SHADOW_PLANE_HELPER_FUNCS,
+ .atomic_check = ssd133x_primary_plane_atomic_check,
+ .atomic_update = ssd133x_primary_plane_atomic_update,
+ .atomic_disable = ssd133x_primary_plane_atomic_disable,
}
};
@@ -1534,6 +1643,10 @@ static const struct drm_crtc_helper_funcs ssd130x_crtc_helper_funcs[] = {
.mode_valid = ssd130x_crtc_mode_valid,
.atomic_check = ssd133x_crtc_atomic_check,
},
+ [SSD135X_FAMILY] = {
+ .mode_valid = ssd130x_crtc_mode_valid,
+ .atomic_check = ssd133x_crtc_atomic_check,
+ },
};
static const struct drm_crtc_funcs ssd130x_crtc_funcs = {
@@ -1621,6 +1734,31 @@ static void ssd133x_encoder_atomic_enable(struct drm_encoder *encoder,
ssd130x_power_off(ssd130x);
}
+static void ssd135x_encoder_atomic_enable(struct drm_encoder *encoder,
+ struct drm_atomic_commit *state)
+{
+ struct drm_device *drm = encoder->dev;
+ struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
+ int ret;
+
+ ret = ssd130x_power_on(ssd130x);
+ if (ret)
+ return;
+
+ ret = ssd135x_init(ssd130x);
+ if (ret)
+ goto power_off;
+
+ ssd130x_write_cmd(ssd130x, 1, SSD13XX_DISPLAY_ON);
+
+ backlight_enable(ssd130x->bl_dev);
+
+ return;
+
+power_off:
+ ssd130x_power_off(ssd130x);
+}
+
static void ssd130x_encoder_atomic_disable(struct drm_encoder *encoder,
struct drm_atomic_commit *state)
{
@@ -1646,6 +1784,10 @@ static const struct drm_encoder_helper_funcs ssd130x_encoder_helper_funcs[] = {
[SSD133X_FAMILY] = {
.atomic_enable = ssd133x_encoder_atomic_enable,
.atomic_disable = ssd130x_encoder_atomic_disable,
+ },
+ [SSD135X_FAMILY] = {
+ .atomic_enable = ssd135x_encoder_atomic_enable,
+ .atomic_disable = ssd130x_encoder_atomic_disable,
}
};
diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h
index b0b487c06e04..da89d4455270 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -26,7 +26,8 @@
enum ssd130x_family_ids {
SSD130X_FAMILY,
SSD132X_FAMILY,
- SSD133X_FAMILY
+ SSD133X_FAMILY,
+ SSD135X_FAMILY
};
enum ssd130x_variants {
@@ -42,6 +43,8 @@ enum ssd130x_variants {
SSD1327_ID,
/* ssd133x family */
SSD1331_ID,
+ /* ssd135x family */
+ SSD1351_ID,
NR_SSD130X_VARIANTS
};
--
2.54.0
^ permalink raw reply related
* [PATCH v2 2/4] drm/ssd130x: Add RGB565 support to SSD133X family
From: Amit Barzilai @ 2026-06-22 15:25 UTC (permalink / raw)
To: javierm, maarten.lankhorst, mripard, tzimmermann, airlied, simona,
robh, krzk+dt, conor+dt, andy, gregkh, deller
Cc: azuddinadam, chintanlike, dri-devel, devicetree, linux-kernel,
linux-fbdev, linux-staging, Amit Barzilai
In-Reply-To: <20260622152506.78627-1-amit.barzilai22@gmail.com>
SSD133X screens were getting 8bpp (RGB332) instead of the 16bpp
(RGB565) that they support. This change adds a boolean to the
deviceinfo struct selecting whether the variant is driven at
DRM_FORMAT_RGB565.
Changed SSD133X to now utilize 65k color (RGB565).
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Amit Barzilai <amit.barzilai22@gmail.com>
---
drivers/gpu/drm/solomon/ssd130x.c | 55 +++++++++++++++++++++++++------
drivers/gpu/drm/solomon/ssd130x.h | 7 ++++
2 files changed, 52 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index 04da4f2f7d08..2b0a8218f529 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -140,6 +140,12 @@
#define SSD133X_SET_PRECHARGE_VOLTAGE 0xbb
#define SSD133X_SET_VCOMH_VOLTAGE 0xbe
+/* ssd133x remap byte (data of SSD13XX_SET_SEG_REMAP) */
+#define SSD133X_SET_REMAP_COM_SPLIT BIT(5)
+#define SSD133X_SET_REMAP_COLOR_DEPTH_MASK GENMASK(7, 6)
+#define SSD133X_COLOR_DEPTH_256 0x0
+#define SSD133X_COLOR_DEPTH_65K 0x1
+
#define MAX_CONTRAST 255
const struct ssd130x_deviceinfo ssd130x_variants[] = {
@@ -206,6 +212,7 @@ const struct ssd130x_deviceinfo ssd130x_variants[] = {
[SSD1331_ID] = {
.default_width = 96,
.default_height = 64,
+ .format_rgb565 = 1,
.family_id = SSD133X_FAMILY,
}
};
@@ -584,6 +591,10 @@ static int ssd132x_init(struct ssd130x_device *ssd130x)
static int ssd133x_init(struct ssd130x_device *ssd130x)
{
+ u8 remap = SSD133X_SET_REMAP_COM_SPLIT |
+ FIELD_PREP(SSD133X_SET_REMAP_COLOR_DEPTH_MASK,
+ ssd130x->device_info->format_rgb565 ?
+ SSD133X_COLOR_DEPTH_65K : SSD133X_COLOR_DEPTH_256);
const u8 cmds[] = {
2, SSD133X_CONTRAST_A, 0x91,
2, SSD133X_CONTRAST_B, 0x50,
@@ -595,9 +606,9 @@ static int ssd133x_init(struct ssd130x_device *ssd130x)
* Horizontal Address Increment
* Normal order SA,SB,SC (e.g. RGB)
* COM Split Odd Even
- * 256 color format
+ * 256 or 65k color format, depending on the variant
*/
- 2, SSD13XX_SET_SEG_REMAP, 0x20,
+ 2, SSD13XX_SET_SEG_REMAP, remap,
2, SSD133X_SET_DISPLAY_START, 0x00,
2, SSD133X_SET_DISPLAY_OFFSET, 0x00,
1, SSD133X_SET_DISPLAY_NORMAL,
@@ -794,14 +805,20 @@ static int ssd133x_update_rect(struct ssd130x_device *ssd130x,
* COM0 to COM[N - 1] are the rows and SEG0 to SEG[M - 1] are
* the columns.
*
- * Each Segment has a 8-bit pixel and each Common output has a
- * row of pixels. When using the (default) horizontal address
- * increment mode, each byte of data sent to the controller has
- * a Segment (e.g: SEG0).
+ * Each Segment holds one pixel and each Common output has a row
+ * of pixels. A pixel is 8 bits (one byte) in the 256 color
+ * (RGB332) format or 16 bits (two bytes) in the 65k color
+ * (RGB565) format. When using the (default) horizontal address
+ * increment mode, the pixel data is sent Segment by Segment
+ * (e.g: SEG0 first).
*
* When using the 256 color depth format, each pixel contains 3
* sub-pixels for color A, B and C. These have 3 bit, 3 bit and
* 2 bits respectively.
+ *
+ * When using the 65k color depth format, each pixel contains 3
+ * sub-pixels for color A, B and C. These have 5 bit, 6 bit and
+ * 5 bits respectively.
*/
/* Set column start and end */
@@ -872,9 +889,24 @@ static void ssd132x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
ssd130x_write_data(ssd130x, data_array, columns * height);
}
+/*
+ * The SSD133X family can drive the panel in either RGB332 (1 byte per pixel)
+ * or RGB565 (2 bytes per pixel). The format is a per-variant policy choice
+ * selected through ssd130x_deviceinfo::format_rgb565, not a capability probe.
+ * Centralize the choice here so that the buffer sizing (allocation, clear and
+ * blit pitch) can never disagree.
+ */
+static const struct drm_format_info *ssd133x_format_info(struct ssd130x_device *ssd130x)
+{
+ if (ssd130x->device_info->format_rgb565)
+ return drm_format_info(DRM_FORMAT_RGB565);
+
+ return drm_format_info(DRM_FORMAT_RGB332);
+}
+
static void ssd133x_clear_screen(struct ssd130x_device *ssd130x, u8 *data_array)
{
- const struct drm_format_info *fi = drm_format_info(DRM_FORMAT_RGB332);
+ const struct drm_format_info *fi = ssd133x_format_info(ssd130x);
unsigned int pitch;
if (!fi)
@@ -945,7 +977,7 @@ static int ssd133x_fb_blit_rect(struct drm_framebuffer *fb,
struct drm_format_conv_state *fmtcnv_state)
{
struct ssd130x_device *ssd130x = drm_to_ssd130x(fb->dev);
- const struct drm_format_info *fi = drm_format_info(DRM_FORMAT_RGB332);
+ const struct drm_format_info *fi = ssd133x_format_info(ssd130x);
unsigned int dst_pitch;
struct iosys_map dst;
int ret = 0;
@@ -956,7 +988,10 @@ static int ssd133x_fb_blit_rect(struct drm_framebuffer *fb,
dst_pitch = drm_format_info_min_pitch(fi, 0, drm_rect_width(rect));
iosys_map_set_vaddr(&dst, data_array);
- drm_fb_xrgb8888_to_rgb332(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
+ if (ssd130x->device_info->format_rgb565)
+ drm_fb_xrgb8888_to_rgb565be(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
+ else
+ drm_fb_xrgb8888_to_rgb332(&dst, &dst_pitch, vmap, fb, rect, fmtcnv_state);
ssd133x_update_rect(ssd130x, rect, data_array, dst_pitch);
@@ -1414,7 +1449,7 @@ static int ssd133x_crtc_atomic_check(struct drm_crtc *crtc,
struct ssd130x_device *ssd130x = drm_to_ssd130x(drm);
struct drm_crtc_state *crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
struct ssd130x_crtc_state *ssd130x_state = to_ssd130x_crtc_state(crtc_state);
- const struct drm_format_info *fi = drm_format_info(DRM_FORMAT_RGB332);
+ const struct drm_format_info *fi = ssd133x_format_info(ssd130x);
unsigned int pitch;
int ret;
diff --git a/drivers/gpu/drm/solomon/ssd130x.h b/drivers/gpu/drm/solomon/ssd130x.h
index a4554018bb2a..b0b487c06e04 100644
--- a/drivers/gpu/drm/solomon/ssd130x.h
+++ b/drivers/gpu/drm/solomon/ssd130x.h
@@ -54,6 +54,13 @@ struct ssd130x_deviceinfo {
bool need_pwm;
bool need_chargepump;
bool page_mode_only;
+ /*
+ * Per-variant output format selector for the SSD133X data path. The
+ * hardware can drive the panel in RGB332 (1 byte/pixel) or RGB565
+ * (2 bytes/pixel); this is a policy choice per variant, not a
+ * capability probe. When set, the variant is driven at RGB565.
+ */
+ bool format_rgb565;
enum ssd130x_family_ids family_id;
};
--
2.54.0
^ permalink raw reply related
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