Re: [PATCH v2] dt-bindings: renesas: Document preferred compatible naming

["Niklas Söderlund" <niklas.soderlund+renesas@ragnatech.se>]

Hi Geert,

On 2024-02-13 09:29:55 +0100, Geert Uytterhoeven wrote:
> Hi Niklas,
> 
> On Tue, Feb 13, 2024 at 12:38 AM Niklas Söderlund
> <niklas.soderlund+renesas@ragnatech.se> wrote:
> > On 2024-02-12 20:36:12 +0100, Geert Uytterhoeven wrote:
> > > On Sat, Jan 27, 2024 at 1:20 PM Niklas Söderlund
> > > <niklas.soderlund+renesas@ragnatech.se> wrote:
> > > > Compatibles can come in two formats. Either "vendor,ip-soc" or
> > > > "vendor,soc-ip". Add a DT schema file documenting Renesas preferred
> > > > policy and enforcing it for all new compatibles, except few existing
> > > > patterns.
> > > >
> > > > Suggested-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > > > Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
> > > > ---
> > > > * Changes since v1
> > > > - Split the "SoC agnostic compatibles" section into two to make it's
> > > >   intent clearer.
> > > > - Improved the documentation for each group of compatibles.
> > > > - Reduced the number of regexp to create a larger target area. As
> > > >   suggested by Krzysztof the goal is not to validate each SoC name but
> > > >   check for the correct order of SoC-IP.
> > >
> > > Thanks for the update!
> > >
> > > > --- /dev/null
> > > > +++ b/Documentation/devicetree/bindings/soc/renesas/renesas-soc.yaml
> 
> > > > +properties:
> > > > +  compatible:
> > > > +    oneOf:
> > > > +      # Preferred naming style for compatibles of SoC components
> > > > +      - pattern: "^renesas,emev2-[a-z0-9-]+$"
> > > > +      - pattern: "^renesas,r(7s|8a|9a)[a-z0-9]+-[a-z0-9-]+$"
> > > > +      - pattern: "^renesas,rcar-[a-z0-9-]+$"
> > > > +      - pattern: "^renesas,rz[a-z0-9]*-[a-z0-9-]+$"
> > > > +      - pattern: "^renesas,sh-[a-z0-9-]+$"
> > > > +      - pattern: "^renesas,sh7[a-z0-9]+-[a-z0-9-]+$"
> > >
> > > I guess it's not worth adding rmobile and shmobile prefixes?
> >
> > Maybe we start with this and see how it goes?
> 
> Sure.
> 
> >
> > >
> > > > +      # Fixed legacy compatibles
> > > > +      #
> > > > +      # List cannot grow with new bindings.
> > > > +      - enum:
> > > > +          - renesas,bsc-r8a73a4
> > > > +          - renesas,bsc-sh73a0
> > > > +          - renesas,dbsc-r8a73a4
> > > > +          - renesas,dbsc3-r8a7740
> > > > +          - renesas,em-gio
> > > > +          - renesas,em-sti
> > > > +          - renesas,em-uart
> > >
> > > Perhaps combine these three: "renesas,em-(gpio|sti|usrt)"?
> >
> > Will do.
> 
> That does mean these lines need to use
> 
>   - pattern: "^renesas,em-(gpio|sti|uart)$"
> 
> right?

Yes, a pattern is needed. I will try to condense the lists as much as 
possible at the cost of strictness as this seems to be the common theme 
in reviewer. This will be,

    pattern: "^renesas,em-[a-z0-9]+$"

Or
    pattern: "^renesas,(em|foo|bar|baz)-[a-z0-9]+$"

> 
> > > Now, how do I trigger violations?
> > >
> > > I added the following to a binding file:
> > >
> > >           - enum:
> > >               - renesas,bogus-r8a7778
> > >               - renesas,bogus-r8a7779
> > >           - const: renesas,bogus
> > >
> > > but nothing happened with "make dt_binding_check".
> > >
> > > I added the following to a DTS file:
> > >
> > >         compatible = "renesas,bogus-r8a7778", "renesas,bogus";
> > >
> > > again, nothing happened with "make dtbs_check".
> > >
> > > What am I missing?
> >
> > Hum, this is odd. I have confirmed your finding that
> >
> >     compatible = "renesas,bogus-r8a7778", "renesas,bogus";
> >
> > or
> >
> >     compatible = "renesas,bogus-r8a7778", "renesas,bogus-bar";
> >
> > Do not trigger an issue, but a single compatible,
> >
> >     compatible = "renesas,bogus-r8a7778";
> >
> > Do trigger.
> >
> > I tested this before I reduced the regexp and IIRC it worked as expected
> > for the RFC. Not sure if I have updated dt-schema since, but I know I
> > rebased the branch for v2. I will try to figure out what have gone
> > wrong, if anyone know if something changed in this area pleas let me
> > know.
> 
> Thanks for confirming, and for looking into this!

I figured it out, the error is in the yaml file and I have a fix. Not 
sure why it ever worked in the RFC without it.

> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> -- 
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds

-- 
Kind Regards,
Niklas Söderlund