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

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

NOTE: to align with "core_rst", the new added apb reset is named as
"apb_rst", IOW, the reset names: "core_rst" + "apb_rst". I can modify
it if maintainers prefer "core_rst" + "apb"

Since v3:
  - back to v2, I.E keep the "_rst" suffix, since removing it is an
    ABI breakage.

Since v2:
  - remove "_rst" suffix

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 v4 1/3] dt-bindings: i3c: dw: Describe core reset

[Jisheng Zhang]

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

Then why not remove the "_rst" suffix? Krzysztof pointed out "Cleanup
of name is not really worth affecting users. core_rst is not the best
name but it is not incorrect, either."

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 v4 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 v4 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 v4 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 `resets` and `reset-names` without specifying `minItems: 1` makes the new reset mandatory, breaking validation for device trees that only provide the core reset.
--

commit 620ece5c39191636270140b0c99ea8ddfa402c6f
Author: Jisheng Zhang <jszhang@kernel.org>

dt-bindings: i3c: dw: Add apb reset

Add dt-binding for support of apb reset which is 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 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
>  
>    reset-names:
>      items:
>        - const: core_rst
> +      - const: apb_rst

[Severity: Medium]
Does adding a second item to resets and reset-names without minItems: 1
make the new reset mandatory?

In dt-schema, when items is defined as a list, the minItems and maxItems
properties implicitly default to the number of items in the list. By adding
apb_rst as a second item without specifying minItems: 1, the binding now
strictly requires exactly two items whenever the resets property is present.

This breaks backward compatibility for existing device trees that supply only
core_rst, causing them to fail make dtbs_check. To fix this schema issue,
minItems: 1 must be explicitly added to both resets and reset-names.

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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Asymmetric reset assertion/deassertion order for APB and Core resets violates hardware sequencing rules.

Pre-existing issues:
- [High] Missing error handling for clock enablement and reset deassertion in runtime resume leads to potential kernel panics.
--

commit f0e43b6549a854c56bb22e329a26e4ad0bcd7e1c
Author: Jisheng Zhang <jszhang@kernel.org>
Subject: i3c: dw: Add apb reset support

This commit adds support for an APB interface reset in the DesignWare I3C
master driver. It fetches the optional reset during probe and updates the
runtime power management callbacks to assert and deassert it alongside the
core reset.

> diff --git a/drivers/i3c/master/dw-i3c-master.c b/drivers/i3c/master/dw-i3c-master.c
> index a7593d6efac5a..03efd46db8d0c 100644
> --- a/drivers/i3c/master/dw-i3c-master.c
> +++ b/drivers/i3c/master/dw-i3c-master.c
[ ... ]
> @@ -1784,6 +1789,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;
> @@ -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);

[Severity: High]
Should the reset deassertion order in resume be reversed to follow standard
hardware sequencing?

Currently, the resets are asserted in the suspend path as core_rst followed
by apb_rst, and they are deasserted in the resume path in the exact same
order. Standard practice usually requires a last-in, first-out sequence.

Additionally, dw_i3c_common_probe() requests these resets in core_rst then
apb_rst order. This means the automatic devres teardown will assert them as
apb_rst then core_rst, which is the opposite of the manual runtime suspend
logic introduced here.

Could this asymmetric sequencing cause bus hangs or IP initialization
failures if a core is released from reset while its APB bus interface
remains in reset?

[Severity: High]
This is a pre-existing issue, but this patch extends the pattern to the newly
added apb_rst. Are we missing error handling for the clock enablement and
reset deassertion?

The return values from dw_i3c_master_enable_clks() and both calls to
reset_control_deassert() are being discarded. If any of these operations
fail, the driver will proceed to configure the hardware by calling
dw_i3c_master_set_intr_regs().

Would it be safer to check these return values and return an error code to
the PM framework to avoid potential synchronous external aborts when
accessing inactive IP registers?

>  
>  	dw_i3c_master_set_intr_regs(master);
>  	dw_i3c_master_restore_timing_regs(master);

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

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

[Krzysztof Kozlowski]

On 25/05/2026 16:00, Jisheng Zhang wrote:
> The "core_rst" reset support has been in the code from day1, but the

<Driver requests the "core_rst" since day1 (commit foo bar)....>

Reset cannot be supported and it is about ABI implemented by the driver.

> dt-binding doesn't exist. Add dt-binding to describe reset property.


> 
> Then why not remove the "_rst" suffix? Krzysztof pointed out "Cleanup
> of name is not really worth affecting users. core_rst is not the best
> name but it is not incorrect, either."

This drop, irrelevant. You can provide Link: to previous discussions, OTOH.

With above improved:

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>


Best regards,
Krzysztof

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

[Krzysztof Kozlowski]

On 25/05/2026 16:00, Jisheng Zhang wrote:
> Add dt-binding for support of apb reset which is to reset the APB
> interface.

And this is ABI break, so you must explain WHY breaking ABI is worth
doing that or what is the impact. Additionally you should explain which
devices have it. Does Altera have it? You really lack explanation WHY
you are doing it and which hardware you exactly describe.

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

apb

>  
>    interrupts:
>      maxItems: 1


Best regards,
Krzysztof