i3c: dw: Add apb reset support

PATCH Superseded Jisheng Zhang 3 participants 12 messages May 19 – May 24

Subsystems (3)

Device tree bindings I3C I3C driver for synopsys DesignWare

lore

Share & tools

lore

Jisheng Zhang

author

2026-05-19 06:10 UTC #0 lore raw

v3 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.

Since v2:
- remove "_rst" suffix

Since v1:
- add dt-binding

Jisheng Zhang (4):
i3c: dw: Remove core reset "_rst" suffix
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                     |  9 ++++++++-
 drivers/i3c/master/dw-i3c-master.h                     |  1 +
 3 files changed, 19 insertions(+), 1 deletion(-)

Signature · 1 line

2.53.0

Jisheng Zhang

author

2026-05-19 06:10 UTC #1 lore raw

PATCH v3 1/4 i3c: dw: Remove core reset "_rst" suffix

It's redundant. This suffix has been in the code from day1, fortunately there's no such dt property usage in all dw i3c users after grepping all dts files, so we can remove it.

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

---
 drivers/i3c/master/dw-i3c-master.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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

index 655693a2187e..c4a848cc978a 100644
@@ -1587,7 +1587,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 		return PTR_ERR(master->pclk);
 
 	master->core_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
-										"core_rst");
+										"core");
 	if (IS_ERR(master->core_rst))
 		return PTR_ERR(master->core_rst);

Signature · 1 line

2.53.0

Krzysztof Kozlowski

2026-05-20 07:14 UTC #2 lore raw

Re: [PATCH v3 1/4] i3c: dw: Remove core reset "_rst" suffix

Quoted 3 lines · Jisheng Zhang

It's redundant. This suffix has been in the code from day1, fortunately there's no such dt property usage in all dw i3c users after grepping all dts files, so we can remove it.

Hm, how could you grep all 3rd party / out of tree users of this?

Quoted 11 lines

Signed-off-by Jisheng Zhang <[email protected]>
---
 drivers/i3c/master/dw-i3c-master.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i3c/master/dw-i3c-master.c +0 -0
index 655693a2187e..c4a848cc978a 100644

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

Quoted 5 lines

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

@@ -1587,7 +1587,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 		return PTR_ERR(master->pclk);
 
 	master->core_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
-										"core_rst");
+										"core");

ABI impact for something released since 2018. Cleanup of name is not really worth affecting users. core_rst is not the best name but it is not incorrect, either.

Best regards,
Krzysztof

Jisheng Zhang

author

2026-05-20 12:02 UTC #3 lore raw

Re: [PATCH v3 1/4] i3c: dw: Remove core reset "_rst" suffix

Quoted 20 lines · Krzysztof Kozlowski

Quoted 3 lines · Jisheng Zhang
It's redundant. This suffix has been in the code from day1, fortunately there's no such dt property usage in all dw i3c users after grepping all dts files, so we can remove it.
Hm, how could you grep all 3rd party / out of tree users of this?
Quoted 13 lines
Signed-off-by Jisheng Zhang <[email protected]>
---
 drivers/i3c/master/dw-i3c-master.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i3c/master/dw-i3c-master.c +0 -0
index 655693a2187e..c4a848cc978a 100644
@@ -1587,7 +1587,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 		return PTR_ERR(master->pclk);
 

Quoted 8 lines

drivers/i3c/master/dw-i3c-master.c int dw_i3c_common_probe(struct dw_i3c_master *master, · line 1590
Quoted 3 lines
drivers/i3c/master/dw-i3c-master.c +1 -1
 	master->core_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
-										"core_rst");
+										"core");
ABI impact for something released since 2018. Cleanup of name is not really worth affecting users. core_rst is not the best name but it is not incorrect, either.

Hmm make sense.
Two questions: if the ABI is introduced but never used by any intree users, is modifying the ABI taken as "ABI breakage"?

if the ABI is only used by outtree users, can we modify the ABI?

Thanks

Krzysztof Kozlowski

2026-05-24 18:41 UTC #4 lore raw

Re: [PATCH v3 1/4] i3c: dw: Remove core reset "_rst" suffix

Quoted 29 lines · Jisheng Zhang

Quoted 20 lines · Krzysztof Kozlowski
Quoted 3 lines · Jisheng Zhang
It's redundant. This suffix has been in the code from day1, fortunately there's no such dt property usage in all dw i3c users after grepping all dts files, so we can remove it.
Hm, how could you grep all 3rd party / out of tree users of this?
Quoted 13 lines
Signed-off-by Jisheng Zhang <[email protected]>
---
 drivers/i3c/master/dw-i3c-master.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/i3c/master/dw-i3c-master.c +0 -0
