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

[Yao Zi <ziyao@disroot.org>]

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