[PATCH v7 1/3] dt-bindings: i3c: Add support for Qualcomm I3C controller

[PATCH v7 1/3] dt-bindings: i3c: Add support for Qualcomm I3C controller

[Mukesh Kumar Savaliya]

Add device tree bindings for the Qualcomm GENI-based I3C controller.

The controller supports I3C SDR transfers in mixed-bus mode with both I2C
and I3C target devices present on the same bus. The binding describes the
I3C controller instance along with its associated In-Band Interrupt (IBI)
controller used for handling I3C target IBI events.

Document the required resources, interrupts, clocks, and dependencies
needed to represent the hardware in device tree.

Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
 .../bindings/i3c/qcom,geni-i3c.yaml           | 115 ++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml

diff --git a/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
new file mode 100644
index 000000000000..bab89aeee23f
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i3c/qcom,geni-i3c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Geni based QUP I3C Controller
+
+maintainers:
+  - Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
+
+description:
+  Qualcomm GENI-based I3C controller implementing an MIPI I3C bus master.
+  The controller supports SDR transfers in mixed bus mode with both I2C and
+  I3C target devices present on the same bus.
+
+  The controller consists of a GENI Serial Engine (SE) responsible for I3C
+  bus transactions and an associated dedicated IBI controller(IBI_C) block
+  used for Hot-Join, In-Band Interrupt (IBI) handling and management of GPII
+  resources associated with the I3C controller.
+
+  I3C controller nodes must be child nodes of a GENI-based Qualcomm Universal
+  Peripheral(QUP) wrapper. Refer to the GENI QUP wrapper binding described in
+  Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
+
+allOf:
+  - $ref: i3c.yaml#
+  - $ref: /schemas/soc/qcom/qcom,se-common-props.yaml#
+
+properties:
+  compatible:
+    const: qcom,geni-i3c
+
+  reg:
+    items:
+      - description: SE (Serial Engine) register address space
+      - description: IBI controller (IBI_C) register address space
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: se
+
+  interrupts:
+    items:
+      - description: Core Serial Engine (SE) IRQ
+      - description: IBI controller (IBI_C) manager IRQ
+      - description: IBI controller (IBI_C) GPII IRQ
+
+  interconnects:
+    minItems: 2
+    maxItems: 3
+
+  interconnect-names:
+    minItems: 2
+    items:
+      - const: qup-core
+      - const: qup-config
+      - const: qup-memory
+
+  pinctrl-names:
+    minItems: 1
+    items:
+      - const: default
+      - const: sleep
+
+  dmas:
+    maxItems: 2
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - interconnects
+  - interconnect-names
+  - pinctrl-0
+  - pinctrl-1
+  - pinctrl-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,sm8550-gcc.h>
+    #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
+
+    i3c@884000 {
+        compatible = "qcom,geni-i3c";
+        reg = <0x00884000 0x4000>,
+              <0x008c0000 0x10000>;
+        clocks = <&gcc GCC_QUPV3_WRAP2_S1_CLK>;
+        clock-names = "se";
+        interrupts = <GIC_SPI 583 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 796 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 797 IRQ_TYPE_LEVEL_HIGH>;
+        interconnects = <&clk_virt MASTER_QUP_CORE_2 &clk_virt SLAVE_QUP_CORE_2>,
+                        <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_QUP_2>,
+                        <&aggre2_noc MASTER_QUP_2 &mc_virt SLAVE_EBI1>;
+        interconnect-names = "qup-core", "qup-config", "qup-memory";
+        pinctrl-0 = <&qup_i3c1_data_clk>;
+        pinctrl-1 = <&qup_i3c1_data_clk_sleep>;
+        pinctrl-names = "default", "sleep";
+        #address-cells = <3>;
+        #size-cells = <0>;
+    };
+...
-- 
2.43.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v7 2/3] i3c: master: Add Qualcomm I3C controller driver

[Mukesh Kumar Savaliya]

Add support for the Qualcomm I3C controller driver, which implements
I3C master functionality as defined in the MIPI Alliance Specification
for I3C, Version 1.0.

This driver supports master role in SDR mode.

Unlike some other I3C master controllers, this implementation
does not support In-Band Interrupts (IBI) and Hot-join requests.

Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
 drivers/i3c/master/Kconfig         |   12 +
 drivers/i3c/master/Makefile        |    1 +
 drivers/i3c/master/i3c-qcom-geni.c | 1133 ++++++++++++++++++++++++++++
 3 files changed, 1146 insertions(+)
 create mode 100644 drivers/i3c/master/i3c-qcom-geni.c

diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index da96d2aaa399..24db1d0f9476 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -52,6 +52,18 @@ config SVC_I3C_MASTER
 	help
 	  Support for Silvaco I3C Dual-Role Master Controller.
 
+config I3C_QCOM_GENI
+	tristate "Qualcomm Technologies Inc's I3C controller driver"
+	depends on QCOM_GENI_SE
+	help
+	  This driver supports QUPV3 GENI based I3C controller in master
+	  mode on the Qualcomm Technologies Inc's SoCs. If you say yes to
+	  this option, support will be included for the built-in I3C interface
+	  on the Qualcomm Technologies Inc's SoCs.
+
+	  This driver can also be built as a module.  If so, the module
+	  will be called i3c-qcom-geni.
+
 config MIPI_I3C_HCI
 	tristate "MIPI I3C Host Controller Interface driver (EXPERIMENTAL)"
 	depends on HAS_IOMEM
diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
index 8d82196dcf83..5d02569f5051 100644
--- a/drivers/i3c/master/Makefile
+++ b/drivers/i3c/master/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0-only
 obj-$(CONFIG_ADI_I3C_MASTER)		+= adi-i3c-master.o
 obj-$(CONFIG_CDNS_I3C_MASTER)		+= i3c-master-cdns.o
+obj-$(CONFIG_I3C_QCOM_GENI)		+= i3c-qcom-geni.o
 obj-$(CONFIG_DW_I3C_MASTER)		+= dw-i3c-master.o
 obj-$(CONFIG_AST2600_I3C_MASTER)	+= ast2600-i3c-master.o
 obj-$(CONFIG_SVC_I3C_MASTER)		+= svc-i3c-master.o
