netdev - Re: [net-next PATCH v4 09/11] net: macb: Move most of mac_config to mac

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: <6a8f1a28-29c0-4a8b-b3c2-d746a3b57950@amd.com>
Date: Tue, 13 May 2025 20:59:02 +0530
From: "Karumanchi, Vineeth" <vineeth.karumanchi@....com>
To: Sean Anderson <sean.anderson@...ux.dev>, netdev@...r.kernel.org,
 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>,
 Russell King <linux@...linux.org.uk>
Cc: upstream@...oha.com, Simon Horman <horms@...nel.org>,
 Heiner Kallweit <hkallweit1@...il.com>,
 Kory Maincent <kory.maincent@...tlin.com>, linux-kernel@...r.kernel.org,
 Christian Marangi <ansuelsmth@...il.com>,
 Claudiu Beznea <claudiu.beznea@...rochip.com>,
 Nicolas Ferre <nicolas.ferre@...rochip.com>
Subject: Re: [net-next PATCH v4 09/11] net: macb: Move most of mac_config to
 mac_prepare

Hi Sean,

Sorry for the delayed response.

We are working on MACB with two internal PCS's (10G-BASER, 1000-BASEX) 
supporting 1G, 2.5G, 5G, and 10G with AN disabled.

I have sent an initial RFC : 
https://lore.kernel.org/netdev/20241009053946.3198805-1-vineeth.karumanchi@amd.com/

Currently, we are working on integrating the MAC in fixed-link and 
phy-mode.

Please see my inline comments.

On 5/12/2025 9:44 PM, Sean Anderson wrote:
> mac_prepare is called every time the interface is changed, so we can do
> all of our configuration there, instead of in mac_config. This will be
> useful for the next patch where we will set the PCS bit based on whether
> we are using our internal PCS. No functional change intended.
> 
> Signed-off-by: Sean Anderson <sean.anderson@...ux.dev>
> ---
> 

<...>

> +static int macb_pcs_config(struct phylink_pcs *pcs, unsigned int mode,
> +			   phy_interface_t interface,
> +			   const unsigned long *advertising,
> +			   bool permit_pause_to_mac)
> +{
> +	struct macb *bp = container_of(pcs, struct macb, phylink_sgmii_pcs);
> +	bool changed = false;
> +	unsigned long flags;
> +	u32 old, new;
> +
> +	spin_lock_irqsave(&bp->lock, flags);
> +	old = new = gem_readl(bp, NCFGR);
> +	new |= GEM_BIT(SGMIIEN);

This bit represents the AN feature, can we make it conditional to 
facilitate IP's with AN disabled.

> +	if (old != new) {
> +		changed = true;
> +		gem_writel(bp, NCFGR, new);
> +	}

<..>

>   
>   static void macb_usx_pcs_get_state(struct phylink_pcs *pcs,
> @@ -589,45 +661,60 @@ static int macb_usx_pcs_config(struct phylink_pcs *pcs,
>   			       bool permit_pause_to_mac)
>   {
>   	struct macb *bp = container_of(pcs, struct macb, phylink_usx_pcs);
> +	unsigned long flags;
> +	bool changed;
> +	u16 old, new;
>   
> -	gem_writel(bp, USX_CONTROL, gem_readl(bp, USX_CONTROL) |
> -		   GEM_BIT(SIGNAL_OK));
> +	spin_lock_irqsave(&bp->lock, flags);
> +	if (macb_pcs_config_an(bp, neg_mode, interface, advertising))
> +		changed = true;
>   
> -	return 0;
> -}
> +	old = new = gem_readl(bp, USX_CONTROL);
> +	new |= GEM_BIT(SIGNAL_OK);
> +	if (old != new) {
> +		changed = true;
> +		gem_writel(bp, USX_CONTROL, new);
> +	}
>   
> -static void macb_pcs_get_state(struct phylink_pcs *pcs, unsigned int neg_mode,
> -			       struct phylink_link_state *state)
> -{
> -	state->link = 0;
> -}
> +	old = new = gem_readl(bp, USX_CONTROL);
> +	new = GEM_BFINS(SERDES_RATE, MACB_SERDES_RATE_10G, new);
> +	new = GEM_BFINS(USX_CTRL_SPEED, HS_SPEED_10000M, new);
> +	new &= ~(GEM_BIT(TX_SCR_BYPASS) | GEM_BIT(RX_SCR_BYPASS));
> +	new |= GEM_BIT(TX_EN);
> +	if (old != new) {
> +		changed = true;
> +		gem_writel(bp, USX_CONTROL, new);
> +	}

The above speed/rate configuration was moved from macb_usx_pcs_link_up() 
where speed is an argument, which can be leveraged to configure multiple 
speeds.

Can we achieve configuring for multiple speeds from 
macb_usx_pcs_config() in fixed-link and phy-mode ?

-- 
🙏 vineeth