i3c: dw: Add apb reset support

PATCH Superseded Jisheng Zhang 5 participants 12 messages May 11 – May 14

Subsystems (3)

Device tree bindings I3C I3C driver for synopsys DesignWare

lore

Share & tools

lore

Jisheng Zhang

author

2026-05-11 03:38 UTC #0 lore raw

v2 cover i3c: dw: Add apb reset support

Add support of apb reset which is to reset the APB interface. The first patch is to document the exisiting reset dt-binding. 2nd patch is to add apb reset dt-binding. The last patch is to add apb reset support.

Hi Frank,

comments to question "why not name the reset as "apb" instead of "apb_rst":
exisiting core reset is named as "core_rst", this is to align with its style.

Thanks

Since v1:
- add dt-binding

Jisheng Zhang (3):
dt-bindings: i3c: dw: Describe core reset
dt-bindings: i3c: dw: Add apb reset
i3c: dw: Add apb reset support

 .../devicetree/bindings/i3c/snps,dw-i3c-master.yaml    | 10 ++++++++++
 drivers/i3c/master/dw-i3c-master.c                     |  7 +++++++
 drivers/i3c/master/dw-i3c-master.h                     |  1 +
 3 files changed, 18 insertions(+)

Signature · 1 line

2.53.0

Jisheng Zhang

author

2026-05-11 03:38 UTC #1 lore raw

PATCH v2 1/3 dt-bindings: i3c: dw: Describe core reset

The core reset support has been in the code from day1, but the dt-binding doesn't exist. Add dt-binding to describe reset property.

Signed-off-by Jisheng Zhang <[email protected]>

---
 .../devicetree/bindings/i3c/snps,dw-i3c-master.yaml       | 8 ++++++++
 1 file changed, 8 insertions(+)

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +8 -0

index e803457d3f55..613dce7757bc 100644
@@ -35,6 +35,14 @@ properties:
       - const: core
       - const: apb
 
+  resets:
+    items:
+      - description: Reset signal
+
+  reset-names:
+    items:
+      - const: core_rst
+
   interrupts:
     maxItems: 1

Signature · 1 line

2.53.0

Frank Li

reviewer

2026-05-11 19:27 UTC #2 lore raw

Re: [PATCH v2 1/3] dt-bindings: i3c: dw: Describe core reset

Quoted 14 lines · Jisheng Zhang

The core reset support has been in the code from day1, but the dt-binding doesn't exist. Add dt-binding to describe reset property.
Signed-off-by Jisheng Zhang <[email protected]>
---
 .../devicetree/bindings/i3c/snps,dw-i3c-master.yaml       | 8 ++++++++
 1 file changed, 8 insertions(+)
Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +0 -0
index e803457d3f55..613dce7757bc 100644
@@ -35,6 +35,14 @@ properties:
       - const: core

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml properties: · line 45

Quoted 10 lines

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +8 -0

       - const: apb
 
+  resets:
+    items:
+      - description: Reset signal
+
+  reset-names:
+    items:
+      - const: core_rst
+

Remove "_rst" suffix, it is totally reduncted.

Frank

Quoted 6 lines

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +0 -1
   interrupts:
     maxItems: 1
 
--
2.53.0

Jisheng Zhang

author

2026-05-11 03:38 UTC #3 lore raw

PATCH v2 2/3 dt-bindings: i3c: dw: Add apb reset

In reply to Jisheng Zhang

Add dt-binding for support of apb reset which is to reset the APB interface.

Signed-off-by Jisheng Zhang <[email protected]>

---
 Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml | 2 ++
 1 file changed, 2 insertions(+)

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +2 -0

index 613dce7757bc..2575442b28ff 100644
@@ -38,10 +38,12 @@ properties:
   resets:
     items:
       - description: Reset signal
+      - description: APB interface reset signal
 
   reset-names:
     items:
       - const: core_rst
+      - const: apb_rst
 
   interrupts:
     maxItems: 1

