[v2,02/13] dt-bindings: serial: renesas,em-uart: Document r9a09g011 bindings

The Renesas RZ/V2M (r9a09g011) SoC uses a uart that is compatible with the
EMMA Mobile SoC.

Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
---
v2: Fix dtbs_check by adding missing alternative binding
---
 .../devicetree/bindings/serial/renesas,em-uart.yaml      | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

On Wed, 30 Mar 2022 16:40:13 +0100, Phil Edworthy wrote:
> The Renesas RZ/V2M (r9a09g011) SoC uses a uart that is compatible with the
> EMMA Mobile SoC.
> 
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2: Fix dtbs_check by adding missing alternative binding
> ---
>  .../devicetree/bindings/serial/renesas,em-uart.yaml      | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 

Acked-by: Rob Herring <robh@kernel.org>

Hi Phil,

On Wed, Mar 30, 2022 at 5:41 PM Phil Edworthy <phil.edworthy@renesas.com> wrote:
> The Renesas RZ/V2M (r9a09g011) SoC uses a uart that is compatible with the
> EMMA Mobile SoC.
>
> Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> ---
> v2: Fix dtbs_check by adding missing alternative binding

Thanks for your patch, which is now commit 7bb301812b628099
("dt-bindings: serial: renesas,em-uart: Document r9a09g011
bindings") in tty/tty-next.

> --- a/Documentation/devicetree/bindings/serial/renesas,em-uart.yaml
> +++ b/Documentation/devicetree/bindings/serial/renesas,em-uart.yaml
> @@ -14,7 +14,14 @@ allOf:
>
>  properties:
>    compatible:
> -    const: renesas,em-uart
> +    oneOf:
> +      - items:
> +          - enum:
> +              - renesas,r9a09g011-uart    # RZ/V2M
> +          - const: renesas,em-uart        # generic EMMA Mobile compatible UART
> +
> +      - items:
> +          - const: renesas,em-uart        # generic EMMA Mobile compatible UART

The above looks good to me.

>
>    reg:
>      maxItems: 1

However, unlike EMEV2, RZ/V2M defines two clocks: pclk and sclk.
Hence please update the clocks section to reflect that.

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

Hi Phil,

On Fri, Apr 22, 2022 at 11:31 AM Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> On 22 April 2022 09:45 Geert Uytterhoeven wrote:
> > On Fri, Apr 22, 2022 at 10:28 AM Phil Edworthy wrote:
> > > On 20 April 2022 22:26 Geert Uytterhoeven wrote:
> > > > On Wed, Mar 30, 2022 at 5:41 PM Phil Edworthy wrote:
> > > > > The Renesas RZ/V2M (r9a09g011) SoC uses a uart that is compatible
> > with
> > > > the
> > > > > EMMA Mobile SoC.
> > > > >
> > > > > Signed-off-by: Phil Edworthy <phil.edworthy@renesas.com>
> > > > > Reviewed-by: Biju Das <biju.das.jz@bp.renesas.com>
> > > > > ---
> > > > > v2: Fix dtbs_check by adding missing alternative binding
> > > >
> > > > Thanks for your patch, which is now commit 7bb301812b628099
> > > > ("dt-bindings: serial: renesas,em-uart: Document r9a09g011
> > > > bindings") in tty/tty-next.
> > > >
> > > > > --- a/Documentation/devicetree/bindings/serial/renesas,em-uart.yaml
> > > > > +++ b/Documentation/devicetree/bindings/serial/renesas,em-uart.yaml
> > > > However, unlike EMEV2, RZ/V2M defines two clocks: pclk and sclk.
> > > > Hence please update the clocks section to reflect that.
> > > You are right that the uart has two clocks.
> > >
> > > Note though that pclk is shared by both uarts. The HW manual says:
> > > "ch. 1 is for use with the ISP support package, so do not
> > > use registers related to this channel.". Due to this, section
> > > 48.5.2.50 Clock ON/OFF Control Register 15 (CPG_CLK_ON15) says
> > > that bit 20, CLK4_ONWEN (enable for URT_PCLK) should be written
> > > as 0.
> > >
> > > I took this to mean that the URT_PCLK is enabled by the ISP firmware
> > > and software must not touch it. I am not sure if the DT bindings
> > > should document a clock that is specified as do not touch in the
> > > HW manual. This is a bit of a grey area.
> >
> > "DT describes hardware, not software policy".
> >
> > But I agree this is a grey area.
> I wish the HW manual either didn’t mention this clock that you should
> not touch, or didn’t specify anything as "used by the ISP firmware" :)

Yeah, hardware manuals making too many assumptions about the software
that will run on it will lead to headaches...

> > One option would be to mark URT_PCLK critical, so it won't be disabled.
> > But that would still mean it's enabled by Linux, i.e. Linux would set
> > CLK4_ONWEN to 1 while enabling the clock.
> >
> > Another option would be to create URT_PCLK as a non-gateable clock,
> > so Linux won't ever touch the register bits.
> >
> > Or just ignore URT_PCLK and do nothing, like you did ;-)
> > Would it be possible for a user to not use the ISP firmware at all,
> > and go full Linux, hence using both UART channels and URT_PCLK?
> It is possible to not use the ISP firmware, but them what do we do?
> Ignore everything in the HW manual that says "ISP firmware"?
>
> Ideally, we want to only enable a clock if it's not already enabled,
> but not turn it off if it is enabled. Isn't that a critical clk?

__clk_core_init() explicitly enables clocks marked with
CLK_IS_CRITICAL.  I think it does so without checking the hardware
if the clock is already enabled or not, so probably it will access
the reserved hardware bits regardless.

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