Hi Geert,
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@xxxxxxxxxxx>
> > > > Reviewed-by: Biju Das <biju.das.jz@xxxxxxxxxxxxxx>
> > > > ---
> > > > 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.
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" :)
> 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?
Thanks
Phil
