netdev - Re: [PATCH net-next v2 2/7] net: phy: add rxc_always_on flag to phylink

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: <3e1617d8-b8c9-64d6-030d-5b6781544692@bootlin.com>
Date: Tue, 30 Jan 2024 14:40:31 +0100 (CET)
From: Romain Gantois <romain.gantois@...tlin.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
cc: Romain Gantois <romain.gantois@...tlin.com>, Andrew Lunn <andrew@...n.ch>, 
    Heiner Kallweit <hkallweit1@...il.com>, 
    "David S. Miller" <davem@...emloft.net>, 
    Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>, 
    Paolo Abeni <pabeni@...hat.com>, 
    Alexandre Torgue <alexandre.torgue@...s.st.com>, 
    Jose Abreu <joabreu@...opsys.com>, 
    Maxime Coquelin <mcoquelin.stm32@...il.com>, 
    Clément Léger <clement.leger@...tlin.com>, 
    Maxime Chevallier <maxime.chevallier@...tlin.com>, 
    Miquel Raynal <miquel.raynal@...tlin.com>, 
    Thomas Petazzoni <thomas.petazzoni@...tlin.com>, netdev@...r.kernel.org, 
    linux-stm32@...md-mailman.stormreply.com, 
    linux-arm-kernel@...ts.infradead.org, linux-renesas-soc@...r.kernel.org
Subject: Re: [PATCH net-next v2 2/7] net: phy: add rxc_always_on flag to
 phylink_pcs

Hello Russell,

On Tue, 30 Jan 2024, Russell King (Oracle) wrote:
...
> > +int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs)
> > +{
> > +	int ret = 0;
> > +
> > +	/* Signal to PCS driver that MAC requires RX clock for init */
> > +	if (pl->config->mac_requires_rxc)
> > +		pcs->rxc_always_on = true;
> > +
> > +	if (pcs->ops->pcs_pre_init)
> > +		ret = pcs->ops->pcs_pre_init(pcs, pl->link_config.interface);
> 
> Given that:
> 1) phylink supports switching between mutliple different interfaces,
> 2) from what I can see you are only calling this from stmmac's
>    initialisation path,
> 3) you pass the interface mode to the PCS here
> 
> then we don't want the PCS to configure itself for the interface mode
> passed in, because this function won't be called when the interface
> mode changes - and PCS driver authors will have to bear that in mind.
> So...
> 
...
> However, do we really need it - if the PCS is supplying the RXC to
> the MAC, then is the interface mode between the PCS and PHY all that
> relevant at this point?

If a PCS can set the needed clock signal without configuring the details 
of a particular link mode, then passing the interface mode to pcs_pre_init() 
would indeed not be relevant. Generally, I agree that setting the interface mode 
shouldn't be the concern of the pre-initialization function. I'll dig a bit 
more into the PCS datasheet and run more tests to see if I can get away with 
enabling the RX clock selectively for this particular PCS model. If not, then 
maybe I can hardcode a "default" interface mode for the pre-initialization that 
will not interfere with the rest of the link setup process.

Best Regards,

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com