linux-kernel - Re: [RFC PATCH net-next 4/5] net: macb: Configure High Speed Mac for given speed.

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: <20241009083653.3b4ffd6d@device-21.home>
Date: Wed, 9 Oct 2024 08:36:53 +0200
From: Maxime Chevallier <maxime.chevallier@...tlin.com>
To: Vineeth Karumanchi <vineeth.karumanchi@....com>
Cc: <nicolas.ferre@...rochip.com>, <claudiu.beznea@...on.dev>,
 <davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
 <pabeni@...hat.com>, <robh@...nel.org>, <krzk+dt@...nel.org>,
 <conor+dt@...nel.org>, <linux@...linux.org.uk>, <andrew@...n.ch>,
 <netdev@...r.kernel.org>, <devicetree@...r.kernel.org>,
 <linux-kernel@...r.kernel.org>, <git@....com>
Subject: Re: [RFC PATCH net-next 4/5] net: macb: Configure High Speed Mac
 for given speed.

Hello,

On Wed, 9 Oct 2024 11:09:45 +0530
Vineeth Karumanchi <vineeth.karumanchi@....com> wrote:

> HS Mac configuration steps:
> - Configure speed and serdes rate bits of USX_CONTROL register from
>   user specified speed in the device-tree.
> - Enable HS Mac for 5G and 10G speeds.
> - Reset RX receive path to achieve USX block lock for the
>   configured serdes rate.
> - Wait for USX block lock synchronization.
> 
> Move the initialization instances to macb_usx_pcs_link_up().
> 
> Signed-off-by: Vineeth Karumanchi <vineeth.karumanchi@....com>

[...]

>  
>  /* DMA buffer descriptor might be different size
>   * depends on hardware configuration:
> @@ -564,14 +565,59 @@ static void macb_usx_pcs_link_up(struct phylink_pcs *pcs, unsigned int neg_mode,
>  				 int duplex)
>  {
>  	struct macb *bp = container_of(pcs, struct macb, phylink_usx_pcs);
> -	u32 config;
> +	u32 speed_val, serdes_rate, config;
> +	bool hs_mac = false;
> +
> +	switch (speed) {
> +	case SPEED_1000:
> +		speed_val = HS_SPEED_1000M;
> +		serdes_rate = MACB_SERDES_RATE_1G;
> +		break;
> +	case SPEED_2500:
> +		speed_val = HS_SPEED_2500M;
> +		serdes_rate = MACB_SERDES_RATE_2_5G;
> +		break;
> +	case SPEED_5000:
> +		speed_val = HS_SPEED_5000M;
> +		serdes_rate = MACB_SERDES_RATE_5G;
> +		hs_mac = true;
> +		break;

You support some new speeds and modes, so you also need to update :

 - The macb_select_pcs() code, as right now it will return NULL for any
mode that isn't 10GBaseR or SGMII, so for 2500/5000 speeds, that
probably won't work. And for 1000, the default PCS will be used and not
USX

 - the phylink mac_capabilities, so far 2500 and 5000 speeds aren't
reported as supported.

 - the phylink supported_interfaces, I suppose the IP uses 2500BaseX
and 5GBaseT ? or maybe some usxgmii flavors ?

> +	case SPEED_10000:
> +		speed_val = HS_SPEED_10000M;
> +		serdes_rate = MACB_SERDES_RATE_10G;
> +		hs_mac = true;
> +		break;
> +	default:
> +		netdev_err(bp->dev, "Specified speed not supported\n");
> +		return;
> +	}
> +
> +	/* Enable HS MAC for high speeds */
> +	if (hs_mac) {
> +		config = macb_or_gem_readl(bp, NCR);
> +		config |= GEM_BIT(ENABLE_HS_MAC);
> +		macb_or_gem_writel(bp, NCR, config);
> +	}

It looks like you moved the MAC selection between HS MAC and non-HS MAC
from the phylink .mac_config to PCS config.

This configuration is indeed a MAC-side configuration from what I
understand, you shouldn't need to set that in PCS code. Maybe instead,
check the interface mode in macb_mac_config, and look if you're in
5GBaseR / 10GBaseR to select the MAC ?

Thanks,

Maxime