Hi Phil,
On Fri, Apr 22, 2022 at 11:31 AM Phil Edworthy
<phil.edworthy@xxxxxxxxxxx> 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@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
> > > > 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@xxxxxxxxxxxxxx
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