index 655693a2187e..c4a848cc978a 100644
@@ -1587,7 +1587,7 @@ int dw_i3c_common_probe(struct dw_i3c_master *master,
 		return PTR_ERR(master->pclk);
 
Quoted 8 lines
drivers/i3c/master/dw-i3c-master.c int dw_i3c_common_probe(struct dw_i3c_master *master, · line 1590
Quoted 3 lines
drivers/i3c/master/dw-i3c-master.c +1 -1
 	master->core_rst = devm_reset_control_get_optional_exclusive_deasserted(&pdev->dev,
-										"core_rst");
+										"core");
ABI impact for something released since 2018. Cleanup of name is not really worth affecting users. core_rst is not the best name but it is not incorrect, either.

Quoted 3 lines

Hmm make sense.
Two questions: if the ABI is introduced but never used by any intree users, is modifying the ABI taken as "ABI breakage"?

Yes. The out of tree user was working before. Now it is not working. How is it not a "breakage"?

Quoted 2 lines

if the ABI is only used by outtree users, can we modify the ABI?

Yes and no. Depends. It's up to platform maintainer and you need reasons. The reasons MUST be documented. See writing-bindings. I do not see here a good reason.

Best regards,
Krzysztof

Jisheng Zhang

author

2026-05-19 06:10 UTC #5 lore raw

PATCH v3 2/4 dt-bindings: i3c: dw: Describe core reset

In reply to 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 +8 -0

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

Signature · 1 line

2.53.0

Krzysztof Kozlowski

2026-05-20 07:14 UTC #6 lore raw

Re: [PATCH v3 2/4] dt-bindings: i3c: dw: Describe core reset

Quoted 1 line · Jisheng Zhang

The core reset support has been in the code from day1, but the

Well, no. There was no "core" reset from day1, so your entire explanation makes no sense now. Previous version was correct.

Best regards,
Krzysztof

Jisheng Zhang

author

2026-05-20 12:09 UTC #7 lore raw

Re: [PATCH v3 2/4] dt-bindings: i3c: dw: Describe core reset

Quoted 4 lines · Krzysztof Kozlowski

Quoted 1 line · Jisheng Zhang
The core reset support has been in the code from day1, but the
Well, no. There was no "core" reset from day1, so your entire

aha, by "core reset" I mean the reset signal for i3c core. Not the reset name. But I will update the commit msg.

Quoted 2 lines

explanation makes no sense now. Previous version was correct.

OK, so I will send v4.

Jisheng Zhang

author

2026-05-19 06:10 UTC #8 lore raw

PATCH v3 3/4 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 519797c6b4fe..12845206772f 100644
@@ -38,10 +38,12 @@ properties:
   resets:
     items:
       - description: Reset signal
+      - description: APB interface reset signal
 
   reset-names:
     items:
       - const: core
+      - const: apb
 
   interrupts:
     maxItems: 1

Signature · 1 line

2.53.0

[email protected]

AI review

2026-05-19 06:48 UTC #9 lore raw

Re: [PATCH v3 3/4] dt-bindings: i3c: dw: Add apb reset

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Adding a second item to `resets` and `reset-names` 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 resets and reset-names without specifying minItems: 1 inadvertently makes the APB reset mandatory, which breaks schema validation for valid hardware configurations and older device trees. --

commit f868716b21aa0a0740d04da18bdfc0d7495473fb
Author: Jisheng Zhang <[email protected]>

dt-bindings: i3c: dw: Add apb reset

This commit adds a dt-binding for the APB reset signal, which is used to reset the APB interface.

Quoted 5 lines

Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml +0 -0
index 519797c6b4fe4..12845206772f7 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
+      - const: apb

Does adding a second item without specifying minItems: 1 inadvertently make the APB reset mandatory?

In dt-schema, when an items list is provided, the implicit minItems and maxItems constraints default to the length of the list, which is now 2. Any existing device tree or hardware configuration that specifies only the core reset will fail dtbs_check validation.

The driver patch in the series retrieves the APB reset using devm_reset_control_get_optional_exclusive_deasserted(), which suggests that this reset is intended to be optional.

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

Quoted 3 lines

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

Signature · 1 line

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

Jisheng Zhang

author

2026-05-19 06:10 UTC #10 lore raw

PATCH v3 4/4 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 c4a848cc978a..ae7aa6880e8f 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");
+	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-19 06:58 UTC #11 lore raw

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Incorrect and asymmetric hardware reset sequencing for the APB bus and core. --

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect and asymmetric hardware reset sequencing for the APB bus and core. --

commit 1f3afa3e9cfb230aef26a85d4d3a519a377255b1
Author: Jisheng Zhang <[email protected]>

i3c: dw: Add apb reset support

This commit adds support for an APB interface reset control to the DesignWare I3C master driver. The reset is obtained during probe and managed across runtime suspend and resume cycles.

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

Quoted 10 lines

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

index 6286dc5d09568..75608fbc3cbd1 100644
@@ -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");

Does allocating apb_rst after core_rst cause a devm cleanup ordering issue?

Because devm executes cleanups in reverse allocation order, apb_rst will be asserted before core_rst during driver unbind or probe failure. This stops the bus while the core is still running, which contradicts the assertion order in dw_i3c_master_runtime_suspend().

Also, because devm_reset_control_get_optional_exclusive_deasserted() immediately deasserts the reset, core_rst is deasserted before apb_rst during probe. This brings the core out of reset while its bus interface is still held in reset.

[ ... ]

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);

Does this sequence fail to correctly reverse the suspend order?

In dw_i3c_master_runtime_suspend(), core_rst is asserted followed by apb_rst, which correctly stops the core before the bus.

However, here core_rst is deasserted before apb_rst. This means the core is brought out of reset while its APB bus interface is still held in reset, which could trigger bus faults.

Should apb_rst be deasserted before core_rst to ensure the bus is available before the core IP starts operating?

Signature · 1 line

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