From: "Russell King (Oracle)" <linux@armlinux•org.uk>
To: Andrew Lunn <andrew@lunn•ch>
Cc: netdev <netdev@vger•kernel.org>,
Florian Fainelli <f.fainelli@gmail•com>,
Heiner Kallweit <hkallweit1@gmail•com>,
Oleksij Rempel <o.rempel@pengutronix•de>
Subject: Re: [RFC/RFT 02/23] net: phylink: Plumb eee_active in mac_link_up call
Date: Mon, 27 Mar 2023 22:53:36 +0100 [thread overview]
Message-ID: <ZCIQYJVMoG3RUfN3@shell.armlinux.org.uk> (raw)
In-Reply-To: <20230327170201.2036708-3-andrew@lunn.ch>
Hi Andrew,
Thinking about this more having read the follow-on patches, I retract
my r-b tag, because there is an issue that needs solving.
On Mon, Mar 27, 2023 at 07:01:40PM +0200, Andrew Lunn wrote:
> @@ -1257,7 +1260,8 @@ static void phylink_link_up(struct phylink *pl,
>
> pl->mac_ops->mac_link_up(pl->config, pl->phydev, pl->cur_link_an_mode,
> pl->cur_interface, speed, duplex,
> - !!(link_state.pause & MLO_PAUSE_TX), rx_pause);
> + !!(link_state.pause & MLO_PAUSE_TX), rx_pause,
> + eee_active);
In one of your later patches, you have phylib call phy_link_up() when
the state changes as a result of configuration. That will cause
phy_link_change(), which will update phylink's stored link state, and
trigger phylink to re-resolve the link.
However, phylink guarantees that mac_link_up() will only be called
if mac_link_down() was previously called. This will *not* cause
mac_link_up() to be called.
Moreover, we don't want mac_link_up() to be called because the link
hasn't gone down and to do so will violate that guarantee that
phylink makes to MAC drivers.
So, I don't think this is going to work fully as seems to be intended,
if I'm understanding things correctly.
Maybe we should have a new mac_set_eee() method which we can call
when the EEE state changes? Would we need to call it with the LPI
delay parameter and wheneever that changes, or should we rely on
the MAC to do that? What if the LPI parameter is dependent on the
speed the MAC is operating? Just brain-storming...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2023-03-27 21:53 UTC|newest]
Thread overview: 37+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-27 17:01 [RFC/RFT 00/23] net: ethernet: Rework EEE Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 01/23] net: phy: Add phydev->eee_active to simplify adjust link callbacks Andrew Lunn
2023-03-27 17:54 ` Russell King (Oracle)
2023-03-27 17:01 ` [RFC/RFT 02/23] net: phylink: Plumb eee_active in mac_link_up call Andrew Lunn
2023-03-27 17:57 ` Russell King (Oracle)
2023-03-27 21:53 ` Russell King (Oracle) [this message]
2023-03-27 22:45 ` Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 03/23] net: phy: Add helper to set EEE Clock stop enable bit Andrew Lunn
2023-03-27 17:58 ` Russell King (Oracle)
2023-03-28 5:03 ` Oleksij Rempel
2023-03-28 5:13 ` Oleksij Rempel
2023-03-28 12:09 ` Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 04/23] net: phy: Keep track of EEE tx_lpi_enabled Andrew Lunn
2023-03-27 17:58 ` Russell King (Oracle)
2023-03-27 17:01 ` [RFC/RFT 05/23] net: phy: Immediately call adjust_link if only tx_lpi_enabled changes Andrew Lunn
2023-03-27 18:02 ` Russell King (Oracle)
2023-03-27 22:13 ` Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 06/23] net: marvell: mvneta: Simplify EEE configuration Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 07/23] net: stmmac: Drop usage of phy_init_eee() Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 08/23] net: stmmac: Simplify ethtool get eee Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 09/23] net: lan743x: Fixup EEE Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 10/23] net: fec: Move fec_enet_eee_mode_set() and helper earlier Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 11/23] net: FEC: Fixup EEE Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 12/23] net: genet: " Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 13/23] net: sxgdb: " Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 14/23] net: dsa: mt7530: Swap to using phydev->eee_active Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 15/23] net: dsa: b53: " Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 16/23] net: phylink: Remove unused phylink_init_eee() Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 17/23] net: phy: remove unused phy_init_eee() Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 18/23] net: usb: lan78xx: Fixup EEE Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 19/23] net: phy: Add phy_support_eee() indicating MAC support EEE Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 20/23] net: phylink: Add MAC_EEE to mac_capabilites Andrew Lunn
2023-03-27 21:59 ` Russell King (Oracle)
2023-03-27 22:15 ` Andrew Lunn
2023-03-27 17:01 ` [RFC/RFT 21/23] net: phylink: Extend mac_capabilities in MAC drivers which support EEE Andrew Lunn
2023-03-27 17:02 ` [RFC/RFT 22/23] net: phylib: call phy_support_eee() " Andrew Lunn
2023-03-27 17:02 ` [RFC/RFT 23/23] net: phy: Disable EEE advertisement by default Andrew Lunn
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=ZCIQYJVMoG3RUfN3@shell.armlinux.org.uk \
--to=linux@armlinux$(echo .)org.uk \
--cc=andrew@lunn$(echo .)ch \
--cc=f.fainelli@gmail$(echo .)com \
--cc=hkallweit1@gmail$(echo .)com \
--cc=netdev@vger$(echo .)kernel.org \
--cc=o.rempel@pengutronix$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
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