Hi Geert,
Thank you for the review.
On Tue, Sep 2, 2025 at 2:01 PM Geert Uytterhoeven <geert@xxxxxxxxxxxxxx> wrote:
>
> Hi Prabhakar,
>
> On Mon, 1 Sept 2025 at 20:30, Prabhakar <prabhakar.csengg@xxxxxxxxx> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> >
> > Add module and core clocks used by Ethernet Subsystem (Ethernet_SS),
> > Ethernet MAC (GMAC), Ethernet Switch (ETHSW).
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
>
> Thanks for your patch!
>
> > --- a/drivers/clk/renesas/r9a09g077-cpg.c
> > +++ b/drivers/clk/renesas/r9a09g077-cpg.c
> > @@ -72,7 +72,7 @@ enum rzt2h_clk_types {
> >
> > enum clk_ids {
> > /* Core Clock Outputs exported to DT */
> > - LAST_DT_CORE_CLK = R9A09G077_USB_CLK,
> > + LAST_DT_CORE_CLK = R9A09G077_GMAC2_PCLKAH,
> >
> > /* External Input Clocks */
> > CLK_EXTAL,
> > @@ -166,11 +166,21 @@ static const struct cpg_core_clk r9a09g077_core_clks[] __initconst = {
> > DEF_DIV("CA55S", R9A09G077_CLK_CA55S, CLK_SEL_CLK_PLL0, DIVCA55S,
> > dtable_1_2),
> > DEF_FIXED("PCLKGPTL", R9A09G077_CLK_PCLKGPTL, CLK_SEL_CLK_PLL1, 2, 1),
> > + DEF_FIXED("PCLKH", R9A09G077_CLK_PCLKH, CLK_SEL_CLK_PLL1, 4, 1),
> > DEF_FIXED("PCLKM", R9A09G077_CLK_PCLKM, CLK_SEL_CLK_PLL1, 8, 1),
> > DEF_FIXED("PCLKL", R9A09G077_CLK_PCLKL, CLK_SEL_CLK_PLL1, 16, 1),
> > + DEF_FIXED("PCLKAH", R9A09G077_CLK_PCLKAH, CLK_PLL4D1, 6, 1),
> > DEF_FIXED("PCLKAM", R9A09G077_CLK_PCLKAM, CLK_PLL4D1, 12, 1),
> > DEF_FIXED("SDHI_CLKHS", R9A09G077_SDHI_CLKHS, CLK_SEL_CLK_PLL2, 1, 1),
> > DEF_FIXED("USB_CLK", R9A09G077_USB_CLK, CLK_PLL4D1, 48, 1),
> > + DEF_FIXED("ETCLKA", R9A09G077_ETCLKA, CLK_SEL_CLK_PLL1, 5, 1),
> > + DEF_FIXED("ETCLKB", R9A09G077_ETCLKB, CLK_SEL_CLK_PLL1, 8, 1),
> > + DEF_FIXED("ETCLKC", R9A09G077_ETCLKC, CLK_SEL_CLK_PLL1, 10, 1),
> > + DEF_FIXED("ETCLKD", R9A09G077_ETCLKD, CLK_SEL_CLK_PLL1, 20, 1),
> > + DEF_FIXED("ETCLKE", R9A09G077_ETCLKE, CLK_SEL_CLK_PLL1, 40, 1),
> > + DEF_FIXED("GMAC0_PCLKH", R9A09G077_GMAC0_PCLKH, R9A09G077_CLK_PCLKH, 1, 1),
> > + DEF_FIXED("GMAC1_PCLKH", R9A09G077_GMAC1_PCLKAH, R9A09G077_CLK_PCLKAH, 1, 1),
> > + DEF_FIXED("GMAC2_PCLKH", R9A09G077_GMAC2_PCLKAH, R9A09G077_CLK_PCLKAH, 1, 1),
>
> Do you need these? I can't seem to find them in the documentation,
> so they are not just for aiding the casual reader. As their
> multipliers/dividers are 1/1, you can just use R9A09G077_CLK_PCLKH
> resp. R9A09G077_CLK_PCLKAH in the DTS?
>
Agreed, we can get rid of these and just use R9A09G077_CLK_PCLK{A}H in
the SoC DTSI.
> > };
> >
> > static const struct mssr_mod_clk r9a09g077_mod_clks[] __initconst = {
> > @@ -181,7 +191,12 @@ static const struct mssr_mod_clk r9a09g077_mod_clks[] __initconst = {
> > DEF_MOD("sci4fck", 12, CLK_SCI4ASYNC),
> > DEF_MOD("iic0", 100, R9A09G077_CLK_PCLKL),
> > DEF_MOD("iic1", 101, R9A09G077_CLK_PCLKL),
> > + DEF_MOD("gmac0", 400, R9A09G077_CLK_PCLKM),
> > + DEF_MOD("ethsw", 401, R9A09G077_CLK_PCLKM),
>
> According to Table 7.13 ("Overview of Clock Generation Circuit
> Specifications (Internal Clock)"), ETCLKA is used as the operating
> clock for ETHSW?
>
There are 3 clock inputs to ETHSW,
- PCLKM - bus clock
- ETCLKA - operating clock
- ETCLKB - Ts clock
Based on Documentation/devicetree/bindings/net/dsa/renesas,rzn1-a5psw.yaml
we have two clocks for RZ/N1 (Ts clock is missing)
- description: AHB clock used for the switch register interface
- description: Switch system clock
- const: hclk
- const: clk
So I was treating,
- hclk -> PCLKM,
- clk -> ETCLKA
- ts -> ETCLKB
Since pclkm is used for register access, I added this entry to the
r9a09g077_mod_clks array as I was under the impression the clocks used
for reg access need to go into this array.
> > + DEF_MOD("ethss", 403, R9A09G077_CLK_PCLKM),
> > DEF_MOD("usb", 408, R9A09G077_CLK_PCLKAM),
> > + DEF_MOD("gmac1", 416, R9A09G077_CLK_PCLKAM),
> > + DEF_MOD("gmac2", 417, R9A09G077_CLK_PCLKAM),
> > DEF_MOD("sci5fck", 600, CLK_SCI5ASYNC),
> > DEF_MOD("iic2", 601, R9A09G077_CLK_PCLKL),
> > DEF_MOD("sdhi0", 1212, R9A09G077_CLK_PCLKAM),
>
> The rest LGTM. But as the full wiring is not clear to me, I guess
> I'll have to wait for the DTS...
>
I'll soon post the Ethernet enabling patches.
Cheers,
Prabhakar
