[PATCH v2 0/3] i3c: dw: Add apb reset support

[PATCH v2 0/3] 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.

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

-- 
2.53.0

[PATCH v2 1/3] 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..613dce7757bc 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_rst
+
   interrupts:
     maxItems: 1
 
-- 
2.53.0

[PATCH v2 2/3] 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 613dce7757bc..2575442b28ff 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_rst
+      - const: apb_rst
 
   interrupts:
     maxItems: 1
-- 
2.53.0

[PATCH v2 3/3] 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 655693a2187e..9de54d584bc3 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_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);
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 v2 2/3] dt-bindings: i3c: dw: Add apb reset

[Conor Dooley]

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

On Mon, May 11, 2026 at 11:19:44AM +0800, Jisheng Zhang wrote:
> Add dt-binding for support of apb reset which is to reset the APB
> interface.
> 
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>

Please squash both dt-binding patches.

> ---
>  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 613dce7757bc..2575442b28ff 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_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.

>  
>    interrupts:
>      maxItems: 1
> -- 
> 2.53.0
> 

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

RE: [EXT] [PATCH v2 0/3] i3c: dw: Add apb reset support

[Frank Li]

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

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

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

[Frank Li]

On Mon, May 11, 2026 at 11:19:43AM +0800, Jisheng Zhang wrote:
> 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..613dce7757bc 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_rst
> +

Remove "_rst" suffix, it is totally reduncted.

Frank

>    interrupts:
>      maxItems: 1
>
> --
> 2.53.0
>

Re: [PATCH v2 2/3] 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 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 <jszhang@kernel.org>

dt-bindings: i3c: dw: Add apb reset

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

> diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> index 613dce7757bc3..2575442b28ff4 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

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.

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

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

Re: [PATCH v2 3/3] i3c: dw: Add apb reset support

[sashiko-bot]

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

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.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> --- 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_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.

[ ... ]

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

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

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

[Conor Dooley]

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

On Mon, May 11, 2026 at 11:00:00PM +0000, sashiko-bot@kernel.org 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 <jszhang@kernel.org>
> 
> dt-bindings: i3c: dw: Add apb reset
> 
> This commit updates the DesignWare I3C master binding to support an
> optional APB interface reset signal.
> 
> > diff --git a/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml b/Documentation/devicetree/bindings/i3c/snps,dw-i3c-master.yaml
> > index 613dce7757bc3..2575442b28ff4 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
> 
> 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.
> 
> >  
> >    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.

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

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