[PATCH v2 0/3] Scope TH1520 reset driver to VO subsystem

[PATCH v2 0/3] Scope TH1520 reset driver to VO subsystem

[Yao Zi]

T-Head TH1520 SoC integrates nine distinct reset controllers (AON, AP,
DDR, MISC, VI, VO, VP, DSP and AUDIO). Currently only the reset
controller for the VO (Video Output) subsystem is implemented in
mainline Linux.

However, the controller is described with a generic compatible string
"thead,th1520-reset", which may imply control over all reset signals
on the SoC and thus is confusing. It may also cause conflicts when
reset support for other subsystems are introduced.

This series deprecates the generic "thead,th1520-reset" compatible
string, documents a new one, "thead,th1520-reset-vo", which explicitly
describes scope of the reset controller, and converts driver and
devicetree to use the new one.

Krzysztof and Drew, I agree that we should document the bindings for
other reset controllers in TH1520 ASAP, but this requires a lot of new
code: taking TEE-only ones into account, there're eight new controllers
to be documented. Since this is a fix series that may be backported, it
think it may help to keep it small, thus decided to make a separate
series for the new reset controllers.

Changed from v1
- Split the original patch
- Deprecate the old compatible instead of removing it in dt-binding
- Drop the redundant label and outer SoC node for the dt-binding
  example
  - Keep the old compatible in driver for compatibility
  - Link to v1: https://lore.kernel.org/all/20250810-fix_reset_2-v1-1-b0d1900ba578@samsung.com/

Yao Zi (3):
  dt-bindings: reset: Scope the compatible to VO subsystem explicitly
  reset: th1520: Support the new compatible for VO-subsystem controller
  riscv: dts: thead: Scope the reset controller to VO for TH1520

 .../bindings/reset/thead,th1520-reset.yaml      | 17 ++++++++---------
 arch/riscv/boot/dts/thead/th1520.dtsi           |  6 +++---
 drivers/reset/reset-th1520.c                    |  1 +
 3 files changed, 12 insertions(+), 12 deletions(-)

-- 
2.50.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 1/3] dt-bindings: reset: Scope the compatible to VO subsystem explicitly

[Yao Zi]

The reset controller driver for the TH1520 was using the generic
compatible string "thead,th1520-reset". However, the controller
described by this compatible only manages the resets for the Video
Output (VO) subsystem.

Using a generic compatible is confusing as it implies control over all
reset units on the SoC. This could lead to conflicts if support for
other reset controllers on the TH1520 is added in the future like AP.

Let's introduce a new compatible string, "thead,th1520-reset-vo", to
explicitly scope the controller to VO-subsystem. The old one is marked
as deprecated.

Fixes: 30e7573babdc ("dt-bindings: reset: Add T-HEAD TH1520 SoC Reset Controller")
Cc: stable@vger.kernel.org
Reported-by: Icenowy Zheng <uwu@icenowy.me>
Co-developed-by: Michal Wilczynski <m.wilczynski@samsung.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 .../bindings/reset/thead,th1520-reset.yaml      | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
index f2e91d0add7a..3930475dcc04 100644
--- a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
+++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
@@ -15,8 +15,11 @@ maintainers:
 
 properties:
   compatible:
-    enum:
-      - thead,th1520-reset
+    oneOf:
+      - enum:
+          - thead,th1520-reset-vo
+      - const: thead,th1520-reset
+        deprecated: true
 
   reg:
     maxItems: 1
@@ -33,12 +36,8 @@ additionalProperties: false
 
 examples:
   - |
-    soc {
-      #address-cells = <2>;
-      #size-cells = <2>;
-      rst: reset-controller@ffef528000 {
-        compatible = "thead,th1520-reset";
-        reg = <0xff 0xef528000 0x0 0x1000>;
+    reset-controller@ffef528000 {
+        compatible = "thead,th1520-reset-vo";
+        reg = <0xef528000 0x1000>;
         #reset-cells = <1>;
-      };
     };
-- 
2.50.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/3] dt-bindings: reset: Scope the compatible to VO subsystem explicitly

[Krzysztof Kozlowski]

On Wed, Aug 20, 2025 at 07:42:43AM +0000, Yao Zi wrote:
> The reset controller driver for the TH1520 was using the generic
> compatible string "thead,th1520-reset". However, the controller
> described by this compatible only manages the resets for the Video
> Output (VO) subsystem.

Please use subject prefixes matching the subsystem. You can get them for
example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
your patch is touching. For bindings, the preferred subjects are
explained here:
https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

> 
> Using a generic compatible is confusing as it implies control over all
> reset units on the SoC. This could lead to conflicts if support for

No, it won't lead to conflicts. Stop making up reasons.

> other reset controllers on the TH1520 is added in the future like AP.
> 
> Let's introduce a new compatible string, "thead,th1520-reset-vo", to
> explicitly scope the controller to VO-subsystem. The old one is marked
> as deprecated.
> 
> Fixes: 30e7573babdc ("dt-bindings: reset: Add T-HEAD TH1520 SoC Reset Controller")
> Cc: stable@vger.kernel.org

Especially for backporting... Describe the actual bug being fixed here.

> Reported-by: Icenowy Zheng <uwu@icenowy.me>
> Co-developed-by: Michal Wilczynski <m.wilczynski@samsung.com>
> Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> Signed-off-by: Yao Zi <ziyao@disroot.org>
> ---
>  .../bindings/reset/thead,th1520-reset.yaml      | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> index f2e91d0add7a..3930475dcc04 100644
> --- a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> +++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> @@ -15,8 +15,11 @@ maintainers:
>  
>  properties:
>    compatible:
> -    enum:
> -      - thead,th1520-reset
> +    oneOf:
> +      - enum:
> +          - thead,th1520-reset-vo
> +      - const: thead,th1520-reset
> +        deprecated: true

This you can do, but none of this is getting to backports and your DTS
is a NAK. This basically means that this is kind of pointless.

Compatibles do not have particular meanings, so entire explanation that
it implies something is not true. We have been here, this was discussed
for other SoCs and you were told in v1 - don't do that.

You are stuck with the old compatible. Is here an issue to fix? No.

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/3] dt-bindings: reset: Scope the compatible to VO subsystem explicitly

[Yao Zi]

On Thu, Aug 21, 2025 at 09:54:08AM +0200, Krzysztof Kozlowski wrote:
> On Wed, Aug 20, 2025 at 07:42:43AM +0000, Yao Zi wrote:
> > The reset controller driver for the TH1520 was using the generic
> > compatible string "thead,th1520-reset". However, the controller
> > described by this compatible only manages the resets for the Video
> > Output (VO) subsystem.
> 
> Please use subject prefixes matching the subsystem. You can get them for
> example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
> your patch is touching. For bindings, the preferred subjects are
> explained here:
> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters

Thanks for your review, I appreciate it and will stick to
thead,th1520-reset for the VO reset controller.

But I'm not very clear about the subject prefix: I already have a
"dt-bindings: reset: " prefix, should I also make the subject more
precise, including the exact file changed when changing a binding file?

I've seen commits either with or without the precise name of the changed
binding in subjects. For example,

a341bcfbfa7 dt-bindings: reset: add compatible for bcm63xx ephy control

doesn't scope the prefix to brcm,bcm6345-reset.yaml, while

4e55fb7d60e1 dt-bindings: reset: atmel,at91sam9260-reset: add microchip,sama7d65-rstc

does.

Or do I miss other parts in the subject prefix? Thanks for your
explanation.

Best regards,
Yao Zi

> > 
> > Using a generic compatible is confusing as it implies control over all
> > reset units on the SoC. This could lead to conflicts if support for
> 
> No, it won't lead to conflicts. Stop making up reasons.
> 
> > other reset controllers on the TH1520 is added in the future like AP.
> > 
> > Let's introduce a new compatible string, "thead,th1520-reset-vo", to
> > explicitly scope the controller to VO-subsystem. The old one is marked
> > as deprecated.
> > 
> > Fixes: 30e7573babdc ("dt-bindings: reset: Add T-HEAD TH1520 SoC Reset Controller")
> > Cc: stable@vger.kernel.org
> 
> Especially for backporting... Describe the actual bug being fixed here.
> 
> > Reported-by: Icenowy Zheng <uwu@icenowy.me>
> > Co-developed-by: Michal Wilczynski <m.wilczynski@samsung.com>
> > Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
> > Signed-off-by: Yao Zi <ziyao@disroot.org>
> > ---
> >  .../bindings/reset/thead,th1520-reset.yaml      | 17 ++++++++---------
> >  1 file changed, 8 insertions(+), 9 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> > index f2e91d0add7a..3930475dcc04 100644
> > --- a/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> > +++ b/Documentation/devicetree/bindings/reset/thead,th1520-reset.yaml
> > @@ -15,8 +15,11 @@ maintainers:
> >  
> >  properties:
> >    compatible:
> > -    enum:
> > -      - thead,th1520-reset
> > +    oneOf:
> > +      - enum:
> > +          - thead,th1520-reset-vo
> > +      - const: thead,th1520-reset
> > +        deprecated: true
> 
> This you can do, but none of this is getting to backports and your DTS
> is a NAK. This basically means that this is kind of pointless.
> 
> Compatibles do not have particular meanings, so entire explanation that
> it implies something is not true. We have been here, this was discussed
> for other SoCs and you were told in v1 - don't do that.
> 
> You are stuck with the old compatible. Is here an issue to fix? No.
> 
> Best regards,
> Krzysztof
> 

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 1/3] dt-bindings: reset: Scope the compatible to VO subsystem explicitly

[Krzysztof Kozlowski]

On 22/08/2025 10:28, Yao Zi wrote:
> On Thu, Aug 21, 2025 at 09:54:08AM +0200, Krzysztof Kozlowski wrote:
>> On Wed, Aug 20, 2025 at 07:42:43AM +0000, Yao Zi wrote:
>>> The reset controller driver for the TH1520 was using the generic
>>> compatible string "thead,th1520-reset". However, the controller
>>> described by this compatible only manages the resets for the Video
>>> Output (VO) subsystem.
>>
>> Please use subject prefixes matching the subsystem. You can get them for
>> example with 'git log --oneline -- DIRECTORY_OR_FILE' on the directory
>> your patch is touching. For bindings, the preferred subjects are
>> explained here:
>> https://www.kernel.org/doc/html/latest/devicetree/bindings/submitting-patches.html#i-for-patch-submitters
> 
> Thanks for your review, I appreciate it and will stick to
> thead,th1520-reset for the VO reset controller.
> 
> But I'm not very clear about the subject prefix: I already have a
> "dt-bindings: reset: " prefix, should I also make the subject more
> precise, including the exact file changed when changing a binding file?

Yes, as you can check in other commits. Your subject claims that you
change " Scope the compatible to VO subsystem explicitly" for all reset
controllers.

> 
> I've seen commits either with or without the precise name of the changed
> binding in subjects. For example,
> 
> a341bcfbfa7 dt-bindings: reset: add compatible for bcm63xx ephy control

But it says bcm63xx at least. Well, you will find plenty of poor
examples as well. I gave you template response because you made typical
mistake.

Anyway, you ignored rest of my response, so I assume you agree with it
completely.

Best regards,
Krzysztof

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 2/3] reset: th1520: Support the new compatible for VO-subsystem controller

[Yao Zi]

The reset controller for VO-subsystem is previously described with
compatible "thead,th1520-reset", which is misleading since it implies
control over all reset units on the SoC.

A new compatible, "thead,th1520-reset-vo", has been introduced to
describe the controller's scope explicitly, while the old one has been
deprecated. Let's support the compatible in the driver.

Fixes: 4a65326311ab ("reset: thead: Add TH1520 reset controller driver")
Reported-by: Icenowy Zheng <uwu@icenowy.me>
Co-developed-by: Michal Wilczynski <m.wilczynski@samsung.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 drivers/reset/reset-th1520.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/reset/reset-th1520.c b/drivers/reset/reset-th1520.c
index 14d964a9c6b6..4c8ea35b174e 100644
--- a/drivers/reset/reset-th1520.c
+++ b/drivers/reset/reset-th1520.c
@@ -154,6 +154,7 @@ static int th1520_reset_probe(struct platform_device *pdev)
 
 static const struct of_device_id th1520_reset_match[] = {
 	{ .compatible = "thead,th1520-reset" },
+	{ .compatible = "thead,th1520-reset-vo" },
 	{ /* sentinel */ }
 };
 MODULE_DEVICE_TABLE(of, th1520_reset_match);
