| Message ID | 20240509-gemini-ethernet-fix-tso-v1-2-10cd07b54d1c@linaro.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: ethernet: cortina: TSO and pause param | expand |
On Thu, May 09, 2024 at 09:48:38AM +0200, Linus Walleij wrote: > The Cortina Gemini ethernet can very well set up TX or RX > pausing, so add this functionality to the driver in a > .set_pauseparam() callback. > > Signed-off-by: Linus Walleij <linus.walleij@linaro.org> > --- > drivers/net/ethernet/cortina/gemini.c | 23 +++++++++++++++++++++++ > 1 file changed, 23 insertions(+) > > diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c > index 599de7914122..c732e985055f 100644 > --- a/drivers/net/ethernet/cortina/gemini.c > +++ b/drivers/net/ethernet/cortina/gemini.c > @@ -2138,6 +2138,28 @@ static void gmac_get_pauseparam(struct net_device *netdev, > pparam->autoneg = true; > } > > +static int gmac_set_pauseparam(struct net_device *netdev, > + struct ethtool_pauseparam *pparam) > +{ > + struct gemini_ethernet_port *port = netdev_priv(netdev); > + union gmac_config0 config0; > + > + config0.bits32 = readl(port->gmac_base + GMAC_CONFIG0); > + > + if (pparam->rx_pause) > + config0.bits.rx_fc_en = 1; > + if (pparam->tx_pause) > + config0.bits.tx_fc_en = 1; > + /* We only support autonegotiation */ > + if (!pparam->autoneg) > + return -EINVAL; > + > + writel(config0.bits32, port->gmac_base + GMAC_CONFIG0); Everybody gets pause wrong. You even say it yourself: > + /* We only support autonegotiation */ After connecting to the PHY, you need to call phy_support_asym_pause(), to let phylib know the MAC support asymmetric pause. The PHY will then advertise this to the link peer. Gemini does seem to be doing that already. When auto-neg is complete, it calls the adjust link callback. Ideally you then want to call phy_get_pause() which gives you the results of the negotiation, and so how to configure the MAC. gmac_speed_set() kind of does this, but it directly reads PHY registers, rather than asking phylib. It would be nice to update this code. This ethtool call allows you to change what the device advertises to the link partner. You should pass this onto phylib using phy_set_asym_pause(). If something has changed, phylib will kick off a new auto-neg, and then call the adjust link callback so you can configure the hardware with the new negotiation results. It is unlikely that gmac_set_pauseparam() needs to change the hardware configuration when pparam->autoneg is true. If pparam->autoneg is false, that means we are not using auto-neg for pause, and then you can directly program the MAC hardware. But it does not look like you intend to support this, which is fine. Andrew --- pw-bot: cr
diff --git a/drivers/net/ethernet/cortina/gemini.c b/drivers/net/ethernet/cortina/gemini.c index 599de7914122..c732e985055f 100644 --- a/drivers/net/ethernet/cortina/gemini.c +++ b/drivers/net/ethernet/cortina/gemini.c @@ -2138,6 +2138,28 @@ static void gmac_get_pauseparam(struct net_device *netdev, pparam->autoneg = true; } +static int gmac_set_pauseparam(struct net_device *netdev, + struct ethtool_pauseparam *pparam) +{ + struct gemini_ethernet_port *port = netdev_priv(netdev); + union gmac_config0 config0; + + config0.bits32 = readl(port->gmac_base + GMAC_CONFIG0); + + if (pparam->rx_pause) + config0.bits.rx_fc_en = 1; + if (pparam->tx_pause) + config0.bits.tx_fc_en = 1; + /* We only support autonegotiation */ + if (!pparam->autoneg) + return -EINVAL; + + writel(config0.bits32, port->gmac_base + GMAC_CONFIG0); + + return 0; +} + + static void gmac_get_ringparam(struct net_device *netdev, struct ethtool_ringparam *rp, struct kernel_ethtool_ringparam *kernel_rp, @@ -2258,6 +2280,7 @@ static const struct ethtool_ops gmac_351x_ethtool_ops = { .set_link_ksettings = gmac_set_ksettings, .nway_reset = gmac_nway_reset, .get_pauseparam = gmac_get_pauseparam, + .set_pauseparam = gmac_set_pauseparam, .get_ringparam = gmac_get_ringparam, .set_ringparam = gmac_set_ringparam, .get_coalesce = gmac_get_coalesce,
The Cortina Gemini ethernet can very well set up TX or RX pausing, so add this functionality to the driver in a .set_pauseparam() callback. Signed-off-by: Linus Walleij <linus.walleij@linaro.org> --- drivers/net/ethernet/cortina/gemini.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+)