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

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

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