[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <BL0PR11MB29139867F521F90347B6904EE7DA2@BL0PR11MB2913.namprd11.prod.outlook.com>
Date: Mon, 8 Jul 2024 16:31:53 +0000
From: <Woojung.Huh@...rochip.com>
To: <o.rempel@...gutronix.de>, <mkubecek@...e.cz>, <davem@...emloft.net>,
<edumazet@...gle.com>, <kuba@...nel.org>, <pabeni@...hat.com>,
<jiri@...nulli.us>, <vladimir.oltean@....com>, <andrew@...n.ch>,
<Arun.Ramadoss@...rochip.com>
CC: <kernel@...gutronix.de>, <netdev@...r.kernel.org>,
<UNGLinuxDriver@...rochip.com>, <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH net v2 1/1] ethtool: netlink: do not return SQI value if
link is down
Hi Oleksij,
> diff --git a/net/ethtool/linkstate.c b/net/ethtool/linkstate.c
> index b2de2108b356a..4efd327ba5d92 100644
> --- a/net/ethtool/linkstate.c
> +++ b/net/ethtool/linkstate.c
> @@ -37,6 +37,8 @@ static int linkstate_get_sqi(struct net_device *dev)
> mutex_lock(&phydev->lock);
> if (!phydev->drv || !phydev->drv->get_sqi)
> ret = -EOPNOTSUPP;
> + else if (!phydev->link)
> + ret = -ENETDOWN;
> else
> ret = phydev->drv->get_sqi(phydev);
> mutex_unlock(&phydev->lock);
> @@ -55,6 +57,8 @@ static int linkstate_get_sqi_max(struct net_device *dev)
> mutex_lock(&phydev->lock);
> if (!phydev->drv || !phydev->drv->get_sqi_max)
> ret = -EOPNOTSUPP;
> + else if (!phydev->link)
> + ret = -ENETDOWN;
> else
> ret = phydev->drv->get_sqi_max(phydev);
> mutex_unlock(&phydev->lock);
> @@ -62,6 +66,16 @@ static int linkstate_get_sqi_max(struct net_device *dev)
> return ret;
> };
>
> +static bool linkstate_sqi_critical_error(int sqi)
> +{
> + return sqi < 0 && sqi != -EOPNOTSUPP && sqi != -ENETDOWN;
> +}
> +
> +static bool linkstate_sqi_valid(struct linkstate_reply_data *data)
> +{
> + return data->sqi >= 0 && data->sqi_max >= 0;
If PHY driver has get_sqi, but not get_sqi_max, then data->sqi could have
a valid value, but data->sqi_max will have -EOPNOTSUPP.
In this case, linkstate_sqi_valid() will return FALSE and not getting
SQI value at all.
If both APIs are required, then we could add another condition of
data->sqi <= data->sqi_max in linkstate_sqi_valid()
And, beside this, calling linkstate_get_sqi and linkstate_get_sqi_max
could be moved under "if (dev->flags & IFF_UP)" with setting default
value to data->sqi & data->sqi_max.
> +}
> +
Thanks.
Woojung
Powered by blists - more mailing lists