| Message ID | 20241111145558.1965325-6-shaojijie@huawei.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Support some features for the HIBMCGE driver | expand |
On Mon, Nov 11, 2024 at 10:55:56PM +0800, Jijie Shao wrote: > The MAC can automatically send or respond to pause frames. > This patch supports the function of enabling pause frames > by using ethtool. > > Pause auto-negotiation is not supported currently. What is actually missing to support auto-neg pause? You are using phylib, so it will do most of the work. You just need your adjust_link callback to configure the hardware to the result of the negotiation. And call phy_support_asym_pause() to let phylib know what the MAC supports. Andrew
on 2024/11/12 1:58, Andrew Lunn wrote: > On Mon, Nov 11, 2024 at 10:55:56PM +0800, Jijie Shao wrote: >> The MAC can automatically send or respond to pause frames. >> This patch supports the function of enabling pause frames >> by using ethtool. >> >> Pause auto-negotiation is not supported currently. > What is actually missing to support auto-neg pause? You are using > phylib, so it will do most of the work. You just need your adjust_link > callback to configure the hardware to the result of the negotiation. > And call phy_support_asym_pause() to let phylib know what the MAC > supports. > > Andrew Thanks for your guidance, I haven't really figured out the difference between phy_support_sym_pause() and phy_support_asym_paus(). However, according to your guidance and referring to the phylib interface and other drivers code, I implemented the auto-neg pause function: +static void hbg_ethtool_get_pauseparam(struct net_device *net_dev, + struct ethtool_pauseparam *param) +{ + struct hbg_priv *priv = netdev_priv(net_dev); + + param->autoneg = priv->mac.pause_autoneg; + hbg_hw_get_pause_enable(priv, ¶m->tx_pause, ¶m->rx_pause); +} + +static int hbg_ethtool_set_pauseparam(struct net_device *net_dev, + struct ethtool_pauseparam *param) +{ + struct hbg_priv *priv = netdev_priv(net_dev); + struct phy_device *phydev = priv->mac.phydev; + + phy_set_asym_pause(phydev, param->rx_pause, param->tx_pause); + + priv->mac.pause_autoneg = param->autoneg; + if (!param->autoneg) + hbg_hw_set_pause_enable(priv, param->tx_pause, param->rx_pause); + + return 0; +} ...... +static void hbg_flowctrl_cfg(struct hbg_priv *priv) +{ + struct phy_device *phydev = priv->mac.phydev; + bool rx_pause; + bool tx_pause; + + if (!priv->mac.pause_autoneg) + return; + + phy_get_pause(phydev, &tx_pause, &rx_pause); + hbg_hw_set_pause_enable(priv, tx_pause, rx_pause); +} + static void hbg_phy_adjust_link(struct net_device *netdev) { struct hbg_priv *priv = netdev_priv(netdev); @@ -140,6 +153,7 @@ static void hbg_phy_adjust_link(struct net_device *netdev) priv->mac.duplex = phydev->duplex; priv->mac.autoneg = phydev->autoneg; hbg_hw_adjust_link(priv, speed, phydev->duplex); + hbg_flowctrl_cfg(priv); } priv->mac.link_status = phydev->link; @@ -168,6 +182,7 @@ static int hbg_phy_connect(struct hbg_priv *priv) return ret; phy_remove_link_mode(phydev, ETHTOOL_LINK_MODE_1000baseT_Half_BIT); + phy_support_asym_pause(phydev); phy_attached_info(phydev); return 0; ...... Can the auto-neg pause function be fully supported? If the code is ok, I'll add it in the next version. Thanks, Jijie Shao
On Tue, Nov 12, 2024 at 10:37:27PM +0800, Jijie Shao wrote: > > on 2024/11/12 1:58, Andrew Lunn wrote: > > On Mon, Nov 11, 2024 at 10:55:56PM +0800, Jijie Shao wrote: > > > The MAC can automatically send or respond to pause frames. > > > This patch supports the function of enabling pause frames > > > by using ethtool. > > > > > > Pause auto-negotiation is not supported currently. > > What is actually missing to support auto-neg pause? You are using > > phylib, so it will do most of the work. You just need your adjust_link > > callback to configure the hardware to the result of the negotiation. > > And call phy_support_asym_pause() to let phylib know what the MAC > > supports. > > > > Andrew > > Thanks for your guidance, > > I haven't really figured out the difference between phy_support_sym_pause() > and phy_support_asym_paus(). sym_pause means that when the MAC pauses, it does it in both directions, receive and transmit. Asymmetric pause means it can pause just receive, or just transmit. Since you have both tx_pause and rx_pause, you can do both. > +static void hbg_ethtool_get_pauseparam(struct net_device *net_dev, > + struct ethtool_pauseparam *param) > +{ > + struct hbg_priv *priv = netdev_priv(net_dev); > + > + param->autoneg = priv->mac.pause_autoneg; > + hbg_hw_get_pause_enable(priv, ¶m->tx_pause, ¶m->rx_pause); > +} > + > +static int hbg_ethtool_set_pauseparam(struct net_device *net_dev, > + struct ethtool_pauseparam *param) > +{ > + struct hbg_priv *priv = netdev_priv(net_dev); > + struct phy_device *phydev = priv->mac.phydev; > + > + phy_set_asym_pause(phydev, param->rx_pause, param->tx_pause); Not needed. This just tells phylib what the MAC is capable of. The capabilities does not change, so telling it once in hbg_phy_connect() is sufficient. Andrew
on 2024/11/13 0:32, Andrew Lunn wrote: > On Tue, Nov 12, 2024 at 10:37:27PM +0800, Jijie Shao wrote: >> on 2024/11/12 1:58, Andrew Lunn wrote: >>> On Mon, Nov 11, 2024 at 10:55:56PM +0800, Jijie Shao wrote: >>>> The MAC can automatically send or respond to pause frames. >>>> This patch supports the function of enabling pause frames >>>> by using ethtool. >>>> >>>> Pause auto-negotiation is not supported currently. >>> What is actually missing to support auto-neg pause? You are using >>> phylib, so it will do most of the work. You just need your adjust_link >>> callback to configure the hardware to the result of the negotiation. >>> And call phy_support_asym_pause() to let phylib know what the MAC >>> supports. >>> >>> Andrew >> Thanks for your guidance, >> >> I haven't really figured out the difference between phy_support_sym_pause() >> and phy_support_asym_paus(). > sym_pause means that when the MAC pauses, it does it in both > directions, receive and transmit. Asymmetric pause means it can pause > just receive, or just transmit. > > Since you have both tx_pause and rx_pause, you can do both. > >> +static void hbg_ethtool_get_pauseparam(struct net_device *net_dev, >> + struct ethtool_pauseparam *param) >> +{ >> + struct hbg_priv *priv = netdev_priv(net_dev); >> + >> + param->autoneg = priv->mac.pause_autoneg; >> + hbg_hw_get_pause_enable(priv, ¶m->tx_pause, ¶m->rx_pause); >> +} >> + >> +static int hbg_ethtool_set_pauseparam(struct net_device *net_dev, >> + struct ethtool_pauseparam *param) >> +{ >> + struct hbg_priv *priv = netdev_priv(net_dev); >> + struct phy_device *phydev = priv->mac.phydev; >> + >> + phy_set_asym_pause(phydev, param->rx_pause, param->tx_pause); > Not needed. This just tells phylib what the MAC is capable of. The > capabilities does not change, so telling it once in hbg_phy_connect() > is sufficient. > > Andrew Maybe there is an error in this code. If I want to disable auto-neg pause, do I need to set phy_set_asym_pause(phydev, 0, 0)? Thanks Jijie Shao
> Maybe there is an error in this code. > If I want to disable auto-neg pause, do I need to set phy_set_asym_pause(phydev, 0, 0)? You could. It is not actually clear to me what you should tell the link peer when you decide to not use pause autoneg. By passing 0, 0, you are telling the peer you don't support pause, when in fact you do, and the configuration is hard coded. Not using pause autoneg really only makes sense when you do it to both peers. And then the negotiation configuration does not matter. phylink makes this a lot easier, it hides this all away, leaving the MAC driver to just program the hardware. Andrew
diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c index e7f169d2abb7..edad07826b94 100644 --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_ethtool.c @@ -143,12 +143,34 @@ static void hbg_ethtool_get_regs(struct net_device *netdev, } } +static void hbg_ethtool_get_pauseparam(struct net_device *net_dev, + struct ethtool_pauseparam *param) +{ + struct hbg_priv *priv = netdev_priv(net_dev); + + hbg_hw_get_pause_enable(priv, ¶m->tx_pause, ¶m->rx_pause); +} + +static int hbg_ethtool_set_pauseparam(struct net_device *net_dev, + struct ethtool_pauseparam *param) +{ + struct hbg_priv *priv = netdev_priv(net_dev); + + if (param->autoneg) + return -EOPNOTSUPP; + + hbg_hw_set_pause_enable(priv, !!param->tx_pause, !!param->rx_pause); + return 0; +} + static const struct ethtool_ops hbg_ethtool_ops = { .get_link = ethtool_op_get_link, .get_link_ksettings = phy_ethtool_get_link_ksettings, .set_link_ksettings = phy_ethtool_set_link_ksettings, .get_regs_len = hbg_ethtool_get_regs_len, .get_regs = hbg_ethtool_get_regs, + .get_pauseparam = hbg_ethtool_get_pauseparam, + .set_pauseparam = hbg_ethtool_set_pauseparam, }; void hbg_ethtool_set_ops(struct net_device *netdev) diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c index 29d66a0ea0a6..0cbe9f7229b3 100644 --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.c @@ -220,6 +220,27 @@ void hbg_hw_set_mac_filter_enable(struct hbg_priv *priv, u32 enable) HBG_REG_REC_FILT_CTRL_UC_MATCH_EN_B, enable); } +void hbg_hw_set_pause_enable(struct hbg_priv *priv, u32 tx_en, u32 rx_en) +{ + hbg_reg_write_field(priv, HBG_REG_PAUSE_ENABLE_ADDR, + HBG_REG_PAUSE_ENABLE_TX_B, tx_en); + hbg_reg_write_field(priv, HBG_REG_PAUSE_ENABLE_ADDR, + HBG_REG_PAUSE_ENABLE_RX_B, rx_en); +} + +void hbg_hw_get_pause_enable(struct hbg_priv *priv, u32 *tx_en, u32 *rx_en) +{ + *tx_en = hbg_reg_read_field(priv, HBG_REG_PAUSE_ENABLE_ADDR, + HBG_REG_PAUSE_ENABLE_TX_B); + *rx_en = hbg_reg_read_field(priv, HBG_REG_PAUSE_ENABLE_ADDR, + HBG_REG_PAUSE_ENABLE_RX_B); +} + +void hbg_hw_set_rx_pause_mac_addr(struct hbg_priv *priv, u64 mac_addr) +{ + hbg_reg_write64(priv, HBG_REG_FD_FC_ADDR_LOW_ADDR, mac_addr); +} + static void hbg_hw_init_transmit_ctrl(struct hbg_priv *priv) { u32 ctrl = 0; diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h index 6eb4b7d2cba8..a4a049b5121d 100644 --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_hw.h @@ -56,5 +56,8 @@ u32 hbg_hw_get_fifo_used_num(struct hbg_priv *priv, enum hbg_dir dir); void hbg_hw_set_tx_desc(struct hbg_priv *priv, struct hbg_tx_desc *tx_desc); void hbg_hw_fill_buffer(struct hbg_priv *priv, u32 buffer_dma_addr); void hbg_hw_set_mac_filter_enable(struct hbg_priv *priv, u32 enable); +void hbg_hw_set_pause_enable(struct hbg_priv *priv, u32 tx_en, u32 rx_en); +void hbg_hw_get_pause_enable(struct hbg_priv *priv, u32 *tx_en, u32 *rx_en); +void hbg_hw_set_rx_pause_mac_addr(struct hbg_priv *priv, u64 mac_addr); #endif diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c index 0ad03681b706..a45a63856e61 100644 --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_main.c @@ -204,6 +204,7 @@ static int hbg_net_set_mac_address(struct net_device *netdev, void *addr) if (is_exists) hbg_set_mac_to_mac_table(priv, index, NULL); + hbg_hw_set_rx_pause_mac_addr(priv, ether_addr_to_u64(mac_addr)); dev_addr_set(netdev, mac_addr); return 0; } diff --git a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h index 665666712c7c..f12efc12f3c5 100644 --- a/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h +++ b/drivers/net/ethernet/hisilicon/hibmcge/hbg_reg.h @@ -51,6 +51,8 @@ #define HBG_REG_PORT_ENABLE_RX_B BIT(1) #define HBG_REG_PORT_ENABLE_TX_B BIT(2) #define HBG_REG_PAUSE_ENABLE_ADDR (HBG_REG_SGMII_BASE + 0x0048) +#define HBG_REG_PAUSE_ENABLE_RX_B BIT(0) +#define HBG_REG_PAUSE_ENABLE_TX_B BIT(1) #define HBG_REG_AN_NEG_STATE_ADDR (HBG_REG_SGMII_BASE + 0x0058) #define HBG_REG_TRANSMIT_CTRL_ADDR (HBG_REG_SGMII_BASE + 0x0060) #define HBG_REG_TRANSMIT_CTRL_PAD_EN_B BIT(7)
The MAC can automatically send or respond to pause frames. This patch supports the function of enabling pause frames by using ethtool. Pause auto-negotiation is not supported currently. Signed-off-by: Jijie Shao <shaojijie@huawei.com> --- .../ethernet/hisilicon/hibmcge/hbg_ethtool.c | 22 +++++++++++++++++++ .../net/ethernet/hisilicon/hibmcge/hbg_hw.c | 21 ++++++++++++++++++ .../net/ethernet/hisilicon/hibmcge/hbg_hw.h | 3 +++ .../net/ethernet/hisilicon/hibmcge/hbg_main.c | 1 + .../net/ethernet/hisilicon/hibmcge/hbg_reg.h | 2 ++ 5 files changed, 49 insertions(+)