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

[Conor Dooley]

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@xxxxxxxxx>
> ---
>  .../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?

As it stands though, patch is an improvement, so:
Reviewed-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>

Thanks,
Conor.

Attachment: signature.asc
Description: PGP signature