Signature · 1 line

2.53.0

Conor Dooley

2026-05-11 16:02 UTC #4 lore raw

Re: [PATCH v2 2/3] dt-bindings: i3c: dw: Add apb reset

Quoted 4 lines · Jisheng Zhang

Add dt-binding for support of apb reset which is to reset the APB interface.
Signed-off-by Jisheng Zhang <[email protected]>

Please squash both dt-binding patches.

Quoted 9 lines

---
 Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml | 2 ++
 1 file changed, 2 insertions(+)
Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +0 -0
index 613dce7757bc..2575442b28ff 100644

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml properties: · line 46

Quoted 9 lines

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +2 -0

@@ -38,10 +38,12 @@ properties:
   resets:
     items:
       - description: Reset signal
+      - description: APB interface reset signal
 
   reset-names:
     items:
       - const: core_rst
+      - const: apb_rst

Drop the _rst here please, not as if these can be anything other than a reset!

pw-bot: changes-requested

Cheers,
Conor.

Quoted 6 lines

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +0 -0
   interrupts:
     maxItems: 1
Signature · 1 line
2.53.0

Krzysztof Kozlowski

2026-05-14 12:17 UTC #5 lore raw

Re: [PATCH v2 2/3] dt-bindings: i3c: dw: Add apb reset

Quoted 7 lines · Conor Dooley

Quoted 4 lines · Jisheng Zhang
Add dt-binding for support of apb reset which is to reset the APB interface.
Signed-off-by Jisheng Zhang <[email protected]>
Please squash both dt-binding patches.

I think this should stay separate, because first commit is trying to fix undocumented existing ABI. It will have different rationale and could be chosen for backports.

Best regards,
Krzysztof

Conor Dooley

2026-05-14 16:22 UTC #6 lore raw

Re: [PATCH v2 2/3] dt-bindings: i3c: dw: Add apb reset

Quoted 5 lines · Krzysztof Kozlowski

Quoted 4 lines · Conor Dooley
Quoted 3 lines · Jisheng Zhang
Add dt-binding for support of apb reset which is to reset the APB interface.

Quoted 7 lines

Quoted 3 lines
Quoted 1 line
Signed-off-by Jisheng Zhang <[email protected]>
Please squash both dt-binding patches.
I think this should stay separate, because first commit is trying to fix undocumented existing ABI. It will have different rationale and could be chosen for backports.

Ah, I guess I didn't read the commit message. Sorry about that Jisheng!

[email protected]

AI review

2026-05-11 23:00 UTC #7 lore raw

Re: [PATCH v2 2/3] dt-bindings: i3c: dw: Add apb reset

In reply to Jisheng Zhang

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Adding a second item to the `resets` and `reset-names` list without specifying `minItems: 1`…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Adding a second item to the resets and reset-names list without specifying minItems: 1 makes both resets strictly required whenever the property is present, breaking DT schema backward compatibility. --

commit 0c4b11b6e45b719eb15a1a87171391a4bfb310b9
Author: Jisheng Zhang <[email protected]>

dt-bindings: i3c: dw: Add apb reset

This commit updates the DesignWare I3C master binding to support an optional APB interface reset signal.

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml properties: · line 41

Quoted 9 lines

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +1 -0

index 613dce7757bc3..2575442b28ff4 100644
@@ -38,10 +38,12 @@ properties:
   resets:
     items:
       - description: Reset signal
+      - description: APB interface reset signal

Does adding this second item inadvertently make the APB reset strictly mandatory for validation? In dt-schema, when items are defined as a list, minItems implicitly defaults to the length of that list. Without an explicit minItems: 1 constraint, device trees defining only the single core_rst reset might fail schema validation.

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml properties: · line 46

Quoted 5 lines

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +1 -0
   reset-names:
     items:
       - const: core_rst
+      - const: apb_rst

Should minItems: 1 be added to both the resets and reset-names properties to maintain backward compatibility?

