[RFC,2/6] dt-bindings: riscv: add riscv,isa-extension-* property and incompatible example

From: Conor Dooley <conor.dooley@microchip.com>

This dt-binding is illustrative *only*, it doesn't yet do what I want it
to do in terms of enforcement etc. I am yet to figure out exactly how to
wrangle the binding such that the individual properties have more
generous versions than the generic pattern property.
This binding *will* generate errors, and needs rework before it can
seriously be considered.
Nevertheless, it should demonstrate how I intend such a property be
used.

Not-signed-off-by: Conor Dooley <conor.dooley@microchip.com>
---
 .../devicetree/bindings/riscv/cpus.yaml       | 61 ++++++++++++++++++-
 1 file changed, 60 insertions(+), 1 deletion(-)

On 08/05/2023 20:16, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
> 
> This dt-binding is illustrative *only*, it doesn't yet do what I want it
> to do in terms of enforcement etc. I am yet to figure out exactly how to
> wrangle the binding such that the individual properties have more
> generous versions than the generic pattern property.
> This binding *will* generate errors, and needs rework before it can
> seriously be considered.
> Nevertheless, it should demonstrate how I intend such a property be
> used.
> 
> Not-signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
>  .../devicetree/bindings/riscv/cpus.yaml       | 61 ++++++++++++++++++-
>  1 file changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/riscv/cpus.yaml b/Documentation/devicetree/bindings/riscv/cpus.yaml
> index 405915b04d69..cccb3b2ae23d 100644
> --- a/Documentation/devicetree/bindings/riscv/cpus.yaml
> +++ b/Documentation/devicetree/bindings/riscv/cpus.yaml
> @@ -100,6 +100,15 @@ properties:
>        lowercase.
>      $ref: "/schemas/types.yaml#/definitions/string"
>      pattern: ^rv(?:64|32)imaf?d?q?c?b?k?j?p?v?h?(?:[hsxz](?:[a-z])+)?(?:_[hsxz](?:[a-z])+)*$
> +    deprecated: true
> +
> +  riscv,isa-base:
> +    description:
> +      Identifies the base ISA supported by a hart.
> +    $ref: "/schemas/types.yaml#/definitions/string"

Drop quotes.

> +    enum:
> +      - rv32i
> +      - rv64i
>  
>    # RISC-V requires 'timebase-frequency' in /cpus, so disallow it here
>    timebase-frequency: false
> @@ -136,8 +145,32 @@ properties:
>        DMIPS/MHz, relative to highest capacity-dmips-mhz
>        in the system.
>  
> +  riscv,isa-extension-v:
> +    description: RISC-V Vector extension
> +    $ref: "/schemas/types.yaml#/definitions/string"

Drop quotes.

> +    oneOf:
> +      - const: v1.0.0
> +        description: the original incarnation
> +      - const: v1.0.1
> +        description: backwards compat was broken here
> +
> +patternProperties:
> +  "^riscv,isa-extension-*":

Are all these -i/-m/-a extensions obvious/known to RISC-V folks? I have
no clue what's this, so the question is: do they need some explanation
in the bindings?

> +    description:
> +      Catch-all property for ISA extensions that do not need any special
> +      handling, and of which all known versions are compatible with their
> +      original revision.
> +    $ref: "/schemas/types.yaml#/definitions/string"

Drop quotes.

> +    enum:
> +      - v1.0.0

Your example should not validate here... you have there v2.0.0 and v1.0.1

> +
> +oneOf:
> +  - required:
> +      - riscv,isa-base
> +  - required:
> +      - riscv,isa
> +
>  required:
> -  - riscv,isa
>    - interrupt-controller
>  
>  additionalProperties: true
> @@ -208,4 +241,30 @@ examples:
>                  };
>          };
>      };
> +
> +  - |
> +    // Example 3: Extension specification
> +    cpus {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        cpu@0 {
> +                device_type = "cpu";
> +                reg = <0>;
> +                compatible = "riscv";
> +                riscv,isa-base = "rv64i";
> +                riscv,isa-extension-i = "v1.0.0";
> +                riscv,isa-extension-m = "v1.0.0";
> +                riscv,isa-extension-a = "v1.0.0";
> +                riscv,isa-extension-f = "v1.0.0";
> +                riscv,isa-extension-d = "v1.0.0";
> +                riscv,isa-extension-c = "v2.0.0";
> +                riscv,isa-extension-v = "v1.0.1";
> +                mmu-type = "riscv,sv48";
> +                interrupt-controller {
> +                        #interrupt-cells = <1>;
> +                        interrupt-controller;
> +                        compatible = "riscv,cpu-intc";
> +                };
> +        };
> +    };
>  ...

Best regards,
Krzysztof

On Sat, May 13, 2023 at 07:50:22PM +0200, Krzysztof Kozlowski wrote:
> On 08/05/2023 20:16, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> > 
> > This dt-binding is illustrative *only*, it doesn't yet do what I want it
> > to do in terms of enforcement etc. I am yet to figure out exactly how to
> > wrangle the binding such that the individual properties have more
> > generous versions than the generic pattern property.
> > This binding *will* generate errors, and needs rework before it can
> > seriously be considered.
> > Nevertheless, it should demonstrate how I intend such a property be
> > used.

> > +    oneOf:
> > +      - const: v1.0.0
> > +        description: the original incarnation
> > +      - const: v1.0.1
> > +        description: backwards compat was broken here
> > +
> > +patternProperties:
> > +  "^riscv,isa-extension-*":
> 
> Are all these -i/-m/-a extensions obvious/known to RISC-V folks? I have
> no clue what's this, so the question is: do they need some explanation
> in the bindings?

Yes, these should be well known. In the same way that "neon" should mean
something to someone doing arm64. Nevertheless, the plan is to drop the
string side of this entirely & actually document the meaning of -i/-m/-a
etc.

> > +    description:
> > +      Catch-all property for ISA extensions that do not need any special
> > +      handling, and of which all known versions are compatible with their
> > +      original revision.
> > +    $ref: "/schemas/types.yaml#/definitions/string"
> 
> Drop quotes.
> 
> > +    enum:
> > +      - v1.0.0
> 
> Your example should not validate here... you have there v2.0.0 and v1.0.1

As noted in the commit message, this is illustrative only & cannot work.
There doesn't appear to be a way to make the patternProperty fallback
more specific than the explicitly defined properties.
I wanted to get something out for initial thoughts before trying to do
further wrangling, lest it be a complete waste of time.
Consensus seems to be that versions are a thing of the past and that
property-presence based probing is a better idea. See the discussion
on the cover for that.
It does conveniently mean that all this complexity can be thrown in the
bin.

Cheers,
Conor.