Re: [PATCH v3 01/12] dt-bindings: serial: renesas,em-uart: Add RZ/V2M clock to access the registers

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Phil Edworthy <phil.edworthy@renesas.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
	Geert Uytterhoeven <geert+renesas@glider.be>,
	Magnus Damm <magnus.damm@gmail.com>,
	Biju Das <biju.das.jz@bp.renesas.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"open list:SERIAL DRIVERS" <linux-serial@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>
Subject: Re: [PATCH v3 01/12] dt-bindings: serial: renesas,em-uart: Add RZ/V2M clock to access the registers
Date: Wed, 4 May 2022 10:06:51 +0200	[thread overview]
Message-ID: <CAMuHMdXjdoY3qrQhHxCdc=W7-MrX7FVeWqZ8Vma2-1Sc5_=hEQ@mail.gmail.com> (raw)
In-Reply-To: <20220503115557.53370-2-phil.edworthy@renesas.com>

Hi Phil,

On Tue, May 3, 2022 at 2:01 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> The RZ/V2M SoC has an additional clock to access the registers. The HW
> manual says this clock should not be touched as it is used by the
> "ISP Firmware".
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>

Thanks for your patch!

> --- a/Documentation/devicetree/bindings/serial/renesas,em-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,em-uart.yaml
> @@ -9,9 +9,6 @@ title: Renesas EMMA Mobile UART Interface
>  maintainers:
>    - Magnus Damm <magnus.damm@gmail.com>
>
> -allOf:
> -  - $ref: serial.yaml#
> -
>  properties:
>    compatible:
>      oneOf:
> @@ -29,11 +26,32 @@ properties:
>    interrupts:
>      maxItems: 1
>
> -  clocks:
> -    maxItems: 1
> +allOf:
> +  - $ref: serial.yaml#
>
> -  clock-names:
> -    const: sclk
> +  - if:
> +      properties:
> +        compatible:
> +          contains:
> +            const: renesas,r9a09g011-uart
> +    then:
> +      properties:
> +        clocks:
> +          items:
> +            - description: UART functional clock
> +            - description: Optional internal clock to access the registers

It's not optional on r9a09g011, right?

> +        clock-names:
> +          items:
> +            - const: sclk
> +            - const: pclk
> +    else:
> +      properties:
> +        clocks:
> +          items:
> +            - description: UART functional clock
> +        clock-names:
> +          items:
> +            - const: sclk
>
>  required:
>    - compatible

The rest LGTM, so with the above fixed:
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

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