linux-renesas-soc.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E
@ 2025-06-11  9:39 Wolfram Sang
  2025-06-11  9:39 ` [PATCH RFC 1/7] clk: renesas: r9a08g045: Add I3C clocks, resets and power domain Wolfram Sang
                   ` (7 more replies)
  0 siblings, 8 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11  9:39 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Tommaso Merciai, Wolfram Sang, Alexandre Belloni, Conor Dooley,
	devicetree, Frank Li, Geert Uytterhoeven, Gustavo A. R. Silva,
	Kees Cook, Krzysztof Kozlowski, linux-clk, linux-hardening,
	linux-i3c, Magnus Damm, Michael Turquette, Philipp Zabel,
	Rob Herring, Stephen Boyd

Here is finally the first RFC of a driver for the Renesas I3C IP. It was
created by merging two versions of it from two different BSPs. Then,
improved according to code analyzers, cleaned up with regard to coding
style, and then refactored to hopefully match I3C subsystem standards.

It is a basic driver for the I3C IP found in various SoCs like RZ/G3S
and G3E. Missing features to be added incrementally are IBI, HotJoin and
maybe target support. Other than that, this driver has been tested with
I3C pure busses (2 targets) and mixed busses (2 I3C + various I2C
targets). DAA and reading/writing to the temperature sensors worked
reliably at different speeds. Scoping the bus, the output from the
protocol analyzer seems reasonable, too. But hey, I am still new to all
this, so I might have overlooked something.

The first patches are needed to enable I3C on the RZ/G3S and G3E boards.
Once this series loses RFC status, they will be sent out individually,
of course. All is on top of 6.16-rc1. A branch can be found here:

git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/g3s/i3c

Why is this still RFC?

- On G3E (but not G3S), we get a spurious irq during boot. We are
  working on it. This is just platform dependent, though, kind of
  independent of the high level design of the driver. For this, we
  would love to get comments already. So, we can fix things in parallel

- G3S has 17 irqs, G3E only 16. The way we handle this might need
  discussion (see patch 3)

- On G3S, clocks are named 'i3c' while on G3E they are named 'i3c0'
  I don't have all the needed docs for this, but Tommaso can surely
  figure this out meanwhile

- There are some open questions regarding the driver itself
  (see patch 4)

Really looking forward to comments! This has been quite a ride. Getting
a suitable test setup was a surprisingly big task. If someone knows an
off-the-shelf device supporting HotJoin, I am all ears. I couldn't find
one.

So much for now here, some patches have more details.

All the best,

   Wolfram


Quynh Nguyen (1):
  arm64: dts: renesas: r9a08g045: Add I3C node

Tommaso Merciai (3):
  clk: renesas: r9a09g047: Add I3C0 clocks and resets
  dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  arm64: dts: renesas: r9a09g047: Add I3C node

Wolfram Sang (3):
  clk: renesas: r9a08g045: Add I3C clocks, resets and power domain
  i3c: add driver for Renesas I3C IP
  WIP: arm64: dts: renesas: rzg3s-smarc-som: Enable I3C

 .../devicetree/bindings/i3c/renesas,i3c.yaml  |  186 +++
 MAINTAINERS                                   |    7 +
 arch/arm64/boot/dts/renesas/r9a08g045.dtsi    |   35 +
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |   35 +
 .../boot/dts/renesas/rzg3s-smarc-som.dtsi     |   33 +
 drivers/clk/renesas/r9a08g045-cpg.c           |    7 +
 drivers/clk/renesas/r9a09g047-cpg.c           |    8 +
 drivers/i3c/master/Kconfig                    |   10 +
 drivers/i3c/master/Makefile                   |    1 +
 drivers/i3c/master/renesas-i3c.c              | 1441 +++++++++++++++++
 10 files changed, 1763 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
 create mode 100644 drivers/i3c/master/renesas-i3c.c

-- 
2.47.2


^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH RFC 1/7] clk: renesas: r9a08g045: Add I3C clocks, resets and power domain
  2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
@ 2025-06-11  9:39 ` Wolfram Sang
  2025-06-19 12:09   ` Geert Uytterhoeven
  2025-06-11  9:39 ` [PATCH RFC 2/7] clk: renesas: r9a09g047: Add I3C0 clocks and resets Wolfram Sang
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11  9:39 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Tommaso Merciai, Wolfram Sang, Claudiu Beznea, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-clk

Clocks extracted from the BSP driver and rebased. Power domain handling
added by Claudiu.

Co-developed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/clk/renesas/r9a08g045-cpg.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/clk/renesas/r9a08g045-cpg.c b/drivers/clk/renesas/r9a08g045-cpg.c
index 4035f3443598..afc23bb25181 100644
--- a/drivers/clk/renesas/r9a08g045-cpg.c
+++ b/drivers/clk/renesas/r9a08g045-cpg.c
@@ -189,6 +189,7 @@ static const struct cpg_core_clk r9a08g045_core_clks[] __initconst = {
 	DEF_FIXED("OSC2", R9A08G045_OSCCLK2, CLK_EXTAL, 1, 3),
 	DEF_FIXED("HP", R9A08G045_CLK_HP, CLK_PLL6, 1, 2),
 	DEF_FIXED("TSU", R9A08G045_CLK_TSU, CLK_PLL2_DIV2, 1, 8),
+	DEF_FIXED("P5", R9A08G045_CLK_P5, CLK_PLL2_DIV2, 1, 4),
 };
 
 static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = {
@@ -243,6 +244,8 @@ static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = {
 	DEF_MOD("adc_adclk",		R9A08G045_ADC_ADCLK, R9A08G045_CLK_TSU, 0x5a8, 0),
 	DEF_MOD("adc_pclk",		R9A08G045_ADC_PCLK, R9A08G045_CLK_TSU, 0x5a8, 1),
 	DEF_MOD("tsu_pclk",		R9A08G045_TSU_PCLK, R9A08G045_CLK_TSU, 0x5ac, 0),
+	DEF_MOD("i3c_pclk",		R9A08G045_I3C_PCLK, R9A08G045_CLK_TSU, 0x610, 0),
+	DEF_MOD("i3c_tclk",		R9A08G045_I3C_TCLK, R9A08G045_CLK_P5, 0x610, 1),
 	DEF_MOD("vbat_bclk",		R9A08G045_VBAT_BCLK, R9A08G045_OSCCLK, 0x614, 0),
 };
 
@@ -282,6 +285,8 @@ static const struct rzg2l_reset r9a08g045_resets[] = {
 	DEF_RST(R9A08G045_ADC_PRESETN, 0x8a8, 0),
 	DEF_RST(R9A08G045_ADC_ADRST_N, 0x8a8, 1),
 	DEF_RST(R9A08G045_TSU_PRESETN, 0x8ac, 0),
+	DEF_RST(R9A08G045_I3C_TRESETN, 0x910, 0),
+	DEF_RST(R9A08G045_I3C_PRESETN, 0x910, 1),
 	DEF_RST(R9A08G045_VBAT_BRESETN, 0x914, 0),
 };
 
@@ -358,6 +363,8 @@ static const struct rzg2l_cpg_pm_domain_init_data r9a08g045_pm_domains[] = {
 				DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(14)), 0),
 	DEF_PD("tsu",		R9A08G045_PD_TSU,
 				DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(15)), 0),
+	DEF_PD("i3c",		R9A08G045_PD_I3C,
+				DEF_REG_CONF(CPG_BUS_MCPU3_MSTOP, BIT(10)), 0),
 	DEF_PD("vbat",		R9A08G045_PD_VBAT,
 				DEF_REG_CONF(CPG_BUS_MCPU3_MSTOP, BIT(8)),
 				GENPD_FLAG_ALWAYS_ON),
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH RFC 2/7] clk: renesas: r9a09g047: Add I3C0 clocks and resets
  2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
  2025-06-11  9:39 ` [PATCH RFC 1/7] clk: renesas: r9a08g045: Add I3C clocks, resets and power domain Wolfram Sang
@ 2025-06-11  9:39 ` Wolfram Sang
  2025-06-19 12:16   ` Geert Uytterhoeven
  2025-06-11  9:39 ` [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller Wolfram Sang
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11  9:39 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Tommaso Merciai, Wolfram Sang, Geert Uytterhoeven,
	Michael Turquette, Stephen Boyd, linux-clk

From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Add Renesas RZ/G3E R9A09G047 I3C0 clocks and reset support into
cpg driver.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 drivers/clk/renesas/r9a09g047-cpg.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/clk/renesas/r9a09g047-cpg.c b/drivers/clk/renesas/r9a09g047-cpg.c
index 21699999cedd..3e50447ed9f3 100644
--- a/drivers/clk/renesas/r9a09g047-cpg.c
+++ b/drivers/clk/renesas/r9a09g047-cpg.c
@@ -160,6 +160,12 @@ static const struct rzv2h_mod_clk r9a09g047_mod_clks[] __initconst = {
 						BUS_MSTOP(5, BIT(13))),
 	DEF_MOD("scif_0_clk_pck",		CLK_PLLCM33_DIV16, 8, 15, 4, 15,
 						BUS_MSTOP(3, BIT(14))),
+	DEF_MOD("i3c_0_pclkrw",			CLK_PLLCLN_DIV16, 9, 0, 4, 16,
+						BUS_MSTOP(10, BIT(15))),
+	DEF_MOD("i3c_0_pclk",			CLK_PLLCLN_DIV16, 9, 1, 4, 17,
+						BUS_MSTOP(10, BIT(15))),
+	DEF_MOD("i3c_0_tclk",			CLK_PLLCLN_DIV8, 9, 2, 4, 18,
+						BUS_MSTOP(10, BIT(15))),
 	DEF_MOD("riic_8_ckm",			CLK_PLLCM33_DIV16, 9, 3, 4, 19,
 						BUS_MSTOP(3, BIT(13))),
 	DEF_MOD("riic_0_ckm",			CLK_PLLCLN_DIV16, 9, 4, 4, 20,
@@ -239,6 +245,8 @@ static const struct rzv2h_reset r9a09g047_resets[] __initconst = {
 	DEF_RST(7, 7, 3, 8),		/* WDT_2_RESET */
 	DEF_RST(7, 8, 3, 9),		/* WDT_3_RESET */
 	DEF_RST(9, 5, 4, 6),		/* SCIF_0_RST_SYSTEM_N */
+	DEF_RST(9, 6, 4, 7),		/* I3C_0_PRESETN */
+	DEF_RST(9, 7, 4, 8),		/* I3C_0_TRESETN */
 	DEF_RST(9, 8, 4, 9),		/* RIIC_0_MRST */
 	DEF_RST(9, 9, 4, 10),		/* RIIC_1_MRST */
 	DEF_RST(9, 10, 4, 11),		/* RIIC_2_MRST */
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
  2025-06-11  9:39 ` [PATCH RFC 1/7] clk: renesas: r9a08g045: Add I3C clocks, resets and power domain Wolfram Sang
  2025-06-11  9:39 ` [PATCH RFC 2/7] clk: renesas: r9a09g047: Add I3C0 clocks and resets Wolfram Sang
@ 2025-06-11  9:39 ` Wolfram Sang
  2025-06-11 15:40   ` Frank Li
  2025-06-25 20:07   ` Rob Herring
  2025-06-11  9:39 ` [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP Wolfram Sang
                   ` (4 subsequent siblings)
  7 siblings, 2 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11  9:39 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Tommaso Merciai, Wolfram Sang, Alexandre Belloni, Frank Li,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, linux-i3c, devicetree

From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Available in RZ/G3S and G3E SoCs. The G3S has 17 interrupts, the G3E 16
with the "HDR exit" interrupt missing. This interrupt is put to the end
of the list, so we can handle the difference using "minItems" instead of
defining a separate interrupt list per SoC.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Tommaso also did a version with two interrupt lists and using 'oneOf'. I
prefer this version but maybe the other one is preferred upstream?

 .../devicetree/bindings/i3c/renesas,i3c.yaml  | 186 ++++++++++++++++++
 1 file changed, 186 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/i3c/renesas,i3c.yaml