Since the driver gracefully handles a missing apb_rst by using devm_reset_control_get_optional_exclusive_deasserted(), adding the minItems constraint would keep single-reset configurations perfectly valid.

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2

Conor Dooley

2026-05-12 16:44 UTC #8 lore raw

Re: [PATCH v2 2/3] dt-bindings: i3c: dw: Add apb reset

Quoted 34 lines · On Mon, May 11, 2026 at 11:00:00PM +0000, [email protected] wrote:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Adding a second item to the resets and reset-names list without specifying minItems: 1 makes both resets strictly required whenever the property is present, breaking DT schema backward compatibility. --
commit 0c4b11b6e45b719eb15a1a87171391a4bfb310b9
Author: Jisheng Zhang <[email protected]>
dt-bindings: i3c: dw: Add apb reset
This commit updates the DesignWare I3C master binding to support an optional APB interface reset signal.
Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml properties: · line 41
Quoted 9 lines
Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +1 -0
index 613dce7757bc3..2575442b28ff4 100644
@@ -38,10 +38,12 @@ properties:
   resets:
     items:
       - description: Reset signal
+      - description: APB interface reset signal
Does adding this second item inadvertently make the APB reset strictly mandatory for validation? In dt-schema, when items are defined as a list, minItems implicitly defaults to the length of that list. Without an explicit minItems: 1 constraint, device trees defining only the single core_rst reset might fail schema validation.
Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml properties: · line 46
Quoted 5 lines
Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +1 -0
   reset-names:
     items:
       - const: core_rst
+      - const: apb_rst

Quoted 6 lines

Should minItems: 1 be added to both the resets and reset-names properties to maintain backward compatibility?
Since the driver gracefully handles a missing apb_rst by using devm_reset_control_get_optional_exclusive_deasserted(), adding the minItems constraint would keep single-reset configurations perfectly valid.

Obviously this is all crap and can be ignored, since the other reset is added in the series.

Jisheng Zhang

author

2026-05-11 03:38 UTC #9 lore raw

PATCH v2 3/3 i3c: dw: Add apb reset support

In reply to Jisheng Zhang

Add support of apb reset which is to reset the APB interface.

Signed-off-by Jisheng Zhang <[email protected]>

---
 drivers/i3c/master/dw-i3c-master.c | 7 +++++++
 drivers/i3c/master/dw-i3c-master.h | 1 +
 2 files changed, 8 insertions(+)

drivers/i3c/master/dw-i3c-master.c +7 -0

index 655693a2187e..9de54d584bc3 100644
@@ -1591,6 +1591,11 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 	if (IS_ERR(master->core_rst))
 		return PTR_ERR(master->core_rst);
 
+	master->apb_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
+									       "apb_rst");
+	if (IS_ERR(master->apb_rst))
+		return PTR_ERR(master->apb_rst);
+
 	spin_lock_init(&master->xferqueue.lock);
 	INIT_LIST_HEAD(&master->xferqueue.list);

@@ -1765,6 +1770,7 @@ static int __maybe_unused dw_i3c_master_runtime_suspend(struct device *dev)
 	dw_i3c_master_disable(master);
 
 	reset_control_assert(master->core_rst);
+	reset_control_assert(master->apb_rst);
 	dw_i3c_master_disable_clks(master);
 	pinctrl_pm_select_sleep_state(dev);
 	return 0;

@@ -1777,6 +1783,7 @@ static int __maybe_unused dw_i3c_master_runtime_resume(struct device *dev)
 	pinctrl_pm_select_default_state(dev);
 	dw_i3c_master_enable_clks(master);
 	reset_control_deassert(master->core_rst);
+	reset_control_deassert(master->apb_rst);
 
 	dw_i3c_master_set_intr_regs(master);
 	dw_i3c_master_restore_timing_regs(master);

drivers/i3c/master/dw-i3c-master.h +1 -0

index c5cb695c16ab..a4ba60043288 100644
@@ -37,6 +37,7 @@ struct dw_i3c_master {
 	struct dw_i3c_master_caps caps;
 	void __iomem *regs;
 	struct reset_control *core_rst;
+	struct reset_control *apb_rst;
 	struct clk *core_clk;
 	struct clk *pclk;
 	char version[5];

Signature · 1 line

2.53.0

[email protected]

AI review

2026-05-11 23:13 UTC #10 lore raw

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] The assertion and deassertion ordering of `core_rst` and `apb_rst` violates correct bus-peripheral…

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The assertion and deassertion ordering of core_rst and apb_rst violates correct bus-peripheral hardware sequencing and LIFO resource management symmetry. --

commit 9be063129d3014c8590f046b253447e4090e7f37
Author: Jisheng Zhang <[email protected]>

i3c: dw: Add apb reset support

This commit adds support for the optional APB reset in the DesignWare I3C master driver. The reset is acquired during probe and asserted or deasserted during runtime PM suspend and resume cycles.

drivers/i3c/master/dw-i3c-master.c int dw_i3c_common_probe(struct dw_i3c_master *master, · line 1603

Quoted 12 lines

drivers/i3c/master/dw-i3c-master.c +5 -0

@@ -1596,6 +1596,11 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 	if (IS_ERR(master->core_rst))
 		return PTR_ERR(master->core_rst);
 
+	master->apb_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
+									       "apb_rst");
+	if (IS_ERR(master->apb_rst))
+		return PTR_ERR(master->apb_rst);
+

Should apb_rst be requested before core_rst?

Since devm handles cleanup in reverse allocation order, requesting core_rst first means apb_rst will be asserted before core_rst during driver unload or probe failure. This would shut down the bus interface while the IP core is still active, contradicting the safe shutdown sequence correctly implemented in dw_i3c_master_runtime_suspend().

Additionally, since devm_reset_control_get_optional_exclusive_deasserted() automatically deasserts the reset, this ordering brings the IP core out of reset before the APB bus interface is active.

[ ... ]

drivers/i3c/master/dw-i3c-master.c static int __maybe_unused dw_i3c_master_runtime_resume(struct device *dev) · line 1805

Quoted 5 lines

drivers/i3c/master/dw-i3c-master.c +1 -0

@@ -1796,6 +1802,7 @@ static int __maybe_unused dw_i3c_master_runtime_resume(struct device *dev)
 	pinctrl_pm_select_default_state(dev);
 	dw_i3c_master_enable_clks(master);
 	reset_control_deassert(master->core_rst);
+	reset_control_deassert(master->apb_rst);

Is this deassertion order correct?

Deasserting core_rst before apb_rst brings the IP core out of reset before its APB bus interface is active. Could this cause the IP to run while disconnected from the bus, leading to unpredictable states?

Would it be safer to deassert apb_rst before core_rst?

Signature · 1 line

Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3

Frank Li

reviewer

2026-05-11 19:04 UTC #11 lore raw

In reply to Jisheng Zhang

Quoted 11 lines

Add support of apb reset which is to reset the APB interface. The first patch is to document the exisiting reset dt-binding. 2nd patch is to add apb reset dt-binding. The last patch is to add apb reset support.
Hi Frank,
comments to question "why not name the reset as "apb" instead of "apb_rst":
exisiting core reset is named as "core_rst", this is to align with its style.

You add new bind at path 1. It is new reset name. All needn't "_rst" suffix.

Frank

Quoted 19 lines

Thanks
Since v1:
- add dt-binding
Jisheng Zhang (3):
dt-bindings: i3c: dw: Describe core reset
dt-bindings: i3c: dw: Add apb reset
i3c: dw: Add apb reset support
 .../devicetree/bindings/i3c/snps,dw-i3c-master.yaml    | 10 ++++++++++
 drivers/i3c/master/dw-i3c-master.c                     |  7 +++++++
 drivers/i3c/master/dw-i3c-master.h                     |  1 +
 3 files changed, 18 insertions(+)
--
2.53.0