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;
Date:   Tue, 16 Aug 2022 16:06:33 +0200
From:   Andrew Jones <ajones@ventanamicro.com>
To:     Conor.Dooley@microchip.com
Cc:     jrtc27@jrtc27.com, palmer@dabbelt.com, robh@kernel.org,
        tglx@linutronix.de, maz@kernel.org,
        krzysztof.kozlowski+dt@linaro.org, paul.walmsley@sifive.com,
        aou@eecs.berkeley.edu, daniel.lezcano@linaro.org,
        anup@brainfault.org, guoren@kernel.org, sagar.kadam@sifive.com,
        linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
        linux-riscv@lists.infradead.org, qemu-riscv@nongnu.org
Subject: Re: [PATCH 0/3] Fix dt-validate issues on qemu dtbdumps due to
 dt-bindings
Message-ID: <20220816140633.pkjws7n3qxsescjh@kamzik>
References: <20220805162844.1554247-1-mail@conchuod.ie>
 <YvGBYKZyW0B2/wSr@Jessicas-MacBook-Pro>
 <94fe7e46-6156-1cc5-a4dc-1eee78e99bc4@microchip.com>
 <20220809141436.GA1706120-robh@kernel.org>
 <61829ccd-20d7-e2f0-0a6b-bcd0e076b9ea@microchip.com>
 <1d94e18d-7f51-0619-95a9-6ca5fa7d9671@microchip.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <1d94e18d-7f51-0619-95a9-6ca5fa7d9671@microchip.com>
Precedence: bulk

On Mon, Aug 15, 2022 at 07:18:02PM +0000, Conor.Dooley@microchip.com wrote:
> Any takers on trashing my regex? Otherwise I'll just submit
> a v2 with the regex and it can be shat on there instead :)
> 
> On 09/08/2022 19:36, Conor Dooley wrote:
> > On 09/08/2022 15:14, Rob Herring wrote:
> >> On Mon, Aug 08, 2022 at 10:01:11PM +0000, Conor.Dooley@microchip.com wrote:
> >>> On 08/08/2022 22:34, Jessica Clarke wrote:
> >>>> On Fri, Aug 05, 2022 at 05:28:42PM +0100, Conor Dooley wrote:
> >>>>> From: Conor Dooley <conor.dooley@microchip.com>
> >>>>> The final patch adds some new ISA strings
> >>>>> which needs scruitiny from someone with more knowledge about what ISA
> >>>>> extension strings should be reported in a dt than I have.
> >>>>
> >>>> Listing every possible ISA string supported by the Linux kernel really
> >>>> is not going to scale...
> >>
> >> How does the kernel scale? (No need to answer)
> >>
> >>> Yeah, totally correct there. Case for adding a regex I suppose, but I
> >>> am not sure how to go about handling the multi-letter extensions or
> >>> if parsing them is required from a binding compliance point of view.
> >>> Hoping for some input from Palmer really.
> >>
> >> Yeah, looks like a regex pattern is needed.
> > 
> > I started pottering away at this but I have arrived at:
> > rv64imaf?d?c?h?(_z[imafdqcbvkh]([a-z])*)*$

Don't forget the ^ at the start.

Do we need to worry about optional major and minor version numbers?
Or check that Z names have at least one character following the category
character? Actually, the first letter after Z being a category is only a
convention. Maybe we don't want to enforce that. What about X extensions?

Thanks,
drew

> > 
> > I suspect that before "h?" there should be more single letter
> > extensions added for completeness sake. So then it'd bloat out to:
> > rv64imaf?d?q?c?b?v?k?h?(_z[imafdqcbvkh]([a-z])*)*$
> > 
> > I checked a couple different "bad" isa strings against it and
> > nothing went up in flames but my regex skills are far from great
> > so I'm sure there's better ways to represent this.
> > 
> > Anyways, this pattern is based on my understanding that:
> > - the single letter order is fixed & we don't care about things that
> >   can't even do "ima"
> > - the multi letter extensions are all in a "_z<foo>" format where the
> >   first letter of <foo> is a valid single letter extension
> > - we don't care about the e extension from an OS PoV (this could be a
> >   very flawed take...)
> > - after the first two chars, the extension name could be an english
> >   word (ifencei anyone?) so it's not worth restricting the charset
> > - that attempting to validate the contents of the multiletter extensions
> >   with dt-validate beyond the formatting is a futile, massively verbose
> >   or unwieldy exercise at best
> > 
> > Some or all of those assumptions could be very very wrong so if {someone,
> > anyone} wants to correct me - feel ***more*** than free.. 
> > 
> > Thanks,
> > Conor.
> > 
> > patch would then look like:
> > 
> > diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > index d632ac76532e..1e54e7746190 100644
> > --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> > +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> > @@ -74,9 +74,7 @@ properties:
> >        insensitive, letters in the riscv,isa string must be all
> >        lowercase to simplify parsing.
> >      $ref: "/schemas/types.yaml#/definitions/string"
> > -    enum:
> > -      - rv64imac
> > -      - rv64imafdc
> > +    pattern: rv64imaf?d?q?c?b?v?k?h?(_z[imafdqcbvkh]([a-z])*)*$
> >  
> >    # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
> >    timebase-frequency: false
>