[PATCH v3 0/4] i3c: dw: Add apb reset support

[PATCH v3 0/4] i3c: dw: Add apb reset support

[Jisheng Zhang]

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

-- 
2.53.0

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

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

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/i3c/master/dw-i3c-master.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index 655693a2187e..c4a848cc978a 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -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);
 
-- 
2.53.0

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

[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 <jszhang@kernel.org>
---
 .../devicetree/bindings/i3c/snps,dw-i3c-master.yaml       | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
index e803457d3f55..519797c6b4fe 100644
--- a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
+++ b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
@@ -35,6 +35,14 @@ properties:
       - const: core
       - const: apb
 
+  resets:
+    items:
+      - description: Reset signal
+
+  reset-names:
+    items:
+      - const: core
+
   interrupts:
     maxItems: 1
 
-- 
2.53.0

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

[Jisheng Zhang]

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

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
index 519797c6b4fe..12845206772f 100644
--- a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
+++ b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
@@ -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
-- 
2.53.0

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

[sashiko-bot]

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 <jszhang@kernel.org>

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.

> diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> index 519797c6b4fe4..12845206772f7 100644
> --- a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> +++ b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> @@ -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?

>  
>    interrupts:
>      maxItems: 1

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519055105.13079-1-jszhang@kernel.org?part=3

[PATCH v3 4/4] i3c: dw: Add apb reset support

[Jisheng Zhang]

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

Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
 drivers/i3c/master/dw-i3c-master.c | 7 +++++++
 drivers/i3c/master/dw-i3c-master.h | 1 +
 2 files changed, 8 insertions(+)

diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
index c4a848cc978a..ae7aa6880e8f 100644
--- a/drivers/i3c/master/dw-i3c-master.c
+++ b/drivers/i3c/master/dw-i3c-master.c
@@ -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);
diff --git a/drivers/i3c/master/dw-i3c-master.h b/drivers/i3c/master/dw-i3c-master.h
index c5cb695c16ab..a4ba60043288 100644
--- a/drivers/i3c/master/dw-i3c-master.h
+++ b/drivers/i3c/master/dw-i3c-master.h
@@ -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];
-- 
2.53.0

Re: [PATCH v3 4/4] i3c: dw: Add apb reset support

[sashiko-bot]

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 <jszhang@kernel.org>

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.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index 6286dc5d09568..75608fbc3cbd1 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
> @@ -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.

[ ... ]

> @@ -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?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260519055105.13079-1-jszhang@kernel.org?part=4