Re: [PATCH net-next v3 6/7] net: usb: lan78xx: Transition get/set_pause to phylink

Maxime Chevallier
2025-03-11T17:31:12Z

Hi Oleksij,

On Mon, 10 Mar 2025 12:57:36 +0100
Oleksij Rempel <o.rempel@pengutronix.de> wrote:

> Replace lan78xx_get_pause and lan78xx_set_pause implementations with
> phylink-based functions. This transition aligns pause parameter handling
> with the phylink API, simplifying the code and improving
> maintainability.
>
> Signed-off-by: Oleksij Rempel <o.rempel@pengutronix.de>
> ---

Some very minor things below :

> drivers/net/usb/lan78xx.c | 51 ++-------------------------------------
> 1 file changed, 2 insertions(+), 49 deletions(-)
>
> diff --git a/drivers/net/usb/lan78xx.c b/drivers/net/usb/lan78xx.c
> index 7107eaa440e5..3aa916a9ee0b 100644
> --- a/drivers/net/usb/lan78xx.c
> +++ b/drivers/net/usb/lan78xx.c
> @@ -1878,63 +1878,16 @@ static void lan78xx_get_pause(struct net_device *net,
> struct ethtool_pauseparam *pause)
> {
> struct lan78xx_net *dev = netdev_priv(net);
> - struct phy_device *phydev = net->phydev;
> - struct ethtool_link_ksettings ecmd;
> -
> - phy_ethtool_ksettings_get(phydev, &ecmd);
> -
> - pause->autoneg = dev->fc_autoneg;
>
> - if (dev->fc_request_control & FLOW_CTRL_TX)
> - pause->tx_pause = 1;
> -
> - if (dev->fc_request_control & FLOW_CTRL_RX)
> - pause->rx_pause = 1;
> + phylink_ethtool_get_pauseparam(dev->phylink, pause);
> }
>
> static int lan78xx_set_pause(struct net_device *net,
> struct ethtool_pauseparam *pause)
> {
> struct lan78xx_net *dev = netdev_priv(net);
> - struct phy_device *phydev = net->phydev;
> - struct ethtool_link_ksettings ecmd;
> - int ret;
> -
> - phy_ethtool_ksettings_get(phydev, &ecmd);
> -
> - if (pause->autoneg && !ecmd.base.autoneg) {
> - ret = -EINVAL;
> - goto exit;
> - }
>
> - dev->fc_request_control = 0;
> - if (pause->rx_pause)
> - dev->fc_request_control |= FLOW_CTRL_RX;
> -
> - if (pause->tx_pause)
> - dev->fc_request_control |= FLOW_CTRL_TX;

Sorry not to have spotted that before, but after that patch you no
longer need dev->fc_request_control, you can get rid of that in
struct lan78xx_net.

Related to other patches (probably patch 1), it also appears you can get
rid of :
- dev->fc_autoneg
- dev->interface
- dev->link_on

Thanks,

Maxime