netdev - Re: [PATCH net-next v2 5/5] net: ethernet: cortina: Implement .set

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: <78fb31fb-98a6-4ab5-9896-f1db8ce5d0fd@lunn.ch>
Date: Sat, 11 May 2024 02:11:30 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Linus Walleij <linus.walleij@...aro.org>
Cc: Hans Ulli Kroll <ulli.kroll@...glemail.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net-next v2 5/5] net: ethernet: cortina: Implement
 .set_pauseparam()

On Sat, May 11, 2024 at 12:08:43AM +0200, Linus Walleij wrote:
> The Cortina Gemini ethernet can very well set up TX or RX
> pausing, so add this functionality to the driver in a
> .set_pauseparam() callback.
> 
> Signed-off-by: Linus Walleij <linus.walleij@...aro.org>
> ---
>  drivers/net/ethernet/cortina/gemini.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c
> index d3134db032a2..137242a4977c 100644
> --- a/drivers/net/ethernet/cortina/gemini.c
> +++ b/drivers/net/ethernet/cortina/gemini.c
> @@ -2145,6 +2145,17 @@ static void gmac_get_pauseparam(struct net_device *netdev,
>  	pparam->autoneg = true;
>  }
>  
> +static int gmac_set_pauseparam(struct net_device *netdev,
> +			       struct ethtool_pauseparam *pparam)
> +{
> +	struct phy_device *phydev = netdev->phydev;
> +
> +	gmac_set_flow_control(netdev, pparam->tx_pause, pparam->rx_pause);
> +	phy_set_asym_pause(phydev, pparam->rx_pause, pparam->tx_pause);

This is a bit of an odd implementation. Normally the value of the
pparam.autoneg is used to decide if to directly program the hardware,
or to pass it to phylib.

Also, the adjust link callback probably should know so that it does
not take the autoneg values when auto-neg of pause is disabled.  Or,
as in your first version, don't allow pparam.autoneg == False.

    Andrew