Ethernet controller in i.MX6*/i.MX7* series do not provide EEE support.
But this chips are used sometimes in combinations with SmartEEE capable
PHYs.
So, instead of aborting get/set_eee access on MACs without EEE support,
ask PHY if it is able to do the EEE job by using SmartEEE.
Signed-off-by: Oleksij Rempel <[email protected]>
---
drivers/net/ethernet/freescale/fec_main.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index e6238e53940d..25a2a9d860de 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -3102,8 +3102,15 @@ fec_enet_get_eee(struct net_device *ndev, struct ethtool_eee *edata)
struct fec_enet_private *fep = netdev_priv(ndev);
struct ethtool_eee *p = &fep->eee;
- if (!(fep->quirks & FEC_QUIRK_HAS_EEE))
- return -EOPNOTSUPP;
+ if (!(fep->quirks & FEC_QUIRK_HAS_EEE)) {
+ if (!netif_running(ndev))
+ return -ENETDOWN;
+
+ if (!phy_has_smarteee(ndev->phydev))
+ return -EOPNOTSUPP;
+
+ return phy_ethtool_get_eee(ndev->phydev, edata);
+ }
if (!netif_running(ndev))
return -ENETDOWN;
@@ -3123,8 +3130,15 @@ fec_enet_set_eee(struct net_device *ndev, struct ethtool_eee *edata)
struct ethtool_eee *p = &fep->eee;
int ret = 0;
- if (!(fep->quirks & FEC_QUIRK_HAS_EEE))
- return -EOPNOTSUPP;
+ if (!(fep->quirks & FEC_QUIRK_HAS_EEE)) {
+ if (!netif_running(ndev))
+ return -ENETDOWN;
+
+ if (!phy_has_smarteee(ndev->phydev))
+ return -EOPNOTSUPP;
+
+ return phy_ethtool_set_eee(ndev->phydev, edata);
+ }
if (!netif_running(ndev))
return -ENETDOWN;
--
2.30.2
On Mon, Jan 30, 2023 at 09:07:14AM +0100, Oleksij Rempel wrote:
> Ethernet controller in i.MX6*/i.MX7* series do not provide EEE support.
> But this chips are used sometimes in combinations with SmartEEE capable
> PHYs.
> So, instead of aborting get/set_eee access on MACs without EEE support,
> ask PHY if it is able to do the EEE job by using SmartEEE.
>
> Signed-off-by: Oleksij Rempel <[email protected]>
> ---
> drivers/net/ethernet/freescale/fec_main.c | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index e6238e53940d..25a2a9d860de 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -3102,8 +3102,15 @@ fec_enet_get_eee(struct net_device *ndev, struct ethtool_eee *edata)
> struct fec_enet_private *fep = netdev_priv(ndev);
> struct ethtool_eee *p = &fep->eee;
>
> - if (!(fep->quirks & FEC_QUIRK_HAS_EEE))
> - return -EOPNOTSUPP;
> + if (!(fep->quirks & FEC_QUIRK_HAS_EEE)) {
> + if (!netif_running(ndev))
> + return -ENETDOWN;
> +
> + if (!phy_has_smarteee(ndev->phydev))
> + return -EOPNOTSUPP;
> +
> + return phy_ethtool_get_eee(ndev->phydev, edata);
I see many places in the fec driver guarding against a NULL
ndev->phydev, and TBH I don't completely understand why.
I guess it's because ndev->phydev is populated at fec_enet_open() time.
But then again, if the netif_running() check is sufficient to imply
presence of ndev->phydev as you suggest, then why does fec_enet_ioctl()
have this?
if (!netif_running(ndev))
return -EINVAL;
if (!phydev)
return -ENODEV;
Asking because phy_init_eee(), phy_ethtool_set_eee() and
phy_ethtool_get_eee() don't support being called with a NULL phydev.
On Tue, Jan 31, 2023 at 10:52:31PM +0200, Vladimir Oltean wrote:
> On Mon, Jan 30, 2023 at 09:07:14AM +0100, Oleksij Rempel wrote:
> > Ethernet controller in i.MX6*/i.MX7* series do not provide EEE support.
> > But this chips are used sometimes in combinations with SmartEEE capable
> > PHYs.
> > So, instead of aborting get/set_eee access on MACs without EEE support,
> > ask PHY if it is able to do the EEE job by using SmartEEE.
> >
> > Signed-off-by: Oleksij Rempel <[email protected]>
> > ---
> > drivers/net/ethernet/freescale/fec_main.c | 22 ++++++++++++++++++----
> > 1 file changed, 18 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> > index e6238e53940d..25a2a9d860de 100644
> > --- a/drivers/net/ethernet/freescale/fec_main.c
> > +++ b/drivers/net/ethernet/freescale/fec_main.c
> > @@ -3102,8 +3102,15 @@ fec_enet_get_eee(struct net_device *ndev, struct ethtool_eee *edata)
> > struct fec_enet_private *fep = netdev_priv(ndev);
> > struct ethtool_eee *p = &fep->eee;
> >
> > - if (!(fep->quirks & FEC_QUIRK_HAS_EEE))
> > - return -EOPNOTSUPP;
> > + if (!(fep->quirks & FEC_QUIRK_HAS_EEE)) {
> > + if (!netif_running(ndev))
> > + return -ENETDOWN;
> > +
> > + if (!phy_has_smarteee(ndev->phydev))
> > + return -EOPNOTSUPP;
> > +
> > + return phy_ethtool_get_eee(ndev->phydev, edata);
>
> I see many places in the fec driver guarding against a NULL
> ndev->phydev, and TBH I don't completely understand why.
> I guess it's because ndev->phydev is populated at fec_enet_open() time.
>
> But then again, if the netif_running() check is sufficient to imply
> presence of ndev->phydev as you suggest, then why does fec_enet_ioctl()
> have this?
>
> if (!netif_running(ndev))
> return -EINVAL;
>
> if (!phydev)
> return -ENODEV;
>
> Asking because phy_init_eee(), phy_ethtool_set_eee() and
> phy_ethtool_get_eee() don't support being called with a NULL phydev.
Hm..
phy_start() is protected against NULL phydev and it is used in
fec_enet_open().
Right now i do not know what is better way go. Any preferences?
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |