>From 1f2a514fe6095e586203af5ffd1b0653d72d1513 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Fri, 15 Nov 2024 18:08:56 +0200 Subject: [PATCH] net: phylink: pretty-print Kconfig name and status of PHY missing driver Figuring out, based on the PHY ID, which driver is appropriate is not always easy. This involves walking the source code and masking the PHY ID we can get from sysfs or other places with the phy_driver phy_id_mask. Typically users care about the PHY ID when there isn't a matching driver already loaded. That's a problem, because exactly then is when the kernel also has no idea what PHY driver might be adequate for a PHY ID. My idea is to move each driver's struct mdio_device_id to a separate stub file, always compiled into phylib. I've implemented this for the Marvell and BCM84881 drivers, but it will have to scale for all PHY drivers eventually. The good part is that it can be rolled out gradually, and not all PHY drivers need to use this infrastructure from the get go. Signed-off-by: Vladimir Oltean --- drivers/net/phy/Makefile | 3 + drivers/net/phy/bcm84881.c | 7 +-- drivers/net/phy/bcm84881_stubs.c | 10 ++++ drivers/net/phy/bcm84881_stubs.h | 3 + drivers/net/phy/marvell.c | 28 +-------- drivers/net/phy/marvell_stubs.c | 32 ++++++++++ drivers/net/phy/marvell_stubs.h | 3 + drivers/net/phy/phy_driver_stubs.c | 96 ++++++++++++++++++++++++++++++ drivers/net/phy/phy_driver_stubs.h | 17 ++++++ drivers/net/phy/phylink.c | 13 +++- include/linux/phy.h | 2 + 11 files changed, 181 insertions(+), 33 deletions(-) create mode 100644 drivers/net/phy/bcm84881_stubs.c create mode 100644 drivers/net/phy/bcm84881_stubs.h create mode 100644 drivers/net/phy/marvell_stubs.c create mode 100644 drivers/net/phy/marvell_stubs.h create mode 100644 drivers/net/phy/phy_driver_stubs.c create mode 100644 drivers/net/phy/phy_driver_stubs.h diff --git a/drivers/net/phy/Makefile b/drivers/net/phy/Makefile index e6145153e837..123effc6fac9 100644 --- a/drivers/net/phy/Makefile +++ b/drivers/net/phy/Makefile @@ -14,6 +14,9 @@ endif # dedicated loadable module, so we bundle them all together into libphy.ko ifdef CONFIG_PHYLIB libphy-y += $(mdio-bus-y) +libphy-y += phy_driver_stubs.o +libphy-y += bcm84881_stubs.o +libphy-y += marvell_stubs.o # the stubs are built-in whenever PHYLIB is built-in or module obj-y += stubs.o else diff --git a/drivers/net/phy/bcm84881.c b/drivers/net/phy/bcm84881.c index 97da3aee4942..436c028074ca 100644 --- a/drivers/net/phy/bcm84881.c +++ b/drivers/net/phy/bcm84881.c @@ -16,6 +16,8 @@ #include #include +#include "bcm84881_stubs.h" + enum { MDIO_AN_C22 = 0xffe0, }; @@ -251,11 +253,6 @@ static struct phy_driver bcm84881_drivers[] = { module_phy_driver(bcm84881_drivers); -/* FIXME: module auto-loading for Clause 45 PHYs seems non-functional */ -static struct mdio_device_id __maybe_unused bcm84881_tbl[] = { - { 0xae025150, 0xfffffff0 }, - { }, -}; MODULE_AUTHOR("Russell King"); MODULE_DESCRIPTION("Broadcom BCM84881 PHY driver"); MODULE_DEVICE_TABLE(mdio, bcm84881_tbl); diff --git a/drivers/net/phy/bcm84881_stubs.c b/drivers/net/phy/bcm84881_stubs.c new file mode 100644 index 000000000000..cb646c8b89fd --- /dev/null +++ b/drivers/net/phy/bcm84881_stubs.c @@ -0,0 +1,10 @@ +#include "bcm84881_stubs.h" +#include "phy_driver_stubs.h" + +/* FIXME: module auto-loading for Clause 45 PHYs seems non-functional */ +struct mdio_device_id __maybe_unused bcm84881_tbl[] = { + { 0xae025150, 0xfffffff0 }, + { }, +}; + +PHY_DRIVER_STUBS(bcm84881_tbl, CONFIG_BCM84881_PHY); diff --git a/drivers/net/phy/bcm84881_stubs.h b/drivers/net/phy/bcm84881_stubs.h new file mode 100644 index 000000000000..d28797596720 --- /dev/null +++ b/drivers/net/phy/bcm84881_stubs.h @@ -0,0 +1,3 @@ +#include + +extern struct mdio_device_id __maybe_unused bcm84881_tbl[]; diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c index cd50cd6a7f75..b3313bb8681b 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -38,6 +38,8 @@ #include #include +#include "marvell_stubs.h" + #define MII_MARVELL_PHY_PAGE 22 #define MII_MARVELL_COPPER_PAGE 0x00 #define MII_MARVELL_FIBER_PAGE 0x01 @@ -4143,30 +4145,4 @@ static struct phy_driver marvell_drivers[] = { module_phy_driver(marvell_drivers); -static struct mdio_device_id __maybe_unused marvell_tbl[] = { - { MARVELL_PHY_ID_88E1101, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E3082, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1112, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1111, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1111_FINISAR, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1118, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1121R, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1145, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1149R, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1240, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1318S, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1116R, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1510, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1540, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1545, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E3016, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E6250_FAMILY, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E6341_FAMILY, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E6390_FAMILY, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E6393_FAMILY, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1340S, MARVELL_PHY_ID_MASK }, - { MARVELL_PHY_ID_88E1548P, MARVELL_PHY_ID_MASK }, - { } -}; - MODULE_DEVICE_TABLE(mdio, marvell_tbl); diff --git a/drivers/net/phy/marvell_stubs.c b/drivers/net/phy/marvell_stubs.c new file mode 100644 index 000000000000..f4770c9b38f7 --- /dev/null +++ b/drivers/net/phy/marvell_stubs.c @@ -0,0 +1,32 @@ +#include + +#include "marvell_stubs.h" +#include "phy_driver_stubs.h" + +struct mdio_device_id __maybe_unused marvell_tbl[] = { + { MARVELL_PHY_ID_88E1101, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E3082, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1112, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1111, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1111_FINISAR, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1118, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1121R, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1145, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1149R, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1240, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1318S, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1116R, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1510, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1540, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1545, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E3016, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E6250_FAMILY, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E6341_FAMILY, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E6390_FAMILY, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E6393_FAMILY, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1340S, MARVELL_PHY_ID_MASK }, + { MARVELL_PHY_ID_88E1548P, MARVELL_PHY_ID_MASK }, + { } +}; + +PHY_DRIVER_STUBS(marvell_tbl, CONFIG_MARVELL_PHY); diff --git a/drivers/net/phy/marvell_stubs.h b/drivers/net/phy/marvell_stubs.h new file mode 100644 index 000000000000..2ba1f5af526f --- /dev/null +++ b/drivers/net/phy/marvell_stubs.h @@ -0,0 +1,3 @@ +#include + +extern struct mdio_device_id __maybe_unused marvell_tbl[]; diff --git a/drivers/net/phy/phy_driver_stubs.c b/drivers/net/phy/phy_driver_stubs.c new file mode 100644 index 000000000000..bffb9788d2b9 --- /dev/null +++ b/drivers/net/phy/phy_driver_stubs.c @@ -0,0 +1,96 @@ +#include + +#include "phy_driver_stubs.h" + +struct phy_driver_stub { + u32 phy_id; + u32 phy_id_mask; + const char *kconfig_name; + const char *kconfig_status; + struct list_head list; +}; + +static LIST_HEAD(phy_driver_stubs); +static DEFINE_MUTEX(phy_driver_stubs_lock); + +int phy_driver_stubs_register(const struct mdio_device_id *device_tbl, + const char *kconfig_name, + const char *kconfig_status) +{ + size_t i; + + mutex_lock(&phy_driver_stubs_lock); + + for (i = 0; device_tbl[i].phy_id_mask; i++) { + struct phy_driver_stub *stub; + + stub = kzalloc(sizeof(*stub), GFP_KERNEL); + if (!stub) { + while (--i >= 0) { + stub = list_last_entry(&phy_driver_stubs, + struct phy_driver_stub, + list); + list_del(&stub->list); + kfree(stub); + } + mutex_unlock(&phy_driver_stubs_lock); + return -ENOMEM; + } + + stub->phy_id = device_tbl[i].phy_id; + stub->phy_id_mask = device_tbl[i].phy_id_mask; + stub->kconfig_name = kconfig_name; + stub->kconfig_status = kconfig_status; + list_add_tail(&stub->list, &phy_driver_stubs); + } + + mutex_unlock(&phy_driver_stubs_lock); + + return 0; +} + +const char *phy_library_driver_kconfig_name(u32 phy_id) +{ + const char *kconfig_name = NULL; + struct phy_driver_stub *stub; + + mutex_lock(&phy_driver_stubs_lock); + + list_for_each_entry(stub, &phy_driver_stubs, list) { + if ((phy_id & stub->phy_id_mask) != stub->phy_id) + continue; + + /* Safe to return this pointer after unlocking the mutex, + * because we never remove the stubs from memory + */ + kconfig_name = stub->kconfig_name; + break; + } + + mutex_unlock(&phy_driver_stubs_lock); + + return kconfig_name; +} + +const char *phy_library_driver_kconfig_status(u32 phy_id) +{ + const char *kconfig_status = NULL; + struct phy_driver_stub *stub; + + mutex_lock(&phy_driver_stubs_lock); + + list_for_each_entry(stub, &phy_driver_stubs, list) { + if ((phy_id & stub->phy_id_mask) != stub->phy_id) + continue; + + /* Safe to return this pointer after unlocking the mutex, + * because we never remove the stubs from memory + */ + kconfig_status = stub->kconfig_status; + break; + } + + mutex_unlock(&phy_driver_stubs_lock); + + return kconfig_status; +} diff --git a/drivers/net/phy/phy_driver_stubs.h b/drivers/net/phy/phy_driver_stubs.h new file mode 100644 index 000000000000..66b1165a658b --- /dev/null +++ b/drivers/net/phy/phy_driver_stubs.h @@ -0,0 +1,17 @@ +#include +#include + +int phy_driver_stubs_register(const struct mdio_device_id *device_tbl, + const char *kconfig_name, + const char *kconfig_status); + +#define PHY_DRIVER_STUBS(device_id_tbl, kconfig_name) \ +static int __init phy_driver_register_stubs(void) \ +{ \ + return phy_driver_stubs_register(device_id_tbl, \ + __stringify(kconfig_name), \ + IS_MODULE(kconfig_name) ? "compiled as module" : \ + IS_BUILTIN(kconfig_name) ? "built into the kernel" : \ + "disabled"); \ +} \ +module_init(phy_driver_register_stubs) diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c index efeff8733a52..ea8f8a47d11a 100644 --- a/drivers/net/phy/phylink.c +++ b/drivers/net/phy/phylink.c @@ -3220,8 +3220,17 @@ static int phylink_sfp_config_phy(struct phylink *pl, u8 mode, linkmode_copy(support, phy->supported); if (linkmode_empty(support)) { - phylink_err(pl, "PHY %s (id 0x%.8lx) supports no link modes. Maybe its specific PHY driver not loaded?\n", - phydev_name(phy), (unsigned long)phy->phy_id); + const char *kconfig_name; + + kconfig_name = phy_library_driver_kconfig_name(phy->phy_id); + if (kconfig_name) { + phylink_err(pl, "PHY %s supports no link modes. Its driver, %s, is %s.\n", + phydev_name(phy), kconfig_name, + phy_library_driver_kconfig_status(phy->phy_id)); + } else { + phylink_err(pl, "PHY %s (id 0x%.8lx) supports no link modes, and appears to have no known driver\n", + phydev_name(phy), (unsigned long)phy->phy_id); + } return -EINVAL; } diff --git a/include/linux/phy.h b/include/linux/phy.h index 1e4127c495c0..c4ab5a1d225e 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -2214,5 +2214,7 @@ module_exit(phy_module_exit) bool phy_driver_is_genphy(struct phy_device *phydev); bool phy_driver_is_genphy_10g(struct phy_device *phydev); +const char *phy_library_driver_kconfig_name(u32 phy_id); +const char *phy_library_driver_kconfig_status(u32 phy_id); #endif /* __PHY_H */ -- 2.43.0