Re: [net-next RFC PATCH 0/3] net: phy: detach PHY driver OPs from phy_driver struct

From: "Russell King (Oracle)" <linux@armlinux.org.uk>
To: Christian Marangi <ansuelsmth@gmail.com>
Cc: "Michael Hennerich" <michael.hennerich@analog.com>,
	"Andrew Lunn" <andrew@lunn.ch>,
	"Heiner Kallweit" <hkallweit1@gmail.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Eric Dumazet" <edumazet@google.com>,
	"Jakub Kicinski" <kuba@kernel.org>,
	"Paolo Abeni" <pabeni@redhat.com>,
	"Florian Fainelli" <florian.fainelli@broadcom.com>,
	"Broadcom internal kernel review list"
	<bcm-kernel-feedback-list@broadcom.com>,
	"Ray Jui" <rjui@broadcom.com>,
	"Scott Branden" <sbranden@broadcom.com>,
	"Richard Cochran" <richardcochran@gmail.com>,
	"Marek Behún" <kabel@kernel.org>,
	"Daniel Golle" <daniel@makrotopia.org>,
	"Qingfang Deng" <dqfext@gmail.com>,
	"SkyLake Huang" <SkyLake.Huang@mediatek.com>,
	"Neil Armstrong" <neil.armstrong@linaro.org>,
	"Kevin Hilman" <khilman@baylibre.com>,
	"Jerome Brunet" <jbrunet@baylibre.com>,
	"Martin Blumenstingl" <martin.blumenstingl@googlemail.com>,
	"Arun Ramadoss" <arun.ramadoss@microchip.com>,
	UNGLinuxDriver@microchip.com, "Peter Geis" <pgwipeout@gmail.com>,
	Frank <Frank.Sae@motor-comm.com>, "Xu Liang" <lxu@maxlinear.com>,
	"Piergiorgio Beruto" <piergiorgio.beruto@gmail.com>,
	"Andrei Botila" <andrei.botila@oss.nxp.com>,
	"Bjorn Andersson" <andersson@kernel.org>,
	"Konrad Dybcio" <konrad.dybcio@linaro.org>,
	"Heiko Stuebner" <heiko@sntech.de>,
	"Michal Simek" <michal.simek@amd.com>,
	"Miguel Ojeda" <ojeda@kernel.org>,
	"Alex Gaynor" <alex.gaynor@gmail.com>,
	"Wedson Almeida Filho" <wedsonaf@gmail.com>,
	"Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Benno Lossin" <benno.lossin@proton.me>,
	"Andreas Hindborg" <a.hindborg@samsung.com>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Matthias Brugger" <matthias.bgg@gmail.com>,
	"AngeloGioacchino Del Regno"
	<angelogioacchino.delregno@collabora.com>,
	"Robert Marko" <robimarko@gmail.com>,
	"Vladimir Oltean" <vladimir.oltean@nxp.com>,
	"David Epping" <david.epping@missinglinkelectronics.com>,
	"Harini Katakam" <harini.katakam@amd.com>,
	"Simon Horman" <horms@kernel.org>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org,
	linux-amlogic@lists.infradead.org, linux-arm-msm@vger.kernel.org,
	linux-rockchip@lists.infradead.org,
	rust-for-linux@vger.kernel.org,
	linux-mediatek@lists.infradead.org
Subject: Re: [net-next RFC PATCH 0/3] net: phy: detach PHY driver OPs from phy_driver struct
Date: Sat, 17 Feb 2024 19:53:08 +0000	[thread overview]
Message-ID: <ZdEOpB1oVDE8+Qhq@shell.armlinux.org.uk> (raw)
In-Reply-To: <20240217194116.8565-1-ansuelsmth@gmail.com>

On Sat, Feb 17, 2024 at 08:41:11PM +0100, Christian Marangi wrote:
> Posting as RFC due to the massive change to a fundamental struct.
> 
> While adding some PHY ID for Aquantia, I notice that there is a
> big problem with duplicating OPs with each PHY.
> 
> The original idea to prevent this was to use mask on the PHY ID
> and identify PHY Family. Problem is that OEM started to use all
> kind of PHY ID and this is not doable, hence for PHY that have
> the same OPs, we have to duplicate all of them.
> 
> This is present in Aquantia PHY, but is much more present in
> other PHY, especially in the BCM7XXX where they use a big macro
> for common PHYs.
> 
> To reduce patch delta, I added the additional variable without
> adding tabs as this would have resulted in a massive patch.
> Also to have patch bisectable, this change has to be in one go
> hence I had to use this trick to reduce patch delta.
> 
> Other solution to this problem were to introduce additional
> variables to phy_driver struct but that would have resulted
> in having 2 different way to do the same thing and that is not O.K.
> 
> I took care to compile-test all the PHY, only exception is the unique
> RUST driver, where I still have to learn that funny language and
> I didn't had time to update it, so that is the only driver that
> I think require some fixup.
> 
> I posted 2 example that would benefits from this change, but I can
> find much more in other PHY driver.

Would it make more sense instead of this big churn, to instead
introduce into struct phy_driver:

	struct mdio_device_id	*ids;

which would then allow a phy_driver structure to be matched by
several device IDs?

We then would not need to touch any of the existing drivers initially,
and a later cleanup could be to identify those where all the ops are
the same for several phy_driver structures, and convert them over.

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

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic