Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Wolfram Sang <wsa+renesas@sang-engineering.com>
Cc: Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	Linux MMC List <linux-mmc@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>, Rob Herring <robh+dt@kernel.org>,
	Ulf Hansson <ulf.hansson@linaro.org>
Subject: Re: [PATCH v3] dt-bindings: mmc: renesas,sdhi: add optional SDnH clock
Date: Fri, 19 Nov 2021 11:12:57 +0100	[thread overview]
Message-ID: <CAMuHMdVR=6KNY2bZcdDPGm7QA+7x=-0qNpa3Ev-2C3ut09KMHQ@mail.gmail.com> (raw)
In-Reply-To: <20211115160600.4455-1-wsa+renesas@sang-engineering.com>

Hi Wolfram,

On Mon, Nov 15, 2021 at 5:08 PM Wolfram Sang
<wsa+renesas@sang-engineering.com> wrote:
> This only applies to R-Car Gen2 and later generations, so we need to
> distinguish.
>
> Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com>
> Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Thanks for your patch!

> ---
>
> v1 and v2 were part of a 21-patch-series which was accepted now except
> for this patch. Updated according to Geert's comments and finally also
> sent to Rob and the DT mailing list.
>
> Tested with:
> m dtbs_check DT_SCHEMA_FILES=Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
>
> I hope it really does what I intended to check.
>
> If so, the patch can be applied individually. I think, however, it is
> most convenient if Geert picks it up together with the 20 other patches.

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-devel for v5.17, with the below fixed.

> --- a/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> +++ b/Documentation/devicetree/bindings/mmc/renesas,sdhi.yaml
> @@ -129,15 +129,37 @@ allOf:
>          - clock-names
>          - resets
>      else:
> -      properties:
> -        clocks:
> -          minItems: 1
> -          maxItems: 2
> -        clock-names:
> -          minItems: 1
> -          items:
> -            - const: core
> -            - const: cd
> +      if:
> +        properties:
> +          compatible:
> +            contains:
> +              enum:
> +                - renesas,rcar-gen2-sdhi
> +                - renesas,rcar-gen3-sdhi
> +      then:
> +        properties:
> +          clocks:
> +            minItems: 1
> +            maxItems: 3
> +          clock-names:
> +            minItems: 1
> +            maxItems: 3

"maxItems" is not needed with an "items" list

"make dt_bindings_check" doesn't complain, presumably because this
is part of an if/else block.

> +            uniqueItems: true
> +            items:
> +              - const: core
> +              - enum: [ clkh, cd ]
> +              - const: cd
> +      else:
> +        properties:
> +          clocks:
> +            minItems: 1
> +            maxItems: 2
> +          clock-names:
> +            minItems: 1
> +            maxItems: 2

Likewise ("git show --color-words" shows it wasn't present before).

> +            items:
> +              - const: core
> +              - const: cd
>
>    - if:
>        properties:

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