From: Andrew Lunn <andrew@lunn.ch>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Hans Ulli Kroll <ulli.kroll@googlemail.com>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
netdev@vger.kernel.org
Subject: Re: [PATCH net-next 2/2] net: ethernet: cortina: Implement .set_pauseparam()
Date: Thu, 9 May 2024 22:08:45 +0200 [thread overview]
Message-ID: <55d915d2-28f0-4ed3-8f60-b43e7f4469ac@lunn.ch> (raw)
In-Reply-To: <20240509-gemini-ethernet-fix-tso-v1-2-10cd07b54d1c@linaro.org>
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
prev parent reply other threads:[~2024-05-09 20:08 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-09 7:48 [PATCH net-next 0/2] net: ethernet: cortina: TSO and pause param Linus Walleij
2024-05-09 7:48 ` [PATCH net-next 1/2] net: ethernet: cortina: Restore TSO support Linus Walleij
2024-05-09 8:21 ` Eric Dumazet
2024-05-09 14:38 ` Linus Walleij
2024-05-09 14:49 ` Eric Dumazet
2024-05-09 15:06 ` Eric Dumazet
2024-05-09 15:09 ` Eric Dumazet
2024-05-09 17:50 ` Linus Walleij
2024-05-09 17:42 ` Linus Walleij
2024-05-09 7:48 ` [PATCH net-next 2/2] net: ethernet: cortina: Implement .set_pauseparam() Linus Walleij
2024-05-09 20:08 ` Andrew Lunn [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=55d915d2-28f0-4ed3-8f60-b43e7f4469ac@lunn.ch \
--to=andrew@lunn.ch \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=kuba@kernel.org \
--cc=linus.walleij@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=ulli.kroll@googlemail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).