From: Christian Marangi <ansuelsmth@gmail•com>
To: "Russell King (Oracle)" <linux@armlinux•org.uk>
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: Sun, 18 Feb 2024 00:22:45 +0100 [thread overview]
Message-ID: <65d13fcd.050a0220.88fe3.665f@mx.google.com> (raw)
In-Reply-To: <ZdEOpB1oVDE8+Qhq@shell.armlinux.org.uk>
On Sat, Feb 17, 2024 at 07:53:08PM +0000, Russell King (Oracle) wrote:
> 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?
Yes that was an alternative idea, but is it good to then have 2 way to
declare PHY ID?
Also the name should be changed... Maybe an array of a struct PHY_ID,
name that ends with a sentinel?
>
> 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.
We have many PHY that already have macro to define the same OPs and
change only name PHY ID and mask.
--
Ansuel
next prev parent reply other threads:[~2024-02-17 23:22 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-17 19:41 [net-next RFC PATCH 0/3] net: phy: detach PHY driver OPs from phy_driver struct Christian Marangi
2024-02-17 19:41 ` [net-next RFC PATCH 1/3] " Christian Marangi
2024-02-17 19:41 ` [net-next RFC PATCH 2/3] net: phy: aquantia: use common OPs for PHYs where possible Christian Marangi
2024-02-17 19:41 ` [net-next RFC PATCH 3/3] net: phy: bcm7xxx: " Christian Marangi
2024-02-17 19:53 ` [net-next RFC PATCH 0/3] net: phy: detach PHY driver OPs from phy_driver struct Russell King (Oracle)
2024-02-17 23:22 ` Christian Marangi [this message]
2024-02-18 12:39 ` Russell King (Oracle)
2024-02-17 22:21 ` Andrew Lunn
2024-02-17 23:27 ` Christian Marangi
2024-02-17 23:48 ` Andrew Lunn
2024-02-17 22:37 ` Trevor Gross
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=65d13fcd.050a0220.88fe3.665f@mx.google.com \
--to=ansuelsmth@gmail$(echo .)com \
--cc=Frank.Sae@motor-comm$(echo .)com \
--cc=SkyLake.Huang@mediatek$(echo .)com \
--cc=UNGLinuxDriver@microchip$(echo .)com \
--cc=a.hindborg@samsung$(echo .)com \
--cc=alex.gaynor@gmail$(echo .)com \
--cc=aliceryhl@google$(echo .)com \
--cc=andersson@kernel$(echo .)org \
--cc=andrei.botila@oss$(echo .)nxp.com \
--cc=andrew@lunn$(echo .)ch \
--cc=angelogioacchino.delregno@collabora$(echo .)com \
--cc=arun.ramadoss@microchip$(echo .)com \
--cc=bcm-kernel-feedback-list@broadcom$(echo .)com \
--cc=benno.lossin@proton$(echo .)me \
--cc=bjorn3_gh@protonmail$(echo .)com \
--cc=boqun.feng@gmail$(echo .)com \
--cc=daniel@makrotopia$(echo .)org \
--cc=davem@davemloft$(echo .)net \
--cc=david.epping@missinglinkelectronics$(echo .)com \
--cc=dqfext@gmail$(echo .)com \
--cc=edumazet@google$(echo .)com \
--cc=florian.fainelli@broadcom$(echo .)com \
--cc=gary@garyguo$(echo .)net \
--cc=harini.katakam@amd$(echo .)com \
--cc=heiko@sntech$(echo .)de \
--cc=hkallweit1@gmail$(echo .)com \
--cc=horms@kernel$(echo .)org \
--cc=jbrunet@baylibre$(echo .)com \
--cc=kabel@kernel$(echo .)org \
--cc=khilman@baylibre$(echo .)com \
--cc=konrad.dybcio@linaro$(echo .)org \
--cc=kuba@kernel$(echo .)org \
--cc=linux-amlogic@lists$(echo .)infradead.org \
--cc=linux-arm-kernel@lists$(echo .)infradead.org \
--cc=linux-arm-msm@vger$(echo .)kernel.org \
--cc=linux-kernel@vger$(echo .)kernel.org \
--cc=linux-mediatek@lists$(echo .)infradead.org \
--cc=linux-rockchip@lists$(echo .)infradead.org \
--cc=linux@armlinux$(echo .)org.uk \
--cc=lxu@maxlinear$(echo .)com \
--cc=martin.blumenstingl@googlemail$(echo .)com \
--cc=matthias.bgg@gmail$(echo .)com \
--cc=michael.hennerich@analog$(echo .)com \
--cc=michal.simek@amd$(echo .)com \
--cc=neil.armstrong@linaro$(echo .)org \
--cc=netdev@vger$(echo .)kernel.org \
--cc=ojeda@kernel$(echo .)org \
--cc=pabeni@redhat$(echo .)com \
--cc=pgwipeout@gmail$(echo .)com \
--cc=piergiorgio.beruto@gmail$(echo .)com \
--cc=richardcochran@gmail$(echo .)com \
--cc=rjui@broadcom$(echo .)com \
--cc=robimarko@gmail$(echo .)com \
--cc=rust-for-linux@vger$(echo .)kernel.org \
--cc=sbranden@broadcom$(echo .)com \
--cc=vladimir.oltean@nxp$(echo .)com \
--cc=wedsonaf@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox