All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Oleksij Rempel <o.rempel@pengutronix.de>
Cc: "David S. Miller" <davem@davemloft.net>,
	Andrew Lunn <andrew@lunn.ch>, Eric Dumazet <edumazet@google.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	Woojung Huh <woojung.huh@microchip.com>,
	kernel@pengutronix.de, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, UNGLinuxDriver@microchip.com
Subject: Re: [PATCH net-next v2 1/1] net: dsa: microchip: Remove ineffective checks from ksz_set_mac_eee()
Date: Fri, 25 Apr 2025 17:48:28 +0100	[thread overview]
Message-ID: <aAu83HGXOxQ6H8on@shell.armlinux.org.uk> (raw)
In-Reply-To: <aAu41tjRIir8oMK7@pengutronix.de>

On Fri, Apr 25, 2025 at 06:31:18PM +0200, Oleksij Rempel wrote:
> Additionally, it seems that setting eee_enabled automatically based on
> advertisement in phy_probe() is no longer appropriate.
> If you agree, I would propose a patch to remove this initialization.

Remember how eee_enabled is implemented. It controls whether we program
the EEE advertisement with the contents of the software advertisement
state, or clear it.

So, if the hardware has a non-zero advertisement, then it is completely
correct that we read the advertisement and set eee_enabled to be true.
The alternative is, we set eee_enabled false and clear the
advertisement.

We can't leave the advertisement and set eee_enabled to be false. That
is inconsistent with the way we handle any attempt to set the
eee_enabled state.

I think the correct approach in this case is to set
config->eee_enabled_default to be true in ksz_phylink_get_caps(),
thereby correcting the bug introduced in March 2024. That has the
effect of setting phy->eee_cfg.tx_lpi_enabled, which means we should
report tx_lpi_enabled as true when reading the EEE state.

For the stable kernels between March 2024 and the integration of
phylink EEE support, a similar approach will be necessary to restore
the pre-March 2024 behaviour, but that will be much harder to correct
as the DSA driver doesn't have an appropriate hook to set that field
at the appropriate time.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

      reply	other threads:[~2025-04-25 16:48 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-25 11:08 [PATCH net-next v2 1/1] net: dsa: microchip: Remove ineffective checks from ksz_set_mac_eee() Oleksij Rempel
2025-04-25 13:41 ` Russell King (Oracle)
2025-04-25 14:26   ` Oleksij Rempel
2025-04-25 14:43     ` Russell King (Oracle)
2025-04-25 16:31       ` Oleksij Rempel
2025-04-25 16:48         ` Russell King (Oracle) [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=aAu83HGXOxQ6H8on@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=andrew@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=kernel@pengutronix.de \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=olteanv@gmail.com \
    --cc=pabeni@redhat.com \
    --cc=woojung.huh@microchip.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.