Re: [PATCH net-next 01/15] net: phylink: add PCS negotiation mode

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>,
	Alexander Couzens <lynxis@fe80.eu>,
	AngeloGioacchino Del Regno
	<angelogioacchino.delregno@collabora.com>,
	Cc:  Claudiu Beznea <claudiu.beznea@microchip.com>,
	Daniel Golle <daniel@makrotopia.org>,
	Daniel Machon <daniel.machon@microchip.com>,
	"David S. Miller" <davem@davemloft.net>,
	DENG Qingfang <dqfext@gmail.com>,
	Eric Dumazet <edumazet@google.com>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Horatiu Vultur <horatiu.vultur@microchip.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	Jakub Kicinski <kuba@kernel.org>,
	Jose Abreu <Jose.Abreu@synopsys.com>,
	Landen Chao <Landen.Chao@mediatek.com>,
	Lars Povlsen <lars.povlsen@microchip.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-mediatek@lists.infradead.org,
	Madalin Bucur <madalin.bucur@nxp.com>,
	Marcin Wojtas <mw@semihalf.com>,
	Matthias Brugger <matthias.bgg@gmail.com>,
	Michal Simek <michal.simek@amd.com>,
	netdev@vger.kernel.org,
	Nicolas Ferre <nicolas.ferre@microchip.com>,
	Paolo Abeni <pabeni@redhat.com>,
	Radhey Shyam Pandey <radhey.shyam.pandey@xilinx.com>,
	Sean Anderson <sean.anderson@seco.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Steen Hegelund <Steen.Hegelund@microchip.com>,
	Taras Chornyi <taras.chornyi@plvision.eu>,
	Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	UNGLinuxDriver@microchip.com
Subject: Re: [PATCH net-next 01/15] net: phylink: add PCS negotiation mode
Date: Tue, 20 Jun 2023 16:51:08 +0100	[thread overview]
Message-ID: <ZJHK7MiZUR4Kr2ZT@shell.armlinux.org.uk> (raw)
In-Reply-To: <20230620113730.vm2buvcifdcvhujb@skbuf>

On Tue, Jun 20, 2023 at 02:37:30PM +0300, Vladimir Oltean wrote:
> On Fri, Jun 16, 2023 at 01:06:22PM +0100, Russell King (Oracle) wrote:
> > @@ -443,6 +526,7 @@ struct phylink_pcs_ops;
> >   */
> >  struct phylink_pcs {
> >  	const struct phylink_pcs_ops *ops;
> > +	bool neg_mode;
> >  	bool poll;
> >  };
> 
> I deleted one of my own comments while trimming the email... Yay me :)
> 
> Would it be more appropriate to name this "bool pass_neg_mode" to avoid
> a naming collision between "bool neg_mode" and "unsigned int neg_mode"?

I'd entertain "want_neg_mode" but I don't think there's much scope for
confusion between the two - PCS drivers only get to set this flag
during the creation of the PCS.

In any case, I don't want this "neg_mode" to hang around for ages
and I see it as a transitory mechanism that will go away in a max of
a couple of kernel releases, especially as this patch set ensures
that all current users are converted. At the moment, it will catch
the case where some new PCS driver gets merged that doesn't use the
new "neg_mode" (and thus doesn't set this boolean flag.)

I know it's heresy, but it also helps trees like OpenWRT deal with
the interface change - which after all will _not_ generate any
compiler diagnostics between a converted PCS driver and an
unconverted PCS driver.

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