Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <40ff1fc7f5220db7d527c57ac4bad16c3945ae08.1683725179.git.geert+renesas@glider.be>
 <20230510-headsman-deskwork-6826f9f174df@spud>
In-Reply-To: <20230510-headsman-deskwork-6826f9f174df@spud>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Thu, 11 May 2023 09:07:23 +0200
Message-ID: <CAMuHMdVKES1dgTMrLwNA3t37m-wscYETejRBM2aroB_ZF-pbaw@mail.gmail.com>
Subject: Re: [PATCH] dt-bindings: timer: sifive,clint: Clean up compatible
 value section
To:     Conor Dooley <conor@kernel.org>
Cc:     Daniel Lezcano <daniel.lezcano@linaro.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        Conor Dooley <conor+dt@kernel.org>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Paul Walmsley <paul.walmsley@sifive.com>,
        Anup Patel <anup@brainfault.org>, devicetree@vger.kernel.org,
        linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8BIT
Precedence: bulk

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