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

From: Geert Uytterhoeven <geert@linux-m68k.org>
To: Phil Edworthy <phil.edworthy@renesas.com>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Magnus Damm <magnus.damm@gmail.com>,
	Rob Herring <robh+dt@kernel.org>,
	Krzysztof Kozlowski <krzk@kernel.org>,
	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>,
	Biju Das <biju.das.jz@bp.renesas.com>
Subject: Re: [PATCH v2 02/13] dt-bindings: serial: renesas,em-uart: Document r9a09g011 bindings
Date: Fri, 22 Apr 2022 10:45:11 +0200	[thread overview]
Message-ID: <CAMuHMdWtLxdXztfLxSvb57WOO46qTW6c+fD=P0O5qicop=b+3g@mail.gmail.com> (raw)
In-Reply-To: <TYYPR01MB7086BD705F91E855F0D3FD45F5F79@TYYPR01MB7086.jpnprd01.prod.outlook.com>

Hi Phil,

On Fri, Apr 22, 2022 at 10:28 AM Phil Edworthy
<phil.edworthy@renesas.com> wrote:
> On 20 April 2022 22:26 Geert Uytterhoeven wrote:
> > 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.
> 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.

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?

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