Add support to ethtool phylink functions:
- get/set settings like speed, duplex etc
- get/set the wake-on-lan (WOL)
- get/set the energy-efficient ethernet (EEE)
- get/set the pause
Signed-off-by: Raju Lakkaraju <Raju.Lakkaraju@microchip.com>
---
Change List:
============
V2 -> V3:
- No change
V1 -> V2:
- Fix the phylink changes
.../net/ethernet/microchip/lan743x_ethtool.c | 118 ++++++------------
drivers/net/ethernet/microchip/lan743x_main.c | 25 ++++
drivers/net/ethernet/microchip/lan743x_main.h | 4 +
3 files changed, 67 insertions(+), 80 deletions(-)
diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c
index 3a63ec091413..a649ea7442a4 100644
--- a/drivers/net/ethernet/microchip/lan743x_ethtool.c
+++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c
@@ -1058,61 +1058,48 @@ static int lan743x_ethtool_get_eee(struct net_device *netdev,
struct ethtool_keee *eee)
{
struct lan743x_adapter *adapter = netdev_priv(netdev);
- struct phy_device *phydev = netdev->phydev;
- u32 buf;
- int ret;
-
- if (!phydev)
- return -EIO;
- if (!phydev->drv) {
- netif_err(adapter, drv, adapter->netdev,
- "Missing PHY Driver\n");
- return -EIO;
- }
- ret = phy_ethtool_get_eee(phydev, eee);
- if (ret < 0)
- return ret;
-
- buf = lan743x_csr_read(adapter, MAC_CR);
- if (buf & MAC_CR_EEE_EN_) {
- /* EEE_TX_LPI_REQ_DLY & tx_lpi_timer are same uSec unit */
- buf = lan743x_csr_read(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT);
- eee->tx_lpi_timer = buf;
- } else {
- eee->tx_lpi_timer = 0;
- }
+ eee->tx_lpi_timer = lan743x_csr_read(adapter,
+ MAC_EEE_TX_LPI_REQ_DLY_CNT);
+ eee->eee_enabled = adapter->eee_enabled;
+ eee->eee_active = adapter->eee_active;
+ eee->tx_lpi_enabled = adapter->tx_lpi_enabled;
- return 0;
+ return phylink_ethtool_get_eee(adapter->phylink, eee);
}
static int lan743x_ethtool_set_eee(struct net_device *netdev,
struct ethtool_keee *eee)
{
- struct lan743x_adapter *adapter;
- struct phy_device *phydev;
- u32 buf = 0;
+ struct lan743x_adapter *adapter = netdev_priv(netdev);
- if (!netdev)
- return -EINVAL;
- adapter = netdev_priv(netdev);
- if (!adapter)
- return -EINVAL;
- phydev = netdev->phydev;
- if (!phydev)
- return -EIO;
- if (!phydev->drv) {
- netif_err(adapter, drv, adapter->netdev,
- "Missing PHY Driver\n");
- return -EIO;
- }
+ if (eee->tx_lpi_enabled)
+ lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT,
+ eee->tx_lpi_timer);
+ else
+ lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, 0);
- if (eee->eee_enabled) {
- buf = (u32)eee->tx_lpi_timer;
- lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, buf);
- }
+ adapter->eee_enabled = eee->eee_enabled;
+ adapter->tx_lpi_enabled = eee->tx_lpi_enabled;
+ lan743x_set_eee(adapter, eee->tx_lpi_enabled && eee->eee_enabled);
- return phy_ethtool_set_eee(phydev, eee);
+ return phylink_ethtool_set_eee(adapter->phylink, eee);
+}
+
+static int lan743x_ethtool_set_link_ksettings(struct net_device *netdev,
+ const struct ethtool_link_ksettings *cmd)
+{
+ struct lan743x_adapter *adapter = netdev_priv(netdev);
+
+ return phylink_ethtool_ksettings_set(adapter->phylink, cmd);
+}
+
+static int lan743x_ethtool_get_link_ksettings(struct net_device *netdev,
+ struct ethtool_link_ksettings *cmd)
+{
+ struct lan743x_adapter *adapter = netdev_priv(netdev);
+
+ return phylink_ethtool_ksettings_get(adapter->phylink, cmd);
}
#ifdef CONFIG_PM
@@ -1124,8 +1111,7 @@ static void lan743x_ethtool_get_wol(struct net_device *netdev,
wol->supported = 0;
wol->wolopts = 0;
- if (netdev->phydev)
- phy_ethtool_get_wol(netdev->phydev, wol);
+ phylink_ethtool_get_wol(adapter->phylink, wol);
if (wol->supported != adapter->phy_wol_supported)
netif_warn(adapter, drv, adapter->netdev,
@@ -1166,7 +1152,7 @@ static int lan743x_ethtool_set_wol(struct net_device *netdev,
!(adapter->phy_wol_supported & WAKE_MAGICSECURE))
phy_wol.wolopts &= ~WAKE_MAGIC;
- ret = phy_ethtool_set_wol(netdev->phydev, &phy_wol);
+ ret = phylink_ethtool_set_wol(adapter->phylink, wol);
if (ret && (ret != -EOPNOTSUPP))
return ret;
@@ -1355,44 +1341,16 @@ static void lan743x_get_pauseparam(struct net_device *dev,
struct ethtool_pauseparam *pause)
{
struct lan743x_adapter *adapter = netdev_priv(dev);
- struct lan743x_phy *phy = &adapter->phy;
- if (phy->fc_request_control & FLOW_CTRL_TX)
- pause->tx_pause = 1;
- if (phy->fc_request_control & FLOW_CTRL_RX)
- pause->rx_pause = 1;
- pause->autoneg = phy->fc_autoneg;
+ phylink_ethtool_get_pauseparam(adapter->phylink, pause);
}
static int lan743x_set_pauseparam(struct net_device *dev,
struct ethtool_pauseparam *pause)
{
struct lan743x_adapter *adapter = netdev_priv(dev);
- struct phy_device *phydev = dev->phydev;
- struct lan743x_phy *phy = &adapter->phy;
- if (!phydev)
- return -ENODEV;
-
- if (!phy_validate_pause(phydev, pause))
- return -EINVAL;
-
- phy->fc_request_control = 0;
- if (pause->rx_pause)
- phy->fc_request_control |= FLOW_CTRL_RX;
-
- if (pause->tx_pause)
- phy->fc_request_control |= FLOW_CTRL_TX;
-
- phy->fc_autoneg = pause->autoneg;
-
- if (pause->autoneg == AUTONEG_DISABLE)
- lan743x_mac_flow_ctrl_set_enables(adapter, pause->tx_pause,
- pause->rx_pause);
- else
- phy_set_asym_pause(phydev, pause->rx_pause, pause->tx_pause);
-
- return 0;
+ return phylink_ethtool_set_pauseparam(adapter->phylink, pause);
}
const struct ethtool_ops lan743x_ethtool_ops = {
@@ -1417,8 +1375,8 @@ const struct ethtool_ops lan743x_ethtool_ops = {
.get_ts_info = lan743x_ethtool_get_ts_info,
.get_eee = lan743x_ethtool_get_eee,
.set_eee = lan743x_ethtool_set_eee,
- .get_link_ksettings = phy_ethtool_get_link_ksettings,
- .set_link_ksettings = phy_ethtool_set_link_ksettings,
+ .get_link_ksettings = lan743x_ethtool_get_link_ksettings,
+ .set_link_ksettings = lan743x_ethtool_set_link_ksettings,
.get_regs_len = lan743x_get_regs_len,
.get_regs = lan743x_get_regs,
.get_pauseparam = lan743x_get_pauseparam,
diff --git a/drivers/net/ethernet/microchip/lan743x_main.c b/drivers/net/ethernet/microchip/lan743x_main.c
index 845a48ca497d..dc46686529a2 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.c
+++ b/drivers/net/ethernet/microchip/lan743x_main.c
@@ -2966,6 +2966,18 @@ static int lan743x_phylink_2500basex_config(struct lan743x_adapter *adapter)
return lan743x_pcs_power_reset(adapter);
}
+void lan743x_set_eee(struct lan743x_adapter *adapter, bool enable)
+{
+ u32 lpi_ctl1;
+
+ lpi_ctl1 = lan743x_csr_read(adapter, MAC_CR);
+ if (enable)
+ lpi_ctl1 |= MAC_CR_EEE_EN_;
+ else
+ lpi_ctl1 &= ~MAC_CR_EEE_EN_;
+ lan743x_csr_write(adapter, MAC_CR, lpi_ctl1);
+}
+
static void lan743x_phylink_mac_config(struct phylink_config *config,
unsigned int link_an_mode,
const struct phylink_link_state *state)
@@ -3013,7 +3025,12 @@ static void lan743x_phylink_mac_link_down(struct phylink_config *config,
unsigned int link_an_mode,
phy_interface_t interface)
{
+ struct net_device *netdev = to_net_dev(config->dev);
+ struct lan743x_adapter *adapter = netdev_priv(netdev);
+
netif_tx_stop_all_queues(to_net_dev(config->dev));
+ adapter->eee_active = false;
+ lan743x_set_eee(adapter, false);
}
static void lan743x_phylink_mac_link_up(struct phylink_config *config,
@@ -3055,6 +3072,14 @@ static void lan743x_phylink_mac_link_up(struct phylink_config *config,
cap & FLOW_CTRL_TX,
cap & FLOW_CTRL_RX);
+ if (phydev && adapter->eee_enabled) {
+ bool enable;
+
+ adapter->eee_active = phy_init_eee(phydev, false) >= 0;
+ enable = adapter->eee_active && adapter->tx_lpi_enabled;
+ lan743x_set_eee(adapter, enable);
+ }
+
netif_tx_wake_all_queues(netdev);
}
diff --git a/drivers/net/ethernet/microchip/lan743x_main.h b/drivers/net/ethernet/microchip/lan743x_main.h
index 7f73d66854be..79f21789eb32 100644
--- a/drivers/net/ethernet/microchip/lan743x_main.h
+++ b/drivers/net/ethernet/microchip/lan743x_main.h
@@ -1086,6 +1086,9 @@ struct lan743x_adapter {
phy_interface_t phy_interface;
struct phylink *phylink;
struct phylink_config phylink_config;
+ bool eee_enabled;
+ bool eee_active;
+ bool tx_lpi_enabled;
};
#define LAN743X_COMPONENT_FLAG_RX(channel) BIT(20 + (channel))
@@ -1206,5 +1209,6 @@ void lan743x_hs_syslock_release(struct lan743x_adapter *adapter);
void lan743x_mac_flow_ctrl_set_enables(struct lan743x_adapter *adapter,
bool tx_enable, bool rx_enable);
int lan743x_sgmii_read(struct lan743x_adapter *adapter, u8 mmd, u16 addr);
+void lan743x_set_eee(struct lan743x_adapter *adapter, bool enable);
#endif /* _LAN743X_H */
--
2.34.1On Tue, Jul 30, 2024 at 07:36:19PM +0530, Raju Lakkaraju wrote:
> diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c
> index 3a63ec091413..a649ea7442a4 100644
> --- a/drivers/net/ethernet/microchip/lan743x_ethtool.c
> +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c
> @@ -1058,61 +1058,48 @@ static int lan743x_ethtool_get_eee(struct net_device *netdev,
> struct ethtool_keee *eee)
> {
> struct lan743x_adapter *adapter = netdev_priv(netdev);
> - struct phy_device *phydev = netdev->phydev;
> - u32 buf;
> - int ret;
> -
> - if (!phydev)
> - return -EIO;
> - if (!phydev->drv) {
> - netif_err(adapter, drv, adapter->netdev,
> - "Missing PHY Driver\n");
> - return -EIO;
> - }
>
> - ret = phy_ethtool_get_eee(phydev, eee);
> - if (ret < 0)
> - return ret;
> -
> - buf = lan743x_csr_read(adapter, MAC_CR);
> - if (buf & MAC_CR_EEE_EN_) {
> - /* EEE_TX_LPI_REQ_DLY & tx_lpi_timer are same uSec unit */
> - buf = lan743x_csr_read(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT);
> - eee->tx_lpi_timer = buf;
> - } else {
> - eee->tx_lpi_timer = 0;
> - }
> + eee->tx_lpi_timer = lan743x_csr_read(adapter,
> + MAC_EEE_TX_LPI_REQ_DLY_CNT);
> + eee->eee_enabled = adapter->eee_enabled;
> + eee->eee_active = adapter->eee_active;
> + eee->tx_lpi_enabled = adapter->tx_lpi_enabled;
You really need to start paying attention to the commits other people
make as a result of development to other parts of the kernel.
First, see:
commit ef460a8986fa0dae1cdcb158a06127f7af27c92d
Author: Andrew Lunn <andrew@lunn.ch>
Date: Sat Apr 6 15:16:00 2024 -0500
net: lan743x: Fixup EEE
and note that the assignment of eee->eee_enabled, eee->eee_active, and
eee->tx_lpi_enabled were all removed.
Next...
>
> - return 0;
> + return phylink_ethtool_get_eee(adapter->phylink, eee);
phylink_ethtool_get_eee() will call phy_ethtool_get_eee(), which
in turn calls genphy_c45_ethtool_get_eee() and eeecfg_to_eee().
genphy_c45_ethtool_get_eee() will do this:
data->eee_enabled = is_enabled;
data->eee_active = ret;
thus overwriting your assignment to eee->eee_enabled and
eee->eee_active.
eeecfg_to_eee() will overwrite eee->tx_lpi_enabled and
eee->eee_enabled.
Thus, writing to these fields and then calling
phylink_ethtool_get_eee() results in these fields being overwritten.
> static int lan743x_ethtool_set_eee(struct net_device *netdev,
> struct ethtool_keee *eee)
> {
> - struct lan743x_adapter *adapter;
> - struct phy_device *phydev;
> - u32 buf = 0;
> + struct lan743x_adapter *adapter = netdev_priv(netdev);
>
> - if (!netdev)
> - return -EINVAL;
> - adapter = netdev_priv(netdev);
> - if (!adapter)
> - return -EINVAL;
> - phydev = netdev->phydev;
> - if (!phydev)
> - return -EIO;
> - if (!phydev->drv) {
> - netif_err(adapter, drv, adapter->netdev,
> - "Missing PHY Driver\n");
> - return -EIO;
> - }
> + if (eee->tx_lpi_enabled)
> + lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT,
> + eee->tx_lpi_timer);
> + else
> + lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, 0);
>
> - if (eee->eee_enabled) {
> - buf = (u32)eee->tx_lpi_timer;
> - lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, buf);
> - }
> + adapter->eee_enabled = eee->eee_enabled;
> + adapter->tx_lpi_enabled = eee->tx_lpi_enabled;
Given that phylib stores these and overwrites your copies in the get_eee
method above, do you need to store these?
> @@ -3013,7 +3025,12 @@ static void lan743x_phylink_mac_link_down(struct phylink_config *config,
> unsigned int link_an_mode,
> phy_interface_t interface)
> {
> + struct net_device *netdev = to_net_dev(config->dev);
> + struct lan743x_adapter *adapter = netdev_priv(netdev);
> +
> netif_tx_stop_all_queues(to_net_dev(config->dev));
> + adapter->eee_active = false;
phylib tracks this for you.
> + lan743x_set_eee(adapter, false);
> }
>
> static void lan743x_phylink_mac_link_up(struct phylink_config *config,
> @@ -3055,6 +3072,14 @@ static void lan743x_phylink_mac_link_up(struct phylink_config *config,
> cap & FLOW_CTRL_TX,
> cap & FLOW_CTRL_RX);
>
> + if (phydev && adapter->eee_enabled) {
> + bool enable;
> +
> + adapter->eee_active = phy_init_eee(phydev, false) >= 0;
> + enable = adapter->eee_active && adapter->tx_lpi_enabled;
No need. Your enable should be conditional on phydev->enable_tx_lpi
here. See Andrew's commit and understand his changes, rather than
just ignoring Andrew's work and continuing with your old pattern of
EEE support. Things have moved forward.
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell King,
The 07/30/2024 16:11, Russell King (Oracle) wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
>
> On Tue, Jul 30, 2024 at 07:36:19PM +0530, Raju Lakkaraju wrote:
> > diff --git a/drivers/net/ethernet/microchip/lan743x_ethtool.c b/drivers/net/ethernet/microchip/lan743x_ethtool.c
> > index 3a63ec091413..a649ea7442a4 100644
> > --- a/drivers/net/ethernet/microchip/lan743x_ethtool.c
> > +++ b/drivers/net/ethernet/microchip/lan743x_ethtool.c
> > @@ -1058,61 +1058,48 @@ static int lan743x_ethtool_get_eee(struct net_device *netdev,
> > struct ethtool_keee *eee)
> > {
> > struct lan743x_adapter *adapter = netdev_priv(netdev);
> > - struct phy_device *phydev = netdev->phydev;
> > - u32 buf;
> > - int ret;
> > -
> > - if (!phydev)
> > - return -EIO;
> > - if (!phydev->drv) {
> > - netif_err(adapter, drv, adapter->netdev,
> > - "Missing PHY Driver\n");
> > - return -EIO;
> > - }
> >
> > - ret = phy_ethtool_get_eee(phydev, eee);
> > - if (ret < 0)
> > - return ret;
> > -
> > - buf = lan743x_csr_read(adapter, MAC_CR);
> > - if (buf & MAC_CR_EEE_EN_) {
> > - /* EEE_TX_LPI_REQ_DLY & tx_lpi_timer are same uSec unit */
> > - buf = lan743x_csr_read(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT);
> > - eee->tx_lpi_timer = buf;
> > - } else {
> > - eee->tx_lpi_timer = 0;
> > - }
> > + eee->tx_lpi_timer = lan743x_csr_read(adapter,
> > + MAC_EEE_TX_LPI_REQ_DLY_CNT);
> > + eee->eee_enabled = adapter->eee_enabled;
> > + eee->eee_active = adapter->eee_active;
> > + eee->tx_lpi_enabled = adapter->tx_lpi_enabled;
>
> You really need to start paying attention to the commits other people
> make as a result of development to other parts of the kernel.
>
> First, see:
>
> commit ef460a8986fa0dae1cdcb158a06127f7af27c92d
> Author: Andrew Lunn <andrew@lunn.ch>
> Date: Sat Apr 6 15:16:00 2024 -0500
>
> net: lan743x: Fixup EEE
>
> and note that the assignment of eee->eee_enabled, eee->eee_active, and
> eee->tx_lpi_enabled were all removed.
>
Accepted. I will fix
> Next...
>
> >
> > - return 0;
> > + return phylink_ethtool_get_eee(adapter->phylink, eee);
>
> phylink_ethtool_get_eee() will call phy_ethtool_get_eee(), which
> in turn calls genphy_c45_ethtool_get_eee() and eeecfg_to_eee().
>
> genphy_c45_ethtool_get_eee() will do this:
>
> data->eee_enabled = is_enabled;
> data->eee_active = ret;
>
> thus overwriting your assignment to eee->eee_enabled and
> eee->eee_active.
>
> eeecfg_to_eee() will overwrite eee->tx_lpi_enabled and
> eee->eee_enabled.
>
> Thus, writing to these fields and then calling
> phylink_ethtool_get_eee() results in these fields being overwritten.
>
Ok. I will fix.
> > static int lan743x_ethtool_set_eee(struct net_device *netdev,
> > struct ethtool_keee *eee)
> > {
> > - struct lan743x_adapter *adapter;
> > - struct phy_device *phydev;
> > - u32 buf = 0;
> > + struct lan743x_adapter *adapter = netdev_priv(netdev);
> >
> > - if (!netdev)
> > - return -EINVAL;
> > - adapter = netdev_priv(netdev);
> > - if (!adapter)
> > - return -EINVAL;
> > - phydev = netdev->phydev;
> > - if (!phydev)
> > - return -EIO;
> > - if (!phydev->drv) {
> > - netif_err(adapter, drv, adapter->netdev,
> > - "Missing PHY Driver\n");
> > - return -EIO;
> > - }
> > + if (eee->tx_lpi_enabled)
> > + lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT,
> > + eee->tx_lpi_timer);
> > + else
> > + lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, 0);
> >
> > - if (eee->eee_enabled) {
> > - buf = (u32)eee->tx_lpi_timer;
> > - lan743x_csr_write(adapter, MAC_EEE_TX_LPI_REQ_DLY_CNT, buf);
> > - }
> > + adapter->eee_enabled = eee->eee_enabled;
> > + adapter->tx_lpi_enabled = eee->tx_lpi_enabled;
>
> Given that phylib stores these and overwrites your copies in the get_eee
> method above, do you need to store these?
>
OK
> > @@ -3013,7 +3025,12 @@ static void lan743x_phylink_mac_link_down(struct phylink_config *config,
> > unsigned int link_an_mode,
> > phy_interface_t interface)
> > {
> > + struct net_device *netdev = to_net_dev(config->dev);
> > + struct lan743x_adapter *adapter = netdev_priv(netdev);
> > +
> > netif_tx_stop_all_queues(to_net_dev(config->dev));
> > + adapter->eee_active = false;
>
> phylib tracks this for you.
>
Ok
> > + lan743x_set_eee(adapter, false);
> > }
> >
> > static void lan743x_phylink_mac_link_up(struct phylink_config *config,
> > @@ -3055,6 +3072,14 @@ static void lan743x_phylink_mac_link_up(struct phylink_config *config,
> > cap & FLOW_CTRL_TX,
> > cap & FLOW_CTRL_RX);
> >
> > + if (phydev && adapter->eee_enabled) {
> > + bool enable;
> > +
> > + adapter->eee_active = phy_init_eee(phydev, false) >= 0;
> > + enable = adapter->eee_active && adapter->tx_lpi_enabled;
>
> No need. Your enable should be conditional on phydev->enable_tx_lpi
> here. See Andrew's commit and understand his changes, rather than
> just ignoring Andrew's work and continuing with your old pattern of
> EEE support. Things have moved forward.
Ok. I will fix
>
> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
--
Thanks,
Raju
© 2016 - 2026 Red Hat, Inc.