| Message ID | 20220107161222.14043-1-pali@kernel.org |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | [1/2] of: net: Add helper function of_get_ethdev_label() | expand |
| Context | Check | Description |
|---|---|---|
| robh/checkpatch | success | |
| robh/dtbs-check | success | |
| robh/dt-meta-schema | success |
On Fri, Jan 07, 2022 at 05:12:21PM +0100, Pali Rohár wrote: > Adds a new helper function of_get_ethdev_label() which sets initial name of > specified netdev interface based on DT "label" property. It is same what is > doing DSA function dsa_port_parse_of() for DSA ports. > > This helper function can be useful for drivers to make consistency between > DSA and netdev interface names. > > Signed-off-by: Pali Rohár <pali@kernel.org> Doesn't this also need a patch to update the DT binding document Documentation/devicetree/bindings/net/ethernet-controller.yaml ? Also it needs a covering message for the series, and a well thought out argument why this is required. Consistency with DSA probably isn't a good enough reason. From what I remember, there have been a number of network interface naming proposals over the years, and as you can see, none of them have been successful... but who knows what will happen this time.
On Fri, Jan 07, 2022 at 04:29:31PM +0000, Russell King (Oracle) wrote: > On Fri, Jan 07, 2022 at 05:12:21PM +0100, Pali Rohár wrote: > > Adds a new helper function of_get_ethdev_label() which sets initial name of > > specified netdev interface based on DT "label" property. It is same what is > > doing DSA function dsa_port_parse_of() for DSA ports. > > > > This helper function can be useful for drivers to make consistency between > > DSA and netdev interface names. > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > Doesn't this also need a patch to update the DT binding document > Documentation/devicetree/bindings/net/ethernet-controller.yaml ? > > Also it needs a covering message for the series, and a well thought > out argument why this is required. Consistency with DSA probably > isn't a good enough reason. > > >From what I remember, there have been a number of network interface > naming proposals over the years, and as you can see, none of them have > been successful... but who knows what will happen this time. I agree with Russell here. I doubt this is going to be accepted. DSA is special because DSA is very old, much older than DT, and maybe older than udev. The old DSA platform drivers had a mechanism to supply the interface name to the DSA core. When we added a DT binding to DSA we kept that mechanism, since that mechanism had been used for a long time. Even if you could show there was a generic old mechanism, from before the days of DT, that allowed interface names to be set from platform drivers, i doubt it would be accepted because there is no continuity, which DSA has. Andrew
On Friday 07 January 2022 17:55:25 Andrew Lunn wrote: > On Fri, Jan 07, 2022 at 04:29:31PM +0000, Russell King (Oracle) wrote: > > On Fri, Jan 07, 2022 at 05:12:21PM +0100, Pali Rohár wrote: > > > Adds a new helper function of_get_ethdev_label() which sets initial name of > > > specified netdev interface based on DT "label" property. It is same what is > > > doing DSA function dsa_port_parse_of() for DSA ports. > > > > > > This helper function can be useful for drivers to make consistency between > > > DSA and netdev interface names. > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > Doesn't this also need a patch to update the DT binding document > > Documentation/devicetree/bindings/net/ethernet-controller.yaml ? > > > > Also it needs a covering message for the series, and a well thought > > out argument why this is required. Consistency with DSA probably > > isn't a good enough reason. > > > > >From what I remember, there have been a number of network interface > > naming proposals over the years, and as you can see, none of them have > > been successful... but who knows what will happen this time. > > I agree with Russell here. I doubt this is going to be accepted. > > DSA is special because DSA is very old, much older than DT, and maybe > older than udev. The old DSA platform drivers had a mechanism to > supply the interface name to the DSA core. When we added a DT binding > to DSA we kept that mechanism, since that mechanism had been used for > a long time. > > Even if you could show there was a generic old mechanism, from before > the days of DT, that allowed interface names to be set from platform > drivers, i doubt it would be accepted because there is no continuity, > which DSA has. Well, DT should universally describe HW board wiring. From HW point of view, it is really does not matter if RJ45 port is connected to embedded PHY on SoC itself or to the external PHY chip, or to the switch chip with embedded PHY. And if board has mix of these options, also labels (as printed on product box) should be in DTS described in the same way, independently of which software solution / driver is used for particular chip. It really should not matter for DTS if kernel is using for particular HW part DSA driver or ethernet driver. So there really should be some common way. And if the one which DSA is using is the old mechanism, what is the new mechanism then?
On Thursday 13 January 2022 19:27:19 Pali Rohár wrote: > On Friday 07 January 2022 17:55:25 Andrew Lunn wrote: > > On Fri, Jan 07, 2022 at 04:29:31PM +0000, Russell King (Oracle) wrote: > > > On Fri, Jan 07, 2022 at 05:12:21PM +0100, Pali Rohár wrote: > > > > Adds a new helper function of_get_ethdev_label() which sets initial name of > > > > specified netdev interface based on DT "label" property. It is same what is > > > > doing DSA function dsa_port_parse_of() for DSA ports. > > > > > > > > This helper function can be useful for drivers to make consistency between > > > > DSA and netdev interface names. > > > > > > > > Signed-off-by: Pali Rohár <pali@kernel.org> > > > > > > Doesn't this also need a patch to update the DT binding document > > > Documentation/devicetree/bindings/net/ethernet-controller.yaml ? > > > > > > Also it needs a covering message for the series, and a well thought > > > out argument why this is required. Consistency with DSA probably > > > isn't a good enough reason. > > > > > > >From what I remember, there have been a number of network interface > > > naming proposals over the years, and as you can see, none of them have > > > been successful... but who knows what will happen this time. > > > > I agree with Russell here. I doubt this is going to be accepted. > > > > DSA is special because DSA is very old, much older than DT, and maybe > > older than udev. The old DSA platform drivers had a mechanism to > > supply the interface name to the DSA core. When we added a DT binding > > to DSA we kept that mechanism, since that mechanism had been used for > > a long time. > > > > Even if you could show there was a generic old mechanism, from before > > the days of DT, that allowed interface names to be set from platform > > drivers, i doubt it would be accepted because there is no continuity, > > which DSA has. > > Well, DT should universally describe HW board wiring. From HW point of > view, it is really does not matter if RJ45 port is connected to embedded > PHY on SoC itself or to the external PHY chip, or to the switch chip > with embedded PHY. And if board has mix of these options, also labels > (as printed on product box) should be in DTS described in the same way, > independently of which software solution / driver is used for particular > chip. It really should not matter for DTS if kernel is using for > particular HW part DSA driver or ethernet driver. > > So there really should be some common way. And if the one which DSA is > using is the old mechanism, what is the new mechanism then? Hello! Any comments on this?
diff --git a/include/linux/of_net.h b/include/linux/of_net.h index 0484b613ca64..532d88127b42 100644 --- a/include/linux/of_net.h +++ b/include/linux/of_net.h @@ -15,6 +15,7 @@ struct net_device; extern int of_get_phy_mode(struct device_node *np, phy_interface_t *interface); extern int of_get_mac_address(struct device_node *np, u8 *mac); int of_get_ethdev_address(struct device_node *np, struct net_device *dev); +int of_get_ethdev_label(struct device_node *np, struct net_device *dev); extern struct net_device *of_find_net_device_by_node(struct device_node *np); #else static inline int of_get_phy_mode(struct device_node *np, @@ -33,6 +34,11 @@ static inline int of_get_ethdev_address(struct device_node *np, struct net_devic return -ENODEV; } +static inline int of_get_ethdev_label(struct device_node *np, struct net_device *dev) +{ + return -ENODEV; +} + static inline struct net_device *of_find_net_device_by_node(struct device_node *np) { return NULL; diff --git a/net/core/of_net.c b/net/core/of_net.c index f1a9bf7578e7..18fc99c42461 100644 --- a/net/core/of_net.c +++ b/net/core/of_net.c @@ -168,3 +168,18 @@ int of_get_ethdev_address(struct device_node *np, struct net_device *dev) return ret; } EXPORT_SYMBOL(of_get_ethdev_address); + +int of_get_ethdev_label(struct device_node *np, struct net_device *dev) +{ + const char *name = of_get_property(np, "label", NULL); + + if (!name) + return -ENOENT; + + if (strlen(name) >= sizeof(dev->name)) + return -ENAMETOOLONG; + + strcpy(dev->name, name); + return 0; +} +EXPORT_SYMBOL(of_get_ethdev_label);
Adds a new helper function of_get_ethdev_label() which sets initial name of specified netdev interface based on DT "label" property. It is same what is doing DSA function dsa_port_parse_of() for DSA ports. This helper function can be useful for drivers to make consistency between DSA and netdev interface names. Signed-off-by: Pali Rohár <pali@kernel.org> --- include/linux/of_net.h | 6 ++++++ net/core/of_net.c | 15 +++++++++++++++ 2 files changed, 21 insertions(+)