* [PATCH v6 3/3] arm64: dts: nuvoton: Add I2C nodes for MA35D1 SoC
From: Zi-Yu Chen @ 2026-07-20 3:06 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-arm-kernel, linux-kernel
Cc: Jacky Huang, Shan-Chun Hung, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Zi-Yu Chen
In-Reply-To: <20260720030616.1067660-1-zychennvt@gmail.com>
Add I2C controller nodes to the MA35D1 SoC dtsi file.
Also, enable the I2C2 interface on the MA35D1 SOM board and
configure its pinctrl to allow communication with the onboard
NAU8822 audio codec
Signed-off-by: Zi-Yu Chen <zychennvt@gmail.com>
---
.../boot/dts/nuvoton/ma35d1-som-256m.dts | 20 ++++++
arch/arm64/boot/dts/nuvoton/ma35d1.dtsi | 72 +++++++++++++++++++
2 files changed, 92 insertions(+)
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
index a029b660e8dc..422468dbc3bf 100644
--- a/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1-som-256m.dts
@@ -57,6 +57,18 @@ &clk {
"integer";
};
+&i2c2 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&pinctrl_i2c2>;
+
+ nau8822: audio-codec@1a {
+ compatible = "nuvoton,nau8822";
+ reg = <0x1a> ;
+ nuvoton,spk-btl;
+ };
+};
+
&pinctrl {
uart-grp {
pinctrl_uart0: uart0-pins {
@@ -100,6 +112,14 @@ pinctrl_uart16: uart16-pins {
power-source = <1>;
};
};
+
+ i2c-grp {
+ pinctrl_i2c2: i2c2-pins {
+ nuvoton,pins = <1 8 4>,
+ <1 9 4>;
+ bias-disable;
+ };
+ };
};
&uart0 {
diff --git a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
index 7228ad4735b5..28488889f564 100644
--- a/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
+++ b/arch/arm64/boot/dts/nuvoton/ma35d1.dtsi
@@ -372,6 +372,78 @@ uart15: serial@407f0000 {
status = "disabled";
};
+ i2c0: i2c@40800000 {
+ compatible = "nuvoton,ma35d1-i2c";
+ reg = <0x0 0x40800000 0x0 0x1000>;
+ interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk I2C0_GATE>;
+ clock-frequency = <100000>;
+ resets = <&sys MA35D1_RESET_I2C0>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c1: i2c@40810000 {
+ compatible = "nuvoton,ma35d1-i2c";
+ reg = <0x0 0x40810000 0x0 0x1000>;
+ interrupts = <GIC_SPI 69 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk I2C1_GATE>;
+ clock-frequency = <100000>;
+ resets = <&sys MA35D1_RESET_I2C1>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c2: i2c@40820000 {
+ compatible = "nuvoton,ma35d1-i2c";
+ reg = <0x0 0x40820000 0x0 0x1000>;
+ interrupts = <GIC_SPI 97 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk I2C2_GATE>;
+ clock-frequency = <100000>;
+ resets = <&sys MA35D1_RESET_I2C2>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c3: i2c@40830000 {
+ compatible = "nuvoton,ma35d1-i2c";
+ reg = <0x0 0x40830000 0x0 0x1000>;
+ interrupts = <GIC_SPI 98 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk I2C3_GATE>;
+ clock-frequency = <100000>;
+ resets = <&sys MA35D1_RESET_I2C3>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c4: i2c@40840000 {
+ compatible = "nuvoton,ma35d1-i2c";
+ reg = <0x0 0x40840000 0x0 0x1000>;
+ interrupts = <GIC_SPI 123 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk I2C4_GATE>;
+ clock-frequency = <100000>;
+ resets = <&sys MA35D1_RESET_I2C4>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
+ i2c5: i2c@40850000 {
+ compatible = "nuvoton,ma35d1-i2c";
+ reg = <0x0 0x40850000 0x0 0x1000>;
+ interrupts = <GIC_SPI 124 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk I2C5_GATE>;
+ clock-frequency = <100000>;
+ resets = <&sys MA35D1_RESET_I2C5>;
+ status = "disabled";
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+
uart16: serial@40880000 {
compatible = "nuvoton,ma35d1-uart";
reg = <0x0 0x40880000 0x0 0x100>;
--
2.34.1
^ permalink raw reply related
* [PATCH v6 2/3] i2c: ma35d1: Add Nuvoton MA35D1 I2C driver support
From: Zi-Yu Chen @ 2026-07-20 3:06 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-arm-kernel, linux-kernel
Cc: Jacky Huang, Shan-Chun Hung, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Zi-Yu Chen
In-Reply-To: <20260720030616.1067660-1-zychennvt@gmail.com>
Add I2C support for Nuvoton MA35D1 SoC.
The controller supports standard, fast and fast-plus modes,
and provides controller/target functionality.
Signed-off-by: Zi-Yu Chen <zychennvt@gmail.com>
---
drivers/i2c/busses/Kconfig | 13 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-ma35d1.c | 780 ++++++++++++++++++++++++++++++++
3 files changed, 794 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-ma35d1.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index d7b89508311f..e69120d46977 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1602,4 +1602,17 @@ config I2C_VIRTIO
This driver can also be built as a module. If so, the module
will be called i2c-virtio.
+config I2C_MA35D1
+ tristate "Nuvoton MA35D1 I2C driver"
+ depends on ARCH_MA35 || COMPILE_TEST
+ select I2C_SLAVE
+ help
+ If you say yes to this option, support will be included for the
+ I2C controller in the Nuvoton MA35D1 SoC. This driver
+ supports the standard I2C bus protocols, including master and
+ slave modes.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-ma35d1.
+
endmenu
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 3755c54b3d82..ca75dae4955c 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -131,6 +131,7 @@ obj-$(CONFIG_I2C_XILINX) += i2c-xiic.o
obj-$(CONFIG_I2C_XLP9XX) += i2c-xlp9xx.o
obj-$(CONFIG_I2C_RCAR) += i2c-rcar.o
obj-$(CONFIG_I2C_GXP) += i2c-gxp.o
+obj-$(CONFIG_I2C_MA35D1) += i2c-ma35d1.o
# External I2C/SMBus adapter drivers
obj-$(CONFIG_I2C_DIOLAN_U2C) += i2c-diolan-u2c.o
diff --git a/drivers/i2c/busses/i2c-ma35d1.c b/drivers/i2c/busses/i2c-ma35d1.c
new file mode 100644
index 000000000000..4ac49b1b22e0
--- /dev/null
+++ b/drivers/i2c/busses/i2c-ma35d1.c
@@ -0,0 +1,780 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2026 Nuvoton technology corporation.
+ *
+ * Author: Zi-Yu Chen <zychennvt@gmail.com>
+ */
+
+#include <linux/bitfield.h>
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/iopoll.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/property.h>
+#include <linux/reset.h>
+#include <linux/units.h>
+
+/* MA35D1 I2C registers offset */
+#define MA35_CTL0 0x00
+#define MA35_ADDR0 0x04
+#define MA35_DAT 0x08
+#define MA35_STATUS0 0x0c
+#define MA35_CLKDIV 0x10
+#define MA35_TOCTL 0x14
+#define MA35_ADDR1 0x18
+#define MA35_ADDR2 0x1c
+#define MA35_ADDR3 0x20
+#define MA35_ADDRMSK0 0x24
+#define MA35_ADDRMSK1 0x28
+#define MA35_ADDRMSK2 0x2c
+#define MA35_ADDRMSK3 0x30
+#define MA35_WKCTL 0x3c
+#define MA35_WKSTS 0x40
+#define MA35_CTL1 0x44
+#define MA35_STATUS1 0x48
+#define MA35_TMCTL 0x4c
+#define MA35_BUSCTL 0x50
+#define MA35_BUSTCTL 0x54
+#define MA35_BUSSTS 0x58
+#define MA35_PKTSIZE 0x5c
+#define MA35_PKTCRC 0x60
+#define MA35_BUSTOUT 0x64
+#define MA35_CLKTOUT 0x68
+#define MA35_AUTOCNT 0x78
+
+/* MA35D1 I2C Status */
+/* Controller */
+#define MA35_M_START 0x08 /* Start */
+#define MA35_M_REPEAT_START 0x10 /* Controller Repeat Start */
+#define MA35_M_TRAN_ADDR_ACK 0x18 /* Controller Transmit Address ACK */
+#define MA35_M_TRAN_ADDR_NACK 0x20 /* Controller Transmit Address NACK */
+#define MA35_M_TRAN_DATA_ACK 0x28 /* Controller Transmit Data ACK */
+#define MA35_M_TRAN_DATA_NACK 0x30 /* Controller Transmit Data NACK */
+#define MA35_M_ARB_LOST 0x38 /* Controller Arbitration Lost */
+#define MA35_M_RECE_ADDR_ACK 0x40 /* Controller Receive Address ACK */
+#define MA35_M_RECE_ADDR_NACK 0x48 /* Controller Receive Address NACK */
+#define MA35_M_RECE_DATA_ACK 0x50 /* Controller Receive Data ACK */
+#define MA35_M_RECE_DATA_NACK 0x58 /* Controller Receive Data NACK */
+#define MA35_BUS_ERROR 0x00 /* Bus error */
+
+/* Target */
+#define MA35_S_REPEAT_START_STOP 0xa0 /* Target Transmit Repeat Start or Stop */
+#define MA35_S_TRAN_ADDR_ACK 0xa8 /* Target Transmit Address ACK */
+#define MA35_S_TRAN_DATA_ACK 0xb8 /* Target Transmit Data ACK */
+#define MA35_S_TRAN_DATA_NACK 0xc0 /* Target Transmit Data NACK */
+#define MA35_S_TRAN_LAST_DATA_ACK 0xc8 /* Target Transmit Last Data ACK */
+#define MA35_S_RECE_ADDR_ACK 0x60 /* Target Receive Address ACK */
+#define MA35_S_RECE_ARB_LOST 0x68 /* Target Receive Arbitration Lost */
+#define MA35_S_RECE_DATA_ACK 0x80 /* Target Receive Data ACK */
+#define MA35_S_RECE_DATA_NACK 0x88 /* Target Receive Data NACK */
+
+/* Other */
+#define MA35_ADDR_TRAN_ARB_LOST 0xb0 /* Address Transmit Arbitration Lost */
+#define MA35_BUS_RELEASED 0xf8 /* Bus Released */
+
+/* I2C_CTL constant definitions. */
+#define MA35_CTL_AA BIT(2)
+#define MA35_CTL_SI BIT(3)
+#define MA35_CTL_STO BIT(4)
+#define MA35_CTL_STA BIT(5)
+#define MA35_CTL_I2CEN BIT(6)
+#define MA35_CTL_INTEN BIT(7)
+#define MA35_CTL_SI_AA (MA35_CTL_SI | MA35_CTL_AA)
+#define MA35_CTL_STO_SI (MA35_CTL_STO | MA35_CTL_SI)
+#define MA35_CTL_STA_SI (MA35_CTL_STA | MA35_CTL_SI)
+#define MA35_CTL_STA_SI_AA (MA35_CTL_STA | MA35_CTL_SI | MA35_CTL_AA)
+#define MA35_CTL_STO_SI_AA (MA35_CTL_STO | MA35_CTL_SI | MA35_CTL_AA)
+
+/* Constants */
+#define MA35_CLKDIV_MSK GENMASK(15, 0)
+#define I2C_PM_TIMEOUT_MS 5000
+#define STOP_TIMEOUT_MS 50
+
+struct ma35d1_i2c {
+ spinlock_t lock;
+ wait_queue_head_t wait;
+ struct i2c_msg *msg;
+ unsigned int msg_num;
+ unsigned int msg_idx;
+ unsigned int msg_ptr;
+ int err;
+ int irq;
+ void __iomem *regs;
+ struct clk *clk;
+ struct device *dev;
+ struct i2c_adapter adap;
+ struct i2c_client *target;
+ struct reset_control *rst;
+};
+
+static inline bool ma35d1_is_controller_status(unsigned int status)
+{
+ return status >= MA35_M_START && status <= MA35_M_RECE_DATA_NACK;
+}
+
+/*
+ * ma35d1_i2c_write_ctl - Update the I2C control register
+ * @i2c: Pointer to the ma35d1 i2c instance
+ * @ctl: Control bits to set (e.g., MA35_CTL_STA, SI, AA)
+ *
+ * This helper reads CTL0, clears the sticky state-change bits (STA, STO, SI, AA),
+ * and then applies the new control bits provided by @ctl.
+ */
+static void ma35d1_i2c_write_ctl(struct ma35d1_i2c *i2c, unsigned int ctl)
+{
+ unsigned int val;
+
+ guard(spinlock_irqsave)(&i2c->lock);
+ val = readl(i2c->regs + MA35_CTL0);
+ val &= ~(MA35_CTL_STA_SI_AA | MA35_CTL_STO);
+ val |= ctl;
+ writel(val, i2c->regs + MA35_CTL0);
+}
+
+static void ma35d1_i2c_set_addr(struct ma35d1_i2c *i2c)
+{
+ unsigned int rw = i2c->msg->flags & I2C_M_RD;
+
+ writel(((i2c->msg->addr & 0x7f) << 1) | rw, i2c->regs + MA35_DAT);
+}
+
+static void ma35d1_i2c_controller_complete(struct ma35d1_i2c *i2c)
+{
+ i2c->msg_ptr = 0;
+ i2c->msg = NULL;
+ i2c->msg_idx++;
+ i2c->msg_num = 0;
+
+ wake_up(&i2c->wait);
+}
+
+static void ma35d1_i2c_reset(struct ma35d1_i2c *i2c)
+{
+ unsigned int clkdiv, slvaddr;
+
+ clkdiv = readl(i2c->regs + MA35_CLKDIV);
+ slvaddr = readl(i2c->regs + MA35_ADDR0);
+
+ reset_control_assert(i2c->rst);
+ usleep_range(10, 20);
+ reset_control_deassert(i2c->rst);
+
+ writel(clkdiv, (i2c->regs + MA35_CLKDIV));
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_I2CEN);
+
+ if (i2c->target)
+ writel(slvaddr, i2c->regs + MA35_ADDR0);
+}
+
+static void ma35d1_i2c_stop(struct ma35d1_i2c *i2c, int ret)
+{
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_STO_SI);
+
+ if (ret)
+ i2c->err = ret;
+
+ ma35d1_i2c_controller_complete(i2c);
+}
+
+/* Check if this is the last message in the set */
+static inline bool is_last_msg(struct ma35d1_i2c *i2c)
+{
+ return i2c->msg_idx >= (i2c->msg_num - 1);
+}
+
+/* Check if this is the last byte in the current message */
+static inline bool is_last_byte(struct ma35d1_i2c *i2c)
+{
+ return i2c->msg_ptr == i2c->msg->len - 1;
+}
+
+/* Check if reached the end of the current message */
+static inline bool is_msgend(struct ma35d1_i2c *i2c)
+{
+ return i2c->msg_ptr >= i2c->msg->len;
+}
+
+/*
+ * ma35d1_i2c_irq_target_trx - I2C Target state machine handler
+ * @i2c: ma35d1 i2c instance
+ * @i2c_status: hardware status code from MA35_STATUS0
+ */
+static void ma35d1_i2c_irq_target_trx(struct ma35d1_i2c *i2c,
+ unsigned long i2c_status)
+{
+ unsigned char byte;
+
+ switch (i2c_status) {
+ case MA35_S_RECE_ARB_LOST:
+ /*
+ * Arbitration lost during SLA+R/W (Address) phase.
+ * The hardware automatically switches to Target mode as our
+ * own slave address was detected.
+ */
+ i2c->err = -EAGAIN;
+ ma35d1_i2c_controller_complete(i2c);
+ i2c_slave_event(i2c->target, I2C_SLAVE_WRITE_REQUESTED, &byte);
+ break;
+
+ case MA35_S_RECE_ADDR_ACK:
+ /* Own SLA+W has been receive; ACK has been return */
+ i2c_slave_event(i2c->target, I2C_SLAVE_WRITE_REQUESTED, &byte);
+ break;
+
+ case MA35_S_TRAN_DATA_NACK:
+ case MA35_S_RECE_DATA_NACK:
+ /*
+ * Data byte or last data in I2CDAT has been transmitted and NACK received,
+ * or previously addressed with own SLA address and NACK returned.
+ */
+ break;
+
+ case MA35_S_RECE_DATA_ACK:
+ /* Previously address with own SLA address Data has been received;
+ * ACK has been returned
+ */
+ byte = readb(i2c->regs + MA35_DAT);
+ i2c_slave_event(i2c->target, I2C_SLAVE_WRITE_RECEIVED, &byte);
+ break;
+
+ case MA35_ADDR_TRAN_ARB_LOST:
+ /*
+ * Arbitration lost during Data byte transmission phase.
+ * The hardware automatically switches to Target mode as our
+ * own slave address was detected.
+ */
+ i2c->err = -EAGAIN;
+ ma35d1_i2c_controller_complete(i2c);
+ i2c_slave_event(i2c->target, I2C_SLAVE_WRITE_REQUESTED, &byte);
+ break;
+
+ case MA35_S_TRAN_ADDR_ACK:
+ /* Own SLA+R has been receive; ACK has been return */
+ i2c_slave_event(i2c->target, I2C_SLAVE_READ_REQUESTED, &byte);
+ writel(byte, i2c->regs + MA35_DAT);
+ break;
+
+ case MA35_S_TRAN_DATA_ACK:
+ i2c_slave_event(i2c->target, I2C_SLAVE_READ_PROCESSED, &byte);
+ writel(byte, i2c->regs + MA35_DAT);
+ break;
+
+ case MA35_S_REPEAT_START_STOP:
+ /* A STOP or repeated START has been received
+ * while still addressed as Target/Receiver
+ */
+ i2c_slave_event(i2c->target, I2C_SLAVE_STOP, &byte);
+ break;
+
+ default:
+ dev_err(i2c->dev, "Status 0x%02lx is NOT processed\n",
+ i2c_status);
+ break;
+ }
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI_AA);
+}
+
+/*
+ * ma35d1_i2c_irq_controller_trx - I2C Controller state machine handler
+ * @i2c: ma35d1 i2c instance
+ * @i2c_status: hardware status code from MA35_STATUS0
+ */
+static void ma35d1_i2c_irq_controller_trx(struct ma35d1_i2c *i2c,
+ unsigned long i2c_status)
+{
+ unsigned char byte;
+
+ /* Ensuring messages are absolutely valid in Controller mode. */
+ if (unlikely(!i2c->msg)) {
+ dev_err(i2c->dev, "Spurious controller interrupt (status: 0x%02lx)\n", i2c_status);
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI);
+ return;
+ }
+
+ switch (i2c_status) {
+ case MA35_M_START:
+ case MA35_M_REPEAT_START:
+ ma35d1_i2c_set_addr(i2c);
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI);
+ break;
+
+ case MA35_M_TRAN_ADDR_ACK:
+ case MA35_M_TRAN_DATA_ACK:
+ /* SLA+W has been transmitted and ACK has been received */
+ if (i2c_status == MA35_M_TRAN_ADDR_ACK) {
+ if (is_last_msg(i2c) && i2c->msg->len == 0) {
+ ma35d1_i2c_stop(i2c, 0);
+ return;
+ }
+ }
+
+ if (!is_msgend(i2c)) {
+ byte = i2c->msg->buf[i2c->msg_ptr++];
+ writel(byte, i2c->regs + MA35_DAT);
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI);
+ } else if (!is_last_msg(i2c)) {
+ dev_dbg(i2c->dev, "WRITE: Next Message\n");
+
+ i2c->msg_ptr = 0;
+ i2c->msg_idx++;
+ i2c->msg++;
+
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_STA | MA35_CTL_SI);
+ } else {
+ ma35d1_i2c_stop(i2c, 0);
+ }
+ break;
+
+ case MA35_M_TRAN_DATA_NACK:
+ ma35d1_i2c_stop(i2c, -EIO);
+ break;
+
+ case MA35_M_TRAN_ADDR_NACK:
+ case MA35_M_RECE_ADDR_NACK:
+ /* Controller Transmit Address NACK */
+ /* 0x20: SLA+W has been transmitted and NACK has been received */
+ /* 0x48: SLA+R has been transmitted and NACK has been received */
+ dev_dbg(i2c->dev, "\n i2c: ack was not received\n");
+ ma35d1_i2c_stop(i2c, -ENXIO);
+ break;
+
+ case MA35_M_RECE_ADDR_ACK:
+ if (is_msgend(i2c)) {
+ if (is_last_msg(i2c)) {
+ ma35d1_i2c_stop(i2c, 0);
+ } else {
+ dev_dbg(i2c->dev, "READ: Next Transfer\n");
+
+ i2c->msg_ptr = 0;
+ i2c->msg_idx++;
+ i2c->msg++;
+
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_STA_SI);
+ }
+ } else if (i2c->msg->len == 1) {
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI);
+ } else {
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI_AA);
+ }
+ break;
+
+ case MA35_M_RECE_DATA_ACK:
+ case MA35_M_RECE_DATA_NACK:
+ /* DATA has been received and ACK/NACK has been received */
+ byte = readb(i2c->regs + MA35_DAT);
+ i2c->msg->buf[i2c->msg_ptr++] = byte;
+
+ if (is_last_byte(i2c)) {
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI);
+ } else if (is_msgend(i2c)) {
+ if (is_last_msg(i2c)) {
+ dev_dbg(i2c->dev, "READ: Send Stop\n");
+
+ ma35d1_i2c_stop(i2c, 0);
+ } else {
+ dev_dbg(i2c->dev, "READ: Next Transfer\n");
+
+ i2c->msg_ptr = 0;
+ i2c->msg_idx++;
+ i2c->msg++;
+
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_STA_SI);
+ }
+ } else {
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI_AA);
+ }
+ break;
+
+ case MA35_M_ARB_LOST:
+ /* Arbitration Lost */
+ i2c->err = -EAGAIN;
+ ma35d1_i2c_controller_complete(i2c);
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI);
+ break;
+
+ default:
+ dev_err(i2c->dev, "Status 0x%02lx is NOT processed\n",
+ i2c_status);
+ ma35d1_i2c_stop(i2c, -EIO);
+ break;
+ }
+}
+
+static irqreturn_t ma35d1_i2c_irq(int irqno, void *dev_id)
+{
+ struct ma35d1_i2c *i2c = dev_id;
+ unsigned long status;
+
+ status = readl(i2c->regs + MA35_STATUS0);
+
+ if (status == MA35_BUS_ERROR) {
+ dev_err(i2c->dev, "Bus error during transfer\n");
+ ma35d1_i2c_stop(i2c, -EIO);
+ goto out;
+ }
+
+ if (ma35d1_is_controller_status(status))
+ ma35d1_i2c_irq_controller_trx(i2c, status);
+ else
+ ma35d1_i2c_irq_target_trx(i2c, status);
+
+out:
+ return IRQ_HANDLED;
+}
+
+static int ma35d1_i2c_doxfer(struct ma35d1_i2c *i2c, struct i2c_msg *msgs,
+ int num)
+{
+ unsigned long timeout;
+ unsigned int val;
+ int ret, err;
+
+ i2c->msg = msgs;
+ i2c->msg_num = num;
+ i2c->msg_ptr = 0;
+ i2c->msg_idx = 0;
+ i2c->err = 0;
+
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_STA_SI);
+
+ timeout = wait_event_timeout(i2c->wait, i2c->msg_num == 0, HZ * 5);
+ ret = i2c->msg_idx;
+
+ if (timeout == 0) {
+ dev_dbg(i2c->dev, "xfer timeout\n");
+ i2c->msg = NULL;
+ ret = -ETIMEDOUT;
+ goto reset;
+ }
+
+ err = readl_poll_timeout(i2c->regs + MA35_CTL0, val,
+ !(val & MA35_CTL_STO), 100,
+ STOP_TIMEOUT_MS * 1000);
+ if (err) {
+ dev_err(i2c->dev, "bus idle timeout\n");
+ ret = -EBUSY;
+ goto reset;
+ } else if (i2c->err) {
+ dev_dbg(i2c->dev, "xfer error %d\n", i2c->err);
+ ret = i2c->err;
+ }
+ return ret;
+
+reset:
+ ma35d1_i2c_reset(i2c);
+ return ret;
+}
+
+static int ma35d1_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg *msgs,
+ int num)
+{
+ struct ma35d1_i2c *i2c = i2c_get_adapdata(adap);
+ int ret;
+
+ ret = pm_runtime_resume_and_get(i2c->dev);
+ if (ret)
+ return ret;
+
+ ret = ma35d1_i2c_doxfer(i2c, msgs, num);
+
+ if (i2c->target)
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI_AA);
+ pm_runtime_put_autosuspend(i2c->dev);
+
+ return ret;
+}
+
+static int ma35d1_reg_target(struct i2c_client *target)
+{
+ struct ma35d1_i2c *i2c = i2c_get_adapdata(target->adapter);
+ unsigned int slvaddr;
+ int ret;
+
+ if (i2c->target)
+ return -EBUSY;
+
+ if (target->flags & I2C_CLIENT_TEN)
+ return -EAFNOSUPPORT;
+
+ /* Keep device active for target mode listening. */
+ ret = pm_runtime_resume_and_get(i2c->dev);
+ if (ret) {
+ dev_err(i2c->dev, "failed to resume i2c controller\n");
+ return ret;
+ }
+
+ i2c->target = target;
+
+ slvaddr = target->addr << 1;
+ writel(slvaddr, i2c->regs + MA35_ADDR0);
+
+ /* I2C enter SLV mode */
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI_AA);
+
+ return 0;
+}
+
+static int ma35d1_unreg_target(struct i2c_client *target)
+{
+ struct ma35d1_i2c *i2c = i2c_get_adapdata(target->adapter);
+ int ret;
+
+ if (!i2c->target)
+ return -EINVAL;
+
+ writel(0, i2c->regs + MA35_ADDR0);
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI);
+ i2c->target = NULL;
+
+ ret = pm_runtime_put_sync(i2c->dev);
+ if (ret < 0)
+ dev_err(i2c->dev, "failed to suspend i2c controller");
+
+ return 0;
+}
+
+/* Declare Our I2C Functionality */
+static u32 ma35d1_i2c_func(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL;
+}
+
+/* I2C Bus Registration Info */
+static const struct i2c_algorithm ma35d1_i2c_algorithm = {
+ .xfer = ma35d1_i2c_xfer,
+ .functionality = ma35d1_i2c_func,
+ .reg_target = ma35d1_reg_target,
+ .unreg_target = ma35d1_unreg_target,
+};
+
+static const struct i2c_adapter_quirks ma35d1_i2c_quirks = {
+ .flags = I2C_AQ_NO_ZERO_LEN_READ,
+};
+
+static int ma35d1_i2c_suspend(struct device *dev)
+{
+ struct ma35d1_i2c *i2c = dev_get_drvdata(dev);
+ unsigned int val;
+
+ /* Prepare for wake-up from I2C events if target mode is active */
+ if (i2c->target) {
+ ma35d1_i2c_write_ctl(i2c, MA35_CTL_SI_AA);
+
+ /* Setup wake-up control */
+ writel(0x1, i2c->regs + MA35_WKCTL);
+
+ /* Clear pending wake-up flags */
+ val = readl(i2c->regs + MA35_WKSTS);
+ writel(val, i2c->regs + MA35_WKSTS);
+
+ enable_irq_wake(i2c->irq);
+
+ return 0;
+ }
+ return pm_runtime_force_suspend(dev);
+}
+
+static int ma35d1_i2c_resume(struct device *dev)
+{
+ struct ma35d1_i2c *i2c = dev_get_drvdata(dev);
+ unsigned int val;
+
+ if (i2c->target) {
+ /* Disable wake-up */
+ writel(0x0, i2c->regs + MA35_WKCTL);
+
+ /* Clear pending wake-up flags */
+ val = readl(i2c->regs + MA35_WKSTS);
+ writel(val, i2c->regs + MA35_WKSTS);
+
+ disable_irq_wake(i2c->irq);
+ return 0;
+ }
+ return pm_runtime_force_resume(dev);
+}
+
+static int ma35d1_i2c_runtime_suspend(struct device *dev)
+{
+ struct ma35d1_i2c *i2c = dev_get_drvdata(dev);
+ unsigned int val;
+
+ /* Disable I2C controller */
+ val = readl(i2c->regs + MA35_CTL0);
+ val &= ~(MA35_CTL_I2CEN | MA35_CTL_INTEN);
+ writel(val, i2c->regs + MA35_CTL0);
+
+ clk_disable(i2c->clk);
+
+ return 0;
+}
+
+static int ma35d1_i2c_runtime_resume(struct device *dev)
+{
+ struct ma35d1_i2c *i2c = dev_get_drvdata(dev);
+ unsigned int val;
+ int ret;
+
+ ret = clk_enable(i2c->clk);
+ if (ret) {
+ dev_err(dev, "failed to enable clock in resume\n");
+ return ret;
+ }
+
+ /* Enable I2C controller */
+ val = readl(i2c->regs + MA35_CTL0);
+ val |= MA35_CTL_I2CEN | MA35_CTL_INTEN;
+ writel(val, i2c->regs + MA35_CTL0);
+
+ return 0;
+}
+
+static void ma35d1_i2c_pm_cleanup(void *data)
+{
+ struct device *dev = data;
+
+ pm_runtime_dont_use_autosuspend(dev);
+
+ pm_runtime_disable(dev);
+
+ /*
+ * prevents usage count inconsistencies if the user
+ * has manually forced the device active via sysfs.
+ */
+ if (!pm_runtime_status_suspended(dev))
+ ma35d1_i2c_runtime_suspend(dev);
+}
+
+
+static int ma35d1_i2c_probe(struct platform_device *pdev)
+{
+ struct ma35d1_i2c *i2c;
+ struct resource *res;
+ int ret, clkdiv;
+ u32 val;
+ unsigned int busfreq;
+ struct device *dev = &pdev->dev;
+
+ i2c = devm_kzalloc(dev, sizeof(*i2c), GFP_KERNEL);
+ if (!i2c)
+ return -ENOMEM;
+
+ init_waitqueue_head(&i2c->wait);
+ spin_lock_init(&i2c->lock);
+
+ i2c->dev = dev;
+
+ i2c->clk = devm_clk_get_prepared(dev, NULL);
+ if (IS_ERR(i2c->clk))
+ return dev_err_probe(dev, PTR_ERR(i2c->clk),
+ "failed to get core clk\n");
+
+ i2c->regs = devm_platform_get_and_ioremap_resource(pdev, 0, &res);
+ if (IS_ERR(i2c->regs))
+ return PTR_ERR(i2c->regs);
+
+ i2c->rst = devm_reset_control_get_exclusive(&pdev->dev, NULL);
+ if (IS_ERR(i2c->rst))
+ return dev_err_probe(dev, PTR_ERR(i2c->rst),
+ "failed to get reset control\n");
+
+ /* Setup info block for the I2C core */
+ strscpy(i2c->adap.name, "ma35d1-i2c", sizeof(i2c->adap.name));
+ i2c->adap.owner = THIS_MODULE;
+ i2c->adap.algo = &ma35d1_i2c_algorithm;
+ i2c->adap.quirks = &ma35d1_i2c_quirks;
+ i2c->adap.retries = 2;
+ i2c->adap.algo_data = i2c;
+ i2c->adap.dev.parent = &pdev->dev;
+ i2c->adap.dev.of_node = pdev->dev.of_node;
+ i2c_set_adapdata(&i2c->adap, i2c);
+
+ busfreq = 100000;
+ /* Default to 100kHz if not specified in DT */
+ if (!device_property_read_u32(dev, "clock-frequency", &val)) {
+ if (val != 0 && val <= MEGA)
+ busfreq = val;
+ }
+ /* Calculate divider based on the current peripheral clock rate */
+ clkdiv = DIV_ROUND_CLOSEST(clk_get_rate(i2c->clk), busfreq * 4) - 1;
+ if (clkdiv < 0 || clkdiv > 0xffff)
+ return dev_err_probe(dev, -EINVAL, "invalid clkdiv value: %d\n",
+ clkdiv);
+
+ i2c->irq = platform_get_irq(pdev, 0);
+ if (i2c->irq < 0)
+ return dev_err_probe(dev, i2c->irq, "failed to get irq\n");
+
+ platform_set_drvdata(pdev, i2c);
+
+ pm_runtime_set_autosuspend_delay(dev, I2C_PM_TIMEOUT_MS);
+ pm_runtime_use_autosuspend(dev);
+ pm_runtime_enable(dev);
+
+ ret = devm_add_action_or_reset(dev, ma35d1_i2c_pm_cleanup, dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to register PM cleanup\n");
+
+ ret = pm_runtime_resume_and_get(dev);
+ if (ret)
+ return dev_err_probe(dev, ret, "failed to resume device\n");
+
+ writel(FIELD_PREP(MA35_CLKDIV_MSK, clkdiv), i2c->regs + MA35_CLKDIV);
+
+ ret = devm_request_irq(dev, i2c->irq, ma35d1_i2c_irq, 0, dev_name(dev),
+ i2c);
+ if (ret) {
+ dev_err_probe(dev, ret, "cannot claim IRQ %d\n", i2c->irq);
+ goto rpm_put;
+ }
+
+ ret = devm_i2c_add_adapter(dev, &i2c->adap);
+ if (ret) {
+ dev_err_probe(dev, ret, "failed to add bus to i2c core\n");
+ goto rpm_put;
+ }
+
+ pm_runtime_put_autosuspend(dev);
+
+ dev_info(&i2c->adap.dev, "%pa MA35D1 I2C adapter registered\n",
+ &res->start);
+ return 0;
+
+rpm_put:
+ pm_runtime_put_sync(dev);
+ return ret;
+}
+
+static const struct dev_pm_ops ma35d1_i2c_pmops = {
+ SYSTEM_SLEEP_PM_OPS(ma35d1_i2c_suspend, ma35d1_i2c_resume)
+ RUNTIME_PM_OPS(ma35d1_i2c_runtime_suspend,
+ ma35d1_i2c_runtime_resume, NULL)
+};
+
+static const struct of_device_id ma35d1_i2c_of_match[] = {
+ { .compatible = "nuvoton,ma35d1-i2c" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ma35d1_i2c_of_match);
+
+static struct platform_driver ma35d1_i2c_driver = {
+ .probe = ma35d1_i2c_probe,
+ .driver = {
+ .name = "ma35d1-i2c",
+ .of_match_table = ma35d1_i2c_of_match,
+ .pm = pm_ptr(&ma35d1_i2c_pmops),
+ },
+};
+module_platform_driver(ma35d1_i2c_driver);
+
+MODULE_AUTHOR("Zi-Yu Chen <zychennvt@gmail.com>");
+MODULE_DESCRIPTION("MA35D1 I2C Bus Driver");
+MODULE_LICENSE("GPL");
--
2.34.1
^ permalink raw reply related
* [PATCH v6 1/3] dt-bindings: i2c: nuvoton,ma35d1-i2c: Add MA35D1 I2C controller
From: Zi-Yu Chen @ 2026-07-20 3:06 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-arm-kernel, linux-kernel
Cc: Jacky Huang, Shan-Chun Hung, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Zi-Yu Chen, Krzysztof Kozlowski
In-Reply-To: <20260720030616.1067660-1-zychennvt@gmail.com>
Add device tree binding documentation for the I2C controller
found in the Nuvoton MA35D1 SoC.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Zi-Yu Chen <zychennvt@gmail.com>
---
.../bindings/i2c/nuvoton,ma35d1-i2c.yaml | 63 +++++++++++++++++++
1 file changed, 63 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,ma35d1-i2c.yaml
diff --git a/Documentation/devicetree/bindings/i2c/nuvoton,ma35d1-i2c.yaml b/Documentation/devicetree/bindings/i2c/nuvoton,ma35d1-i2c.yaml
new file mode 100644
index 000000000000..0713e1c1ecb8
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/nuvoton,ma35d1-i2c.yaml
@@ -0,0 +1,63 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i2c/nuvoton,ma35d1-i2c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Nuvoton MA35D1 I2C Controller
+
+maintainers:
+ - Zi-Yu Chen <zychennvt@gmail.com>
+
+description:
+ The Nuvoton MA35D1 I2C controller supports controller and optional target mode.
+
+allOf:
+ - $ref: /schemas/i2c/i2c-controller.yaml#
+
+properties:
+ compatible:
+ const: nuvoton,ma35d1-i2c
+
+ reg:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ clocks:
+ maxItems: 1
+
+ clock-frequency:
+ description:
+ Desired I2C bus clock frequency in Hz. The absence of this property
+ indicates the default frequency 100 kHz.
+
+ resets:
+ maxItems: 1
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - resets
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ #include <dt-bindings/clock/nuvoton,ma35d1-clk.h>
+ #include <dt-bindings/reset/nuvoton,ma35d1-reset.h>
+
+ i2c0: i2c@40800000 {
+ compatible = "nuvoton,ma35d1-i2c";
+ reg = <0x40800000 0x1000>;
+ interrupts = <GIC_SPI 13 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&clk I2C0_GATE>;
+ clock-frequency = <100000>;
+ resets = <&sys MA35D1_RESET_I2C0>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
--
2.34.1
^ permalink raw reply related
* [PATCH v6 0/3] i2c: ma35d1: Add support for MA35D1 I2C controller
From: Zi-Yu Chen @ 2026-07-20 3:06 UTC (permalink / raw)
To: Andi Shyti, linux-i2c, devicetree, linux-arm-kernel, linux-kernel
Cc: Jacky Huang, Shan-Chun Hung, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Philipp Zabel, Zi-Yu Chen
This series adds support for the I2C controller found in the Nuvoton
MA35D1 SoC. The driver supports controller and optional target mode
and runtime power management.
The implementation has been tested on the Nuvoton MA35D1 SOM board.
Changes in v6:
- Patch 2 (driver):
- Removed dynamic IRQ toggling helpers for simpler interrupt management.
Based on Sashiko AI Feedback:
- Added zero-length read quirk.
- Set i2c->msg to NULL on timeout to avoid use-after-free.
- Cleared hardware registers on target unregistration to release the bus.
- Protected suspend-path register writes with a spinlock.
- Validated clock-frequency to prevent division-by-zero.
- Removed incorrect I2C_M_IGNORE_NAK handling.
- Corrected a comment typo in the receive data path.
- Patch 3 (dts):
- use a generic functional name audio-codec@1a
v5: https://lore.kernel.org/r/20260630012106.3616821-1-zychennvt@gmail.com/
Changes in v5:
- Patch 2 (driver):
- Add spinlock protection for MA35_CTL0 register access.
- Add NULL pointer check for i2c->msg in the ISR.
- Use devres-managed cleanup (devm_add_action_or_reset) for runtime PM.
- Use pm_runtime_force_suspend/resume() for Controller mode
system sleep.
- Control I2CEN bit enabling/disabling via runtime PM.
- Refactor arbitration lost handling in state machines.
- Remove unsupported I2C_FUNC_PROTOCOL_MANGLING flag.
- Clean up readl_poll_timeout() parameters, retry loop, and
unused macros.
v4: https://lore.kernel.org/r/20260610030208.2020275-1-zychennvt@gmail.com/
Changes in v4:
- Patch 1 (dt-bindings):
- Update example interrupt number.
- Patch 2 (driver):
- Remove redundant spinlocks and fix 'irq' type to signed.
- Fix target address recovery in ma35d1_i2c_reset().
- Refactor IRQ handler for multi-msg reads and NACK/IGNORE_NAK.
- Fix timeout UAF via explicit IRQ disabling during recovery.
- Drop IRQF_SHARED and fix PM paths in probe/unregistration.
- Limit suspend/resume wakeup logic to target mode only.
- Patch 3 (dts):
- Add missing i2c0 node to dtsi.
- Switch SoM configuration from i2c1 to i2c2 with updated pinctrl.
- Add nau8822 audio codec node under i2c2 on the SoM board.
- Drop redundant i2c0 alias and clean up dtsi property ordering.
v3: https://lore.kernel.org/r/20260512073953.564323-1-zychennvt@gmail.com/
Changes in v3:
- Fix minor DTS formatting issues (whitespace, missing newline)
v2: https://lore.kernel.org/r/20260316063726.41048-1-zychennvt@gmail.com
Changes in v2:
- Overall:
- Rebase on linux-i2c/i2c-next
- Switched terminology from "master/slave" to "controller/target".
- Patch 1 (dt-bindings):
- Simplified description and fixed 'reg' size in example.
- Patch 2 (driver):
- Modernized using devm_*, generic device properties, and FIELD_PREP/GENMASK.
- Optimized power management by moving clock control to runtime PM.
- Simplified code by removing redundant .remove(), .owner, and inlines.
- Added dev_err_probe() and default bus frequency handling.
- Patch 3 (dts):
- Moved i2c aliases to board dts and reordered nodes alphabetically.
v1: https://lore.kernel.org/r/20260302020822.13936-1-zychennvt@gmail.com
Zi-Yu Chen (3):
dt-bindings: i2c: nuvoton,ma35d1-i2c: Add MA35D1 I2C controller
i2c: ma35d1: Add Nuvoton MA35D1 I2C driver support
arm64: dts: nuvoton: Add I2C nodes for MA35D1 SoC
.../bindings/i2c/nuvoton,ma35d1-i2c.yaml | 63 ++
.../boot/dts/nuvoton/ma35d1-som-256m.dts | 20 +
arch/arm64/boot/dts/nuvoton/ma35d1.dtsi | 72 ++
drivers/i2c/busses/Kconfig | 13 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-ma35d1.c | 780 ++++++++++++++++++
6 files changed, 949 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/nuvoton,ma35d1-i2c.yaml
create mode 100644 drivers/i2c/busses/i2c-ma35d1.c
--
2.34.1
^ permalink raw reply
* Re: [PATCH v4 1/2] dt-bindings: qcom,snps-dwc3: Add property indicating presence of eUSB2 phy
From: Peter Chen @ 2026-07-20 3:03 UTC (permalink / raw)
To: Thinh Nguyen
Cc: Krishna Kurapati, Konrad Dybcio, Krzysztof Kozlowski,
Dmitry Baryshkov, Greg Kroah-Hartman, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Wesley Cheng,
linux-arm-msm@vger.kernel.org, linux-usb@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <alqZQL_TrQ7Da4Zw@vbox>
On 26-07-17 21:34:37, Thinh Nguyen wrote:
> >
> > Konrad did suggest this on v3:
> >
> > https://urldefense.com/v3/__https://lore.kernel.org/all/3de365a0-4632-42ea-8a8a-5a4765945a76@oss.qualcomm.com/__;!!A4F2R9G_pg!bFfHVkKIznULZPI2qLzahGobT6OIz4j3-XA_n_3hHnRuvzps_XuAGJdphzxkK4bk6twEah-Zn6xRS1uCQCjZ1ud3HnRftKhw2SXfqA$
> >
> > This involves cleaning all drivers using these enums if we take that route.
>
> We should not be using phy_attrs.mode. It's a runtime operation mode.
>
> Instead, we can introduce a new phy attribute phy_attrs.type and
> phy_get_type(). The phy driver can set this at probe and the dwc3-qcom.c
> can query it from the phy phandle.
>
> We can define PHY_TYPE_EUSB2 along with the existing types in
> include/dt-bindings/phy/phy.h for this attribute.
>
> BR,
> Thinh
Good suggestion, Thinh.
Krishna, would you try to do that?
--
Thanks,
Peter Chen
^ permalink raw reply
* Re: [PATCH 4/5] PCI: dwc: rcar-gen4: Handle PERST via reset subsystem
From: Marek Vasut @ 2026-07-20 1:16 UTC (permalink / raw)
To: Manivannan Sadhasivam, Marek Vasut
Cc: linux-pci, Krzysztof Wilczyński, Bjorn Helgaas, Conor Dooley,
Geert Uytterhoeven, Krzysztof Kozlowski, Lorenzo Pieralisi,
Rob Herring, Yoshihiro Shimoda, devicetree, linux-kernel,
linux-renesas-soc
In-Reply-To: <ceimnmpsyfmrkuvrrsgkjg3fk2yf7fj7z5ie3cj6vfyighatag@hyb3vr2nclpm>
On 7/15/26 4:36 PM, Manivannan Sadhasivam wrote:
Hello Manivannan,
[...]
>> @@ -671,6 +705,8 @@ static void rcar_gen4_pcie_remove(struct platform_device *pdev)
>>
>> rcar_gen4_remove_dw_pcie(rcar);
>> rcar_gen4_pcie_unprepare(rcar);
>> + if (rcar->perst)
>> + reset_control_put(rcar->perst);
>
> I don't see reset_control_put() in any error paths of probe().
I will address all the items in V2, thanks !
^ permalink raw reply
* Re: [PATCH 4/5] PCI: dwc: rcar-gen4: Handle PERST via reset subsystem
From: Marek Vasut @ 2026-07-20 1:15 UTC (permalink / raw)
To: Geert Uytterhoeven, Marek Vasut
Cc: linux-pci, Krzysztof Wilczyński, Bjorn Helgaas, Conor Dooley,
Geert Uytterhoeven, Krzysztof Kozlowski, Lorenzo Pieralisi,
Manivannan Sadhasivam, Rob Herring, Yoshihiro Shimoda, devicetree,
linux-kernel, linux-renesas-soc
In-Reply-To: <CAMuHMdUVqw-gqydVg5N7WiMj04rVAPuLjJjzgrk+eEgJpbzS2g@mail.gmail.com>
On 7/14/26 5:07 PM, Geert Uytterhoeven wrote:
Hello Geert,
>> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
>
>> @@ -299,10 +300,27 @@ static void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
>>
>> static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar)
>> {
>> + struct device *dev = rcar->dw.dev;
>> + struct reset_control *perst;
>> +
>> rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy");
>> if (IS_ERR(rcar->phy_base))
>> return PTR_ERR(rcar->phy_base);
>>
>> + rcar->perst = NULL;
>> + for_each_available_child_of_node_scoped(dev->of_node, of_port) {
>> + perst = of_reset_control_get(of_port, "perst");
>> + if (IS_ERR(perst)) {
>> + if (PTR_ERR(perst) != -EPROBE_DEFER)
>> + dev_err(dev, "Failed to get PERST#\n");
>> + return PTR_ERR(perst);
>
> return dev_err_probe(...)?
>
> This error condition is triggered on Sparrow Hawk and White Hawk,
> as their DTS does not have perst:
>
> pcie-rcar-gen4 e65d0000.pcie: Failed to get PERST#
> pcie-rcar-gen4 e65d0000.pcie: probe with driver pcie-rcar-gen4
> failed with error -2
I switched to of_reset_control_get_optional_exclusive() and cleaned the
reset handling further in V2.
^ permalink raw reply
* Re: [PATCH 2/5] PCI: dwc: rcar-gen4: Return error code from .additional_common_init
From: Marek Vasut @ 2026-07-20 1:14 UTC (permalink / raw)
To: Manivannan Sadhasivam, Marek Vasut
Cc: linux-pci, Krzysztof Wilczyński, Bjorn Helgaas, Conor Dooley,
Geert Uytterhoeven, Krzysztof Kozlowski, Lorenzo Pieralisi,
Rob Herring, Yoshihiro Shimoda, devicetree, linux-kernel,
linux-renesas-soc
In-Reply-To: <k4huw3bsimjq5kyjweuwzksu5lc34rk5acbvxwhrt67tdwbom6@gx3lho54h6i6>
On 7/15/26 4:22 PM, Manivannan Sadhasivam wrote:
> On Sat, Jul 04, 2026 at 10:25:01PM +0200, Marek Vasut wrote:
>> Return error code from .additional_common_init callback and check
>> the error code. This is a preparatory patch for R-Car Gen5 support,
>> which may return non-zero error code from .additional_common_init.
>>
>> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
>> ---
>> Cc: "Krzysztof Wilczyński" <kwilczynski@kernel.org>
>> Cc: Bjorn Helgaas <bhelgaas@google.com>
>> Cc: Conor Dooley <conor+dt@kernel.org>
>> Cc: Geert Uytterhoeven <geert+renesas@glider.be>
>> Cc: Krzysztof Kozlowski <krzk+dt@kernel.org>
>> Cc: Lorenzo Pieralisi <lpieralisi@kernel.org>
>> Cc: Manivannan Sadhasivam <mani@kernel.org>
>> Cc: Rob Herring <robh@kernel.org>
>> Cc: Yoshihiro Shimoda <yoshihiro.shimoda.uh@renesas.com>
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-kernel@vger.kernel.org
>> Cc: linux-pci@vger.kernel.org
>> Cc: linux-renesas-soc@vger.kernel.org
>> ---
>> drivers/pci/controller/dwc/pcie-rcar-gen4.c | 15 +++++++++++----
>> 1 file changed, 11 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
>> index 5f7211b91ee5b..7625cd338262d 100644
>> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
>> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
>> @@ -87,7 +87,7 @@ MODULE_FIRMWARE(RCAR_GEN4_PCIE_FIRMWARE_NAME);
>>
>> struct rcar_gen4_pcie;
>> struct rcar_gen4_pcie_drvdata {
>> - void (*additional_common_init)(struct rcar_gen4_pcie *rcar);
>> + int (*additional_common_init)(struct rcar_gen4_pcie *rcar);
>
> Can you also rename this callback as init() and implementations as
> rcar_gen4_pcie_{ep}_init()?
>
> Though this callback is a grab-bag of various register settings, naming it as
> just '.init()' will make it more readable and less redundant.
I did a bit more refactoring of this in V2.
^ permalink raw reply
* Re: [PATCH v2] scsi: ufs: dt-bindings: Add missing mcq reg for qcom,sa8255p-ufshc
From: sashiko-bot @ 2026-07-20 2:44 UTC (permalink / raw)
To: Shawn Guo; +Cc: linux-scsi, devicetree, robh, conor+dt
In-Reply-To: <20260720023552.2667237-1-shengchao.guo@oss.qualcomm.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The Device Tree schema update fails to enforce the presence of the newly documented `mcq` register space and `reg-names` property.
--
--- Patch [1]: [PATCH v2] scsi: ufs: dt-bindings: Add missing mcq reg for qcom,sa8255p-ufshc ---
commit 871a91874eb00f9814a93dfb44bc94d0b96e8791
Author: Shawn Guo <shengchao.guo@oss.qualcomm.com>
scsi: ufs: dt-bindings: Add missing mcq reg for qcom,sa8255p-ufshc
Qualcomm UFS controller found on SoCs SA8255P/SA8797P has a MCQ I/O
address space. It should be defined in the bindings even though Linux
driver currently doesn't utilize it. Fix the binding before it gets
adopted by DTS.
> diff --git a/Documentation/devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml b/Documentation/devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml
> index db165a235cb6d..66d73e9025d6b 100644
> --- a/Documentation/devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml
> +++ b/Documentation/devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml
> @@ -18,7 +18,14 @@ properties:
> - const: qcom,sa8255p-ufshc
>
> reg:
> - maxItems: 1
> + minItems: 1
> + maxItems: 2
> +
> + reg-names:
> + minItems: 1
> + items:
> + - const: std
> + - const: mcq
[Severity: Medium]
Since the commit message indicates the MCQ I/O address space should be defined
before it gets adopted by DTS, should reg and reg-names strictly require two
items?
Setting minItems to 1 for both properties allows device trees to pass
schema validation with only a single register space.
Additionally, should reg-names be added to the required list to enforce its
presence?
The examples section was also not updated and still demonstrates a single
register space:
compatible = "qcom,sa8255p-ufshc";
reg = <0x01d84000 0x3000>;
interrupts = <GIC_SPI 265 IRQ_TYPE_LEVEL_HIGH>;
Could this be updated to demonstrate the newly added mcq register and
reg-names?
>
> interrupts:
> maxItems: 1
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720023552.2667237-1-shengchao.guo@oss.qualcomm.com?part=1
^ permalink raw reply
* [PATCH v2] scsi: ufs: dt-bindings: Add missing mcq reg for qcom,sa8255p-ufshc
From: Shawn Guo @ 2026-07-20 2:35 UTC (permalink / raw)
To: Martin K . Petersen
Cc: Manivannan Sadhasivam, Alim Akhtar, Avri Altman, Bart Van Assche,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Kumar Dwivedi,
Anjana Hari, linux-arm-msm, linux-scsi, devicetree, linux-kernel,
Shawn Guo, stable
Qualcomm UFS controller found on SoCs SA8255P/SA8797P has a MCQ I/O
address space. It should be defined in the bindings even though Linux
driver currently doesn't utilize it. Fix the binding before it gets
adopted by DTS.
Fixes: e2725ed2a7fb ("scsi: ufs: dt-bindings: Document bindings for SA8255P UFS Host Controller")
Cc: stable@vger.kernel.org
Signed-off-by: Shawn Guo <shengchao.guo@oss.qualcomm.com>
---
Changes for v2:
- Recover the newline which got dropped by accident
- Link to v1: https://lore.kernel.org/r/20260716083313.2367249-1-shengchao.guo@oss.qualcomm.com
.../devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml b/Documentation/devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml
index db165a235cb6..66d73e9025d6 100644
--- a/Documentation/devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml
+++ b/Documentation/devicetree/bindings/ufs/qcom,sa8255p-ufshc.yaml
@@ -18,7 +18,14 @@ properties:
- const: qcom,sa8255p-ufshc
reg:
- maxItems: 1
+ minItems: 1
+ maxItems: 2
+
+ reg-names:
+ minItems: 1
+ items:
+ - const: std
+ - const: mcq
interrupts:
maxItems: 1
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v10 3/3] thermal/drivers/imx: Add calibration offset support
From: sashiko-bot @ 2026-07-20 2:30 UTC (permalink / raw)
To: Haoning CHENG; +Cc: imx, devicetree, conor+dt, robh, Frank.Li
In-Reply-To: <20260720-b4-symana21-11221-imx-thermal-support-upstream-6-18-v10-3-f8c3dcd9a57e@cn.bosch.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [Critical] Alarm IRQ can fire after thermal zone is unregistered, causing a Use-After-Free.
--
--- Patch [3]: [PATCH v10 3/3] thermal/drivers/imx: Add calibration offset support ---
commit bb0e3bef4010d2e2be1d5402d29e194aaf49548f
Author: Haoning CHENG <Haoning.CHENG@cn.bosch.com>
thermal/drivers/imx: Add calibration offset support
The TEMPMON sensor reading may deviate from the theoretical SoC junction
temperature. This commit adds support for an optional DT property to apply a
calibration offset to the temperature readings. The offset is used when setting
alarm/panic thresholds and reading the current temperature.
> diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
> index 168069a16e930..87ae2e40cc6fe 100644
> --- a/drivers/thermal/imx_thermal.c
> +++ b/drivers/thermal/imx_thermal.c
[ ... ]
> @@ -635,6 +643,25 @@ static int imx_thermal_probe(struct platform_device *pdev)
>
> platform_set_drvdata(pdev, data);
>
> + if (of_property_present(dev->of_node,
> + "fsl,temp-calibration-offset-millicelsius")) {
> + ret = of_property_read_s32(dev->of_node,
> + "fsl,temp-calibration-offset-millicelsius",
> + &data->calibration_offset);
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "failed to read calibration offset\n");
> +
> + if (data->calibration_offset < IMX_TEMP_CALIB_OFFSET_MIN ||
> + data->calibration_offset > IMX_TEMP_CALIB_OFFSET_MAX)
> + return dev_err_probe(dev, -EINVAL,
> + "calibration offset %d millicelsius out of range\n",
> + data->calibration_offset);
> +
> + dev_dbg(dev, "calibration offset: %d millicelsius\n",
> + data->calibration_offset);
> + }
> +
> if (of_property_present(dev->of_node, "nvmem-cells")) {
> ret = imx_init_from_nvmem_cells(pdev);
> if (ret)
[Severity: Critical]
This isn't a bug introduced by this patch, but while reviewing the probe
routine, I noticed a use-after-free issue related to how the alarm IRQ is
managed.
Can an interrupt fire after the thermal zone has been unregistered?
The alarm IRQ is requested with devres in probe:
drivers/thermal/imx_thermal.c:imx_thermal_probe() {
...
ret = devm_request_threaded_irq(dev, data->irq,
imx_thermal_alarm_irq, imx_thermal_alarm_irq_thread,
0, "imx_thermal", data);
...
}
Because it is a devm-managed resource, it is not freed until after
imx_thermal_remove() completes. However, the remove path manually unregisters
and frees the thermal zone:
drivers/thermal/imx_thermal.c:imx_thermal_remove() {
...
thermal_zone_device_unregister(data->tz);
...
}
If an administrative module unload or device unbind is triggered while the
SoC is hot, could an interrupt fire during or immediately after the unregister
call? If so, would the still-active IRQ thread dereference the freed data->tz
pointer?
drivers/thermal/imx_thermal.c:imx_thermal_alarm_irq_thread() {
...
thermal_zone_device_update(data->tz, THERMAL_EVENT_UNSPECIFIED);
...
}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-b4-symana21-11221-imx-thermal-support-upstream-6-18-v10-0-f8c3dcd9a57e@cn.bosch.com?part=3
^ permalink raw reply
* Re: [PATCH 3/4] drm/rockchip: lvds: add RK3568 support
From: Chaoyi Chen @ 2026-07-20 1:54 UTC (permalink / raw)
To: Rok Markovic, Heiko Stuebner, Sandy Huang, Andy Yan,
Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: dri-devel, devicetree, linux-arm-kernel, linux-rockchip,
linux-kernel, Alibek Omarov
In-Reply-To: <20260717120005.2087386-4-rok@kanardia.eu>
Hello Rok,
Thank you for your patch. Please see the comment below:
On 7/17/2026 8:00 PM, Rok Markovic wrote:
> The RK3568 LVDS transmitter has no register block of its own. It is
> driven entirely through the GRF and re-uses the MIPI DSI0 D-PHY in
> PHY_MODE_LVDS, which phy-rockchip-inno-dsidphy already supports.
>
> Based on Alibek Omarov's earlier posting [1], with the changes below.
>
> Power the D-PHY from the encoder enable path rather than from probe.
> phy_power_on() runs the phy driver's whole LVDS bring-up: PLL and
> bandgap power-on, a settle, PLL mode select, then a reset pulse of the
> serializer and the lane enables. None of that is safe at probe time -
> the GRF has not yet switched the block to LVDS mode (LVDS0_MODE_EN is
> set from rk3568_lvds_poweron(), i.e. the enable path) and the VOP is
> not driving dclk. The serializer is clocked from dclk and latches its
> state coming out of that reset, so it comes up dead and stays dead.
> The failure is silent: every register in the phy, the GRF and the VOP
> reads back exactly as on a working system while the lanes sit at
> common mode and never toggle.
>
Could you please confirm this? Based on my earlier tests, after PHY
poweron, re-disabling and re-enabling the GRF did not reproduce the
issue you described.
> Program RK3568_LVDS0_DCLK_INV_SEL from the CRTC state's bus_flags so
> the panel's declared pixelclk-active is honoured on the LVDS block as
> well as on the VOP pin polarity. Both have to agree with the edge the
> panel samples on.
>
> Re-assert RK3568_LVDS0_P2S_EN in rk3568_lvds_poweron().
> rk3568_lvds_poweroff() clears MODE_EN and P2S_EN together, so setting
> P2S_EN once at probe would leave the parallel-to-serial converter off
> after the first disable/enable cycle.
>
> Use regmap_write() rather than regmap_update_bits() for the GRF. These
> registers are write-masked - the upper 16 bits select which of the
> lower 16 a write may change - so there is nothing to preserve and no
> reason to read first. Passing a FIELD_PREP_WM16() value as both the
> mask and the value of an update_bits() applies the masking twice and
> only works by accident.
>
> Between the two, nothing needs programming at probe at all: the GRF is
> written entirely from the enable path, so px30_lvds_probe() is left
> alone rather than being refactored into a shared phy helper.
>
> [1] https://lore.kernel.org/all/20230119184807.171132-1-a1ba.omarov@gmail.com/
>
> Co-developed-by: Alibek Omarov <a1ba.omarov@gmail.com>
> Signed-off-by: Alibek Omarov <a1ba.omarov@gmail.com>
> Signed-off-by: Rok Markovic <rok@kanardia.eu>
> Assisted-by: Claude:claude-opus-4-8
> ---
> drivers/gpu/drm/rockchip/rockchip_lvds.c | 161 +++++++++++++++++++++++
> drivers/gpu/drm/rockchip/rockchip_lvds.h | 10 ++
> 2 files changed, 171 insertions(+)
>
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.c b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> index 95fa0a9..f45d04a 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.c
> @@ -435,6 +435,133 @@ static void px30_lvds_encoder_disable(struct drm_encoder *encoder)
> drm_panel_unprepare(lvds->panel);
> }
>
> +static int rk3568_lvds_poweron(struct rockchip_lvds *lvds)
> +{
> + int ret;
> +
> + ret = clk_enable(lvds->pclk);
> + if (ret < 0) {
> + DRM_DEV_ERROR(lvds->dev, "failed to enable lvds pclk %d\n", ret);
> + return ret;
> + }
> +
> + ret = pm_runtime_get_sync(lvds->dev);
> + if (ret < 0) {
> + DRM_DEV_ERROR(lvds->dev, "failed to get pm runtime: %d\n", ret);
> + clk_disable(lvds->pclk);
> + return ret;
> + }
> +
> + /*
> + * Enable LVDS mode and the parallel-to-serial converter. These are
> + * write-masked registers, so a plain write only touches the bits named
> + * here; there is nothing to preserve and no need to read first.
> + */
I think this comment is redundant. We all know this is a common design
on Rockchip platform, right? :)
> + return regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> + RK3568_LVDS0_MODE_EN(1) |
> + RK3568_LVDS0_P2S_EN(1));
> +}
> +
> +static void rk3568_lvds_poweroff(struct rockchip_lvds *lvds)
> +{
> + regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> + RK3568_LVDS0_MODE_EN(0) | RK3568_LVDS0_P2S_EN(0));
> +
> + pm_runtime_put(lvds->dev);
> + clk_disable(lvds->pclk);
> +}
> +
> +static int rk3568_lvds_grf_config(struct drm_encoder *encoder,
> + struct drm_display_mode *mode)
> +{
> + struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> + struct rockchip_crtc_state *s =
> + to_rockchip_crtc_state(encoder->crtc->state);
> + bool negedge = !!(s->bus_flags & DRM_BUS_FLAG_PIXDATA_DRIVE_NEGEDGE);
> +
> + if (lvds->output != DISPLAY_OUTPUT_LVDS) {
> + DRM_DEV_ERROR(lvds->dev, "Unsupported display output %d\n",
> + lvds->output);
> + return -EINVAL;
> + }
> +
> + /*
> + * The LVDS block has its own dclk inversion select, separate from the
> + * VOP's pin polarity. Both have to agree with what the panel samples on.
> + */
> + regmap_write(lvds->grf, RK3568_GRF_VO_CON2,
> + RK3568_LVDS0_DCLK_INV_SEL(negedge));
> +
> + /* Set format */
> + return regmap_write(lvds->grf, RK3568_GRF_VO_CON0,
> + RK3568_LVDS0_SELECT(lvds->format) |
> + RK3568_LVDS0_MSBSEL(1));
> +}
I think rk3568_lvds_poweron() and rk3568_lvds_grf_config() can be merged
to reduce extra register operations.
> +
> +static void rk3568_lvds_encoder_enable(struct drm_encoder *encoder)
> +{
> + struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> + struct drm_display_mode *mode = &encoder->crtc->state->adjusted_mode;
> + int ret;
> +
> + drm_panel_prepare(lvds->panel);
> +
> + ret = rk3568_lvds_poweron(lvds);
> + if (ret) {
> + DRM_DEV_ERROR(lvds->dev, "failed to power on LVDS: %d\n", ret);
> + drm_panel_unprepare(lvds->panel);
> + return;
> + }
> +
> + ret = rk3568_lvds_grf_config(encoder, mode);
> + if (ret) {
> + DRM_DEV_ERROR(lvds->dev, "failed to configure LVDS: %d\n", ret);
> + drm_panel_unprepare(lvds->panel);
> + return;
> + }
> +
> + /*
> + * Only now bring the D-PHY up. phy_power_on() runs the whole
> + * inno_dsidphy_lvds_mode_enable() sequence - PLL and bandgap power-on,
> + * a settle, PLL mode select, then a reset pulse of the serializer and
> + * the lane enables. All of that has to happen with the block already
> + * switched to LVDS mode in the GRF (above) and with the VOP's dclk
> + * running, because the serializer is clocked from dclk and latches its
> + * state out of that reset.
> + *
> + * Doing it at probe instead - as this driver used to - resets and
> + * enables the serializer against a block that is not in LVDS mode yet
> + * and has no input clock. Every register then reads back correct while
> + * the lanes sit at common mode forever. Rockchip's BSP orders it this
> + * way (GRF writes, then phy_set_mode + phy_power_on).
> + */
I think this comment is redundant. These are internal details of
phy_set_mode() and don't need to be explained here. Also, the
ordering requirements are quite common. You can describe them in the
commit message.
> + ret = phy_set_mode(lvds->dphy, PHY_MODE_LVDS);
> + if (ret) {
> + DRM_DEV_ERROR(lvds->dev, "failed to set phy mode: %d\n", ret);
> + drm_panel_unprepare(lvds->panel);
> + return;
> + }
> +
> + ret = phy_power_on(lvds->dphy);
> + if (ret) {
> + DRM_DEV_ERROR(lvds->dev, "failed to power on phy: %d\n", ret);
> + drm_panel_unprepare(lvds->panel);
> + return;
> + }
> +
> + drm_panel_enable(lvds->panel);
> +}
> +
> +static void rk3568_lvds_encoder_disable(struct drm_encoder *encoder)
> +{
> + struct rockchip_lvds *lvds = encoder_to_lvds(encoder);
> +
> + drm_panel_disable(lvds->panel);
> + phy_power_off(lvds->dphy);
> + rk3568_lvds_poweroff(lvds);
> + drm_panel_unprepare(lvds->panel);
> +}
> +
> static const
> struct drm_encoder_helper_funcs rk3288_lvds_encoder_helper_funcs = {
> .enable = rk3288_lvds_encoder_enable,
> @@ -449,6 +576,13 @@ struct drm_encoder_helper_funcs px30_lvds_encoder_helper_funcs = {
> .atomic_check = rockchip_lvds_encoder_atomic_check,
> };
>
> +static const
> +struct drm_encoder_helper_funcs rk3568_lvds_encoder_helper_funcs = {
> + .enable = rk3568_lvds_encoder_enable,
> + .disable = rk3568_lvds_encoder_disable,
> + .atomic_check = rockchip_lvds_encoder_atomic_check,
> +};
> +
> static int rk3288_lvds_probe(struct platform_device *pdev,
> struct rockchip_lvds *lvds)
> {
> @@ -512,6 +646,22 @@ static int px30_lvds_probe(struct platform_device *pdev,
> return phy_power_on(lvds->dphy);
> }
>
> +static int rk3568_lvds_probe(struct platform_device *pdev,
> + struct rockchip_lvds *lvds)
> +{
> + /*
> + * Grab and init the phy, but do NOT power it on here - that is done in
> + * rk3568_lvds_encoder_enable() once the GRF is in LVDS mode and dclk is
> + * running. See the comment there. The GRF is not touched at probe
> + * either: every bit of it is programmed from the enable path.
> + */
Please see the comments above.
> + lvds->dphy = devm_phy_get(&pdev->dev, "dphy");
> + if (IS_ERR(lvds->dphy))
> + return PTR_ERR(lvds->dphy);
> +
> + return phy_init(lvds->dphy);
> +}
> +
> static const struct rockchip_lvds_soc_data rk3288_lvds_data = {
> .probe = rk3288_lvds_probe,
> .helper_funcs = &rk3288_lvds_encoder_helper_funcs,
> @@ -522,6 +672,11 @@ static const struct rockchip_lvds_soc_data px30_lvds_data = {
> .helper_funcs = &px30_lvds_encoder_helper_funcs,
> };
>
> +static const struct rockchip_lvds_soc_data rk3568_lvds_data = {
> + .probe = rk3568_lvds_probe,
> + .helper_funcs = &rk3568_lvds_encoder_helper_funcs,
> +};
> +
> static const struct of_device_id rockchip_lvds_dt_ids[] = {
> {
> .compatible = "rockchip,rk3288-lvds",
> @@ -531,6 +686,10 @@ static const struct of_device_id rockchip_lvds_dt_ids[] = {
> .compatible = "rockchip,px30-lvds",
> .data = &px30_lvds_data
> },
> + {
> + .compatible = "rockchip,rk3568-lvds",
> + .data = &rk3568_lvds_data
> + },
> {}
> };
> MODULE_DEVICE_TABLE(of, rockchip_lvds_dt_ids);
> @@ -601,6 +760,8 @@ static int rockchip_lvds_bind(struct device *dev, struct device *master,
> encoder = &lvds->encoder.encoder;
> encoder->possible_crtcs = drm_of_find_possible_crtcs(drm_dev,
> dev->of_node);
> + rockchip_drm_encoder_set_crtc_endpoint_id(&lvds->encoder,
> + dev->of_node, 0, 0);
>
> ret = drm_simple_encoder_init(drm_dev, encoder, DRM_MODE_ENCODER_LVDS);
> if (ret < 0) {
> diff --git a/drivers/gpu/drm/rockchip/rockchip_lvds.h b/drivers/gpu/drm/rockchip/rockchip_lvds.h
> index 2d92447..93d3415 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_lvds.h
> +++ b/drivers/gpu/drm/rockchip/rockchip_lvds.h
> @@ -121,4 +121,14 @@
> #define PX30_LVDS_P2S_EN(val) FIELD_PREP_WM16(BIT(6), (val))
> #define PX30_LVDS_VOP_SEL(val) FIELD_PREP_WM16(BIT(1), (val))
>
> +#define RK3568_GRF_VO_CON0 0x0360
> +#define RK3568_LVDS0_SELECT(val) FIELD_PREP_WM16(GENMASK(5, 4), (val))
> +#define RK3568_LVDS0_MSBSEL(val) FIELD_PREP_WM16(BIT(3), (val))
> +
> +#define RK3568_GRF_VO_CON2 0x0368
> +#define RK3568_LVDS0_DCLK_INV_SEL(val) FIELD_PREP_WM16(BIT(9), (val))
> +#define RK3568_LVDS0_DCLK_DIV2_SEL(val) FIELD_PREP_WM16(BIT(8), (val))
> +#define RK3568_LVDS0_MODE_EN(val) FIELD_PREP_WM16(BIT(1), (val))
> +#define RK3568_LVDS0_P2S_EN(val) FIELD_PREP_WM16(BIT(0), (val))
> +
> #endif /* _ROCKCHIP_LVDS_ */
--
Best,
Chaoyi
^ permalink raw reply
* Re: [PATCH v5 0/4] iio: flow: Sensirion SLF3S liquid flow sensor
From: Jonathan Cameron @ 2026-07-20 2:29 UTC (permalink / raw)
To: Wadim Mueller
Cc: Krzysztof Kozlowski, Rob Herring, Conor Dooley, David Lechner,
Nuno Sá, Andy Shevchenko, Maxwell Doose, linux-iio,
devicetree, linux-kernel, Marcelo Schmitt, Rodrigo Alencar
In-Reply-To: <20260714085358.20842-1-wafgo01@gmail.com>
On Tue, 14 Jul 2026 10:53:54 +0200
Wadim Mueller <wafgo01@gmail.com> wrote:
> Hi all,
>
> this series adds a driver for the Sensirion SLF3S family of I2C liquid
> flow sensors (SLF3S-0600F / -1300F / -4000B) and a new IIO_VOLUMEFLOW
> channel type.
Hi Wadim
Something seems to have gone wrong somewhere. I'm not seeing patch 4
and it didn't reach patchwork either.
Thanks,
Jonathan
>
> The series is now based on iio.git testing, which already contains
> Rodrigo Alencar's IIO_VAL_DECIMAL64_* core formatting work, so the
> external dependency listed in v4 is gone. Patch 3/4 adds the
> IIO_VAL_DECIMAL64_FEMTO format type on top of it.
>
> The compatible/fallback layout of the dt-binding is intentionally
> unchanged from v4 while the discussion on the v4 thread about the
> preferred fallback scheme is still open:
>
> https://lore.kernel.org/linux-iio/20260621145117.70b2d50e@jic23-huawei/
>
> Changes since v4
> ----------------
> * rebased onto iio.git testing; trivial conflict with the new
> IIO_COVERAGE entries in the channel-type lists resolved (1/4)
> * driver: include err.h for PTR_ERR() and math.h for struct
> s32_fract, per Andy
> * driver: fold the per-variant scale_num/scale_den pair into a
> struct s32_fract, per Andy
> * driver: add enum slf3s_variant_id and use it for the variant
> table and the I2C/OF match tables instead of bare indices, per
> Andy
> * driver: slf3s_send_cmd(): drop the "at_least 2" parameter
> annotation, decouple the ret assignment from its declaration and
> check errors first, per Andy
> * driver: use sizeof() instead of ARRAY_SIZE() for the measurement
> byte buffer, per Andy
> * driver: parenthesise the (FEMTO / MILLI) factor so it folds into
> a single constant multiplication, per Andy
> * driver: derive the temperature scale from MILLIDEGREE_PER_DEGREE
> instead of a bare 1000, per Andy
> * driver: checked the struct slf3s_data layout (re Andy's pahole
> question): the non-pointer members total 260 bytes, so one 4-byte
> hole remains for any member order; left as is
> * driver: drop the inline comment on the mutex that duplicated the
> kernel-doc, per Jonathan
> * driver: fix the duplicated argument in the DT/sensor variant
> mismatch dev_info(), per Jonathan
> * driver: reformat the i2c_device_id/of_device_id tables to one
> member per line, per Jonathan
> * MAINTAINERS: updated Maxwells address in the R: entry to
> maxwell@maxwelld.cc, as he requested on the v4 thread
>
> The signaling-flags word (air-in-line / high-flow / smoothing status) in
> each measurement frame is intentionally not read; exposing it can be a
> later follow-up.
>
> Thanks,
> Wadim
>
> Wadim Mueller (4):
> iio: types: add IIO_VOLUMEFLOW channel type
> dt-bindings: iio: flow: add Sensirion SLF3S liquid flow sensor
> iio: core: add IIO_VAL_DECIMAL64_FEMTO format type
> iio: flow: add Sensirion SLF3S liquid flow sensor driver
>
> Documentation/ABI/testing/sysfs-bus-iio | 11 +
> Documentation/ABI/testing/sysfs-bus-iio-flow | 21 +
> .../bindings/iio/flow/sensirion,slf3s.yaml | 58 ++
> MAINTAINERS | 8 +
> drivers/iio/Kconfig | 1 +
> drivers/iio/Makefile | 1 +
> drivers/iio/flow/Kconfig | 22 +
> drivers/iio/flow/Makefile | 7 +
> drivers/iio/flow/slf3s.c | 541 ++++++++++++++++++
> drivers/iio/industrialio-core.c | 3 +
> include/linux/iio/types.h | 1 +
> include/uapi/linux/iio/types.h | 1 +
> tools/iio/iio_event_monitor.c | 2 +
> 13 files changed, 677 insertions(+)
> create mode 100644 Documentation/ABI/testing/sysfs-bus-iio-flow
> create mode 100644 Documentation/devicetree/bindings/iio/flow/sensirion,slf3s.yaml
> create mode 100644 drivers/iio/flow/Kconfig
> create mode 100644 drivers/iio/flow/Makefile
> create mode 100644 drivers/iio/flow/slf3s.c
>
>
> base-commit: a50909aa46dec46de3c73235fc15a7d6f763d996
^ permalink raw reply
* Re: [PATCH v5 2/4] dt-bindings: iio: flow: add Sensirion SLF3S liquid flow sensor
From: Jonathan Cameron @ 2026-07-20 2:28 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Wadim Mueller, Krzysztof Kozlowski, Rob Herring, Conor Dooley,
David Lechner, Nuno Sá, Andy Shevchenko, Maxwell Doose,
linux-iio, devicetree, linux-kernel, Marcelo Schmitt,
Rodrigo Alencar
In-Reply-To: <b2c6cc45-5a52-4992-b72b-dfde02cbc94d@kernel.org>
On Sun, 19 Jul 2026 08:19:34 +0200
Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On 14/07/2026 10:53, Wadim Mueller wrote:
> > Document the bindings for the Sensirion SLF3S family of digital
> > liquid-flow sensors on I2C. The family currently covers the
> > SLF3S-0600F, SLF3S-1300F and SLF3S-4000B variants.
> >
> > All variants share the same register map and are fully detectable
> > from the product-information register at probe time, so
> > sensirion,slf3s-1300f serves as the fallback compatible for the
> > other variants.
> >
> > The active calibration medium (water / IPA) is runtime-switchable
> > via the in_volumeflow_medium sysfs attribute and therefore not a
> > DT property.
> >
> > Signed-off-by: Wadim Mueller <wafgo01@gmail.com>
> > Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> > ---
> > .../bindings/iio/flow/sensirion,slf3s.yaml | 58 +++++++++++++++++++
> > MAINTAINERS | 7 +++
> > 2 files changed, 65 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/iio/flow/sensirion,slf3s.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/iio/flow/sensirion,slf3s.yaml b/Documentation/devicetree/bindings/iio/flow/sensirion,slf3s.yaml
> > new file mode 100644
> > index 000000000000..c054a505b75d
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/iio/flow/sensirion,slf3s.yaml
> > @@ -0,0 +1,58 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/iio/flow/sensirion,slf3s.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Sensirion SLF3S liquid flow sensor
> > +
> > +maintainers:
> > + - Wadim Mueller <wafgo01@gmail.com>
> > +
> > +description:
> > + Family of digital liquid-flow sensors from Sensirion with I2C interface. All
> > + family members share the same register map; sub-types differ only in the flow
> > + scale factor and the calibrated measurement range. The sub-type can be
> > + identified from the product-information register.
> > +
> > +properties:
> > + compatible:
> > + oneOf:
> > + - const: sensirion,slf3s-1300f
> > + - items:
> > + - enum:
> > + - sensirion,slf3s-0600f
> > + - sensirion,slf3s-4000b
> > + - const: sensirion,slf3s-1300f
>
> This is fine with me, also fine with separate non-compatible compatibles.
Thanks!
I'd prefer separate to avoid confusing me at least! :)
It needs new version anyway as the driver (patch 4) doesn't seem to have
reached me or patchwork.
Jonathan
>
> Best regards,
> Krzysztof
^ permalink raw reply
* RE: [PATCH v2 3/5] hwmon: (pmbus/max20830): add VOUT feedback resistor scaling support
From: Torreno, Alexis Czezar @ 2026-07-20 2:27 UTC (permalink / raw)
To: Guenter Roeck, sashiko-reviews@lists.linux.dev
Cc: linux-hwmon@vger.kernel.org, conor+dt@kernel.org,
devicetree@vger.kernel.org, robh@kernel.org
In-Reply-To: <2e93fb55-5055-4c54-8ea2-6ef59248c842@roeck-us.net>
> >
> >>> +
> >>> static struct pmbus_driver_info max20830_info = {
> >>> .pages = 1,
> >>> .format[PSC_VOLTAGE_IN] = linear, @@ -24,13 +64,25 @@ static
> >>> struct pmbus_driver_info max20830_info = {
> >>> PMBUS_HAVE_TEMP |
> >>> PMBUS_HAVE_STATUS_VOUT | PMBUS_HAVE_STATUS_IOUT |
> >>> PMBUS_HAVE_STATUS_INPUT | PMBUS_HAVE_STATUS_TEMP,
> >>> + .read_word_data = max20830_read_word_data,
> >>
> >> [Severity: High]
> >> Does this struct also need a .write_word_data callback?
> >>
> >> Since PMBUS_HAVE_VOUT is enabled, user writes to limit attributes
> >> will fall through to the PMBus core, which writes the scaled PMBus
> >> word directly into the hardware's limit register. Because the
> >> hardware expects limits in the unscaled feedback voltage range, the
> >> written value will be completely out of bounds.
> >>
> >
> > Not applicable. No writable VOUT registers are exposed. MAX20830 has
> > no limit registers to write to. also regulator support is not implemented.
> >
>
> Please add a comment to the code explaining this (if you dod not do that
> already), to remind patch authors that there is additional work to do if regulator
> support is added.
>
Already added but will further clarify the comments I wrote.
Regards,
Alexis
^ permalink raw reply
* [PATCH v10 3/3] thermal/drivers/imx: Add calibration offset support
From: Haoning CHENG via B4 Relay @ 2026-07-20 2:19 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-pm, devicetree, imx, linux-arm-kernel, linux-kernel,
Haoning CHENG, Frank Li
In-Reply-To: <20260720-b4-symana21-11221-imx-thermal-support-upstream-6-18-v10-0-f8c3dcd9a57e@cn.bosch.com>
From: Haoning CHENG <Haoning.CHENG@cn.bosch.com>
The TEMPMON sensor reading may deviate from the theoretical SoC
junction temperature. Hardware characterization shows a measurable
offset between the sensor output and the calculated junction
temperature, which exists even at the die level without board
influence.
Read the optional fsl,temp-calibration-offset-millicelsius property
from DT and apply it uniformly to the i.MX6/6SX/7D formulas.
The offset is applied symmetrically at two points to keep trip point
semantics consistent:
1. In imx_set_alarm_temp() and imx_set_panic_temp(): the temperature
threshold is *subtracted* by the offset before being converted to a
hardware register value. This keeps the hardware IRQ trigger aligned
with the physical sensor reading that corresponds to the intended
temperature threshold.
2. In imx_get_temp(): after computing the temperature from the hardware
register, the offset is *added* to produce the calibrated value.
The thermal framework always sees the corrected temperature.
For example, if DT sets offset = +3000 m°C and the passive trip is 95°C:
imx_set_alarm_temp(95000):
alarm_temp = 95000 - 3000 = 92000
→ hardware register programmed for 92°C sensor reading
Hardware IRQ fires when sensor reads 92°C
imx_get_temp():
reads hardware, computes 92°C
*temp = 92000 + 3000 = 95000
→ thermal framework sees 95°C → correct trip
When the property is not present, the offset defaults to 0, preserving
the current behavior.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Haoning Cheng <Haoning.CHENG@cn.bosch.com>
---
drivers/thermal/imx_thermal.c | 27 +++++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 7f7d1116b9d6..d471acc16bce 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -85,6 +85,10 @@ enum imx_thermal_trip {
#define TEMPMON_IMX6SX 2
#define TEMPMON_IMX7D 3
+/* Calibration offset limits (±20 °C in millicelsius) */
+#define IMX_TEMP_CALIB_OFFSET_MIN (-20000)
+#define IMX_TEMP_CALIB_OFFSET_MAX 20000
+
struct thermal_soc_data {
u32 version;
@@ -207,6 +211,7 @@ struct imx_thermal_data {
struct regmap *tempmon;
u32 c1, c2; /* See formula in imx_init_calib() */
int temp_max;
+ s32 calibration_offset;
int alarm_temp;
int last_temp;
bool irq_enabled;
@@ -223,6 +228,7 @@ static void imx_set_panic_temp(struct imx_thermal_data *data,
struct regmap *map = data->tempmon;
int critical_value;
+ panic_temp -= data->calibration_offset;
critical_value = (data->c2 - panic_temp) / data->c1;
regmap_write(map, soc_data->panic_alarm_ctrl + REG_CLR,
@@ -239,6 +245,7 @@ static void imx_set_alarm_temp(struct imx_thermal_data *data,
int alarm_value;
data->alarm_temp = alarm_temp;
+ alarm_temp -= data->calibration_offset;
if (data->socdata->version == TEMPMON_IMX7D) {
if (alarm_temp >= 0)
@@ -283,6 +290,7 @@ static int imx_get_temp(struct thermal_zone_device *tz, int *temp)
*temp = (n_meas - data->c1 + 25) * 1000;
else
*temp = data->c2 - n_meas * data->c1;
+ *temp += data->calibration_offset;
/* Update alarm value to next higher trip point for TEMPMON_IMX6Q */
if (data->socdata->version == TEMPMON_IMX6Q) {
@@ -635,6 +643,25 @@ static int imx_thermal_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, data);
+ if (of_property_present(dev->of_node,
+ "fsl,temp-calibration-offset-millicelsius")) {
+ ret = of_property_read_s32(dev->of_node,
+ "fsl,temp-calibration-offset-millicelsius",
+ &data->calibration_offset);
+ if (ret)
+ return dev_err_probe(dev, ret,
+ "failed to read calibration offset\n");
+
+ if (data->calibration_offset < IMX_TEMP_CALIB_OFFSET_MIN ||
+ data->calibration_offset > IMX_TEMP_CALIB_OFFSET_MAX)
+ return dev_err_probe(dev, -EINVAL,
+ "calibration offset %d millicelsius out of range\n",
+ data->calibration_offset);
+
+ dev_dbg(dev, "calibration offset: %d millicelsius\n",
+ data->calibration_offset);
+ }
+
if (of_property_present(dev->of_node, "nvmem-cells")) {
ret = imx_init_from_nvmem_cells(pdev);
if (ret)
--
2.43.0
^ permalink raw reply related
* [PATCH v10 2/3] thermal/drivers/imx: Fix rounding and clamp for i.MX7D alarm
From: Haoning CHENG via B4 Relay @ 2026-07-20 2:19 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-pm, devicetree, imx, linux-arm-kernel, linux-kernel,
Haoning CHENG, Frank Li
In-Reply-To: <20260720-b4-symana21-11221-imx-thermal-support-upstream-6-18-v10-0-f8c3dcd9a57e@cn.bosch.com>
From: Haoning CHENG <Haoning.CHENG@cn.bosch.com>
Convert the alarm temperature from millicelsius to degrees for i.MX7D
using ceiling division instead of integer division, ensuring rounding
errors do not cause the alarm to trigger below the intended threshold.
Use DIV_ROUND_UP() for non-negative values and plain integer division
for negative values, since C rounds toward zero which is equivalent to
ceiling when the divisor is positive.
Add clamp() to ensure the hardware register value stays within the 9-bit
range (0..0x1ff) of the i.MX7D alarm field, preventing silent truncation
if an out-of-range value is written.
Reviewed-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Haoning Cheng <Haoning.CHENG@cn.bosch.com>
---
drivers/thermal/imx_thermal.c | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/imx_thermal.c b/drivers/thermal/imx_thermal.c
index 38c993d1bcb3..7f7d1116b9d6 100644
--- a/drivers/thermal/imx_thermal.c
+++ b/drivers/thermal/imx_thermal.c
@@ -240,10 +240,16 @@ static void imx_set_alarm_temp(struct imx_thermal_data *data,
data->alarm_temp = alarm_temp;
- if (data->socdata->version == TEMPMON_IMX7D)
- alarm_value = alarm_temp / 1000 + data->c1 - 25;
- else
+ if (data->socdata->version == TEMPMON_IMX7D) {
+ if (alarm_temp >= 0)
+ alarm_temp = DIV_ROUND_UP(alarm_temp, 1000);
+ else
+ alarm_temp /= 1000;
+ alarm_value = alarm_temp + data->c1 - 25;
+ alarm_value = clamp(alarm_value, 0, 0x1ff);
+ } else {
alarm_value = (data->c2 - alarm_temp) / data->c1;
+ }
regmap_write(map, soc_data->high_alarm_ctrl + REG_CLR,
soc_data->high_alarm_mask);
--
2.43.0
^ permalink raw reply related
* [PATCH v10 0/3] thermal: imx: Add calibration offset support
From: Haoning CHENG via B4 Relay @ 2026-07-20 2:19 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-pm, devicetree, imx, linux-arm-kernel, linux-kernel,
Haoning CHENG, Krzysztof Kozlowski, Frank Li
The TEMPMON sensor reading may deviate from the theoretical SoC
junction temperature even at the die level, before any board
influence. This series adds an optional DT property to specify a
calibration offset and implements the corresponding support in the
imx_thermal driver.
Patch 1 documents the new fsl,temp-calibration-offset-millicelsius
property in the i.MX thermal DT binding. The offset is determined
through thermal characterization by comparing the sensor output
against the calculated junction temperature, and is limited to
±20 °C.
Patch 2 fixes the i.MX7D alarm temperature conversion to use
ceiling division (DIV_ROUND_UP() for non-negative, plain /
for negative) and clamp() to stay within the 9-bit register range.
Patch 3 reads this property in the imx_thermal driver and applies
the offset symmetrically: it is added when reporting temperature to
the thermal framework and subtracted when programming hardware alarm
thresholds (to keep trip point semantics consistent). Out-of-range
values are rejected with -EINVAL at probe time. When the property
is absent, the default offset is 0.
Signed-off-by: Haoning CHENG <Haoning.CHENG@cn.bosch.com>
---
Changes in v10:
- Rephrase the series to describe the offset as sensor calibration
toward the theoretical junction temperature, reverting the v9
terminology that characterized it as a board-specific die-to-package-
surface conversion. Hardware characterization confirmed the deviation
exists at the die level, independent of board influence.
- dt-bindings: update the property description to state the offset
corrects the TEMPMON sensor reading toward the theoretical junction
temperature
- driver commit message: replace package-surface estimate wording with
calibrated correction toward junction temperature
- Link to v9: https://patch.msgid.link/20260717-b4-symana21-11221-imx-thermal-support-upstream-6-18-v9-0-75f4af8974f4@cn.bosch.com
Changes in v9:
- Rephrase the series to clarify that the offset converts the internal
TEMPMON reading to a package-surface temperature estimate, rather than
calibrating the sensor for better die temperature accuracy (Frieder)
- dt-bindings: update the property description to explicitly state it is
a board-specific conversion offset, not a sensor calibration
- driver commit message: distinguish die temperature from package-surface
temperature in the symmetric offset mechanism explanation
- Pick up Reviewed-by tags from Frank Li (patches 2,3) and Lukasz Luba
(patches 1,2,3)
- Link to v8: https://patch.msgid.link/20260714-b4-symana21-11221-imx-thermal-support-upstream-6-18-v8-0-d54d8690e16e@cn.bosch.com
Changes in v8:
- binding: s/behaviour/behavior/ (Lukasz)
- New prep patch: use ceiling division (DIV_ROUND_UP() for non-negative, plain /
for negative) and clamp() for i.MX7D, extracted into a separate patch (Frank)
- driver commit message: add offset mechanism explanation with a +3000 m°C calculation example (Lukasz)
- probe: reject out-of-range offset with -EINVAL instead of silently clamping (Lukasz)
- prep patch: handle negative alarm_temp with plain division (C rounds toward zero)
- Link to v7: https://patch.msgid.link/20260713-b4-symana21-11221-imx-thermal-support-upstream-6-18-v7-0-09b6b7669be1@cn.bosch.com
Changes in v7:
- Align the author name with the Signed-off-by trailer.
- Link to v6: https://patch.msgid.link/20260713-b4-symana21-11221-imx-thermal-support-upstream-6-18-v6-0-88a378faeca5@cn.bosch.com
Changes in v6:
- Align the author name with the Signed-off-by trailer.
- Link to v5: https://patch.msgid.link/20260713-b4-symana21-11221-imx-thermal-support-upstream-6-18-v5-0-69405c306c6b@cn.bosch.com
Changes in v5:
- Move calibration offset application out of c2 initialization, instead
apply it in imx_get_temp(), imx_set_alarm_temp() and imx_set_panic_temp()
for consistent style across i.MX6 and i.MX7D
- Subtract calibration offset from alarm_temp before the if-else block in
imx_set_alarm_temp(), simplifying both SoC branches (Frank's suggestion)
- Link to v4: https://patch.msgid.link/20260710-b4-symana21-11221-imx-thermal-support-upstream-6-18-v4-0-1fef97d1c750@cn.bosch.com
Changes in v4:
- dt-bindings: drop maxItems to allow minimum/maximum constraints for
temp-calibration-offset-millicelsius
- Link to v3: https://patch.msgid.link/20260710-b4-symana21-11221-imx-thermal-support-upstream-6-18-v3-0-db9fc2947c55@cn.bosch.com
Changes in v3:
- dt-bindings: add minimum/maximum (-28580/+28580) for
temp-calibration-offset-millicelsius, as suggested by Conor Dooley
- Link to v2: https://patch.msgid.link/20260709-b4-symana21-11221-imx-thermal-support-upstream-6-18-v2-0-00ff72495e24@cn.bosch.com
Changes in v2:
- dt-bindings: Removed explicit `$ref: /schemas/types.yaml#/definitions/int32`
for fsl,temp-calibration-offset-millicelsius; the `-millicelsius` suffix
already resolves to int32-array via property-units.yaml (Rob).
- dt-bindings: Fixed example indentation.
- driver: Replaced C integer division (/) with DIV_ROUND_CLOSEST() in
imx_set_alarm_temp() to prevent off-by-one alarm threshold mismatch that
could cause IRQ storms on i.MX7D.
- driver: Added clamp() bounds check for alarm_value [0, 0x1ff] to avoid
corrupting adjacent register fields (PANIC_ALARM).
- Link to v1: https://patch.msgid.link/20260709-b4-symana21-11221-imx-thermal-support-upstream-6-18-v1-0-00f88f42930b@cn.bosch.com
To: "Rafael J. Wysocki" <rafael@kernel.org>
To: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Zhang Rui <rui.zhang@intel.com>
To: Lukasz Luba <lukasz.luba@arm.com>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Shawn Guo <shawnguo@kernel.org>
To: Sascha Hauer <s.hauer@pengutronix.de>
To: Pengutronix Kernel Team <kernel@pengutronix.de>
To: Fabio Estevam <festevam@gmail.com>
Cc: linux-pm@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: imx@lists.linux.dev
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
---
Haoning CHENG (3):
dt-bindings: thermal: imx: Document calibration offset property
thermal/drivers/imx: Fix rounding and clamp for i.MX7D alarm
thermal/drivers/imx: Add calibration offset support
.../devicetree/bindings/thermal/imx-thermal.yaml | 12 +++++++
drivers/thermal/imx_thermal.c | 39 ++++++++++++++++++++--
2 files changed, 48 insertions(+), 3 deletions(-)
---
base-commit: c50a940dcde35c647e097e89a9150003abd48329
change-id: 20260709-b4-symana21-11221-imx-thermal-support-upstream-6-18-a084984deb76
Best regards,
--
Haoning Cheng <Haoning.CHENG@cn.bosch.com>
^ permalink raw reply
* [PATCH v10 1/3] dt-bindings: thermal: imx: Document calibration offset property
From: Haoning CHENG via B4 Relay @ 2026-07-20 2:19 UTC (permalink / raw)
To: Rafael J. Wysocki, Daniel Lezcano, Zhang Rui, Lukasz Luba,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: linux-pm, devicetree, imx, linux-arm-kernel, linux-kernel,
Haoning CHENG, Krzysztof Kozlowski
In-Reply-To: <20260720-b4-symana21-11221-imx-thermal-support-upstream-6-18-v10-0-f8c3dcd9a57e@cn.bosch.com>
From: Haoning CHENG <Haoning.CHENG@cn.bosch.com>
The TEMPMON sensor reading may deviate from the theoretical SoC
junction temperature. Document the optional
fsl,temp-calibration-offset-millicelsius property, a signed offset
in millicelsius applied to correct the sensor reading toward the
theoretical junction temperature.
The offset is determined through thermal characterization by
comparing the sensor output against the calculated junction
temperature. When absent, the offset is zero.
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Signed-off-by: Haoning CHENG <Haoning.CHENG@cn.bosch.com>
---
Documentation/devicetree/bindings/thermal/imx-thermal.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/Documentation/devicetree/bindings/thermal/imx-thermal.yaml b/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
index 949b154856c5..503a6e7a2c68 100644
--- a/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
+++ b/Documentation/devicetree/bindings/thermal/imx-thermal.yaml
@@ -59,6 +59,17 @@ properties:
clocks:
maxItems: 1
+ fsl,temp-calibration-offset-millicelsius:
+ minimum: -20000
+ maximum: 20000
+ description:
+ A signed offset, in millicelsius, applied to the TEMPMON
+ sensor reading to correct it toward the theoretical SoC
+ junction temperature. The offset is determined through
+ thermal characterization by comparing the sensor output
+ against the calculated junction temperature. The range is
+ limited to ±20 °C. When absent, no offset is applied.
+
"#thermal-sensor-cells":
const: 0
@@ -109,6 +120,7 @@ examples:
nvmem-cells = <&tempmon_calib>, <&tempmon_temp_grade>;
nvmem-cell-names = "calib", "temp_grade";
clocks = <&clks IMX6SX_CLK_PLL3_USB_OTG>;
+ fsl,temp-calibration-offset-millicelsius = <(-6400)>;
#thermal-sensor-cells = <0>;
};
};
--
2.43.0
^ permalink raw reply related
* Re: [PATCH v7 2/2] arm64: dts: cix: add sky1 DMA-350 node with channel IRQ entries
From: Gary Yang @ 2026-07-20 2:18 UTC (permalink / raw)
To: Jun Guo
Cc: peter.chen, fugang.duan, robh, krzk+dt, conor+dt, vkoul, ychuang3,
schung, robin.murphy, Frank.Li, dmaengine, devicetree,
linux-kernel, cix-kernel-upstream, linux-arm-kernel
In-Reply-To: <20260521072924.3000282-3-jun.guo@cixtech.com>
On 2026-05-21 15:29, Jun Guo wrote:
> Describe the DMA-350 channel interrupt sources in DT using 8
> interrupt entries, while all entries map to the same GIC SPI
> as wired on this platform.
>
> Signed-off-by: Jun Guo <jun.guo@cixtech.com>
Applied, thanks!
Gary
> ---
> arch/arm64/boot/dts/cix/sky1.dtsi | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/cix/sky1.dtsi b/arch/arm64/boot/dts/cix/sky1.dtsi
> index bb5cfb1f2113..823adeef51f1 100644
> --- a/arch/arm64/boot/dts/cix/sky1.dtsi
> +++ b/arch/arm64/boot/dts/cix/sky1.dtsi
> @@ -444,6 +444,20 @@ iomuxc: pinctrl@4170000 {
> reg = <0x0 0x04170000 0x0 0x1000>;
> };
>
> + fch_dmac: dma-controller@4190000 {
> + compatible = "arm,dma-350";
> + reg = <0x0 0x4190000 0x0 0x10000>;
> + interrupts = <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>,
> + <GIC_SPI 303 IRQ_TYPE_LEVEL_HIGH 0>;
> + #dma-cells = <1>;
> + };
> +
> mbox_ap2se: mailbox@5060000 {
> compatible = "cix,sky1-mbox";
> reg = <0x0 0x05060000 0x0 0x10000>;
> --
> 2.34.1
>
^ permalink raw reply
* Re: [PATCH v2 1/5] dt-bindings: PCI: rcar-gen4-pci-host: Add R-Car X5H PCIe4 compatible
From: sashiko-bot @ 2026-07-20 2:18 UTC (permalink / raw)
To: Marek Vasut; +Cc: wsa+renesas, devicetree, linux-pci, robh, conor+dt
In-Reply-To: <20260720020505.315335-2-marek.vasut+renesas@mailbox.org>
> Document bindings for R-Car X5H (R8A78000) PCIe4 host module.
>
> The binding document is deliberately using "renesas,rcar-gen5-pcie4"
> DT compatible string to discern R-Car X5H PCIe4 controller supported
> by this binding, from R-Car X5H PCIe6 controller which will use a
> separate binding.
>
> The R-Car X5H PCIe4 controller does no longer include PHY register
> range, the PHY is now a separate IP and referenced via the 'phy' DT
> property. The 'reg' and 'reg-names' DT properties therefore differ
> between R-Car Gen4 PCIe and R-Car X5H PCIe4, and the difference is
> handled in the allOf section.
>
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720020505.315335-1-marek.vasut+renesas@mailbox.org?part=1
^ permalink raw reply
* Re: [PATCH v2 5/5] PCI: dwc: rcar-gen4: Add support for R-Car X5H PCIe4
From: sashiko-bot @ 2026-07-20 2:18 UTC (permalink / raw)
To: Marek Vasut; +Cc: devicetree, conor+dt, linux-pci, wsa+renesas, robh
In-Reply-To: <20260720020505.315335-6-marek.vasut+renesas@mailbox.org>
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing `phy_exit()` call on error paths and driver teardown causes a leak of PHY resources.
- [High] Missing cleanup of core clocks and reset states when `phy_init()` fails in `rcar_gen5_pcie_init()`.
- [High] Incorrect initialization sequence in `rcar_gen5_pcie_ltssm_control()` and ignored error check for `phy_power_on()`.
- [High] A 32-bit read-modify-write of `PCI_EXP_LNKCTL` inadvertently clears Write-1-to-Clear (W1C) bits in the adjacent `PCI_EXP_LNKSTA` register.
- [High] Dereference of `ERR_PTR` in `rcar->phy_base` on Gen4 code paths if a malformed Device Tree is provided.
--
--- Patch [5]: [PATCH v2 5/5] PCI: dwc: rcar-gen4: Add support for R-Car X5H PCIe4 ---
commit cab5ad82173a65768d51a17315ea45161e805bdf
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>
PCI: dwc: rcar-gen4: Add support for R-Car X5H PCIe4
Add support for R8A78000 (R-Car X5H) PCIe4. The controller initialization
sequence is factored out into controller-specific callbacks. Endpoint mode
is currently not implemented for R-Car Gen5 PCIe4.
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index f88168f2df195..794c42c53380d 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
[ ... ]
> @@ -169,6 +177,35 @@ static int rcar_gen4_pcie_speed_control(struct rcar_gen4_pcie *rcar)
> +static int rcar_gen5_pcie_speed_control(struct rcar_gen4_pcie *rcar)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> + u32 lnkcap = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCAP));
> + u32 lnksta = dw_pcie_readw_dbi(dw, EXPCAP(PCI_EXP_LNKSTA));
> + u32 val, retries;
> +
> + if ((lnksta & PCI_EXP_LNKSTA_CLS) == (lnkcap & PCI_EXP_LNKCAP_SLS))
> + return 0;
> +
> + /* Retrain link */
> + val = dw_pcie_readl_dbi(dw, EXPCAP(PCI_EXP_LNKCTL));
> + val |= PCI_EXP_LNKCTL_RL;
> + dw_pcie_writel_dbi(dw, EXPCAP(PCI_EXP_LNKCTL), val);
[Severity: High]
Will this 32-bit read-modify-write access unintentionally clear Write-1-to-Clear
(W1C) bits in the adjacent PCI_EXP_LNKSTA register?
Since PCI_EXP_LNKCTL (offset 0x10) and PCI_EXP_LNKSTA (offset 0x12) share the
same 32-bit Dword, any W1C bits set in the Link Status register will be read as
1 and written back as 1, clearing them. Could a 16-bit accessor be used instead?
> +
> + /* Wait for link retrain */
[ ... ]
> @@ -284,6 +321,44 @@ static int rcar_gen4_v4h_v4m_pcie_init(struct rcar_gen4_pcie *rcar)
> +static int rcar_gen5_pcie_init(struct rcar_gen4_pcie *rcar)
> +{
> + struct dw_pcie *dw = &rcar->dw;
> + int ret;
> + u32 val;
> +
> + /* R-Car Gen4 and Gen5 common initialization. */
> + ret = rcar_gen4_pcie_common_init(rcar);
> + if (ret)
> + return ret;
> +
> + /* R-Car Gen5 specific additional initialization. */
> + ret = phy_init(rcar->phy);
> + if (ret)
> + return ret;
[Severity: High]
If this fails, do we need to clean up the resources just initialized by
rcar_gen4_pcie_common_init()? It looks like returning the error directly leaves
core clocks enabled and resets unhandled.
[Severity: High]
Since phy_init() is called on the successful initialization path, shouldn't
there be a matching phy_exit() in the deinitialization paths (such as
rcar_gen4_pcie_common_deinit) to prevent leaking PHY resources when the device
is unbound?
> +
> + dw_pcie_dbi_ro_wr_en(dw);
[ ... ]
> @@ -321,8 +396,11 @@ static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar)
> struct device_node *root_port;
>
> rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy");
> - if (IS_ERR(rcar->phy_base))
> - return PTR_ERR(rcar->phy_base);
> + if (IS_ERR(rcar->phy_base)) {
> + rcar->phy = devm_phy_get(dev, NULL);
> + if (IS_ERR(rcar->phy))
> + return PTR_ERR(rcar->phy);
> + }
[Severity: High]
If the device tree lacks the phy memory region but provides a phys node,
the fallback devm_phy_get() succeeds, leaving rcar->phy_base containing an
error pointer.
Later on, if Gen4 code paths like rcar_gen4_pcie_ltssm_control() execute,
they unconditionally call rcar_gen4_pcie_phy_reg_update_bits():
rcar_gen4_pcie_phy_reg_update_bits()
tmp = readl(rcar->phy_base + offset);
Can this dereference the error pointer and crash? Should rcar->phy_base be set
to NULL after falling back to devm_phy_get()?
>
> root_port = of_get_next_available_child(dev->of_node, NULL);
[ ... ]
> @@ -743,6 +821,28 @@ static int r8a779f0_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable)
> +static int rcar_gen5_pcie_ltssm_control(struct rcar_gen4_pcie *rcar, bool enable)
> +{
> + u32 val;
> +
> + val = readl(rcar->base + PCIERSTCTRL1);
> + if (enable) {
> + val |= APP_LTSSM_ENABLE;
> + val &= ~APP_HOLD_PHY_RST;
> + } else {
> + val &= ~APP_LTSSM_ENABLE;
> + val |= APP_HOLD_PHY_RST;
> + }
> + writel(val, rcar->base + PCIERSTCTRL1);
> +
> + if (enable)
> + phy_power_on(rcar->phy);
> + else
> + phy_power_off(rcar->phy);
> +
> + return 0;
> +}
[Severity: High]
Is it expected to enable the LTSSM and drop the PHY reset before calling
phy_power_on()? Normally, wouldn't the PHY need to be powered and stabilized
before activating the MAC's LTSSM?
Also, if phy_power_on() fails, shouldn't we handle the error instead of
unconditionally returning 0?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720020505.315335-1-marek.vasut+renesas@mailbox.org?part=5
^ permalink raw reply
* Re: [PATCH v2 4/5] PCI: dwc: rcar-gen4: Handle PERST via reset subsystem
From: sashiko-bot @ 2026-07-20 2:16 UTC (permalink / raw)
To: Marek Vasut; +Cc: linux-pci, conor+dt, wsa+renesas, robh, devicetree
In-Reply-To: <20260720020505.315335-5-marek.vasut+renesas@mailbox.org>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Incorrectly ordered error labels in `rcar_gen4_pcie_probe()` result in a leaked reset control and unbalanced runtime PM operations.
- [High] Probe fails unconditionally with -EINVAL on device trees without a root port child node (e.g., existing R-Car Gen4 DTs and PCIe endpoints).
--
--- Patch [4]: [PATCH v2 4/5] PCI: dwc: rcar-gen4: Handle PERST via reset subsystem ---
commit ee4c8b0343b92a78672b94439fca7074d908ca06
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>
PCI: dwc: rcar-gen4: Handle PERST via reset subsystem
Handle PERST via both GPIO and reset subsystem. On R-Car Gen4, the PERST
signal is operated as a GPIO, on R-Car Gen5 it might only be accessible via
SCMI reset via reset subsystem. Support both options.
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 73b01e74b42cb..f88168f2df195 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
[ ... ]
> @@ -316,12 +317,23 @@ static void rcar_gen4_pcie_unprepare(struct rcar_gen4_pcie *rcar)
>
> static int rcar_gen4_pcie_get_resources(struct rcar_gen4_pcie *rcar)
> {
> + struct device *dev = rcar->dw.dev;
> + struct device_node *root_port;
> +
> rcar->phy_base = devm_platform_ioremap_resource_byname(rcar->pdev, "phy");
> if (IS_ERR(rcar->phy_base))
> return PTR_ERR(rcar->phy_base);
>
> + root_port = of_get_next_available_child(dev->of_node, NULL);
> + rcar->perst = of_reset_control_get_optional_exclusive(root_port, "perst");
[Severity: High]
Does this cause a probe failure on device trees without a root port child
node?
If of_get_next_available_child() returns NULL (for example, on existing R-Car
Gen4 setups or endpoints), passing a NULL root_port to
of_reset_control_get_optional_exclusive() will return ERR_PTR(-EINVAL). This
error is then returned, causing the probe to fail unconditionally.
> + of_node_put(root_port);
> + if (IS_ERR(rcar->perst))
> + return dev_err_probe(dev, PTR_ERR(rcar->perst), "Failed to get PERST#\n");
> +
> /* Renesas-specific registers */
> rcar->base = devm_platform_ioremap_resource_byname(rcar->pdev, "app");
> + if (IS_ERR(rcar->base))
> + reset_control_put(rcar->perst);
>
> return PTR_ERR_OR_ZERO(rcar->base);
> }
[ ... ]
> @@ -654,7 +682,7 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
>
> err = rcar_gen4_pcie_prepare(rcar);
> if (err)
> - return err;
> + goto err_prepare;
>
> err = rcar_gen4_add_dw_pcie(rcar);
> if (err)
> @@ -662,6 +690,8 @@ static int rcar_gen4_pcie_probe(struct platform_device *pdev)
>
> return 0;
>
> +err_prepare:
> + reset_control_put(rcar->perst);
> err_unprepare:
> rcar_gen4_pcie_unprepare(rcar);
[Severity: High]
Are the error labels out of order here?
If rcar_gen4_pcie_prepare() fails, the code jumps to err_prepare which then
falls through to err_unprepare. This calls rcar_gen4_pcie_unprepare() and
performs an unbalanced pm_runtime_put().
Conversely, if rcar_gen4_add_dw_pcie() fails, the code jumps to err_unprepare,
skipping err_prepare entirely. This results in a leaked rcar->perst reset
control.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720020505.315335-1-marek.vasut+renesas@mailbox.org?part=4
^ permalink raw reply
* Re: [PATCH v2 2/5] PCI: dwc: rcar-gen4: Rework rcar_gen4_pcie_drvdata .additional_common_init into .init
From: sashiko-bot @ 2026-07-20 2:15 UTC (permalink / raw)
To: Marek Vasut; +Cc: linux-pci, wsa+renesas, conor+dt, devicetree, robh
In-Reply-To: <20260720020505.315335-3-marek.vasut+renesas@mailbox.org>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Silent failure in `rcar_gen4_pcie_ep_pre_init()` and asymmetric teardown in endpoint error paths lead to kernel panics and unbalanced clock disables.
--
--- Patch [2]: [PATCH v2 2/5] PCI: dwc: rcar-gen4: Rework rcar_gen4_pcie_drvdata .additional_common_init into .init ---
commit f4863c849aaf3cf13744faee6e2f333b5420b025
Author: Marek Vasut <marek.vasut+renesas@mailbox.org>
PCI: dwc: rcar-gen4: Rework rcar_gen4_pcie_drvdata .additional_common_init into .init
This commit simplifies the PCIe initialization callchain for R-Car Gen4 SoCs.
It replaces the additional_common_init callback with a unified init callback
for both host and endpoint configurations.
> diff --git a/drivers/pci/controller/dwc/pcie-rcar-gen4.c b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> index 5f7211b91ee5b..5724f64eadfad 100644
> --- a/drivers/pci/controller/dwc/pcie-rcar-gen4.c
> +++ b/drivers/pci/controller/dwc/pcie-rcar-gen4.c
[ ... ]
> @@ -487,7 +509,7 @@ static void rcar_gen4_pcie_ep_pre_init(struct dw_pcie_ep *ep)
> struct rcar_gen4_pcie *rcar = to_rcar_gen4_pcie(dw);
> int ret;
>
> - ret = rcar_gen4_pcie_common_init(rcar);
> + ret = rcar->drvdata->init(rcar);
> if (ret)
> return;
[Severity: High]
This is a pre-existing issue, but since this area is being modified, does the
early return here lead to a kernel panic if initialization fails?
If rcar->drvdata->init() fails, it returns early but cannot propagate the
error because pre_init() returns void. Execution then continues into
dw_pcie_ep_init_registers(), which accesses unclocked DWC registers, resulting
in an SError panic.
Also, if dw_pcie_ep_init() fails before calling ep->ops->pre_init() (such as
in devm_pci_epc_create()), the error path in rcar_gen4_add_dw_pcie_ep()
unconditionally calls rcar_gen4_pcie_ep_deinit(). That function performs an
MMIO write to PCIEDMAINTSTSEN while clocks are disabled, causing another
SError panic, and warns about unbalanced clock disables.
Could these endpoint error paths be reworked so the kernel fails gracefully
instead of crashing?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720020505.315335-1-marek.vasut+renesas@mailbox.org?part=2
^ permalink raw reply
* Re: [PATCH v2 3/5] PCI: dwc: rcar-gen4: Split .start_link into ltssm_control and speed_control
From: sashiko-bot @ 2026-07-20 2:10 UTC (permalink / raw)
To: Marek Vasut; +Cc: devicetree, linux-pci, conor+dt, wsa+renesas, robh
In-Reply-To: <20260720020505.315335-4-marek.vasut+renesas@mailbox.org>
> Introduce new .speed_control callback and implement it on R-Car Gen4.
> The callback implements the second half of what is currently present
> in rcar_gen4_pcie_start_link(), and rcar_gen4_pcie_start_link() does
> call the .speed_control callback. This is a preparatory patch for
> R-Car Gen5 support, where the .speed_control implementation is
> different.
>
> Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720020505.315335-1-marek.vasut+renesas@mailbox.org?part=3
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox