All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Vladimir Oltean <vladimir.oltean@nxp.com>
Cc: netdev@vger.kernel.org, Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	UNGLinuxDriver@microchip.com,
	Heiner Kallweit <hkallweit1@gmail.com>,
	Sean Anderson <sean.anderson@seco.com>,
	Colin Foster <colin.foster@in-advantage.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Subject: Re: [PATCH net-next 4/4] net: dsa: remove phylink_validate() method
Date: Tue, 1 Nov 2022 12:01:32 +0000	[thread overview]
Message-ID: <Y2EKnLt2SyhjvcNI@shell.armlinux.org.uk> (raw)
In-Reply-To: <20221101114806.1186516-5-vladimir.oltean@nxp.com>

On Tue, Nov 01, 2022 at 01:48:06PM +0200, Vladimir Oltean wrote:
> Not all DSA drivers provide config->mac_capabilities, for example
> mv88e6060, lan9303 and vsc73xx don't. However, there have been users of
> those drivers on recent kernels and no one reported that they fail to
> establish a link, so I'm guessing that they work (somehow). But I must
> admit I don't understand why phylink_generic_validate() works when
> mac_capabilities=0. Anyway, these drivers did not provide a
> phylink_validate() method before and do not provide one now, so nothing
> changes for them.

There is a specific exception:

static void dsa_port_phylink_validate(struct phylink_config *config,
                                      unsigned long *supported,
                                      struct phylink_link_state *state)
{
        struct dsa_port *dp = container_of(config, struct dsa_port, pl_config);
        struct dsa_switch *ds = dp->ds;

        if (!ds->ops->phylink_validate) {
                if (config->mac_capabilities)
                        phylink_generic_validate(config, supported, state);
                return;

When config->mac_capabilities is zero, and there is no phylink_validate()
function, dsa_port_phylink_validate() becomes a no-op, and the no-op
case basically means "everything is allowed", which is how things worked
before the generic validation was added, as you will see from commit
5938bce4b6e2 ("net: dsa: support use of phylink_generic_validate()").

Changing this as you propose below will likely break these drivers.

A safer change would be to elimate ds->ops->phylink_validate, leaving
the call to phylink_generic_validate() conditional on mac_capabilities
having been filled in - which will save breaking these older drivers.

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

  parent reply	other threads:[~2022-11-01 12:01 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-01 11:48 [PATCH net-next 0/4] Remove phylink_validate() from Felix DSA driver Vladimir Oltean
2022-11-01 11:48 ` [PATCH net-next 1/4] net: phy: aquantia: add AQR112 and AQR412 PHY IDs Vladimir Oltean
2022-11-01 15:36   ` Sean Anderson
2022-11-03 11:39     ` Vladimir Oltean
2022-11-01 11:48 ` [PATCH net-next 2/4] net: dsa: felix: use phylink_generic_validate() Vladimir Oltean
2022-11-01 11:48 ` [PATCH net-next 3/4] net: mscc: ocelot: drop workaround for forcing RX flow control Vladimir Oltean
2022-11-01 11:48 ` [PATCH net-next 4/4] net: dsa: remove phylink_validate() method Vladimir Oltean
2022-11-01 11:59   ` Vladimir Oltean
2022-11-01 12:01   ` Russell King (Oracle) [this message]
2022-11-01 12:40     ` Vladimir Oltean
2022-11-01 15:42       ` Russell King (Oracle)
2022-11-04 11:24   ` Russell King (Oracle)
2022-11-04 11:35     ` Russell King (Oracle)
2022-11-04 13:32       ` Vladimir Oltean
2022-11-04 14:01         ` Russell King (Oracle)
2022-11-04 14:25           ` Vladimir Oltean
2022-11-04 14:48             ` Russell King (Oracle)
2022-11-04 15:33               ` Vladimir Oltean
2022-11-04 15:40       ` Florian Fainelli
2022-11-04 16:35         ` Russell King (Oracle)
2022-11-06  1:04           ` Florian Fainelli

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=Y2EKnLt2SyhjvcNI@shell.armlinux.org.uk \
    --to=linux@armlinux.org.uk \
    --cc=UNGLinuxDriver@microchip.com \
    --cc=alexandre.belloni@bootlin.com \
    --cc=andrew@lunn.ch \
    --cc=claudiu.manoil@nxp.com \
    --cc=colin.foster@in-advantage.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=f.fainelli@gmail.com \
    --cc=hkallweit1@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=sean.anderson@seco.com \
    --cc=vivien.didelot@gmail.com \
    --cc=vladimir.oltean@nxp.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.