Message ID | 20220330154024.112270-3-phil.edworthy@renesas.com (mailing list archive) |
---|---|
State | Superseded |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Series | Add new Renesas RZ/V2M SoC and Renesas RZ/V2M EVK support | expand |
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 Geert, (updated Krzysztof's email) 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. Thanks Phil
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
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@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. 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
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
diff --git a/Documentation/devicetree/bindings/serial/renesas,em-uart.yaml b/Documentation/devicetree/bindings/serial/renesas,em-uart.yaml index e98ec48fee46..332c385618e1 100644 --- 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 reg: maxItems: 1