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

Geert Uytterhoeven
2023-05-11T07:08:07Z

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