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: Sun, 18 Feb 2024 12:39:14 +0000 [thread overview]
Message-ID: <ZdH6csVX1ZOPYP84@shell.armlinux.org.uk> (raw)
In-Reply-To: <65d13fcd.050a0220.88fe3.665f@mx.google.com>
On Sun, Feb 18, 2024 at 12:22:45AM +0100, Christian Marangi wrote:
> On Sat, Feb 17, 2024 at 07:53:08PM +0000, Russell King (Oracle) wrote:
> > 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 already have arrays of mdio_device_id in every driver, whether they
can be re-used or not is a separate issue. In any case, merely adding
support for this (only build tested patch below) is a much smaller
change.
drivers/net/phy/phy_device.c | 64 +++++++++++++++++++++++++++++++++-----------
include/linux/phy.h | 16 ++++++++++-
2 files changed, 63 insertions(+), 17 deletions(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 2e7d5bfb338e..7e02bf51a2a5 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -522,12 +522,51 @@ static int phy_scan_fixups(struct phy_device *phydev)
return 0;
}
+static const struct mdio_device_id *
+phy_driver_match_id(struct phy_driver *phydrv, u32 id)
+{
+ const struct mdio_device_id *ids = phydrv->ids;
+
+ if (ids) {
+ while (ids->phy_id) {
+ if (phy_id_compare(id, ids->phy_id, ids->phy_id_mask))
+ return ids;
+ ids++;
+ }
+ }
+
+ if (phy_id_compare(id, phydrv->id.phy_id, phydrv->id.phy_id_mask))
+ return &phydrv->id;
+
+ return NULL;
+}
+
+static const struct mdio_device_id *
+phy_driver_match(struct phy_driver *phydrv, struct phy_device *phydev)
+{
+ const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
+ const struct mdio_device_id *id;
+ int i;
+
+ if (!phydev->is_c45)
+ return phy_driver_match_id(phydrv, phydev->phy_id);
+
+ for (i = 1; i < num_ids; i++) {
+ if (phydev->c45_ids.device_ids[i] == 0xffffffff)
+ continue;
+
+ id = phy_driver_match_id(phydrv, phydev->c45_ids.device_ids[i]);
+ if (id)
+ return id;
+ }
+
+ return NULL;
+}
+
static int phy_bus_match(struct device *dev, struct device_driver *drv)
{
struct phy_device *phydev = to_phy_device(dev);
struct phy_driver *phydrv = to_phy_driver(drv);
- const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
- int i;
if (!(phydrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY))
return 0;
@@ -535,20 +574,7 @@ static int phy_bus_match(struct device *dev, struct device_driver *drv)
if (phydrv->match_phy_device)
return phydrv->match_phy_device(phydev);
- if (phydev->is_c45) {
- for (i = 1; i < num_ids; i++) {
- if (phydev->c45_ids.device_ids[i] == 0xffffffff)
- continue;
-
- if (phy_id_compare(phydev->c45_ids.device_ids[i],
- phydrv->phy_id, phydrv->phy_id_mask))
- return 1;
- }
- return 0;
- } else {
- return phy_id_compare(phydev->phy_id, phydrv->phy_id,
- phydrv->phy_id_mask);
- }
+ return !!phy_driver_match(phydrv, phydev);
}
static ssize_t
@@ -3311,6 +3337,7 @@ static int phy_probe(struct device *dev)
int err = 0;
phydev->drv = phydrv;
+ phydev->drv_id = phy_driver_match(phydrv, phydev);
/* Disable the interrupt if the PHY doesn't support it
* but the interrupt is still a valid one
@@ -3485,6 +3512,11 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
new_driver->mdiodrv.driver.owner = owner;
new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
+ if (!new_driver->id.phy_id) {
+ new_driver->id.phy_id = new_driver->phy_id;
+ new_driver->id.phy_id_mask = new_driver->phy_id_mask;
+ }
+
retval = driver_register(&new_driver->mdiodrv.driver);
if (retval) {
pr_err("%s: Error %d in registering driver\n",
diff --git a/include/linux/phy.h b/include/linux/phy.h
index fd8dbea9b4d9..2f2ebbd41535 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -542,6 +542,7 @@ struct macsec_ops;
*
* @mdio: MDIO bus this PHY is on
* @drv: Pointer to the driver for this PHY instance
+ * @drv_id: The matched driver ID for this PHY instance
* @devlink: Create a link between phy dev and mac dev, if the external phy
* used by current mac interface is managed by another mac interface.
* @phy_id: UID for this device found during discovery
@@ -639,6 +640,7 @@ struct phy_device {
/* Information about the PHY type */
/* And management functions */
const struct phy_driver *drv;
+ const struct mdio_device_id *drv_id;
struct device_link *devlink;
@@ -882,6 +884,9 @@ struct phy_led {
* struct phy_driver - Driver structure for a particular PHY type
*
* @mdiodrv: Data common to all MDIO devices
+ * @ids: array of mdio device IDs to match this driver (terminated with
+ * zero phy_id)
+ * @id: mdio device ID to match
* @phy_id: The result of reading the UID registers of this PHY
* type, and ANDing them with the phy_id_mask. This driver
* only works for PHYs with IDs which match this field
@@ -903,6 +908,8 @@ struct phy_led {
*/
struct phy_driver {
struct mdio_driver_common mdiodrv;
+ const struct mdio_device_id *ids;
+ struct mdio_device_id id;
u32 phy_id;
char *name;
u32 phy_id_mask;
@@ -1203,7 +1210,14 @@ static inline bool phy_id_compare(u32 id1, u32 id2, u32 mask)
*/
static inline bool phydev_id_compare(struct phy_device *phydev, u32 id)
{
- return phy_id_compare(id, phydev->phy_id, phydev->drv->phy_id_mask);
+ u32 mask;
+
+ if (phydev->drv_id)
+ mask = phydev->drv_id->phy_id_mask;
+ else
+ mask = phydev->drv->phy_id_mask;
+
+ return phy_id_compare(id, phydev->phy_id, mask);
}
/* A Structure for boards to register fixups with the PHY Lib */
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
next prev parent reply other threads:[~2024-02-18 12:40 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
2024-02-18 12:39 ` Russell King (Oracle) [this message]
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=ZdH6csVX1ZOPYP84@shell.armlinux.org.uk \
--to=linux@armlinux.org.uk \
--cc=Frank.Sae@motor-comm.com \
--cc=SkyLake.Huang@mediatek.com \
--cc=UNGLinuxDriver@microchip.com \
--cc=a.hindborg@samsung.com \
--cc=alex.gaynor@gmail.com \
--cc=aliceryhl@google.com \
--cc=andersson@kernel.org \
--cc=andrei.botila@oss.nxp.com \
--cc=andrew@lunn.ch \
--cc=angelogioacchino.delregno@collabora.com \
--cc=ansuelsmth@gmail.com \
--cc=arun.ramadoss@microchip.com \
--cc=bcm-kernel-feedback-list@broadcom.com \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=daniel@makrotopia.org \
--cc=davem@davemloft.net \
--cc=david.epping@missinglinkelectronics.com \
--cc=dqfext@gmail.com \
--cc=edumazet@google.com \
--cc=florian.fainelli@broadcom.com \
--cc=gary@garyguo.net \
--cc=harini.katakam@amd.com \
--cc=heiko@sntech.de \
--cc=hkallweit1@gmail.com \
--cc=horms@kernel.org \
--cc=jbrunet@baylibre.com \
--cc=kabel@kernel.org \
--cc=khilman@baylibre.com \
--cc=konrad.dybcio@linaro.org \
--cc=kuba@kernel.org \
--cc=linux-amlogic@lists.infradead.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-rockchip@lists.infradead.org \
--cc=lxu@maxlinear.com \
--cc=martin.blumenstingl@googlemail.com \
--cc=matthias.bgg@gmail.com \
--cc=michael.hennerich@analog.com \
--cc=michal.simek@amd.com \
--cc=neil.armstrong@linaro.org \
--cc=netdev@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=pabeni@redhat.com \
--cc=pgwipeout@gmail.com \
--cc=piergiorgio.beruto@gmail.com \
--cc=richardcochran@gmail.com \
--cc=rjui@broadcom.com \
--cc=robimarko@gmail.com \
--cc=rust-for-linux@vger.kernel.org \
--cc=sbranden@broadcom.com \
--cc=vladimir.oltean@nxp.com \
--cc=wedsonaf@gmail.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).