linux-kernel - Re: [PATCH net-next v2 3/3] net: stmmac: Add DWMAC glue layer for Renesas GBETH

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-a8stuYLJMA5UEKpyLpH1kcgEvA=b5BzUOEaCKcfNtdSSfg@mail.gmail.com>
Date: Sun, 9 Mar 2025 11:24:57 +0000
From: "Lad, Prabhakar" <prabhakar.csengg@...il.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>, 
	Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Philipp Zabel <p.zabel@...gutronix.de>, Geert Uytterhoeven <geert+renesas@...der.be>, 
	Giuseppe Cavallaro <peppe.cavallaro@...com>, Jose Abreu <joabreu@...opsys.com>, 
	Alexandre Torgue <alexandre.torgue@...s.st.com>, netdev@...r.kernel.org, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-renesas-soc@...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 net-next v2 3/3] net: stmmac: Add DWMAC glue layer for
 Renesas GBETH

Hi Russell,

Thank you for the review.

On Sun, Mar 9, 2025 at 8:50 AM Russell King (Oracle)
<linux@...linux.org.uk> wrote:
>
> On Sat, Mar 08, 2025 at 08:09:21PM +0000, Prabhakar wrote:
> > From: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> >
> > Add the DWMAC glue layer for the GBETH IP found in the Renesas RZ/V2H(P)
> > SoC.
> >
> > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@...renesas.com>
> > ---
> > v1->v2
> > - Dropped __initconst for renesas_gbeth_clks array
> > - Added clks_config callback
> > - Dropped STMMAC_FLAG_RX_CLK_RUNS_IN_LPI flag as this needs
> >   investigation.
>
> I thought you had got to the bottom of this, and it was a bug in your
> clock driver?
>
I have added a fix in the clock driver to ignore CLK_MON bits for
external clocks. The main reason for dropping this flag was despite
trying the below i.e. adding phy_eee_rx_clock_stop() just before
unregister_netdev() in stmmac_dvr_remove() still doesnt stop the Rx
clocks.

         if (ndev->phydev)
                 phy_eee_rx_clock_stop(ndev->phydev, false);

Note, on another platform where I can issue a reset to the PHY I
issued the reset after unbind operation and monitored the Rx clock
using CLK_MON and I noticed they reported  Rx clocks were OFF. But on
the current platform I cannot issue a reset to the PHY after unbind
operation.

> > + * The Rx and Tx clocks are supplied as follows for the GBETH IP.
> > + *
> > + *                         Rx / Tx
> > + *   -------+------------- on / off -------
> > + *          |
> > + *          |            Rx-180 / Tx-180
> > + *          +---- not ---- on / off -------
>
> Thanks for the diagram.
>
> > +struct renesas_gbeth {
> > +     struct device *dev;
> > +     void __iomem *regs;
> > +     unsigned int num_clks;
> > +     struct clk *clk_tx_i;
> > +     struct clk_bulk_data *clks;
> > +     struct reset_control *rstc;
> > +};
>
> If you stored a pointer to struct plat_stmmacenet_data, then you
> wouldn't need num_clks, clk_tx_i or clks. If you look at
> dwmac-dwc-qos-eth.c, I recently added a helper (dwc_eth_find_clk())
> which could be made generic.
>
> You can then include the clk_tx_i clock in the bulk clock, and
> use the helper to set plat_dat->clk_tx_i.
>
Thanks for the pointer, I'll switch to that.

> > +     plat_dat->flags |= STMMAC_FLAG_HWTSTAMP_CORRECT_LATENCY |
> > +                        STMMAC_FLAG_EN_TX_LPI_CLOCKGATING |
>
> Didn't I send you a patch that provides
> STMMAC_FLAG_EN_TX_LPI_CLK_PHY_CAP so we can move towards the PHY
> saying whether it permits the TX clock to be disabled?
>
I'll rebase my changes on top of [0]. Do you want me to run any
specific tests for this?

[0] https://patchwork.kernel.org/project/netdevbpf/patch/E1trCPy-005jZf-Ou@rmk-PC.armlinux.org.uk/

Cheers,
Prabhakar