linux-kernel - Re: [PATCH v2 9/9] clk: renesas: r9a09g057: Add clock and reset entries for GBETH0/1

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+V-a8uSB62TSxgcGTaWbfkPsNOmxZmO31jH3TREE6g2HHtcXA@mail.gmail.com>
Date: Tue, 15 Apr 2025 20:24:47 +0100
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Michael Turquette <mturquette@...libre.com>, Stephen Boyd <sboyd@...nel.org>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Magnus Damm <magnus.damm@...il.com>, linux-renesas-soc@...r.kernel.org, 
	linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org, 
	devicetree@...r.kernel.org, Biju Das <biju.das.jz@...renesas.com>, 
	Fabrizio Castro <fabrizio.castro.jz@...esas.com>, 
	Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
Subject: Re: [PATCH v2 9/9] clk: renesas: r9a09g057: Add clock and reset
 entries for GBETH0/1

Hi Geert,

Thank you for the review.

On Tue, Apr 15, 2025 at 3:37 PM Geert Uytterhoeven <geert@...ux-m68k.org> wrote:
>
> Hi Prabhakar,
>
> On Mon, 7 Apr 2025 at 18:52, Prabhakar <prabhakar.csengg@...il.com> wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > Add clock and reset entries for GBETH instances. Include core clocks for
> > PTP, sourced from PLLETH, and add PLLs, dividers, and static mux clocks
> > used as clock sources for the GBETH IP.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
>
> Thanks for your patch!
>
> > --- a/drivers/clk/renesas/r9a09g057-cpg.c
> > +++ b/drivers/clk/renesas/r9a09g057-cpg.c
>
> > @@ -78,6 +87,19 @@ static const struct clk_div_table dtable_2_64[] = {
> >         {0, 0},
> >  };
> >
> > +static const struct clk_div_table dtable_2_100[] = {
> > +       {0, 2},
> > +       {1, 10},
> > +       {2, 100},
> > +       {0, 0},
> > +};
> > +
> > +/* Mux clock tables */
> > +static const char * const smux2_gbe0_rxclk[] = { ".plleth_gbe0", "et0-rxc-rxclk" };
> > +static const char * const smux2_gbe0_txclk[] = { ".plleth_gbe0", "et0-txc-txclk" };
> > +static const char * const smux2_gbe1_rxclk[] = { ".plleth_gbe1", "et1-rxc-rxclk" };
> > +static const char * const smux2_gbe1_txclk[] = { ".plleth_gbe1", "et1-txc-txclk" };
>
> The "et[01]-[rt]xc-[rt]xclk" clocks are not created by this driver.
> IIUIC, they are actually Ethernet PHY signals.
> How is this supposed to work?
>
My intention was to add support for PHY drivers to provide the clocks
and hook them up accordingly. Currently, for the RX clocks, we get a
rate of 0 since they are external.

# cat /sys/kernel/debug/clk/clk_summary | grep eth_0
                gbeth_0_clk_tx_180_i     1   1   0   125000000   0   0
  50000   Y   15c30000.ethernet   tx-180
                gbeth_0_clk_tx_i         1   1   0   125000000   0   0
  50000   Y   15c30000.ethernet   tx
                gbeth_0_clk_ptp_ref_i    1   1   0   125000000   0   0
  50000   Y   15c30000.ethernet   ptp_ref
                gbeth_0_aclk_i           1   1   0   200000000   0   0
  50000   Y   15c30000.ethernet   stmmaceth
                gbeth_0_aclk_csr_i       1   1   0   200000000   0   0
  50000   Y   15c30000.ethernet   pclk
                gbeth_0_clk_rx_180_i     1   1   0   0           0   0
  50000   Y   15c30000.ethernet   rx-180
                gbeth_0_clk_rx_i         1   1   0   0           0   0
  50000   Y   15c30000.ethernet   rx

I haven’t written a prototype yet for the PHY driver to provide the
clocks, but the plan is to get the initial pieces in place and then
extend support for that.

Is my understanding correct that the PHY should provide the clocks? Or
would you suggest a different approach?

Cheers,
Prabhakar