Re: [PATCH] dt-bindings: timer: sifive,clint: Clean up compatible value section

[Geert Uytterhoeven <geert@linux-m68k.org>]

Hi Conor,

On Wed, May 10, 2023 at 6:02 PM Conor Dooley <conor@kernel.org> wrote:
> On Wed, May 10, 2023 at 03:27:24PM +0200, Geert Uytterhoeven wrote:
> > Replace the sentences in the description listing some supported variants
> > by comments on the individual compatible values, to ease future
> > maintenance.  While at it, restore alphabetical sort order.
> >
> > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
> > ---
> >  .../bindings/timer/sifive,clint.yaml          | 21 +++++++------------
> >  1 file changed, 8 insertions(+), 13 deletions(-)
> >
> > diff --git a/Documentation/devicetree/bindings/timer/sifive,clint.yaml b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
> > index 94bef9424df1bc6a..34a81510678134eb 100644
> > --- a/Documentation/devicetree/bindings/timer/sifive,clint.yaml
> > +++ b/Documentation/devicetree/bindings/timer/sifive,clint.yaml
> > @@ -29,11 +29,11 @@ properties:
> >      oneOf:
> >        - items:
> >            - enum:
> > -              - sifive,fu540-c000-clint
> > -              - starfive,jh7100-clint
> > -              - starfive,jh7110-clint
> > -              - canaan,k210-clint
> > -          - const: sifive,clint0
> > +              - canaan,k210-clint       # Canaan Kendryte K210
> > +              - sifive,fu540-c000-clint # SiFive FU540
> > +              - starfive,jh7100-clint   # StarFive JH7100
> > +              - starfive,jh7110-clint   # StarFive JH7110
> > +          - const: sifive,clint0        # SiFive CLINT v0 IP block
> >        - items:
> >            - enum:
> >                - allwinner,sun20i-d1-clint
> > @@ -45,14 +45,9 @@ properties:
> >          description: For the QEMU virt machine only
> >
> >      description:
> > -      Should be "<vendor>,<chip>-clint" and "sifive,clint<version>".
> > -      Supported compatible strings are -
> > -      "sifive,fu540-c000-clint" for the SiFive CLINT v0 as integrated
> > -      onto the SiFive FU540 chip, "canaan,k210-clint" for the SiFive
> > -      CLINT v0 as integrated onto the Canaan Kendryte K210 chip, and
> > -      "sifive,clint0" for the SiFive CLINT v0 IP block with no chip
> > -      integration tweaks.
> > -      Please refer to sifive-blocks-ip-versioning.txt for details
>
> > +      Should be "<vendor>,<chip>-clint", followed by "sifive,clint<version>"
> > +      when compatible with a SiFive CLINT.  Please refer to
> > +      sifive-blocks-ip-versioning.txt for details regarding the latter.
>
> Does this section actually add any value at all?
> By nature of oneOf structure above, such an ordering is required for
> existing entries. For something not here, should we instead be noting
> that sifive,clint0 is to be used when compatible with the SiFive/RISC-V
> clint, rather than doing a sifive-blocks-ip-versioning.txt dance?

I considered removing the paragraph.  But I kept it, just not to loose
the reference to sifive-blocks-ip-versioning.txt, which someone might
find useful.

> As it stands though, patch is an improvement, so:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks!

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