diff mbox series

dt-bindings: clock: renesas,rzg2l: Document RZ/Five SoC

Message ID 20220726174525.620-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive)
State Mainlined
Commit e312ae92077f90d6ccdca05fb6d640bd9624c37c
Delegated to: Geert Uytterhoeven
Headers show
Series dt-bindings: clock: renesas,rzg2l: Document RZ/Five SoC | expand

Commit Message

Lad Prabhakar July 26, 2022, 5:45 p.m. UTC
The CPG block on the RZ/Five SoC is almost identical to one found on the
RZ/G2UL SoC. "renesas,r9a07g043-cpg" compatible string will be used on
the RZ/Five SoC so to make this clear, update the comment to include
RZ/Five SoC.

Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
---
Note the driver changes [0] have been already queued for v5.20.

[0] https://patchwork.kernel.org/project/linux-renesas-soc/cover/
20220622181723.13033-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
---
 Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Rob Herring (Arm) July 27, 2022, 3:37 p.m. UTC | #1
On Tue, Jul 26, 2022 at 06:45:25PM +0100, Lad Prabhakar wrote:
> The CPG block on the RZ/Five SoC is almost identical to one found on the
> RZ/G2UL SoC. "renesas,r9a07g043-cpg" compatible string will be used on
> the RZ/Five SoC so to make this clear, update the comment to include
> RZ/Five SoC.

It's either the same part or it isn't. 'almost identical' doesn't sound 
like the former. Unless it's the former, it's a nak for me.

Litering the drivers with #ifdef CONFIG_ARM64/CONFIG_RISCV is not great 
either. That's not great for compile coverage and they have nothing to 
do with the architecture.

> 
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> ---
> Note the driver changes [0] have been already queued for v5.20.
> 
> [0] https://patchwork.kernel.org/project/linux-renesas-soc/cover/
> 20220622181723.13033-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
> ---
>  Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
> index d036675e0779..487f74cdc749 100644
> --- a/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
> +++ b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
> @@ -24,7 +24,7 @@ description: |
>  properties:
>    compatible:
>      enum:
> -      - renesas,r9a07g043-cpg # RZ/G2UL{Type-1,Type-2}
> +      - renesas,r9a07g043-cpg # RZ/G2UL{Type-1,Type-2} and RZ/Five
>        - renesas,r9a07g044-cpg # RZ/G2{L,LC}
>        - renesas,r9a07g054-cpg # RZ/V2L
>        - renesas,r9a09g011-cpg # RZ/V2M
> -- 
> 2.17.1
> 
>
Lad, Prabhakar Aug. 12, 2022, 8:47 a.m. UTC | #2
Hi Rob,

Thank you for the review.

On Wed, Jul 27, 2022 at 4:37 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Jul 26, 2022 at 06:45:25PM +0100, Lad Prabhakar wrote:
> > The CPG block on the RZ/Five SoC is almost identical to one found on the
> > RZ/G2UL SoC. "renesas,r9a07g043-cpg" compatible string will be used on
> > the RZ/Five SoC so to make this clear, update the comment to include
> > RZ/Five SoC.
>
> It's either the same part or it isn't. 'almost identical' doesn't sound
> like the former. Unless it's the former, it's a nak for me.
>
It's the latter.

> Litering the drivers with #ifdef CONFIG_ARM64/CONFIG_RISCV is not great
> either. That's not great for compile coverage and they have nothing to
> do with the architecture.
>
Geert any thoughts?

Cheers,
Prabhakar

> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> > ---
> > Note the driver changes [0] have been already queued for v5.20.
> >
> > [0] https://patchwork.kernel.org/project/linux-renesas-soc/cover/
> > 20220622181723.13033-1-prabhakar.mahadev-lad.rj@bp.renesas.com/
> > ---
> >  Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
> > index d036675e0779..487f74cdc749 100644
> > --- a/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
> > +++ b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
> > @@ -24,7 +24,7 @@ description: |
> >  properties:
> >    compatible:
> >      enum:
> > -      - renesas,r9a07g043-cpg # RZ/G2UL{Type-1,Type-2}
> > +      - renesas,r9a07g043-cpg # RZ/G2UL{Type-1,Type-2} and RZ/Five
> >        - renesas,r9a07g044-cpg # RZ/G2{L,LC}
> >        - renesas,r9a07g054-cpg # RZ/V2L
> >        - renesas,r9a09g011-cpg # RZ/V2M
> > --
> > 2.17.1
> >
> >
Geert Uytterhoeven Aug. 12, 2022, 9:32 a.m. UTC | #3
Hi Rob,

On Fri, Aug 12, 2022 at 10:48 AM Lad, Prabhakar
<prabhakar.csengg@gmail.com> wrote:
> On Wed, Jul 27, 2022 at 4:37 PM Rob Herring <robh@kernel.org> wrote:
> > On Tue, Jul 26, 2022 at 06:45:25PM +0100, Lad Prabhakar wrote:
> > > The CPG block on the RZ/Five SoC is almost identical to one found on the
> > > RZ/G2UL SoC. "renesas,r9a07g043-cpg" compatible string will be used on
> > > the RZ/Five SoC so to make this clear, update the comment to include
> > > RZ/Five SoC.
> >
> > It's either the same part or it isn't. 'almost identical' doesn't sound
> > like the former. Unless it's the former, it's a nak for me.
> >
> It's the latter.

To me, it looks like both blocks are identical, and the differences
are in the integration into the SoC:
  1. Some clocks do not exist (are not documented?) on RZ/Five,
     because the consumer blocks do not exist (are not documented?).
  2. Some interrupt controller clocks and resets have different names,
     but use the exact same registers and bits.

For 1, probably we could have kept those clocks anyway (they would
be disabled by CCF due to being unused). But I'm not 100% sure it is
safe to write to the corresponding registers (probably the hardware
engineers would recommend not to access the registers, regardless if
it is safe or not ;-), so we do not instantiate these clocks on RISC-V.

For 2, we decided to play it safe, too, and follow the naming in the
documentation, in both bindings and driver.

> > Litering the drivers with #ifdef CONFIG_ARM64/CONFIG_RISCV is not great
> > either. That's not great for compile coverage and they have nothing to
> > do with the architecture.

I agree #ifdef's do have disadvantages.  But they seemed to be the
best pragmatic solution, to avoid two separate drivers.

And the architecture does specify SoC integration.

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
Geert Uytterhoeven Sept. 1, 2022, 10:16 a.m. UTC | #4
On Tue, Jul 26, 2022 at 7:45 PM Lad Prabhakar
<prabhakar.mahadev-lad.rj@bp.renesas.com> wrote:
> The CPG block on the RZ/Five SoC is almost identical to one found on the
> RZ/G2UL SoC. "renesas,r9a07g043-cpg" compatible string will be used on
> the RZ/Five SoC so to make this clear, update the comment to include
> RZ/Five SoC.
>
> Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
i.e. will queue in renesas-clk-for-v6.1.

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 mbox series

Patch

diff --git a/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
index d036675e0779..487f74cdc749 100644
--- a/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
+++ b/Documentation/devicetree/bindings/clock/renesas,rzg2l-cpg.yaml
@@ -24,7 +24,7 @@  description: |
 properties:
   compatible:
     enum:
-      - renesas,r9a07g043-cpg # RZ/G2UL{Type-1,Type-2}
+      - renesas,r9a07g043-cpg # RZ/G2UL{Type-1,Type-2} and RZ/Five
       - renesas,r9a07g044-cpg # RZ/G2{L,LC}
       - renesas,r9a07g054-cpg # RZ/V2L
       - renesas,r9a09g011-cpg # RZ/V2M