| Message ID | ea6fde13-9183-4c7c-8434-6c0eb64fc72c@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | ea47e70e476ffb3fc8969c842d95609da24266b1 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | net: phy: clean up phy.h | expand |
On 2/13/2025 10:48 PM, Heiner Kallweit wrote: > Certain fixup-related definitions aren't used outside phy_device.c. > So make them private and remove them from phy.h. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/phy/phy_device.c | 16 +++++++++++++--- > include/linux/phy.h | 14 -------------- > 2 files changed, 13 insertions(+), 17 deletions(-) > > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > index 9b06ba92f..14c312ad2 100644 > --- a/drivers/net/phy/phy_device.c > +++ b/drivers/net/phy/phy_device.c > @@ -45,6 +45,17 @@ MODULE_DESCRIPTION("PHY library"); > MODULE_AUTHOR("Andy Fleming"); > MODULE_LICENSE("GPL"); > > +#define PHY_ANY_ID "MATCH ANY PHY" > +#define PHY_ANY_UID 0xffffffff > + Overall looks like a nice cleanup but I am not sure about this space between #define and PHY_ANY_ID or PHY_ANY_UID... > +struct phy_fixup { > + struct list_head list; > + char bus_id[MII_BUS_ID_SIZE + 3]; > + u32 phy_uid; > + u32 phy_uid_mask; > + int (*run)(struct phy_device *phydev); > +}; > + > __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_basic_features) __ro_after_init; > EXPORT_SYMBOL_GPL(phy_basic_features); > > @@ -378,8 +389,8 @@ static SIMPLE_DEV_PM_OPS(mdio_bus_phy_pm_ops, mdio_bus_phy_suspend, > * comparison > * @run: The actual code to be run when a matching PHY is found > */ > -int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, > - int (*run)(struct phy_device *)) > +static int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, > + int (*run)(struct phy_device *)) > { > struct phy_fixup *fixup = kzalloc(sizeof(*fixup), GFP_KERNEL); > > @@ -397,7 +408,6 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, > > return 0; > } > -EXPORT_SYMBOL(phy_register_fixup); > > /* Registers a fixup to be run on any PHY with the UID in phy_uid */ > int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask, > diff --git a/include/linux/phy.h b/include/linux/phy.h > index 29df4c602..96e427c2c 100644 > --- a/include/linux/phy.h > +++ b/include/linux/phy.h > @@ -1277,9 +1277,6 @@ struct phy_driver { > #define to_phy_driver(d) container_of_const(to_mdio_common_driver(d), \ > struct phy_driver, mdiodrv) > > -#define PHY_ANY_ID "MATCH ANY PHY" > -#define PHY_ANY_UID 0xffffffff > - > #define PHY_ID_MATCH_EXACT(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 0) > #define PHY_ID_MATCH_MODEL(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 4) > #define PHY_ID_MATCH_VENDOR(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 10) > @@ -1312,15 +1309,6 @@ 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); > } > > -/* A Structure for boards to register fixups with the PHY Lib */ > -struct phy_fixup { > - struct list_head list; > - char bus_id[MII_BUS_ID_SIZE + 3]; > - u32 phy_uid; > - u32 phy_uid_mask; > - int (*run)(struct phy_device *phydev); > -}; > - > const char *phy_speed_to_str(int speed); > const char *phy_duplex_to_str(unsigned int duplex); > const char *phy_rate_matching_to_str(int rate_matching); > @@ -2117,8 +2105,6 @@ s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev, > void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv, > bool *tx_pause, bool *rx_pause); > > -int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, > - int (*run)(struct phy_device *)); > int phy_register_fixup_for_id(const char *bus_id, > int (*run)(struct phy_device *)); > int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask,
On 14.02.2025 11:59, Mateusz Polchlopek wrote: > > > On 2/13/2025 10:48 PM, Heiner Kallweit wrote: >> Certain fixup-related definitions aren't used outside phy_device.c. >> So make them private and remove them from phy.h. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/net/phy/phy_device.c | 16 +++++++++++++--- >> include/linux/phy.h | 14 -------------- >> 2 files changed, 13 insertions(+), 17 deletions(-) >> >> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c >> index 9b06ba92f..14c312ad2 100644 >> --- a/drivers/net/phy/phy_device.c >> +++ b/drivers/net/phy/phy_device.c >> @@ -45,6 +45,17 @@ MODULE_DESCRIPTION("PHY library"); >> MODULE_AUTHOR("Andy Fleming"); >> MODULE_LICENSE("GPL"); >> +#define PHY_ANY_ID "MATCH ANY PHY" >> +#define PHY_ANY_UID 0xffffffff >> + > > Overall looks like a nice cleanup but I am not sure about this space > between #define and PHY_ANY_ID or PHY_ANY_UID... > There's a tab, which effectively equals a space. Maybe it's just the diff which is misleading. At least checkpatch didn't complain. >> +struct phy_fixup { >> + struct list_head list; >> + char bus_id[MII_BUS_ID_SIZE + 3]; >> + u32 phy_uid; >> + u32 phy_uid_mask; >> + int (*run)(struct phy_device *phydev); >> +}; >> + >> __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_basic_features) __ro_after_init; >> EXPORT_SYMBOL_GPL(phy_basic_features); >> @@ -378,8 +389,8 @@ static SIMPLE_DEV_PM_OPS(mdio_bus_phy_pm_ops, mdio_bus_phy_suspend, >> * comparison >> * @run: The actual code to be run when a matching PHY is found >> */ >> -int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, >> - int (*run)(struct phy_device *)) >> +static int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, >> + int (*run)(struct phy_device *)) >> { >> struct phy_fixup *fixup = kzalloc(sizeof(*fixup), GFP_KERNEL); >> @@ -397,7 +408,6 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, >> return 0; >> } >> -EXPORT_SYMBOL(phy_register_fixup); >> /* Registers a fixup to be run on any PHY with the UID in phy_uid */ >> int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask, >> diff --git a/include/linux/phy.h b/include/linux/phy.h >> index 29df4c602..96e427c2c 100644 >> --- a/include/linux/phy.h >> +++ b/include/linux/phy.h >> @@ -1277,9 +1277,6 @@ struct phy_driver { >> #define to_phy_driver(d) container_of_const(to_mdio_common_driver(d), \ >> struct phy_driver, mdiodrv) >> -#define PHY_ANY_ID "MATCH ANY PHY" >> -#define PHY_ANY_UID 0xffffffff >> - >> #define PHY_ID_MATCH_EXACT(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 0) >> #define PHY_ID_MATCH_MODEL(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 4) >> #define PHY_ID_MATCH_VENDOR(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 10) >> @@ -1312,15 +1309,6 @@ 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); >> } >> -/* A Structure for boards to register fixups with the PHY Lib */ >> -struct phy_fixup { >> - struct list_head list; >> - char bus_id[MII_BUS_ID_SIZE + 3]; >> - u32 phy_uid; >> - u32 phy_uid_mask; >> - int (*run)(struct phy_device *phydev); >> -}; >> - >> const char *phy_speed_to_str(int speed); >> const char *phy_duplex_to_str(unsigned int duplex); >> const char *phy_rate_matching_to_str(int rate_matching); >> @@ -2117,8 +2105,6 @@ s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev, >> void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv, >> bool *tx_pause, bool *rx_pause); >> -int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, >> - int (*run)(struct phy_device *)); >> int phy_register_fixup_for_id(const char *bus_id, >> int (*run)(struct phy_device *)); >> int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask, >
On Thu, Feb 13, 2025 at 10:48:11PM +0100, Heiner Kallweit wrote: > Certain fixup-related definitions aren't used outside phy_device.c. > So make them private and remove them from phy.h. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> Reviewed-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Thanks!
On Fri, Feb 14, 2025 at 12:11:09PM +0100, Heiner Kallweit wrote: > On 14.02.2025 11:59, Mateusz Polchlopek wrote: > > > > > > On 2/13/2025 10:48 PM, Heiner Kallweit wrote: > >> Certain fixup-related definitions aren't used outside phy_device.c. > >> So make them private and remove them from phy.h. > >> > >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > >> --- > >> drivers/net/phy/phy_device.c | 16 +++++++++++++--- > >> include/linux/phy.h | 14 -------------- > >> 2 files changed, 13 insertions(+), 17 deletions(-) > >> > >> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c > >> index 9b06ba92f..14c312ad2 100644 > >> --- a/drivers/net/phy/phy_device.c > >> +++ b/drivers/net/phy/phy_device.c > >> @@ -45,6 +45,17 @@ MODULE_DESCRIPTION("PHY library"); > >> MODULE_AUTHOR("Andy Fleming"); > >> MODULE_LICENSE("GPL"); > >> +#define PHY_ANY_ID "MATCH ANY PHY" > >> +#define PHY_ANY_UID 0xffffffff > >> + > > > > Overall looks like a nice cleanup but I am not sure about this space > > between #define and PHY_ANY_ID or PHY_ANY_UID... > > > There's a tab, which effectively equals a space. Maybe it's just the > diff which is misleading. At least checkpatch didn't complain. So long as it is a straight cut/paste from the old location, this is fine. Reviewed-by: Andrew Lunn <andrew@lunn.ch> Andrew
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 9b06ba92f..14c312ad2 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -45,6 +45,17 @@ MODULE_DESCRIPTION("PHY library"); MODULE_AUTHOR("Andy Fleming"); MODULE_LICENSE("GPL"); +#define PHY_ANY_ID "MATCH ANY PHY" +#define PHY_ANY_UID 0xffffffff + +struct phy_fixup { + struct list_head list; + char bus_id[MII_BUS_ID_SIZE + 3]; + u32 phy_uid; + u32 phy_uid_mask; + int (*run)(struct phy_device *phydev); +}; + __ETHTOOL_DECLARE_LINK_MODE_MASK(phy_basic_features) __ro_after_init; EXPORT_SYMBOL_GPL(phy_basic_features); @@ -378,8 +389,8 @@ static SIMPLE_DEV_PM_OPS(mdio_bus_phy_pm_ops, mdio_bus_phy_suspend, * comparison * @run: The actual code to be run when a matching PHY is found */ -int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, - int (*run)(struct phy_device *)) +static int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, + int (*run)(struct phy_device *)) { struct phy_fixup *fixup = kzalloc(sizeof(*fixup), GFP_KERNEL); @@ -397,7 +408,6 @@ int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, return 0; } -EXPORT_SYMBOL(phy_register_fixup); /* Registers a fixup to be run on any PHY with the UID in phy_uid */ int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask, diff --git a/include/linux/phy.h b/include/linux/phy.h index 29df4c602..96e427c2c 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1277,9 +1277,6 @@ struct phy_driver { #define to_phy_driver(d) container_of_const(to_mdio_common_driver(d), \ struct phy_driver, mdiodrv) -#define PHY_ANY_ID "MATCH ANY PHY" -#define PHY_ANY_UID 0xffffffff - #define PHY_ID_MATCH_EXACT(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 0) #define PHY_ID_MATCH_MODEL(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 4) #define PHY_ID_MATCH_VENDOR(id) .phy_id = (id), .phy_id_mask = GENMASK(31, 10) @@ -1312,15 +1309,6 @@ 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); } -/* A Structure for boards to register fixups with the PHY Lib */ -struct phy_fixup { - struct list_head list; - char bus_id[MII_BUS_ID_SIZE + 3]; - u32 phy_uid; - u32 phy_uid_mask; - int (*run)(struct phy_device *phydev); -}; - const char *phy_speed_to_str(int speed); const char *phy_duplex_to_str(unsigned int duplex); const char *phy_rate_matching_to_str(int rate_matching); @@ -2117,8 +2105,6 @@ s32 phy_get_internal_delay(struct phy_device *phydev, struct device *dev, void phy_resolve_pause(unsigned long *local_adv, unsigned long *partner_adv, bool *tx_pause, bool *rx_pause); -int phy_register_fixup(const char *bus_id, u32 phy_uid, u32 phy_uid_mask, - int (*run)(struct phy_device *)); int phy_register_fixup_for_id(const char *bus_id, int (*run)(struct phy_device *)); int phy_register_fixup_for_uid(u32 phy_uid, u32 phy_uid_mask,
Certain fixup-related definitions aren't used outside phy_device.c. So make them private and remove them from phy.h. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/phy_device.c | 16 +++++++++++++--- include/linux/phy.h | 14 -------------- 2 files changed, 13 insertions(+), 17 deletions(-)