diff --git a/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml b/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
new file mode 100644
index 000000000000..e6e6c57bb56c
--- /dev/null
+++ b/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
@@ -0,0 +1,186 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/i3c/renesas,i3c.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Renesas I3C Bus Interface
+
+maintainers:
+  - Wolfram Sang <wsa+renesas@sang-engineering.com>
+  - Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - renesas,r9a08g045-i3c # RZ/G3S
+          - renesas,r9a09g047-i3c # RZ/G3E
+      - const: renesas,i3c
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    items:
+      - description: Non-recoverable internal error interrupt
+      - description: Normal transfer error interrupt
+      - description: Normal transfer abort interrupt
+      - description: Normal response status buffer full interrupt
+      - description: Normal command buffer empty interrupt
+      - description: Normal IBI status buffer full interrupt
+      - description: Normal Rx data buffer full interrupt
+      - description: Normal Tx data buffer empty interrupt
+      - description: Normal receive status buffer full interrupt
+      - description: START condition detection interrupt
+      - description: STOP condition detection interrupt
+      - description: Transmit end interrupt
+      - description: NACK detection interrupt
+      - description: Arbitration lost interrupt
+      - description: Timeout detection interrupt
+      - description: Wake-up condition detection interrupt
+      - description: HDR Exit Pattern detection interrupt
+    minItems: 16
+
+  interrupt-names:
+    items:
+      - const: ierr
+      - const: terr
+      - const: abort
+      - const: resp
+      - const: cmd
+      - const: ibi
+      - const: rx
+      - const: tx
+      - const: rcv
+      - const: st
+      - const: sp
+      - const: tend
+      - const: nack
+      - const: al
+      - const: tmo
+      - const: wu
+      - const: exit
+    minItems: 16
+
+  clocks:
+    oneOf:
+      - items:
+          - description: APB bus clock
+          - description: transfer clock
+      - items:
+          - description: APB bus clock
+          - description: SFRs clock
+          - description: transfer clock
+
+  clock-names:
+    oneOf:
+      - items:
+          - const: pclk
+          - const: tclk
+      - items:
+          - const: pclk
+          - const: pclkrw
+          - const: tclk
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    items:
+      - description: Reset signal
+      - description: APB interface reset signal/SCAN reset signal
+
+  reset-names:
+    items:
+      - const: presetn
+      - const: tresetn
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clock-names
+  - clocks
+  - power-domains
+  - resets
+  - reset-names
+
+allOf:
+  - $ref: i3c.yaml#
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a08g045-i3c
+    then:
+      properties:
+        clocks:
+          maxItems: 2
+        clock-names:
+          maxItems: 2
+        interrupts:
+          minItems: 17
+        interrupt-names:
+          minItems: 17
+
+  - if:
+      properties:
+        compatible:
+          contains:
+            const: renesas,r9a09g047-i3c
+    then:
+      properties:
+        clocks:
+          minItems: 3
+        clock-names:
+          minItems: 3
+        interrupts:
+          maxItems: 16
+        interrupt-names:
+          maxItems: 16
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/r9a08g045-cpg.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+
+    i3c@1005b000 {
+        compatible = "renesas,r9a08g045-i3c", "renesas,i3c";
+        reg = <0x1005b000 0x1000>;
+        clocks = <&cpg CPG_MOD R9A08G045_I3C_PCLK>,
+                 <&cpg CPG_MOD R9A08G045_I3C_TCLK>;
+        clock-names = "pclk", "tclk";
+        interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 293 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 294 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 295 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 296 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 297 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 298 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 299 IRQ_TYPE_EDGE_RISING>,
+                     <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
+                     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
+        interrupt-names = "ierr", "terr", "abort", "resp",
+                          "cmd", "ibi", "rx", "tx", "rcv",
+                          "st", "sp", "tend", "nack",
+                          "al", "tmo", "wu", "exit";
+        resets = <&cpg R9A08G045_I3C_PRESETN>,
+                 <&cpg R9A08G045_I3C_TRESETN>;
+        reset-names = "presetn", "tresetn";
+        power-domains = <&cpg>;
+        #address-cells = <3>;
+        #size-cells = <0>;
+    };
+...
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
                   ` (2 preceding siblings ...)
  2025-06-11  9:39 ` [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller Wolfram Sang
@ 2025-06-11  9:39 ` Wolfram Sang
  2025-06-11 16:37   ` Frank Li
  2025-06-11  9:39 ` [PATCH RFC 5/7] arm64: dts: renesas: r9a08g045: Add I3C node Wolfram Sang
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11  9:39 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Tommaso Merciai, Wolfram Sang, Alexandre Belloni, Frank Li,
	Kees Cook, Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening

This adds a basic driver for the I3C IP found in various SoCs like
RZ/G3S and G3E. Missing features to be added incrementally are IBI,
HotJoin and target support. Other than that, this driver has been tested
with I3C pure busses (2 targets) and mixed busses (2 I3C + various I2C
targets). DAA and reading/writing to the temperature sensors worked
reliably at different speeds.

Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---

Open questions:

- RENESAS_I3C_MAX_DEVS is a constant currently. So, we could get rid of
  the 'maxdevs' variable. I tend to keep it this way in case future
  controllers may use a different value, then we can change it easily.

- I am not exactly happy with "renesas,i3c" as the generic fallback
  compatible. But I couldn't derive a better one from the docs.
  Suggestions welcome.

- For accessing the FIFOs, this driver uses the same code as existing
  upstream drivers or the recenlty submitted "ADI" driver. There, the
  question came up, if this could be a helper function?

 MAINTAINERS                      |    7 +
 drivers/i3c/master/Kconfig       |   10 +
 drivers/i3c/master/Makefile      |    1 +
 drivers/i3c/master/renesas-i3c.c | 1441 ++++++++++++++++++++++++++++++
 4 files changed, 1459 insertions(+)
 create mode 100644 drivers/i3c/master/renesas-i3c.c

diff --git a/MAINTAINERS b/MAINTAINERS
index a92290fffa16..a90daa29b48b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11458,6 +11458,13 @@ S:	Maintained
 F:	Documentation/devicetree/bindings/i3c/cdns,i3c-master.yaml
 F:	drivers/i3c/master/i3c-master-cdns.c
 
+I3C DRIVER FOR RENESAS
+M:	Wolfram Sang <wsa+renesas@sang-engineering.com>
+M:	Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
+S:	Supported
+F:	Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
+F:	drivers/i3c/master/renesas-i3c.c
+
 I3C DRIVER FOR SYNOPSYS DESIGNWARE
 S:	Orphan
 F:	Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
index 7b30db3253af..13df2944f2ec 100644
--- a/drivers/i3c/master/Kconfig
+++ b/drivers/i3c/master/Kconfig
@@ -64,3 +64,13 @@ config MIPI_I3C_HCI_PCI
 
 	  This driver can also be built as a module. If so, the module will be
 	  called mipi-i3c-hci-pci.
+
+config RENESAS_I3C
+	tristate "Renesas I3C controller driver"
+	depends on HAS_IOMEM
+	depends on ARCH_RENESAS || COMPILE_TEST
+	help
+	  Support the Renesas I3C controller as found in some RZ variants.
+
+	  This driver can also be built as a module. If so, the module will be
+	  called renesas-i3c.
diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
index 3e97960160bc..aac74f3e3851 100644
--- a/drivers/i3c/master/Makefile
+++ b/drivers/i3c/master/Makefile
@@ -4,3 +4,4 @@ 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
 obj-$(CONFIG_MIPI_I3C_HCI)		+= mipi-i3c-hci/
+obj-$(CONFIG_RENESAS_I3C)		+= renesas-i3c.o
diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
new file mode 100644
index 000000000000..ff276c955d07
--- /dev/null
+++ b/drivers/i3c/master/renesas-i3c.c
@@ -0,0 +1,1441 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Renesas I3C Controller driver
+ * Copyright (C) 2023-25 Renesas Electronics Corp.
+ *
+ * TODO: IBI support, HotJoin support, Target support
+ */
+
+#include <linux/bitops.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/i3c/master.h>
+#include <linux/interrupt.h>
+#include <linux/ioport.h>
+#include <linux/iopoll.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+#include <linux/reset.h>
+#include <linux/slab.h>
+
+#define PRTS			0x00
+#define PRTS_PRTMD		BIT(0)
+
+#define BCTL			0x14
+#define BCTL_HJACKCTL		BIT(8)
+#define BCTL_ABT		BIT(29)
+#define BCTL_BUSE		BIT(31)
+
+#define MSDVAD			0x18
+#define MSDVAD_MDYAD(x)		(((x) & 0x3f) << 16)
+#define MSDVAD_MDYADV		BIT(31)
+
+#define RSTCTL			0x20
+#define RSTCTL_RI3CRST		BIT(0)
+#define RSTCTL_INTLRST		BIT(16)
+
+#define INST			0x30
+
+#define IBINCTL			0x58
+#define IBINCTL_NRHJCTL		BIT(0)
+#define IBINCTL_NRMRCTL		BIT(1)
+#define IBINCTL_NRSIRCTL	BIT(3)
+
+#define SVCTL			0x64
+
+#define REFCKCTL		0x70
+#define REFCKCTL_IREFCKS(x)	(((x) & 0x7) << 0)
+
+#define STDBR			0x74
+#define STDBR_SBRLO(cond, x)	((((x) >> (cond)) & 0xff) << 0)
+#define STDBR_SBRHO(cond, x)	((((x) >> (cond)) & 0xff) << 8)
+#define STDBR_SBRLP(x)		(((x) & 0x3f) << 16)
+#define STDBR_SBRHP(x)		(((x) & 0x3f) << 24)
+#define STDBR_DSBRPO		BIT(31)
+
+#define EXTBR			0x78
+#define EXTBR_EBRLO(x)		(((x) & 0xff) << 0)
+#define EXTBR_EBRHO(x)		(((x) & 0xff) << 8)
+#define EXTBR_EBRLP(x)		(((x) & 0x3f) << 16)
+#define EXTBR_EBRHP(x)		(((x) & 0x3f) << 24)
+
+#define BFRECDT			0x7c
+#define BFRECDT_FRECYC(x)	(((x) & 0x1ff) << 0)
+
+#define BAVLCDT			0x80
+#define BAVLCDT_AVLCYC(x)	(((x) & 0x1ff) << 0)
+
+#define BIDLCDT			0x84
+#define BIDLCDT_IDLCYC(x)	(((x) & 0x3ffff) << 0)
+
+#define ACKCTL			0xa0
+#define ACKCTL_ACKT		BIT(1)
+#define ACKCTL_ACKTWP		BIT(2)
+
+#define SCSTRCTL		0xa4
+#define SCSTRCTL_ACKTWE		BIT(0)
+#define SCSTRCTL_RWE		BIT(1)
+
+#define SCSTLCTL		0xb0
+
+#define CNDCTL			0x140
+#define CNDCTL_STCND		BIT(0)
+#define CNDCTL_SRCND		BIT(1)
+#define CNDCTL_SPCND		BIT(2)
+
+#define NCMDQP			0x150 /* Normal Command Queue */
+#define NCMDQP_CMD_ATTR(x)	(((x) & 0x7) << 0)
+#define NCMDQP_IMMED_XFER	0x01
+#define NCMDQP_ADDR_ASSGN	0x02
+#define NCMDQP_TID(x)		(((x) & 0xf) << 3)
+#define NCMDQP_CMD(x)		(((x) & 0xff) << 7)
+#define NCMDQP_CP		BIT(15)
+#define NCMDQP_DEV_INDEX(x)	(((x) & 0x1f) << 16)
+#define NCMDQP_BYTE_CNT(x)	(((x) & 0x7) << 23)
+#define NCMDQP_DEV_COUNT(x)	(((x) & 0xf) << 26)
+#define NCMDQP_MODE(x)		(((x) & 0x7) << 26)
+#define NCMDQP_RNW(x)		(((x) & 0x1) << 29)
+#define NCMDQP_ROC		BIT(30)
+#define NCMDQP_TOC		BIT(31)
+#define NCMDQP_DATA_LENGTH(x)	(((x) & 0xffff) << 16)
+
+#define NRSPQP			0x154 /* Normal Respone Queue */
+#define NRSPQP_NO_ERROR			0
+#define NRSPQP_ERROR_CRC		1
+#define NRSPQP_ERROR_PARITY		2
+#define NRSPQP_ERROR_FRAME		3
+#define NRSPQP_ERROR_IBA_NACK		4
+#define NRSPQP_ERROR_ADDRESS_NACK	5
+#define NRSPQP_ERROR_OVER_UNDER_FLOW	6
+#define NRSPQP_ERROR_TRANSF_ABORT	8
+#define NRSPQP_ERROR_I2C_W_NACK_ERR	9
+#define NRSPQP_ERR_STATUS(x)	(((x) & GENMASK(31, 28)) >> 28)
+#define NRSPQP_DATA_LEN(x)	((x) & GENMASK(15, 0))
+
+#define NTDTBP0			0x158 /* Normal Transfer Data Buffer */
+#define NTDTBP0_DEPTH		16
+
+#define NQTHCTL			0x190
+#define NQTHCTL_CMDQTH(x)	(((x) & 0x3) << 0)
+#define NQTHCTL_IBIDSSZ(x)	(((x) & 0xff) << 16)
+
+#define NTBTHCTL0		0x194
+
+#define NRQTHCTL		0x1c0
+
+#define BST			0x1d0
+#define BST_STCNDDF		BIT(0)
+#define BST_SPCNDDF		BIT(1)
+#define BST_NACKDF		BIT(4)
+#define BST_TENDF		BIT(8)
+
+#define BSTE			0x1d4
+#define BSTE_STCNDDE		BIT(0)
+#define BSTE_SPCNDDE		BIT(1)
+#define BSTE_NACKDE		BIT(4)
+#define BSTE_TENDE		BIT(8)
+#define BSTE_ALE		BIT(16)
+#define BSTE_TODE		BIT(20)
+#define BSTE_WUCNDDE		BIT(24)
+#define BSTE_ALL_FLAG		(BSTE_STCNDDE | BSTE_SPCNDDE |\
+				BSTE_NACKDE | BSTE_TENDE |\
+				BSTE_ALE | BSTE_TODE | BSTE_WUCNDDE)
+
+#define BIE			0x1d8
+#define BIE_STCNDDIE		BIT(0)
+#define BIE_SPCNDDIE		BIT(1)
+#define BIE_NACKDIE		BIT(4)
+#define BIE_TENDIE		BIT(8)
+
+#define NTST			0x1e0
+#define NTST_TDBEF0		BIT(0)
+#define NTST_RDBFF0		BIT(1)
+#define NTST_CMDQEF		BIT(3)
+#define NTST_RSPQFF		BIT(4)
+#define NTST_TABTF		BIT(5)
+#define NTST_TEF		BIT(9)
+
+#define NTSTE			0x1e4
+#define NTSTE_TDBEE0		BIT(0)
+#define NTSTE_RDBFE0		BIT(1)
+#define NTSTE_IBIQEFE		BIT(2)
+#define NTSTE_CMDQEE		BIT(3)
+#define NTSTE_RSPQFE		BIT(4)
+#define NTSTE_TABTE		BIT(5)
+#define NTSTE_TEE		BIT(9)
+#define NTSTE_RSQFE		BIT(20)
+#define NTSTE_ALL_FLAG		(NTSTE_TDBEE0 | NTSTE_RDBFE0 |\
+				NTSTE_IBIQEFE | NTSTE_CMDQEE |\
+				NTSTE_RSPQFE | NTSTE_TABTE |\
+				NTSTE_TEE | NTSTE_RSQFE)
+
+#define NTIE			0x1e8
+#define NTIE_TDBEIE0		BIT(0)
+#define NTIE_RDBFIE0		BIT(1)
+#define NTIE_IBIQEFIE		BIT(2)
+#define NTIE_RSPQFIE		BIT(4)
+#define NTIE_RSQFIE		BIT(20)
+
+#define BCST			0x210
+#define BCST_BFREF		BIT(0)
+
+#define DATBAS(x)		(0x224 + 0x8 * (x))
+#define DATBAS_DVSTAD(x)	(((x) & 0x7f) << 0)
+#define DATBAS_DVDYAD(x)	(((x) & 0xff) << 16)
+
+#define NDBSTLV0		0x398
+#define NDBSTLV0_RDBLV(x)	(((x) >> 8) & 0xff)
+
+#define RENESAS_I3C_MAX_DEVS	8
+#define I2C_INIT_MSG		-1
+
+/* Bus condition timing */
+#define I3C_BUS_THIGH_MIXED_NS	40		/* 40ns */
+#define I3C_BUS_FREE_TIME_NS	1300		/* 1.3us for Mixed Bus with I2C FM Device*/
+#define I3C_BUS_AVAL_TIME_NS	1000		/* 1us */
+#define I3C_BUS_IDLE_TIME_NS	200000		/* 200us */
+
+#define XFER_TIMEOUT		(msecs_to_jiffies(1000))
+
+enum i3c_internal_state {
+	I3C_INTERNAL_STATE_DISABLED,
+	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
+	I3C_INTERNAL_STATE_CONTROLLER_ENTDAA,
+	I3C_INTERNAL_STATE_CONTROLLER_SETDASA,
+	I3C_INTERNAL_STATE_CONTROLLER_WRITE,
+	I3C_INTERNAL_STATE_CONTROLLER_READ,
+	I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE,
+	I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ,
+};
+
+enum i3c_event {
+	I3C_COMMAND_ADDRESS_ASSIGNMENT,
+	I3C_WRITE,
+	I3C_READ,
+	I3C_COMMAND_WRITE,
+	I3C_COMMAND_READ,
+};
+
+struct renesas_i3c_cmd {
+	u32 cmd0;
+	u32 len;
+	const void *tx_buf;
+	u32 tx_count;
+	void *rx_buf;
+	u32 rx_count;
+	u32 err;
+	u8 rnw;
+	/* i2c xfer */
+	int i2c_bytes_left;
+	int i2c_is_last;
+	u8 *i2c_buf;
+	const struct i2c_msg *msg;
+};
+
+struct renesas_i3c_xfer {
+	struct list_head node;
+	struct completion comp;
+	int ret;
+	bool is_i2c_xfer;
+	unsigned int ncmds;
+	struct renesas_i3c_cmd cmds[] __counted_by(ncmds);
+};
+
+struct renesas_i3c_xferqueue {
+	struct list_head list;
+	struct renesas_i3c_xfer *cur;
+	/* Lock for accessing the xfer queue */
+	spinlock_t lock;
+};
+
+struct renesas_i3c {
+	struct i3c_master_controller base;
+	enum i3c_internal_state internal_state;
+	u16 maxdevs;
+	u32 free_pos;
+	u32 i2c_STDBR;
+	u32 i3c_STDBR;
+	u8 addrs[RENESAS_I3C_MAX_DEVS];
+	struct renesas_i3c_xferqueue xferqueue;
+	void __iomem *regs;
+	struct clk *tclk;
+};
+
+struct renesas_i3c_i2c_dev_data {
+	u8 index;
+};
+
+struct renesas_i3c_irq_desc {
+	char *name;
+	irq_handler_t isr;
+	char *desc;
+};
+
+struct renesas_i3c_config {
+	unsigned int has_pclkrw:1;
+};
+
+static inline void i3c_reg_update(u32 mask, u32 val, void __iomem *reg)
+{
+	u32 data = readl(reg);
+
+	data &= ~mask;
+	data |= (val & mask);
+	writel(data, reg);
+}
+
+static inline u32 i3c_reg_read(void __iomem *base, u32 reg)
+{
+	return readl(base + reg);
+}
+
+static inline void i3c_reg_write(void __iomem *base, u32 reg, u32 val)
+{
+	writel(val, base + reg);
+}
+
+static void i3c_reg_set_bit(void __iomem *base, u32 reg, u32 val)
+{
+	i3c_reg_update(val, val, base + reg);
+}
+
+static void i3c_reg_clear_bit(void __iomem *base, u32 reg, u32 val)
+{
+	i3c_reg_update(val, 0, base + reg);
+}
+
+static void i3c_reg_update_bit(void __iomem *base, u32 reg,
+			       u32 mask, u32 val)
+{
+	i3c_reg_update(mask, val, base + reg);
+}
+
+static inline struct renesas_i3c *
+to_renesas_i3c(struct i3c_master_controller *master)
+{
+	return container_of(master, struct renesas_i3c, base);
+}
+
+static inline u32 datbas_dvdyad_with_parity(u8 addr)
+{
+	return DATBAS_DVDYAD(addr | (parity8(addr) ? 0 : BIT(7)));
+}
+
+static int renesas_i3c_get_free_pos(struct renesas_i3c *i3c)
+{
+	if (!(i3c->free_pos & GENMASK(i3c->maxdevs - 1, 0)))
+		return -ENOSPC;
+
+	return ffs(i3c->free_pos) - 1;
+}
+
+static int renesas_i3c_get_addr_pos(struct renesas_i3c *i3c, u8 addr)
+{
+	int pos;
+
+	for (pos = 0; pos < i3c->maxdevs; pos++) {
+		if (addr == i3c->addrs[pos])
+			return pos;
+	}
+
+	return -EINVAL;
+}
+
+static struct renesas_i3c_xfer *
+renesas_i3c_alloc_xfer(struct renesas_i3c *i3c, unsigned int ncmds)
+{
+	struct renesas_i3c_xfer *xfer;
+
+	xfer = kzalloc(struct_size(xfer, cmds, ncmds), GFP_KERNEL);
+	if (!xfer)
+		return NULL;
+
+	INIT_LIST_HEAD(&xfer->node);
+	xfer->ncmds = ncmds;
+	xfer->ret = -ETIMEDOUT;
+
+	return xfer;
+}
+
+static void renesas_i3c_read_from_rx_fifo(struct renesas_i3c *i3c,
+						 u8 *data, int nbytes)
+{
+	readsl(i3c->regs + NTDTBP0, data, nbytes / 4);
+	if (nbytes & 3) {
+		u32 tmp;
+
+		readsl(i3c->regs + NTDTBP0, &tmp, 1);
+		memcpy(data + (nbytes & ~3), &tmp, nbytes & 3);
+	}
+}
+
+static void renesas_i3c_write_to_tx_fifo(struct renesas_i3c *i3c,
+						const u32 *data, int nbytes)
+{
+	writesl(i3c->regs + NTDTBP0, data, nbytes / 4);
+	if (nbytes & 3) {
+		u32 tmp = 0;
+
+		memcpy(&tmp, data + (nbytes & ~3), nbytes & 3);
+		writesl(i3c->regs + NTDTBP0, &tmp, 1);
+	}
+}
+
+static void renesas_i3c_start_xfer_locked(struct renesas_i3c *i3c)
+{
+	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
+	struct renesas_i3c_cmd *cmd;
+	u32 cmd1;
+
+	if (!xfer)
+		return;
+
+	cmd = xfer->cmds;
+
+	switch (i3c->internal_state) {
+	case I3C_INTERNAL_STATE_CONTROLLER_ENTDAA:
+	case I3C_INTERNAL_STATE_CONTROLLER_SETDASA:
+		i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RSPQFIE);
+		i3c_reg_write(i3c->regs, NCMDQP, cmd->cmd0);
+		i3c_reg_write(i3c->regs, NCMDQP, 0);
+		break;
+	case I3C_INTERNAL_STATE_CONTROLLER_WRITE:
+	case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE:
+		i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RSPQFIE);
+		if (cmd->len <= 4) {
+			cmd->cmd0 |= NCMDQP_CMD_ATTR(NCMDQP_IMMED_XFER);
+			cmd->cmd0 |= NCMDQP_BYTE_CNT(cmd->len);
+			cmd->tx_count = cmd->len;
+			cmd1 = cmd->len == 0 ? 0 : *(u32 *)cmd->tx_buf;
+		} else {
+			cmd1 = NCMDQP_DATA_LENGTH(cmd->len);
+		}
+		i3c_reg_write(i3c->regs, NCMDQP, cmd->cmd0);
+		i3c_reg_write(i3c->regs, NCMDQP, cmd1);
+		break;
+	case I3C_INTERNAL_STATE_CONTROLLER_READ:
+	case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ:
+		i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
+		cmd1 = NCMDQP_DATA_LENGTH(cmd->len);
+		i3c_reg_write(i3c->regs, NCMDQP, cmd->cmd0);
+		i3c_reg_write(i3c->regs, NCMDQP, cmd1);
+		break;
+	default:
+		break;
+	}
+
+	/* Clear the command queue empty flag */
+	i3c_reg_clear_bit(i3c->regs, NTST, NTST_CMDQEF);
+
+	if (cmd->len > 4 && i3c->internal_state == I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE) {
+		renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
+		/*
+		 * If there is still data remaining in the transfer
+		 * then it will be written in the Write Buffer Empty IRQ.
+		 */
+		if (cmd->len > NTDTBP0_DEPTH * sizeof(u32))
+			/* Enable the Write Buffer Empty IRQ. */
+			i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
+	}
+}
+
+static void renesas_i3c_dequeue_xfer_locked(struct renesas_i3c *i3c,
+						   struct renesas_i3c_xfer *xfer)
+{
+	if (i3c->xferqueue.cur == xfer)
+		i3c->xferqueue.cur = NULL;
+	else
+		list_del_init(&xfer->node);
+}
+
+static void renesas_i3c_dequeue_xfer(struct renesas_i3c *i3c,
+					    struct renesas_i3c_xfer *xfer)
+{
+	scoped_guard(spinlock_irqsave, &i3c->xferqueue.lock)
+		renesas_i3c_dequeue_xfer_locked(i3c, xfer);
+}
+
+static void renesas_i3c_enqueue_xfer(struct renesas_i3c *i3c,
+					    struct renesas_i3c_xfer *xfer)
+{
+	reinit_completion(&xfer->comp);
+	scoped_guard(spinlock_irqsave, &i3c->xferqueue.lock) {
+		if (i3c->xferqueue.cur) {
+			list_add_tail(&xfer->node, &i3c->xferqueue.list);
+		} else {
+			i3c->xferqueue.cur = xfer;
+			if (!xfer->is_i2c_xfer)
+				renesas_i3c_start_xfer_locked(i3c);
+		}
+	}
+}
+
+static void renesas_i3c_set_prts(struct renesas_i3c *i3c, u32 val)
+{
+	/* Required sequence according to tnrza0140ae */
+	i3c_reg_set_bit(i3c->regs, RSTCTL, RSTCTL_INTLRST);
+	i3c_reg_write(i3c->regs, PRTS, val);
+	i3c_reg_clear_bit(i3c->regs, RSTCTL, RSTCTL_INTLRST);
+}
+
+static void renesas_i3c_bus_enable(struct i3c_master_controller *m, bool i3c_mode)
+{
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+
+	/* Setup either I3C or I2C protocol */
+	if (i3c_mode) {
+		renesas_i3c_set_prts(i3c, 0);
+		/* Revisit: INCBA handling, especially after I2C transfers */
+		i3c_reg_set_bit(i3c->regs, BCTL, BCTL_HJACKCTL);
+		i3c_reg_set_bit(i3c->regs, MSDVAD, MSDVAD_MDYADV);
+		i3c_reg_write(i3c->regs, STDBR, i3c->i3c_STDBR);
+	} else {
+		renesas_i3c_set_prts(i3c, PRTS_PRTMD);
+		i3c_reg_write(i3c->regs, STDBR, i3c->i2c_STDBR);
+	}
+
+	/* Enable I3C bus */
+	i3c_reg_set_bit(i3c->regs, BCTL, BCTL_BUSE);
+}
+
+static int renesas_i3c_reset(struct renesas_i3c *i3c)
+{
+	u32 val;
+
+	i3c_reg_write(i3c->regs, BCTL, 0);
+	i3c_reg_set_bit(i3c->regs, RSTCTL, RSTCTL_RI3CRST);
+
+	/* Wait for reset completion  */
+	return readl_relaxed_poll_timeout(i3c->regs + RSTCTL, val,
+					  !(val & RSTCTL_RI3CRST), 0, 1000);
+}
+
+static int renesas_i3c_bus_init(struct i3c_master_controller *m)
+{
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	struct i3c_bus *bus = i3c_master_get_bus(m);
+	struct i3c_device_info info = {};
+	struct i2c_timings t;
+	unsigned long rate;
+	u32 val;
+	int cks, pp_high_ticks, pp_low_ticks, i3c_total_ticks;
+	int od_high_ticks, od_low_ticks, i2c_total_ticks;
+	int ret;
+	bool double_SBR;
+
+	rate = clk_get_rate(i3c->tclk);
+	if (!rate)
+		return -EINVAL;
+
+	ret = renesas_i3c_reset(i3c);
+	if (ret)
+		return ret;
+
+	i2c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i2c);
+	i3c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i3c);
+
+	i2c_parse_fw_timings(&m->dev, &t, true);
+
+	for (cks = 0; cks < 7; cks++) {
+		/* SCL low-period calculation in Open-drain mode */
+		od_low_ticks = ((i2c_total_ticks * 6) / 10);
+
+		/* SCL clock calculation in Push-Pull mode */
+		if (bus->mode == I3C_BUS_MODE_PURE)
+			pp_high_ticks = ((i3c_total_ticks * 5) / 10);
+		else
+			pp_high_ticks = DIV_ROUND_UP(I3C_BUS_THIGH_MIXED_NS,
+						     1000000000 / rate);
+		pp_low_ticks = i3c_total_ticks - pp_high_ticks;
+
+		if ((od_low_ticks / 2) <= 0xFF && pp_low_ticks < 0x3F)
+			break;
+
+		i2c_total_ticks /= 2;
+		i3c_total_ticks /= 2;
+		rate /= 2;
+	}
+
+	/* SCL clock period calculation in Open-drain mode */
+	if ((od_low_ticks / 2) > 0xFF || pp_low_ticks > 0x3F) {
+		dev_err(&m->dev, "invalid speed (i2c-scl = %lu Hz, i3c-scl = %lu Hz). Too slow.\n",
+			(unsigned long)bus->scl_rate.i2c, (unsigned long)bus->scl_rate.i3c);
+		ret = -EINVAL;
+		return ret;
+	}
+
+	/* SCL high-period calculation in Open-drain mode */
+	od_high_ticks = i2c_total_ticks - od_low_ticks;
+
+	/* Standard Bit Rate setting */
+	double_SBR = od_low_ticks > 0xFF ? true : false;
+	i3c->i3c_STDBR = (double_SBR ? STDBR_DSBRPO : 0) |
+			STDBR_SBRLO(double_SBR, od_low_ticks) |
+			STDBR_SBRHO(double_SBR, od_high_ticks) |
+			STDBR_SBRLP(pp_low_ticks) |
+			STDBR_SBRHP(pp_high_ticks);
+
+	od_low_ticks -= t.scl_fall_ns / (1000000000 / rate) + 1;
+	od_high_ticks -= t.scl_rise_ns / (1000000000 / rate) + 1;
+	i3c->i2c_STDBR = (double_SBR ? STDBR_DSBRPO : 0) |
+			STDBR_SBRLO(double_SBR, od_low_ticks) |
+			STDBR_SBRHO(double_SBR, od_high_ticks) |
+			STDBR_SBRLP(pp_low_ticks) |
+			STDBR_SBRHP(pp_high_ticks);
+	i3c_reg_write(i3c->regs, STDBR, i3c->i3c_STDBR);
+
+	/* Extended Bit Rate setting */
+	i3c_reg_write(i3c->regs, EXTBR, EXTBR_EBRLO(od_low_ticks) |
+					   EXTBR_EBRHO(od_high_ticks) |
+					   EXTBR_EBRLP(pp_low_ticks) |
+					   EXTBR_EBRHP(pp_high_ticks));
+
+	i3c_reg_write(i3c->regs, REFCKCTL, REFCKCTL_IREFCKS(cks));
+
+	/* Disable Slave Mode */
+	i3c_reg_write(i3c->regs, SVCTL, 0);
+
+	/* Initialize Queue/Buffer threshold */
+	i3c_reg_write(i3c->regs, NQTHCTL, NQTHCTL_IBIDSSZ(6) |
+					     NQTHCTL_CMDQTH(1));
+
+	/* The only supported configuration is two entries*/
+	i3c_reg_write(i3c->regs, NTBTHCTL0, 0);
+	/* Interrupt when there is one entry in the queue */
+	i3c_reg_write(i3c->regs, NRQTHCTL, 0);
+
+	/* Enable all Bus/Transfer Status Flags */
+	i3c_reg_write(i3c->regs, BSTE, BSTE_ALL_FLAG);
+	i3c_reg_write(i3c->regs, NTSTE, NTSTE_ALL_FLAG);
+
+	/* Interrupt enable settings */
+	i3c_reg_write(i3c->regs, BIE, BIE_NACKDIE | BIE_TENDIE);
+	i3c_reg_write(i3c->regs, NTIE, NTIE_RSQFIE |
+					  NTIE_IBIQEFIE | NTIE_RDBFIE0);
+
+	/* Clear Status register */
+	i3c_reg_write(i3c->regs, NTST, 0);
+	i3c_reg_write(i3c->regs, INST, 0);
+	i3c_reg_write(i3c->regs, BST, 0);
+
+	/* Hot-Join Acknowlege setting. */
+	i3c_reg_update_bit(i3c->regs, BCTL, BCTL_HJACKCTL, BCTL_HJACKCTL);
+
+	i3c_reg_write(i3c->regs, IBINCTL, IBINCTL_NRHJCTL | IBINCTL_NRMRCTL |
+					     IBINCTL_NRSIRCTL);
+
+	i3c_reg_write(i3c->regs, SCSTLCTL, 0);
+	i3c_reg_set_bit(i3c->regs, SCSTRCTL, SCSTRCTL_ACKTWE);
+
+	/* Bus condition timing */
+	val = DIV_ROUND_UP(I3C_BUS_FREE_TIME_NS, 1000000000 / rate);
+	i3c_reg_write(i3c->regs, BFRECDT, BFRECDT_FRECYC(val));
+
+	val = DIV_ROUND_UP(I3C_BUS_AVAL_TIME_NS, 1000000000 / rate);
+	i3c_reg_write(i3c->regs, BAVLCDT, BAVLCDT_AVLCYC(val));
+
+	val = DIV_ROUND_UP(I3C_BUS_IDLE_TIME_NS, 1000000000 / rate);
+	i3c_reg_write(i3c->regs, BIDLCDT, BIDLCDT_IDLCYC(val));
+
+	ret = i3c_master_get_free_addr(m, 0);
+	if (ret < 0)
+		return ret;
+
+	i3c_reg_write(i3c->regs, MSDVAD, MSDVAD_MDYAD(ret) | MSDVAD_MDYADV);
+
+	memset(&info, 0, sizeof(info));
+	info.dyn_addr = ret;
+	return i3c_master_set_info(&i3c->base, &info);
+}
+
+static void renesas_i3c_bus_cleanup(struct i3c_master_controller *m)
+{
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	u32 val;
+
+	i3c_reg_write(i3c->regs, BCTL, 0);
+	i3c_reg_update_bit(i3c->regs, RSTCTL, RSTCTL_RI3CRST, RSTCTL_RI3CRST);
+	/* Wait for reset completion  */
+	readl_relaxed_poll_timeout(i3c->regs + RSTCTL, val,
+				   !(val & RSTCTL_RI3CRST), 0, 1000);
+}
+
+static int renesas_i3c_daa(struct i3c_master_controller *m)
+{
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	struct renesas_i3c_xfer *xfer;
+	struct renesas_i3c_cmd *cmd;
+	u32 olddevs, newdevs;
+	u8 last_addr = 0, pos;
+	int ret;
+
+	/* Enable I3C bus. */
+	renesas_i3c_bus_enable(m, true);
+
+	olddevs = ~(i3c->free_pos);
+	i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_ENTDAA;
+
+	/* Setting DATBASn registers for target devices. */
+	for (pos = 0; pos < i3c->maxdevs; pos++) {
+		if (olddevs & BIT(pos))
+			continue;
+
+		ret = i3c_master_get_free_addr(m, last_addr + 1);
+		if (ret < 0)
+			return -ENOSPC;
+
+		i3c->addrs[pos] = ret;
+		last_addr = ret;
+
+		i3c_reg_write(i3c->regs, DATBAS(pos), datbas_dvdyad_with_parity(ret));
+	}
+
+	xfer = renesas_i3c_alloc_xfer(i3c, 1);
+	if (!xfer)
+		return -ENOMEM;
+
+	init_completion(&xfer->comp);
+	cmd = xfer->cmds;
+	cmd->rx_count = 0;
+
+	ret = renesas_i3c_get_free_pos(i3c);
+	if (ret < 0)
+		goto out;
+
+	/*
+	 * Setup the command descriptor to start the ENTDAA command
+	 * and starting at the selected device index.
+	 */
+	cmd->cmd0 = NCMDQP_CMD_ATTR(NCMDQP_ADDR_ASSGN) | NCMDQP_ROC |
+		    NCMDQP_TID(I3C_COMMAND_ADDRESS_ASSIGNMENT) |
+		    NCMDQP_CMD(I3C_CCC_ENTDAA) | NCMDQP_DEV_INDEX(ret) |
+		    NCMDQP_DEV_COUNT(i3c->maxdevs - ret) | NCMDQP_TOC;
+
+	renesas_i3c_enqueue_xfer(i3c, xfer);
+	if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000)))
+		renesas_i3c_dequeue_xfer(i3c, xfer);
+
+	newdevs = GENMASK(i3c->maxdevs - cmd->rx_count - 1, 0);
+	newdevs &= ~olddevs;
+
+	for (pos = 0; pos < i3c->maxdevs; pos++) {
+		if (newdevs & BIT(pos))
+			i3c_master_add_i3c_dev_locked(m, i3c->addrs[pos]);
+	}
+
+out:
+	kfree(xfer);
+	return ret < 0 ? ret : 0;
+}
+
+static bool renesas_i3c_supports_ccc_cmd(struct i3c_master_controller *m,
+						const struct i3c_ccc_cmd *cmd)
+{
+	if (cmd->ndests > 1)
+		return false;
+
+	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_ENTHDR(0):
+	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_GETMXDS:
+	case I3C_CCC_GETHDRCAP:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static int renesas_i3c_send_ccc_cmd(struct i3c_master_controller *m,
+					   struct i3c_ccc_cmd *ccc)
+{
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	struct renesas_i3c_xfer *xfer;
+	struct renesas_i3c_cmd *cmd;
+	int ret, pos = 0;
+
+	if (ccc->id & I3C_CCC_DIRECT) {
+		pos = renesas_i3c_get_addr_pos(i3c, ccc->dests[0].addr);
+		if (pos < 0)
+			return pos;
+	}
+
+	xfer = renesas_i3c_alloc_xfer(i3c, 1);
+	if (!xfer)
+		return -ENOMEM;
+
+	renesas_i3c_bus_enable(m, true);
+
+	init_completion(&xfer->comp);
+	cmd = xfer->cmds;
+	cmd->rnw = ccc->rnw;
+	cmd->cmd0 = 0;
+
+	/* Calculate the command descriptor. */
+	switch (ccc->id) {
+	case I3C_CCC_SETDASA:
+		i3c_reg_write(i3c->regs, DATBAS(pos),
+			DATBAS_DVSTAD(ccc->dests[0].addr) |
+			DATBAS_DVDYAD(*(u8 *)ccc->dests[0].payload.data >> 1));
+		cmd->cmd0 = NCMDQP_CMD_ATTR(NCMDQP_ADDR_ASSGN) | NCMDQP_ROC |
+			NCMDQP_TID(I3C_COMMAND_ADDRESS_ASSIGNMENT) |
+			NCMDQP_CMD(I3C_CCC_SETDASA) | NCMDQP_DEV_INDEX(pos) |
+			NCMDQP_DEV_COUNT(0) | NCMDQP_TOC;
+		i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_SETDASA;
+		break;
+	default:
+		/* Calculate the command descriptor. */
+		cmd->cmd0 = NCMDQP_TID(I3C_COMMAND_WRITE) | NCMDQP_MODE(0) |
+				NCMDQP_RNW(ccc->rnw) | NCMDQP_CMD(ccc->id) |
+				NCMDQP_ROC | NCMDQP_TOC | NCMDQP_CP |
+				NCMDQP_DEV_INDEX(pos);
+
+		if (ccc->rnw) {
+			cmd->rx_buf = ccc->dests[0].payload.data;
+			cmd->len = ccc->dests[0].payload.len;
+			cmd->rx_count = 0;
+			i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ;
+		} else {
+			cmd->tx_buf = ccc->dests[0].payload.data;
+			cmd->len = ccc->dests[0].payload.len;
+			cmd->tx_count = 0;
+			i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE;
+		}
+	}
+
+	renesas_i3c_enqueue_xfer(i3c, xfer);
+	if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000)))
+		renesas_i3c_dequeue_xfer(i3c, xfer);
+
+	ret = xfer->ret;
+	if (ret)
+		ccc->err = I3C_ERROR_M2;
+
+	kfree(xfer);
+
+	return ret;
+}
+
+static int renesas_i3c_priv_xfers(struct i3c_dev_desc *dev, struct i3c_priv_xfer *i3c_xfers,
+					 int i3c_nxfers)
+{
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	struct renesas_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+	struct renesas_i3c_xfer *xfer;
+	int i;
+
+	/* Enable I3C bus. */
+	renesas_i3c_bus_enable(m, true);
+
+	xfer = renesas_i3c_alloc_xfer(i3c, 1);
+	if (!xfer)
+		return -ENOMEM;
+
+	init_completion(&xfer->comp);
+
+	for (i = 0; i < i3c_nxfers; i++) {
+		struct renesas_i3c_cmd *cmd = xfer->cmds;
+
+		/* Calculate the Transfer Command Descriptor */
+		cmd->rnw = i3c_xfers[i].rnw;
+		cmd->cmd0 = NCMDQP_DEV_INDEX(data->index) | NCMDQP_MODE(0) |
+			    NCMDQP_RNW(cmd->rnw) | NCMDQP_ROC | NCMDQP_TOC;
+
+		if (i3c_xfers[i].rnw) {
+			cmd->rx_count = 0;
+			cmd->cmd0 |= NCMDQP_TID(I3C_READ);
+			cmd->rx_buf = i3c_xfers[i].data.in;
+			cmd->len = i3c_xfers[i].len;
+			i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_READ;
+		} else {
+			cmd->tx_count = 0;
+			cmd->cmd0 |= NCMDQP_TID(I3C_WRITE);
+			cmd->tx_buf = i3c_xfers[i].data.out;
+			cmd->len = i3c_xfers[i].len;
+			i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_WRITE;
+		}
+
+		if (!i3c_xfers[i].rnw && i3c_xfers[i].len > 4) {
+			renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
+			if (cmd->len > NTDTBP0_DEPTH * sizeof(u32))
+				i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
+		}
+
+		renesas_i3c_enqueue_xfer(i3c, xfer);
+		if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
+			renesas_i3c_dequeue_xfer(i3c, xfer);
+	}
+
+	return 0;
+}
+
+static int renesas_i3c_attach_i3c_dev(struct i3c_dev_desc *dev)
+{
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	struct renesas_i3c_i2c_dev_data *data;
+	int pos;
+
+	pos = renesas_i3c_get_free_pos(i3c);
+	if (pos < 0)
+		return pos;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->index = pos;
+	i3c->addrs[pos] = dev->info.dyn_addr ? : dev->info.static_addr;
+	i3c->free_pos &= ~BIT(pos);
+
+	i3c_reg_write(i3c->regs, DATBAS(pos), DATBAS_DVSTAD(dev->info.static_addr) |
+				    datbas_dvdyad_with_parity(i3c->addrs[pos]));
+	i3c_dev_set_master_data(dev, data);
+
+	return 0;
+}
+
+static int renesas_i3c_reattach_i3c_dev(struct i3c_dev_desc *dev,
+					       u8 old_dyn_addr)
+{
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	struct renesas_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+
+	i3c->addrs[data->index] = dev->info.dyn_addr ? dev->info.dyn_addr :
+							dev->info.static_addr;
+
+	return 0;
+}
+
+static void renesas_i3c_detach_i3c_dev(struct i3c_dev_desc *dev)
+{
+	struct renesas_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
+	struct i3c_master_controller *m = i3c_dev_get_master(dev);
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+
+	i3c_dev_set_master_data(dev, NULL);
+	i3c->addrs[data->index] = 0;
+	i3c->free_pos |= BIT(data->index);
+	kfree(data);
+}
+
+static int renesas_i3c_i2c_xfers(struct i2c_dev_desc *dev,
+					struct i2c_msg *i2c_xfers,
+					int i2c_nxfers)
+{
+	struct i3c_master_controller *m = i2c_dev_get_master(dev);
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	struct renesas_i3c_xfer *xfer;
+	struct renesas_i3c_cmd *cmd;
+	u8 start_bit = CNDCTL_STCND;
+	int ret, i;
+
+	if (!i2c_nxfers)
+		return 0;
+
+	renesas_i3c_bus_enable(m, false);
+
+	xfer = renesas_i3c_alloc_xfer(i3c, 1);
+	if (!xfer)
+		return -ENOMEM;
+
+	init_completion(&xfer->comp);
+	xfer->is_i2c_xfer = true;
+	cmd = xfer->cmds;
+
+	if (!(i3c_reg_read(i3c->regs, BCST) & BCST_BFREF)) {
+		cmd->err = -EBUSY;
+		goto out;
+	}
+
+	i3c_reg_write(i3c->regs, BST, 0);
+
+	renesas_i3c_enqueue_xfer(i3c, xfer);
+
+	for (i = 0; i < i2c_nxfers; i++) {
+		cmd->i2c_bytes_left = I2C_INIT_MSG;
+		cmd->i2c_buf = i2c_xfers[i].buf;
+		cmd->msg = &i2c_xfers[i];
+		cmd->i2c_is_last = (i == i2c_nxfers - 1);
+
+		i3c_reg_set_bit(i3c->regs, BIE, BIE_NACKDIE);
+		i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
+		i3c_reg_set_bit(i3c->regs, BIE, BIE_STCNDDIE);
+
+		/* Issue Start condition */
+		i3c_reg_set_bit(i3c->regs, CNDCTL, start_bit);
+
+		i3c_reg_set_bit(i3c->regs, NTSTE, NTSTE_TDBEE0);
+
+		wait_for_completion_timeout(&xfer->comp, m->i2c.timeout);
+
+		if (cmd->err)
+			break;
+
+		start_bit = CNDCTL_SRCND;
+	}
+out:
+	renesas_i3c_dequeue_xfer(i3c, xfer);
+	ret = cmd->err;
+	kfree(xfer);
+	return ret ?: 0;
+}
+
+static int renesas_i3c_attach_i2c_dev(struct i2c_dev_desc *dev)
+{
+	struct i3c_master_controller *m = i2c_dev_get_master(dev);
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+	struct renesas_i3c_i2c_dev_data *data;
+	int pos;
+
+	pos = renesas_i3c_get_free_pos(i3c);
+	if (pos < 0)
+		return pos;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->index = pos;
+	i3c->addrs[pos] = dev->addr;
+	i3c->free_pos &= ~BIT(pos);
+	i2c_dev_set_master_data(dev, data);
+
+	return 0;
+}
+
+static void renesas_i3c_detach_i2c_dev(struct i2c_dev_desc *dev)
+{
+	struct renesas_i3c_i2c_dev_data *data = i2c_dev_get_master_data(dev);
+	struct i3c_master_controller *m = i2c_dev_get_master(dev);
+	struct renesas_i3c *i3c = to_renesas_i3c(m);
+
+	i2c_dev_set_master_data(dev, NULL);
+	i3c->addrs[data->index] = 0;
+	i3c->free_pos |= BIT(data->index);
+	kfree(data);
+}
+
+static irqreturn_t i3c_tx_isr(int irq, void *data)
+{
+	struct renesas_i3c *i3c = data;
+	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
+	struct renesas_i3c_cmd *cmd = xfer->cmds;
+	u8 val;
+
+	if (xfer->is_i2c_xfer) {
+		if (!cmd->i2c_bytes_left)
+			return IRQ_NONE;
+
+		if (cmd->i2c_bytes_left != I2C_INIT_MSG) {
+			val = *cmd->i2c_buf;
+			cmd->i2c_buf++;
+			cmd->i2c_bytes_left--;
+			i3c_reg_write(i3c->regs, NTDTBP0, val);
+		}
+
+		if (cmd->i2c_bytes_left == 0) {
+			i3c_reg_clear_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
+			i3c_reg_set_bit(i3c->regs, BIE, BIE_TENDIE);
+		}
+
+		/* Clear the Transmit Buffer Empty status flag. */
+		i3c_reg_clear_bit(i3c->regs, NTST, NTST_TDBEF0);
+	} else {
+		renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
+	}
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t i3c_resp_isr(int irq, void *data)
+{
+	struct renesas_i3c *i3c = data;
+	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
+	struct renesas_i3c_cmd *cmd = xfer->cmds;
+	u32 resp_descriptor = i3c_reg_read(i3c->regs, NRSPQP);
+	u32 bytes_remaining = 0;
+	u32 ntst, data_len;
+	int ret = 0;
+
+	/* Clear the Respone Queue Full status flag*/
+	i3c_reg_clear_bit(i3c->regs, NTST, NTST_RSPQFF);
+
+	data_len = NRSPQP_DATA_LEN(resp_descriptor);
+
+	switch (i3c->internal_state) {
+	case I3C_INTERNAL_STATE_CONTROLLER_ENTDAA:
+		cmd->rx_count = data_len;
+		break;
+	case I3C_INTERNAL_STATE_CONTROLLER_WRITE:
+	case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE:
+		/* Disable the transmit IRQ if it hasn't been disabled already. */
+		i3c_reg_clear_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
+		break;
+	case I3C_INTERNAL_STATE_CONTROLLER_READ:
+	case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ:
+		if (NDBSTLV0_RDBLV(i3c_reg_read(i3c->regs, NDBSTLV0)) && !cmd->err)
+			bytes_remaining = data_len - cmd->rx_count;
+
+		renesas_i3c_read_from_rx_fifo(i3c, cmd->rx_buf, bytes_remaining);
+		i3c_reg_clear_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
+		break;
+	default:
+		break;
+	}
+
+	switch (NRSPQP_ERR_STATUS(resp_descriptor)) {
+	case NRSPQP_NO_ERROR:
+		break;
+	case NRSPQP_ERROR_PARITY:
+	case NRSPQP_ERROR_IBA_NACK:
+	case NRSPQP_ERROR_TRANSF_ABORT:
+	case NRSPQP_ERROR_CRC:
+	case NRSPQP_ERROR_FRAME:
+		ret = -EIO;
+		break;
+	case NRSPQP_ERROR_OVER_UNDER_FLOW:
+		ret = -ENOSPC;
+		break;
+	case NRSPQP_ERROR_I2C_W_NACK_ERR:
+	case NRSPQP_ERROR_ADDRESS_NACK:
+	default:
+		ret = -EINVAL;
+		break;
+	}
+
+	ntst = i3c_reg_read(i3c->regs, NTST);
+
+	/*
+	 * If the transfer was aborted, then the abort flag must be cleared
+	 * before notifying the application that a transfer has completed.
+	 */
+	if (ntst & NTST_TABTF)
+		i3c_reg_clear_bit(i3c->regs, BCTL, BCTL_ABT);
+
+	/* Clear error status flags. */
+	i3c_reg_clear_bit(i3c->regs, NTST, NTST_TEF | NTST_TABTF);
+
+	xfer->ret = ret;
+	complete(&xfer->comp);
+
+	xfer = list_first_entry_or_null(&i3c->xferqueue.list,
+					struct renesas_i3c_xfer, node);
+	if (xfer)
+		list_del_init(&xfer->node);
+
+	i3c->xferqueue.cur = xfer;
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t i3c_tend_isr(int irq, void *data)
+{
+	struct renesas_i3c *i3c = data;
+	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
+	struct renesas_i3c_cmd *cmd = xfer->cmds;
+
+	if (xfer->is_i2c_xfer) {
+		if (i3c_reg_read(i3c->regs, BST) & BST_NACKDF) {
+			/* We got a NACKIE */
+			i3c_reg_read(i3c->regs, NTDTBP0); /* dummy read */
+			i3c_reg_clear_bit(i3c->regs, BST, BST_NACKDF);
+			cmd->err = -ENXIO;
+		} else if (cmd->i2c_bytes_left) {
+			i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
+			return IRQ_NONE;
+		}
+
+		if (cmd->i2c_is_last || cmd->err) {
+			i3c_reg_clear_bit(i3c->regs, BIE, BIE_TENDIE);
+			i3c_reg_set_bit(i3c->regs, BIE, BIE_SPCNDDIE);
+			i3c_reg_set_bit(i3c->regs, CNDCTL, CNDCTL_SPCND);
+		} else {
+			/* Transfer is complete, but do not send STOP */
+			i3c_reg_clear_bit(i3c->regs, NTSTE, NTSTE_TDBEE0);
+			i3c_reg_clear_bit(i3c->regs, BIE, BIE_TENDIE);
+			xfer->ret = 0;
+			complete(&xfer->comp);
+		}
+	}
+
+	/* Clear the Transmit Buffer Empty status flag. */
+	i3c_reg_clear_bit(i3c->regs, BST, BST_TENDF);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t i3c_rx_isr(int irq, void *data)
+{
+	struct renesas_i3c *i3c = data;
+	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
+	struct renesas_i3c_cmd *cmd = xfer->cmds;
+	int read_bytes;
+
+	if (xfer->is_i2c_xfer) {
+		if (!cmd->i2c_bytes_left)
+			return IRQ_NONE;
+
+		if (cmd->i2c_bytes_left == I2C_INIT_MSG) {
+			cmd->i2c_bytes_left = cmd->msg->len;
+			i3c_reg_set_bit(i3c->regs, SCSTRCTL, SCSTRCTL_RWE);
+			i3c_reg_read(i3c->regs, NTDTBP0); /* dummy read */
+			if (cmd->i2c_bytes_left == 1)
+				i3c_reg_write(i3c->regs, ACKCTL, ACKCTL_ACKT | ACKCTL_ACKTWP);
+			return IRQ_HANDLED;
+		}
+
+		if (cmd->i2c_bytes_left == 1) {
+			/* STOP must come before we set ACKCTL! */
+			if (cmd->i2c_is_last) {
+				i3c_reg_set_bit(i3c->regs, BIE, BIE_SPCNDDIE);
+				i3c_reg_clear_bit(i3c->regs, BST, BST_SPCNDDF);
+				i3c_reg_set_bit(i3c->regs, CNDCTL, CNDCTL_SPCND);
+			}
+			i3c_reg_write(i3c->regs, ACKCTL, ACKCTL_ACKT | ACKCTL_ACKTWP);
+		} else {
+			i3c_reg_write(i3c->regs, ACKCTL, ACKCTL_ACKTWP);
+		}
+
+		/* Reading acks the RIE interrupt */
+		*cmd->i2c_buf = i3c_reg_read(i3c->regs, NTDTBP0);
+		cmd->i2c_buf++;
+		cmd->i2c_bytes_left--;
+	} else {
+		read_bytes = NDBSTLV0_RDBLV(i3c_reg_read(i3c->regs, NDBSTLV0)) * sizeof(u32);
+
+		if (i3c->internal_state == I3C_INTERNAL_STATE_CONTROLLER_ENTDAA &&
+							read_bytes == 8) {
+			i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RSPQFIE);
+			/* Read PID, BCR, DCR data */
+			i3c_reg_read(i3c->regs, NTDTBP0);
+			i3c_reg_read(i3c->regs, NTDTBP0);
+			cmd->rx_count++;
+		} else {
+			renesas_i3c_read_from_rx_fifo(i3c, cmd->rx_buf, read_bytes);
+			cmd->rx_count = read_bytes;
+		}
+	}
+
+	/* Clear the Read Buffer Full status flag. */
+	i3c_reg_clear_bit(i3c->regs, NTST, NTST_RDBFF0);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t i3c_stop_isr(int irq, void *data)
+{
+	struct renesas_i3c *i3c = data;
+	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
+
+	/* read back registers to confirm writes have fully propagated */
+	i3c_reg_write(i3c->regs, BST, 0);
+	i3c_reg_read(i3c->regs, BST);
+	i3c_reg_write(i3c->regs, BIE, 0);
+	i3c_reg_clear_bit(i3c->regs, NTST, NTST_TDBEF0 | NTST_RDBFF0);
+	i3c_reg_clear_bit(i3c->regs, SCSTRCTL, SCSTRCTL_RWE);
+
+	xfer->ret = 0;
+	complete(&xfer->comp);
+
+	return IRQ_HANDLED;
+}
+
+static irqreturn_t i3c_start_isr(int irq, void *data)
+{
+	struct renesas_i3c *i3c = data;
+	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
+	struct renesas_i3c_cmd *cmd = xfer->cmds;
+	u8 val;
+
+	if (xfer->is_i2c_xfer) {
+		if (!cmd->i2c_bytes_left)
+			return IRQ_NONE;
+
+		if (cmd->i2c_bytes_left == I2C_INIT_MSG) {
+			if (cmd->msg->flags & I2C_M_RD) {
+				/* On read, switch over to receive interrupt */
+				i3c_reg_clear_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
+				i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
+			} else {
+				/* On write, initialize length */
+				cmd->i2c_bytes_left = cmd->msg->len;
+			}
+
+			val = i2c_8bit_addr_from_msg(cmd->msg);
+			i3c_reg_write(i3c->regs, NTDTBP0, val);
+		}
+	}
+
+	i3c_reg_clear_bit(i3c->regs, BIE, BIE_STCNDDIE);
+	i3c_reg_clear_bit(i3c->regs, BST, BST_STCNDDF);
+	return IRQ_HANDLED;
+}
+
+static const struct i3c_master_controller_ops renesas_i3c_ops = {
+	.bus_init = renesas_i3c_bus_init,
+	.bus_cleanup = renesas_i3c_bus_cleanup,
+	.attach_i3c_dev = renesas_i3c_attach_i3c_dev,
+	.reattach_i3c_dev = renesas_i3c_reattach_i3c_dev,
+	.detach_i3c_dev = renesas_i3c_detach_i3c_dev,
+	.do_daa = renesas_i3c_daa,
+	.supports_ccc_cmd = renesas_i3c_supports_ccc_cmd,
+	.send_ccc_cmd = renesas_i3c_send_ccc_cmd,
+	.priv_xfers = renesas_i3c_priv_xfers,
+	.attach_i2c_dev = renesas_i3c_attach_i2c_dev,
+	.detach_i2c_dev = renesas_i3c_detach_i2c_dev,
+	.i2c_xfers = renesas_i3c_i2c_xfers,
+};
+
+static struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
+	{ .name = "resp", .isr = i3c_resp_isr, .desc = "i3c-resp" },
+	{ .name = "rx", .isr = i3c_rx_isr, .desc = "i3c-rx" },
+	{ .name = "tx", .isr = i3c_tx_isr, .desc = "i3c-tx" },
+	{ .name = "st", .isr = i3c_start_isr, .desc = "i3c-start" },
+	{ .name = "sp", .isr = i3c_stop_isr, .desc = "i3c-stop" },
+	{ .name = "tend", .isr = i3c_tend_isr, .desc = "i3c-tend" },
+	{ .name = "nack", .isr = i3c_tend_isr, .desc = "i3c-nack" },
+};
+
+static int renesas_i3c_probe(struct platform_device *pdev)
+{
+	struct renesas_i3c *i3c;
+	struct reset_control *reset;
+	struct clk *clk;
+	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
+	int ret, i;
+
+	if (!config)
+		return -ENODATA;
+
+	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
+	if (!i3c)
+		return -ENOMEM;
+
+	i3c->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(i3c->regs))
+		return PTR_ERR(i3c->regs);
+
+	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
+	if (IS_ERR(clk))
+		return PTR_ERR(clk);
+
+	if (config->has_pclkrw) {
+		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
+		if (IS_ERR(clk))
+			return PTR_ERR(clk);
+	}
+
+	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
+	if (IS_ERR(i3c->tclk))
+		return PTR_ERR(i3c->tclk);
+
+	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
+	if (IS_ERR(reset))
+		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
+				     "Error: missing tresetn ctrl\n");
+
+	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "presetn");
+	if (IS_ERR(reset))
+		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
+				     "Error: missing presetn ctrl\n");
+
+	spin_lock_init(&i3c->xferqueue.lock);
+	INIT_LIST_HEAD(&i3c->xferqueue.list);
+
+	ret = renesas_i3c_reset(i3c);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < ARRAY_SIZE(renesas_i3c_irqs); i++) {
+		ret = platform_get_irq_byname(pdev, renesas_i3c_irqs[i].name);
+		if (ret < 0)
+			return ret;
+
+		ret = devm_request_irq(&pdev->dev, ret, renesas_i3c_irqs[i].isr,
+				       0, renesas_i3c_irqs[i].desc, i3c);
+		if (ret) {
+			dev_err(&pdev->dev, "failed to request irq %s\n",
+				renesas_i3c_irqs[i].desc);
+			return ret;
+		}
+	}
+
+	platform_set_drvdata(pdev, i3c);
+
+	i3c->maxdevs = RENESAS_I3C_MAX_DEVS;
+	i3c->free_pos = GENMASK(i3c->maxdevs - 1, 0);
+
+	ret = i3c_master_register(&i3c->base, &pdev->dev,
+				  &renesas_i3c_ops, false);
+	if (ret)
+		return ret;
+
+	dev_info(&pdev->dev, "register with i2c-scl = %ldHz i3c-scl = %ldHz\n",
+		 i3c->base.bus.scl_rate.i2c, i3c->base.bus.scl_rate.i3c);
+
+	return 0;
+}
+
+static void renesas_i3c_remove(struct platform_device *pdev)
+{
+	struct renesas_i3c *i3c = platform_get_drvdata(pdev);
+
+	i3c_master_unregister(&i3c->base);
+}
+
+static const struct renesas_i3c_config empty_i3c_config = {
+};
+
+static const struct renesas_i3c_config r9a09g047_i3c_config = {
+	.has_pclkrw = 1,
+};
+
+static const struct of_device_id renesas_i3c_of_ids[] = {
+	{ .compatible = "renesas,r9a08g045-i3c", .data = &empty_i3c_config },
+	{ .compatible = "renesas,r9a09g047-i3c", .data = &r9a09g047_i3c_config },
+	{ .compatible = "renesas,i3c", .data = &empty_i3c_config },
+	{ /* sentinel */ },
+};
+MODULE_DEVICE_TABLE(of, renesas_i3c_of_ids);
+
+static struct platform_driver renesas_i3c = {
+	.probe = renesas_i3c_probe,
+	.remove = renesas_i3c_remove,
+	.driver = {
+		.name = "renesas-i3c",
+		.of_match_table = renesas_i3c_of_ids,
+	},
+};
+module_platform_driver(renesas_i3c);
+
+MODULE_AUTHOR("Wolfram Sang <wsa+renesas@sang-engineering.com>");
+MODULE_AUTHOR("Renesas BSP teams");
+MODULE_DESCRIPTION("Renesas I3C controller driver");
+MODULE_LICENSE("GPL");
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH RFC 5/7] arm64: dts: renesas: r9a08g045: Add I3C node
  2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
                   ` (3 preceding siblings ...)
  2025-06-11  9:39 ` [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP Wolfram Sang
@ 2025-06-11  9:39 ` Wolfram Sang
  2025-06-11  9:39 ` [PATCH RFC 6/7] arm64: dts: renesas: r9a09g047: " Wolfram Sang
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11  9:39 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Tommaso Merciai, Quynh Nguyen, Wolfram Sang, Geert Uytterhoeven,
	Magnus Damm, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	devicetree

From: Quynh Nguyen <quynh.nguyen.xb@renesas.com>

Add I3C node to RZ/G3S SoC DTSI

Signed-off-by: Quynh Nguyen <quynh.nguyen.xb@renesas.com>
[wsa: rebased, moved bus frequencies to board file]
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm64/boot/dts/renesas/r9a08g045.dtsi | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
index 0364f89776e6..03de06209d3c 100644
--- a/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a08g045.dtsi
@@ -272,6 +272,41 @@ channel@8 {
 			};
 		};
 
+		i3c: i3c@1005b000 {
+			compatible = "renesas,r9a08g045-i3c", "renesas,i3c";
+			reg = <0 0x1005b000 0 0x1000>;
+			clocks = <&cpg CPG_MOD R9A08G045_I3C_PCLK>,
+				 <&cpg CPG_MOD R9A08G045_I3C_TCLK>;
+			clock-names = "pclk", "tclk";
+			interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 293 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 294 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 295 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 296 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 297 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 298 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 299 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "ierr", "terr", "abort", "resp", "cmd",
+					  "ibi", "rx", "tx", "rcv","st", "sp",
+					  "tend", "nack", "al", "tmo", "wu", "exit";
+			resets = <&cpg R9A08G045_I3C_PRESETN>,
+				 <&cpg R9A08G045_I3C_TRESETN>;
+			reset-names = "presetn", "tresetn";
+			power-domains = <&cpg>;
+			#address-cells = <3>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		vbattb: clock-controller@1005c000 {
 			compatible = "renesas,r9a08g045-vbattb";
 			reg = <0 0x1005c000 0 0x1000>;
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH RFC 6/7] arm64: dts: renesas: r9a09g047: Add I3C node
  2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
                   ` (4 preceding siblings ...)
  2025-06-11  9:39 ` [PATCH RFC 5/7] arm64: dts: renesas: r9a08g045: Add I3C node Wolfram Sang
@ 2025-06-11  9:39 ` Wolfram Sang
  2025-06-11  9:39 ` [PATCH RFC 7/7] WIP: arm64: dts: renesas: rzg3s-smarc-som: Enable I3C Wolfram Sang
  2025-06-11 13:11 ` [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Rob Herring (Arm)
  7 siblings, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11  9:39 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Tommaso Merciai, Wolfram Sang, Geert Uytterhoeven, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree

From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>

Add I3C node support.

Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 arch/arm64/boot/dts/renesas/r9a09g047.dtsi | 35 ++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
index 876f70fed433..0c0fbddfd7d4 100644
--- a/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
+++ b/arch/arm64/boot/dts/renesas/r9a09g047.dtsi
@@ -580,6 +580,41 @@ i2c8: i2c@11c01000 {
 			status = "disabled";
 		};
 
+		i3c0: i3c@12400000 {
+			compatible = "renesas,r9a09g047-i3c", "renesas,i3c";
+			reg = <0 0x12400000 0 0x10000>;
+			clocks = <&cpg CPG_MOD 0x90>,
+				 <&cpg CPG_MOD 0x91>,
+				 <&cpg CPG_MOD 0x92>;
+			clock-names = "pclk", "pclkrw", "tclk";
+			interrupts = <GIC_SPI 674 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 675 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 676 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 677 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 678 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 679 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 680 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 681 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 682 IRQ_TYPE_EDGE_RISING>,
+				     <GIC_SPI 689 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 690 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 692 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 693 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 694 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 695 IRQ_TYPE_LEVEL_HIGH>,
+				     <GIC_SPI 696 IRQ_TYPE_LEVEL_HIGH>;
+			interrupt-names = "ierr", "terr", "abort", "resp",
+					  "cmd", "ibi", "rx", "tx", "rcv",
+					  "st", "sp", "tend", "nack", "al",
+					  "tmo", "wu";
+			resets = <&cpg 0x96>, <&cpg 0x97>;
+			reset-names = "presetn", "tresetn";
+			power-domains = <&cpg>;
+			#address-cells = <3>;
+			#size-cells = <0>;
+			status = "disabled";
+		};
+
 		gpu: gpu@14850000 {
 			compatible = "renesas,r9a09g047-mali",
 				     "arm,mali-bifrost";
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH RFC 7/7] WIP: arm64: dts: renesas: rzg3s-smarc-som: Enable I3C
  2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
                   ` (5 preceding siblings ...)
  2025-06-11  9:39 ` [PATCH RFC 6/7] arm64: dts: renesas: r9a09g047: " Wolfram Sang
@ 2025-06-11  9:39 ` Wolfram Sang
  2025-06-11 13:11 ` [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Rob Herring (Arm)
  7 siblings, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11  9:39 UTC (permalink / raw)
  To: linux-renesas-soc
  Cc: Tommaso Merciai, Wolfram Sang, Geert Uytterhoeven, Magnus Damm,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley, devicetree

This is really only for testing. Not for upstream!

Not-signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
---
 .../boot/dts/renesas/rzg3s-smarc-som.dtsi     | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
index 39845faec894..f3f391c609d3 100644
--- a/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
+++ b/arch/arm64/boot/dts/renesas/rzg3s-smarc-som.dtsi
@@ -7,6 +7,7 @@
 
 #include <dt-bindings/clock/renesas,r9a08g045-vbattb.h>
 #include <dt-bindings/gpio/gpio.h>
+#include <dt-bindings/i3c/i3c.h>
 #include <dt-bindings/pinctrl/rzg2l-pinctrl.h>
 
 #include "rzg3s-smarc-switches.h"
@@ -172,6 +173,38 @@ a0 80 30 30 9c
 	};
 };
 
+#undef I3C_BUS_PURE
+#ifdef I3C_BUS_PURE
+&i3c {
+	i2c-scl-hz = <1000000>;
+	i3c-scl-hz = <2000000>;		/* slow Logic Analyzer here */
+	//i3c-scl-hz = <12500000>;
+	status = "okay";
+};
+#else
+&i3c {
+	i2c-scl-hz = <400000>;		/* Max speed of the ADT7411 below */
+	/*
+	 * 10MHz works somewhat with my 24MHz logic analyzer as well as
+	 * signal width < 50ns for the legacy I2C filters
+	 */
+	i3c-scl-hz = <10000000>;
+	status = "okay";
+
+	/* I2C bus from SMARC via PMOD6A. EEPROM driver only used to verify register content */
+	eeprom@1a {
+		compatible = "atmel,24c02";
+		reg = <0x1a 0 (I2C_FM | I2C_FILTER)>;
+	};
+
+	/* externel development board */
+	temp@4a {
+		compatible = "adi,adt7411";
+		reg = <0x4a 0 (I2C_FM | I2C_FILTER)>;
+	};
+};
+#endif
+
 #if SW_CONFIG2 == SW_ON
 /* SD0 slot */
 &sdhi0 {
-- 
2.47.2


^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E
  2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
                   ` (6 preceding siblings ...)
  2025-06-11  9:39 ` [PATCH RFC 7/7] WIP: arm64: dts: renesas: rzg3s-smarc-som: Enable I3C Wolfram Sang
@ 2025-06-11 13:11 ` Rob Herring (Arm)
  2025-06-11 18:56   ` Wolfram Sang
  7 siblings, 1 reply; 35+ messages in thread
From: Rob Herring (Arm) @ 2025-06-11 13:11 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-clk, devicetree, Tommaso Merciai, Krzysztof Kozlowski,
	linux-hardening, linux-i3c, Alexandre Belloni, Philipp Zabel,
	Kees Cook, Magnus Damm, linux-renesas-soc, Gustavo A. R. Silva,
	Stephen Boyd, Frank Li, Geert Uytterhoeven, Conor Dooley,
	Michael Turquette


On Wed, 11 Jun 2025 11:39:24 +0200, Wolfram Sang wrote:
> Here is finally the first RFC of a driver for the Renesas I3C IP. It was
> created by merging two versions of it from two different BSPs. Then,
> improved according to code analyzers, cleaned up with regard to coding
> style, and then refactored to hopefully match I3C subsystem standards.
> 
> It is a basic driver for the I3C IP found in various SoCs like RZ/G3S
> and G3E. Missing features to be added incrementally are IBI, HotJoin and
> maybe target support. Other than that, this driver has been tested with
> I3C pure busses (2 targets) and mixed busses (2 I3C + various I2C
> targets). DAA and reading/writing to the temperature sensors worked
> reliably at different speeds. Scoping the bus, the output from the
> protocol analyzer seems reasonable, too. But hey, I am still new to all
> this, so I might have overlooked something.
> 
> The first patches are needed to enable I3C on the RZ/G3S and G3E boards.
> Once this series loses RFC status, they will be sent out individually,
> of course. All is on top of 6.16-rc1. A branch can be found here:
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git renesas/g3s/i3c
> 
> Why is this still RFC?
> 
> - On G3E (but not G3S), we get a spurious irq during boot. We are
>   working on it. This is just platform dependent, though, kind of
>   independent of the high level design of the driver. For this, we
>   would love to get comments already. So, we can fix things in parallel
> 
> - G3S has 17 irqs, G3E only 16. The way we handle this might need
>   discussion (see patch 3)
> 
> - On G3S, clocks are named 'i3c' while on G3E they are named 'i3c0'
>   I don't have all the needed docs for this, but Tommaso can surely
>   figure this out meanwhile
> 
> - There are some open questions regarding the driver itself
>   (see patch 4)
> 
> Really looking forward to comments! This has been quite a ride. Getting
> a suitable test setup was a surprisingly big task. If someone knows an
> off-the-shelf device supporting HotJoin, I am all ears. I couldn't find
> one.
> 
> So much for now here, some patches have more details.
> 
> All the best,
> 
>    Wolfram
> 
> 
> Quynh Nguyen (1):
>   arm64: dts: renesas: r9a08g045: Add I3C node
> 
> Tommaso Merciai (3):
>   clk: renesas: r9a09g047: Add I3C0 clocks and resets
>   dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
>   arm64: dts: renesas: r9a09g047: Add I3C node
> 
> Wolfram Sang (3):
>   clk: renesas: r9a08g045: Add I3C clocks, resets and power domain
>   i3c: add driver for Renesas I3C IP
>   WIP: arm64: dts: renesas: rzg3s-smarc-som: Enable I3C
> 
>  .../devicetree/bindings/i3c/renesas,i3c.yaml  |  186 +++
>  MAINTAINERS                                   |    7 +
>  arch/arm64/boot/dts/renesas/r9a08g045.dtsi    |   35 +
>  arch/arm64/boot/dts/renesas/r9a09g047.dtsi    |   35 +
>  .../boot/dts/renesas/rzg3s-smarc-som.dtsi     |   33 +
>  drivers/clk/renesas/r9a08g045-cpg.c           |    7 +
>  drivers/clk/renesas/r9a09g047-cpg.c           |    8 +
>  drivers/i3c/master/Kconfig                    |   10 +
>  drivers/i3c/master/Makefile                   |    1 +
>  drivers/i3c/master/renesas-i3c.c              | 1441 +++++++++++++++++
>  10 files changed, 1763 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
>  create mode 100644 drivers/i3c/master/renesas-i3c.c
> 
> --
> 2.47.2
> 
> 
> 


My bot found new DTB warnings on the .dts files added or changed in this
series.

Some warnings may be from an existing SoC .dtsi. Or perhaps the warnings
are fixed by another series. Ultimately, it is up to the platform
maintainer whether these warnings are acceptable or not. No need to reply
unless the platform maintainer has comments.

If you already ran DT checks and didn't see these error(s), then
make sure dt-schema is up to date:

  pip3 install dtschema --upgrade


This patch series was applied (using b4) to base:
 Base: attempting to guess base-commit...
 Base: tags/v6.16-rc1 (exact match)

If this is not the correct base, please add 'base-commit' tag
(or use b4 which does this automatically)

New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/renesas/' for 20250611093934.4208-1-wsa+renesas@sang-engineering.com:

arch/arm64/boot/dts/renesas/r9a08g045s33-smarc.dtb: /soc/i3c@1005b000/temp@4a: failed to match any schema with compatible: ['adi,adt7411']






^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-11  9:39 ` [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller Wolfram Sang
@ 2025-06-11 15:40   ` Frank Li
  2025-06-12 14:31     ` Wolfram Sang
  2025-06-25 20:07   ` Rob Herring
  1 sibling, 1 reply; 35+ messages in thread
From: Frank Li @ 2025-06-11 15:40 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, linux-i3c, devicetree

On Wed, Jun 11, 2025 at 11:39:27AM +0200, Wolfram Sang wrote:
> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
>
> Available in RZ/G3S and G3E SoCs. The G3S has 17 interrupts, the G3E 16
> with the "HDR exit" interrupt missing. This interrupt is put to the end
> of the list, so we can handle the difference using "minItems" instead of
> defining a separate interrupt list per SoC.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Tommaso also did a version with two interrupt lists and using 'oneOf'. I
> prefer this version but maybe the other one is preferred upstream?
>
>  .../devicetree/bindings/i3c/renesas,i3c.yaml  | 186 ++++++++++++++++++
>  1 file changed, 186 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
>
> diff --git a/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml b/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
> new file mode 100644
> index 000000000000..e6e6c57bb56c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i3c/renesas,i3c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas I3C Bus Interface
> +
> +maintainers:
> +  - Wolfram Sang <wsa+renesas@sang-engineering.com>
> +  - Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,r9a08g045-i3c # RZ/G3S
> +          - renesas,r9a09g047-i3c # RZ/G3E
> +      - const: renesas,i3c
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: Non-recoverable internal error interrupt
> +      - description: Normal transfer error interrupt
> +      - description: Normal transfer abort interrupt
> +      - description: Normal response status buffer full interrupt
> +      - description: Normal command buffer empty interrupt
> +      - description: Normal IBI status buffer full interrupt
> +      - description: Normal Rx data buffer full interrupt
> +      - description: Normal Tx data buffer empty interrupt
> +      - description: Normal receive status buffer full interrupt
> +      - description: START condition detection interrupt
> +      - description: STOP condition detection interrupt
> +      - description: Transmit end interrupt
> +      - description: NACK detection interrupt
> +      - description: Arbitration lost interrupt
> +      - description: Timeout detection interrupt
> +      - description: Wake-up condition detection interrupt
> +      - description: HDR Exit Pattern detection interrupt
> +    minItems: 16
> +
> +  interrupt-names:
> +    items:
> +      - const: ierr
> +      - const: terr
> +      - const: abort
> +      - const: resp
> +      - const: cmd
> +      - const: ibi
> +      - const: rx
> +      - const: tx
> +      - const: rcv
> +      - const: st
> +      - const: sp
> +      - const: tend
> +      - const: nack
> +      - const: al
> +      - const: tmo
> +      - const: wu
> +      - const: exit
> +    minItems: 16
> +
> +  clocks:
> +    oneOf:
> +      - items:
> +          - description: APB bus clock
> +          - description: transfer clock
> +      - items:
> +          - description: APB bus clock
> +          - description: SFRs clock
> +          - description: transfer clock
> +
> +  clock-names:
> +    oneOf:
> +      - items:
> +          - const: pclk
> +          - const: tclk
> +      - items:
> +          - const: pclk
> +          - const: pclkrw
> +          - const: tclk

Suggested clock name is, apb, trans, srfs.
Needn't clk surfix and provide meanful name.

> +
> +  power-domains:
> +    maxItems: 1
> +
> +  resets:
> +    items:
> +      - description: Reset signal
> +      - description: APB interface reset signal/SCAN reset signal
> +
> +  reset-names:
> +    items:
> +      - const: presetn
> +      - const: tresetn

the same here:  per, apb

> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts
> +  - interrupt-names
> +  - clock-names
> +  - clocks
> +  - power-domains
> +  - resets
> +  - reset-names
> +
> +allOf:
> +  - $ref: i3c.yaml#
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a08g045-i3c
> +    then:
> +      properties:
> +        clocks:
> +          maxItems: 2
> +        clock-names:
> +          maxItems: 2
> +        interrupts:
> +          minItems: 17
> +        interrupt-names:
> +          minItems: 17
> +
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a09g047-i3c
> +    then:
> +      properties:
> +        clocks:
> +          minItems: 3
> +        clock-names:
> +          minItems: 3
> +        interrupts:
> +          maxItems: 16
> +        interrupt-names:
> +          maxItems: 16
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/r9a08g045-cpg.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +
> +    i3c@1005b000 {
> +        compatible = "renesas,r9a08g045-i3c", "renesas,i3c";
> +        reg = <0x1005b000 0x1000>;
> +        clocks = <&cpg CPG_MOD R9A08G045_I3C_PCLK>,
> +                 <&cpg CPG_MOD R9A08G045_I3C_TCLK>;
> +        clock-names = "pclk", "tclk";
> +        interrupts = <GIC_SPI 289 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 290 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 293 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 294 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 295 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 296 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 297 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 298 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 299 IRQ_TYPE_EDGE_RISING>,
> +                     <GIC_SPI 304 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 308 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
> +                     <GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>;
> +        interrupt-names = "ierr", "terr", "abort", "resp",
> +                          "cmd", "ibi", "rx", "tx", "rcv",
> +                          "st", "sp", "tend", "nack",
> +                          "al", "tmo", "wu", "exit";
> +        resets = <&cpg R9A08G045_I3C_PRESETN>,
> +                 <&cpg R9A08G045_I3C_TRESETN>;
> +        reset-names = "presetn", "tresetn";
> +        power-domains = <&cpg>;
> +        #address-cells = <3>;
> +        #size-cells = <0>;

I prefer put #address-cells and #size-cells after reg

Frank
> +    };
> +...
> --
> 2.47.2
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-11  9:39 ` [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP Wolfram Sang
@ 2025-06-11 16:37   ` Frank Li
  2025-06-12 14:55     ` Wolfram Sang
                       ` (2 more replies)
  0 siblings, 3 replies; 35+ messages in thread
From: Frank Li @ 2025-06-11 16:37 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening

On Wed, Jun 11, 2025 at 11:39:28AM +0200, Wolfram Sang wrote:
> This adds a basic driver for the I3C IP found in various SoCs like
> RZ/G3S and G3E. Missing features to be added incrementally are IBI,
> HotJoin and target support. Other than that, this driver has been tested
> with I3C pure busses (2 targets) and mixed busses (2 I3C + various I2C
> targets). DAA and reading/writing to the temperature sensors worked
> reliably at different speeds.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
>
> Open questions:
>
> - RENESAS_I3C_MAX_DEVS is a constant currently. So, we could get rid of
>   the 'maxdevs' variable. I tend to keep it this way in case future
>   controllers may use a different value, then we can change it easily.

It is similar ADI version. Can you simple descript hardware behavor to show
why need RENESAS_I3C_MAX_DEVS,

after get one whole PID and set dync addr, you should have chance to get
free address to let hardware to continue DAA.

>
> - I am not exactly happy with "renesas,i3c" as the generic fallback
>   compatible. But I couldn't derive a better one from the docs.
>   Suggestions welcome.
>
> - For accessing the FIFOs, this driver uses the same code as existing
>   upstream drivers or the recenlty submitted "ADI" driver. There, the
>   question came up, if this could be a helper function?

Add common header in drivers/i3c/master/core.h implement inline helper
function

static inline i3c_writel_fifo(void __iomem *fifo, void *buff, size_t sz)
{
	...
}

static inline i3c_readl_fifo(void __iomem *fifo, void *buff, size_t sz)
{
        ...
}

>
>  MAINTAINERS                      |    7 +
>  drivers/i3c/master/Kconfig       |   10 +
>  drivers/i3c/master/Makefile      |    1 +
>  drivers/i3c/master/renesas-i3c.c | 1441 ++++++++++++++++++++++++++++++
>  4 files changed, 1459 insertions(+)
>  create mode 100644 drivers/i3c/master/renesas-i3c.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index a92290fffa16..a90daa29b48b 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11458,6 +11458,13 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/i3c/cdns,i3c-master.yaml
>  F:	drivers/i3c/master/i3c-master-cdns.c
>
> +I3C DRIVER FOR RENESAS
> +M:	Wolfram Sang <wsa+renesas@sang-engineering.com>
> +M:	Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> +S:	Supported
> +F:	Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
> +F:	drivers/i3c/master/renesas-i3c.c
> +
>  I3C DRIVER FOR SYNOPSYS DESIGNWARE
>  S:	Orphan
>  F:	Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> diff --git a/drivers/i3c/master/Kconfig b/drivers/i3c/master/Kconfig
> index 7b30db3253af..13df2944f2ec 100644
> --- a/drivers/i3c/master/Kconfig
> +++ b/drivers/i3c/master/Kconfig
> @@ -64,3 +64,13 @@ config MIPI_I3C_HCI_PCI
>
>  	  This driver can also be built as a module. If so, the module will be
>  	  called mipi-i3c-hci-pci.
> +
> +config RENESAS_I3C
> +	tristate "Renesas I3C controller driver"
> +	depends on HAS_IOMEM
> +	depends on ARCH_RENESAS || COMPILE_TEST
> +	help
> +	  Support the Renesas I3C controller as found in some RZ variants.
> +
> +	  This driver can also be built as a module. If so, the module will be
> +	  called renesas-i3c.
> diff --git a/drivers/i3c/master/Makefile b/drivers/i3c/master/Makefile
> index 3e97960160bc..aac74f3e3851 100644
> --- a/drivers/i3c/master/Makefile
> +++ b/drivers/i3c/master/Makefile
> @@ -4,3 +4,4 @@ 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
>  obj-$(CONFIG_MIPI_I3C_HCI)		+= mipi-i3c-hci/
> +obj-$(CONFIG_RENESAS_I3C)		+= renesas-i3c.o
> diff --git a/drivers/i3c/master/renesas-i3c.c b/drivers/i3c/master/renesas-i3c.c
> new file mode 100644
> index 000000000000..ff276c955d07
> --- /dev/null
> +++ b/drivers/i3c/master/renesas-i3c.c
> @@ -0,0 +1,1441 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Renesas I3C Controller driver
> + * Copyright (C) 2023-25 Renesas Electronics Corp.
> + *
> + * TODO: IBI support, HotJoin support, Target support
> + */
> +
> +#include <linux/bitops.h>
> +#include <linux/clk.h>
> +#include <linux/completion.h>
> +#include <linux/err.h>
> +#include <linux/errno.h>
> +#include <linux/i2c.h>
> +#include <linux/i3c/master.h>
> +#include <linux/interrupt.h>
> +#include <linux/ioport.h>
> +#include <linux/iopoll.h>
> +#include <linux/list.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset.h>
> +#include <linux/slab.h>
> +
> +#define PRTS			0x00
> +#define PRTS_PRTMD		BIT(0)

#define PRTS                 0x00
#define  PRTS_PRTMD          BIT(0)

Add extra space help distinguish register and field define.

> +
> +#define BCTL			0x14
> +#define BCTL_HJACKCTL		BIT(8)
> +#define BCTL_ABT		BIT(29)
> +#define BCTL_BUSE		BIT(31)
> +
> +#define MSDVAD			0x18
> +#define MSDVAD_MDYAD(x)		(((x) & 0x3f) << 16)

Use GEN_MASK

> +#define MSDVAD_MDYADV		BIT(31)
> +
> +#define RSTCTL			0x20
> +#define RSTCTL_RI3CRST		BIT(0)
> +#define RSTCTL_INTLRST		BIT(16)
> +
> +#define INST			0x30
> +
> +#define IBINCTL			0x58
> +#define IBINCTL_NRHJCTL		BIT(0)
> +#define IBINCTL_NRMRCTL		BIT(1)
> +#define IBINCTL_NRSIRCTL	BIT(3)
> +
> +#define SVCTL			0x64
> +
> +#define REFCKCTL		0x70
> +#define REFCKCTL_IREFCKS(x)	(((x) & 0x7) << 0)
> +
> +#define STDBR			0x74
> +#define STDBR_SBRLO(cond, x)	((((x) >> (cond)) & 0xff) << 0)
> +#define STDBR_SBRHO(cond, x)	((((x) >> (cond)) & 0xff) << 8)

FIELD_GET FIELD_PREP

check other define

> +#define STDBR_SBRLP(x)		(((x) & 0x3f) << 16)
> +#define STDBR_SBRHP(x)		(((x) & 0x3f) << 24)
> +#define STDBR_DSBRPO		BIT(31)
> +
> +#define EXTBR			0x78
> +#define EXTBR_EBRLO(x)		(((x) & 0xff) << 0)
> +#define EXTBR_EBRHO(x)		(((x) & 0xff) << 8)
> +#define EXTBR_EBRLP(x)		(((x) & 0x3f) << 16)
> +#define EXTBR_EBRHP(x)		(((x) & 0x3f) << 24)
> +
> +#define BFRECDT			0x7c
> +#define BFRECDT_FRECYC(x)	(((x) & 0x1ff) << 0)
> +
> +#define BAVLCDT			0x80
> +#define BAVLCDT_AVLCYC(x)	(((x) & 0x1ff) << 0)
> +
> +#define BIDLCDT			0x84
> +#define BIDLCDT_IDLCYC(x)	(((x) & 0x3ffff) << 0)
> +
> +#define ACKCTL			0xa0
> +#define ACKCTL_ACKT		BIT(1)
> +#define ACKCTL_ACKTWP		BIT(2)
> +
> +#define SCSTRCTL		0xa4
> +#define SCSTRCTL_ACKTWE		BIT(0)
> +#define SCSTRCTL_RWE		BIT(1)
> +
> +#define SCSTLCTL		0xb0
> +
> +#define CNDCTL			0x140
> +#define CNDCTL_STCND		BIT(0)
> +#define CNDCTL_SRCND		BIT(1)
> +#define CNDCTL_SPCND		BIT(2)
> +
> +#define NCMDQP			0x150 /* Normal Command Queue */
> +#define NCMDQP_CMD_ATTR(x)	(((x) & 0x7) << 0)
> +#define NCMDQP_IMMED_XFER	0x01
> +#define NCMDQP_ADDR_ASSGN	0x02
> +#define NCMDQP_TID(x)		(((x) & 0xf) << 3)
> +#define NCMDQP_CMD(x)		(((x) & 0xff) << 7)
> +#define NCMDQP_CP		BIT(15)
> +#define NCMDQP_DEV_INDEX(x)	(((x) & 0x1f) << 16)
> +#define NCMDQP_BYTE_CNT(x)	(((x) & 0x7) << 23)
> +#define NCMDQP_DEV_COUNT(x)	(((x) & 0xf) << 26)
> +#define NCMDQP_MODE(x)		(((x) & 0x7) << 26)
> +#define NCMDQP_RNW(x)		(((x) & 0x1) << 29)
> +#define NCMDQP_ROC		BIT(30)
> +#define NCMDQP_TOC		BIT(31)
> +#define NCMDQP_DATA_LENGTH(x)	(((x) & 0xffff) << 16)
> +
> +#define NRSPQP			0x154 /* Normal Respone Queue */
> +#define NRSPQP_NO_ERROR			0
> +#define NRSPQP_ERROR_CRC		1
> +#define NRSPQP_ERROR_PARITY		2
> +#define NRSPQP_ERROR_FRAME		3
> +#define NRSPQP_ERROR_IBA_NACK		4
> +#define NRSPQP_ERROR_ADDRESS_NACK	5
> +#define NRSPQP_ERROR_OVER_UNDER_FLOW	6
> +#define NRSPQP_ERROR_TRANSF_ABORT	8
> +#define NRSPQP_ERROR_I2C_W_NACK_ERR	9
> +#define NRSPQP_ERR_STATUS(x)	(((x) & GENMASK(31, 28)) >> 28)
> +#define NRSPQP_DATA_LEN(x)	((x) & GENMASK(15, 0))
> +
> +#define NTDTBP0			0x158 /* Normal Transfer Data Buffer */
> +#define NTDTBP0_DEPTH		16
> +
> +#define NQTHCTL			0x190
> +#define NQTHCTL_CMDQTH(x)	(((x) & 0x3) << 0)
> +#define NQTHCTL_IBIDSSZ(x)	(((x) & 0xff) << 16)
> +
> +#define NTBTHCTL0		0x194
> +
> +#define NRQTHCTL		0x1c0
> +
> +#define BST			0x1d0
> +#define BST_STCNDDF		BIT(0)
> +#define BST_SPCNDDF		BIT(1)
> +#define BST_NACKDF		BIT(4)
> +#define BST_TENDF		BIT(8)
> +
> +#define BSTE			0x1d4
> +#define BSTE_STCNDDE		BIT(0)
> +#define BSTE_SPCNDDE		BIT(1)
> +#define BSTE_NACKDE		BIT(4)
> +#define BSTE_TENDE		BIT(8)
> +#define BSTE_ALE		BIT(16)
> +#define BSTE_TODE		BIT(20)
> +#define BSTE_WUCNDDE		BIT(24)
> +#define BSTE_ALL_FLAG		(BSTE_STCNDDE | BSTE_SPCNDDE |\
> +				BSTE_NACKDE | BSTE_TENDE |\
> +				BSTE_ALE | BSTE_TODE | BSTE_WUCNDDE)
> +
> +#define BIE			0x1d8
> +#define BIE_STCNDDIE		BIT(0)
> +#define BIE_SPCNDDIE		BIT(1)
> +#define BIE_NACKDIE		BIT(4)
> +#define BIE_TENDIE		BIT(8)
> +
> +#define NTST			0x1e0
> +#define NTST_TDBEF0		BIT(0)
> +#define NTST_RDBFF0		BIT(1)
> +#define NTST_CMDQEF		BIT(3)
> +#define NTST_RSPQFF		BIT(4)
> +#define NTST_TABTF		BIT(5)
> +#define NTST_TEF		BIT(9)
> +
> +#define NTSTE			0x1e4
> +#define NTSTE_TDBEE0		BIT(0)
> +#define NTSTE_RDBFE0		BIT(1)
> +#define NTSTE_IBIQEFE		BIT(2)
> +#define NTSTE_CMDQEE		BIT(3)
> +#define NTSTE_RSPQFE		BIT(4)
> +#define NTSTE_TABTE		BIT(5)
> +#define NTSTE_TEE		BIT(9)
> +#define NTSTE_RSQFE		BIT(20)
> +#define NTSTE_ALL_FLAG		(NTSTE_TDBEE0 | NTSTE_RDBFE0 |\
> +				NTSTE_IBIQEFE | NTSTE_CMDQEE |\
> +				NTSTE_RSPQFE | NTSTE_TABTE |\
> +				NTSTE_TEE | NTSTE_RSQFE)
> +
> +#define NTIE			0x1e8
> +#define NTIE_TDBEIE0		BIT(0)
> +#define NTIE_RDBFIE0		BIT(1)
> +#define NTIE_IBIQEFIE		BIT(2)
> +#define NTIE_RSPQFIE		BIT(4)
> +#define NTIE_RSQFIE		BIT(20)
> +
> +#define BCST			0x210
> +#define BCST_BFREF		BIT(0)
> +
> +#define DATBAS(x)		(0x224 + 0x8 * (x))
> +#define DATBAS_DVSTAD(x)	(((x) & 0x7f) << 0)
> +#define DATBAS_DVDYAD(x)	(((x) & 0xff) << 16)
> +
> +#define NDBSTLV0		0x398
> +#define NDBSTLV0_RDBLV(x)	(((x) >> 8) & 0xff)
> +
> +#define RENESAS_I3C_MAX_DEVS	8
> +#define I2C_INIT_MSG		-1
> +
> +/* Bus condition timing */
> +#define I3C_BUS_THIGH_MIXED_NS	40		/* 40ns */
> +#define I3C_BUS_FREE_TIME_NS	1300		/* 1.3us for Mixed Bus with I2C FM Device*/
> +#define I3C_BUS_AVAL_TIME_NS	1000		/* 1us */
> +#define I3C_BUS_IDLE_TIME_NS	200000		/* 200us */
> +
> +#define XFER_TIMEOUT		(msecs_to_jiffies(1000))
> +
> +enum i3c_internal_state {
> +	I3C_INTERNAL_STATE_DISABLED,
> +	I3C_INTERNAL_STATE_CONTROLLER_IDLE,
> +	I3C_INTERNAL_STATE_CONTROLLER_ENTDAA,
> +	I3C_INTERNAL_STATE_CONTROLLER_SETDASA,
> +	I3C_INTERNAL_STATE_CONTROLLER_WRITE,
> +	I3C_INTERNAL_STATE_CONTROLLER_READ,
> +	I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE,
> +	I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ,
> +};
> +
> +enum i3c_event {
> +	I3C_COMMAND_ADDRESS_ASSIGNMENT,
> +	I3C_WRITE,
> +	I3C_READ,
> +	I3C_COMMAND_WRITE,
> +	I3C_COMMAND_READ,
> +};
> +
> +struct renesas_i3c_cmd {
> +	u32 cmd0;
> +	u32 len;
> +	const void *tx_buf;
> +	u32 tx_count;
> +	void *rx_buf;
> +	u32 rx_count;
> +	u32 err;
> +	u8 rnw;
> +	/* i2c xfer */
> +	int i2c_bytes_left;
> +	int i2c_is_last;
> +	u8 *i2c_buf;
> +	const struct i2c_msg *msg;
> +};
> +
> +struct renesas_i3c_xfer {
> +	struct list_head node;
> +	struct completion comp;
> +	int ret;
> +	bool is_i2c_xfer;
> +	unsigned int ncmds;
> +	struct renesas_i3c_cmd cmds[] __counted_by(ncmds);
> +};
> +
> +struct renesas_i3c_xferqueue {
> +	struct list_head list;
> +	struct renesas_i3c_xfer *cur;
> +	/* Lock for accessing the xfer queue */
> +	spinlock_t lock;
> +};
> +
> +struct renesas_i3c {
> +	struct i3c_master_controller base;
> +	enum i3c_internal_state internal_state;
> +	u16 maxdevs;
> +	u32 free_pos;
> +	u32 i2c_STDBR;
> +	u32 i3c_STDBR;
> +	u8 addrs[RENESAS_I3C_MAX_DEVS];
> +	struct renesas_i3c_xferqueue xferqueue;
> +	void __iomem *regs;
> +	struct clk *tclk;
> +};
> +
> +struct renesas_i3c_i2c_dev_data {
> +	u8 index;
> +};
> +
> +struct renesas_i3c_irq_desc {
> +	char *name;
> +	irq_handler_t isr;
> +	char *desc;
> +};
> +
> +struct renesas_i3c_config {
> +	unsigned int has_pclkrw:1;
> +};
> +
> +static inline void i3c_reg_update(u32 mask, u32 val, void __iomem *reg)
> +{
> +	u32 data = readl(reg);
> +
> +	data &= ~mask;
> +	data |= (val & mask);
> +	writel(data, reg);
> +}

can you move reg to first argument to align below help function?

> +
> +static inline u32 i3c_reg_read(void __iomem *base, u32 reg)
> +{
> +	return readl(base + reg);
> +}
> +
> +static inline void i3c_reg_write(void __iomem *base, u32 reg, u32 val)
> +{
> +	writel(val, base + reg);
> +}
> +
> +static void i3c_reg_set_bit(void __iomem *base, u32 reg, u32 val)
> +{
> +	i3c_reg_update(val, val, base + reg);
> +}
> +
> +static void i3c_reg_clear_bit(void __iomem *base, u32 reg, u32 val)
> +{
> +	i3c_reg_update(val, 0, base + reg);
> +}
> +
> +static void i3c_reg_update_bit(void __iomem *base, u32 reg,
> +			       u32 mask, u32 val)
> +{
> +	i3c_reg_update(mask, val, base + reg);
> +}
> +
> +static inline struct renesas_i3c *
> +to_renesas_i3c(struct i3c_master_controller *master)
> +{
> +	return container_of(master, struct renesas_i3c, base);
> +}
> +
> +static inline u32 datbas_dvdyad_with_parity(u8 addr)
> +{
> +	return DATBAS_DVDYAD(addr | (parity8(addr) ? 0 : BIT(7)));
> +}
> +
> +static int renesas_i3c_get_free_pos(struct renesas_i3c *i3c)
> +{
> +	if (!(i3c->free_pos & GENMASK(i3c->maxdevs - 1, 0)))
> +		return -ENOSPC;
> +
> +	return ffs(i3c->free_pos) - 1;
> +}
> +
> +static int renesas_i3c_get_addr_pos(struct renesas_i3c *i3c, u8 addr)
> +{
> +	int pos;
> +
> +	for (pos = 0; pos < i3c->maxdevs; pos++) {
> +		if (addr == i3c->addrs[pos])
> +			return pos;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +static struct renesas_i3c_xfer *
> +renesas_i3c_alloc_xfer(struct renesas_i3c *i3c, unsigned int ncmds)
> +{
> +	struct renesas_i3c_xfer *xfer;
> +
> +	xfer = kzalloc(struct_size(xfer, cmds, ncmds), GFP_KERNEL);
> +	if (!xfer)
> +		return NULL;
> +
> +	INIT_LIST_HEAD(&xfer->node);
> +	xfer->ncmds = ncmds;
> +	xfer->ret = -ETIMEDOUT;
> +
> +	return xfer;
> +}
> +
> +static void renesas_i3c_read_from_rx_fifo(struct renesas_i3c *i3c,
> +						 u8 *data, int nbytes)
> +{
> +	readsl(i3c->regs + NTDTBP0, data, nbytes / 4);
> +	if (nbytes & 3) {
> +		u32 tmp;
> +
> +		readsl(i3c->regs + NTDTBP0, &tmp, 1);
> +		memcpy(data + (nbytes & ~3), &tmp, nbytes & 3);
> +	}
> +}
> +
> +static void renesas_i3c_write_to_tx_fifo(struct renesas_i3c *i3c,
> +						const u32 *data, int nbytes)
> +{
> +	writesl(i3c->regs + NTDTBP0, data, nbytes / 4);
> +	if (nbytes & 3) {
> +		u32 tmp = 0;
> +
> +		memcpy(&tmp, data + (nbytes & ~3), nbytes & 3);
> +		writesl(i3c->regs + NTDTBP0, &tmp, 1);
> +	}
> +}
> +
> +static void renesas_i3c_start_xfer_locked(struct renesas_i3c *i3c)
> +{
> +	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
> +	struct renesas_i3c_cmd *cmd;
> +	u32 cmd1;
> +
> +	if (!xfer)
> +		return;
> +
> +	cmd = xfer->cmds;
> +
> +	switch (i3c->internal_state) {
> +	case I3C_INTERNAL_STATE_CONTROLLER_ENTDAA:
> +	case I3C_INTERNAL_STATE_CONTROLLER_SETDASA:
> +		i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RSPQFIE);
> +		i3c_reg_write(i3c->regs, NCMDQP, cmd->cmd0);
> +		i3c_reg_write(i3c->regs, NCMDQP, 0);
> +		break;
> +	case I3C_INTERNAL_STATE_CONTROLLER_WRITE:
> +	case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE:
> +		i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RSPQFIE);
> +		if (cmd->len <= 4) {
> +			cmd->cmd0 |= NCMDQP_CMD_ATTR(NCMDQP_IMMED_XFER);
> +			cmd->cmd0 |= NCMDQP_BYTE_CNT(cmd->len);
> +			cmd->tx_count = cmd->len;
> +			cmd1 = cmd->len == 0 ? 0 : *(u32 *)cmd->tx_buf;
> +		} else {
> +			cmd1 = NCMDQP_DATA_LENGTH(cmd->len);
> +		}
> +		i3c_reg_write(i3c->regs, NCMDQP, cmd->cmd0);
> +		i3c_reg_write(i3c->regs, NCMDQP, cmd1);
> +		break;
> +	case I3C_INTERNAL_STATE_CONTROLLER_READ:
> +	case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ:
> +		i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
> +		cmd1 = NCMDQP_DATA_LENGTH(cmd->len);
> +		i3c_reg_write(i3c->regs, NCMDQP, cmd->cmd0);
> +		i3c_reg_write(i3c->regs, NCMDQP, cmd1);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	/* Clear the command queue empty flag */
> +	i3c_reg_clear_bit(i3c->regs, NTST, NTST_CMDQEF);
> +
> +	if (cmd->len > 4 && i3c->internal_state == I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE) {
> +		renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
> +		/*
> +		 * If there is still data remaining in the transfer
> +		 * then it will be written in the Write Buffer Empty IRQ.
> +		 */
> +		if (cmd->len > NTDTBP0_DEPTH * sizeof(u32))
> +			/* Enable the Write Buffer Empty IRQ. */
> +			i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> +	}
> +}
> +
> +static void renesas_i3c_dequeue_xfer_locked(struct renesas_i3c *i3c,
> +						   struct renesas_i3c_xfer *xfer)
> +{
> +	if (i3c->xferqueue.cur == xfer)
> +		i3c->xferqueue.cur = NULL;
> +	else
> +		list_del_init(&xfer->node);
> +}
> +
> +static void renesas_i3c_dequeue_xfer(struct renesas_i3c *i3c,
> +					    struct renesas_i3c_xfer *xfer)
> +{
> +	scoped_guard(spinlock_irqsave, &i3c->xferqueue.lock)
> +		renesas_i3c_dequeue_xfer_locked(i3c, xfer);
> +}
> +
> +static void renesas_i3c_enqueue_xfer(struct renesas_i3c *i3c,
> +					    struct renesas_i3c_xfer *xfer)
> +{
> +	reinit_completion(&xfer->comp);
> +	scoped_guard(spinlock_irqsave, &i3c->xferqueue.lock) {
> +		if (i3c->xferqueue.cur) {
> +			list_add_tail(&xfer->node, &i3c->xferqueue.list);
> +		} else {
> +			i3c->xferqueue.cur = xfer;
> +			if (!xfer->is_i2c_xfer)
> +				renesas_i3c_start_xfer_locked(i3c);
> +		}
> +	}
> +}
> +
> +static void renesas_i3c_set_prts(struct renesas_i3c *i3c, u32 val)
> +{
> +	/* Required sequence according to tnrza0140ae */
> +	i3c_reg_set_bit(i3c->regs, RSTCTL, RSTCTL_INTLRST);
> +	i3c_reg_write(i3c->regs, PRTS, val);
> +	i3c_reg_clear_bit(i3c->regs, RSTCTL, RSTCTL_INTLRST);
> +}
> +
> +static void renesas_i3c_bus_enable(struct i3c_master_controller *m, bool i3c_mode)
> +{
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +
> +	/* Setup either I3C or I2C protocol */
> +	if (i3c_mode) {
> +		renesas_i3c_set_prts(i3c, 0);
> +		/* Revisit: INCBA handling, especially after I2C transfers */
> +		i3c_reg_set_bit(i3c->regs, BCTL, BCTL_HJACKCTL);
> +		i3c_reg_set_bit(i3c->regs, MSDVAD, MSDVAD_MDYADV);
> +		i3c_reg_write(i3c->regs, STDBR, i3c->i3c_STDBR);
> +	} else {
> +		renesas_i3c_set_prts(i3c, PRTS_PRTMD);
> +		i3c_reg_write(i3c->regs, STDBR, i3c->i2c_STDBR);
> +	}
> +
> +	/* Enable I3C bus */
> +	i3c_reg_set_bit(i3c->regs, BCTL, BCTL_BUSE);
> +}
> +
> +static int renesas_i3c_reset(struct renesas_i3c *i3c)
> +{
> +	u32 val;
> +
> +	i3c_reg_write(i3c->regs, BCTL, 0);
> +	i3c_reg_set_bit(i3c->regs, RSTCTL, RSTCTL_RI3CRST);
> +
> +	/* Wait for reset completion  */
> +	return readl_relaxed_poll_timeout(i3c->regs + RSTCTL, val,
> +					  !(val & RSTCTL_RI3CRST), 0, 1000);
> +}
> +
> +static int renesas_i3c_bus_init(struct i3c_master_controller *m)
> +{
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	struct i3c_bus *bus = i3c_master_get_bus(m);
> +	struct i3c_device_info info = {};
> +	struct i2c_timings t;
> +	unsigned long rate;
> +	u32 val;
> +	int cks, pp_high_ticks, pp_low_ticks, i3c_total_ticks;
> +	int od_high_ticks, od_low_ticks, i2c_total_ticks;
> +	int ret;
> +	bool double_SBR;
> +
> +	rate = clk_get_rate(i3c->tclk);
> +	if (!rate)
> +		return -EINVAL;
> +
> +	ret = renesas_i3c_reset(i3c);
> +	if (ret)
> +		return ret;
> +
> +	i2c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i2c);
> +	i3c_total_ticks = DIV_ROUND_UP(rate, bus->scl_rate.i3c);
> +
> +	i2c_parse_fw_timings(&m->dev, &t, true);
> +
> +	for (cks = 0; cks < 7; cks++) {
> +		/* SCL low-period calculation in Open-drain mode */
> +		od_low_ticks = ((i2c_total_ticks * 6) / 10);
> +
> +		/* SCL clock calculation in Push-Pull mode */
> +		if (bus->mode == I3C_BUS_MODE_PURE)
> +			pp_high_ticks = ((i3c_total_ticks * 5) / 10);
> +		else
> +			pp_high_ticks = DIV_ROUND_UP(I3C_BUS_THIGH_MIXED_NS,
> +						     1000000000 / rate);
> +		pp_low_ticks = i3c_total_ticks - pp_high_ticks;
> +
> +		if ((od_low_ticks / 2) <= 0xFF && pp_low_ticks < 0x3F)
> +			break;
> +
> +		i2c_total_ticks /= 2;
> +		i3c_total_ticks /= 2;
> +		rate /= 2;
> +	}
> +
> +	/* SCL clock period calculation in Open-drain mode */
> +	if ((od_low_ticks / 2) > 0xFF || pp_low_ticks > 0x3F) {
> +		dev_err(&m->dev, "invalid speed (i2c-scl = %lu Hz, i3c-scl = %lu Hz). Too slow.\n",
> +			(unsigned long)bus->scl_rate.i2c, (unsigned long)bus->scl_rate.i3c);
> +		ret = -EINVAL;
> +		return ret;
> +	}
> +
> +	/* SCL high-period calculation in Open-drain mode */
> +	od_high_ticks = i2c_total_ticks - od_low_ticks;
> +
> +	/* Standard Bit Rate setting */
> +	double_SBR = od_low_ticks > 0xFF ? true : false;
> +	i3c->i3c_STDBR = (double_SBR ? STDBR_DSBRPO : 0) |
> +			STDBR_SBRLO(double_SBR, od_low_ticks) |
> +			STDBR_SBRHO(double_SBR, od_high_ticks) |
> +			STDBR_SBRLP(pp_low_ticks) |
> +			STDBR_SBRHP(pp_high_ticks);
> +
> +	od_low_ticks -= t.scl_fall_ns / (1000000000 / rate) + 1;
> +	od_high_ticks -= t.scl_rise_ns / (1000000000 / rate) + 1;
> +	i3c->i2c_STDBR = (double_SBR ? STDBR_DSBRPO : 0) |
> +			STDBR_SBRLO(double_SBR, od_low_ticks) |
> +			STDBR_SBRHO(double_SBR, od_high_ticks) |
> +			STDBR_SBRLP(pp_low_ticks) |
> +			STDBR_SBRHP(pp_high_ticks);
> +	i3c_reg_write(i3c->regs, STDBR, i3c->i3c_STDBR);
> +
> +	/* Extended Bit Rate setting */
> +	i3c_reg_write(i3c->regs, EXTBR, EXTBR_EBRLO(od_low_ticks) |
> +					   EXTBR_EBRHO(od_high_ticks) |
> +					   EXTBR_EBRLP(pp_low_ticks) |
> +					   EXTBR_EBRHP(pp_high_ticks));
> +
> +	i3c_reg_write(i3c->regs, REFCKCTL, REFCKCTL_IREFCKS(cks));
> +
> +	/* Disable Slave Mode */
> +	i3c_reg_write(i3c->regs, SVCTL, 0);
> +
> +	/* Initialize Queue/Buffer threshold */
> +	i3c_reg_write(i3c->regs, NQTHCTL, NQTHCTL_IBIDSSZ(6) |
> +					     NQTHCTL_CMDQTH(1));
> +
> +	/* The only supported configuration is two entries*/
> +	i3c_reg_write(i3c->regs, NTBTHCTL0, 0);
> +	/* Interrupt when there is one entry in the queue */
> +	i3c_reg_write(i3c->regs, NRQTHCTL, 0);
> +
> +	/* Enable all Bus/Transfer Status Flags */
> +	i3c_reg_write(i3c->regs, BSTE, BSTE_ALL_FLAG);
> +	i3c_reg_write(i3c->regs, NTSTE, NTSTE_ALL_FLAG);
> +
> +	/* Interrupt enable settings */
> +	i3c_reg_write(i3c->regs, BIE, BIE_NACKDIE | BIE_TENDIE);
> +	i3c_reg_write(i3c->regs, NTIE, NTIE_RSQFIE |
> +					  NTIE_IBIQEFIE | NTIE_RDBFIE0);
> +
> +	/* Clear Status register */
> +	i3c_reg_write(i3c->regs, NTST, 0);
> +	i3c_reg_write(i3c->regs, INST, 0);
> +	i3c_reg_write(i3c->regs, BST, 0);
> +
> +	/* Hot-Join Acknowlege setting. */
> +	i3c_reg_update_bit(i3c->regs, BCTL, BCTL_HJACKCTL, BCTL_HJACKCTL);
> +
> +	i3c_reg_write(i3c->regs, IBINCTL, IBINCTL_NRHJCTL | IBINCTL_NRMRCTL |
> +					     IBINCTL_NRSIRCTL);
> +
> +	i3c_reg_write(i3c->regs, SCSTLCTL, 0);
> +	i3c_reg_set_bit(i3c->regs, SCSTRCTL, SCSTRCTL_ACKTWE);
> +
> +	/* Bus condition timing */
> +	val = DIV_ROUND_UP(I3C_BUS_FREE_TIME_NS, 1000000000 / rate);
> +	i3c_reg_write(i3c->regs, BFRECDT, BFRECDT_FRECYC(val));
> +
> +	val = DIV_ROUND_UP(I3C_BUS_AVAL_TIME_NS, 1000000000 / rate);
> +	i3c_reg_write(i3c->regs, BAVLCDT, BAVLCDT_AVLCYC(val));
> +
> +	val = DIV_ROUND_UP(I3C_BUS_IDLE_TIME_NS, 1000000000 / rate);
> +	i3c_reg_write(i3c->regs, BIDLCDT, BIDLCDT_IDLCYC(val));
> +
> +	ret = i3c_master_get_free_addr(m, 0);
> +	if (ret < 0)
> +		return ret;
> +
> +	i3c_reg_write(i3c->regs, MSDVAD, MSDVAD_MDYAD(ret) | MSDVAD_MDYADV);
> +
> +	memset(&info, 0, sizeof(info));
> +	info.dyn_addr = ret;
> +	return i3c_master_set_info(&i3c->base, &info);
> +}
> +
> +static void renesas_i3c_bus_cleanup(struct i3c_master_controller *m)
> +{
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	u32 val;
> +
> +	i3c_reg_write(i3c->regs, BCTL, 0);
> +	i3c_reg_update_bit(i3c->regs, RSTCTL, RSTCTL_RI3CRST, RSTCTL_RI3CRST);
> +	/* Wait for reset completion  */
> +	readl_relaxed_poll_timeout(i3c->regs + RSTCTL, val,
> +				   !(val & RSTCTL_RI3CRST), 0, 1000);
> +}
> +
> +static int renesas_i3c_daa(struct i3c_master_controller *m)
> +{
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	struct renesas_i3c_xfer *xfer;
> +	struct renesas_i3c_cmd *cmd;
> +	u32 olddevs, newdevs;
> +	u8 last_addr = 0, pos;
> +	int ret;
> +
> +	/* Enable I3C bus. */
> +	renesas_i3c_bus_enable(m, true);
> +
> +	olddevs = ~(i3c->free_pos);
> +	i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_ENTDAA;
> +
> +	/* Setting DATBASn registers for target devices. */
> +	for (pos = 0; pos < i3c->maxdevs; pos++) {
> +		if (olddevs & BIT(pos))
> +			continue;
> +
> +		ret = i3c_master_get_free_addr(m, last_addr + 1);
> +		if (ret < 0)
> +			return -ENOSPC;
> +
> +		i3c->addrs[pos] = ret;
> +		last_addr = ret;
> +
> +		i3c_reg_write(i3c->regs, DATBAS(pos), datbas_dvdyad_with_parity(ret));
> +	}
> +
> +	xfer = renesas_i3c_alloc_xfer(i3c, 1);
> +	if (!xfer)
> +		return -ENOMEM;
> +
> +	init_completion(&xfer->comp);
> +	cmd = xfer->cmds;
> +	cmd->rx_count = 0;
> +
> +	ret = renesas_i3c_get_free_pos(i3c);
> +	if (ret < 0)
> +		goto out;
> +
> +	/*
> +	 * Setup the command descriptor to start the ENTDAA command
> +	 * and starting at the selected device index.
> +	 */
> +	cmd->cmd0 = NCMDQP_CMD_ATTR(NCMDQP_ADDR_ASSGN) | NCMDQP_ROC |
> +		    NCMDQP_TID(I3C_COMMAND_ADDRESS_ASSIGNMENT) |
> +		    NCMDQP_CMD(I3C_CCC_ENTDAA) | NCMDQP_DEV_INDEX(ret) |
> +		    NCMDQP_DEV_COUNT(i3c->maxdevs - ret) | NCMDQP_TOC;
> +
> +	renesas_i3c_enqueue_xfer(i3c, xfer);
> +	if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000)))
> +		renesas_i3c_dequeue_xfer(i3c, xfer);
> +
> +	newdevs = GENMASK(i3c->maxdevs - cmd->rx_count - 1, 0);
> +	newdevs &= ~olddevs;
> +
> +	for (pos = 0; pos < i3c->maxdevs; pos++) {
> +		if (newdevs & BIT(pos))
> +			i3c_master_add_i3c_dev_locked(m, i3c->addrs[pos]);
> +	}
> +
> +out:
> +	kfree(xfer);

you can __free(kfree) xfer = NULL at declear to avoid this goto branch.

> +	return ret < 0 ? ret : 0;
> +}
> +
> +static bool renesas_i3c_supports_ccc_cmd(struct i3c_master_controller *m,
> +						const struct i3c_ccc_cmd *cmd)
> +{
> +	if (cmd->ndests > 1)
> +		return false;
> +
> +	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_ENTHDR(0):
> +	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_GETMXDS:
> +	case I3C_CCC_GETHDRCAP:
> +		return true;
> +	default:
> +		return false;
> +	}
> +}
> +
> +static int renesas_i3c_send_ccc_cmd(struct i3c_master_controller *m,
> +					   struct i3c_ccc_cmd *ccc)
> +{
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	struct renesas_i3c_xfer *xfer;
> +	struct renesas_i3c_cmd *cmd;
> +	int ret, pos = 0;
> +
> +	if (ccc->id & I3C_CCC_DIRECT) {
> +		pos = renesas_i3c_get_addr_pos(i3c, ccc->dests[0].addr);
> +		if (pos < 0)
> +			return pos;
> +	}
> +
> +	xfer = renesas_i3c_alloc_xfer(i3c, 1);
> +	if (!xfer)
> +		return -ENOMEM;
> +
> +	renesas_i3c_bus_enable(m, true);
> +
> +	init_completion(&xfer->comp);
> +	cmd = xfer->cmds;
> +	cmd->rnw = ccc->rnw;
> +	cmd->cmd0 = 0;
> +
> +	/* Calculate the command descriptor. */
> +	switch (ccc->id) {
> +	case I3C_CCC_SETDASA:
> +		i3c_reg_write(i3c->regs, DATBAS(pos),
> +			DATBAS_DVSTAD(ccc->dests[0].addr) |
> +			DATBAS_DVDYAD(*(u8 *)ccc->dests[0].payload.data >> 1));
> +		cmd->cmd0 = NCMDQP_CMD_ATTR(NCMDQP_ADDR_ASSGN) | NCMDQP_ROC |
> +			NCMDQP_TID(I3C_COMMAND_ADDRESS_ASSIGNMENT) |
> +			NCMDQP_CMD(I3C_CCC_SETDASA) | NCMDQP_DEV_INDEX(pos) |
> +			NCMDQP_DEV_COUNT(0) | NCMDQP_TOC;
> +		i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_SETDASA;
> +		break;
> +	default:
> +		/* Calculate the command descriptor. */
> +		cmd->cmd0 = NCMDQP_TID(I3C_COMMAND_WRITE) | NCMDQP_MODE(0) |
> +				NCMDQP_RNW(ccc->rnw) | NCMDQP_CMD(ccc->id) |
> +				NCMDQP_ROC | NCMDQP_TOC | NCMDQP_CP |
> +				NCMDQP_DEV_INDEX(pos);
> +
> +		if (ccc->rnw) {
> +			cmd->rx_buf = ccc->dests[0].payload.data;
> +			cmd->len = ccc->dests[0].payload.len;
> +			cmd->rx_count = 0;
> +			i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ;
> +		} else {
> +			cmd->tx_buf = ccc->dests[0].payload.data;
> +			cmd->len = ccc->dests[0].payload.len;
> +			cmd->tx_count = 0;
> +			i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE;
> +		}
> +	}
> +
> +	renesas_i3c_enqueue_xfer(i3c, xfer);
> +	if (!wait_for_completion_timeout(&xfer->comp, msecs_to_jiffies(1000)))
> +		renesas_i3c_dequeue_xfer(i3c, xfer);
> +
> +	ret = xfer->ret;
> +	if (ret)
> +		ccc->err = I3C_ERROR_M2;
> +
> +	kfree(xfer);
> +
> +	return ret;
> +}
> +
> +static int renesas_i3c_priv_xfers(struct i3c_dev_desc *dev, struct i3c_priv_xfer *i3c_xfers,
> +					 int i3c_nxfers)
> +{
> +	struct i3c_master_controller *m = i3c_dev_get_master(dev);
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	struct renesas_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
> +	struct renesas_i3c_xfer *xfer;
> +	int i;
> +
> +	/* Enable I3C bus. */
> +	renesas_i3c_bus_enable(m, true);
> +
> +	xfer = renesas_i3c_alloc_xfer(i3c, 1);
> +	if (!xfer)
> +		return -ENOMEM;
> +
> +	init_completion(&xfer->comp);
> +
> +	for (i = 0; i < i3c_nxfers; i++) {
> +		struct renesas_i3c_cmd *cmd = xfer->cmds;
> +
> +		/* Calculate the Transfer Command Descriptor */
> +		cmd->rnw = i3c_xfers[i].rnw;
> +		cmd->cmd0 = NCMDQP_DEV_INDEX(data->index) | NCMDQP_MODE(0) |
> +			    NCMDQP_RNW(cmd->rnw) | NCMDQP_ROC | NCMDQP_TOC;
> +
> +		if (i3c_xfers[i].rnw) {
> +			cmd->rx_count = 0;
> +			cmd->cmd0 |= NCMDQP_TID(I3C_READ);
> +			cmd->rx_buf = i3c_xfers[i].data.in;
> +			cmd->len = i3c_xfers[i].len;
> +			i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_READ;
> +		} else {
> +			cmd->tx_count = 0;
> +			cmd->cmd0 |= NCMDQP_TID(I3C_WRITE);
> +			cmd->tx_buf = i3c_xfers[i].data.out;
> +			cmd->len = i3c_xfers[i].len;
> +			i3c->internal_state = I3C_INTERNAL_STATE_CONTROLLER_WRITE;
> +		}
> +
> +		if (!i3c_xfers[i].rnw && i3c_xfers[i].len > 4) {

Only pre fill fifo when len >4? what happen if only write 1 byte?

> +			renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
> +			if (cmd->len > NTDTBP0_DEPTH * sizeof(u32))
> +				i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> +		}
> +
> +		renesas_i3c_enqueue_xfer(i3c, xfer);
> +		if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
> +			renesas_i3c_dequeue_xfer(i3c, xfer);

This may common problem, I3C spec have 100us timeout, target side may
timeout when driver wait for irq. So target side treat "repeat start" as
"start" and issue address arbitration.

> +	}
> +
> +	return 0;
> +}
> +
> +static int renesas_i3c_attach_i3c_dev(struct i3c_dev_desc *dev)
> +{
> +	struct i3c_master_controller *m = i3c_dev_get_master(dev);
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	struct renesas_i3c_i2c_dev_data *data;
> +	int pos;
> +
> +	pos = renesas_i3c_get_free_pos(i3c);
> +	if (pos < 0)
> +		return pos;
> +
> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->index = pos;
> +	i3c->addrs[pos] = dev->info.dyn_addr ? : dev->info.static_addr;

Are there extension of "?" operator in C ? I remember

dev->info.dyn_addr ? dev->info.dyn_addr : dev->info.static_addr;

> +	i3c->free_pos &= ~BIT(pos);
> +
> +	i3c_reg_write(i3c->regs, DATBAS(pos), DATBAS_DVSTAD(dev->info.static_addr) |
> +				    datbas_dvdyad_with_parity(i3c->addrs[pos]));
> +	i3c_dev_set_master_data(dev, data);
> +
> +	return 0;
> +}
> +
> +static int renesas_i3c_reattach_i3c_dev(struct i3c_dev_desc *dev,
> +					       u8 old_dyn_addr)
> +{
> +	struct i3c_master_controller *m = i3c_dev_get_master(dev);
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	struct renesas_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
> +
> +	i3c->addrs[data->index] = dev->info.dyn_addr ? dev->info.dyn_addr :
> +							dev->info.static_addr;
> +
> +	return 0;
> +}
> +
> +static void renesas_i3c_detach_i3c_dev(struct i3c_dev_desc *dev)
> +{
> +	struct renesas_i3c_i2c_dev_data *data = i3c_dev_get_master_data(dev);
> +	struct i3c_master_controller *m = i3c_dev_get_master(dev);
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +
> +	i3c_dev_set_master_data(dev, NULL);
> +	i3c->addrs[data->index] = 0;
> +	i3c->free_pos |= BIT(data->index);
> +	kfree(data);
> +}
> +
> +static int renesas_i3c_i2c_xfers(struct i2c_dev_desc *dev,
> +					struct i2c_msg *i2c_xfers,
> +					int i2c_nxfers)
> +{
> +	struct i3c_master_controller *m = i2c_dev_get_master(dev);
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	struct renesas_i3c_xfer *xfer;
> +	struct renesas_i3c_cmd *cmd;
> +	u8 start_bit = CNDCTL_STCND;
> +	int ret, i;
> +
> +	if (!i2c_nxfers)
> +		return 0;
> +
> +	renesas_i3c_bus_enable(m, false);
> +
> +	xfer = renesas_i3c_alloc_xfer(i3c, 1);
> +	if (!xfer)
> +		return -ENOMEM;
> +
> +	init_completion(&xfer->comp);
> +	xfer->is_i2c_xfer = true;
> +	cmd = xfer->cmds;
> +
> +	if (!(i3c_reg_read(i3c->regs, BCST) & BCST_BFREF)) {
> +		cmd->err = -EBUSY;
> +		goto out;
> +	}
> +
> +	i3c_reg_write(i3c->regs, BST, 0);
> +
> +	renesas_i3c_enqueue_xfer(i3c, xfer);
> +
> +	for (i = 0; i < i2c_nxfers; i++) {
> +		cmd->i2c_bytes_left = I2C_INIT_MSG;
> +		cmd->i2c_buf = i2c_xfers[i].buf;
> +		cmd->msg = &i2c_xfers[i];
> +		cmd->i2c_is_last = (i == i2c_nxfers - 1);
> +
> +		i3c_reg_set_bit(i3c->regs, BIE, BIE_NACKDIE);
> +		i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> +		i3c_reg_set_bit(i3c->regs, BIE, BIE_STCNDDIE);
> +
> +		/* Issue Start condition */
> +		i3c_reg_set_bit(i3c->regs, CNDCTL, start_bit);
> +
> +		i3c_reg_set_bit(i3c->regs, NTSTE, NTSTE_TDBEE0);
> +
> +		wait_for_completion_timeout(&xfer->comp, m->i2c.timeout);
> +
> +		if (cmd->err)
> +			break;
> +
> +		start_bit = CNDCTL_SRCND;
> +	}
> +out:
> +	renesas_i3c_dequeue_xfer(i3c, xfer);
> +	ret = cmd->err;
> +	kfree(xfer);
> +	return ret ?: 0;
> +}
> +
> +static int renesas_i3c_attach_i2c_dev(struct i2c_dev_desc *dev)
> +{
> +	struct i3c_master_controller *m = i2c_dev_get_master(dev);
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +	struct renesas_i3c_i2c_dev_data *data;
> +	int pos;
> +
> +	pos = renesas_i3c_get_free_pos(i3c);
> +	if (pos < 0)
> +		return pos;
> +
> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->index = pos;
> +	i3c->addrs[pos] = dev->addr;
> +	i3c->free_pos &= ~BIT(pos);
> +	i2c_dev_set_master_data(dev, data);
> +
> +	return 0;
> +}
> +
> +static void renesas_i3c_detach_i2c_dev(struct i2c_dev_desc *dev)
> +{
> +	struct renesas_i3c_i2c_dev_data *data = i2c_dev_get_master_data(dev);
> +	struct i3c_master_controller *m = i2c_dev_get_master(dev);
> +	struct renesas_i3c *i3c = to_renesas_i3c(m);
> +
> +	i2c_dev_set_master_data(dev, NULL);
> +	i3c->addrs[data->index] = 0;
> +	i3c->free_pos |= BIT(data->index);
> +	kfree(data);
> +}
> +
> +static irqreturn_t i3c_tx_isr(int irq, void *data)
> +{
> +	struct renesas_i3c *i3c = data;
> +	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
> +	struct renesas_i3c_cmd *cmd = xfer->cmds;
> +	u8 val;
> +
> +	if (xfer->is_i2c_xfer) {
> +		if (!cmd->i2c_bytes_left)
> +			return IRQ_NONE;
> +
> +		if (cmd->i2c_bytes_left != I2C_INIT_MSG) {
> +			val = *cmd->i2c_buf;
> +			cmd->i2c_buf++;
> +			cmd->i2c_bytes_left--;
> +			i3c_reg_write(i3c->regs, NTDTBP0, val);
> +		}
> +
> +		if (cmd->i2c_bytes_left == 0) {
> +			i3c_reg_clear_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> +			i3c_reg_set_bit(i3c->regs, BIE, BIE_TENDIE);
> +		}
> +
> +		/* Clear the Transmit Buffer Empty status flag. */
> +		i3c_reg_clear_bit(i3c->regs, NTST, NTST_TDBEF0);
> +	} else {
> +		renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t i3c_resp_isr(int irq, void *data)
> +{
> +	struct renesas_i3c *i3c = data;
> +	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
> +	struct renesas_i3c_cmd *cmd = xfer->cmds;
> +	u32 resp_descriptor = i3c_reg_read(i3c->regs, NRSPQP);
> +	u32 bytes_remaining = 0;
> +	u32 ntst, data_len;
> +	int ret = 0;
> +
> +	/* Clear the Respone Queue Full status flag*/
> +	i3c_reg_clear_bit(i3c->regs, NTST, NTST_RSPQFF);
> +
> +	data_len = NRSPQP_DATA_LEN(resp_descriptor);
> +
> +	switch (i3c->internal_state) {
> +	case I3C_INTERNAL_STATE_CONTROLLER_ENTDAA:
> +		cmd->rx_count = data_len;
> +		break;
> +	case I3C_INTERNAL_STATE_CONTROLLER_WRITE:
> +	case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_WRITE:
> +		/* Disable the transmit IRQ if it hasn't been disabled already. */
> +		i3c_reg_clear_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> +		break;
> +	case I3C_INTERNAL_STATE_CONTROLLER_READ:
> +	case I3C_INTERNAL_STATE_CONTROLLER_COMMAND_READ:
> +		if (NDBSTLV0_RDBLV(i3c_reg_read(i3c->regs, NDBSTLV0)) && !cmd->err)
> +			bytes_remaining = data_len - cmd->rx_count;
> +
> +		renesas_i3c_read_from_rx_fifo(i3c, cmd->rx_buf, bytes_remaining);
> +		i3c_reg_clear_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	switch (NRSPQP_ERR_STATUS(resp_descriptor)) {
> +	case NRSPQP_NO_ERROR:
> +		break;
> +	case NRSPQP_ERROR_PARITY:
> +	case NRSPQP_ERROR_IBA_NACK:
> +	case NRSPQP_ERROR_TRANSF_ABORT:
> +	case NRSPQP_ERROR_CRC:
> +	case NRSPQP_ERROR_FRAME:
> +		ret = -EIO;
> +		break;
> +	case NRSPQP_ERROR_OVER_UNDER_FLOW:
> +		ret = -ENOSPC;
> +		break;
> +	case NRSPQP_ERROR_I2C_W_NACK_ERR:
> +	case NRSPQP_ERROR_ADDRESS_NACK:
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	ntst = i3c_reg_read(i3c->regs, NTST);
> +
> +	/*
> +	 * If the transfer was aborted, then the abort flag must be cleared
> +	 * before notifying the application that a transfer has completed.
> +	 */
> +	if (ntst & NTST_TABTF)
> +		i3c_reg_clear_bit(i3c->regs, BCTL, BCTL_ABT);
> +
> +	/* Clear error status flags. */
> +	i3c_reg_clear_bit(i3c->regs, NTST, NTST_TEF | NTST_TABTF);
> +
> +	xfer->ret = ret;
> +	complete(&xfer->comp);
> +
> +	xfer = list_first_entry_or_null(&i3c->xferqueue.list,
> +					struct renesas_i3c_xfer, node);
> +	if (xfer)
> +		list_del_init(&xfer->node);
> +
> +	i3c->xferqueue.cur = xfer;
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t i3c_tend_isr(int irq, void *data)
> +{
> +	struct renesas_i3c *i3c = data;
> +	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
> +	struct renesas_i3c_cmd *cmd = xfer->cmds;
> +
> +	if (xfer->is_i2c_xfer) {
> +		if (i3c_reg_read(i3c->regs, BST) & BST_NACKDF) {
> +			/* We got a NACKIE */
> +			i3c_reg_read(i3c->regs, NTDTBP0); /* dummy read */
> +			i3c_reg_clear_bit(i3c->regs, BST, BST_NACKDF);
> +			cmd->err = -ENXIO;
> +		} else if (cmd->i2c_bytes_left) {
> +			i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> +			return IRQ_NONE;
> +		}
> +
> +		if (cmd->i2c_is_last || cmd->err) {
> +			i3c_reg_clear_bit(i3c->regs, BIE, BIE_TENDIE);
> +			i3c_reg_set_bit(i3c->regs, BIE, BIE_SPCNDDIE);
> +			i3c_reg_set_bit(i3c->regs, CNDCTL, CNDCTL_SPCND);
> +		} else {
> +			/* Transfer is complete, but do not send STOP */
> +			i3c_reg_clear_bit(i3c->regs, NTSTE, NTSTE_TDBEE0);
> +			i3c_reg_clear_bit(i3c->regs, BIE, BIE_TENDIE);
> +			xfer->ret = 0;
> +			complete(&xfer->comp);
> +		}
> +	}
> +
> +	/* Clear the Transmit Buffer Empty status flag. */
> +	i3c_reg_clear_bit(i3c->regs, BST, BST_TENDF);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t i3c_rx_isr(int irq, void *data)
> +{
> +	struct renesas_i3c *i3c = data;
> +	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
> +	struct renesas_i3c_cmd *cmd = xfer->cmds;
> +	int read_bytes;
> +
> +	if (xfer->is_i2c_xfer) {
> +		if (!cmd->i2c_bytes_left)
> +			return IRQ_NONE;
> +
> +		if (cmd->i2c_bytes_left == I2C_INIT_MSG) {
> +			cmd->i2c_bytes_left = cmd->msg->len;
> +			i3c_reg_set_bit(i3c->regs, SCSTRCTL, SCSTRCTL_RWE);
> +			i3c_reg_read(i3c->regs, NTDTBP0); /* dummy read */
> +			if (cmd->i2c_bytes_left == 1)
> +				i3c_reg_write(i3c->regs, ACKCTL, ACKCTL_ACKT | ACKCTL_ACKTWP);
> +			return IRQ_HANDLED;
> +		}
> +
> +		if (cmd->i2c_bytes_left == 1) {
> +			/* STOP must come before we set ACKCTL! */
> +			if (cmd->i2c_is_last) {
> +				i3c_reg_set_bit(i3c->regs, BIE, BIE_SPCNDDIE);
> +				i3c_reg_clear_bit(i3c->regs, BST, BST_SPCNDDF);
> +				i3c_reg_set_bit(i3c->regs, CNDCTL, CNDCTL_SPCND);
> +			}
> +			i3c_reg_write(i3c->regs, ACKCTL, ACKCTL_ACKT | ACKCTL_ACKTWP);
> +		} else {
> +			i3c_reg_write(i3c->regs, ACKCTL, ACKCTL_ACKTWP);
> +		}
> +
> +		/* Reading acks the RIE interrupt */
> +		*cmd->i2c_buf = i3c_reg_read(i3c->regs, NTDTBP0);
> +		cmd->i2c_buf++;
> +		cmd->i2c_bytes_left--;
> +	} else {
> +		read_bytes = NDBSTLV0_RDBLV(i3c_reg_read(i3c->regs, NDBSTLV0)) * sizeof(u32);
> +
> +		if (i3c->internal_state == I3C_INTERNAL_STATE_CONTROLLER_ENTDAA &&
> +							read_bytes == 8) {
> +			i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RSPQFIE);
> +			/* Read PID, BCR, DCR data */
> +			i3c_reg_read(i3c->regs, NTDTBP0);
> +			i3c_reg_read(i3c->regs, NTDTBP0);
> +			cmd->rx_count++;
> +		} else {
> +			renesas_i3c_read_from_rx_fifo(i3c, cmd->rx_buf, read_bytes);
> +			cmd->rx_count = read_bytes;
> +		}
> +	}
> +
> +	/* Clear the Read Buffer Full status flag. */
> +	i3c_reg_clear_bit(i3c->regs, NTST, NTST_RDBFF0);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t i3c_stop_isr(int irq, void *data)
> +{
> +	struct renesas_i3c *i3c = data;
> +	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
> +
> +	/* read back registers to confirm writes have fully propagated */
> +	i3c_reg_write(i3c->regs, BST, 0);
> +	i3c_reg_read(i3c->regs, BST);
> +	i3c_reg_write(i3c->regs, BIE, 0);
> +	i3c_reg_clear_bit(i3c->regs, NTST, NTST_TDBEF0 | NTST_RDBFF0);
> +	i3c_reg_clear_bit(i3c->regs, SCSTRCTL, SCSTRCTL_RWE);
> +
> +	xfer->ret = 0;
> +	complete(&xfer->comp);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static irqreturn_t i3c_start_isr(int irq, void *data)
> +{
> +	struct renesas_i3c *i3c = data;
> +	struct renesas_i3c_xfer *xfer = i3c->xferqueue.cur;
> +	struct renesas_i3c_cmd *cmd = xfer->cmds;
> +	u8 val;
> +
> +	if (xfer->is_i2c_xfer) {
> +		if (!cmd->i2c_bytes_left)
> +			return IRQ_NONE;
> +
> +		if (cmd->i2c_bytes_left == I2C_INIT_MSG) {
> +			if (cmd->msg->flags & I2C_M_RD) {
> +				/* On read, switch over to receive interrupt */
> +				i3c_reg_clear_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> +				i3c_reg_set_bit(i3c->regs, NTIE, NTIE_RDBFIE0);
> +			} else {
> +				/* On write, initialize length */
> +				cmd->i2c_bytes_left = cmd->msg->len;
> +			}
> +
> +			val = i2c_8bit_addr_from_msg(cmd->msg);
> +			i3c_reg_write(i3c->regs, NTDTBP0, val);
> +		}
> +	}
> +
> +	i3c_reg_clear_bit(i3c->regs, BIE, BIE_STCNDDIE);
> +	i3c_reg_clear_bit(i3c->regs, BST, BST_STCNDDF);
> +	return IRQ_HANDLED;
> +}
> +
> +static const struct i3c_master_controller_ops renesas_i3c_ops = {
> +	.bus_init = renesas_i3c_bus_init,
> +	.bus_cleanup = renesas_i3c_bus_cleanup,
> +	.attach_i3c_dev = renesas_i3c_attach_i3c_dev,
> +	.reattach_i3c_dev = renesas_i3c_reattach_i3c_dev,
> +	.detach_i3c_dev = renesas_i3c_detach_i3c_dev,
> +	.do_daa = renesas_i3c_daa,
> +	.supports_ccc_cmd = renesas_i3c_supports_ccc_cmd,
> +	.send_ccc_cmd = renesas_i3c_send_ccc_cmd,
> +	.priv_xfers = renesas_i3c_priv_xfers,
> +	.attach_i2c_dev = renesas_i3c_attach_i2c_dev,
> +	.detach_i2c_dev = renesas_i3c_detach_i2c_dev,
> +	.i2c_xfers = renesas_i3c_i2c_xfers,
> +};
> +
> +static struct renesas_i3c_irq_desc renesas_i3c_irqs[] = {
> +	{ .name = "resp", .isr = i3c_resp_isr, .desc = "i3c-resp" },
> +	{ .name = "rx", .isr = i3c_rx_isr, .desc = "i3c-rx" },
> +	{ .name = "tx", .isr = i3c_tx_isr, .desc = "i3c-tx" },
> +	{ .name = "st", .isr = i3c_start_isr, .desc = "i3c-start" },
> +	{ .name = "sp", .isr = i3c_stop_isr, .desc = "i3c-stop" },
> +	{ .name = "tend", .isr = i3c_tend_isr, .desc = "i3c-tend" },
> +	{ .name = "nack", .isr = i3c_tend_isr, .desc = "i3c-nack" },
> +};
> +
> +static int renesas_i3c_probe(struct platform_device *pdev)
> +{
> +	struct renesas_i3c *i3c;
> +	struct reset_control *reset;
> +	struct clk *clk;
> +	const struct renesas_i3c_config *config = of_device_get_match_data(&pdev->dev);
> +	int ret, i;
> +
> +	if (!config)
> +		return -ENODATA;
> +
> +	i3c = devm_kzalloc(&pdev->dev, sizeof(*i3c), GFP_KERNEL);
> +	if (!i3c)
> +		return -ENOMEM;
> +
> +	i3c->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(i3c->regs))
> +		return PTR_ERR(i3c->regs);
> +
> +	clk = devm_clk_get_enabled(&pdev->dev, "pclk");
> +	if (IS_ERR(clk))
> +		return PTR_ERR(clk);
> +
> +	if (config->has_pclkrw) {
> +		clk = devm_clk_get_enabled(&pdev->dev, "pclkrw");
> +		if (IS_ERR(clk))
> +			return PTR_ERR(clk);
> +	}
> +
> +	i3c->tclk = devm_clk_get_enabled(&pdev->dev, "tclk");
> +	if (IS_ERR(i3c->tclk))
> +		return PTR_ERR(i3c->tclk);
> +
> +	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "tresetn");
> +	if (IS_ERR(reset))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
> +				     "Error: missing tresetn ctrl\n");
> +
> +	reset = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev, "presetn");
> +	if (IS_ERR(reset))
> +		return dev_err_probe(&pdev->dev, PTR_ERR(reset),
> +				     "Error: missing presetn ctrl\n");
> +
> +	spin_lock_init(&i3c->xferqueue.lock);
> +	INIT_LIST_HEAD(&i3c->xferqueue.list);
> +
> +	ret = renesas_i3c_reset(i3c);
> +	if (ret)
> +		return ret;
> +
> +	for (i = 0; i < ARRAY_SIZE(renesas_i3c_irqs); i++) {
> +		ret = platform_get_irq_byname(pdev, renesas_i3c_irqs[i].name);
> +		if (ret < 0)
> +			return ret;
> +
> +		ret = devm_request_irq(&pdev->dev, ret, renesas_i3c_irqs[i].isr,
> +				       0, renesas_i3c_irqs[i].desc, i3c);
> +		if (ret) {
> +			dev_err(&pdev->dev, "failed to request irq %s\n",
> +				renesas_i3c_irqs[i].desc);
> +			return ret;

return dev_err_probe()

Frank
> +		}
> +	}
> +
> +	platform_set_drvdata(pdev, i3c);
> +
> +	i3c->maxdevs = RENESAS_I3C_MAX_DEVS;
> +	i3c->free_pos = GENMASK(i3c->maxdevs - 1, 0);
> +
> +	ret = i3c_master_register(&i3c->base, &pdev->dev,
> +				  &renesas_i3c_ops, false);
> +	if (ret)
> +		return ret;
> +
> +	dev_info(&pdev->dev, "register with i2c-scl = %ldHz i3c-scl = %ldHz\n",
> +		 i3c->base.bus.scl_rate.i2c, i3c->base.bus.scl_rate.i3c);
> +
> +	return 0;
> +}
> +
> +static void renesas_i3c_remove(struct platform_device *pdev)
> +{
> +	struct renesas_i3c *i3c = platform_get_drvdata(pdev);
> +
> +	i3c_master_unregister(&i3c->base);
> +}
> +
> +static const struct renesas_i3c_config empty_i3c_config = {
> +};
> +
> +static const struct renesas_i3c_config r9a09g047_i3c_config = {
> +	.has_pclkrw = 1,
> +};
> +
> +static const struct of_device_id renesas_i3c_of_ids[] = {
> +	{ .compatible = "renesas,r9a08g045-i3c", .data = &empty_i3c_config },
> +	{ .compatible = "renesas,r9a09g047-i3c", .data = &r9a09g047_i3c_config },
> +	{ .compatible = "renesas,i3c", .data = &empty_i3c_config },
> +	{ /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, renesas_i3c_of_ids);
> +
> +static struct platform_driver renesas_i3c = {
> +	.probe = renesas_i3c_probe,
> +	.remove = renesas_i3c_remove,
> +	.driver = {
> +		.name = "renesas-i3c",
> +		.of_match_table = renesas_i3c_of_ids,
> +	},
> +};
> +module_platform_driver(renesas_i3c);
> +
> +MODULE_AUTHOR("Wolfram Sang <wsa+renesas@sang-engineering.com>");
> +MODULE_AUTHOR("Renesas BSP teams");
> +MODULE_DESCRIPTION("Renesas I3C controller driver");
> +MODULE_LICENSE("GPL");
> --
> 2.47.2
>

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E
  2025-06-11 13:11 ` [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Rob Herring (Arm)
@ 2025-06-11 18:56   ` Wolfram Sang
  0 siblings, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-11 18:56 UTC (permalink / raw)
  To: Rob Herring (Arm)
  Cc: linux-clk, devicetree, Tommaso Merciai, Krzysztof Kozlowski,
	linux-hardening, linux-i3c, Alexandre Belloni, Philipp Zabel,
	Kees Cook, Magnus Damm, linux-renesas-soc, Gustavo A. R. Silva,
	Stephen Boyd, Frank Li, Geert Uytterhoeven, Conor Dooley,
	Michael Turquette

[-- Attachment #1: Type: text/plain, Size: 483 bytes --]


> New warnings running 'make CHECK_DTBS=y for arch/arm64/boot/dts/renesas/' for 20250611093934.4208-1-wsa+renesas@sang-engineering.com:
> 
> arch/arm64/boot/dts/renesas/r9a08g045s33-smarc.dtb: /soc/i3c@1005b000/temp@4a: failed to match any schema with compatible: ['adi,adt7411']

I sent a patch for 'trivial-devices' to fix this already [1]. I forgot
to add it to this series, sorry.

[1] https://lore.kernel.org/r/20250608162240.3023-2-wsa+renesas@sang-engineering.com


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-11 15:40   ` Frank Li
@ 2025-06-12 14:31     ` Wolfram Sang
  2025-06-12 14:51       ` Tommaso Merciai
  0 siblings, 1 reply; 35+ messages in thread
From: Wolfram Sang @ 2025-06-12 14:31 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, linux-i3c, devicetree

[-- Attachment #1: Type: text/plain, Size: 617 bytes --]

Hi Frank,

> > +  clock-names:
> > +    oneOf:
> > +      - items:
> > +          - const: pclk
> > +          - const: tclk
> > +      - items:
> > +          - const: pclk
> > +          - const: pclkrw
> > +          - const: tclk
> 
> Suggested clock name is, apb, trans, srfs.
> Needn't clk surfix and provide meanful name.

? Why can't we have the name from the datasheets? It will be
unnecessarily hard for users to verify the correctness of the clock
assignments if they need an artifical mapping of these names. If they
are the same as in the datasheet, the same task becomes very easy.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-12 14:31     ` Wolfram Sang
@ 2025-06-12 14:51       ` Tommaso Merciai
  2025-06-12 15:35         ` Frank Li
  2025-06-25 20:04         ` Rob Herring
  0 siblings, 2 replies; 35+ messages in thread
From: Tommaso Merciai @ 2025-06-12 14:51 UTC (permalink / raw)
  To: Wolfram Sang, Frank Li, linux-renesas-soc, Alexandre Belloni,
	Rob Herring, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, linux-i3c, devicetree

Hi Frank, Wolfram,

Thanks both for your work.

On 12/06/25 16:31, Wolfram Sang wrote:
> Hi Frank,
> 
>>> +  clock-names:
>>> +    oneOf:
>>> +      - items:
>>> +          - const: pclk
>>> +          - const: tclk
>>> +      - items:
>>> +          - const: pclk
>>> +          - const: pclkrw
>>> +          - const: tclk
>>
>> Suggested clock name is, apb, trans, srfs.
>> Needn't clk surfix and provide meanful name.
> 
> ? Why can't we have the name from the datasheets? It will be
> unnecessarily hard for users to verify the correctness of the clock
> assignments if they need an artifical mapping of these names. If they
> are the same as in the datasheet, the same task becomes very easy.
> 

Agreed.
Since the name comes from the hw manual I think is better to leave it as is.

Thanks & Regards,
Tommaso

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-11 16:37   ` Frank Li
@ 2025-06-12 14:55     ` Wolfram Sang
  2025-06-12 16:42       ` Frank Li
  2025-06-13 14:41       ` Frank Li
  2025-06-17  9:42     ` Wolfram Sang
  2025-07-24  8:58     ` Wolfram Sang
  2 siblings, 2 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-12 14:55 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening

[-- Attachment #1: Type: text/plain, Size: 3806 bytes --]

Hi Frank,

thanks again for the super-fast review.

> > - RENESAS_I3C_MAX_DEVS is a constant currently. So, we could get rid of
> >   the 'maxdevs' variable. I tend to keep it this way in case future
> >   controllers may use a different value, then we can change it easily.
> 
> It is similar ADI version. Can you simple descript hardware behavor to show
> why need RENESAS_I3C_MAX_DEVS,

Yes, for example this register has basic status info per target:

> > +#define DATBAS(x)		(0x224 + 0x8 * (x))

And there are only 8 of these registers. So, there is a maximum of 8 for
this controller. We could hardcode 8. But we could leave the handling as
is, just in case a future controller has more or less of these
registers.

> > - For accessing the FIFOs, this driver uses the same code as existing
> >   upstream drivers or the recenlty submitted "ADI" driver. There, the
> >   question came up, if this could be a helper function?
> 
> Add common header in drivers/i3c/master/core.h implement inline helper
> function

Sure thing. I think I didn't get feedback on my original suggestion so
far. If I now know you are positive about it, I will give it a try.

> #define PRTS                 0x00
> #define  PRTS_PRTMD          BIT(0)
> 
> Add extra space help distinguish register and field define.

Okay.

> 
> > +
> > +#define BCTL			0x14
> > +#define BCTL_HJACKCTL		BIT(8)
> > +#define BCTL_ABT		BIT(29)
> > +#define BCTL_BUSE		BIT(31)
> > +
> > +#define MSDVAD			0x18
> > +#define MSDVAD_MDYAD(x)		(((x) & 0x3f) << 16)
> 
> Use GEN_MASK

Sigh, if you think this is more readable, then OK.

> > +#define STDBR			0x74
> > +#define STDBR_SBRLO(cond, x)	((((x) >> (cond)) & 0xff) << 0)
> > +#define STDBR_SBRHO(cond, x)	((((x) >> (cond)) & 0xff) << 8)
> 
> FIELD_GET FIELD_PREP

Ditto.

> > +static inline void i3c_reg_update(u32 mask, u32 val, void __iomem *reg)
> > +{
> > +	u32 data = readl(reg);
> > +
> > +	data &= ~mask;
> > +	data |= (val & mask);
> > +	writel(data, reg);
> > +}
> 
> can you move reg to first argument to align below help function?

Yup, coccinelle should make that easy.

> > +out:
> > +	kfree(xfer);
> 
> you can __free(kfree) xfer = NULL at declear to avoid this goto branch.

I'll give it a try...

> > +		if (!i3c_xfers[i].rnw && i3c_xfers[i].len > 4) {
> 
> Only pre fill fifo when len >4? what happen if only write 1 byte?

Ehrm, good catch. I will check this in more detail.

> > +			renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
> > +			if (cmd->len > NTDTBP0_DEPTH * sizeof(u32))
> > +				i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> > +		}
> > +
> > +		renesas_i3c_enqueue_xfer(i3c, xfer);
> > +		if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
> > +			renesas_i3c_dequeue_xfer(i3c, xfer);
> 
> This may common problem, I3C spec have 100us timeout, target side may
> timeout when driver wait for irq. So target side treat "repeat start" as
> "start" and issue address arbitration.

There is a specified timeout? I couldn't find one in the specs, can you
kindly point me to it? So, the solution is to use 100us as timeout?

> > +	i3c->addrs[pos] = dev->info.dyn_addr ? : dev->info.static_addr;
> 
> Are there extension of "?" operator in C ? I remember
> 
> dev->info.dyn_addr ? dev->info.dyn_addr : dev->info.static_addr;

Dunno if it made it into the standard these days, but as a GCC extension
it is used in the kernel for ages. I encourage its use in I2C, other
maintainers don't like it much. Mileages vary.

> > +		if (ret) {
> > +			dev_err(&pdev->dev, "failed to request irq %s\n",
> > +				renesas_i3c_irqs[i].desc);
> > +			return ret;
> 
> return dev_err_probe()

OK.

Thanks and happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-12 14:51       ` Tommaso Merciai
@ 2025-06-12 15:35         ` Frank Li
  2025-06-25 20:04         ` Rob Herring
  1 sibling, 0 replies; 35+ messages in thread
From: Frank Li @ 2025-06-12 15:35 UTC (permalink / raw)
  To: Tommaso Merciai
  Cc: Wolfram Sang, linux-renesas-soc, Alexandre Belloni, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, devicetree

On Thu, Jun 12, 2025 at 04:51:42PM +0200, Tommaso Merciai wrote:
> Hi Frank, Wolfram,
>
> Thanks both for your work.
>
> On 12/06/25 16:31, Wolfram Sang wrote:
> > Hi Frank,
> >
> > > > +  clock-names:
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - const: pclk
> > > > +          - const: tclk
> > > > +      - items:
> > > > +          - const: pclk
> > > > +          - const: pclkrw
> > > > +          - const: tclk
> > >
> > > Suggested clock name is, apb, trans, srfs.
> > > Needn't clk surfix and provide meanful name.
> >
> > ? Why can't we have the name from the datasheets? It will be
> > unnecessarily hard for users to verify the correctness of the clock
> > assignments if they need an artifical mapping of these names. If they
> > are the same as in the datasheet, the same task becomes very easy.
> >
>
> Agreed.
> Since the name comes from the hw manual I think is better to leave it as is.

Previous Krzysztof give some feedback about clock-names, they don't perfer
clk surfix, because clock-names already provide information, which is clock.

Some hardware manual use it as signal name, for example, pclk which is apb
bus clock.

I am fine with align signal name. you can wait for DT team's feedback about
this.

Frank


>
> Thanks & Regards,
> Tommaso

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-12 14:55     ` Wolfram Sang
@ 2025-06-12 16:42       ` Frank Li
  2025-06-13  9:42         ` Wolfram Sang
  2025-06-13 14:41       ` Frank Li
  1 sibling, 1 reply; 35+ messages in thread
From: Frank Li @ 2025-06-12 16:42 UTC (permalink / raw)
  To: Wolfram Sang, linux-renesas-soc, Tommaso Merciai,
	Alexandre Belloni, Kees Cook, Gustavo A. R. Silva, Philipp Zabel,
	Geert Uytterhoeven, Magnus Damm, linux-i3c, linux-hardening

On Thu, Jun 12, 2025 at 04:55:31PM +0200, Wolfram Sang wrote:
> Hi Frank,
>
> thanks again for the super-fast review.
>
> > > - RENESAS_I3C_MAX_DEVS is a constant currently. So, we could get rid of
> > >   the 'maxdevs' variable. I tend to keep it this way in case future
> > >   controllers may use a different value, then we can change it easily.
> >
> > It is similar ADI version. Can you simple descript hardware behavor to show
> > why need RENESAS_I3C_MAX_DEVS,
>
> Yes, for example this register has basic status info per target:
>
> > > +#define DATBAS(x)		(0x224 + 0x8 * (x))
>
> And there are only 8 of these registers. So, there is a maximum of 8 for
> this controller. We could hardcode 8. But we could leave the handling as
> is, just in case a future controller has more or less of these
> registers.

Okay, can you point me spec link.

>
> > > - For accessing the FIFOs, this driver uses the same code as existing
> > >   upstream drivers or the recenlty submitted "ADI" driver. There, the
> > >   question came up, if this could be a helper function?
> >
> > Add common header in drivers/i3c/master/core.h implement inline helper
> > function
>
> Sure thing. I think I didn't get feedback on my original suggestion so
> far. If I now know you are positive about it, I will give it a try.

Sorry, linux-i3c mail list always delay your post, did you register linux-i3c
mail list.

>
> > #define PRTS                 0x00
> > #define  PRTS_PRTMD          BIT(0)
> >
> > Add extra space help distinguish register and field define.
>
> Okay.
>
> >
> > > +
> > > +#define BCTL			0x14
> > > +#define BCTL_HJACKCTL		BIT(8)
> > > +#define BCTL_ABT		BIT(29)
> > > +#define BCTL_BUSE		BIT(31)
> > > +
> > > +#define MSDVAD			0x18
> > > +#define MSDVAD_MDYAD(x)		(((x) & 0x3f) << 16)
> >
> > Use GEN_MASK
>
> Sigh, if you think this is more readable, then OK.
>
> > > +#define STDBR			0x74
> > > +#define STDBR_SBRLO(cond, x)	((((x) >> (cond)) & 0xff) << 0)
> > > +#define STDBR_SBRHO(cond, x)	((((x) >> (cond)) & 0xff) << 8)
> >
> > FIELD_GET FIELD_PREP
>
> Ditto.
>
> > > +static inline void i3c_reg_update(u32 mask, u32 val, void __iomem *reg)
> > > +{
> > > +	u32 data = readl(reg);
> > > +
> > > +	data &= ~mask;
> > > +	data |= (val & mask);
> > > +	writel(data, reg);
> > > +}
> >
> > can you move reg to first argument to align below help function?
>
> Yup, coccinelle should make that easy.
>
> > > +out:
> > > +	kfree(xfer);
> >
> > you can __free(kfree) xfer = NULL at declear to avoid this goto branch.
>
> I'll give it a try...
>
> > > +		if (!i3c_xfers[i].rnw && i3c_xfers[i].len > 4) {
> >
> > Only pre fill fifo when len >4? what happen if only write 1 byte?
>
> Ehrm, good catch. I will check this in more detail.
>
> > > +			renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
> > > +			if (cmd->len > NTDTBP0_DEPTH * sizeof(u32))
> > > +				i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> > > +		}
> > > +
> > > +		renesas_i3c_enqueue_xfer(i3c, xfer);
> > > +		if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
> > > +			renesas_i3c_dequeue_xfer(i3c, xfer);
> >
> > This may common problem, I3C spec have 100us timeout, target side may
> > timeout when driver wait for irq. So target side treat "repeat start" as
> > "start" and issue address arbitration.
>
> There is a specified timeout? I couldn't find one in the specs, can you
> kindly point me to it? So, the solution is to use 100us as timeout?

See: 5.1.2.5 Controller Clock Stalling

The spec have not defined what exactly happen if stall clock more than
100us.

I think it is hard to resolve this problem if hardware have not DMA queue.

Frank

>
> > > +	i3c->addrs[pos] = dev->info.dyn_addr ? : dev->info.static_addr;
> >
> > Are there extension of "?" operator in C ? I remember
> >
> > dev->info.dyn_addr ? dev->info.dyn_addr : dev->info.static_addr;
>
> Dunno if it made it into the standard these days, but as a GCC extension
> it is used in the kernel for ages. I encourage its use in I2C, other
> maintainers don't like it much. Mileages vary.
>
> > > +		if (ret) {
> > > +			dev_err(&pdev->dev, "failed to request irq %s\n",
> > > +				renesas_i3c_irqs[i].desc);
> > > +			return ret;
> >
> > return dev_err_probe()
>
> OK.
>
> Thanks and happy hacking,
>
>    Wolfram
>



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-12 16:42       ` Frank Li
@ 2025-06-13  9:42         ` Wolfram Sang
  2025-06-13 12:23           ` Geert Uytterhoeven
  2025-06-24 20:34           ` Wolfram Sang
  0 siblings, 2 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-13  9:42 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening

[-- Attachment #1: Type: text/plain, Size: 1531 bytes --]


> > And there are only 8 of these registers. So, there is a maximum of 8 for
> > this controller. We could hardcode 8. But we could leave the handling as
> > is, just in case a future controller has more or less of these
> > registers.
> 
> Okay, can you point me spec link.

Overview of the SoC:
https://www.renesas.com/en/products/microcontrollers-microprocessors/rz-mpus/rzg3s-general-purpose-microprocessors-single-core-arm-cortex-a55-11ghz-cpu-and-dual-core-cortex-m33-250mhz#documents

Datasheet:
https://www.renesas.com/en/document/mah/rzg3s-group-users-manual-hardware?r=25458591

> > Sure thing. I think I didn't get feedback on my original suggestion so
> > far. If I now know you are positive about it, I will give it a try.
> 
> Sorry, linux-i3c mail list always delay your post, did you register linux-i3c
> mail list.

I did subscribe. I receive mails and my patches using git-send-email go
through directly. My responses to mails are always held back just saying
"suspicious header". But I don't know what is "suspicious", so I can't
work on it.

> > There is a specified timeout? I couldn't find one in the specs, can you
> > kindly point me to it? So, the solution is to use 100us as timeout?
> 
> See: 5.1.2.5 Controller Clock Stalling

Ah, I missed this one so far. Thanks!

> The spec have not defined what exactly happen if stall clock more than
> 100us.

I am going to the I3C Plugfest in Warszaw in 10 days. I could ask people
there...

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-13  9:42         ` Wolfram Sang
@ 2025-06-13 12:23           ` Geert Uytterhoeven
  2025-06-13 13:10             ` Wolfram Sang
  2025-06-24 20:34           ` Wolfram Sang
  1 sibling, 1 reply; 35+ messages in thread
From: Geert Uytterhoeven @ 2025-06-13 12:23 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening

Hi Wolfram,

On Fri, 13 Jun 2025 at 13:41, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > Sorry, linux-i3c mail list always delay your post, did you register linux-i3c
> > mail list.
>
> I did subscribe. I receive mails and my patches using git-send-email go
> through directly. My responses to mails are always held back just saying
> "suspicious header". But I don't know what is "suspicious", so I can't
> work on it.

Perhaps your overly-long Mail-Followup-To:-header?
When replying to your emails, I always have to prune the To-recipients...

> I am going to the I3C Plugfest in Warszaw in 10 days. I could ask people
> there...

Amazing ;-)

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-13 12:23           ` Geert Uytterhoeven
@ 2025-06-13 13:10             ` Wolfram Sang
  2025-06-17  7:12               ` Geert Uytterhoeven
  0 siblings, 1 reply; 35+ messages in thread
From: Wolfram Sang @ 2025-06-13 13:10 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening

[-- Attachment #1: Type: text/plain, Size: 202 bytes --]


> Perhaps your overly-long Mail-Followup-To:-header?
> When replying to your emails, I always have to prune the To-recipients...

Let's try if this comes through. I told mutt to not add it anymore...


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-12 14:55     ` Wolfram Sang
  2025-06-12 16:42       ` Frank Li
@ 2025-06-13 14:41       ` Frank Li
  1 sibling, 0 replies; 35+ messages in thread
From: Frank Li @ 2025-06-13 14:41 UTC (permalink / raw)
  To: Wolfram Sang, linux-renesas-soc, Tommaso Merciai,
	Alexandre Belloni, Kees Cook, Gustavo A. R. Silva, Philipp Zabel,
	Geert Uytterhoeven, Magnus Damm, linux-i3c, linux-hardening

On Thu, Jun 12, 2025 at 04:55:31PM +0200, Wolfram Sang wrote:
> Hi Frank,
>
> thanks again for the super-fast review.
>
> > > - RENESAS_I3C_MAX_DEVS is a constant currently. So, we could get rid of
> > >   the 'maxdevs' variable. I tend to keep it this way in case future
> > >   controllers may use a different value, then we can change it easily.
> >
> > It is similar ADI version. Can you simple descript hardware behavor to show
> > why need RENESAS_I3C_MAX_DEVS,
>
> Yes, for example this register has basic status info per target:
>
> > > +#define DATBAS(x)		(0x224 + 0x8 * (x))
>
> And there are only 8 of these registers. So, there is a maximum of 8 for
> this controller. We could hardcode 8. But we could leave the handling as
> is, just in case a future controller has more or less of these
> registers.
>
> > > - For accessing the FIFOs, this driver uses the same code as existing
> > >   upstream drivers or the recenlty submitted "ADI" driver. There, the
> > >   question came up, if this could be a helper function?
> >
> > Add common header in drivers/i3c/master/core.h implement inline helper
> > function
>
> Sure thing. I think I didn't get feedback on my original suggestion so
> far. If I now know you are positive about it, I will give it a try.
>
> > #define PRTS                 0x00
> > #define  PRTS_PRTMD          BIT(0)
> >
> > Add extra space help distinguish register and field define.
>
> Okay.
>
> >
> > > +
> > > +#define BCTL			0x14
> > > +#define BCTL_HJACKCTL		BIT(8)
> > > +#define BCTL_ABT		BIT(29)
> > > +#define BCTL_BUSE		BIT(31)
> > > +
> > > +#define MSDVAD			0x18
> > > +#define MSDVAD_MDYAD(x)		(((x) & 0x3f) << 16)
> >
> > Use GEN_MASK
>
> Sigh, if you think this is more readable, then OK.
>
> > > +#define STDBR			0x74
> > > +#define STDBR_SBRLO(cond, x)	((((x) >> (cond)) & 0xff) << 0)
> > > +#define STDBR_SBRHO(cond, x)	((((x) >> (cond)) & 0xff) << 8)
> >
> > FIELD_GET FIELD_PREP
>
> Ditto.
>
> > > +static inline void i3c_reg_update(u32 mask, u32 val, void __iomem *reg)
> > > +{
> > > +	u32 data = readl(reg);
> > > +
> > > +	data &= ~mask;
> > > +	data |= (val & mask);
> > > +	writel(data, reg);
> > > +}
> >
> > can you move reg to first argument to align below help function?
>
> Yup, coccinelle should make that easy.
>
> > > +out:
> > > +	kfree(xfer);
> >
> > you can __free(kfree) xfer = NULL at declear to avoid this goto branch.
>
> I'll give it a try...
>
> > > +		if (!i3c_xfers[i].rnw && i3c_xfers[i].len > 4) {
> >
> > Only pre fill fifo when len >4? what happen if only write 1 byte?
>
> Ehrm, good catch. I will check this in more detail.
>
> > > +			renesas_i3c_write_to_tx_fifo(i3c, cmd->tx_buf, cmd->len);
> > > +			if (cmd->len > NTDTBP0_DEPTH * sizeof(u32))
> > > +				i3c_reg_set_bit(i3c->regs, NTIE, NTIE_TDBEIE0);
> > > +		}
> > > +
> > > +		renesas_i3c_enqueue_xfer(i3c, xfer);
> > > +		if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
> > > +			renesas_i3c_dequeue_xfer(i3c, xfer);
> >
> > This may common problem, I3C spec have 100us timeout, target side may
> > timeout when driver wait for irq. So target side treat "repeat start" as
> > "start" and issue address arbitration.
>
> There is a specified timeout? I couldn't find one in the specs, can you
> kindly point me to it? So, the solution is to use 100us as timeout?
>
> > > +	i3c->addrs[pos] = dev->info.dyn_addr ? : dev->info.static_addr;
> >
> > Are there extension of "?" operator in C ? I remember
> >
> > dev->info.dyn_addr ? dev->info.dyn_addr : dev->info.static_addr;
>
> Dunno if it made it into the standard these days, but as a GCC extension
> it is used in the kernel for ages. I encourage its use in I2C, other
> maintainers don't like it much. Mileages vary.

Okay, that's fine if it was already existed in kernel. I am just curious.

Frank

>
> > > +		if (ret) {
> > > +			dev_err(&pdev->dev, "failed to request irq %s\n",
> > > +				renesas_i3c_irqs[i].desc);
> > > +			return ret;
> >
> > return dev_err_probe()
>
> OK.
>
> Thanks and happy hacking,
>
>    Wolfram
>



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-13 13:10             ` Wolfram Sang
@ 2025-06-17  7:12               ` Geert Uytterhoeven
  2025-06-17 11:22                 ` Wolfram Sang
  0 siblings, 1 reply; 35+ messages in thread
From: Geert Uytterhoeven @ 2025-06-17  7:12 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Magnus Damm, linux-i3c,
	linux-hardening

Hi Wolfram,

On Fri, 13 Jun 2025 at 17:15, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> > Perhaps your overly-long Mail-Followup-To:-header?
> > When replying to your emails, I always have to prune the To-recipients...
>
> Let's try if this comes through. I told mutt to not add it anymore...

Thanks, much better!

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-11 16:37   ` Frank Li
  2025-06-12 14:55     ` Wolfram Sang
@ 2025-06-17  9:42     ` Wolfram Sang
  2025-07-24  8:58     ` Wolfram Sang
  2 siblings, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-17  9:42 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening

[-- Attachment #1: Type: text/plain, Size: 2898 bytes --]

Hi,

some status updates...

> It is similar ADI version. Can you simple descript hardware behavor to show
> why need RENESAS_I3C_MAX_DEVS,

I hope the documentation links I sent were helpful.

> Add common header in drivers/i3c/master/core.h implement inline helper
> function

I decided to handle this incrementally. I need to work on the real show
stoppers first, I'd say.

> #define PRTS                 0x00
> #define  PRTS_PRTMD          BIT(0)
> 
> Add extra space help distinguish register and field define.

Done.

> > +#define STDBR			0x74
> > +#define STDBR_SBRLO(cond, x)	((((x) >> (cond)) & 0xff) << 0)
> > +#define STDBR_SBRHO(cond, x)	((((x) >> (cond)) & 0xff) << 8)
> 
> FIELD_GET FIELD_PREP
> 
> check other define

Done. Also with GETMASK

> > +static inline void i3c_reg_update(u32 mask, u32 val, void __iomem *reg)
> > +{
> > +	u32 data = readl(reg);
> > +
> > +	data &= ~mask;
> > +	data |= (val & mask);
> > +	writel(data, reg);
> > +}
> 
> can you move reg to first argument to align below help function?

Done.

> > +out:
> > +	kfree(xfer);
> 
> you can __free(kfree) xfer = NULL at declear to avoid this goto branch.

Do you insist on this? It makes the driver less consistent because there
are other 'out'-branches with kfree() in them which we need to keep
anyhow, because there are more instructions needed. I can do the change
if you want but I personally would prefer to leave the code as is.

> > +		if (!i3c_xfers[i].rnw && i3c_xfers[i].len > 4) {
> 
> Only pre fill fifo when len >4? what happen if only write 1 byte?

That was a really good catch, thank you! I think the code is completely
redundant because we fill the FIFO again at a later time. Then, also
handling the case of < 4 bytes correctly. Sadly, I can't test this with
my current setup right now. I hope to have tested this by next week.

> > +		if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
> > +			renesas_i3c_dequeue_xfer(i3c, xfer);
> 
> This may common problem, I3C spec have 100us timeout, target side may
> timeout when driver wait for irq. So target side treat "repeat start" as
> "start" and issue address arbitration.

As said, I will try to discuss this at the I3C plugfest.

> > +	i3c->addrs[pos] = dev->info.dyn_addr ? : dev->info.static_addr;
> 
> Are there extension of "?" operator in C ? I remember

As agreed, I will keep this.

> > +		ret = devm_request_irq(&pdev->dev, ret, renesas_i3c_irqs[i].isr,
> > +				       0, renesas_i3c_irqs[i].desc, i3c);
> > +		if (ret) {
> > +			dev_err(&pdev->dev, "failed to request irq %s\n",
> > +				renesas_i3c_irqs[i].desc);
> > +			return ret;

I removed the printout comepletely. No need to do it for irqs.

I plan to resubmit the non-RFC version next week after the plugfest.
Fingers crossed.

Thanks and all the best,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-17  7:12               ` Geert Uytterhoeven
@ 2025-06-17 11:22                 ` Wolfram Sang
  2025-06-25 10:00                   ` Wolfram Sang
  0 siblings, 1 reply; 35+ messages in thread
From: Wolfram Sang @ 2025-06-17 11:22 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Magnus Damm, linux-i3c,
	linux-hardening

[-- Attachment #1: Type: text/plain, Size: 87 bytes --]


> Thanks, much better!

Sadly, my mails still get the "suspicious header" warning :(


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 1/7] clk: renesas: r9a08g045: Add I3C clocks, resets and power domain
  2025-06-11  9:39 ` [PATCH RFC 1/7] clk: renesas: r9a08g045: Add I3C clocks, resets and power domain Wolfram Sang
@ 2025-06-19 12:09   ` Geert Uytterhoeven
  0 siblings, 0 replies; 35+ messages in thread
From: Geert Uytterhoeven @ 2025-06-19 12:09 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Tommaso Merciai, Claudiu Beznea,
	Michael Turquette, Stephen Boyd, linux-clk

Hi Wolfram,

On Wed, 11 Jun 2025 at 11:39, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> Clocks extracted from the BSP driver and rebased. Power domain handling
> added by Claudiu.
>
> Co-developed-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@bp.renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Thanks for your patch!

> --- a/drivers/clk/renesas/r9a08g045-cpg.c
> +++ b/drivers/clk/renesas/r9a08g045-cpg.c

> @@ -243,6 +244,8 @@ static const struct rzg2l_mod_clk r9a08g045_mod_clks[] = {
>         DEF_MOD("adc_adclk",            R9A08G045_ADC_ADCLK, R9A08G045_CLK_TSU, 0x5a8, 0),
>         DEF_MOD("adc_pclk",             R9A08G045_ADC_PCLK, R9A08G045_CLK_TSU, 0x5a8, 1),
>         DEF_MOD("tsu_pclk",             R9A08G045_TSU_PCLK, R9A08G045_CLK_TSU, 0x5ac, 0),
> +       DEF_MOD("i3c_pclk",             R9A08G045_I3C_PCLK, R9A08G045_CLK_TSU, 0x610, 0),
> +       DEF_MOD("i3c_tclk",             R9A08G045_I3C_TCLK, R9A08G045_CLK_P5, 0x610, 1),
>         DEF_MOD("vbat_bclk",            R9A08G045_VBAT_BCLK, R9A08G045_OSCCLK, 0x614, 0),
>  };
>

> @@ -358,6 +363,8 @@ static const struct rzg2l_cpg_pm_domain_init_data r9a08g045_pm_domains[] = {
>                                 DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(14)), 0),
>         DEF_PD("tsu",           R9A08G045_PD_TSU,
>                                 DEF_REG_CONF(CPG_BUS_MCPU2_MSTOP, BIT(15)), 0),
> +       DEF_PD("i3c",           R9A08G045_PD_I3C,
> +                               DEF_REG_CONF(CPG_BUS_MCPU3_MSTOP, BIT(10)), 0),
>         DEF_PD("vbat",          R9A08G045_PD_VBAT,
>                                 DEF_REG_CONF(CPG_BUS_MCPU3_MSTOP, BIT(8)),
>                                 GENPD_FLAG_ALWAYS_ON),

r9a08g045_pm_domains[] is gone.
Please add "MSTOP(BUS_MCPU3, BIT(10))" to the "DEF_MOD("i3c_pclk", ...)"
and "DEF_MOD("i3c_tclk", ...)" entries above instead.

The rest LGTM.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 2/7] clk: renesas: r9a09g047: Add I3C0 clocks and resets
  2025-06-11  9:39 ` [PATCH RFC 2/7] clk: renesas: r9a09g047: Add I3C0 clocks and resets Wolfram Sang
@ 2025-06-19 12:16   ` Geert Uytterhoeven
  0 siblings, 0 replies; 35+ messages in thread
From: Geert Uytterhoeven @ 2025-06-19 12:16 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Tommaso Merciai, Michael Turquette,
	Stephen Boyd, linux-clk

On Wed, 11 Jun 2025 at 11:39, Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
>
> Add Renesas RZ/G3E R9A09G047 I3C0 clocks and reset support into
> cpg driver.
>
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk for v6.17.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-13  9:42         ` Wolfram Sang
  2025-06-13 12:23           ` Geert Uytterhoeven
@ 2025-06-24 20:34           ` Wolfram Sang
  1 sibling, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-24 20:34 UTC (permalink / raw)
  To: Frank Li, linux-renesas-soc, Tommaso Merciai, Alexandre Belloni,
	Kees Cook, Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening


> > > There is a specified timeout? I couldn't find one in the specs, can you
> > > kindly point me to it? So, the solution is to use 100us as timeout?
> > 
> > See: 5.1.2.5 Controller Clock Stalling
> 
> Ah, I missed this one so far. Thanks!
> 
> > The spec have not defined what exactly happen if stall clock more than
> > 100us.
> 
> I am going to the I3C Plugfest in Warszaw in 10 days. I could ask people
> there...

Well, I also couldn't get an answer here. It seems that part of the
spec has not been excercised a lot so far. The best recommendation was
also to use a timeout < 100us, so that you have a bit of time for
cancelling.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-17 11:22                 ` Wolfram Sang
@ 2025-06-25 10:00                   ` Wolfram Sang
  0 siblings, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-06-25 10:00 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Magnus Damm, linux-i3c,
	linux-hardening


> > Thanks, much better!
> 
> Sadly, my mails still get the "suspicious header" warning :(

Sigh, seems like infradead was not happy with me signing my emails. I
removed it for this server.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-12 14:51       ` Tommaso Merciai
  2025-06-12 15:35         ` Frank Li
@ 2025-06-25 20:04         ` Rob Herring
  2025-06-26  1:37           ` Frank Li
  1 sibling, 1 reply; 35+ messages in thread
From: Rob Herring @ 2025-06-25 20:04 UTC (permalink / raw)
  To: Tommaso Merciai
  Cc: Wolfram Sang, Frank Li, linux-renesas-soc, Alexandre Belloni,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, devicetree

On Thu, Jun 12, 2025 at 04:51:42PM +0200, Tommaso Merciai wrote:
> Hi Frank, Wolfram,
> 
> Thanks both for your work.
> 
> On 12/06/25 16:31, Wolfram Sang wrote:
> > Hi Frank,
> > 
> > > > +  clock-names:
> > > > +    oneOf:
> > > > +      - items:
> > > > +          - const: pclk
> > > > +          - const: tclk
> > > > +      - items:
> > > > +          - const: pclk
> > > > +          - const: pclkrw
> > > > +          - const: tclk
> > > 
> > > Suggested clock name is, apb, trans, srfs.

Suggested by who?

> > > Needn't clk surfix and provide meanful name.
> > 
> > ? Why can't we have the name from the datasheets? It will be
> > unnecessarily hard for users to verify the correctness of the clock
> > assignments if they need an artifical mapping of these names. If they
> > are the same as in the datasheet, the same task becomes very easy.
> > 
> 
> Agreed.
> Since the name comes from the hw manual I think is better to leave it as is.

Agreed.

While 'clk' is considered redundant, naming clocks 'p' and 't' is too 
terse even for me. So the rule is clk is redundant unless it is with a 
single letter. The rules apply to s/w folks making up names, not h/w 
manuals.

Rob

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-11  9:39 ` [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller Wolfram Sang
  2025-06-11 15:40   ` Frank Li
@ 2025-06-25 20:07   ` Rob Herring
  2025-06-30 19:43     ` Wolfram Sang
  1 sibling, 1 reply; 35+ messages in thread
From: Rob Herring @ 2025-06-25 20:07 UTC (permalink / raw)
  To: Wolfram Sang
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Frank Li,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, devicetree

On Wed, Jun 11, 2025 at 11:39:27AM +0200, Wolfram Sang wrote:
> From: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> 
> Available in RZ/G3S and G3E SoCs. The G3S has 17 interrupts, the G3E 16
> with the "HDR exit" interrupt missing. This interrupt is put to the end
> of the list, so we can handle the difference using "minItems" instead of
> defining a separate interrupt list per SoC.
> 
> Signed-off-by: Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> ---
> 
> Tommaso also did a version with two interrupt lists and using 'oneOf'. I
> prefer this version but maybe the other one is preferred upstream?
> 
>  .../devicetree/bindings/i3c/renesas,i3c.yaml  | 186 ++++++++++++++++++
>  1 file changed, 186 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
> 
> diff --git a/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml b/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
> new file mode 100644
> index 000000000000..e6e6c57bb56c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/i3c/renesas,i3c.yaml
> @@ -0,0 +1,186 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/i3c/renesas,i3c.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Renesas I3C Bus Interface
> +
> +maintainers:
> +  - Wolfram Sang <wsa+renesas@sang-engineering.com>
> +  - Tommaso Merciai <tommaso.merciai.xr@bp.renesas.com>
> +
> +properties:
> +  compatible:
> +    items:
> +      - enum:
> +          - renesas,r9a08g045-i3c # RZ/G3S
> +          - renesas,r9a09g047-i3c # RZ/G3E
> +      - const: renesas,i3c
> +
> +  reg:
> +    maxItems: 1
> +
> +  interrupts:
> +    items:
> +      - description: Non-recoverable internal error interrupt
> +      - description: Normal transfer error interrupt
> +      - description: Normal transfer abort interrupt
> +      - description: Normal response status buffer full interrupt
> +      - description: Normal command buffer empty interrupt
> +      - description: Normal IBI status buffer full interrupt
> +      - description: Normal Rx data buffer full interrupt
> +      - description: Normal Tx data buffer empty interrupt
> +      - description: Normal receive status buffer full interrupt
> +      - description: START condition detection interrupt
> +      - description: STOP condition detection interrupt
> +      - description: Transmit end interrupt
> +      - description: NACK detection interrupt
> +      - description: Arbitration lost interrupt
> +      - description: Timeout detection interrupt
> +      - description: Wake-up condition detection interrupt
> +      - description: HDR Exit Pattern detection interrupt
> +    minItems: 16
> +
> +  interrupt-names:
> +    items:
> +      - const: ierr
> +      - const: terr
> +      - const: abort
> +      - const: resp
> +      - const: cmd
> +      - const: ibi
> +      - const: rx
> +      - const: tx
> +      - const: rcv
> +      - const: st
> +      - const: sp
> +      - const: tend
> +      - const: nack
> +      - const: al
> +      - const: tmo
> +      - const: wu
> +      - const: exit
> +    minItems: 16
> +
> +  clocks:
> +    oneOf:
> +      - items:
> +          - description: APB bus clock
> +          - description: transfer clock
> +      - items:
> +          - description: APB bus clock
> +          - description: SFRs clock
> +          - description: transfer clock

It's a new binding, why don't you put SFRs clock last and simplify the 
schema?

> +
> +  clock-names:
> +    oneOf:
> +      - items:
> +          - const: pclk
> +          - const: tclk
> +      - items:
> +          - const: pclk
> +          - const: pclkrw
> +          - const: tclk

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-25 20:04         ` Rob Herring
@ 2025-06-26  1:37           ` Frank Li
  0 siblings, 0 replies; 35+ messages in thread
From: Frank Li @ 2025-06-26  1:37 UTC (permalink / raw)
  To: Rob Herring
  Cc: Tommaso Merciai, Wolfram Sang, linux-renesas-soc,
	Alexandre Belloni, Krzysztof Kozlowski, Conor Dooley,
	Geert Uytterhoeven, Magnus Damm, linux-i3c, devicetree

On Wed, Jun 25, 2025 at 03:04:51PM -0500, Rob Herring wrote:
> On Thu, Jun 12, 2025 at 04:51:42PM +0200, Tommaso Merciai wrote:
> > Hi Frank, Wolfram,
> >
> > Thanks both for your work.
> >
> > On 12/06/25 16:31, Wolfram Sang wrote:
> > > Hi Frank,
> > >
> > > > > +  clock-names:
> > > > > +    oneOf:
> > > > > +      - items:
> > > > > +          - const: pclk
> > > > > +          - const: tclk
> > > > > +      - items:
> > > > > +          - const: pclk
> > > > > +          - const: pclkrw
> > > > > +          - const: tclk
> > > >
> > > > Suggested clock name is, apb, trans, srfs.
>
> Suggested by who?

It is my suggestion base on
 - 1: needn't clk surfix. But you provide rule below.
 - 2: pclk, tclk ... can't provide any meansful information to user.
It is hard to know what's that without check binding doc. And most case
there are not description under clocks.

but some common clock-names term for IP, like apb, bus, which will be easy
to know apb (control access register). 'bus' feed into i3c bus clock speed.
'bus' is important for i3c to set 12Mhz or lower freqeuncy for old i2c
devices.

Frank


>
> > > > Needn't clk surfix and provide meanful name.
> > >
> > > ? Why can't we have the name from the datasheets? It will be
> > > unnecessarily hard for users to verify the correctness of the clock
> > > assignments if they need an artifical mapping of these names. If they
> > > are the same as in the datasheet, the same task becomes very easy.
> > >
> >
> > Agreed.
> > Since the name comes from the hw manual I think is better to leave it as is.
>
> Agreed.
>
> While 'clk' is considered redundant, naming clocks 'p' and 't' is too
> terse even for me. So the rule is clk is redundant unless it is with a
> single letter. The rules apply to s/w folks making up names, not h/w
> manuals.
>
> Rob
>
> --
> linux-i3c mailing list
> linux-i3c@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-25 20:07   ` Rob Herring
@ 2025-06-30 19:43     ` Wolfram Sang
  2025-07-01  9:09       ` Tommaso Merciai
  0 siblings, 1 reply; 35+ messages in thread
From: Wolfram Sang @ 2025-06-30 19:43 UTC (permalink / raw)
  To: Rob Herring
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Frank Li,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, devicetree


> > +  interrupt-names:
> > +    items:
> > +      - const: ierr
> > +      - const: terr
> > +      - const: abort
> > +      - const: resp
> > +      - const: cmd
> > +      - const: ibi
> > +      - const: rx
> > +      - const: tx
> > +      - const: rcv
> > +      - const: st
> > +      - const: sp
> > +      - const: tend
> > +      - const: nack
> > +      - const: al
> > +      - const: tmo
> > +      - const: wu
> > +      - const: exit
> > +    minItems: 16
> > +
> > +  clocks:
> > +    oneOf:
> > +      - items:
> > +          - description: APB bus clock
> > +          - description: transfer clock
> > +      - items:
> > +          - description: APB bus clock
> > +          - description: SFRs clock
> > +          - description: transfer clock
> 
> It's a new binding, why don't you put SFRs clock last and simplify the 
> schema?

Similar how we handle interrupts? 16 for the one SoC, 17 for the other?
Putting the optional entry to the end of the array? Tommaso, can you
integrate this change, please?


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-06-30 19:43     ` Wolfram Sang
@ 2025-07-01  9:09       ` Tommaso Merciai
  2025-07-03  7:18         ` Wolfram Sang
  0 siblings, 1 reply; 35+ messages in thread
From: Tommaso Merciai @ 2025-07-01  9:09 UTC (permalink / raw)
  To: Wolfram Sang, Rob Herring
  Cc: linux-renesas-soc, Alexandre Belloni, Frank Li,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, devicetree

Hi Wolfram, Rob,

On 30/06/25 21:43, Wolfram Sang wrote:
> 
>>> +  interrupt-names:
>>> +    items:
>>> +      - const: ierr
>>> +      - const: terr
>>> +      - const: abort
>>> +      - const: resp
>>> +      - const: cmd
>>> +      - const: ibi
>>> +      - const: rx
>>> +      - const: tx
>>> +      - const: rcv
>>> +      - const: st
>>> +      - const: sp
>>> +      - const: tend
>>> +      - const: nack
>>> +      - const: al
>>> +      - const: tmo
>>> +      - const: wu
>>> +      - const: exit
>>> +    minItems: 16
>>> +
>>> +  clocks:
>>> +    oneOf:
>>> +      - items:
>>> +          - description: APB bus clock
>>> +          - description: transfer clock
>>> +      - items:
>>> +          - description: APB bus clock
>>> +          - description: SFRs clock
>>> +          - description: transfer clock
>>
>> It's a new binding, why don't you put SFRs clock last and simplify the
>> schema?
> 
> Similar how we handle interrupts? 16 for the one SoC, 17 for the other?
> Putting the optional entry to the end of the array? Tommaso, can you
> integrate this change, please?
> 

   clocks:
     items:
       - description: APB bus clock
       - description: transfer clock
       - description: SFRs clock
     minItems: 2

   clock-names:
     items:
       - const: pclk
       - const: tclk
       - const: pclkrw
     minItems: 2

...

allOf:
   - $ref: i3c.yaml#

   - if:
       properties:
         compatible:
           contains:
             const: renesas,r9a08g045-i3c
     then:
       properties:
         clocks:
           maxItems: 2
         clock-names:
           maxItems: 2
         interrupts:
           minItems: 17
         interrupt-names:
           minItems: 17

   - if:
       properties:
         compatible:
           contains:
             const: renesas,r9a09g047-i3c
     then:
       properties:
         clocks:
           minItems: 3
         clock-names:
           minItems: 3
         interrupts:
           maxItems: 16
         interrupt-names:
           maxItems: 16

Right?
Please correct me if I'm missing somenthing.
Thanks in advance.

Thanks & Regards,
Tommaso





^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller
  2025-07-01  9:09       ` Tommaso Merciai
@ 2025-07-03  7:18         ` Wolfram Sang
  0 siblings, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-07-03  7:18 UTC (permalink / raw)
  To: Tommaso Merciai
  Cc: Rob Herring, linux-renesas-soc, Alexandre Belloni, Frank Li,
	Krzysztof Kozlowski, Conor Dooley, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, devicetree


...

>   - if:
>       properties:
>         compatible:
>           contains:
>             const: renesas,r9a09g047-i3c
>     then:
>       properties:
>         clocks:
>           minItems: 3
>         clock-names:
>           minItems: 3
>         interrupts:
>           maxItems: 16
>         interrupt-names:
>           maxItems: 16
> 
> Right?
> Please correct me if I'm missing somenthing.

LGTM. I will send it out like this in the v1 version.


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP
  2025-06-11 16:37   ` Frank Li
  2025-06-12 14:55     ` Wolfram Sang
  2025-06-17  9:42     ` Wolfram Sang
@ 2025-07-24  8:58     ` Wolfram Sang
  2 siblings, 0 replies; 35+ messages in thread
From: Wolfram Sang @ 2025-07-24  8:58 UTC (permalink / raw)
  To: Frank Li
  Cc: linux-renesas-soc, Tommaso Merciai, Alexandre Belloni, Kees Cook,
	Gustavo A. R. Silva, Philipp Zabel, Geert Uytterhoeven,
	Magnus Damm, linux-i3c, linux-hardening


> > +		renesas_i3c_enqueue_xfer(i3c, xfer);
> > +		if (!wait_for_completion_timeout(&xfer->comp, XFER_TIMEOUT))
> > +			renesas_i3c_dequeue_xfer(i3c, xfer);
> 
> This may common problem, I3C spec have 100us timeout, target side may
> timeout when driver wait for irq. So target side treat "repeat start" as
> "start" and issue address arbitration.

Looking more at this issue, I think these are two separate problems.
Chapter 5.1.2.5 mandates that the controller shall not hold SCL low for
more than 100us in most situations and by no means longer than 15ms in
any situation.

Depending on the controller, this may be need to be handled in software.
Or it may be handled in hardware which could raise an exception and
release SCL on its own if the timing was violated.

The above completion is for the *whole transfer*, though, including the
target response time. Like I2C, it is not specified for I3C. At least, I
couldn't find it and I recall no one at the I3C plugfest could point me
to one as well.

So, this completion timeout is more than just 5.1.2.5. It might make
sense to investigate a more reasonable value. But I don't think this
should be imposed on someone submitting a new driver. It is a dedicated
task. And I am not even sure the result will be a subsystem-wide static
value. It might be a calculated value. Maybe even a driver-specific
calculated value.

So, I think the best we can do until we have this investigation is to
keep drivers consistent with the historically grown value.

Or?


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2025-07-24  8:58 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11  9:39 [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Wolfram Sang
2025-06-11  9:39 ` [PATCH RFC 1/7] clk: renesas: r9a08g045: Add I3C clocks, resets and power domain Wolfram Sang
2025-06-19 12:09   ` Geert Uytterhoeven
2025-06-11  9:39 ` [PATCH RFC 2/7] clk: renesas: r9a09g047: Add I3C0 clocks and resets Wolfram Sang
2025-06-19 12:16   ` Geert Uytterhoeven
2025-06-11  9:39 ` [PATCH RFC 3/7] dt-bindings: i3c: renesas,i3c: Add binding for Renesas I3C controller Wolfram Sang
2025-06-11 15:40   ` Frank Li
2025-06-12 14:31     ` Wolfram Sang
2025-06-12 14:51       ` Tommaso Merciai
2025-06-12 15:35         ` Frank Li
2025-06-25 20:04         ` Rob Herring
2025-06-26  1:37           ` Frank Li
2025-06-25 20:07   ` Rob Herring
2025-06-30 19:43     ` Wolfram Sang
2025-07-01  9:09       ` Tommaso Merciai
2025-07-03  7:18         ` Wolfram Sang
2025-06-11  9:39 ` [PATCH RFC 4/7] i3c: add driver for Renesas I3C IP Wolfram Sang
2025-06-11 16:37   ` Frank Li
2025-06-12 14:55     ` Wolfram Sang
2025-06-12 16:42       ` Frank Li
2025-06-13  9:42         ` Wolfram Sang
2025-06-13 12:23           ` Geert Uytterhoeven
2025-06-13 13:10             ` Wolfram Sang
2025-06-17  7:12               ` Geert Uytterhoeven
2025-06-17 11:22                 ` Wolfram Sang
2025-06-25 10:00                   ` Wolfram Sang
2025-06-24 20:34           ` Wolfram Sang
2025-06-13 14:41       ` Frank Li
2025-06-17  9:42     ` Wolfram Sang
2025-07-24  8:58     ` Wolfram Sang
2025-06-11  9:39 ` [PATCH RFC 5/7] arm64: dts: renesas: r9a08g045: Add I3C node Wolfram Sang
2025-06-11  9:39 ` [PATCH RFC 6/7] arm64: dts: renesas: r9a09g047: " Wolfram Sang
2025-06-11  9:39 ` [PATCH RFC 7/7] WIP: arm64: dts: renesas: rzg3s-smarc-som: Enable I3C Wolfram Sang
2025-06-11 13:11 ` [PATCH RFC 0/7] i3c: add driver for the Renesas IP and support RZ/G3S+G3E Rob Herring (Arm)
2025-06-11 18:56   ` Wolfram Sang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).