diff --git a/drivers/i3c/master/i3c-qcom-geni.c b/drivers/i3c/master/i3c-qcom-geni.c
new file mode 100644
index 000000000000..5cb676006cb6
--- /dev/null
+++ b/drivers/i3c/master/i3c-qcom-geni.c
@@ -0,0 +1,1133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2025 Qualcomm Innovation Center, Inc. All rights reserved.
+ *
+ * Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
+ */
+
+#include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/clk.h>
+#include <linux/err.h>
+#include <linux/i3c/master.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_runtime.h>
+#include <linux/slab.h>
+#include <linux/soc/qcom/geni-se.h>
+#include <linux/units.h>
+
+#include "../internals.h"
+
+#define SE_I3C_SCL_HIGH                 0x268
+#define SE_I3C_TX_TRANS_LEN             0x26c
+#define SE_I3C_RX_TRANS_LEN             0x270
+#define SE_I3C_DELAY_COUNTER            0x274
+#define SE_I2C_SCL_COUNTERS             0x278
+#define SE_I3C_SCL_CYCLE                0x27c
+#define SE_GENI_HW_IRQ_EN               0x920
+#define SE_GENI_HW_IRQ_IGNORE_ON_ACTIVE 0x924
+#define SE_GENI_HW_IRQ_CMD_PARAM_0      0x930
+
+/* HW I3C IBI interrupt enable */
+#define M_IBI_IRQ_EN			BIT(0)
+
+/* M_IBI_IRQ_IGNORE */
+#define M_IBI_IRQ_IGNORE		BIT(0)
+
+/* SE_GENI_M_CLK_CFG field shifts */
+#define	CLK_DIV_VALUE_MASK		GENMASK(23, 4)
+#define SER_CLK_EN			BIT(0)
+
+/* SE_GENI_HW_IRQ_CMD_PARAM_0 field bits */
+#define M_IBI_IRQ_PARAM_7E		BIT(0)
+#define M_IBI_IRQ_PARAM_STOP_STALL	BIT(1)
+
+/* SE_I2C_SCL_COUNTERS field shifts */
+#define I2C_SCL_HIGH_COUNTER_MASK	GENMASK(29, 20)
+#define I2C_SCL_LOW_COUNTER_MASK	GENMASK(19, 10)
+#define I2C_SCL_CYCLE_COUNTER_MASK	GENMASK(9, 0)
+
+#define SE_I3C_ERR  (M_CMD_OVERRUN_EN | M_ILLEGAL_CMD_EN | M_CMD_FAILURE_EN |\
+	M_CMD_ABORT_EN | M_GP_IRQ_0_EN | M_GP_IRQ_1_EN | M_GP_IRQ_2_EN | \
+	M_GP_IRQ_3_EN | M_GP_IRQ_4_EN)
+
+/* M_CMD OP codes for I2C/I3C */
+#define I3C_READ_IBI_HW          0
+#define I2C_WRITE                1
+#define I2C_READ                 2
+#define I2C_WRITE_READ           3
+#define I2C_ADDR_ONLY            4
+#define I3C_INBAND_RESET         5
+#define I2C_BUS_CLEAR            6
+#define I2C_STOP_ON_BUS          7
+#define I3C_HDR_DDR_EXIT         8
+#define I3C_PRIVATE_WRITE        9
+#define I3C_PRIVATE_READ         10
+#define I3C_HDR_DDR_WRITE        11
+#define I3C_HDR_DDR_READ         12
+#define I3C_DIRECT_CCC_ADDR_ONLY 13
+#define I3C_BCAST_CCC_ADDR_ONLY  14
+#define I3C_READ_IBI             15
+#define I3C_BCAST_CCC_WRITE      16
+#define I3C_DIRECT_CCC_WRITE     17
+#define I3C_DIRECT_CCC_READ      18
+
+/* M_CMD params for I3C */
+#define PRE_CMD_DELAY		BIT(0)
+#define TIMESTAMP_BEFORE	BIT(1)
+#define STOP_STRETCH		BIT(2)
+#define TIMESTAMP_AFTER		BIT(3)
+#define POST_COMMAND_DELAY	BIT(4)
+#define IGNORE_ADD_NACK		BIT(6)
+#define READ_FINISHED_WITH_ACK	BIT(7)
+#define CONTINUOUS_MODE_DAA	BIT(8)
+
+#define SLAVE_ADDR_MASK		GENMASK(15, 9)
+
+#define CCC_HDR_CMD_MSK		GENMASK(23, 16)
+#define IBI_NACK_TBL_CTRL	BIT(24)
+#define USE_7E			BIT(25)
+#define BYPASS_ADDR_PHASE	BIT(26)
+
+/* GSI callback error fields - DMA_TX_IRQ_STAT */
+#define GP_IRQ0			BIT(5)
+#define GP_IRQ1			BIT(6)
+#define GP_IRQ2			BIT(7)
+#define GP_IRQ3			BIT(8)
+#define GP_IRQ4			BIT(9)
+#define GP_IRQ5			BIT(10)
+#define DM_I3C_CB_ERR		GENMASK(10, 5)
+
+#define I3C_AUTO_SUSPEND_DELAY	250
+#define PACKING_BYTES_PER_WORD	4
+#define XFER_TIMEOUT		250
+
+#define I3C_ADDR_MASK		(I2C_MAX_ADDR >> 1)
+
+enum geni_i3c_err_code {
+	RD_TERM,
+	NACK,
+	CRC_ERR,
+	BUS_PROTO,
+	NACK_7E,
+	NACK_IBI,
+	GENI_OVERRUN,
+	GENI_ILLEGAL_CMD,
+	GENI_ABORT_DONE,
+	GENI_TIMEOUT,
+};
+
+enum i3c_bus_phase {
+	OPEN_DRAIN_MODE,
+	PUSH_PULL_MODE
+};
+
+struct geni_i3c_dev {
+	struct geni_se se;
+	unsigned int tx_wm;
+	unsigned int tx_depth;
+	int irq;
+	int err;
+	struct i3c_master_controller ctrlr;
+	struct completion done;
+	/* Protects per device CCC command or transfer from get_mutex_lock()/unlock() wrapper */
+	struct mutex lock;
+	/* Per device protection between process and IRQ context */
+	spinlock_t irq_lock;
+	u32 clk_src_freq;
+	u8 *cur_buf;
+	u8 last_addr;
+	bool cur_is_write;
+	int cur_len;
+	int cur_idx;
+	DECLARE_BITMAP(newaddrslots, 64);
+
+	const struct geni_i3c_clk_settings *clk_cfg;
+	const struct geni_i3c_clk_settings *clk_od_cfg;
+};
+
+struct geni_i3c_i2c_dev_data {
+	u32 ibi_keeping;  /* Plan to save IBI information, keep as dummy for now */
+};
+
+struct geni_i3c_xfer_params {
+	enum geni_se_xfer_mode mode;
+	u32 m_cmd;
+	u32 m_param;
+};
+
+static struct geni_i3c_dev *to_geni_i3c_master(struct i3c_master_controller
+							*master)
+{
+	return container_of(master, struct geni_i3c_dev, ctrlr);
+}
+
+struct geni_i3c_clk_settings {
+	u32 clk_freq_out;
+	u32 clk_src_freq;
+	u8  clk_div;
+	u8  i2c_t_high_cnt;
+	u8  i2c_t_low_cnt;
+	u8  i3c_t_high_cnt;
+	u8  i3c_t_cycle_cnt;
+	u8  i2c_t_cycle_cnt;
+};
+
+/*
+ * The hardware uses the following formulas to calculate the time periods
+ * of the SCL clock cycle. The firmware adds a few extra cycles that are not
+ * included in the formulas below. It has been verified that the resulting
+ * timings remain within the I2C/I3C specification limits.
+ *
+ * I2C SCL high period:
+ *     i2c_t_high = (i2c_t_high_cnt * clk_div) / source_clock
+ *
+ * I2C SCL low period:
+ *     i2c_t_low = (i2c_t_low_cnt * clk_div) / source_clock
+ *
+ * I2C SCL full cycle:
+ *     i2c_t_cycle = (i2c_t_cycle_cnt * clk_div) / source_clock
+ *
+ * I3C SCL high period:
+ *     i3c_t_high = (i3c_t_high_cnt * clk_div) / source_clock
+ *
+ * I3C SCL full cycle:
+ *     i3c_t_cycle = (i3c_t_cycle_cnt * clk_div) / source_clock
+ *
+ * Output clock frequency:
+ *     clk_freq_out = t / t_cycle
+ */
+static const struct geni_i3c_clk_settings geni_i3c_clk_map[] = {
+	{
+		.clk_freq_out = 100 * HZ_PER_KHZ,
+		.clk_src_freq = 19200 * HZ_PER_KHZ,
+		.clk_div = 1,
+		.i2c_t_high_cnt = 76,
+		.i2c_t_low_cnt = 90,
+		.i3c_t_high_cnt = 7,
+		.i3c_t_cycle_cnt = 8,
+		.i2c_t_cycle_cnt = 192,
+	},
+	{
+		.clk_freq_out = 400 * HZ_PER_KHZ,
+		.clk_src_freq = 19200 * HZ_PER_KHZ,
+		.clk_div = 1,
+		.i2c_t_high_cnt = 12,
+		.i2c_t_low_cnt = 24,
+		.i3c_t_high_cnt = 7,
+		.i3c_t_cycle_cnt = 8,
+		.i2c_t_cycle_cnt = 48
+	},
+	{
+		.clk_freq_out = 1000 * HZ_PER_KHZ,
+		.clk_src_freq = 19200 * HZ_PER_KHZ,
+		.clk_div = 1,
+		.i2c_t_high_cnt = 4,
+		.i2c_t_low_cnt = 9,
+		.i3c_t_high_cnt = 7,
+		.i3c_t_cycle_cnt = 0,
+		.i2c_t_cycle_cnt = 19
+	},
+	{
+		.clk_freq_out = 12500 * HZ_PER_KHZ,
+		.clk_src_freq = 100000 * HZ_PER_KHZ,
+		.clk_div = 1,
+		.i2c_t_high_cnt = 45,
+		.i2c_t_low_cnt = 63,
+		.i3c_t_high_cnt = 6,
+		.i3c_t_cycle_cnt = 7,
+		.i2c_t_cycle_cnt = 110
+	}
+};
+
+static int geni_i3c_clk_map_idx(struct geni_i3c_dev *gi3c)
+{
+	const struct geni_i3c_clk_settings *clk_table_idx = geni_i3c_clk_map;
+	struct i3c_master_controller *m = &gi3c->ctrlr;
+	struct i3c_bus *bus = i3c_master_get_bus(m);
+
+	for (int i = 0; i < ARRAY_SIZE(geni_i3c_clk_map); i++, clk_table_idx++) {
+		if (clk_table_idx->clk_freq_out == bus->scl_rate.i3c &&
+		    clk_table_idx->clk_src_freq == gi3c->clk_src_freq)
+			gi3c->clk_cfg = clk_table_idx;
+
+		if (clk_table_idx->clk_freq_out == bus->scl_rate.i2c)
+			gi3c->clk_od_cfg = clk_table_idx;
+	}
+
+	if (!gi3c->clk_cfg || !gi3c->clk_od_cfg)
+		return -EINVAL;
+
+	return 0;
+}
+
+static void qcom_geni_i3c_conf(struct geni_i3c_dev *gi3c, enum i3c_bus_phase bus_phase)
+{
+	const struct geni_i3c_clk_settings *clk_idx = gi3c->clk_cfg;
+	unsigned long freq;
+	u32 val, dfs_idx;
+	int ret;
+
+	if (bus_phase == OPEN_DRAIN_MODE)
+		clk_idx = gi3c->clk_od_cfg;
+
+	ret = geni_se_clk_freq_match(&gi3c->se, clk_idx->clk_src_freq,
+				     &dfs_idx, &freq, false);
+	if (ret)
+		dfs_idx = 0;
+
+	writel_relaxed(dfs_idx, gi3c->se.base + SE_GENI_CLK_SEL);
+
+	val = FIELD_PREP(CLK_DIV_VALUE_MASK, clk_idx->clk_div);
+	val |= SER_CLK_EN;
+	writel_relaxed(val, gi3c->se.base + GENI_SER_M_CLK_CFG);
+
+	val = FIELD_PREP(I2C_SCL_HIGH_COUNTER_MASK, clk_idx->i2c_t_high_cnt);
+	val |= FIELD_PREP(I2C_SCL_LOW_COUNTER_MASK, clk_idx->i2c_t_low_cnt);
+	val |= FIELD_PREP(I2C_SCL_CYCLE_COUNTER_MASK, clk_idx->i2c_t_cycle_cnt);
+	writel_relaxed(val, gi3c->se.base + SE_I2C_SCL_COUNTERS);
+
+	writel_relaxed(clk_idx->i3c_t_cycle_cnt, gi3c->se.base + SE_I3C_SCL_CYCLE);
+	writel_relaxed(clk_idx->i3c_t_high_cnt, gi3c->se.base + SE_I3C_SCL_HIGH);
+
+	writel_relaxed(M_IBI_IRQ_IGNORE, gi3c->se.base + SE_GENI_HW_IRQ_IGNORE_ON_ACTIVE);
+
+	val = M_IBI_IRQ_PARAM_STOP_STALL | M_IBI_IRQ_PARAM_7E;
+	writel_relaxed(val, gi3c->se.base + SE_GENI_HW_IRQ_CMD_PARAM_0);
+
+	writel_relaxed(M_IBI_IRQ_EN, gi3c->se.base + SE_GENI_HW_IRQ_EN);
+}
+
+static void geni_i3c_handle_err(struct geni_i3c_dev *gi3c, u32 status)
+{
+	dev_dbg(gi3c->se.dev, "len:%d, %s\n",
+		gi3c->cur_len, gi3c->cur_is_write ? "write" : "read");
+
+	if (status & M_GP_IRQ_0_EN) {
+		gi3c->err = -EINVAL;
+		dev_dbg(gi3c->se.dev, "I3C slave early read termination\n");
+	}
+
+	if (status & M_GP_IRQ_1_EN) {
+		gi3c->err = -ENOTCONN;
+		dev_dbg(gi3c->se.dev, "NACK: slave unresponsive, check power/reset\n");
+	}
+
+	if (status & M_GP_IRQ_2_EN) {
+		gi3c->err = -EINVAL;
+		dev_dbg(gi3c->se.dev, "CRC or parity error\n");
+	}
+
+	if (status & M_GP_IRQ_3_EN) {
+		gi3c->err = -EPROTO;
+		dev_dbg(gi3c->se.dev, "Bus proto Error, noisy/unexpected start/stop\n");
+	}
+
+	if (status & M_GP_IRQ_4_EN) {
+		gi3c->err = -EBUSY;
+		dev_dbg(gi3c->se.dev, "NACK on 7E, unexpected protocol error\n");
+	}
+
+	if (status & M_GP_IRQ_5_EN) {
+		gi3c->err = -EINVAL;
+		dev_dbg(gi3c->se.dev, "NACK on IBI\n");
+	}
+
+	if (status & M_CMD_OVERRUN_EN) {
+		gi3c->err = -EIO;
+		dev_dbg(gi3c->se.dev, "Cmd overrun, check GENI cmd-state machine\n");
+	}
+
+	if (status & M_ILLEGAL_CMD_EN) {
+		gi3c->err = -EILSEQ;
+		dev_dbg(gi3c->se.dev, "Illegal cmd, check GENI cmd-state machine\n");
+	}
+
+	if (status & M_CMD_ABORT_EN) {
+		gi3c->err = -ETIMEDOUT;
+		dev_dbg(gi3c->se.dev, "Abort after timeout successful\n");
+	}
+
+	if (status == GENI_TIMEOUT) {
+		gi3c->err = -ETIMEDOUT;
+		dev_dbg(gi3c->se.dev, "I3C transaction timed out\n");
+	}
+}
+
+static irqreturn_t geni_i3c_irq(int irq, void *dev)
+{
+	struct geni_i3c_dev *gi3c = dev;
+	u32 m_stat_mask, dm_tx_st, dm_rx_st;
+	u32 m_stat, rx_st, dma_mode;
+
+	m_stat = readl_relaxed(gi3c->se.base + SE_GENI_M_IRQ_STATUS);
+	m_stat_mask = readl_relaxed(gi3c->se.base + SE_GENI_M_IRQ_EN);
+	rx_st = readl_relaxed(gi3c->se.base + SE_GENI_RX_FIFO_STATUS);
+	dm_tx_st = readl_relaxed(gi3c->se.base + SE_DMA_TX_IRQ_STAT);
+	dm_rx_st = readl_relaxed(gi3c->se.base + SE_DMA_RX_IRQ_STAT);
+	dma_mode = readl_relaxed(gi3c->se.base + SE_GENI_DMA_MODE_EN);
+
+	scoped_guard(spinlock, &gi3c->irq_lock) {
+		if (m_stat & SE_I3C_ERR || dm_rx_st & DM_I3C_CB_ERR) {
+			geni_i3c_handle_err(gi3c, m_stat);
+			/* Disable the TX Watermark interrupt to stop TX */
+			if (!dma_mode)
+				writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
+		} else if (dma_mode) {
+			dev_dbg(gi3c->se.dev, "i3c dma tx:0x%x, dma rx:0x%x\n", dm_tx_st, dm_rx_st);
+		} else if ((m_stat & (M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN)) &&
+			    !gi3c->cur_is_write && gi3c->cur_buf) {
+			/* FIFO RX handling */
+			u32 rxcnt = rx_st & RX_FIFO_WC_MSK;
+			int avail = rxcnt * 4;
+			int need  = gi3c->cur_len - gi3c->cur_idx;
+			int n     = min(need, avail);
+
+			if (n > 0) {
+				i3c_readl_fifo(gi3c->se.base + SE_GENI_RX_FIFOn,
+					       gi3c->cur_buf + gi3c->cur_idx, n);
+				gi3c->cur_idx += n;
+			}
+		} else if ((m_stat & M_TX_FIFO_WATERMARK_EN) &&
+			   gi3c->cur_is_write && gi3c->cur_buf) {
+			/* FIFO TX handling */
+			int avail = gi3c->tx_wm * 4;
+			int need  = gi3c->cur_len - gi3c->cur_idx;
+			int n     = min(need, avail);
+
+			if (n > 0) {
+				i3c_writel_fifo(gi3c->se.base + SE_GENI_TX_FIFOn,
+						(void *)(gi3c->cur_buf + gi3c->cur_idx), n);
+				gi3c->cur_idx += n;
+			}
+		}
+
+		if (!dma_mode && gi3c->cur_idx == gi3c->cur_len)
+			writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
+	}
+
+	if (m_stat)
+		writel_relaxed(m_stat, gi3c->se.base + SE_GENI_M_IRQ_CLEAR);
+
+	if (dma_mode) {
+		if (dm_tx_st)
+			writel_relaxed(dm_tx_st, gi3c->se.base + SE_DMA_TX_IRQ_CLR);
+		if (dm_rx_st)
+			writel_relaxed(dm_rx_st, gi3c->se.base + SE_DMA_RX_IRQ_CLR);
+	}
+
+	/* if this is err with done-bit not set, handle that through timeout. */
+	if (m_stat & (M_CMD_DONE_EN | M_CMD_ABORT_EN)) {
+		writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
+		complete(&gi3c->done);
+	} else if ((dm_tx_st & TX_DMA_DONE) || (dm_rx_st & (RX_DMA_DONE | RX_RESET_DONE))) {
+		complete(&gi3c->done);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static void geni_i3c_abort_xfer(struct geni_i3c_dev *gi3c)
+{
+	unsigned long time_remaining;
+	unsigned long flags;
+
+	reinit_completion(&gi3c->done);
+	spin_lock_irqsave(&gi3c->irq_lock, flags);
+	geni_i3c_handle_err(gi3c, GENI_TIMEOUT);
+	geni_se_abort_m_cmd(&gi3c->se);
+	spin_unlock_irqrestore(&gi3c->irq_lock, flags);
+	time_remaining = wait_for_completion_timeout(&gi3c->done, XFER_TIMEOUT);
+	if (!time_remaining)
+		dev_err(gi3c->se.dev, "Timeout abort_m_cmd\n");
+}
+
+static int _i3c_geni_execute_command(struct geni_i3c_dev *gi3c, struct geni_i3c_xfer_params *xfer)
+{
+	bool is_write = gi3c->cur_is_write;
+	dma_addr_t tx_dma = 0, rx_dma = 0;
+	unsigned long time_remaining;
+	u32 len = gi3c->cur_len;
+	int ret;
+
+	geni_se_select_mode(&gi3c->se, xfer->mode);
+
+	gi3c->err = 0;
+	gi3c->cur_idx = 0;
+
+	if (is_write) {
+		dev_dbg(gi3c->se.dev, "I3C cmd:0x%x param:0x%x WRITE len:%d\n",
+			xfer->m_cmd, xfer->m_param, len);
+
+		writel_relaxed(len, gi3c->se.base + SE_I3C_TX_TRANS_LEN);
+		geni_se_setup_m_cmd(&gi3c->se, xfer->m_cmd, xfer->m_param);
+
+		if (xfer->mode == GENI_SE_DMA) {
+			ret = geni_se_tx_dma_prep(&gi3c->se, gi3c->cur_buf, len, &tx_dma);
+			if (ret) {
+				xfer->mode = GENI_SE_FIFO;
+				geni_se_select_mode(&gi3c->se, xfer->mode);
+			}
+		}
+
+		if (xfer->mode == GENI_SE_FIFO && len > 0) /* Get FIFO IRQ */
+			writel_relaxed(1, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
+	} else {
+		dev_dbg(gi3c->se.dev, "I3C cmd:0x%x param:0x%x READ len:%d\n",
+			xfer->m_cmd, xfer->m_param, len);
+		writel_relaxed(len, gi3c->se.base + SE_I3C_RX_TRANS_LEN);
+		geni_se_setup_m_cmd(&gi3c->se, xfer->m_cmd, xfer->m_param);
+		if (xfer->mode == GENI_SE_DMA) {
+			ret = geni_se_rx_dma_prep(&gi3c->se, gi3c->cur_buf, len, &rx_dma);
+			if (ret) {
+				xfer->mode = GENI_SE_FIFO;
+				geni_se_select_mode(&gi3c->se, xfer->mode);
+			}
+		}
+	}
+
+	time_remaining = wait_for_completion_timeout(&gi3c->done, XFER_TIMEOUT);
+	if (!time_remaining) {
+		unsigned long flags;
+
+		dev_dbg(gi3c->se.dev, "Timeout completing transfer\n");
+		spin_lock_irqsave(&gi3c->irq_lock, flags);
+		geni_i3c_handle_err(gi3c, GENI_TIMEOUT);
+		gi3c->cur_buf = NULL;
+		spin_unlock_irqrestore(&gi3c->irq_lock, flags);
+		geni_i3c_abort_xfer(gi3c);
+	}
+
+	if (xfer->mode == GENI_SE_DMA) {
+		if (gi3c->err) {
+			if (is_write)
+				writel_relaxed(1, gi3c->se.base + SE_DMA_TX_FSM_RST);
+			else
+				writel_relaxed(1, gi3c->se.base + SE_DMA_RX_FSM_RST);
+
+			time_remaining = wait_for_completion_timeout(&gi3c->done, XFER_TIMEOUT);
+			dev_dbg(gi3c->se.dev, "Timeout completing DMA transfer\n");
+		}
+
+		if (is_write)
+			geni_se_tx_dma_unprep(&gi3c->se, tx_dma, len);
+		else
+			geni_se_rx_dma_unprep(&gi3c->se, rx_dma, len);
+	}
+
+	ret = gi3c->err;
+	if (gi3c->err)
+		dev_err(gi3c->se.dev, "I3C transaction error :%d\n", gi3c->err);
+
+	gi3c->cur_buf = NULL;
+	gi3c->err = 0;
+
+	return ret;
+}
+
+static int i3c_geni_execute_read_command(struct geni_i3c_dev *gi3c,
+					 struct geni_i3c_xfer_params *xfer, u8 *buf, u32 len)
+{
+	gi3c->cur_is_write = false;
+	gi3c->cur_buf = buf;
+	gi3c->cur_len = len;
+
+	return _i3c_geni_execute_command(gi3c, xfer);
+}
+
+static int i3c_geni_execute_write_command(struct geni_i3c_dev *gi3c,
+					  struct geni_i3c_xfer_params *xfer, u8 *buf, u32 len)
+{
+	gi3c->cur_is_write = true;
+	gi3c->cur_buf = buf;
+	gi3c->cur_len = len;
+
+	return _i3c_geni_execute_command(gi3c, xfer);
+}
+
+static int geni_i3c_perform_daa(struct geni_i3c_dev *gi3c)
+{
+	struct geni_i3c_xfer_params xfer = { GENI_SE_FIFO };
+	bool first_device = true;
+	int ret;
+
+	while (1) {
+		u8 rx_buf[8], tx_buf[1];
+		u8 addr;
+		u64 pid;
+
+		/* Read PID[47:0], BCR, DCR during ENTDAA flow */
+		xfer.m_cmd = I2C_READ;
+		xfer.m_param = STOP_STRETCH | CONTINUOUS_MODE_DAA;
+
+		if (first_device) {
+			xfer.m_param |= USE_7E;
+			first_device = false;
+		}
+
+		ret = i3c_geni_execute_read_command(gi3c, &xfer,
+						    rx_buf, sizeof(rx_buf));
+		if (ret) {
+			dev_dbg(gi3c->se.dev,
+				"DAA complete, no more slave devices\n");
+			break;
+		}
+
+		pid = FIELD_PREP(GENMASK_ULL(47, 40), (u64)rx_buf[0]) |
+		      FIELD_PREP(GENMASK_ULL(39, 32), (u64)rx_buf[1]) |
+		      FIELD_PREP(GENMASK_ULL(31, 24), (u64)rx_buf[2]) |
+		      FIELD_PREP(GENMASK_ULL(23, 16), (u64)rx_buf[3]) |
+		      FIELD_PREP(GENMASK_ULL(15, 8),  (u64)rx_buf[4]) |
+		      FIELD_PREP(GENMASK_ULL(7, 0),   (u64)rx_buf[5]);
+
+		dev_dbg(gi3c->se.dev, "DAA device PID: 0x%012llx\n", pid);
+
+		addr = i3c_master_get_free_addr(&gi3c->ctrlr,
+						gi3c->last_addr + 1);
+		if (addr > I3C_ADDR_MASK)
+			return -EINVAL;
+
+		gi3c->last_addr = addr;
+		set_bit(addr, gi3c->newaddrslots);
+
+		/* Dynamic address + odd parity bit */
+		tx_buf[0] = (addr & I3C_ADDR_MASK) << 1;
+		tx_buf[0] |= !parity8(addr & I3C_ADDR_MASK);
+
+		xfer.m_cmd = I2C_WRITE;
+		xfer.m_param = STOP_STRETCH | BYPASS_ADDR_PHASE;
+
+		ret = i3c_geni_execute_write_command(gi3c, &xfer, tx_buf, 1);
+		if (ret)
+			break;
+	}
+
+	return 0;
+}
+
+static int geni_i3c_master_send_ccc_cmd(struct i3c_master_controller *m,
+					struct i3c_ccc_cmd *cmd)
+{
+	struct geni_i3c_dev *gi3c = to_geni_i3c_master(m);
+	int ret;
+
+	if (!(cmd->id & I3C_CCC_DIRECT) && cmd->ndests != 1)
+		return -EINVAL;
+
+	scoped_guard(mutex, &gi3c->lock) {
+		reinit_completion(&gi3c->done);
+		ret = pm_runtime_get_sync(gi3c->se.dev);
+		if (ret < 0) {
+			dev_err(gi3c->se.dev, "error turning on SE resources:%d\n", ret);
+			pm_runtime_put_noidle(gi3c->se.dev);
+			pm_runtime_set_suspended(gi3c->se.dev);
+			return ret;
+		}
+
+		qcom_geni_i3c_conf(gi3c, OPEN_DRAIN_MODE);
+		for (int i = 0; i < cmd->ndests; i++) {
+			int stall = (i < (cmd->ndests - 1)) ||
+				(cmd->id == I3C_CCC_ENTDAA);
+			struct geni_i3c_xfer_params xfer = { GENI_SE_FIFO };
+
+			xfer.m_param  = (stall ? STOP_STRETCH : 0);
+			xfer.m_param |= FIELD_PREP(CCC_HDR_CMD_MSK, cmd->id);
+			xfer.m_param |= IBI_NACK_TBL_CTRL;
+			if (cmd->id & I3C_CCC_DIRECT) {
+				xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, cmd->dests[i].addr);
+				if (cmd->rnw) {
+					if (i == 0)
+						xfer.m_cmd = I3C_DIRECT_CCC_READ;
+					else
+						xfer.m_cmd = I3C_PRIVATE_READ;
+				} else {
+					if (i == 0)
+						xfer.m_cmd =
+						   (cmd->dests[i].payload.len > 0) ?
+							I3C_DIRECT_CCC_WRITE :
+							I3C_DIRECT_CCC_ADDR_ONLY;
+					else
+						xfer.m_cmd = I3C_PRIVATE_WRITE;
+				}
+			} else {
+				if (cmd->dests[i].payload.len > 0)
+					xfer.m_cmd = I3C_BCAST_CCC_WRITE;
+				else
+					xfer.m_cmd = I3C_BCAST_CCC_ADDR_ONLY;
+			}
+
+			if (i == 0)
+				xfer.m_param |= USE_7E;
+
+			if (cmd->rnw) {
+				ret = i3c_geni_execute_read_command(gi3c, &xfer,
+								    cmd->dests[i].payload.data,
+								    cmd->dests[i].payload.len);
+				/*
+				 * Report bytes actually received so the core's
+				 * i3c_ccc_validate_payload_len() doesn't mistake a
+				 * successful short/expected-length read for -EIO.
+				 */
+				if (!ret)
+					cmd->dests[i].payload.actual_len = gi3c->cur_idx;
+			} else {
+				ret = i3c_geni_execute_write_command(gi3c, &xfer,
+								     cmd->dests[i].payload.data,
+								     cmd->dests[i].payload.len);
+			}
+			if (ret)
+				break;
+
+			if (cmd->id == I3C_CCC_ENTDAA)
+				geni_i3c_perform_daa(gi3c);
+		}
+
+		pm_runtime_mark_last_busy(gi3c->se.dev);
+		pm_runtime_put_autosuspend(gi3c->se.dev);
+		if (ret) {
+			/*
+			 * geni_i3c_handle_err() maps NACK conditions (7E NACK or an
+			 * unresponsive/absent target) to -EBUSY/-ENOTCONN. Report
+			 * those as I3C_ERROR_M2, matching how cdns/dw/svc/amd i3c
+			 * masters flag NACK/no-target errors: the core already
+			 * tolerates M2 for broadcast CCCs like ENTDAA/RSTDAA (see
+			 * i3c_master_entdaa_locked()/i3c_master_rstdaa_locked() in
+			 * master.c) instead of failing device/bus init outright.
+			 * Other error codes (timeout, protocol/CRC error, illegal
+			 * cmd, overrun) are left as I3C_ERROR_UNKNOWN so they still
+			 * propagate as real failures.
+			 */
+			if (ret == -ENOTCONN || ret == -EBUSY)
+				cmd->err = I3C_ERROR_M2;
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+static int geni_i3c_master_priv_xfers(struct i3c_dev_desc *dev, struct i3c_xfer *xfers,
+				      int nxfers, enum i3c_xfer_mode mode)
+{
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct geni_i3c_dev *gi3c = to_geni_i3c_master(m);
+	bool use_7e = false;
+	int ret;
+
+	scoped_guard(mutex, &gi3c->lock) {
+		reinit_completion(&gi3c->done);
+		ret = pm_runtime_get_sync(gi3c->se.dev);
+		if (ret < 0) {
+			dev_err(gi3c->se.dev, "error turning on SE resources:%d\n", ret);
+			pm_runtime_put_noidle(gi3c->se.dev);
+			pm_runtime_set_suspended(gi3c->se.dev);
+			return ret;
+		}
+
+		qcom_geni_i3c_conf(gi3c, PUSH_PULL_MODE);
+
+		for (int i = 0; i < nxfers; i++) {
+			bool stall = (i < (nxfers - 1));
+			struct geni_i3c_xfer_params xfer = { GENI_SE_FIFO };
+
+			xfer.m_param  = (stall ? STOP_STRETCH : 0);
+			xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, dev->info.dyn_addr);
+			xfer.m_param |= (use_7e) ? USE_7E : 0;
+
+			/* use_7e = true only for last transfer */
+			use_7e = (i == nxfers - 1);
+
+			if (xfers[i].rnw) {
+				xfer.m_cmd = I3C_PRIVATE_READ;
+				ret = i3c_geni_execute_read_command(gi3c, &xfer,
+								    (u8 *)xfers[i].data.in,
+								    xfers[i].len);
+			} else {
+				xfer.m_cmd = I3C_PRIVATE_WRITE;
+				ret = i3c_geni_execute_write_command(gi3c, &xfer,
+								     (u8 *)xfers[i].data.out,
+								     xfers[i].len);
+			}
+
+			if (ret)
+				break;
+		}
+
+		dev_dbg(gi3c->se.dev, "i3c priv: txn ret:%d\n", ret);
+		pm_runtime_mark_last_busy(gi3c->se.dev);
+		pm_runtime_put_autosuspend(gi3c->se.dev);
+	}
+
+	return ret;
+}
+
+static int geni_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, struct i2c_msg *msgs, int num)
+{
+	struct i3c_master_controller *m = i2c_dev_get_master(dev);
+	struct geni_i3c_dev *gi3c = to_geni_i3c_master(m);
+	int ret;
+
+	guard(mutex)(&gi3c->lock);
+	reinit_completion(&gi3c->done);
+
+	ret = pm_runtime_get_sync(gi3c->se.dev);
+	if (ret < 0) {
+		dev_err(gi3c->se.dev, "Error turning on SE resources:%d\n", ret);
+		pm_runtime_put_noidle(gi3c->se.dev);
+		pm_runtime_set_suspended(gi3c->se.dev);
+		return ret;
+	}
+
+	qcom_geni_i3c_conf(gi3c, OPEN_DRAIN_MODE);
+
+	for (int i = 0; i < num; i++) {
+		struct geni_i3c_xfer_params xfer;
+
+		xfer.m_cmd    = (msgs[i].flags & I2C_M_RD) ? I2C_READ : I2C_WRITE;
+		xfer.m_param  = (i < (num - 1)) ? STOP_STRETCH : 0;
+		xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, msgs[i].addr);
+		xfer.mode     = msgs[i].len > (gi3c->tx_depth / 2) ?
+				GENI_SE_DMA : GENI_SE_FIFO;
+		if (msgs[i].flags & I2C_M_RD)
+			ret = i3c_geni_execute_read_command(gi3c, &xfer, msgs[i].buf,
+							    msgs[i].len);
+		else
+			ret = i3c_geni_execute_write_command(gi3c, &xfer, msgs[i].buf,
+							     msgs[i].len);
+		if (ret)
+			break;
+	}
+
+	dev_dbg(gi3c->se.dev, "i2c: txn ret:%d\n", ret);
+	pm_runtime_mark_last_busy(gi3c->se.dev);
+	pm_runtime_put_autosuspend(gi3c->se.dev);
+
+	return ret;
+}
+
+static int geni_i3c_master_attach_i2c_dev(struct i2c_dev_desc *dev)
+{
+	struct geni_i3c_i2c_dev_data *data;
+
+	data = kzalloc_obj(*data, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	i2c_dev_set_master_data(dev, data);
+
+	return 0;
+}
+
+static void geni_i3c_master_detach_i2c_dev(struct i2c_dev_desc *dev)
+{
+	struct geni_i3c_i2c_dev_data *data = i2c_dev_get_master_data(dev);
+
+	i2c_dev_set_master_data(dev, NULL);
+	kfree(data);
+}
+
+static int geni_i3c_master_attach_i3c_dev(struct i3c_dev_desc *dev)
+{
+	struct geni_i3c_i2c_dev_data *data;
+
+	data = kzalloc_obj(*data, GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	i3c_dev_set_master_data(dev, data);
+
+	return 0;
+}
+
+static void geni_i3c_master_detach_i3c_dev(struct i3c_dev_desc *dev)
+{
+	struct geni_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+
+	i3c_dev_set_master_data(dev, NULL);
+	kfree(data);
+}
+
+static int geni_i3c_master_do_daa(struct i3c_master_controller *m)
+{
+	struct geni_i3c_dev *gi3c = to_geni_i3c_master(m);
+	int ret;
+
+	/*
+	 * i3c_master_entdaa_locked() already swallows I3C_ERROR_M2 (no
+	 * active devices needing an address) and returns 0 for that case,
+	 * so any non-zero ret here is a real error.
+	 */
+	ret = i3c_master_entdaa_locked(m);
+	if (ret)
+		return ret;
+
+	for (u8 addr = 0; addr <= I3C_ADDR_MASK; addr++) {
+		if (__test_and_clear_bit(addr, gi3c->newaddrslots))
+			i3c_master_add_i3c_dev_locked(m, addr);
+	}
+
+	return 0;
+}
+
+static int geni_i3c_master_bus_init(struct i3c_master_controller *m)
+{
+	struct geni_i3c_dev *gi3c = to_geni_i3c_master(m);
+	struct i3c_bus *bus = i3c_master_get_bus(m);
+	struct i3c_device_info info = { };
+	int ret;
+
+	/* Get an address for the master. */
+	ret = i3c_master_get_free_addr(m, 0);
+	if (ret < 0)
+		return dev_err_probe(gi3c->se.dev, ret, "Failed to get master address\n");
+
+	info.dyn_addr = ret;
+	info.dcr = I3C_DCR_GENERIC_DEVICE;
+	info.bcr = I3C_BCR_I3C_MASTER | I3C_BCR_HDR_CAP;
+	info.pid = 0;
+
+	ret = geni_i3c_clk_map_idx(gi3c);
+	if (ret) {
+		dev_err(gi3c->se.dev,
+			"Invalid clk frequency %d Hz src for %ld Hz bus: %d\n",
+			gi3c->clk_src_freq, bus->scl_rate.i3c, ret);
+		return ret;
+	}
+
+	scoped_guard(mutex, &gi3c->lock) {
+		reinit_completion(&gi3c->done);
+		ret = pm_runtime_resume_and_get(gi3c->se.dev);
+		if (ret < 0)
+			return dev_err_probe(gi3c->se.dev, ret, "Failed to resume controller\n");
+
+		qcom_geni_i3c_conf(gi3c, OPEN_DRAIN_MODE);
+
+		ret = i3c_master_set_info(&gi3c->ctrlr, &info);
+		pm_runtime_put_autosuspend(gi3c->se.dev);
+	}
+
+	return ret;
+}
+
+static bool geni_i3c_master_supports_ccc_cmd(struct i3c_master_controller *m,
+					     const struct i3c_ccc_cmd *cmd)
+{
+	switch (cmd->id) {
+	case I3C_CCC_ENEC(true):
+	case I3C_CCC_ENEC(false):
+	case I3C_CCC_DISEC(true):
+	case I3C_CCC_DISEC(false):
+	case I3C_CCC_ENTAS(0, true):
+	case I3C_CCC_ENTAS(0, false):
+	case I3C_CCC_RSTDAA(true):
+	case I3C_CCC_RSTDAA(false):
+	case I3C_CCC_ENTDAA:
+	case I3C_CCC_SETMWL(true):
+	case I3C_CCC_SETMWL(false):
+	case I3C_CCC_SETMRL(true):
+	case I3C_CCC_SETMRL(false):
+	case I3C_CCC_DEFSLVS:
+	case I3C_CCC_SETDASA:
+	case I3C_CCC_SETNEWDA:
+	case I3C_CCC_GETMWL:
+	case I3C_CCC_GETMRL:
+	case I3C_CCC_GETPID:
+	case I3C_CCC_GETBCR:
+	case I3C_CCC_GETDCR:
+	case I3C_CCC_GETSTATUS:
+	case I3C_CCC_GETACCMST:
+	case I3C_CCC_GETMXDS:
+	case I3C_CCC_GETHDRCAP:
+		return true;
+
+	default:
+		return false;
+	}
+}
+
+static const struct i3c_master_controller_ops geni_i3c_master_ops = {
+	.bus_init = geni_i3c_master_bus_init,
+	.do_daa = geni_i3c_master_do_daa,
+	.attach_i3c_dev = geni_i3c_master_attach_i3c_dev,
+	.detach_i3c_dev = geni_i3c_master_detach_i3c_dev,
+	.attach_i2c_dev = geni_i3c_master_attach_i2c_dev,
+	.detach_i2c_dev = geni_i3c_master_detach_i2c_dev,
+	.supports_ccc_cmd = geni_i3c_master_supports_ccc_cmd,
+	.send_ccc_cmd = geni_i3c_master_send_ccc_cmd,
+	.i3c_xfers = geni_i3c_master_priv_xfers,
+	.i2c_xfers = geni_i3c_master_i2c_xfers,
+};
+
+static int i3c_geni_resources_init(struct geni_i3c_dev *gi3c, struct platform_device *pdev)
+{
+	int ret;
+
+	gi3c->se.base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(gi3c->se.base))
+		return PTR_ERR(gi3c->se.base);
+
+	gi3c->se.clk = devm_clk_get(&pdev->dev, NULL);
+	if (IS_ERR(gi3c->se.clk))
+		return dev_err_probe(&pdev->dev, PTR_ERR(gi3c->se.clk),
+							"Unable to get serial engine core clock: %pe\n",
+							gi3c->se.clk);
+	ret = geni_icc_get(&gi3c->se);
+	if (ret)
+		return ret;
+
+	/* Set the bus quota to a reasonable value for register access */
+	gi3c->se.icc_paths[GENI_TO_CORE].avg_bw = GENI_DEFAULT_BW;
+	gi3c->se.icc_paths[CPU_TO_GENI].avg_bw = GENI_DEFAULT_BW;
+	ret = geni_icc_set_bw(&gi3c->se);
+	if (ret)
+		return ret;
+
+	/* Default source clock (se-clock-frequency) freq is 100Mhz */
+	gi3c->clk_src_freq = HZ_PER_MHZ * 100;
+
+	return 0;
+}
+
+static int geni_i3c_hw_init(struct geni_i3c_dev *gi3c, struct platform_device *pdev)
+{
+	u32 proto, fifo_disable;
+
+	proto = geni_se_read_proto(&gi3c->se);
+	if (proto != GENI_SE_I3C) {
+		geni_se_resources_off(&gi3c->se);
+		return dev_err_probe(gi3c->se.dev, -ENXIO, "Invalid proto %d\n", proto);
+	}
+
+	fifo_disable = readl_relaxed(gi3c->se.base + GENI_IF_DISABLE_RO);
+	if (fifo_disable) {
+		geni_se_resources_off(&gi3c->se);
+		return dev_err_probe(gi3c->se.dev, -ENXIO, "GPI DMA mode not supported\n");
+	}
+
+	gi3c->tx_depth = geni_se_get_tx_fifo_depth(&gi3c->se);
+	gi3c->tx_wm = gi3c->tx_depth - 1;
+	geni_se_init(&gi3c->se, gi3c->tx_wm, gi3c->tx_depth);
+	geni_se_config_packing(&gi3c->se, BITS_PER_BYTE, PACKING_BYTES_PER_WORD, true, true, true);
+
+	return 0;
+}
+
+static void geni_i3c_unregister(void *data)
+{
+	struct geni_i3c_dev *gi3c = data;
+
+	i3c_master_unregister(&gi3c->ctrlr);
+}
+
+static int geni_i3c_probe(struct platform_device *pdev)
+{
+	struct geni_i3c_dev *gi3c;
+	int ret;
+
+	gi3c = devm_kzalloc(&pdev->dev, sizeof(*gi3c), GFP_KERNEL);
+	if (!gi3c)
+		return -ENOMEM;
+
+	gi3c->se.dev = &pdev->dev;
+	gi3c->se.wrapper = dev_get_drvdata(pdev->dev.parent);
+
+	init_completion(&gi3c->done);
+	ret = devm_mutex_init(&pdev->dev, &gi3c->lock);
+	if (ret)
+		return ret;
+
+	spin_lock_init(&gi3c->irq_lock);
+	platform_set_drvdata(pdev, gi3c);
+
+	ret = i3c_geni_resources_init(gi3c, pdev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Error Initializing GENI Resources\n");
+
+	gi3c->irq = platform_get_irq(pdev, 0);
+	if (gi3c->irq < 0)
+		return dev_err_probe(&pdev->dev, gi3c->irq, "Error getting IRQ number for I3C\n");
+
+	ret = devm_request_irq(&pdev->dev, gi3c->irq, geni_i3c_irq,
+			       IRQF_NO_AUTOEN, dev_name(&pdev->dev), gi3c);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Error registering core IRQ\n");
+
+	ret = geni_se_resources_on(&gi3c->se);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Error turning resources ON\n");
+
+	ret = geni_i3c_hw_init(gi3c, pdev);
+	geni_se_resources_off(&gi3c->se);
+	if (ret)
+		return ret;
+	dev_dbg(&pdev->dev, "i3c fifo/se-dma mode. fifo depth:%d\n", gi3c->tx_depth);
+
+	pm_runtime_set_suspended(gi3c->se.dev);
+	pm_runtime_set_autosuspend_delay(gi3c->se.dev, I3C_AUTO_SUSPEND_DELAY);
+	pm_runtime_use_autosuspend(gi3c->se.dev);
+	ret = devm_pm_runtime_enable(gi3c->se.dev);
+	if (ret)
+		return dev_err_probe(&pdev->dev, ret, "Failed to enable runtime PM\n");
+
+	ret = i3c_master_register(&gi3c->ctrlr, &pdev->dev, &geni_i3c_master_ops, false);
+	if (ret)
+		return ret;
+
+	ret = devm_add_action_or_reset(&pdev->dev, geni_i3c_unregister, gi3c);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+static int geni_i3c_runtime_suspend(struct device *dev)
+{
+	struct geni_i3c_dev *gi3c = dev_get_drvdata(dev);
+
+	disable_irq(gi3c->irq);
+	geni_se_resources_off(&gi3c->se);
+
+	return 0;
+}
+
+static int geni_i3c_runtime_resume(struct device *dev)
+{
+	int ret;
+	struct geni_i3c_dev *gi3c = dev_get_drvdata(dev);
+
+	ret = geni_se_resources_on(&gi3c->se);
+	if (ret)
+		return ret;
+	enable_irq(gi3c->irq);
+
+	return 0;
+}
+
+static DEFINE_RUNTIME_DEV_PM_OPS(geni_i3c_pm_ops,
+			     geni_i3c_runtime_suspend,
+			     geni_i3c_runtime_resume,
+			     NULL);
+
+static const struct of_device_id geni_i3c_dt_match[] = {
+	{ .compatible = "qcom,geni-i3c" },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, geni_i3c_dt_match);
+
+static struct platform_driver geni_i3c_master = {
+	.probe  = geni_i3c_probe,
+	.driver = {
+		.name = "geni_i3c",
+		.pm = pm_ptr(&geni_i3c_pm_ops),
+		.of_match_table = geni_i3c_dt_match,
+	},
+};
+
+module_platform_driver(geni_i3c_master);
+
+MODULE_AUTHOR("Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>");
+MODULE_DESCRIPTION("Qualcomm I3C Controller Driver for GENI based QUP cores");
+MODULE_LICENSE("GPL");
-- 
2.43.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 2/3] i3c: master: Add Qualcomm I3C controller driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 10 potential issue(s) to consider:
- [High] i3c: qcom-geni: use_7e updated after m_param in geni_i3c_master_priv_xfers()
- [High] i3c: qcom-geni: broken I3C_ADDR_MASK and DAA handling in geni_i3c_perform_daa()
- [High] i3c: qcom-geni: invalid clock lookup and 1 MHz cycle count in geni_i3c_clk_map
- [High] i3c: qcom-geni: unsafe I2C DMA mapping and broken DMA reset handling
- [High] i3c: qcom-geni: broken short RX FIFO reads and early termination in geni_i3c_irq()
- [High] i3c: qcom-geni: NACK error overwrite and status bitmask mismatch in geni_i3c_handle_err()
- [High] i3c: qcom-geni: missing reinit_completion() and TX_RESET_DONE in _i3c_geni_execute_command()
- [High] i3c: qcom-geni: TX watermark race and unhandled IRQ storm in geni_i3c_irq()
- [Medium] i3c: qcom-geni: double geni_se_resources_off() call in geni_i3c_probe()
- [Low] i3c: qcom-geni: unused variable m_stat_mask in geni_i3c_irq()
--

commit 2ad15e60246cd7898205f431d98abb7e053d2d97
Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>

i3c: master: Add Qualcomm I3C controller driver

This commit adds a driver for the Qualcomm QUPv3 GENI-based I3C master
controller supporting I3C SDR and legacy I2C transfers.

> diff --git a/drivers/i3c/master/i3c-qcom-geni.c b/drivers/i3c/master/i3c-qcom-geni.c
> new file mode 100644
> index 0000000000000..2fee77e681e26
> --- /dev/null
> +++ b/drivers/i3c/master/i3c-qcom-geni.c
[ ... ]
> +#define I3C_AUTO_SUSPEND_DELAY	250
> +#define PACKING_BYTES_PER_WORD	4
> +#define XFER_TIMEOUT		250
> +
> +#define I3C_ADDR_MASK		(I2C_MAX_ADDR >> 1)
[ ... ]
> +struct geni_i3c_dev {
[ ... ]
> +	u8 *cur_buf;
> +	u8 last_addr;
> +	bool cur_is_write;
> +	int cur_len;
> +	int cur_idx;
> +	DECLARE_BITMAP(newaddrslots, 64);
> +
> +	const struct geni_i3c_clk_settings *clk_cfg;
> +	const struct geni_i3c_clk_settings *clk_od_cfg;
> +};

[Severity: High]
Since I2C_MAX_ADDR is defined as GENMASK(6, 0) (0x7f) in
include/linux/i3c/master.h, shifting it right by 1 makes I3C_ADDR_MASK 0x3f
(6 bits instead of 7 bits), and newaddrslots is only 64 bits wide instead of
I3C_MAX_ADDR + 1 (128).

Later in geni_i3c_perform_daa():

	addr = i3c_master_get_free_addr(&gi3c->ctrlr,
					gi3c->last_addr + 1);
	if (addr > I3C_ADDR_MASK)
		return -EINVAL;

	gi3c->last_addr = addr;
	set_bit(addr, gi3c->newaddrslots);
	...
	ret = i3c_geni_execute_write_command(gi3c, &xfer, tx_buf, 1);
	if (ret)
		break;

Doesn't this reject valid 7-bit dynamic addresses from 0x40 to 0x7d (as well
as truncating negative error codes from i3c_master_get_free_addr() into u8)?

Also, in geni_i3c_perform_daa() and geni_i3c_master_send_ccc_cmd():
- gi3c->last_addr is never reset between DAA runs, so repeated DAA
  invocations will only search above the previous address until the range is
  exhausted.
- set_bit(addr, gi3c->newaddrslots) is called before
  i3c_geni_execute_write_command() sends the dynamic address to the target.
  If that write fails, geni_i3c_perform_daa() breaks out and returns 0 with
  addr still set in newaddrslots, causing geni_i3c_master_do_daa() to call
  i3c_master_add_i3c_dev_locked() for an unassigned address.
- geni_i3c_master_send_ccc_cmd() ignores the return value of
  geni_i3c_perform_daa(gi3c).

Should I3C_ADDR_MASK be I3C_MAX_ADDR (0x7f) with newaddrslots sized to
I3C_MAX_ADDR + 1, and should set_bit(addr, gi3c->newaddrslots) happen only
after the address write succeeds?

[ ... ]
> +static const struct geni_i3c_clk_settings geni_i3c_clk_map[] = {
> +	{
> +		.clk_freq_out = 100 * HZ_PER_KHZ,
> +		.clk_src_freq = 19200 * HZ_PER_KHZ,
> +		.clk_div = 1,
> +		.i2c_t_high_cnt = 76,
> +		.i2c_t_low_cnt = 90,
> +		.i3c_t_high_cnt = 7,
> +		.i3c_t_cycle_cnt = 8,
> +		.i2c_t_cycle_cnt = 192,
> +	},
> +	{
> +		.clk_freq_out = 400 * HZ_PER_KHZ,
> +		.clk_src_freq = 19200 * HZ_PER_KHZ,
> +		.clk_div = 1,
> +		.i2c_t_high_cnt = 12,
> +		.i2c_t_low_cnt = 24,
> +		.i3c_t_high_cnt = 7,
> +		.i3c_t_cycle_cnt = 8,
> +		.i2c_t_cycle_cnt = 48
> +	},
> +	{
> +		.clk_freq_out = 1000 * HZ_PER_KHZ,
> +		.clk_src_freq = 19200 * HZ_PER_KHZ,
> +		.clk_div = 1,
> +		.i2c_t_high_cnt = 4,
> +		.i2c_t_low_cnt = 9,
> +		.i3c_t_high_cnt = 7,
> +		.i3c_t_cycle_cnt = 0,
> +		.i2c_t_cycle_cnt = 19
> +	},
> +	{
> +		.clk_freq_out = 12500 * HZ_PER_KHZ,
> +		.clk_src_freq = 100000 * HZ_PER_KHZ,
> +		.clk_div = 1,
> +		.i2c_t_high_cnt = 45,
> +		.i2c_t_low_cnt = 63,
> +		.i3c_t_high_cnt = 6,
> +		.i3c_t_cycle_cnt = 7,
> +		.i2c_t_cycle_cnt = 110
> +	}
> +};
> +
> +static int geni_i3c_clk_map_idx(struct geni_i3c_dev *gi3c)
> +{
> +	const struct geni_i3c_clk_settings *clk_table_idx = geni_i3c_clk_map;
> +	struct i3c_master_controller *m = &gi3c->ctrlr;
> +	struct i3c_bus *bus = i3c_master_get_bus(m);
> +
> +	for (int i = 0; i < ARRAY_SIZE(geni_i3c_clk_map); i++, clk_table_idx++) {
> +		if (clk_table_idx->clk_freq_out == bus->scl_rate.i3c &&
> +		    clk_table_idx->clk_src_freq == gi3c->clk_src_freq)
> +			gi3c->clk_cfg = clk_table_idx;
> +
> +		if (clk_table_idx->clk_freq_out == bus->scl_rate.i2c)
> +			gi3c->clk_od_cfg = clk_table_idx;
> +	}
> +
> +	if (!gi3c->clk_cfg || !gi3c->clk_od_cfg)
> +		return -EINVAL;
> +
> +	return 0;
> +}

[Severity: High]
How does geni_i3c_clk_map_idx() behave on a pure I3C bus or a mixed-slow
bus?

In i3c_bus_set_mode(), when bus->mode is I3C_BUS_MODE_PURE and no i2c-scl-hz
property is set in devicetree, bus->scl_rate.i2c remains 0. Because no entry
in geni_i3c_clk_map[] has clk_freq_out == 0, gi3c->clk_od_cfg stays NULL and
geni_i3c_master_bus_init() fails with -EINVAL.

Conversely, in I3C_BUS_MODE_MIXED_SLOW (where i3c_bus_set_mode() clamps
bus->scl_rate.i3c to bus->scl_rate.i2c), i3c_geni_resources_init() hardcodes
gi3c->clk_src_freq to 100 MHz while the 100 kHz, 400 kHz, and 1000 kHz
entries in geni_i3c_clk_map[] all have clk_src_freq set to 19.2 MHz, so
gi3c->clk_cfg stays NULL and geni_i3c_clk_map_idx() also returns -EINVAL.

Also, in the 1000 kHz entry of geni_i3c_clk_map[], i3c_t_cycle_cnt is 0 while
i3c_t_high_cnt is 7 (whereas the 100 kHz and 400 kHz entries use
i3c_t_cycle_cnt = 8). Is programming SE_I3C_SCL_CYCLE to 0 when
qcom_geni_i3c_conf(gi3c, OPEN_DRAIN_MODE) selects the 1000 kHz entry
intentional?

[ ... ]
> +static void geni_i3c_handle_err(struct geni_i3c_dev *gi3c, u32 status)
> +{
> +	dev_dbg(gi3c->se.dev, "len:%d, %s\n",
> +		gi3c->cur_len, gi3c->cur_is_write ? "write" : "read");
> +
> +	if (status & M_GP_IRQ_0_EN) {
> +		gi3c->err = -EINVAL;
> +		dev_dbg(gi3c->se.dev, "I3C slave early read termination\n");
> +	}
[ ... ]
> +	if (status & M_CMD_ABORT_EN) {
> +		gi3c->err = -ETIMEDOUT;
> +		dev_dbg(gi3c->se.dev, "Abort after timeout successful\n");
> +	}
> +
> +	if (status == GENI_TIMEOUT) {
> +		gi3c->err = -ETIMEDOUT;
> +		dev_dbg(gi3c->se.dev, "I3C transaction timed out\n");
> +	}
> +}

[Severity: High]
Won't geni_i3c_handle_err() clobber an earlier NACK error (-ENOTCONN or
-EBUSY) with -ETIMEDOUT when a transfer times out and aborts?

When a NACK interrupt fires without M_CMD_DONE_EN set, geni_i3c_irq() records
-ENOTCONN or -EBUSY in gi3c->err and waits for the timeout path in
_i3c_geni_execute_command() to abort the command. Both the timeout path
(passing GENI_TIMEOUT) and the subsequent M_CMD_ABORT_EN interrupt overwrite
gi3c->err with -ETIMEDOUT, so geni_i3c_master_send_ccc_cmd() no longer sees
-ENOTCONN or -EBUSY and cannot set cmd->err = I3C_ERROR_M2.

In addition, status in geni_i3c_handle_err() is checked both as an
SE_GENI_M_IRQ_STATUS bitmask and for equality with the enum constant
GENI_TIMEOUT (which has value 9, or M_CMD_FAILURE_EN | M_CMD_DONE_EN). And
if geni_i3c_irq() enters the error branch due to dm_rx_st & DM_I3C_CB_ERR or
M_CMD_FAILURE_EN alone, geni_i3c_handle_err(gi3c, m_stat) leaves gi3c->err as
0. Should gi3c->err only be updated when it is not already set, with a
fallback error code when none of the specific M_GP_IRQ_* bits match?

> +
> +static irqreturn_t geni_i3c_irq(int irq, void *dev)
> +{
> +	struct geni_i3c_dev *gi3c = dev;
> +	u32 m_stat_mask, dm_tx_st, dm_rx_st;
> +	u32 m_stat, rx_st, dma_mode;
> +
> +	m_stat = readl_relaxed(gi3c->se.base + SE_GENI_M_IRQ_STATUS);
> +	m_stat_mask = readl_relaxed(gi3c->se.base + SE_GENI_M_IRQ_EN);

[Severity: Low]
In geni_i3c_irq(), m_stat_mask is assigned here but never used, which will
trigger an unused-but-set-variable compiler warning. Should m_stat be masked
with m_stat_mask, or should m_stat_mask be removed?

> +	rx_st = readl_relaxed(gi3c->se.base + SE_GENI_RX_FIFO_STATUS);
> +	dm_tx_st = readl_relaxed(gi3c->se.base + SE_DMA_TX_IRQ_STAT);
> +	dm_rx_st = readl_relaxed(gi3c->se.base + SE_DMA_RX_IRQ_STAT);
> +	dma_mode = readl_relaxed(gi3c->se.base + SE_GENI_DMA_MODE_EN);
> +
> +	scoped_guard(spinlock, &gi3c->irq_lock) {
> +		if (m_stat & SE_I3C_ERR || dm_rx_st & DM_I3C_CB_ERR) {
> +			geni_i3c_handle_err(gi3c, m_stat);
> +			/* Disable the TX Watermark interrupt to stop TX */
> +			if (!dma_mode)
> +				writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
> +		} else if (dma_mode) {
> +			dev_dbg(gi3c->se.dev, "i3c dma tx:0x%x, dma rx:0x%x\n", dm_tx_st, dm_rx_st);
> +		} else if ((m_stat & (M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN)) &&
> +			    !gi3c->cur_is_write && gi3c->cur_buf) {
> +			/* FIFO RX handling */
> +			u32 rxcnt = rx_st & RX_FIFO_WC_MSK;
> +			int avail = rxcnt * 4;
> +			int need  = gi3c->cur_len - gi3c->cur_idx;
> +			int n     = min(need, avail);
> +
> +			if (n > 0) {
> +				i3c_readl_fifo(gi3c->se.base + SE_GENI_RX_FIFOn,
> +					       gi3c->cur_buf + gi3c->cur_idx, n);
> +				gi3c->cur_idx += n;
> +			}

[Severity: High]
What happens in geni_i3c_irq() when an I3C target returns a valid short read
response (for example, a 2-byte response to a 5-byte GETMXDS or 3-byte GETMRL
CCC read) or terminates a read early via the T-bit?

First, RX_FIFO_WC_MSK in SE_GENI_RX_FIFO_STATUS counts 32-bit FIFO words, and
valid bytes in the final partial word are indicated by RX_LAST and
RX_LAST_BYTE_VALID_MSK. Computing avail = rxcnt * 4 rounds a 2-byte response
up to 4 bytes, so geni_i3c_master_send_ccc_cmd() sets
cmd->dests[i].payload.actual_len = gi3c->cur_idx to 4 (for GETMXDS, causing
i3c_master_getmxds_locked() to fail with -EIO) or 3 (for GETMRL, reading a
padding byte as max_ibi_len).

Second, M_GP_IRQ_0_EN ("I3C slave early read termination") is included in
SE_I3C_ERR and sets gi3c->err = -EINVAL in geni_i3c_handle_err(). Because
the FIFO RX handler is in an else-if branch after if (m_stat & SE_I3C_ERR ...),
an early-terminated read fails with -EINVAL and leaves the received words
unread in the RX FIFO.

> +		} else if ((m_stat & M_TX_FIFO_WATERMARK_EN) &&
> +			   gi3c->cur_is_write && gi3c->cur_buf) {
> +			/* FIFO TX handling */
> +			int avail = gi3c->tx_wm * 4;
> +			int need  = gi3c->cur_len - gi3c->cur_idx;
> +			int n     = min(need, avail);
> +
> +			if (n > 0) {
> +				i3c_writel_fifo(gi3c->se.base + SE_GENI_TX_FIFOn,
> +						(void *)(gi3c->cur_buf + gi3c->cur_idx), n);
> +				gi3c->cur_idx += n;
> +			}
> +		}
> +
> +		if (!dma_mode && gi3c->cur_idx == gi3c->cur_len)
> +			writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
> +	}

[Severity: High]
Can M_TX_FIFO_WATERMARK_EN cause an unhandled interrupt storm if gi3c->cur_buf
becomes NULL while gi3c->cur_idx < gi3c->cur_len?

In _i3c_geni_execute_command():

	geni_se_setup_m_cmd(&gi3c->se, xfer->m_cmd, xfer->m_param);
	...
	if (xfer->mode == GENI_SE_FIFO && len > 0) /* Get FIFO IRQ */
		writel_relaxed(1, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);

Because geni_se_setup_m_cmd() is called before enabling
SE_GENI_TX_WATERMARK_REG without holding gi3c->irq_lock, an immediate error
or abort interrupt can run first and clear SE_GENI_TX_WATERMARK_REG, after
which _i3c_geni_execute_command() writes 1 to SE_GENI_TX_WATERMARK_REG and
sets gi3c->cur_buf = NULL. Likewise, on a transfer timeout,
_i3c_geni_execute_command() sets gi3c->cur_buf = NULL without clearing
SE_GENI_TX_WATERMARK_REG.

When M_TX_FIFO_WATERMARK_EN then fires in geni_i3c_irq() with
gi3c->cur_buf == NULL and gi3c->cur_idx < gi3c->cur_len, neither the TX
branch nor the cur_idx == cur_len check disables SE_GENI_TX_WATERMARK_REG,
leaving the level-triggered watermark interrupt asserted continuously.

> +
> +	if (m_stat)
> +		writel_relaxed(m_stat, gi3c->se.base + SE_GENI_M_IRQ_CLEAR);
> +
> +	if (dma_mode) {
> +		if (dm_tx_st)
> +			writel_relaxed(dm_tx_st, gi3c->se.base + SE_DMA_TX_IRQ_CLR);
> +		if (dm_rx_st)
> +			writel_relaxed(dm_rx_st, gi3c->se.base + SE_DMA_RX_IRQ_CLR);
> +	}
> +
> +	/* if this is err with done-bit not set, handle that through timeout. */
> +	if (m_stat & (M_CMD_DONE_EN | M_CMD_ABORT_EN)) {
> +		writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
> +		complete(&gi3c->done);
> +	} else if ((dm_tx_st & TX_DMA_DONE) || (dm_rx_st & (RX_DMA_DONE | RX_RESET_DONE))) {
> +		complete(&gi3c->done);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
[ ... ]
> +static int _i3c_geni_execute_command(struct geni_i3c_dev *gi3c, struct geni_i3c_xfer_params *xfer)
> +{
[ ... ]
> +	if (xfer->mode == GENI_SE_DMA) {
> +		if (gi3c->err) {
> +			if (is_write)
> +				writel_relaxed(1, gi3c->se.base + SE_DMA_TX_FSM_RST);
> +			else
> +				writel_relaxed(1, gi3c->se.base + SE_DMA_RX_FSM_RST);
> +
> +			time_remaining = wait_for_completion_timeout(&gi3c->done, XFER_TIMEOUT);
> +			dev_dbg(gi3c->se.dev, "Timeout completing DMA transfer\n");
> +		}
> +
> +		if (is_write)
> +			geni_se_tx_dma_unprep(&gi3c->se, tx_dma, len);
> +		else
> +			geni_se_rx_dma_unprep(&gi3c->se, rx_dma, len);
> +	}

[Severity: High]
In _i3c_geni_execute_command() and geni_i3c_irq(), how is gi3c->done
synchronized across multi-command loops and DMA FSM resets?

1. Callers only call reinit_completion(&gi3c->done) once before their
   transfer loops rather than in _i3c_geni_execute_command() before each
   command and before waiting on SE_DMA_TX_FSM_RST / SE_DMA_RX_FSM_RST.
2. In geni_i3c_irq(), the DMA completion check handles RX_RESET_DONE in
   dm_rx_st, but omits TX_RESET_DONE in dm_tx_st, so waiting for
   SE_DMA_TX_FSM_RST in _i3c_geni_execute_command() will always time out
   before geni_se_tx_dma_unprep() unmaps the buffer.
3. Because m_stat is not masked with SE_GENI_M_IRQ_EN, if M_CMD_DONE_EN is
   set in m_stat while dma_mode is active, geni_i3c_irq() can complete
   gi3c->done before TX_DMA_DONE or RX_DMA_DONE fires, causing
   _i3c_geni_execute_command() to unmap the DMA buffer while DMA is still in
   flight and leaving an extra completion in gi3c->done for the next command.

[ ... ]
> +static int geni_i3c_master_priv_xfers(struct i3c_dev_desc *dev, struct i3c_xfer *xfers,
> +				      int nxfers, enum i3c_xfer_mode mode)
> +{
> +	struct i3c_master_controller *m = i3c_dev_get_master(dev);
> +	struct geni_i3c_dev *gi3c = to_geni_i3c_master(m);
> +	bool use_7e = false;
> +	int ret;
> +
> +	scoped_guard(mutex, &gi3c->lock) {
[ ... ]
> +		qcom_geni_i3c_conf(gi3c, PUSH_PULL_MODE);
> +
> +		for (int i = 0; i < nxfers; i++) {
> +			bool stall = (i < (nxfers - 1));
> +			struct geni_i3c_xfer_params xfer = { GENI_SE_FIFO };
> +
> +			xfer.m_param  = (stall ? STOP_STRETCH : 0);
> +			xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, dev->info.dyn_addr);
> +			xfer.m_param |= (use_7e) ? USE_7E : 0;
> +
> +			/* use_7e = true only for last transfer */
> +			use_7e = (i == nxfers - 1);

[Severity: High]
In geni_i3c_master_priv_xfers(), use_7e is initialized to false and only set
to (i == nxfers - 1) after xfer.m_param |= (use_7e) ? USE_7E : 0 has already
been evaluated.

On iterations i < nxfers - 1, use_7e is false; on the final iteration
i == nxfers - 1, use_7e is still false when xfer.m_param is built and only
becomes true right before the loop exits. As a result, USE_7E is never set on
any transfer (and shouldn't USE_7E be set on the first transfer i == 0, as in
geni_i3c_master_send_ccc_cmd() and geni_i3c_perform_daa()?).

> +
> +			if (xfers[i].rnw) {
[ ... ]
> +static int geni_i3c_master_i2c_xfers(struct i2c_dev_desc *dev, struct i2c_msg *msgs, int num)
> +{
[ ... ]
> +	for (int i = 0; i < num; i++) {
> +		struct geni_i3c_xfer_params xfer;
> +
> +		xfer.m_cmd    = (msgs[i].flags & I2C_M_RD) ? I2C_READ : I2C_WRITE;
> +		xfer.m_param  = (i < (num - 1)) ? STOP_STRETCH : 0;
> +		xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, msgs[i].addr);
> +		xfer.mode     = msgs[i].len > (gi3c->tx_depth / 2) ?
> +				GENI_SE_DMA : GENI_SE_FIFO;
> +		if (msgs[i].flags & I2C_M_RD)
> +			ret = i3c_geni_execute_read_command(gi3c, &xfer, msgs[i].buf,
> +							    msgs[i].len);
> +		else
> +			ret = i3c_geni_execute_write_command(gi3c, &xfer, msgs[i].buf,
> +							     msgs[i].len);
> +		if (ret)
> +			break;
> +	}

[Severity: High]
In geni_i3c_master_i2c_xfers(), when msgs[i].len > (gi3c->tx_depth / 2),
msgs[i].buf is passed directly to GENI_SE_DMA and mapped with
dma_map_single() in geni_se_tx_dma_prep() / geni_se_rx_dma_prep().

Since i2c_msg buffers are not guaranteed to be DMA-safe (as noted in the
struct i3c_master_controller_ops documentation in include/linux/i3c/master.h),
should this use i2c_get_dma_safe_msg_buf() and i2c_put_dma_safe_msg_buf()?

Also, for GENI_SE_DMA transfers:
- In _i3c_geni_execute_command(), geni_se_setup_m_cmd() starts the hardware
  command before geni_se_tx_dma_prep() or geni_se_rx_dma_prep() sets up the
  DMA transfer (or falls back to GENI_SE_FIFO).
- In i3c_geni_resources_init(), avg_bw is set on GENI_TO_CORE and
  CPU_TO_GENI, but gi3c->se.icc_paths[GENI_TO_DDR].avg_bw is left at 0.

[ ... ]
> +static int geni_i3c_hw_init(struct geni_i3c_dev *gi3c, struct platform_device *pdev)
> +{
> +	u32 proto, fifo_disable;
> +
> +	proto = geni_se_read_proto(&gi3c->se);
> +	if (proto != GENI_SE_I3C) {
> +		geni_se_resources_off(&gi3c->se);
> +		return dev_err_probe(gi3c->se.dev, -ENXIO, "Invalid proto %d\n", proto);
> +	}
> +
> +	fifo_disable = readl_relaxed(gi3c->se.base + GENI_IF_DISABLE_RO);
> +	if (fifo_disable) {
> +		geni_se_resources_off(&gi3c->se);
> +		return dev_err_probe(gi3c->se.dev, -ENXIO, "GPI DMA mode not supported\n");
> +	}
[ ... ]
> +static int geni_i3c_probe(struct platform_device *pdev)
> +{
[ ... ]
> +	ret = geni_se_resources_on(&gi3c->se);
> +	if (ret)
> +		return dev_err_probe(&pdev->dev, ret, "Error turning resources ON\n");
> +
> +	ret = geni_i3c_hw_init(gi3c, pdev);
> +	geni_se_resources_off(&gi3c->se);
> +	if (ret)
> +		return ret;
[ ... ]
> +static int geni_i3c_runtime_suspend(struct device *dev)
> +{
> +	struct geni_i3c_dev *gi3c = dev_get_drvdata(dev);
> +
> +	disable_irq(gi3c->irq);
> +	geni_se_resources_off(&gi3c->se);
> +
> +	return 0;
> +}

[Severity: Medium]
If geni_i3c_hw_init() fails, it calls geni_se_resources_off(&gi3c->se) before
returning, and then geni_i3c_probe() calls geni_se_resources_off(&gi3c->se) a
second time. Won't this underflow the enable/prepare counts on se->clk and
the shared QUP wrapper clocks?

Also, in geni_i3c_runtime_suspend(), if geni_se_resources_off(&gi3c->se)
fails (for instance if pinctrl_pm_select_sleep_state() returns an error
before clocks are disabled), should the error be propagated after re-enabling
gi3c->irq?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104111610.3739095-1-mukesh.savaliya@oss.qualcomm.com?part=2

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 2/3] i3c: master: Add Qualcomm I3C controller driver

[kernel test robot]

Hi Mukesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on i3c/i3c/next]
[also build test ERROR on linus/master v7.3 next-20261030]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mukesh-Kumar-Savaliya/i3c-master-Add-Qualcomm-I3C-controller-driver/20261104-164609
base:   https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git i3c/next
patch link:    https://lore.kernel.org/r/20261104111610.3739095-2-mukesh.savaliya%40oss.qualcomm.com
patch subject: [PATCH v7 2/3] i3c: master: Add Qualcomm I3C controller driver
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20261105/202611051252.K0VIpVGq-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project 812626a41399a1d464a0d69627ab45c83b8ebfb9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261105/202611051252.K0VIpVGq-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202611051252.K0VIpVGq-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/i3c/master/i3c-qcom-geni.c:19:
   include/linux/soc/qcom/geni-se.h:378:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     378 |         val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
         |               ^
   include/linux/soc/qcom/geni-se.h:397:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     397 |         writel(m_cmd, se->base + SE_GENI_M_CMD0);
         |         ^
   include/linux/soc/qcom/geni-se.h:413:10: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     413 |         s_cmd = readl_relaxed(se->base + SE_GENI_S_CMD0);
         |                 ^
   include/linux/soc/qcom/geni-se.h:417:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     417 |         writel(s_cmd, se->base + SE_GENI_S_CMD0);
         |         ^
   include/linux/soc/qcom/geni-se.h:430:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     430 |         writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:443:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     443 |         writel_relaxed(S_GENI_CMD_CANCEL, se->base + SE_GENI_S_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:455:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     455 |         writel_relaxed(M_GENI_CMD_ABORT, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:468:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     468 |         writel_relaxed(S_GENI_CMD_ABORT, se->base + SE_GENI_S_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:494:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     494 |         val = readl_relaxed(se->base + SE_HW_PARAM_0);
         |               ^
   include/linux/soc/qcom/geni-se.h:512:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     512 |         val = readl_relaxed(se->base + SE_HW_PARAM_0);
         |               ^
   include/linux/soc/qcom/geni-se.h:540:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     540 |         val = readl_relaxed(se->base + SE_HW_PARAM_1);
         |               ^
   In file included from drivers/i3c/master/i3c-qcom-geni.c:22:
   In file included from drivers/i3c/master/../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
>> include/asm-generic/io.h:288:20: error: static declaration of 'writel' follows non-static declaration
     288 | static inline void writel(u32 value, volatile void __iomem *addr)
         |                    ^
   include/asm-generic/io.h:287:16: note: expanded from macro 'writel'
     287 | #define writel writel
         |                ^
   include/linux/soc/qcom/geni-se.h:397:2: note: previous implicit declaration is here
     397 |         writel(m_cmd, se->base + SE_GENI_M_CMD0);
         |         ^
   In file included from drivers/i3c/master/i3c-qcom-geni.c:22:
   In file included from drivers/i3c/master/../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
>> include/asm-generic/io.h:353:19: error: static declaration of 'readl_relaxed' follows non-static declaration
     353 | static inline u32 readl_relaxed(const volatile void __iomem *addr)
         |                   ^
   include/asm-generic/io.h:352:23: note: expanded from macro 'readl_relaxed'
     352 | #define readl_relaxed readl_relaxed
         |                       ^
   include/linux/soc/qcom/geni-se.h:378:8: note: previous implicit declaration is here
     378 |         val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
         |               ^
   In file included from drivers/i3c/master/i3c-qcom-geni.c:22:
   In file included from drivers/i3c/master/../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
>> include/asm-generic/io.h:407:20: error: static declaration of 'writel_relaxed' follows non-static declaration
     407 | static inline void writel_relaxed(u32 value, volatile void __iomem *addr)
         |                    ^
   include/asm-generic/io.h:406:24: note: expanded from macro 'writel_relaxed'
     406 | #define writel_relaxed writel_relaxed
         |                        ^
   include/linux/soc/qcom/geni-se.h:430:2: note: previous implicit declaration is here
     430 |         writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
>> drivers/i3c/master/i3c-qcom-geni.c:284:8: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     284 |         val = FIELD_PREP(CLK_DIV_VALUE_MASK, clk_idx->clk_div);
         |               ^
   drivers/i3c/master/i3c-qcom-geni.c:363:6: warning: variable 'm_stat_mask' set but not used [-Wunused-but-set-variable]
     363 |         u32 m_stat_mask, dm_tx_st, dm_rx_st;
         |             ^
   drivers/i3c/master/i3c-qcom-geni.c:579:9: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     579 |                 pid = FIELD_PREP(GENMASK_ULL(47, 40), (u64)rx_buf[0]) |
         |                       ^
   drivers/i3c/master/i3c-qcom-geni.c:637:20: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     637 |                         xfer.m_param |= FIELD_PREP(CCC_HDR_CMD_MSK, cmd->id);
         |                                         ^
   drivers/i3c/master/i3c-qcom-geni.c:737:20: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     737 |                         xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, dev->info.dyn_addr);
         |                                         ^
   drivers/i3c/master/i3c-qcom-geni.c:791:19: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     791 |                 xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, msgs[i].addr);
         |                                 ^
   fatal error: too many errors emitted, stopping now [-ferror-limit=]
   1 warning and 20 errors generated.
--
   In file included from i3c-qcom-geni.c:19:
   include/linux/soc/qcom/geni-se.h:378:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     378 |         val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
         |               ^
   include/linux/soc/qcom/geni-se.h:397:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     397 |         writel(m_cmd, se->base + SE_GENI_M_CMD0);
         |         ^
   include/linux/soc/qcom/geni-se.h:413:10: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     413 |         s_cmd = readl_relaxed(se->base + SE_GENI_S_CMD0);
         |                 ^
   include/linux/soc/qcom/geni-se.h:417:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     417 |         writel(s_cmd, se->base + SE_GENI_S_CMD0);
         |         ^
   include/linux/soc/qcom/geni-se.h:430:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     430 |         writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:443:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     443 |         writel_relaxed(S_GENI_CMD_CANCEL, se->base + SE_GENI_S_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:455:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     455 |         writel_relaxed(M_GENI_CMD_ABORT, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:468:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     468 |         writel_relaxed(S_GENI_CMD_ABORT, se->base + SE_GENI_S_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:494:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     494 |         val = readl_relaxed(se->base + SE_HW_PARAM_0);
         |               ^
   include/linux/soc/qcom/geni-se.h:512:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     512 |         val = readl_relaxed(se->base + SE_HW_PARAM_0);
         |               ^
   include/linux/soc/qcom/geni-se.h:540:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     540 |         val = readl_relaxed(se->base + SE_HW_PARAM_1);
         |               ^
   In file included from i3c-qcom-geni.c:22:
   In file included from ./../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
>> include/asm-generic/io.h:288:20: error: static declaration of 'writel' follows non-static declaration
     288 | static inline void writel(u32 value, volatile void __iomem *addr)
         |                    ^
   include/asm-generic/io.h:287:16: note: expanded from macro 'writel'
     287 | #define writel writel
         |                ^
   include/linux/soc/qcom/geni-se.h:397:2: note: previous implicit declaration is here
     397 |         writel(m_cmd, se->base + SE_GENI_M_CMD0);
         |         ^
   In file included from i3c-qcom-geni.c:22:
   In file included from ./../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
>> include/asm-generic/io.h:353:19: error: static declaration of 'readl_relaxed' follows non-static declaration
     353 | static inline u32 readl_relaxed(const volatile void __iomem *addr)
         |                   ^
   include/asm-generic/io.h:352:23: note: expanded from macro 'readl_relaxed'
     352 | #define readl_relaxed readl_relaxed
         |                       ^
   include/linux/soc/qcom/geni-se.h:378:8: note: previous implicit declaration is here
     378 |         val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
         |               ^
   In file included from i3c-qcom-geni.c:22:
   In file included from ./../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
>> include/asm-generic/io.h:407:20: error: static declaration of 'writel_relaxed' follows non-static declaration
     407 | static inline void writel_relaxed(u32 value, volatile void __iomem *addr)
         |                    ^
   include/asm-generic/io.h:406:24: note: expanded from macro 'writel_relaxed'
     406 | #define writel_relaxed writel_relaxed
         |                        ^
   include/linux/soc/qcom/geni-se.h:430:2: note: previous implicit declaration is here
     430 |         writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   i3c-qcom-geni.c:284:8: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     284 |         val = FIELD_PREP(CLK_DIV_VALUE_MASK, clk_idx->clk_div);
         |               ^
   i3c-qcom-geni.c:363:6: warning: variable 'm_stat_mask' set but not used [-Wunused-but-set-variable]
     363 |         u32 m_stat_mask, dm_tx_st, dm_rx_st;
         |             ^
   i3c-qcom-geni.c:579:9: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     579 |                 pid = FIELD_PREP(GENMASK_ULL(47, 40), (u64)rx_buf[0]) |
         |                       ^
   i3c-qcom-geni.c:637:20: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     637 |                         xfer.m_param |= FIELD_PREP(CCC_HDR_CMD_MSK, cmd->id);
         |                                         ^
   i3c-qcom-geni.c:737:20: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     737 |                         xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, dev->info.dyn_addr);
         |                                         ^
   i3c-qcom-geni.c:791:19: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     791 |                 xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, msgs[i].addr);
         |                                 ^
   fatal error: too many errors emitted, stopping now [-ferror-limit=]
   1 warning and 20 errors generated.


vim +/readl_relaxed +353 include/asm-generic/io.h

3f7e212df82ca04 Arnd Bergmann      2009-05-13  285  
9216efafc52ff99 Thierry Reding     2014-10-01  286  #ifndef writel
9216efafc52ff99 Thierry Reding     2014-10-01  287  #define writel writel
9216efafc52ff99 Thierry Reding     2014-10-01 @288  static inline void writel(u32 value, volatile void __iomem *addr)
3f7e212df82ca04 Arnd Bergmann      2009-05-13  289  {
8327bd4fcb6c1da Varad Gautam       2025-03-30  290  	if (rwmmio_tracepoint_enabled(rwmmio_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  291  		log_write_mmio(value, 32, addr, _THIS_IP_, _RET_IP_);
755bd04aaf4bf7c Sinan Kaya         2018-04-05  292  	__io_bw();
c1d55d50139bea6 Stafford Horne     2020-07-29  293  	__raw_writel((u32 __force)__cpu_to_le32(value), addr);
755bd04aaf4bf7c Sinan Kaya         2018-04-05  294  	__io_aw();
8327bd4fcb6c1da Varad Gautam       2025-03-30  295  	if (rwmmio_tracepoint_enabled(rwmmio_post_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  296  		log_post_write_mmio(value, 32, addr, _THIS_IP_, _RET_IP_);
3f7e212df82ca04 Arnd Bergmann      2009-05-13  297  }
9216efafc52ff99 Thierry Reding     2014-10-01  298  #endif
3f7e212df82ca04 Arnd Bergmann      2009-05-13  299  
9216efafc52ff99 Thierry Reding     2014-10-01  300  #ifdef CONFIG_64BIT
9216efafc52ff99 Thierry Reding     2014-10-01  301  #ifndef writeq
9216efafc52ff99 Thierry Reding     2014-10-01  302  #define writeq writeq
9216efafc52ff99 Thierry Reding     2014-10-01  303  static inline void writeq(u64 value, volatile void __iomem *addr)
3f7e212df82ca04 Arnd Bergmann      2009-05-13  304  {
8327bd4fcb6c1da Varad Gautam       2025-03-30  305  	if (rwmmio_tracepoint_enabled(rwmmio_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  306  		log_write_mmio(value, 64, addr, _THIS_IP_, _RET_IP_);
755bd04aaf4bf7c Sinan Kaya         2018-04-05  307  	__io_bw();
d564fa1ff19e893 Vladimir Oltean    2023-01-09  308  	__raw_writeq((u64 __force)__cpu_to_le64(value), addr);
755bd04aaf4bf7c Sinan Kaya         2018-04-05  309  	__io_aw();
8327bd4fcb6c1da Varad Gautam       2025-03-30  310  	if (rwmmio_tracepoint_enabled(rwmmio_post_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  311  		log_post_write_mmio(value, 64, addr, _THIS_IP_, _RET_IP_);
3f7e212df82ca04 Arnd Bergmann      2009-05-13  312  }
9216efafc52ff99 Thierry Reding     2014-10-01  313  #endif
9216efafc52ff99 Thierry Reding     2014-10-01  314  #endif /* CONFIG_64BIT */
3f7e212df82ca04 Arnd Bergmann      2009-05-13  315  
1c8d29696f0d799 Arnd Bergmann      2014-11-11  316  /*
1c8d29696f0d799 Arnd Bergmann      2014-11-11  317   * {read,write}{b,w,l,q}_relaxed() are like the regular version, but
1c8d29696f0d799 Arnd Bergmann      2014-11-11  318   * are not guaranteed to provide ordering against spinlocks or memory
1c8d29696f0d799 Arnd Bergmann      2014-11-11  319   * accesses.
1c8d29696f0d799 Arnd Bergmann      2014-11-11  320   */
1c8d29696f0d799 Arnd Bergmann      2014-11-11  321  #ifndef readb_relaxed
8875c55437617fa Sinan Kaya         2018-04-06  322  #define readb_relaxed readb_relaxed
8875c55437617fa Sinan Kaya         2018-04-06  323  static inline u8 readb_relaxed(const volatile void __iomem *addr)
8875c55437617fa Sinan Kaya         2018-04-06  324  {
210031971cdd25a Sai Prakash Ranjan 2022-05-18  325  	u8 val;
210031971cdd25a Sai Prakash Ranjan 2022-05-18  326  
8327bd4fcb6c1da Varad Gautam       2025-03-30  327  	if (rwmmio_tracepoint_enabled(rwmmio_read))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  328  		log_read_mmio(8, addr, _THIS_IP_, _RET_IP_);
210031971cdd25a Sai Prakash Ranjan 2022-05-18  329  	val = __raw_readb(addr);
8327bd4fcb6c1da Varad Gautam       2025-03-30  330  	if (rwmmio_tracepoint_enabled(rwmmio_post_read))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  331  		log_post_read_mmio(val, 8, addr, _THIS_IP_, _RET_IP_);
210031971cdd25a Sai Prakash Ranjan 2022-05-18  332  	return val;
8875c55437617fa Sinan Kaya         2018-04-06  333  }
1c8d29696f0d799 Arnd Bergmann      2014-11-11  334  #endif
1c8d29696f0d799 Arnd Bergmann      2014-11-11  335  
1c8d29696f0d799 Arnd Bergmann      2014-11-11  336  #ifndef readw_relaxed
8875c55437617fa Sinan Kaya         2018-04-06  337  #define readw_relaxed readw_relaxed
8875c55437617fa Sinan Kaya         2018-04-06  338  static inline u16 readw_relaxed(const volatile void __iomem *addr)
8875c55437617fa Sinan Kaya         2018-04-06  339  {
210031971cdd25a Sai Prakash Ranjan 2022-05-18  340  	u16 val;
210031971cdd25a Sai Prakash Ranjan 2022-05-18  341  
8327bd4fcb6c1da Varad Gautam       2025-03-30  342  	if (rwmmio_tracepoint_enabled(rwmmio_read))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  343  		log_read_mmio(16, addr, _THIS_IP_, _RET_IP_);
05d3855b4d21ef3 Vladimir Oltean    2023-01-09  344  	val = __le16_to_cpu((__le16 __force)__raw_readw(addr));
8327bd4fcb6c1da Varad Gautam       2025-03-30  345  	if (rwmmio_tracepoint_enabled(rwmmio_post_read))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  346  		log_post_read_mmio(val, 16, addr, _THIS_IP_, _RET_IP_);
210031971cdd25a Sai Prakash Ranjan 2022-05-18  347  	return val;
8875c55437617fa Sinan Kaya         2018-04-06  348  }
1c8d29696f0d799 Arnd Bergmann      2014-11-11  349  #endif
1c8d29696f0d799 Arnd Bergmann      2014-11-11  350  
1c8d29696f0d799 Arnd Bergmann      2014-11-11  351  #ifndef readl_relaxed
8875c55437617fa Sinan Kaya         2018-04-06  352  #define readl_relaxed readl_relaxed
8875c55437617fa Sinan Kaya         2018-04-06 @353  static inline u32 readl_relaxed(const volatile void __iomem *addr)
8875c55437617fa Sinan Kaya         2018-04-06  354  {
210031971cdd25a Sai Prakash Ranjan 2022-05-18  355  	u32 val;
210031971cdd25a Sai Prakash Ranjan 2022-05-18  356  
8327bd4fcb6c1da Varad Gautam       2025-03-30  357  	if (rwmmio_tracepoint_enabled(rwmmio_read))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  358  		log_read_mmio(32, addr, _THIS_IP_, _RET_IP_);
05d3855b4d21ef3 Vladimir Oltean    2023-01-09  359  	val = __le32_to_cpu((__le32 __force)__raw_readl(addr));
8327bd4fcb6c1da Varad Gautam       2025-03-30  360  	if (rwmmio_tracepoint_enabled(rwmmio_post_read))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  361  		log_post_read_mmio(val, 32, addr, _THIS_IP_, _RET_IP_);
210031971cdd25a Sai Prakash Ranjan 2022-05-18  362  	return val;
8875c55437617fa Sinan Kaya         2018-04-06  363  }
1c8d29696f0d799 Arnd Bergmann      2014-11-11  364  #endif
1c8d29696f0d799 Arnd Bergmann      2014-11-11  365  
e511267bc25e189 Robin Murphy       2016-04-26  366  #if defined(readq) && !defined(readq_relaxed)
8875c55437617fa Sinan Kaya         2018-04-06  367  #define readq_relaxed readq_relaxed
8875c55437617fa Sinan Kaya         2018-04-06  368  static inline u64 readq_relaxed(const volatile void __iomem *addr)
8875c55437617fa Sinan Kaya         2018-04-06  369  {
210031971cdd25a Sai Prakash Ranjan 2022-05-18  370  	u64 val;
210031971cdd25a Sai Prakash Ranjan 2022-05-18  371  
8327bd4fcb6c1da Varad Gautam       2025-03-30  372  	if (rwmmio_tracepoint_enabled(rwmmio_read))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  373  		log_read_mmio(64, addr, _THIS_IP_, _RET_IP_);
05d3855b4d21ef3 Vladimir Oltean    2023-01-09  374  	val = __le64_to_cpu((__le64 __force)__raw_readq(addr));
8327bd4fcb6c1da Varad Gautam       2025-03-30  375  	if (rwmmio_tracepoint_enabled(rwmmio_post_read))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  376  		log_post_read_mmio(val, 64, addr, _THIS_IP_, _RET_IP_);
210031971cdd25a Sai Prakash Ranjan 2022-05-18  377  	return val;
8875c55437617fa Sinan Kaya         2018-04-06  378  }
1c8d29696f0d799 Arnd Bergmann      2014-11-11  379  #endif
3f7e212df82ca04 Arnd Bergmann      2009-05-13  380  
9439eb3ab9d1ece Will Deacon        2013-09-03  381  #ifndef writeb_relaxed
a71e7c44ffb7bae Sinan Kaya         2018-04-06  382  #define writeb_relaxed writeb_relaxed
a71e7c44ffb7bae Sinan Kaya         2018-04-06  383  static inline void writeb_relaxed(u8 value, volatile void __iomem *addr)
a71e7c44ffb7bae Sinan Kaya         2018-04-06  384  {
8327bd4fcb6c1da Varad Gautam       2025-03-30  385  	if (rwmmio_tracepoint_enabled(rwmmio_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  386  		log_write_mmio(value, 8, addr, _THIS_IP_, _RET_IP_);
a71e7c44ffb7bae Sinan Kaya         2018-04-06  387  	__raw_writeb(value, addr);
8327bd4fcb6c1da Varad Gautam       2025-03-30  388  	if (rwmmio_tracepoint_enabled(rwmmio_post_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  389  		log_post_write_mmio(value, 8, addr, _THIS_IP_, _RET_IP_);
a71e7c44ffb7bae Sinan Kaya         2018-04-06  390  }
9439eb3ab9d1ece Will Deacon        2013-09-03  391  #endif
9439eb3ab9d1ece Will Deacon        2013-09-03  392  
9439eb3ab9d1ece Will Deacon        2013-09-03  393  #ifndef writew_relaxed
a71e7c44ffb7bae Sinan Kaya         2018-04-06  394  #define writew_relaxed writew_relaxed
a71e7c44ffb7bae Sinan Kaya         2018-04-06  395  static inline void writew_relaxed(u16 value, volatile void __iomem *addr)
a71e7c44ffb7bae Sinan Kaya         2018-04-06  396  {
8327bd4fcb6c1da Varad Gautam       2025-03-30  397  	if (rwmmio_tracepoint_enabled(rwmmio_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  398  		log_write_mmio(value, 16, addr, _THIS_IP_, _RET_IP_);
05d3855b4d21ef3 Vladimir Oltean    2023-01-09  399  	__raw_writew((u16 __force)cpu_to_le16(value), addr);
8327bd4fcb6c1da Varad Gautam       2025-03-30  400  	if (rwmmio_tracepoint_enabled(rwmmio_post_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  401  		log_post_write_mmio(value, 16, addr, _THIS_IP_, _RET_IP_);
a71e7c44ffb7bae Sinan Kaya         2018-04-06  402  }
9439eb3ab9d1ece Will Deacon        2013-09-03  403  #endif
9439eb3ab9d1ece Will Deacon        2013-09-03  404  
9439eb3ab9d1ece Will Deacon        2013-09-03  405  #ifndef writel_relaxed
a71e7c44ffb7bae Sinan Kaya         2018-04-06  406  #define writel_relaxed writel_relaxed
a71e7c44ffb7bae Sinan Kaya         2018-04-06 @407  static inline void writel_relaxed(u32 value, volatile void __iomem *addr)
a71e7c44ffb7bae Sinan Kaya         2018-04-06  408  {
8327bd4fcb6c1da Varad Gautam       2025-03-30  409  	if (rwmmio_tracepoint_enabled(rwmmio_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  410  		log_write_mmio(value, 32, addr, _THIS_IP_, _RET_IP_);
05d3855b4d21ef3 Vladimir Oltean    2023-01-09  411  	__raw_writel((u32 __force)__cpu_to_le32(value), addr);
8327bd4fcb6c1da Varad Gautam       2025-03-30  412  	if (rwmmio_tracepoint_enabled(rwmmio_post_write))
5e5ff73c2e5863f Sai Prakash Ranjan 2022-10-17  413  		log_post_write_mmio(value, 32, addr, _THIS_IP_, _RET_IP_);
a71e7c44ffb7bae Sinan Kaya         2018-04-06  414  }
9439eb3ab9d1ece Will Deacon        2013-09-03  415  #endif
3f7e212df82ca04 Arnd Bergmann      2009-05-13  416  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 2/3] i3c: master: Add Qualcomm I3C controller driver

[kernel test robot]

Hi Mukesh,

kernel test robot noticed the following build errors:

[auto build test ERROR on i3c/i3c/next]
[also build test ERROR on linus/master v7.4-rc1 next-20261030]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mukesh-Kumar-Savaliya/i3c-master-Add-Qualcomm-I3C-controller-driver/20261104-164609
base:   https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git i3c/next
patch link:    https://lore.kernel.org/r/20261104111610.3739095-2-mukesh.savaliya%40oss.qualcomm.com
patch subject: [PATCH v7 2/3] i3c: master: Add Qualcomm I3C controller driver
config: s390-allmodconfig (https://download.01.org/0day-ci/archive/20261105/202611052242.iJ7HMqUH-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project 812626a41399a1d464a0d69627ab45c83b8ebfb9)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261105/202611052242.iJ7HMqUH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202611052242.iJ7HMqUH-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/i3c/master/i3c-qcom-geni.c:19:
>> include/linux/soc/qcom/geni-se.h:378:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     378 |         val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
         |               ^
>> include/linux/soc/qcom/geni-se.h:397:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     397 |         writel(m_cmd, se->base + SE_GENI_M_CMD0);
         |         ^
   include/linux/soc/qcom/geni-se.h:413:10: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     413 |         s_cmd = readl_relaxed(se->base + SE_GENI_S_CMD0);
         |                 ^
   include/linux/soc/qcom/geni-se.h:417:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     417 |         writel(s_cmd, se->base + SE_GENI_S_CMD0);
         |         ^
>> include/linux/soc/qcom/geni-se.h:430:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     430 |         writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:443:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     443 |         writel_relaxed(S_GENI_CMD_CANCEL, se->base + SE_GENI_S_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:455:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     455 |         writel_relaxed(M_GENI_CMD_ABORT, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:468:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     468 |         writel_relaxed(S_GENI_CMD_ABORT, se->base + SE_GENI_S_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:494:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     494 |         val = readl_relaxed(se->base + SE_HW_PARAM_0);
         |               ^
   include/linux/soc/qcom/geni-se.h:512:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     512 |         val = readl_relaxed(se->base + SE_HW_PARAM_0);
         |               ^
   include/linux/soc/qcom/geni-se.h:540:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     540 |         val = readl_relaxed(se->base + SE_HW_PARAM_1);
         |               ^
   In file included from drivers/i3c/master/i3c-qcom-geni.c:22:
   In file included from drivers/i3c/master/../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:288:20: error: static declaration of 'writel' follows non-static declaration
     288 | static inline void writel(u32 value, volatile void __iomem *addr)
         |                    ^
   include/asm-generic/io.h:287:16: note: expanded from macro 'writel'
     287 | #define writel writel
         |                ^
   include/linux/soc/qcom/geni-se.h:397:2: note: previous implicit declaration is here
     397 |         writel(m_cmd, se->base + SE_GENI_M_CMD0);
         |         ^
   In file included from drivers/i3c/master/i3c-qcom-geni.c:22:
   In file included from drivers/i3c/master/../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:353:19: error: static declaration of 'readl_relaxed' follows non-static declaration
     353 | static inline u32 readl_relaxed(const volatile void __iomem *addr)
         |                   ^
   include/asm-generic/io.h:352:23: note: expanded from macro 'readl_relaxed'
     352 | #define readl_relaxed readl_relaxed
         |                       ^
   include/linux/soc/qcom/geni-se.h:378:8: note: previous implicit declaration is here
     378 |         val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
         |               ^
   In file included from drivers/i3c/master/i3c-qcom-geni.c:22:
   In file included from drivers/i3c/master/../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:407:20: error: static declaration of 'writel_relaxed' follows non-static declaration
     407 | static inline void writel_relaxed(u32 value, volatile void __iomem *addr)
         |                    ^
   include/asm-generic/io.h:406:24: note: expanded from macro 'writel_relaxed'
     406 | #define writel_relaxed writel_relaxed
         |                        ^
   include/linux/soc/qcom/geni-se.h:430:2: note: previous implicit declaration is here
     430 |         writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   drivers/i3c/master/i3c-qcom-geni.c:284:8: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     284 |         val = FIELD_PREP(CLK_DIV_VALUE_MASK, clk_idx->clk_div);
         |               ^
   drivers/i3c/master/i3c-qcom-geni.c:363:6: warning: variable 'm_stat_mask' set but not used [-Wunused-but-set-variable]
     363 |         u32 m_stat_mask, dm_tx_st, dm_rx_st;
         |             ^
   drivers/i3c/master/i3c-qcom-geni.c:579:9: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     579 |                 pid = FIELD_PREP(GENMASK_ULL(47, 40), (u64)rx_buf[0]) |
         |                       ^
   drivers/i3c/master/i3c-qcom-geni.c:637:20: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     637 |                         xfer.m_param |= FIELD_PREP(CCC_HDR_CMD_MSK, cmd->id);
         |                                         ^
   drivers/i3c/master/i3c-qcom-geni.c:737:20: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     737 |                         xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, dev->info.dyn_addr);
         |                                         ^
   drivers/i3c/master/i3c-qcom-geni.c:791:19: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     791 |                 xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, msgs[i].addr);
         |                                 ^
   fatal error: too many errors emitted, stopping now [-ferror-limit=]
   1 warning and 20 errors generated.
--
   In file included from i3c-qcom-geni.c:19:
>> include/linux/soc/qcom/geni-se.h:378:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     378 |         val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
         |               ^
>> include/linux/soc/qcom/geni-se.h:397:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     397 |         writel(m_cmd, se->base + SE_GENI_M_CMD0);
         |         ^
   include/linux/soc/qcom/geni-se.h:413:10: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     413 |         s_cmd = readl_relaxed(se->base + SE_GENI_S_CMD0);
         |                 ^
   include/linux/soc/qcom/geni-se.h:417:2: error: call to undeclared function 'writel'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     417 |         writel(s_cmd, se->base + SE_GENI_S_CMD0);
         |         ^
>> include/linux/soc/qcom/geni-se.h:430:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     430 |         writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:443:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     443 |         writel_relaxed(S_GENI_CMD_CANCEL, se->base + SE_GENI_S_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:455:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     455 |         writel_relaxed(M_GENI_CMD_ABORT, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:468:2: error: call to undeclared function 'writel_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     468 |         writel_relaxed(S_GENI_CMD_ABORT, se->base + SE_GENI_S_CMD_CTRL_REG);
         |         ^
   include/linux/soc/qcom/geni-se.h:494:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     494 |         val = readl_relaxed(se->base + SE_HW_PARAM_0);
         |               ^
   include/linux/soc/qcom/geni-se.h:512:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     512 |         val = readl_relaxed(se->base + SE_HW_PARAM_0);
         |               ^
   include/linux/soc/qcom/geni-se.h:540:8: error: call to undeclared function 'readl_relaxed'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     540 |         val = readl_relaxed(se->base + SE_HW_PARAM_1);
         |               ^
   In file included from i3c-qcom-geni.c:22:
   In file included from ./../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:288:20: error: static declaration of 'writel' follows non-static declaration
     288 | static inline void writel(u32 value, volatile void __iomem *addr)
         |                    ^
   include/asm-generic/io.h:287:16: note: expanded from macro 'writel'
     287 | #define writel writel
         |                ^
   include/linux/soc/qcom/geni-se.h:397:2: note: previous implicit declaration is here
     397 |         writel(m_cmd, se->base + SE_GENI_M_CMD0);
         |         ^
   In file included from i3c-qcom-geni.c:22:
   In file included from ./../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:353:19: error: static declaration of 'readl_relaxed' follows non-static declaration
     353 | static inline u32 readl_relaxed(const volatile void __iomem *addr)
         |                   ^
   include/asm-generic/io.h:352:23: note: expanded from macro 'readl_relaxed'
     352 | #define readl_relaxed readl_relaxed
         |                       ^
   include/linux/soc/qcom/geni-se.h:378:8: note: previous implicit declaration is here
     378 |         val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
         |               ^
   In file included from i3c-qcom-geni.c:22:
   In file included from ./../internals.h:12:
   In file included from include/linux/io.h:12:
   In file included from arch/s390/include/asm/io.h:93:
   include/asm-generic/io.h:407:20: error: static declaration of 'writel_relaxed' follows non-static declaration
     407 | static inline void writel_relaxed(u32 value, volatile void __iomem *addr)
         |                    ^
   include/asm-generic/io.h:406:24: note: expanded from macro 'writel_relaxed'
     406 | #define writel_relaxed writel_relaxed
         |                        ^
   include/linux/soc/qcom/geni-se.h:430:2: note: previous implicit declaration is here
     430 |         writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
         |         ^
   i3c-qcom-geni.c:284:8: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     284 |         val = FIELD_PREP(CLK_DIV_VALUE_MASK, clk_idx->clk_div);
         |               ^
   i3c-qcom-geni.c:363:6: warning: variable 'm_stat_mask' set but not used [-Wunused-but-set-variable]
     363 |         u32 m_stat_mask, dm_tx_st, dm_rx_st;
         |             ^
   i3c-qcom-geni.c:579:9: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     579 |                 pid = FIELD_PREP(GENMASK_ULL(47, 40), (u64)rx_buf[0]) |
         |                       ^
   i3c-qcom-geni.c:637:20: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     637 |                         xfer.m_param |= FIELD_PREP(CCC_HDR_CMD_MSK, cmd->id);
         |                                         ^
   i3c-qcom-geni.c:737:20: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     737 |                         xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, dev->info.dyn_addr);
         |                                         ^
   i3c-qcom-geni.c:791:19: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     791 |                 xfer.m_param |= FIELD_PREP(SLAVE_ADDR_MASK, msgs[i].addr);
         |                                 ^
   fatal error: too many errors emitted, stopping now [-ferror-limit=]
   1 warning and 20 errors generated.


vim +/readl_relaxed +378 include/linux/soc/qcom/geni-se.h

eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  367  
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  368  /**
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  369   * geni_se_read_proto() - Read the protocol configured for a serial engine
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  370   * @se:		Pointer to the concerned serial engine.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  371   *
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  372   * Return: Protocol value as configured in the serial engine.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  373   */
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  374  static inline u32 geni_se_read_proto(struct geni_se *se)
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  375  {
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  376  	u32 val;
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  377  
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30 @378  	val = readl_relaxed(se->base + GENI_FW_REVISION_RO);
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  379  
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  380  	return (val & FW_REV_PROTOCOL_MSK) >> FW_REV_PROTOCOL_SHFT;
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  381  }
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  382  
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  383  /**
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  384   * geni_se_setup_m_cmd() - Setup the primary sequencer
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  385   * @se:		Pointer to the concerned serial engine.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  386   * @cmd:	Command/Operation to setup in the primary sequencer.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  387   * @params:	Parameter for the sequencer command.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  388   *
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  389   * This function is used to configure the primary sequencer with the
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  390   * command and its associated parameters.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  391   */
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  392  static inline void geni_se_setup_m_cmd(struct geni_se *se, u32 cmd, u32 params)
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  393  {
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  394  	u32 m_cmd;
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  395  
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  396  	m_cmd = (cmd << M_OPCODE_SHFT) | (params & M_PARAMS_MSK);
0feea33d79825d include/linux/qcom-geni-se.h Douglas Anderson            2020-07-22 @397  	writel(m_cmd, se->base + SE_GENI_M_CMD0);
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  398  }
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  399  
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  400  /**
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  401   * geni_se_setup_s_cmd() - Setup the secondary sequencer
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  402   * @se:		Pointer to the concerned serial engine.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  403   * @cmd:	Command/Operation to setup in the secondary sequencer.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  404   * @params:	Parameter for the sequencer command.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  405   *
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  406   * This function is used to configure the secondary sequencer with the
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  407   * command and its associated parameters.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  408   */
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  409  static inline void geni_se_setup_s_cmd(struct geni_se *se, u32 cmd, u32 params)
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  410  {
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  411  	u32 s_cmd;
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  412  
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  413  	s_cmd = readl_relaxed(se->base + SE_GENI_S_CMD0);
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  414  	s_cmd &= ~(S_OPCODE_MSK | S_PARAMS_MSK);
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  415  	s_cmd |= (cmd << S_OPCODE_SHFT);
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  416  	s_cmd |= (params & S_PARAMS_MSK);
0feea33d79825d include/linux/qcom-geni-se.h Douglas Anderson            2020-07-22  417  	writel(s_cmd, se->base + SE_GENI_S_CMD0);
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  418  }
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  419  
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  420  /**
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  421   * geni_se_cancel_m_cmd() - Cancel the command configured in the primary
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  422   *                          sequencer
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  423   * @se:	Pointer to the concerned serial engine.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  424   *
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  425   * This function is used to cancel the currently configured command in the
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  426   * primary sequencer.
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  427   */
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  428  static inline void geni_se_cancel_m_cmd(struct geni_se *se)
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  429  {
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30 @430  	writel_relaxed(M_GENI_CMD_CANCEL, se->base + SE_GENI_M_CMD_CTRL_REG);
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  431  }
eddac5af06546d include/linux/qcom-geni-se.h Karthikeyan Ramasubramanian 2018-03-30  432  

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 2/3] i3c: master: Add Qualcomm I3C controller driver

[kernel test robot]

Hi Mukesh,

kernel test robot noticed the following build warnings:

[auto build test WARNING on i3c/i3c/next]
[also build test WARNING on linus/master v7.3 next-20261030]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Mukesh-Kumar-Savaliya/i3c-master-Add-Qualcomm-I3C-controller-driver/20261104-164609
base:   https://git.kernel.org/pub/scm/linux/kernel/git/i3c/linux.git i3c/next
patch link:    https://lore.kernel.org/r/20261104111610.3739095-2-mukesh.savaliya%40oss.qualcomm.com
patch subject: [PATCH v7 2/3] i3c: master: Add Qualcomm I3C controller driver
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20261106/202611060055.y4Q35SU8-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 16.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261106/202611060055.y4Q35SU8-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202611060055.y4Q35SU8-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/i3c/master/i3c-qcom-geni.c: In function 'qcom_geni_i3c_conf':
   drivers/i3c/master/i3c-qcom-geni.c:284:15: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
     284 |         val = FIELD_PREP(CLK_DIV_VALUE_MASK, clk_idx->clk_div);
         |               ^~~~~~~~~~
   drivers/i3c/master/i3c-qcom-geni.c: In function 'geni_i3c_irq':
>> drivers/i3c/master/i3c-qcom-geni.c:363:13: warning: variable 'm_stat_mask' set but not used [-Wunused-but-set-variable=]
     363 |         u32 m_stat_mask, dm_tx_st, dm_rx_st;
         |             ^~~~~~~~~~~
   drivers/i3c/master/i3c-qcom-geni.c: In function 'i3c_geni_resources_init':
   drivers/i3c/master/i3c-qcom-geni.c:977:15: error: too few arguments to function 'geni_icc_get'; expected 2, have 1
     977 |         ret = geni_icc_get(&gi3c->se);
         |               ^~~~~~~~~~~~
   In file included from drivers/i3c/master/i3c-qcom-geni.c:19:
   include/linux/soc/qcom/geni-se.h:576:5: note: declared here
     576 | int geni_icc_get(struct geni_se *se, const char *icc_ddr);
         |     ^~~~~~~~~~~~


vim +/m_stat_mask +363 drivers/i3c/master/i3c-qcom-geni.c

   359	
   360	static irqreturn_t geni_i3c_irq(int irq, void *dev)
   361	{
   362		struct geni_i3c_dev *gi3c = dev;
 > 363		u32 m_stat_mask, dm_tx_st, dm_rx_st;
   364		u32 m_stat, rx_st, dma_mode;
   365	
   366		m_stat = readl_relaxed(gi3c->se.base + SE_GENI_M_IRQ_STATUS);
   367		m_stat_mask = readl_relaxed(gi3c->se.base + SE_GENI_M_IRQ_EN);
   368		rx_st = readl_relaxed(gi3c->se.base + SE_GENI_RX_FIFO_STATUS);
   369		dm_tx_st = readl_relaxed(gi3c->se.base + SE_DMA_TX_IRQ_STAT);
   370		dm_rx_st = readl_relaxed(gi3c->se.base + SE_DMA_RX_IRQ_STAT);
   371		dma_mode = readl_relaxed(gi3c->se.base + SE_GENI_DMA_MODE_EN);
   372	
   373		scoped_guard(spinlock, &gi3c->irq_lock) {
   374			if (m_stat & SE_I3C_ERR || dm_rx_st & DM_I3C_CB_ERR) {
   375				geni_i3c_handle_err(gi3c, m_stat);
   376				/* Disable the TX Watermark interrupt to stop TX */
   377				if (!dma_mode)
   378					writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
   379			} else if (dma_mode) {
   380				dev_dbg(gi3c->se.dev, "i3c dma tx:0x%x, dma rx:0x%x\n", dm_tx_st, dm_rx_st);
   381			} else if ((m_stat & (M_RX_FIFO_WATERMARK_EN | M_RX_FIFO_LAST_EN)) &&
   382				    !gi3c->cur_is_write && gi3c->cur_buf) {
   383				/* FIFO RX handling */
   384				u32 rxcnt = rx_st & RX_FIFO_WC_MSK;
   385				int avail = rxcnt * 4;
   386				int need  = gi3c->cur_len - gi3c->cur_idx;
   387				int n     = min(need, avail);
   388	
   389				if (n > 0) {
   390					i3c_readl_fifo(gi3c->se.base + SE_GENI_RX_FIFOn,
   391						       gi3c->cur_buf + gi3c->cur_idx, n);
   392					gi3c->cur_idx += n;
   393				}
   394			} else if ((m_stat & M_TX_FIFO_WATERMARK_EN) &&
   395				   gi3c->cur_is_write && gi3c->cur_buf) {
   396				/* FIFO TX handling */
   397				int avail = gi3c->tx_wm * 4;
   398				int need  = gi3c->cur_len - gi3c->cur_idx;
   399				int n     = min(need, avail);
   400	
   401				if (n > 0) {
   402					i3c_writel_fifo(gi3c->se.base + SE_GENI_TX_FIFOn,
   403							(void *)(gi3c->cur_buf + gi3c->cur_idx), n);
   404					gi3c->cur_idx += n;
   405				}
   406			}
   407	
   408			if (!dma_mode && gi3c->cur_idx == gi3c->cur_len)
   409				writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
   410		}
   411	
   412		if (m_stat)
   413			writel_relaxed(m_stat, gi3c->se.base + SE_GENI_M_IRQ_CLEAR);
   414	
   415		if (dma_mode) {
   416			if (dm_tx_st)
   417				writel_relaxed(dm_tx_st, gi3c->se.base + SE_DMA_TX_IRQ_CLR);
   418			if (dm_rx_st)
   419				writel_relaxed(dm_rx_st, gi3c->se.base + SE_DMA_RX_IRQ_CLR);
   420		}
   421	
   422		/* if this is err with done-bit not set, handle that through timeout. */
   423		if (m_stat & (M_CMD_DONE_EN | M_CMD_ABORT_EN)) {
   424			writel_relaxed(0, gi3c->se.base + SE_GENI_TX_WATERMARK_REG);
   425			complete(&gi3c->done);
   426		} else if ((dm_tx_st & TX_DMA_DONE) || (dm_rx_st & (RX_DMA_DONE | RX_RESET_DONE))) {
   427			complete(&gi3c->done);
   428		}
   429	
   430		return IRQ_HANDLED;
   431	}
   432	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v7 3/3] MAINTAINERS: Add maintainer for Qualcomm's I3C controller driver

[Mukesh Kumar Savaliya]

Add a new entry for the I3C QCOM GENI driver to the MAINTAINERS file.
This entry includes the maintainer's name and contact information,
ensuring proper maintainership and communication for the new driver.

Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 46c416cadacd..43c5730dd007 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -12637,6 +12637,14 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/i3c/cdns,i3c-master.yaml
 F:	drivers/i3c/master/i3c-master-cdns.c
 
+I3C DRIVER FOR QUALCOMM GENI CONTROLLER IP
+M:	Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
+L:	linux-i3c@lists.infradead.org (moderated for non-subscribers)
+L:	linux-arm-msm@vger.kernel.org
+S:	Supported
+F:	Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
+F:	drivers/i3c/master/i3c-qcom-geni.c
+
 I3C DRIVER FOR RENESAS
 M:	Wolfram Sang <wsa+renesas@sang-engineering.com>
 M:	Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
-- 
2.43.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 1/3] dt-bindings: i3c: Add support for Qualcomm I3C controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Medium] dt-bindings: i3c: missing i3c child node pattern in qcom,geni-se.yaml
- [Medium] dt-bindings: i3c: required pinctrl-1 contradicts pinctrl-names minItems: 1
- [Medium] dt-bindings: i3c: missing power-domains and operating-points-v2 properties
- [Low] dt-bindings: i3c: example uses QUP wrapper address for IBI_C and mixes SoCs
--

commit 53511a9ac0238442394253f95c17db4e7c486559
Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>

dt-bindings: i3c: Add support for Qualcomm I3C controller

This commit adds the device tree binding schema for the Qualcomm GENI-based
I3C controller and its associated In-Band Interrupt (IBI) controller block.

> diff --git a/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> new file mode 100644
> index 0000000000000..bab89aeee23f4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i3c/qcom,geni-i3c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Geni based QUP I3C Controller
> +
> +maintainers:
> +  - Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> +
> +description:
> +  Qualcomm GENI-based I3C controller implementing an MIPI I3C bus master.
> +  The controller supports SDR transfers in mixed bus mode with both I2C and
> +  I3C target devices present on the same bus.
> +
> +  The controller consists of a GENI Serial Engine (SE) responsible for I3C
> +  bus transactions and an associated dedicated IBI controller(IBI_C) block
> +  used for Hot-Join, In-Band Interrupt (IBI) handling and management of GPII
> +  resources associated with the I3C controller.
> +
> +  I3C controller nodes must be child nodes of a GENI-based Qualcomm Universal
> +  Peripheral(QUP) wrapper. Refer to the GENI QUP wrapper binding described in
> +  Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.

[Severity: Medium]
Since I3C controller nodes must be child nodes of the GENI QUP wrapper node,
will dtbs_check fail unless Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml
is also updated to permit i3c@[0-9a-f]+$ child nodes?

Currently, qcom,geni-se.yaml sets additionalProperties: false and only allows
spi, i2c, and serial children in patternProperties:

  patternProperties:
    "spi@[0-9a-f]+$":
      ...
    "i2c@[0-9a-f]+$":
      ...
    "serial@[0-9a-f]+$":
      ...
  additionalProperties: false

Should qcom,geni-se.yaml be updated to reference /schemas/i3c/qcom,geni-i3c.yaml#
under patternProperties (and disallow it under qcom,geni-se-i2c-master-hub)?

[ ... ]

> +  pinctrl-names:
> +    minItems: 1
> +    items:
> +      - const: default
> +      - const: sleep
> +
> +  dmas:
> +    maxItems: 2
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interconnects
> +  - interconnect-names
> +  - pinctrl-0
> +  - pinctrl-1
> +  - pinctrl-names

[Severity: Medium]
Does requiring pinctrl-1 here contradict setting minItems: 1 on pinctrl-names
above?

If a board or SoC DTSI only defines the default pinctrl state (pinctrl-0 and
pinctrl-names = "default"), schema validation will fail because pinctrl-1 is
unconditionally listed in required. In addition, sibling GENI bindings such
as Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml do not
require pinctrl properties in the controller schema.

> +
> +unevaluatedProperties: false

[Severity: Medium]
Should operating-points-v2 and power-domains also be permitted in properties
before setting unevaluatedProperties: false?

Neither i3c.yaml nor qcom,se-common-props.yaml defines these properties,
whereas sibling GENI Serial Engine bindings (qcom,i2c-geni-qcom.yaml,
qcom,spi-geni-qcom.yaml, and qcom,serial-geni-qcom.yaml) declare:

  operating-points-v2: true
  power-domains:
    maxItems: 1

Because Qualcomm SoC DTSI files (such as arch/arm64/boot/dts/qcom/sm8550.dtsi)
populate power-domains and operating-points-v2 on QUP Serial Engine nodes and
the common helpers in drivers/soc/qcom/qcom-geni-se.c consume them, won't
dtbs_check reject those properties on qcom,geni-i3c nodes?

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,sm8550-gcc.h>
> +    #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
> +
> +    i3c@884000 {
> +        compatible = "qcom,geni-i3c";
> +        reg = <0x00884000 0x4000>,
> +              <0x008c0000 0x10000>;
> +        clocks = <&gcc GCC_QUPV3_WRAP2_S1_CLK>;
> +        clock-names = "se";
> +        interrupts = <GIC_SPI 583 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 796 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 797 IRQ_TYPE_LEVEL_HIGH>;
> +        interconnects = <&clk_virt MASTER_QUP_CORE_2 &clk_virt SLAVE_QUP_CORE_2>,
> +                        <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_QUP_2>,
> +                        <&aggre2_noc MASTER_QUP_2 &mc_virt SLAVE_EBI1>;

[Severity: Low]
Is 0x008c0000 the intended register address for the IBI controller in this
example, and are the headers meant to mix sm8550 and kaanapali?

In arch/arm64/boot/dts/qcom/sm8550.dtsi and kaanapali.dtsi, 0x008c0000 is the
register base of the parent QUPv3 wrapper node (geniqup@8c0000):

  qupv3_id_1: geniqup@8c0000 {
          compatible = "qcom,geni-se-qup";
          reg = <0 0x008c0000 0 0x2000>;

which geni_se_probe() in drivers/soc/qcom/qcom-geni-se.c maps via
devm_platform_ioremap_resource(pdev, 0). The example also includes both
qcom,sm8550-gcc.h and qcom,kaanapali-rpmh.h while referencing &aggre2_noc.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104111610.3739095-1-mukesh.savaliya@oss.qualcomm.com?part=1

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 1/3] dt-bindings: i3c: Add support for Qualcomm I3C controller

[Mukesh Savaliya]

Apologies for the additional email. I missed including the cover letter 
with this series.

Please ignore this series without the cover letter. I have resent the 
complete series with the cover letter included.

Sorry for the inconvenience.

On 11/4/2026 4:46 PM, Mukesh Kumar Savaliya wrote:
> Add device tree bindings for the Qualcomm GENI-based I3C controller.
> 
> The controller supports I3C SDR transfers in mixed-bus mode with both I2C
> and I3C target devices present on the same bus. The binding describes the
> I3C controller instance along with its associated In-Band Interrupt (IBI)
> controller used for handling I3C target IBI events.
> 
> Document the required resources, interrupts, clocks, and dependencies
> needed to represent the hardware in device tree.
> 
> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> ---
>   .../bindings/i3c/qcom,geni-i3c.yaml           | 115 ++++++++++++++++++
>   1 file changed, 115 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> new file mode 100644
> index 000000000000..bab89aeee23f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i3c/qcom,geni-i3c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Geni based QUP I3C Controller
> +
> +maintainers:
> +  - Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> +
> +description:
> +  Qualcomm GENI-based I3C controller implementing an MIPI I3C bus master.
> +  The controller supports SDR transfers in mixed bus mode with both I2C and
> +  I3C target devices present on the same bus.
> +
> +  The controller consists of a GENI Serial Engine (SE) responsible for I3C
> +  bus transactions and an associated dedicated IBI controller(IBI_C) block
> +  used for Hot-Join, In-Band Interrupt (IBI) handling and management of GPII
> +  resources associated with the I3C controller.
> +
> +  I3C controller nodes must be child nodes of a GENI-based Qualcomm Universal
> +  Peripheral(QUP) wrapper. Refer to the GENI QUP wrapper binding described in
> +  Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
> +
> +allOf:
> +  - $ref: i3c.yaml#
> +  - $ref: /schemas/soc/qcom/qcom,se-common-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,geni-i3c
> +
> +  reg:
> +    items:
> +      - description: SE (Serial Engine) register address space
> +      - description: IBI controller (IBI_C) register address space
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: se
> +
> +  interrupts:
> +    items:
> +      - description: Core Serial Engine (SE) IRQ
> +      - description: IBI controller (IBI_C) manager IRQ
> +      - description: IBI controller (IBI_C) GPII IRQ
> +
> +  interconnects:
> +    minItems: 2
> +    maxItems: 3
> +
> +  interconnect-names:
> +    minItems: 2
> +    items:
> +      - const: qup-core
> +      - const: qup-config
> +      - const: qup-memory
> +
> +  pinctrl-names:
> +    minItems: 1
> +    items:
> +      - const: default
> +      - const: sleep
> +
> +  dmas:
> +    maxItems: 2
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interconnects
> +  - interconnect-names
> +  - pinctrl-0
> +  - pinctrl-1
> +  - pinctrl-names
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,sm8550-gcc.h>
> +    #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
> +
> +    i3c@884000 {
> +        compatible = "qcom,geni-i3c";
> +        reg = <0x00884000 0x4000>,
> +              <0x008c0000 0x10000>;
> +        clocks = <&gcc GCC_QUPV3_WRAP2_S1_CLK>;
> +        clock-names = "se";
> +        interrupts = <GIC_SPI 583 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 796 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 797 IRQ_TYPE_LEVEL_HIGH>;
> +        interconnects = <&clk_virt MASTER_QUP_CORE_2 &clk_virt SLAVE_QUP_CORE_2>,
> +                        <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_QUP_2>,
> +                        <&aggre2_noc MASTER_QUP_2 &mc_virt SLAVE_EBI1>;
> +        interconnect-names = "qup-core", "qup-config", "qup-memory";
> +        pinctrl-0 = <&qup_i3c1_data_clk>;
> +        pinctrl-1 = <&qup_i3c1_data_clk_sleep>;
> +        pinctrl-names = "default", "sleep";
> +        #address-cells = <3>;
> +        #size-cells = <0>;
> +    };
> +...


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 1/3] dt-bindings: i3c: Add support for Qualcomm I3C controller

[Krzysztof Kozlowski]

On Wed, Nov 04, 2026 at 06:01:15PM +0530, Mukesh Savaliya wrote:
> Apologies for the additional email. I missed including the cover letter with
> this series.
> 
> Please ignore this series without the cover letter. I have resent the
> complete series with the cover letter included.
> 
> Sorry for the inconvenience.

You got here some Sashiko review, so that has to be addressed.


Best regards,
Krzysztof


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v7 0/3] Add Qualcomm i3c controller driver support

[Mukesh Kumar Savaliya]

This patchset adds i3c controller support for the qualcomm's QUPV3 based 
Serial engine (SE) hardware controller. 

The I3C SE(Serial Engine) controller implements I3C master functionality
as defined in the MIPI Specifications for I3C, Version 1.0. 

This patchset was tested on kaanapali MTP device and data transfer has been
tested in I3C SDR mode.

Features tested and supported :
  Standard CCC commands.
  I3C SDR mode private transfers in PIO mode.
  I2C transfers in PIO mode.

Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
----
Link to V6: https://lore.kernel.org/lkml/20250701071852.2107800-1-mukesh.savaliya@oss.qualcomm.com/

v6->v7 :
 - Removed inline functions from i3c-qcom-geni.c where unnecessary and changed relevant
   helpers to return error values instead of void so failures can be propagated to callers.
 - Changed priv_xfers() to i3c_xfers() as part of i3c_master_controller_ops latest update.
 - Replaced set_new_addr_slot() and clear_new_addr_slot() with direct set_bit()/clear_bit().
 - Removed unused io.h, of.h, and of_platform.h header files as suggested by Krzysztof.
 - Moved invalid address validation to the caller and made invalid addresses a hard error.
 - Used the tx_depth structure member instead of hard-coding half the FIFO size as 32 bytes.
 - Removed explicit 0 and 1 assignments from enum i3c_bus_phase since they are the default
   enum values.
 - geni_i3c_irq(): Replaced manual FIFO accesses with i3c_readl_fifo() and i3c_writel_fifo().
 - geni_i3c_irq(): Replaced spin_lock() with scoped_guard() and removed the runtime-PM
   mutex lock/unlock helpers in favor of scoped_guard(mutex) at callers.
 - geni_i3c_irq(): Removed goto usage so register clearing and completion handling remain
   outside the scoped lock.
 - geni_i3c_irq(): Kept IRQ clearance after the required interrupt handling instead of
   moving it immediately after reading IRQ status.
 - geni_i3c_perform_daa(): Changed USE_7E handling so it is sent only for the first device.
 - Replaced pm_runtime_enable() with devm_pm_runtime_enable() for device-managed runtime PM.
 - geni_i3c_master_i2c_xfers(): Corrected I2C transfers to use OPEN_DRAIN_MODE instead of
   PUSH_PULL_MODE.
 - Removed attach()/detach() framework callbacks and the unused geni_i3c_i2c_dev_data structure.
 - Kept I3C_ADDR_MASK within the I3C address range instead of the I2C address range definitions.
 - Declared loop variables directly in the for statements.
 - Combined separate bit test and clear operations using __test_and_clear_bit().
 - Removed explicit pm_runtime_mark_last_busy() calls before pm_runtime_put_autosuspend().
 - Removed unnecessary fallthrough statements from the CCC switch in senc_ccc_cmd().
 - Removed explicit NULL initializers and retained only implemented callbacks.
 - Replaced mutex_init() with devm_mutex_init() and propagated its return value.
 - Simplified the probe resource-on/resource-off flow around geni_i3c_hw_init().
 - Used devm_add_action_or_reset() with a helper to unregister the I3C master during probe
   cleanup, allowing geni_i3c_remove() to be removed.
 - Replaced SET_RUNTIME_PM_OPS() with the newer DEFINE_RUNTIME_DEV_PM_OPS() runtime-PM macro.
 - Updated [PATCH v6 2/3] commit log to clarify that the driver does not currently support IBI.
 - dt-bindings: Fixed the unevaluated-properties validation error caused by the unexpected
   clock-names property.
 - dt-bindings: Simplified the commit message and clarified the Qualcomm I3C controller
   description.
 - dt-bindings: Updated qcom,geni-se.yaml to document the Qualcomm I3C controller as a
   valid GENI SE child node.
 - dt-bindings: Added IBI controller/hardware details to complete the hardware description.
 - dt-bindings: Removed Rob's Reviewed-by tag due to above binding changes.

Link to V5: https://lore.kernel.org/all/20250420081530.2708238-1-quic_msavaliy@quicinc.com/
v5->v6 :
 - Cleaned up with the alignment and Addressed comments from Alok.
 - Kconfig change - Do not Depends on I3C. Follow same as other i3c controller drivers.
 - Probe : Do not set PM to active state as controller is not initialized yet.
 - Updated email domain from quicinc.com to oss.qualcomm.com.

Link to V4: https://lore.kernel.org/all/20250411113516.87958-1-quic_msavaliy@quicinc.com/
v4->v5:
 - Define macros value in lower case hex.
 - Removed unused macro SLV_ADDR_SHFT.
 - Used FIELD_PREP to write command into M_PARAM and removed CCC_HDR_CMD_SHFT macro.
 - Include units.h headerfile and use xHZ macros instead of local defined macro.
 - Changed PACKING_BYTES_PW to PACKING_BYTES_PER_WORD.
 - I3C_ADDR_MASK : Use I2C_ADDR_MASK instead of local define and maintain i3c purpose.
 - geni_i3c_clk_map: converted array initializations for frequency mapping as per C99 standard.
 - Removed bus NULL pointer check from geni_i3c_clk_map_idx() as it's not possible to NULL there.
 - inline functions set_new_addr_slot(), clear_new_addr_slot() and is_new_addr_slot_set().
 - geni_i3c_handle_err() : Added line space after each condition and removed %s from string print.
 - geni_i3c_irq() : Used hardcoded value 4 for sizeof(u32 val).
 - geni_i3c_irq() : simplified condition check and alignment with new variable fifo_read_status.
 - _i3c_geni_execute_command() : Reversed if and else block for natural positive look.
 - geni_i3c_perform_daa() : DCR, BCR, PID - used GENMASK and FIELD_PREP instead of manual shift.
 - geni_i3c_perform_daa() : Used parity8() function to simplify and add a comment for readability.

Link to V3: https://lore.kernel.org/lkml/20250403134644.3935983-1-quic_msavaliy@quicinc.com/T/
v3->v4:
 - Dropped "clock-names" property from dt-bindings as suggested by krzysztof.
 - Makefile: Correct order sequence for i3c-qcom-geni.c.
 - Indentation corrected around print statement.
 - geni_i3c_probe() : Exit with return 0 instead of ret for success.
 - Added sparse annotations around i3c_geni_runtime_get_mutex_lock()/_unlock().

Link to V2: https://patchwork.kernel.org/project/linux-arm-msm/cover/20250326141641.3471906-1-quic_msavaliy@quicinc.com/
v2 -> v3:
 - Removed "master" word from compatible and dt-bindings filename.
 - Changed compatible qcom,i3c-master to qcom,geni-i3c.
 - Renamed qcom,i3c-master.yaml to qcom,geni-i3c.yaml matchin to compatible name.
 - Removed const from compatible property from yaml.
 - Changed driver file name from qcom-i3c-master.c to i3c-qcom-geni.c.
 - Changed Kconfig and Makefile accordingly as per driver filename changes.
 - Changed se-clk to se inside devm_clk_get(&pdev->dev, "se-clk").
 - Removed "se-clock-frequency" read from DTSI and fixed frequency within driver.

Link to V1: https://lore.kernel.org/lkml/20250205143109.2955321-1-quic_msavaliy@quicinc.com/
v1-> v2 :
 - Removed bindings word from subject title of dt-bindings patch.
 - Use Controller name instead of Master as per MIPI alliance guidance and updated title.
 - Added description field for the i3c master into dt-bindings.
 - Changed title to "Qualcomm Geni based QUP I3C Controller".
 - Changed compatible to "qcom,i3c-master" matching dt-binding file and driver.
 - Changed "interrupts-extended" property to "interrupts" as suggested by krzysztof.
 - Dropped reg, clock minItems and added maxItems similar to other dt-bindings.
 - Removed clock-names property from dt-bindings suggested by Krzysztof, Bjorn.
 - Set "se-clock-frequency"  set it within drivers as suggested by Rob.
 - Removed "dfs-index" property and manage it within driver as suggested by Rob.
 - Removed "interrupts" maxItems as we need only 1 interrupt in this change.
 - Added comment for mutex lock mentioning purpose in sruct geni_i3c_dev .
 - Return with dev_err_probe() instead of error log and then return -ENXIO from probe().
 - Removed dev_dbg(&pdev->dev, "Geni I3C probed\n") print log as suggested by krzysztof.
 - Removed CONFIG_PM and else part around runtime PM operations following other drivers.
 - Removed Module alias MODULE_ALIAS("platform:geni_i3c_master").
 - Replaced MASTER with GENI in the Title of MAINTAINER file.
 - Removed duplications from the commit log and removed unwanted statement.
 - Formatted license and copyright similar to other files.
 - Removed SLV_ADDR_MSK and used FIELD_PREP/FIELD_GET instead of local bit shifting operations.
 - Used direct bit positions for each internal Error bit of DM_I3C_CB_ERR.
 - Removed Unused SLV_ADDR_MSK and added SLAVE_ADDR_MASK as GENMASK(15,9).
 - Renamed spinlock as irq_lock.
 - Removed dfs_idx from geni_i3c_dev and made it local inside qcom_geni_i3c_conf().
 - Use boolean cur_is_write instead of enum i3c_trans_dir/gi3c->cur_rnw.
 - Used DECLARE_BITMAP and related set/clear_bit APIs instead of manual operation.
 - Inline the error messages from geni_i3c_err_log directly to improve readability
   and avoid unnecessary jumps caused by the geni_i3c_err_code enum.
 - Converted clk_src_freq of struct geni_i3c_clk_settings to HZ.
 - Removed unwanted debug logs from geni_i3c_clk_map_idx().
 - clk_od_fld and itr renamed to clk_od_idx and clk_idx respectively to map actual usage.
 - Added se-clock-frequency to be read from DTSI, if none, then default to 100MHz source.
 - Changed Error log during bus_init() if OD and PP mode frequencies avaiable or not.
 - Used FIELD_PREP and standard BIT operations inside qcom_geni_i3c_conf() instead manual shifting.
 - Removed unnecessary parentheses from geni_i3c_irq().
 - Moved geni_se_abort_m_cmd() implementation to a new helper function geni_i3c_abort_xfer().
 - Removed unwanted reinitialization of cur_len, cur_idx, cur_rnw from _i3c_geni_execute_command().
 - Removed dev_dbg logs which were meant for developmental debug purpose.
 - Removed unnecessary check nxfers <= 0 from geni_i3c_master_priv_xfers().
 - Replaced devm_kzalloc() by kzalloc() inside geni_i3c_master_attach_i2c_dev() to use
   kfree() from counter function geni_i3c_master_detach_i2c_dev().
 - Replaced devm_kzalloc() by kzalloc() inside geni_i3c_master_attach_i3c_dev() to use
   kfree() from counter function geni_i3c_master_detach_i3c_dev().   
 - Removed geni_i3c_master_reattach_i3c_dev() function as default returns 0.
 - Removed goto label from geni_i3c_master_bus_init() by reorganizing internal code.
   Also used i3c_geni_runtime_get_mutex_lock()/unlock() instead of get_sync() similar to other places.
 - Added indent to fallthrough for switch cases inside geni_i3c_master_supports_ccc_cmd().
 - Renamed i3c_geni_rsrcs_init() to i3c_geni_resources_init().
 - Changed devm_ioremap_resource() to devm_platform_ioremap_resource(), removed platform_get_resource().
 - Replaced dev_err() with dev_err_probe() for core clock named se-clk.
 - Removed development debug prints for votings from gi3c->se.icc_paths.
 - Probe(): Changed all dev_err() to dev_error_probe() with proper log messages.
 - Probe(): Moved static resource allocation immediately after gi3c object allocation.
 - Probe(): Disabled PM if i3c master registration fails during probe().
 - Remove(): Unregister master first and then added Disable of PM as opposite to probe().
 - Removed I3C_CCC_ENTHDR support as it's not supported.

---

Mukesh Kumar Savaliya (3):
  dt-bindings: i3c: Add support for Qualcomm I3C controller
  i3c: master: Add Qualcomm I3C controller driver
  MAINTAINERS: Add maintainer for Qualcomm's I3C controller driver

 .../bindings/i3c/qcom,geni-i3c.yaml           |  115 ++
 MAINTAINERS                                   |    8 +
 drivers/i3c/master/Kconfig                    |   12 +
 drivers/i3c/master/Makefile                   |    1 +
 drivers/i3c/master/i3c-qcom-geni.c            | 1133 +++++++++++++++++
 5 files changed, 1269 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
 create mode 100644 drivers/i3c/master/i3c-qcom-geni.c

-- 
2.43.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

[PATCH v7 1/3] dt-bindings: i3c: Add support for Qualcomm I3C controller

[Mukesh Kumar Savaliya]

Add device tree bindings for the Qualcomm GENI-based I3C controller.

The controller supports I3C SDR transfers in mixed-bus mode with both I2C
and I3C target devices present on the same bus. The binding describes the
I3C controller instance along with its associated In-Band Interrupt (IBI)
controller used for handling I3C target IBI events.

Document the required resources, interrupts, clocks, and dependencies
needed to represent the hardware in device tree.

Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
---
 .../bindings/i3c/qcom,geni-i3c.yaml           | 115 ++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml

diff --git a/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
new file mode 100644
index 000000000000..bab89aeee23f
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
@@ -0,0 +1,115 @@
+# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i3c/qcom,geni-i3c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Geni based QUP I3C Controller
+
+maintainers:
+  - Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
+
+description:
+  Qualcomm GENI-based I3C controller implementing an MIPI I3C bus master.
+  The controller supports SDR transfers in mixed bus mode with both I2C and
+  I3C target devices present on the same bus.
+
+  The controller consists of a GENI Serial Engine (SE) responsible for I3C
+  bus transactions and an associated dedicated IBI controller(IBI_C) block
+  used for Hot-Join, In-Band Interrupt (IBI) handling and management of GPII
+  resources associated with the I3C controller.
+
+  I3C controller nodes must be child nodes of a GENI-based Qualcomm Universal
+  Peripheral(QUP) wrapper. Refer to the GENI QUP wrapper binding described in
+  Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
+
+allOf:
+  - $ref: i3c.yaml#
+  - $ref: /schemas/soc/qcom/qcom,se-common-props.yaml#
+
+properties:
+  compatible:
+    const: qcom,geni-i3c
+
+  reg:
+    items:
+      - description: SE (Serial Engine) register address space
+      - description: IBI controller (IBI_C) register address space
+
+  clocks:
+    maxItems: 1
+
+  clock-names:
+    const: se
+
+  interrupts:
+    items:
+      - description: Core Serial Engine (SE) IRQ
+      - description: IBI controller (IBI_C) manager IRQ
+      - description: IBI controller (IBI_C) GPII IRQ
+
+  interconnects:
+    minItems: 2
+    maxItems: 3
+
+  interconnect-names:
+    minItems: 2
+    items:
+      - const: qup-core
+      - const: qup-config
+      - const: qup-memory
+
+  pinctrl-names:
+    minItems: 1
+    items:
+      - const: default
+      - const: sleep
+
+  dmas:
+    maxItems: 2
+
+  dma-names:
+    items:
+      - const: tx
+      - const: rx
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - interrupts
+  - interconnects
+  - interconnect-names
+  - pinctrl-0
+  - pinctrl-1
+  - pinctrl-names
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/clock/qcom,sm8550-gcc.h>
+    #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
+
+    i3c@884000 {
+        compatible = "qcom,geni-i3c";
+        reg = <0x00884000 0x4000>,
+              <0x008c0000 0x10000>;
+        clocks = <&gcc GCC_QUPV3_WRAP2_S1_CLK>;
+        clock-names = "se";
+        interrupts = <GIC_SPI 583 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 796 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 797 IRQ_TYPE_LEVEL_HIGH>;
+        interconnects = <&clk_virt MASTER_QUP_CORE_2 &clk_virt SLAVE_QUP_CORE_2>,
+                        <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_QUP_2>,
+                        <&aggre2_noc MASTER_QUP_2 &mc_virt SLAVE_EBI1>;
+        interconnect-names = "qup-core", "qup-config", "qup-memory";
+        pinctrl-0 = <&qup_i3c1_data_clk>;
+        pinctrl-1 = <&qup_i3c1_data_clk_sleep>;
+        pinctrl-names = "default", "sleep";
+        #address-cells = <3>;
+        #size-cells = <0>;
+    };
+...
-- 
2.43.0


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 1/3] dt-bindings: i3c: Add support for Qualcomm I3C controller

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Low] dt-bindings: i3c: qcom,geni-se.yaml missing i3c child node pattern
- [Low] dt-bindings: i3c: qcom,geni-i3c.yaml requires optional pinctrl-1 state
- [Low] dt-bindings: i3c: qcom,geni-i3c.yaml omits power-domains and OPP properties
- [Low] dt-bindings: i3c: qcom,geni-i3c.yaml example uses QUP base for IBI_C reg
--

commit 2e47e899944b24dd4c6fc08e4da686257bf4137c
Author: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>

dt-bindings: i3c: Add support for Qualcomm I3C controller

This commit adds the device tree binding schema for the Qualcomm GENI-based
I3C controller and its associated In-Band Interrupt controller.

> diff --git a/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> new file mode 100644
> index 0000000000000..bab89aeee23f4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> @@ -0,0 +1,115 @@

[ ... ]

> +  I3C controller nodes must be child nodes of a GENI-based Qualcomm Universal
> +  Peripheral(QUP) wrapper. Refer to the GENI QUP wrapper binding described in
> +  Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.

[Severity: Low]
Will device tree validation fail when an i3c child node is placed under a
GENI QUP wrapper node?

In Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml,
patternProperties only permits spi, i2c, and serial child nodes while
setting additionalProperties: false:

patternProperties:
  "spi@[0-9a-f]+$":
    ...
    $ref: /schemas/spi/qcom,spi-geni-qcom.yaml#

  "i2c@[0-9a-f]+$":
    ...
    $ref: /schemas/i2c/qcom,i2c-geni-qcom.yaml#

  "serial@[0-9a-f]+$":
    ...
    $ref: /schemas/serial/qcom,serial-geni-qcom.yaml#
...
additionalProperties: false

Should Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml also be
updated to allow i3c@[0-9a-f]+$ referencing /schemas/i3c/qcom,geni-i3c.yaml#
(and disallow it for qcom,geni-se-i2c-master-hub)?

> +
> +allOf:
> +  - $ref: i3c.yaml#
> +  - $ref: /schemas/soc/qcom/qcom,se-common-props.yaml#
> +
> +properties:

[ ... ]

> +  pinctrl-names:
> +    minItems: 1
> +    items:
> +      - const: default
> +      - const: sleep
> +
> +  dmas:
> +    maxItems: 2
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interconnects
> +  - interconnect-names
> +  - pinctrl-0
> +  - pinctrl-1
> +  - pinctrl-names

[Severity: Low]
Should pinctrl-1 (and pinctrl-0 / pinctrl-names) be dropped from the
required list, and pinctrl-0: true and pinctrl-1: true be declared in
properties?

Under properties, pinctrl-names specifies minItems: 1 so that the sleep
state is optional, which matches the sibling bindings in
Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml and
Documentation/devicetree/bindings/spi/qcom,spi-geni-qcom.yaml. However,
listing pinctrl-1 in required forces every node to provide a second pinctrl
state and causes dtbs_check failures for boards or SoC dtsi files that only
define the default state.

> +
> +unevaluatedProperties: false

[Severity: Low]
What happens during dtbs_check when a qcom,geni-i3c node specifies
power-domains, operating-points-v2, or required-opps?

Sibling GENI Serial Engine bindings such as
Documentation/devicetree/bindings/i2c/qcom,i2c-geni-qcom.yaml define:

  operating-points-v2: true
...
  power-domains:
    maxItems: 1
...
  required-opps: true

Because neither i3c.yaml nor qcom,se-common-props.yaml defines these
properties and unevaluatedProperties is set to false here, won't QUPv3
Serial Engine nodes that use power-domains and OPP tables fail schema
validation?

> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,sm8550-gcc.h>
> +    #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
> +
> +    i3c@884000 {
> +        compatible = "qcom,geni-i3c";
> +        reg = <0x00884000 0x4000>,
> +              <0x008c0000 0x10000>;

[Severity: Low]
Is 0x008c0000 the intended address for the IBI controller register space in
this example?

In arch/arm64/boot/dts/qcom/kaanapali.dtsi and
arch/arm64/boot/dts/qcom/sm8550.dtsi, 0x008c0000 is the register base of the
parent QUPv3 wrapper node (geniqup@8c0000) that contains SE1 at 0x00884000:

		qupv3_2: geniqup@8c0000 {
			compatible = "qcom,geni-se-qup";
			reg = <0x0 0x008c0000 0x0 0x2000>;

Also, this example mixes the SM8550 clock header with the Kaanapali
interconnect header. Could the example use consistent headers and a valid
IBI controller register address?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261104122155.3789346-1-mukesh.savaliya@oss.qualcomm.com?part=1

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c

Re: [PATCH v7 1/3] dt-bindings: i3c: Add support for Qualcomm I3C controller

[Frank Li]

On Wed, Nov 04, 2026 at 05:51:53PM +0530, Mukesh Kumar Savaliya wrote:
> Add device tree bindings for the Qualcomm GENI-based I3C controller.
>
> The controller supports I3C SDR transfers in mixed-bus mode with both I2C
> and I3C target devices present on the same bus. The binding describes the
> I3C controller instance along with its associated In-Band Interrupt (IBI)
> controller used for handling I3C target IBI events.
>
> Document the required resources, interrupts, clocks, and dependencies
> needed to represent the hardware in device tree.
>
> Signed-off-by: Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> ---
>  .../bindings/i3c/qcom,geni-i3c.yaml           | 115 ++++++++++++++++++
>  1 file changed, 115 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
>
> diff --git a/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> new file mode 100644
> index 000000000000..bab89aeee23f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/qcom,geni-i3c.yaml
> @@ -0,0 +1,115 @@
> +# SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i3c/qcom,geni-i3c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Geni based QUP I3C Controller
> +
> +maintainers:
> +  - Mukesh Kumar Savaliya <mukesh.savaliya@oss.qualcomm.com>
> +
> +description:
> +  Qualcomm GENI-based I3C controller implementing an MIPI I3C bus master.
> +  The controller supports SDR transfers in mixed bus mode with both I2C and
> +  I3C target devices present on the same bus.
> +
> +  The controller consists of a GENI Serial Engine (SE) responsible for I3C
> +  bus transactions and an associated dedicated IBI controller(IBI_C) block
> +  used for Hot-Join, In-Band Interrupt (IBI) handling and management of GPII
> +  resources associated with the I3C controller.
> +
> +  I3C controller nodes must be child nodes of a GENI-based Qualcomm Universal
> +  Peripheral(QUP) wrapper. Refer to the GENI QUP wrapper binding described in
> +  Documentation/devicetree/bindings/soc/qcom/qcom,geni-se.yaml.
> +
> +allOf:
> +  - $ref: i3c.yaml#
> +  - $ref: /schemas/soc/qcom/qcom,se-common-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: qcom,geni-i3c
> +
> +  reg:
> +    items:
> +      - description: SE (Serial Engine) register address space
> +      - description: IBI controller (IBI_C) register address space
> +
> +  clocks:
> +    maxItems: 1
> +
> +  clock-names:
> +    const: se

generally, needn't clock-names if only one clock

> +
> +  interrupts:
> +    items:
> +      - description: Core Serial Engine (SE) IRQ
> +      - description: IBI controller (IBI_C) manager IRQ
> +      - description: IBI controller (IBI_C) GPII IRQ
> +
> +  interconnects:
> +    minItems: 2
> +    maxItems: 3
> +
> +  interconnect-names:
> +    minItems: 2
> +    items:
> +      - const: qup-core
> +      - const: qup-config
> +      - const: qup-memory
> +
> +  pinctrl-names:
> +    minItems: 1
> +    items:
> +      - const: default
> +      - const: sleep

Needn't pinctrl.

> +
> +  dmas:
> +    maxItems: 2
> +
> +  dma-names:
> +    items:
> +      - const: tx
> +      - const: rx
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - interrupts
> +  - interconnects
> +  - interconnect-names
> +  - pinctrl-0
> +  - pinctrl-1
> +  - pinctrl-names

pinctrl-names allow 1 items, why require pinctrl-1.

Frank
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #include <dt-bindings/clock/qcom,sm8550-gcc.h>
> +    #include <dt-bindings/interconnect/qcom,kaanapali-rpmh.h>
> +
> +    i3c@884000 {
> +        compatible = "qcom,geni-i3c";
> +        reg = <0x00884000 0x4000>,
> +              <0x008c0000 0x10000>;
> +        clocks = <&gcc GCC_QUPV3_WRAP2_S1_CLK>;
> +        clock-names = "se";
> +        interrupts = <GIC_SPI 583 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 796 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 797 IRQ_TYPE_LEVEL_HIGH>;
> +        interconnects = <&clk_virt MASTER_QUP_CORE_2 &clk_virt SLAVE_QUP_CORE_2>,
> +                        <&gem_noc MASTER_APPSS_PROC &config_noc SLAVE_QUP_2>,
> +                        <&aggre2_noc MASTER_QUP_2 &mc_virt SLAVE_EBI1>;
> +        interconnect-names = "qup-core", "qup-config", "qup-memory";
> +        pinctrl-0 = <&qup_i3c1_data_clk>;
> +        pinctrl-1 = <&qup_i3c1_data_clk_sleep>;
> +        pinctrl-names = "default", "sleep";
> +        #address-cells = <3>;
> +        #size-cells = <0>;
> +    };
> +...
> --
> 2.43.0
>

-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c