netdev - Re: [PATCH net-next 04/11] net: stmmac: add stmmac_get_phy_intf

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 for Android: free password hash cracker in your pocket

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

Message-ID: <4f029450-df4f-419d-adb4-493a8ca03e63@bootlin.com>
Date: Tue, 4 Nov 2025 10:07:54 +0100
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
 Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>,
 Fabio Estevam <festevam@...il.com>, imx@...ts.linux.dev,
 Jakub Kicinski <kuba@...nel.org>, Jan Petrous <jan.petrous@....nxp.com>,
 linux-arm-kernel@...ts.infradead.org,
 linux-stm32@...md-mailman.stormreply.com,
 Maxime Coquelin <mcoquelin.stm32@...il.com>, netdev@...r.kernel.org,
 Paolo Abeni <pabeni@...hat.com>,
 Pengutronix Kernel Team <kernel@...gutronix.de>, s32@....com,
 Sascha Hauer <s.hauer@...gutronix.de>, Shawn Guo <shawnguo@...nel.org>
Subject: Re: [PATCH net-next 04/11] net: stmmac: add stmmac_get_phy_intf_sel()



On 04/11/2025 09:49, Russell King (Oracle) wrote:
> On Tue, Nov 04, 2025 at 09:34:31AM +0100, Maxime Chevallier wrote:
>>> +int stmmac_get_phy_intf_sel(phy_interface_t interface)
>>> +{
>>> +	int phy_intf_sel = -EINVAL;
>>> +
>>> +	if (interface == PHY_INTERFACE_MODE_MII ||
>>> +	    interface == PHY_INTERFACE_MODE_GMII)
>>> +		phy_intf_sel = PHY_INTF_SEL_GMII_MII;
>>> +	else if (phy_interface_mode_is_rgmii(interface))
>>> +		phy_intf_sel = PHY_INTF_SEL_RGMII;
>>> +	else if (interface == PHY_INTERFACE_MODE_SGMII)
>>> +		phy_intf_sel = PHY_INTF_SEL_SGMII;
>>> +	else if (interface == PHY_INTERFACE_MODE_RMII)
>>> +		phy_intf_sel = PHY_INTF_SEL_RMII;
>>> +	else if (interface == PHY_INTERFACE_MODE_REVMII)
>>> +		phy_intf_sel = PHY_INTF_SEL_REVMII;
>>> +
>>> +	return phy_intf_sel;
>>> +}
>>> +EXPORT_SYMBOL_GPL(stmmac_get_phy_intf_sel);
>>
>> Nothng wrong with your code, this is out of curiosity.
>>
>> I'm wondering how we are going to support cases like socfpga (and
>> probably some other) where the PHY_INTF_SEL_xxx doesn't directly
>> translate to the phy_interface, i.e.  when you have a PCS or other
>> IP that serialises the MAC interface ?
> 
> It also doesn't differentiate between MII and GMII. That's fine for
> this - this is about producing the configuration for the dwmac's
> phy_intf_sel_i signals. It isn't for configuring the glue hardware
> for any other parameters such as RGMII delays.
>  
>> for socfpga for example, we need to set the PHY_INTF_SEL to GMII_MII
>> when we want to use SGMII / 1000BaseX, but we do set it to RGMII when
>> we need to output RGMII.
> 
> From what I remember for socfpga, you use an external PCS that needs
> GMII. This function doesn't take account of external PCS, and thus
> platform glue that makes use of an external PCS can't implement
> .set_phy_intf_sel() yet.

Makes sense

> As noted, it also doesn't handle TBI (which,
> although we have PHY_INTERFACE_MODE_TBI, Synopsys intended this mode
> to be used to connect to a SerDes for 1000BASE-X.)

That's fine by me, thanks for the clarifications :)

> 
>> Do you have a plan in mind for that ? (maybe a .get_phy_intf_sel() ops ?)
> 
> Yes, there will need to be a way to override this when an external
> PCS is being used. I suspect that all external 1G PCS will use GMII,
> thus we can probably work it out in core code.
> 
> Note, however, that socfpga doesn't use the phy_intf_sel encoding:
> 
> #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_GMII_MII 0x0
> #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RGMII 0x1
> #define SYSMGR_EMACGRP_CTRL_PHYSEL_ENUM_RMII 0x2
> 
> #define PHY_INTF_SEL_GMII_MII   0
> #define PHY_INTF_SEL_RGMII      1
> #define PHY_INTF_SEL_RMII       4
> 
> It's close, but it isn't the phy_intf_sel_i[2:0] signal values.

> 
Yeah :/

I'll give this series a try on dwmac-imx once I get a bit of time then !

thanks,

Maxime