-- 
2.50.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 2/3] reset: th1520: Support the new compatible for VO-subsystem controller

[Krzysztof Kozlowski]

On Wed, Aug 20, 2025 at 07:42:44AM +0000, Yao Zi wrote:
> The reset controller for VO-subsystem is previously described with
> compatible "thead,th1520-reset", which is misleading since it implies
> control over all reset units on the SoC.
> 
> A new compatible, "thead,th1520-reset-vo", has been introduced to
> describe the controller's scope explicitly, while the old one has been
> deprecated. Let's support the compatible in the driver.
> 
> Fixes: 4a65326311ab ("reset: thead: Add TH1520 reset controller driver")
> Reported-by: Icenowy Zheng <uwu@icenowy.me>

Missing Closes tag. Explain where is the bug. "Implies" different
meaning is not a bug. You need to describe user observable issues of
this bug.

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

[PATCH v2 3/3] riscv: dts: thead: Scope the reset controller to VO for TH1520

[Yao Zi]

The only reset-controller described in TH1520's SoC devicetree takes
control of reset signals in VO subsystem, while using a generic
"thead,th1520-reset" compatible that may imply control over the whole
SoC.

To avoid such confusion, let's replace the compatible with the new
introduced "thead,th1520-reset-vo" that explicitly describes the
controller's scope. The controller's label is updated as well.

Fixes: 1b136de08b5f ("riscv: dts: thead: Introduce reset controller node")
Reported-by: Icenowy Zheng <uwu@icenowy.me>
Co-developed-by: Michal Wilczynski <m.wilczynski@samsung.com>
Signed-off-by: Michal Wilczynski <m.wilczynski@samsung.com>
Signed-off-by: Yao Zi <ziyao@disroot.org>
---
 arch/riscv/boot/dts/thead/th1520.dtsi | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/boot/dts/thead/th1520.dtsi b/arch/riscv/boot/dts/thead/th1520.dtsi
index 03f1d7319049..025402f6aa21 100644
--- a/arch/riscv/boot/dts/thead/th1520.dtsi
+++ b/arch/riscv/boot/dts/thead/th1520.dtsi
@@ -235,7 +235,7 @@ aon: aon {
 		compatible = "thead,th1520-aon";
 		mboxes = <&mbox_910t 1>;
 		mbox-names = "aon";
-		resets = <&rst TH1520_RESET_ID_GPU_CLKGEN>;
+		resets = <&rst_vo TH1520_RESET_ID_GPU_CLKGEN>;
 		reset-names = "gpu-clkgen";
 		#power-domain-cells = <1>;
 	};
@@ -502,8 +502,8 @@ clk: clock-controller@ffef010000 {
 			#clock-cells = <1>;
 		};
 
-		rst: reset-controller@ffef528000 {
-			compatible = "thead,th1520-reset";
+		rst_vo: reset-controller@ffef528000 {
+			compatible = "thead,th1520-reset-vo";
 			reg = <0xff 0xef528000 0x0 0x4f>;
 			#reset-cells = <1>;
 		};
-- 
2.50.1


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv

Re: [PATCH v2 3/3] riscv: dts: thead: Scope the reset controller to VO for TH1520

[Krzysztof Kozlowski]

On Wed, Aug 20, 2025 at 07:42:45AM +0000, Yao Zi wrote:
> @@ -502,8 +502,8 @@ clk: clock-controller@ffef010000 {
>  			#clock-cells = <1>;
>  		};
>  
> -		rst: reset-controller@ffef528000 {
> -			compatible = "thead,th1520-reset";
> +		rst_vo: reset-controller@ffef528000 {
> +			compatible = "thead,th1520-reset-vo";

This breaks all the users. I already commented on NOT DOING this.

Best regards,
Krzysztof


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv