| Message ID | 290db2fb-01f3-46af-8612-26d30b98d8b3@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] net: phy: add phylib-internal.h | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next, async |
| netdev/apply | fail | Patch does not apply to net-next-0 |
On 18.02.2025 23:43, Heiner Kallweit wrote: > This patch is a starting point for moving phylib-internal > declarations to a private header file. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/phy/phy-c45.c | 1 + > drivers/net/phy/phy-core.c | 3 ++- > drivers/net/phy/phy.c | 2 ++ > drivers/net/phy/phy_device.c | 2 ++ > drivers/net/phy/phy_led_triggers.c | 2 ++ > drivers/net/phy/phylib-internal.h | 25 +++++++++++++++++++++++++ > include/linux/phy.h | 13 ------------- > 7 files changed, 34 insertions(+), 14 deletions(-) > create mode 100644 drivers/net/phy/phylib-internal.h > Patch only applies on top of patches which weren't applied yet. So I'll resubmit. -- pw-bot: cr
On Wed, Feb 19, 2025 at 07:37:52AM +0100, Heiner Kallweit wrote: > On 18.02.2025 23:43, Heiner Kallweit wrote: > > This patch is a starting point for moving phylib-internal > > declarations to a private header file. > > > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > > --- > > drivers/net/phy/phy-c45.c | 1 + > > drivers/net/phy/phy-core.c | 3 ++- > > drivers/net/phy/phy.c | 2 ++ > > drivers/net/phy/phy_device.c | 2 ++ > > drivers/net/phy/phy_led_triggers.c | 2 ++ > > drivers/net/phy/phylib-internal.h | 25 +++++++++++++++++++++++++ > > include/linux/phy.h | 13 ------------- > > 7 files changed, 34 insertions(+), 14 deletions(-) > > create mode 100644 drivers/net/phy/phylib-internal.h Maybe we should discuss a little where we are going with this... I like the idea, we want to limit the scope of some functions so they don't get abused. MAC drivers are the main abusers here, they should have a small set of methods they can use. If you look at some other subsystems they have a header for consumers, and a header for providers. include/linux/gpio/{consumer|driver}.h, clk-provider.h and clk.h, etc. Do we want include/linux/phy.h to contain the upper API for phylib, which MAC drivers can use? Should phylib-internal.h be just the internal API between parts of the core? There will be another header which has everything a PHY driver needs for the lower interface of phylib? Is this what you are thinking? Andrew
On 19.02.2025 14:35, Andrew Lunn wrote: > On Wed, Feb 19, 2025 at 07:37:52AM +0100, Heiner Kallweit wrote: >> On 18.02.2025 23:43, Heiner Kallweit wrote: >>> This patch is a starting point for moving phylib-internal >>> declarations to a private header file. >>> >>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >>> --- >>> drivers/net/phy/phy-c45.c | 1 + >>> drivers/net/phy/phy-core.c | 3 ++- >>> drivers/net/phy/phy.c | 2 ++ >>> drivers/net/phy/phy_device.c | 2 ++ >>> drivers/net/phy/phy_led_triggers.c | 2 ++ >>> drivers/net/phy/phylib-internal.h | 25 +++++++++++++++++++++++++ >>> include/linux/phy.h | 13 ------------- >>> 7 files changed, 34 insertions(+), 14 deletions(-) >>> create mode 100644 drivers/net/phy/phylib-internal.h > > Maybe we should discuss a little where we are going with this... > > I like the idea, we want to limit the scope of some functions so they > don't get abused. MAC drivers are the main abusers here, they should > have a small set of methods they can use. > > If you look at some other subsystems they have a header for consumers, > and a header for providers. include/linux/gpio/{consumer|driver}.h, > clk-provider.h and clk.h, etc. > > Do we want include/linux/phy.h to contain the upper API for phylib, > which MAC drivers can use? Should phylib-internal.h be just the > internal API between parts of the core? There will be another header > which has everything a PHY driver needs for the lower interface of > phylib? > > Is this what you are thinking? > Exactly. Not sure in which category phylink would fall: part of core, or similar to a PHY driver? > Andrew Heiner
diff --git a/drivers/net/phy/phy-c45.c b/drivers/net/phy/phy-c45.c index 37c9a344b..0bcbdce38 100644 --- a/drivers/net/phy/phy-c45.c +++ b/drivers/net/phy/phy-c45.c @@ -9,6 +9,7 @@ #include <linux/phy.h> #include "mdio-open-alliance.h" +#include "phylib-internal.h" /** * genphy_c45_baset1_able - checks if the PMA has BASE-T1 extended abilities diff --git a/drivers/net/phy/phy-core.c b/drivers/net/phy/phy-core.c index 2fd1d153a..b1c1670de 100644 --- a/drivers/net/phy/phy-core.c +++ b/drivers/net/phy/phy-core.c @@ -6,6 +6,8 @@ #include <linux/phy.h> #include <linux/of.h> +#include "phylib-internal.h" + /** * phy_speed_to_str - Return a string representing the PHY link speed * @@ -544,7 +546,6 @@ void phy_check_downshift(struct phy_device *phydev) phydev->downshifted_rate = 1; } -EXPORT_SYMBOL_GPL(phy_check_downshift); static int phy_resolve_min_speed(struct phy_device *phydev, bool fdx_only) { diff --git a/drivers/net/phy/phy.c b/drivers/net/phy/phy.c index b454e31d4..fd8d8dd29 100644 --- a/drivers/net/phy/phy.c +++ b/drivers/net/phy/phy.c @@ -36,6 +36,8 @@ #include <net/genetlink.h> #include <net/sock.h> +#include "phylib-internal.h" + #define PHY_STATE_TIME HZ #define PHY_STATE_STR(_state) \ diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c index 103a4d102..7d21379fa 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -41,6 +41,8 @@ #include <linux/uaccess.h> #include <linux/unistd.h> +#include "phylib-internal.h" + MODULE_DESCRIPTION("PHY library"); MODULE_AUTHOR("Andy Fleming"); MODULE_LICENSE("GPL"); diff --git a/drivers/net/phy/phy_led_triggers.c b/drivers/net/phy/phy_led_triggers.c index f550576eb..bd3c9554f 100644 --- a/drivers/net/phy/phy_led_triggers.c +++ b/drivers/net/phy/phy_led_triggers.c @@ -5,6 +5,8 @@ #include <linux/phy_led_triggers.h> #include <linux/netdevice.h> +#include "phylib-internal.h" + static struct phy_led_trigger *phy_speed_to_led_trigger(struct phy_device *phy, unsigned int speed) { diff --git a/drivers/net/phy/phylib-internal.h b/drivers/net/phy/phylib-internal.h new file mode 100644 index 000000000..dc9592c6b --- /dev/null +++ b/drivers/net/phy/phylib-internal.h @@ -0,0 +1,25 @@ +// SPDX-License-Identifier: GPL-2.0-or-later +/* + * phylib-internal header + */ + +#ifndef __PHYLIB_INTERNAL_H +#define __PHYLIB_INTERNAL_H + +struct phy_device; + +/* + * phy_supported_speeds - return all speeds currently supported by a PHY device + */ +unsigned int phy_supported_speeds(struct phy_device *phy, + unsigned int *speeds, + unsigned int size); +void of_set_phy_supported(struct phy_device *phydev); +void of_set_phy_eee_broken(struct phy_device *phydev); +void of_set_phy_timing_role(struct phy_device *phydev); +int phy_speed_down_core(struct phy_device *phydev); +void phy_check_downshift(struct phy_device *phydev); + +int genphy_c45_read_eee_adv(struct phy_device *phydev, unsigned long *adv); + +#endif /* __PHYLIB_INTERNAL_H */ diff --git a/include/linux/phy.h b/include/linux/phy.h index c0f524579..3076b4caa 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -184,13 +184,6 @@ static inline void phy_interface_set_rgmii(unsigned long *intf) __set_bit(PHY_INTERFACE_MODE_RGMII_TXID, intf); } -/* - * phy_supported_speeds - return all speeds currently supported by a PHY device - */ -unsigned int phy_supported_speeds(struct phy_device *phy, - unsigned int *speeds, - unsigned int size); - /** * phy_modes - map phy_interface_t enum to device tree binding of phy-mode * @interface: enum phy_interface_t value @@ -1324,10 +1317,6 @@ phy_lookup_setting(int speed, int duplex, const unsigned long *mask, bool exact); size_t phy_speeds(unsigned int *speeds, size_t size, unsigned long *mask); -void of_set_phy_supported(struct phy_device *phydev); -void of_set_phy_eee_broken(struct phy_device *phydev); -void of_set_phy_timing_role(struct phy_device *phydev); -int phy_speed_down_core(struct phy_device *phydev); /** * phy_is_started - Convenience function to check whether PHY is started @@ -1353,7 +1342,6 @@ static inline void phy_disable_eee_mode(struct phy_device *phydev, u32 link_mode void phy_resolve_aneg_pause(struct phy_device *phydev); void phy_resolve_aneg_linkmode(struct phy_device *phydev); -void phy_check_downshift(struct phy_device *phydev); /** * phy_read - Convenience function for reading a given PHY register @@ -2028,7 +2016,6 @@ int genphy_c45_ethtool_get_eee(struct phy_device *phydev, int genphy_c45_ethtool_set_eee(struct phy_device *phydev, struct ethtool_keee *data); int genphy_c45_an_config_eee_aneg(struct phy_device *phydev); -int genphy_c45_read_eee_adv(struct phy_device *phydev, unsigned long *adv); /* Generic C45 PHY driver */ extern struct phy_driver genphy_c45_driver;
This patch is a starting point for moving phylib-internal declarations to a private header file. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/phy/phy-c45.c | 1 + drivers/net/phy/phy-core.c | 3 ++- drivers/net/phy/phy.c | 2 ++ drivers/net/phy/phy_device.c | 2 ++ drivers/net/phy/phy_led_triggers.c | 2 ++ drivers/net/phy/phylib-internal.h | 25 +++++++++++++++++++++++++ include/linux/phy.h | 13 ------------- 7 files changed, 34 insertions(+), 14 deletions(-) create mode 100644 drivers/net/phy/phylib-internal.h