The struct mdio_driver_common is a wrapper for driver-model structure,
it contains device_driver and flags. There are only struct phy_driver
and mdio_driver that use it. The flags is used to distinguish between
struct phy_driver and mdio_driver.
We can test that if probe of device_driver is equal to phy_probe. This
way, the struct mdio_driver_common is no longer needed, and struct
phy_driver and usb_mdio_driver will be consistent with other driver
structs.
Cleanup struct mdio_driver_common and introduce is_phy_driver(). Use
is_phy_driver() test that if the driver is a phy or not.
Signed-off-by: Yajun Deng <[email protected]>
---
drivers/net/dsa/b53/b53_mdio.c | 2 +-
drivers/net/dsa/dsa_loop.c | 2 +-
drivers/net/dsa/lan9303_mdio.c | 2 +-
drivers/net/dsa/microchip/ksz8863_smi.c | 2 +-
drivers/net/dsa/mt7530-mdio.c | 2 +-
drivers/net/dsa/mv88e6060.c | 2 +-
drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
drivers/net/dsa/qca/ar9331.c | 2 +-
drivers/net/dsa/qca/qca8k-8xxx.c | 2 +-
drivers/net/dsa/realtek/realtek-mdio.c | 2 +-
drivers/net/dsa/xrs700x/xrs700x_mdio.c | 2 +-
drivers/net/phy/mdio_bus.c | 2 +-
drivers/net/phy/mdio_device.c | 21 +++++++--------
drivers/net/phy/phy_device.c | 35 ++++++++++++++-----------
drivers/net/phy/xilinx_gmii2rgmii.c | 2 +-
drivers/phy/broadcom/phy-bcm-ns-usb3.c | 8 +++---
drivers/phy/broadcom/phy-bcm-ns2-pcie.c | 8 +++---
include/linux/mdio.h | 16 ++---------
include/linux/phy.h | 9 +++----
19 files changed, 54 insertions(+), 69 deletions(-)
diff --git a/drivers/net/dsa/b53/b53_mdio.c b/drivers/net/dsa/b53/b53_mdio.c
index 897e5e8b3d69..1ececa4d44e4 100644
--- a/drivers/net/dsa/b53/b53_mdio.c
+++ b/drivers/net/dsa/b53/b53_mdio.c
@@ -392,7 +392,7 @@ static struct mdio_driver b53_mdio_driver = {
.probe = b53_mdio_probe,
.remove = b53_mdio_remove,
.shutdown = b53_mdio_shutdown,
- .mdiodrv.driver = {
+ .driver = {
.name = "bcm53xx",
.of_match_table = b53_of_match,
},
diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
index c70ed67cc188..3f885878be3a 100644
--- a/drivers/net/dsa/dsa_loop.c
+++ b/drivers/net/dsa/dsa_loop.c
@@ -375,7 +375,7 @@ static void dsa_loop_drv_shutdown(struct mdio_device *mdiodev)
}
static struct mdio_driver dsa_loop_drv = {
- .mdiodrv.driver = {
+ .driver = {
.name = "dsa-loop",
},
.probe = dsa_loop_drv_probe,
diff --git a/drivers/net/dsa/lan9303_mdio.c b/drivers/net/dsa/lan9303_mdio.c
index 167a86f39f27..7cb7e2b1478a 100644
--- a/drivers/net/dsa/lan9303_mdio.c
+++ b/drivers/net/dsa/lan9303_mdio.c
@@ -162,7 +162,7 @@ static const struct of_device_id lan9303_mdio_of_match[] = {
MODULE_DEVICE_TABLE(of, lan9303_mdio_of_match);
static struct mdio_driver lan9303_mdio_driver = {
- .mdiodrv.driver = {
+ .driver = {
.name = "LAN9303_MDIO",
.of_match_table = lan9303_mdio_of_match,
},
diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c
index 5711a59e2ac9..c788cadd7595 100644
--- a/drivers/net/dsa/microchip/ksz8863_smi.c
+++ b/drivers/net/dsa/microchip/ksz8863_smi.c
@@ -213,7 +213,7 @@ static struct mdio_driver ksz8863_driver = {
.probe = ksz8863_smi_probe,
.remove = ksz8863_smi_remove,
.shutdown = ksz8863_smi_shutdown,
- .mdiodrv.driver = {
+ .driver = {
.name = "ksz8863-switch",
.of_match_table = ksz8863_dt_ids,
},
diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
index 088533663b83..7315654a6757 100644
--- a/drivers/net/dsa/mt7530-mdio.c
+++ b/drivers/net/dsa/mt7530-mdio.c
@@ -258,7 +258,7 @@ static struct mdio_driver mt7530_mdio_driver = {
.probe = mt7530_probe,
.remove = mt7530_remove,
.shutdown = mt7530_shutdown,
- .mdiodrv.driver = {
+ .driver = {
.name = "mt7530-mdio",
.of_match_table = mt7530_of_match,
},
diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
index 294312b58e4f..5925f23e7ab3 100644
--- a/drivers/net/dsa/mv88e6060.c
+++ b/drivers/net/dsa/mv88e6060.c
@@ -367,7 +367,7 @@ static struct mdio_driver mv88e6060_driver = {
.probe = mv88e6060_probe,
.remove = mv88e6060_remove,
.shutdown = mv88e6060_shutdown,
- .mdiodrv.driver = {
+ .driver = {
.name = "mv88e6060",
.of_match_table = mv88e6060_of_match,
},
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 383b3c4d6f59..4f24699264d1 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -7258,7 +7258,7 @@ static struct mdio_driver mv88e6xxx_driver = {
.probe = mv88e6xxx_probe,
.remove = mv88e6xxx_remove,
.shutdown = mv88e6xxx_shutdown,
- .mdiodrv.driver = {
+ .driver = {
.name = "mv88e6085",
.of_match_table = mv88e6xxx_of_match,
.pm = &mv88e6xxx_pm_ops,
diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
index 8d9d271ac3af..da392d60c9e7 100644
--- a/drivers/net/dsa/qca/ar9331.c
+++ b/drivers/net/dsa/qca/ar9331.c
@@ -1122,7 +1122,7 @@ static struct mdio_driver ar9331_sw_mdio_driver = {
.probe = ar9331_sw_probe,
.remove = ar9331_sw_remove,
.shutdown = ar9331_sw_shutdown,
- .mdiodrv.driver = {
+ .driver = {
.name = AR9331_SW_NAME,
.of_match_table = ar9331_sw_of_match,
},
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index ec57d9d52072..fe396397f405 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -2187,7 +2187,7 @@ static struct mdio_driver qca8kmdio_driver = {
.probe = qca8k_sw_probe,
.remove = qca8k_sw_remove,
.shutdown = qca8k_sw_shutdown,
- .mdiodrv.driver = {
+ .driver = {
.name = "qca8k",
.of_match_table = qca8k_of_match,
.pm = &qca8k_pm_ops,
diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
index 292e6d087e8b..8e6a951b391c 100644
--- a/drivers/net/dsa/realtek/realtek-mdio.c
+++ b/drivers/net/dsa/realtek/realtek-mdio.c
@@ -274,7 +274,7 @@ static const struct of_device_id realtek_mdio_of_match[] = {
MODULE_DEVICE_TABLE(of, realtek_mdio_of_match);
static struct mdio_driver realtek_mdio_driver = {
- .mdiodrv.driver = {
+ .driver = {
.name = "realtek-mdio",
.of_match_table = realtek_mdio_of_match,
},
diff --git a/drivers/net/dsa/xrs700x/xrs700x_mdio.c b/drivers/net/dsa/xrs700x/xrs700x_mdio.c
index 5f7d344b5d73..1a76d9d49f13 100644
--- a/drivers/net/dsa/xrs700x/xrs700x_mdio.c
+++ b/drivers/net/dsa/xrs700x/xrs700x_mdio.c
@@ -164,7 +164,7 @@ static const struct of_device_id __maybe_unused xrs700x_mdio_dt_ids[] = {
MODULE_DEVICE_TABLE(of, xrs700x_mdio_dt_ids);
static struct mdio_driver xrs700x_mdio_driver = {
- .mdiodrv.driver = {
+ .driver = {
.name = "xrs700x-mdio",
.of_match_table = of_match_ptr(xrs700x_mdio_dt_ids),
},
diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
index 6cf73c15635b..a1092c641d14 100644
--- a/drivers/net/phy/mdio_bus.c
+++ b/drivers/net/phy/mdio_bus.c
@@ -1342,7 +1342,7 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv)
struct mdio_device *mdio = to_mdio_device(dev);
/* Both the driver and device must type-match */
- if (!(mdiodrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY) !=
+ if (!(is_phy_driver(&mdiodrv->driver)) !=
!(mdio->flags & MDIO_DEVICE_FLAG_PHY))
return 0;
diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
index 73f6539b9e50..16232e7a1255 100644
--- a/drivers/net/phy/mdio_device.c
+++ b/drivers/net/phy/mdio_device.c
@@ -40,7 +40,7 @@ int mdio_device_bus_match(struct device *dev, struct device_driver *drv)
struct mdio_device *mdiodev = to_mdio_device(dev);
struct mdio_driver *mdiodrv = to_mdio_driver(drv);
- if (mdiodrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY)
+ if (is_phy_driver(&mdiodrv->driver))
return 0;
return strcmp(mdiodev->modalias, drv->name) == 0;
@@ -203,20 +203,19 @@ static void mdio_shutdown(struct device *dev)
*/
int mdio_driver_register(struct mdio_driver *drv)
{
- struct mdio_driver_common *mdiodrv = &drv->mdiodrv;
int retval;
- pr_debug("%s: %s\n", __func__, mdiodrv->driver.name);
+ pr_debug("%s: %s\n", __func__, drv->driver.name);
- mdiodrv->driver.bus = &mdio_bus_type;
- mdiodrv->driver.probe = mdio_probe;
- mdiodrv->driver.remove = mdio_remove;
- mdiodrv->driver.shutdown = mdio_shutdown;
+ drv->driver.bus = &mdio_bus_type;
+ drv->driver.probe = mdio_probe;
+ drv->driver.remove = mdio_remove;
+ drv->driver.shutdown = mdio_shutdown;
- retval = driver_register(&mdiodrv->driver);
+ retval = driver_register(&drv->driver);
if (retval) {
pr_err("%s: Error %d in registering driver\n",
- mdiodrv->driver.name, retval);
+ drv->driver.name, retval);
return retval;
}
@@ -227,8 +226,6 @@ EXPORT_SYMBOL(mdio_driver_register);
void mdio_driver_unregister(struct mdio_driver *drv)
{
- struct mdio_driver_common *mdiodrv = &drv->mdiodrv;
-
- driver_unregister(&mdiodrv->driver);
+ driver_unregister(&drv->driver);
}
EXPORT_SYMBOL(mdio_driver_unregister);
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index 3611ea64875e..55494a345bd4 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -529,7 +529,7 @@ static int phy_bus_match(struct device *dev, struct device_driver *drv)
const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
int i;
- if (!(phydrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY))
+ if (!(is_phy_driver(&phydrv->driver)))
return 0;
if (phydrv->match_phy_device)
@@ -1456,9 +1456,9 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
*/
if (!d->driver) {
if (phydev->is_c45)
- d->driver = &genphy_c45_driver.mdiodrv.driver;
+ d->driver = &genphy_c45_driver.driver;
else
- d->driver = &genphy_driver.mdiodrv.driver;
+ d->driver = &genphy_driver.driver;
using_genphy = true;
}
@@ -1638,14 +1638,14 @@ static bool phy_driver_is_genphy_kind(struct phy_device *phydev,
bool phy_driver_is_genphy(struct phy_device *phydev)
{
return phy_driver_is_genphy_kind(phydev,
- &genphy_driver.mdiodrv.driver);
+ &genphy_driver.driver);
}
EXPORT_SYMBOL_GPL(phy_driver_is_genphy);
bool phy_driver_is_genphy_10g(struct phy_device *phydev)
{
return phy_driver_is_genphy_kind(phydev,
- &genphy_c45_driver.mdiodrv.driver);
+ &genphy_c45_driver.driver);
}
EXPORT_SYMBOL_GPL(phy_driver_is_genphy_10g);
@@ -3410,6 +3410,12 @@ static int phy_remove(struct device *dev)
return 0;
}
+bool is_phy_driver(struct device_driver *driver)
+{
+ return driver->probe == phy_probe;
+}
+EXPORT_SYMBOL_GPL(is_phy_driver);
+
/**
* phy_driver_register - register a phy_driver with the PHY layer
* @new_driver: new phy_driver to register
@@ -3433,20 +3439,19 @@ int phy_driver_register(struct phy_driver *new_driver, struct module *owner)
* is backed by a struct phy_device. If such a case happens, we will
* make out-of-bounds accesses and lockup in phydev->lock.
*/
- if (WARN(new_driver->mdiodrv.driver.of_match_table,
+ if (WARN(new_driver->driver.of_match_table,
"%s: driver must not provide a DT match table\n",
new_driver->name))
return -EINVAL;
- new_driver->mdiodrv.flags |= MDIO_DEVICE_IS_PHY;
- new_driver->mdiodrv.driver.name = new_driver->name;
- new_driver->mdiodrv.driver.bus = &mdio_bus_type;
- new_driver->mdiodrv.driver.probe = phy_probe;
- new_driver->mdiodrv.driver.remove = phy_remove;
- new_driver->mdiodrv.driver.owner = owner;
- new_driver->mdiodrv.driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
+ new_driver->driver.name = new_driver->name;
+ new_driver->driver.bus = &mdio_bus_type;
+ new_driver->driver.probe = phy_probe;
+ new_driver->driver.remove = phy_remove;
+ new_driver->driver.owner = owner;
+ new_driver->driver.probe_type = PROBE_FORCE_SYNCHRONOUS;
- retval = driver_register(&new_driver->mdiodrv.driver);
+ retval = driver_register(&new_driver->driver);
if (retval) {
pr_err("%s: Error %d in registering driver\n",
new_driver->name, retval);
@@ -3479,7 +3484,7 @@ EXPORT_SYMBOL(phy_drivers_register);
void phy_driver_unregister(struct phy_driver *drv)
{
- driver_unregister(&drv->mdiodrv.driver);
+ driver_unregister(&drv->driver);
}
EXPORT_SYMBOL(phy_driver_unregister);
diff --git a/drivers/net/phy/xilinx_gmii2rgmii.c b/drivers/net/phy/xilinx_gmii2rgmii.c
index 7fd9fe6a602b..94ba87dc1975 100644
--- a/drivers/net/phy/xilinx_gmii2rgmii.c
+++ b/drivers/net/phy/xilinx_gmii2rgmii.c
@@ -129,7 +129,7 @@ MODULE_DEVICE_TABLE(of, xgmiitorgmii_of_match);
static struct mdio_driver xgmiitorgmii_driver = {
.probe = xgmiitorgmii_probe,
- .mdiodrv.driver = {
+ .driver = {
.name = "xgmiitorgmii",
.of_match_table = xgmiitorgmii_of_match,
},
diff --git a/drivers/phy/broadcom/phy-bcm-ns-usb3.c b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
index 2c8b1b7dda5b..cb6e54e9a37e 100644
--- a/drivers/phy/broadcom/phy-bcm-ns-usb3.c
+++ b/drivers/phy/broadcom/phy-bcm-ns-usb3.c
@@ -229,11 +229,9 @@ static int bcm_ns_usb3_mdio_probe(struct mdio_device *mdiodev)
}
static struct mdio_driver bcm_ns_usb3_mdio_driver = {
- .mdiodrv = {
- .driver = {
- .name = "bcm_ns_mdio_usb3",
- .of_match_table = bcm_ns_usb3_id_table,
- },
+ .driver = {
+ .name = "bcm_ns_mdio_usb3",
+ .of_match_table = bcm_ns_usb3_id_table,
},
.probe = bcm_ns_usb3_mdio_probe,
};
diff --git a/drivers/phy/broadcom/phy-bcm-ns2-pcie.c b/drivers/phy/broadcom/phy-bcm-ns2-pcie.c
index 2eaa41f8fc70..d23e19527379 100644
--- a/drivers/phy/broadcom/phy-bcm-ns2-pcie.c
+++ b/drivers/phy/broadcom/phy-bcm-ns2-pcie.c
@@ -73,11 +73,9 @@ static const struct of_device_id ns2_pci_phy_of_match[] = {
MODULE_DEVICE_TABLE(of, ns2_pci_phy_of_match);
static struct mdio_driver ns2_pci_phy_driver = {
- .mdiodrv = {
- .driver = {
- .name = "phy-bcm-ns2-pci",
- .of_match_table = ns2_pci_phy_of_match,
- },
+ .driver = {
+ .name = "phy-bcm-ns2-pci",
+ .of_match_table = ns2_pci_phy_of_match,
},
.probe = ns2_pci_phy_probe,
};
diff --git a/include/linux/mdio.h b/include/linux/mdio.h
index 79ceee3c8673..852f838f52f5 100644
--- a/include/linux/mdio.h
+++ b/include/linux/mdio.h
@@ -50,22 +50,11 @@ static inline struct mdio_device *to_mdio_device(const struct device *dev)
return container_of(dev, struct mdio_device, dev);
}
-/* struct mdio_driver_common: Common to all MDIO drivers */
-struct mdio_driver_common {
- struct device_driver driver;
- int flags;
-};
#define MDIO_DEVICE_FLAG_PHY 1
-static inline struct mdio_driver_common *
-to_mdio_common_driver(const struct device_driver *driver)
-{
- return container_of(driver, struct mdio_driver_common, driver);
-}
-
/* struct mdio_driver: Generic MDIO driver */
struct mdio_driver {
- struct mdio_driver_common mdiodrv;
+ struct device_driver driver;
/*
* Called during discovery. Used to set
@@ -83,8 +72,7 @@ struct mdio_driver {
static inline struct mdio_driver *
to_mdio_driver(const struct device_driver *driver)
{
- return container_of(to_mdio_common_driver(driver), struct mdio_driver,
- mdiodrv);
+ return container_of(driver, struct mdio_driver, driver);
}
/* device driver data */
diff --git a/include/linux/phy.h b/include/linux/phy.h
index e9e85d347587..458bceb4a832 100644
--- a/include/linux/phy.h
+++ b/include/linux/phy.h
@@ -87,7 +87,6 @@ extern const int phy_10gbit_features_array[1];
#define PHY_RST_AFTER_CLK_EN 0x00000002
#define PHY_POLL_CABLE_TEST 0x00000004
#define PHY_ALWAYS_CALL_SUSPEND 0x00000008
-#define MDIO_DEVICE_IS_PHY 0x80000000
/**
* enum phy_interface_t - Interface Mode definitions
@@ -873,7 +872,7 @@ struct phy_led {
/**
* struct phy_driver - Driver structure for a particular PHY type
*
- * @mdiodrv: Data common to all MDIO devices
+ * @driver: The driver-model core driver structure.
* @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
@@ -894,7 +893,7 @@ struct phy_led {
* though it is not currently supported in the driver).
*/
struct phy_driver {
- struct mdio_driver_common mdiodrv;
+ struct device_driver driver;
u32 phy_id;
char *name;
u32 phy_id_mask;
@@ -1147,8 +1146,7 @@ struct phy_driver {
unsigned long *rules);
};
-#define to_phy_driver(d) container_of(to_mdio_common_driver(d), \
- struct phy_driver, mdiodrv)
+#define to_phy_driver(d) container_of(d, struct phy_driver, driver)
#define PHY_ANY_ID "MATCH ANY PHY"
#define PHY_ANY_UID 0xffffffff
@@ -2148,5 +2146,6 @@ module_exit(phy_module_exit)
bool phy_driver_is_genphy(struct phy_device *phydev);
bool phy_driver_is_genphy_10g(struct phy_device *phydev);
+bool is_phy_driver(struct device_driver *driver);
#endif /* __PHY_H */
--
2.25.1
On 12/28/23 08:23, Yajun Deng wrote:
> The struct mdio_driver_common is a wrapper for driver-model structure,
> it contains device_driver and flags. There are only struct phy_driver
> and mdio_driver that use it. The flags is used to distinguish between
> struct phy_driver and mdio_driver.
>
> We can test that if probe of device_driver is equal to phy_probe. This
> way, the struct mdio_driver_common is no longer needed, and struct
> phy_driver and usb_mdio_driver will be consistent with other driver
> structs.
>
> Cleanup struct mdio_driver_common and introduce is_phy_driver(). Use
> is_phy_driver() test that if the driver is a phy or not.
>
> Signed-off-by: Yajun Deng <[email protected]>
> ---
> drivers/net/dsa/b53/b53_mdio.c | 2 +-
> drivers/net/dsa/dsa_loop.c | 2 +-
> drivers/net/dsa/lan9303_mdio.c | 2 +-
> drivers/net/dsa/microchip/ksz8863_smi.c | 2 +-
> drivers/net/dsa/mt7530-mdio.c | 2 +-
> drivers/net/dsa/mv88e6060.c | 2 +-
> drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
> drivers/net/dsa/qca/ar9331.c | 2 +-
> drivers/net/dsa/qca/qca8k-8xxx.c | 2 +-
> drivers/net/dsa/realtek/realtek-mdio.c | 2 +-
> drivers/net/dsa/xrs700x/xrs700x_mdio.c | 2 +-
> drivers/net/phy/mdio_bus.c | 2 +-
> drivers/net/phy/mdio_device.c | 21 +++++++--------
> drivers/net/phy/phy_device.c | 35 ++++++++++++++-----------
> drivers/net/phy/xilinx_gmii2rgmii.c | 2 +-
> drivers/phy/broadcom/phy-bcm-ns-usb3.c | 8 +++---
> drivers/phy/broadcom/phy-bcm-ns2-pcie.c | 8 +++---
> include/linux/mdio.h | 16 ++---------
> include/linux/phy.h | 9 +++----
> 19 files changed, 54 insertions(+), 69 deletions(-)
>
some nitpicks from me,
otherwise looks fine:
Reviewed-by: Przemek Kitszel <[email protected]>
BTW, please send v2 after winter break:
https://patchwork.hopto.org/net-next.html
> diff --git a/drivers/net/dsa/b53/b53_mdio.c b/drivers/net/dsa/b53/b53_mdio.c
> index 897e5e8b3d69..1ececa4d44e4 100644
> --- a/drivers/net/dsa/b53/b53_mdio.c
> +++ b/drivers/net/dsa/b53/b53_mdio.c
> @@ -392,7 +392,7 @@ static struct mdio_driver b53_mdio_driver = {
> .probe = b53_mdio_probe,
> .remove = b53_mdio_remove,
> .shutdown = b53_mdio_shutdown,
> - .mdiodrv.driver = {
> + .driver = {
> .name = "bcm53xx",
> .of_match_table = b53_of_match,
> },
> diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
> index c70ed67cc188..3f885878be3a 100644
> --- a/drivers/net/dsa/dsa_loop.c
> +++ b/drivers/net/dsa/dsa_loop.c
> @@ -375,7 +375,7 @@ static void dsa_loop_drv_shutdown(struct mdio_device *mdiodev)
> }
>
> static struct mdio_driver dsa_loop_drv = {
> - .mdiodrv.driver = {
> + .driver = {
> .name = "dsa-loop",
> },
> .probe = dsa_loop_drv_probe,
> diff --git a/drivers/net/dsa/lan9303_mdio.c b/drivers/net/dsa/lan9303_mdio.c
> index 167a86f39f27..7cb7e2b1478a 100644
> --- a/drivers/net/dsa/lan9303_mdio.c
> +++ b/drivers/net/dsa/lan9303_mdio.c
> @@ -162,7 +162,7 @@ static const struct of_device_id lan9303_mdio_of_match[] = {
> MODULE_DEVICE_TABLE(of, lan9303_mdio_of_match);
>
> static struct mdio_driver lan9303_mdio_driver = {
> - .mdiodrv.driver = {
> + .driver = {
> .name = "LAN9303_MDIO",
> .of_match_table = lan9303_mdio_of_match,
> },
> diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c b/drivers/net/dsa/microchip/ksz8863_smi.c
> index 5711a59e2ac9..c788cadd7595 100644
> --- a/drivers/net/dsa/microchip/ksz8863_smi.c
> +++ b/drivers/net/dsa/microchip/ksz8863_smi.c
> @@ -213,7 +213,7 @@ static struct mdio_driver ksz8863_driver = {
> .probe = ksz8863_smi_probe,
> .remove = ksz8863_smi_remove,
> .shutdown = ksz8863_smi_shutdown,
> - .mdiodrv.driver = {
> + .driver = {
> .name = "ksz8863-switch",
> .of_match_table = ksz8863_dt_ids,
> },
> diff --git a/drivers/net/dsa/mt7530-mdio.c b/drivers/net/dsa/mt7530-mdio.c
> index 088533663b83..7315654a6757 100644
> --- a/drivers/net/dsa/mt7530-mdio.c
> +++ b/drivers/net/dsa/mt7530-mdio.c
> @@ -258,7 +258,7 @@ static struct mdio_driver mt7530_mdio_driver = {
> .probe = mt7530_probe,
> .remove = mt7530_remove,
> .shutdown = mt7530_shutdown,
> - .mdiodrv.driver = {
> + .driver = {
> .name = "mt7530-mdio",
> .of_match_table = mt7530_of_match,
> },
> diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
> index 294312b58e4f..5925f23e7ab3 100644
> --- a/drivers/net/dsa/mv88e6060.c
> +++ b/drivers/net/dsa/mv88e6060.c
> @@ -367,7 +367,7 @@ static struct mdio_driver mv88e6060_driver = {
> .probe = mv88e6060_probe,
> .remove = mv88e6060_remove,
> .shutdown = mv88e6060_shutdown,
> - .mdiodrv.driver = {
> + .driver = {
> .name = "mv88e6060",
> .of_match_table = mv88e6060_of_match,
> },
> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
> index 383b3c4d6f59..4f24699264d1 100644
> --- a/drivers/net/dsa/mv88e6xxx/chip.c
> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
> @@ -7258,7 +7258,7 @@ static struct mdio_driver mv88e6xxx_driver = {
> .probe = mv88e6xxx_probe,
> .remove = mv88e6xxx_remove,
> .shutdown = mv88e6xxx_shutdown,
> - .mdiodrv.driver = {
> + .driver = {
> .name = "mv88e6085",
> .of_match_table = mv88e6xxx_of_match,
> .pm = &mv88e6xxx_pm_ops,
> diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
> index 8d9d271ac3af..da392d60c9e7 100644
> --- a/drivers/net/dsa/qca/ar9331.c
> +++ b/drivers/net/dsa/qca/ar9331.c
> @@ -1122,7 +1122,7 @@ static struct mdio_driver ar9331_sw_mdio_driver = {
> .probe = ar9331_sw_probe,
> .remove = ar9331_sw_remove,
> .shutdown = ar9331_sw_shutdown,
> - .mdiodrv.driver = {
> + .driver = {
> .name = AR9331_SW_NAME,
> .of_match_table = ar9331_sw_of_match,
> },
> diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
> index ec57d9d52072..fe396397f405 100644
> --- a/drivers/net/dsa/qca/qca8k-8xxx.c
> +++ b/drivers/net/dsa/qca/qca8k-8xxx.c
> @@ -2187,7 +2187,7 @@ static struct mdio_driver qca8kmdio_driver = {
> .probe = qca8k_sw_probe,
> .remove = qca8k_sw_remove,
> .shutdown = qca8k_sw_shutdown,
> - .mdiodrv.driver = {
> + .driver = {
> .name = "qca8k",
> .of_match_table = qca8k_of_match,
> .pm = &qca8k_pm_ops,
> diff --git a/drivers/net/dsa/realtek/realtek-mdio.c b/drivers/net/dsa/realtek/realtek-mdio.c
> index 292e6d087e8b..8e6a951b391c 100644
> --- a/drivers/net/dsa/realtek/realtek-mdio.c
> +++ b/drivers/net/dsa/realtek/realtek-mdio.c
> @@ -274,7 +274,7 @@ static const struct of_device_id realtek_mdio_of_match[] = {
> MODULE_DEVICE_TABLE(of, realtek_mdio_of_match);
>
> static struct mdio_driver realtek_mdio_driver = {
> - .mdiodrv.driver = {
> + .driver = {
> .name = "realtek-mdio",
> .of_match_table = realtek_mdio_of_match,
> },
> diff --git a/drivers/net/dsa/xrs700x/xrs700x_mdio.c b/drivers/net/dsa/xrs700x/xrs700x_mdio.c
> index 5f7d344b5d73..1a76d9d49f13 100644
> --- a/drivers/net/dsa/xrs700x/xrs700x_mdio.c
> +++ b/drivers/net/dsa/xrs700x/xrs700x_mdio.c
> @@ -164,7 +164,7 @@ static const struct of_device_id __maybe_unused xrs700x_mdio_dt_ids[] = {
> MODULE_DEVICE_TABLE(of, xrs700x_mdio_dt_ids);
>
> static struct mdio_driver xrs700x_mdio_driver = {
> - .mdiodrv.driver = {
> + .driver = {
> .name = "xrs700x-mdio",
> .of_match_table = of_match_ptr(xrs700x_mdio_dt_ids),
> },
> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
> index 6cf73c15635b..a1092c641d14 100644
> --- a/drivers/net/phy/mdio_bus.c
> +++ b/drivers/net/phy/mdio_bus.c
> @@ -1342,7 +1342,7 @@ static int mdio_bus_match(struct device *dev, struct device_driver *drv)
> struct mdio_device *mdio = to_mdio_device(dev);
>
> /* Both the driver and device must type-match */
> - if (!(mdiodrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY) !=
> + if (!(is_phy_driver(&mdiodrv->driver)) !=
> !(mdio->flags & MDIO_DEVICE_FLAG_PHY))
you could remove one pair of parens, and even change condition to:
if (is_phy_driver(&mdiodrv->driver) == !(mdio->flags &
MDIO_DEVICE_FLAG_PHY))
> return 0;
>
> diff --git a/drivers/net/phy/mdio_device.c b/drivers/net/phy/mdio_device.c
> index 73f6539b9e50..16232e7a1255 100644
> --- a/drivers/net/phy/mdio_device.c
> +++ b/drivers/net/phy/mdio_device.c
> @@ -40,7 +40,7 @@ int mdio_device_bus_match(struct device *dev, struct device_driver *drv)
> struct mdio_device *mdiodev = to_mdio_device(dev);
> struct mdio_driver *mdiodrv = to_mdio_driver(drv);
>
> - if (mdiodrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY)
> + if (is_phy_driver(&mdiodrv->driver))
> return 0;
>
> return strcmp(mdiodev->modalias, drv->name) == 0;
> @@ -203,20 +203,19 @@ static void mdio_shutdown(struct device *dev)
> */
> int mdio_driver_register(struct mdio_driver *drv)
> {
> - struct mdio_driver_common *mdiodrv = &drv->mdiodrv;
> int retval;
>
> - pr_debug("%s: %s\n", __func__, mdiodrv->driver.name);
> + pr_debug("%s: %s\n", __func__, drv->driver.name);
>
> - mdiodrv->driver.bus = &mdio_bus_type;
> - mdiodrv->driver.probe = mdio_probe;
> - mdiodrv->driver.remove = mdio_remove;
> - mdiodrv->driver.shutdown = mdio_shutdown;
> + drv->driver.bus = &mdio_bus_type;
> + drv->driver.probe = mdio_probe;
> + drv->driver.remove = mdio_remove;
> + drv->driver.shutdown = mdio_shutdown;
>
> - retval = driver_register(&mdiodrv->driver);
> + retval = driver_register(&drv->driver);
> if (retval) {
> pr_err("%s: Error %d in registering driver\n",
> - mdiodrv->driver.name, retval);
> + drv->driver.name, retval);
>
> return retval;
> }
> @@ -227,8 +226,6 @@ EXPORT_SYMBOL(mdio_driver_register);
>
> void mdio_driver_unregister(struct mdio_driver *drv)
> {
> - struct mdio_driver_common *mdiodrv = &drv->mdiodrv;
> -
> - driver_unregister(&mdiodrv->driver);
> + driver_unregister(&drv->driver);
> }
> EXPORT_SYMBOL(mdio_driver_unregister);
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 3611ea64875e..55494a345bd4 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -529,7 +529,7 @@ static int phy_bus_match(struct device *dev, struct device_driver *drv)
> const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
> int i;
>
> - if (!(phydrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY))
> + if (!(is_phy_driver(&phydrv->driver)))
here parens are redundant too
> return 0;
>
> if (phydrv->match_phy_device)
> @@ -1456,9 +1456,9 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
> */
> if (!d->driver) {
> if (phydev->is_c45)
> - d->driver = &genphy_c45_driver.mdiodrv.driver;
> + d->driver = &genphy_c45_driver.driver;
> else
> - d->driver = &genphy_driver.mdiodrv.driver;
> + d->driver = &genphy_driver.driver;
>
> using_genphy = true;
> }
> @@ -1638,14 +1638,14 @@ static bool phy_driver_is_genphy_kind(struct phy_device *phydev,
> bool phy_driver_is_genphy(struct phy_device *phydev)
> {
> return phy_driver_is_genphy_kind(phydev,
> - &genphy_driver.mdiodrv.driver);
> + &genphy_driver.driver);
> }
> EXPORT_SYMBOL_GPL(phy_driver_is_genphy);
>
> bool phy_driver_is_genphy_10g(struct phy_device *phydev)
> {
> return phy_driver_is_genphy_kind(phydev,
> - &genphy_c45_driver.mdiodrv.driver);
> + &genphy_c45_driver.driver);
now it fits into one line (same for phy_driver_is_genphy())
> }
> EXPORT_SYMBOL_GPL(phy_driver_is_genphy_10g);
[snip]
On 2023/12/28 16:24, Przemek Kitszel wrote:
> On 12/28/23 08:23, Yajun Deng wrote:
>> The struct mdio_driver_common is a wrapper for driver-model structure,
>> it contains device_driver and flags. There are only struct phy_driver
>> and mdio_driver that use it. The flags is used to distinguish between
>> struct phy_driver and mdio_driver.
>>
>> We can test that if probe of device_driver is equal to phy_probe. This
>> way, the struct mdio_driver_common is no longer needed, and struct
>> phy_driver and usb_mdio_driver will be consistent with other driver
>> structs.
>>
>> Cleanup struct mdio_driver_common and introduce is_phy_driver(). Use
>> is_phy_driver() test that if the driver is a phy or not.
>>
>> Signed-off-by: Yajun Deng <[email protected]>
>> ---
>> drivers/net/dsa/b53/b53_mdio.c | 2 +-
>> drivers/net/dsa/dsa_loop.c | 2 +-
>> drivers/net/dsa/lan9303_mdio.c | 2 +-
>> drivers/net/dsa/microchip/ksz8863_smi.c | 2 +-
>> drivers/net/dsa/mt7530-mdio.c | 2 +-
>> drivers/net/dsa/mv88e6060.c | 2 +-
>> drivers/net/dsa/mv88e6xxx/chip.c | 2 +-
>> drivers/net/dsa/qca/ar9331.c | 2 +-
>> drivers/net/dsa/qca/qca8k-8xxx.c | 2 +-
>> drivers/net/dsa/realtek/realtek-mdio.c | 2 +-
>> drivers/net/dsa/xrs700x/xrs700x_mdio.c | 2 +-
>> drivers/net/phy/mdio_bus.c | 2 +-
>> drivers/net/phy/mdio_device.c | 21 +++++++--------
>> drivers/net/phy/phy_device.c | 35 ++++++++++++++-----------
>> drivers/net/phy/xilinx_gmii2rgmii.c | 2 +-
>> drivers/phy/broadcom/phy-bcm-ns-usb3.c | 8 +++---
>> drivers/phy/broadcom/phy-bcm-ns2-pcie.c | 8 +++---
>> include/linux/mdio.h | 16 ++---------
>> include/linux/phy.h | 9 +++----
>> 19 files changed, 54 insertions(+), 69 deletions(-)
>>
>
> some nitpicks from me,
> otherwise looks fine:
> Reviewed-by: Przemek Kitszel <[email protected]>
>
> BTW, please send v2 after winter break:
> https://patchwork.hopto.org/net-next.html
>
Ok, thanks.
>
>> diff --git a/drivers/net/dsa/b53/b53_mdio.c
>> b/drivers/net/dsa/b53/b53_mdio.c
>> index 897e5e8b3d69..1ececa4d44e4 100644
>> --- a/drivers/net/dsa/b53/b53_mdio.c
>> +++ b/drivers/net/dsa/b53/b53_mdio.c
>> @@ -392,7 +392,7 @@ static struct mdio_driver b53_mdio_driver = {
>> .probe = b53_mdio_probe,
>> .remove = b53_mdio_remove,
>> .shutdown = b53_mdio_shutdown,
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "bcm53xx",
>> .of_match_table = b53_of_match,
>> },
>> diff --git a/drivers/net/dsa/dsa_loop.c b/drivers/net/dsa/dsa_loop.c
>> index c70ed67cc188..3f885878be3a 100644
>> --- a/drivers/net/dsa/dsa_loop.c
>> +++ b/drivers/net/dsa/dsa_loop.c
>> @@ -375,7 +375,7 @@ static void dsa_loop_drv_shutdown(struct
>> mdio_device *mdiodev)
>> }
>> static struct mdio_driver dsa_loop_drv = {
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "dsa-loop",
>> },
>> .probe = dsa_loop_drv_probe,
>> diff --git a/drivers/net/dsa/lan9303_mdio.c
>> b/drivers/net/dsa/lan9303_mdio.c
>> index 167a86f39f27..7cb7e2b1478a 100644
>> --- a/drivers/net/dsa/lan9303_mdio.c
>> +++ b/drivers/net/dsa/lan9303_mdio.c
>> @@ -162,7 +162,7 @@ static const struct of_device_id
>> lan9303_mdio_of_match[] = {
>> MODULE_DEVICE_TABLE(of, lan9303_mdio_of_match);
>> static struct mdio_driver lan9303_mdio_driver = {
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "LAN9303_MDIO",
>> .of_match_table = lan9303_mdio_of_match,
>> },
>> diff --git a/drivers/net/dsa/microchip/ksz8863_smi.c
>> b/drivers/net/dsa/microchip/ksz8863_smi.c
>> index 5711a59e2ac9..c788cadd7595 100644
>> --- a/drivers/net/dsa/microchip/ksz8863_smi.c
>> +++ b/drivers/net/dsa/microchip/ksz8863_smi.c
>> @@ -213,7 +213,7 @@ static struct mdio_driver ksz8863_driver = {
>> .probe = ksz8863_smi_probe,
>> .remove = ksz8863_smi_remove,
>> .shutdown = ksz8863_smi_shutdown,
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "ksz8863-switch",
>> .of_match_table = ksz8863_dt_ids,
>> },
>> diff --git a/drivers/net/dsa/mt7530-mdio.c
>> b/drivers/net/dsa/mt7530-mdio.c
>> index 088533663b83..7315654a6757 100644
>> --- a/drivers/net/dsa/mt7530-mdio.c
>> +++ b/drivers/net/dsa/mt7530-mdio.c
>> @@ -258,7 +258,7 @@ static struct mdio_driver mt7530_mdio_driver = {
>> .probe = mt7530_probe,
>> .remove = mt7530_remove,
>> .shutdown = mt7530_shutdown,
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "mt7530-mdio",
>> .of_match_table = mt7530_of_match,
>> },
>> diff --git a/drivers/net/dsa/mv88e6060.c b/drivers/net/dsa/mv88e6060.c
>> index 294312b58e4f..5925f23e7ab3 100644
>> --- a/drivers/net/dsa/mv88e6060.c
>> +++ b/drivers/net/dsa/mv88e6060.c
>> @@ -367,7 +367,7 @@ static struct mdio_driver mv88e6060_driver = {
>> .probe = mv88e6060_probe,
>> .remove = mv88e6060_remove,
>> .shutdown = mv88e6060_shutdown,
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "mv88e6060",
>> .of_match_table = mv88e6060_of_match,
>> },
>> diff --git a/drivers/net/dsa/mv88e6xxx/chip.c
>> b/drivers/net/dsa/mv88e6xxx/chip.c
>> index 383b3c4d6f59..4f24699264d1 100644
>> --- a/drivers/net/dsa/mv88e6xxx/chip.c
>> +++ b/drivers/net/dsa/mv88e6xxx/chip.c
>> @@ -7258,7 +7258,7 @@ static struct mdio_driver mv88e6xxx_driver = {
>> .probe = mv88e6xxx_probe,
>> .remove = mv88e6xxx_remove,
>> .shutdown = mv88e6xxx_shutdown,
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "mv88e6085",
>> .of_match_table = mv88e6xxx_of_match,
>> .pm = &mv88e6xxx_pm_ops,
>> diff --git a/drivers/net/dsa/qca/ar9331.c b/drivers/net/dsa/qca/ar9331.c
>> index 8d9d271ac3af..da392d60c9e7 100644
>> --- a/drivers/net/dsa/qca/ar9331.c
>> +++ b/drivers/net/dsa/qca/ar9331.c
>> @@ -1122,7 +1122,7 @@ static struct mdio_driver ar9331_sw_mdio_driver
>> = {
>> .probe = ar9331_sw_probe,
>> .remove = ar9331_sw_remove,
>> .shutdown = ar9331_sw_shutdown,
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = AR9331_SW_NAME,
>> .of_match_table = ar9331_sw_of_match,
>> },
>> diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c
>> b/drivers/net/dsa/qca/qca8k-8xxx.c
>> index ec57d9d52072..fe396397f405 100644
>> --- a/drivers/net/dsa/qca/qca8k-8xxx.c
>> +++ b/drivers/net/dsa/qca/qca8k-8xxx.c
>> @@ -2187,7 +2187,7 @@ static struct mdio_driver qca8kmdio_driver = {
>> .probe = qca8k_sw_probe,
>> .remove = qca8k_sw_remove,
>> .shutdown = qca8k_sw_shutdown,
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "qca8k",
>> .of_match_table = qca8k_of_match,
>> .pm = &qca8k_pm_ops,
>> diff --git a/drivers/net/dsa/realtek/realtek-mdio.c
>> b/drivers/net/dsa/realtek/realtek-mdio.c
>> index 292e6d087e8b..8e6a951b391c 100644
>> --- a/drivers/net/dsa/realtek/realtek-mdio.c
>> +++ b/drivers/net/dsa/realtek/realtek-mdio.c
>> @@ -274,7 +274,7 @@ static const struct of_device_id
>> realtek_mdio_of_match[] = {
>> MODULE_DEVICE_TABLE(of, realtek_mdio_of_match);
>> static struct mdio_driver realtek_mdio_driver = {
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "realtek-mdio",
>> .of_match_table = realtek_mdio_of_match,
>> },
>> diff --git a/drivers/net/dsa/xrs700x/xrs700x_mdio.c
>> b/drivers/net/dsa/xrs700x/xrs700x_mdio.c
>> index 5f7d344b5d73..1a76d9d49f13 100644
>> --- a/drivers/net/dsa/xrs700x/xrs700x_mdio.c
>> +++ b/drivers/net/dsa/xrs700x/xrs700x_mdio.c
>> @@ -164,7 +164,7 @@ static const struct of_device_id __maybe_unused
>> xrs700x_mdio_dt_ids[] = {
>> MODULE_DEVICE_TABLE(of, xrs700x_mdio_dt_ids);
>> static struct mdio_driver xrs700x_mdio_driver = {
>> - .mdiodrv.driver = {
>> + .driver = {
>> .name = "xrs700x-mdio",
>> .of_match_table = of_match_ptr(xrs700x_mdio_dt_ids),
>> },
>> diff --git a/drivers/net/phy/mdio_bus.c b/drivers/net/phy/mdio_bus.c
>> index 6cf73c15635b..a1092c641d14 100644
>> --- a/drivers/net/phy/mdio_bus.c
>> +++ b/drivers/net/phy/mdio_bus.c
>> @@ -1342,7 +1342,7 @@ static int mdio_bus_match(struct device *dev,
>> struct device_driver *drv)
>> struct mdio_device *mdio = to_mdio_device(dev);
>> /* Both the driver and device must type-match */
>> - if (!(mdiodrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY) !=
>> + if (!(is_phy_driver(&mdiodrv->driver)) !=
>> !(mdio->flags & MDIO_DEVICE_FLAG_PHY))
>
> you could remove one pair of parens, and even change condition to:
> if (is_phy_driver(&mdiodrv->driver) == !(mdio->flags &
> MDIO_DEVICE_FLAG_PHY))
>
>
>> return 0;
>> diff --git a/drivers/net/phy/mdio_device.c
>> b/drivers/net/phy/mdio_device.c
>> index 73f6539b9e50..16232e7a1255 100644
>> --- a/drivers/net/phy/mdio_device.c
>> +++ b/drivers/net/phy/mdio_device.c
>> @@ -40,7 +40,7 @@ int mdio_device_bus_match(struct device *dev,
>> struct device_driver *drv)
>> struct mdio_device *mdiodev = to_mdio_device(dev);
>> struct mdio_driver *mdiodrv = to_mdio_driver(drv);
>> - if (mdiodrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY)
>> + if (is_phy_driver(&mdiodrv->driver))
>> return 0;
>> return strcmp(mdiodev->modalias, drv->name) == 0;
>> @@ -203,20 +203,19 @@ static void mdio_shutdown(struct device *dev)
>> */
>> int mdio_driver_register(struct mdio_driver *drv)
>> {
>> - struct mdio_driver_common *mdiodrv = &drv->mdiodrv;
>> int retval;
>> - pr_debug("%s: %s\n", __func__, mdiodrv->driver.name);
>> + pr_debug("%s: %s\n", __func__, drv->driver.name);
>> - mdiodrv->driver.bus = &mdio_bus_type;
>> - mdiodrv->driver.probe = mdio_probe;
>> - mdiodrv->driver.remove = mdio_remove;
>> - mdiodrv->driver.shutdown = mdio_shutdown;
>> + drv->driver.bus = &mdio_bus_type;
>> + drv->driver.probe = mdio_probe;
>> + drv->driver.remove = mdio_remove;
>> + drv->driver.shutdown = mdio_shutdown;
>> - retval = driver_register(&mdiodrv->driver);
>> + retval = driver_register(&drv->driver);
>> if (retval) {
>> pr_err("%s: Error %d in registering driver\n",
>> - mdiodrv->driver.name, retval);
>> + drv->driver.name, retval);
>> return retval;
>> }
>> @@ -227,8 +226,6 @@ EXPORT_SYMBOL(mdio_driver_register);
>> void mdio_driver_unregister(struct mdio_driver *drv)
>> {
>> - struct mdio_driver_common *mdiodrv = &drv->mdiodrv;
>> -
>> - driver_unregister(&mdiodrv->driver);
>> + driver_unregister(&drv->driver);
>> }
>> EXPORT_SYMBOL(mdio_driver_unregister);
>> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
>> index 3611ea64875e..55494a345bd4 100644
>> --- a/drivers/net/phy/phy_device.c
>> +++ b/drivers/net/phy/phy_device.c
>> @@ -529,7 +529,7 @@ static int phy_bus_match(struct device *dev,
>> struct device_driver *drv)
>> const int num_ids = ARRAY_SIZE(phydev->c45_ids.device_ids);
>> int i;
>> - if (!(phydrv->mdiodrv.flags & MDIO_DEVICE_IS_PHY))
>> + if (!(is_phy_driver(&phydrv->driver)))
>
> here parens are redundant too
>
>> return 0;
>> if (phydrv->match_phy_device)
>> @@ -1456,9 +1456,9 @@ int phy_attach_direct(struct net_device *dev,
>> struct phy_device *phydev,
>> */
>> if (!d->driver) {
>> if (phydev->is_c45)
>> - d->driver = &genphy_c45_driver.mdiodrv.driver;
>> + d->driver = &genphy_c45_driver.driver;
>> else
>> - d->driver = &genphy_driver.mdiodrv.driver;
>> + d->driver = &genphy_driver.driver;
>> using_genphy = true;
>> }
>> @@ -1638,14 +1638,14 @@ static bool phy_driver_is_genphy_kind(struct
>> phy_device *phydev,
>> bool phy_driver_is_genphy(struct phy_device *phydev)
>> {
>> return phy_driver_is_genphy_kind(phydev,
>> - &genphy_driver.mdiodrv.driver);
>> + &genphy_driver.driver);
>> }
>> EXPORT_SYMBOL_GPL(phy_driver_is_genphy);
>> bool phy_driver_is_genphy_10g(struct phy_device *phydev)
>> {
>> return phy_driver_is_genphy_kind(phydev,
>> - &genphy_c45_driver.mdiodrv.driver);
>> + &genphy_c45_driver.driver);
>
> now it fits into one line (same for phy_driver_is_genphy())
>
>> }
>> EXPORT_SYMBOL_GPL(phy_driver_is_genphy_10g);
>
> [snip]
>
On Thu, Dec 28, 2023 at 03:23:50PM +0800, Yajun Deng wrote:
> The struct mdio_driver_common is a wrapper for driver-model structure,
> it contains device_driver and flags. There are only struct phy_driver
> and mdio_driver that use it. The flags is used to distinguish between
> struct phy_driver and mdio_driver.
>
> We can test that if probe of device_driver is equal to phy_probe. This
> way, the struct mdio_driver_common is no longer needed, and struct
> phy_driver and usb_mdio_driver will be consistent with other driver
> structs.
usb_mdio_driver?
I'm not sure why this consistency is even desired, the commit message
doesn't properly say _why_ this change is being proposed.
> +bool is_phy_driver(struct device_driver *driver)
> +{
> + return driver->probe == phy_probe;
> +}
> +EXPORT_SYMBOL_GPL(is_phy_driver);
Do we really need this exported? It doesn't seem like something anything
other than core MDIO/phylib code should know about, and all that becomes
a single module when building it in a modular way - phylib can't be a
separate module from mdio stuff.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 2024/1/3 01:34, Russell King (Oracle) wrote:
> On Thu, Dec 28, 2023 at 03:23:50PM +0800, Yajun Deng wrote:
>> The struct mdio_driver_common is a wrapper for driver-model structure,
>> it contains device_driver and flags. There are only struct phy_driver
>> and mdio_driver that use it. The flags is used to distinguish between
>> struct phy_driver and mdio_driver.
>>
>> We can test that if probe of device_driver is equal to phy_probe. This
>> way, the struct mdio_driver_common is no longer needed, and struct
>> phy_driver and usb_mdio_driver will be consistent with other driver
>> structs.
> usb_mdio_driver?
This is a mistake. It should be 'mdio_driver'.
>
> I'm not sure why this consistency is even desired, the commit message
> doesn't properly say _why_ this change is being proposed.
Most drivers use device_driver directly. This should be added to the commit.
Like this:
struct sdio_driver {
... ...
struct device_driver drv;
};
struct pcie_port_service_driver {
... ...
struct device_driver driver;
};
and so on ...
>
>> +bool is_phy_driver(struct device_driver *driver)
>> +{
>> + return driver->probe == phy_probe;
>> +}
>> +EXPORT_SYMBOL_GPL(is_phy_driver);
> Do we really need this exported? It doesn't seem like something anything
> other than core MDIO/phylib code should know about, and all that becomes
> a single module when building it in a modular way - phylib can't be a
> separate module from mdio stuff.
I think this exported can be removed.
On Wed, Jan 03, 2024 at 10:03:14AM +0800, Yajun Deng wrote:
>
> On 2024/1/3 01:34, Russell King (Oracle) wrote:
> > I'm not sure why this consistency is even desired, the commit message
> > doesn't properly say _why_ this change is being proposed.
>
> Most drivers use device_driver directly. This should be added to the commit.
>
> Like this:
>
> struct sdio_driver {
>
> ... ...
>
> ??????? struct device_driver drv;
> };
>
>
> struct pcie_port_service_driver {
>
> ... ...
>
> ??????? struct device_driver driver;
> };
>
> and so on ...
... which is fine for those other drivers because they don't share the
same bus. That is not the case here - we have two different classes
of drivers on the same bus.
I don't like a justification that just because other subsystems do
something in one particular way, that is the only way things should be
done. I think there is good reason to have the structure we have, and
thus there needs to be a good reason to change it.
Maybe Andrew has a different opinion, but I think we need a better
justification for this change.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On 2024/1/3 18:51, Russell King (Oracle) wrote:
> On Wed, Jan 03, 2024 at 10:03:14AM +0800, Yajun Deng wrote:
>> On 2024/1/3 01:34, Russell King (Oracle) wrote:
>>> I'm not sure why this consistency is even desired, the commit message
>>> doesn't properly say _why_ this change is being proposed.
>> Most drivers use device_driver directly. This should be added to the commit.
>>
>> Like this:
>>
>> struct sdio_driver {
>>
>> ... ...
>>
>> struct device_driver drv;
>> };
>>
>>
>> struct pcie_port_service_driver {
>>
>> ... ...
>>
>> struct device_driver driver;
>> };
>>
>> and so on ...
> ... which is fine for those other drivers because they don't share the
> same bus. That is not the case here - we have two different classes
> of drivers on the same bus.
Yes, that's true. But we can implement it with is_phy_driver(). I don't
think we need a flag for that.
>
> I don't like a justification that just because other subsystems do
> something in one particular way, that is the only way things should be
> done. I think there is good reason to have the structure we have, and
> thus there needs to be a good reason to change it.
Its purpose is to clean up struct mdio_driver_common, and make the code
cleaner.
> Maybe Andrew has a different opinion, but I think we need a better
> justification for this change.
>
On Wed, Jan 03, 2024 at 07:38:04PM +0800, Yajun Deng wrote:
>
> On 2024/1/3 18:51, Russell King (Oracle) wrote:
> > On Wed, Jan 03, 2024 at 10:03:14AM +0800, Yajun Deng wrote:
> > > On 2024/1/3 01:34, Russell King (Oracle) wrote:
> > > > I'm not sure why this consistency is even desired, the commit message
> > > > doesn't properly say _why_ this change is being proposed.
> > > Most drivers use device_driver directly. This should be added to the commit.
> > >
> > > Like this:
> > >
> > > struct sdio_driver {
> > >
> > > ... ...
> > >
> > > ??????? struct device_driver drv;
> > > };
> > >
> > >
> > > struct pcie_port_service_driver {
> > >
> > > ... ...
> > >
> > > ??????? struct device_driver driver;
> > > };
> > >
> > > and so on ...
> > ... which is fine for those other drivers because they don't share the
> > same bus. That is not the case here - we have two different classes
> > of drivers on the same bus.
>
>
> Yes, that's true. But we can implement it with is_phy_driver(). I don't
> think we need a flag for that.
>
> >
> > I don't like a justification that just because other subsystems do
> > something in one particular way, that is the only way things should be
> > done. I think there is good reason to have the structure we have, and
> > thus there needs to be a good reason to change it.
>
> Its purpose is to clean up struct mdio_driver_common, and make the code
> cleaner.
I have to agree with Russell here. The commit message should explain
the 'Why?'. Why is this better? Why is it cleaner? Why does making it
the same as all other drivers make it better, when in fact we have two
classes of devices stacked on top of it, and making it different to
every other driver actually helps developers realise that?
Andrew
Hi Yajun,
kernel test robot noticed the following build errors:
[auto build test ERROR on net-next/main]
url: https://github.com/intel-lab-lkp/linux/commits/Yajun-Deng/net-phy-Cleanup-struct-mdio_driver_common/20231228-152806
base: net-next/main
patch link: https://lore.kernel.org/r/20231228072350.1294425-1-yajun.deng%40linux.dev
patch subject: [PATCH net-next] net: phy: Cleanup struct mdio_driver_common
config: s390-randconfig-002-20240103 (https://download.01.org/0day-ci/archive/20240104/[email protected]/config)
compiler: s390-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240104/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
s390-linux-ld: drivers/net/phy/mdio_bus.o: in function `mdio_bus_match':
>> mdio_bus.c:(.text+0xecc): undefined reference to `is_phy_driver'
s390-linux-ld: drivers/net/phy/mdio_device.o: in function `mdio_device_bus_match':
>> mdio_device.c:(.text+0x934): undefined reference to `is_phy_driver'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki