All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <olteanv@gmail.com>
Cc: Andrew Lunn <andrew@lunn.ch>,
	Heiner Kallweit <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Florian Fainelli <florian.fainelli@broadcom.com>,
	Jakub Kicinski <kuba@kernel.org>,
	netdev@vger.kernel.org, Paolo Abeni <pabeni@redhat.com>,
	Simon Horman <horms@kernel.org>,
	UNGLinuxDriver@microchip.com,
	Woojung Huh <woojung.huh@microchip.com>
Subject: Re: [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee()
Date: Thu, 2 Jan 2025 17:56:59 +0000	[thread overview]
Message-ID: <Z3bTa0Sq_GG8Khww@shell.armlinux.org.uk> (raw)
In-Reply-To: <20241212194419.4cbb776hc47yyl6z@skbuf>

On Thu, Dec 12, 2024 at 09:44:19PM +0200, Vladimir Oltean wrote:
> On Tue, Dec 10, 2024 at 02:26:19PM +0000, Russell King (Oracle) wrote:
> >  	/* Check whether the switch supports EEE */
> >  	if (!ds->ops->support_eee || !ds->ops->support_eee(ds, dp->index))
> >  		return -EOPNOTSUPP;
> >  
> > -	/* Port's PHY and MAC both need to be EEE capable */
> > -	if (!dev->phydev)
> > -		return -ENODEV;
> 
> It may well be that removing this test is ok given the later call to
> phylink_ethtool_get_eee() which will fail with the same return code,
> but this change does not logically pertain to a patch titled
> "no longer call ds->ops->get_mac_eee()", and no justification is brought
> for it in the commit message (my previous sentence should be sufficient).
> Please move this to a separate patch, for traceability purposes.

Let's say we split this from the patch, and leave the check in place for
this patch.

We then end up with:

	/* Port's PHY and MAC both need to be EEE capable */
	if (!dev->phydev)
		return -ENODEV;

	return phylink_ethtool_get_eee(dp->pl, e);

here.

At this point, we end up with this code:

	if (!dev->phydev)
		return -ENODEV;
followed by:
	ret = -EOPNOTSUPP;

	if (pl->phydev)
		ret = phy_ethtool_get_eee(pl->phydev, eee);

	return ret;

You seem to want that, so I'll drop the removal of this from the patch
series, especially as it changes the error that userspace sees - even
though it's different from what other ethernet drivers do. If we want
to address the !phydev return code issue, that can be done some other
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-01-02 17:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-12-10 14:25 [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 1/7] net: dsa: ksz: remove setting of tx_lpi parameters Russell King (Oracle)
2024-12-12 19:38   ` Vladimir Oltean
2024-12-10 14:26 ` [PATCH RFC net-next 2/7] net: dsa: no longer call ds->ops->get_mac_eee() Russell King (Oracle)
2024-12-12 19:44   ` Vladimir Oltean
2025-01-02 17:56     ` Russell King (Oracle) [this message]
2024-12-10 14:26 ` [PATCH RFC net-next 3/7] net: dsa: b53/bcm_sf2: remove b53_get_mac_eee() Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 4/7] net: dsa: ksz: remove ksz_get_mac_eee() Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 5/7] net: dsa: mv88e6xxx: remove mv88e6xxx_get_mac_eee() Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 6/7] net: dsa: qca: remove qca8k_get_mac_eee() Russell King (Oracle)
2024-12-10 14:26 ` [PATCH RFC net-next 7/7] net: dsa: remove get_mac_eee() method Russell King (Oracle)
2024-12-12 19:48 ` [PATCH RFC net-next 0/7] net: dsa: cleanup EEE (part 2) Vladimir Oltean
2025-01-02 18:18   ` Russell King (Oracle)

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=Z3bTa0Sq_GG8Khww@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=florian.fainelli@broadcom.com \
    --cc=hkallweit1@gmail.com \
    --cc=horms@kernel.org \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --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.