| Message ID | 20220620150225.1307946-2-mw@semihalf.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | ACPI support for DSA | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Clearly marked for net-next, async |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Series has a cover letter |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 4 this patch: 4 |
| netdev/cc_maintainers | success | CCed 8 of 8 maintainers |
| netdev/build_clang | success | Errors and warnings before: 5 this patch: 5 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/check_selftest | success | No net selftest shell script |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 4 this patch: 4 |
| netdev/checkpatch | warning | WARNING: line length of 92 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
On Mon, Jun 20, 2022 at 05:02:14PM +0200, Marcin Wojtas wrote: > This patch allows to use fixed_phy driver and its helper > functions without Device Tree dependency, by swtiching from > of_ to fwnode_ API. ... > -#ifdef CONFIG_OF_GPIO Nice to see this gone, because it's my goal as well. ... > -static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) > +static struct gpio_desc *fixed_phy_get_gpiod(struct fwnode_handle *fwnode) > { > - struct device_node *fixed_link_node; > + struct fwnode_handle *fixed_link_node; > struct gpio_desc *gpiod; > - if (!np) > + if (!fwnode) > return NULL; Can be dropped altogether. The following call will fail and return the same. > - fixed_link_node = of_get_child_by_name(np, "fixed-link"); > + fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link"); > if (!fixed_link_node) > return NULL; > > @@ -204,7 +203,7 @@ static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) > * Linux device associated with it, we simply have obtain > * the GPIO descriptor from the device tree like this. > */ > - gpiod = fwnode_gpiod_get_index(of_fwnode_handle(fixed_link_node), > + gpiod = fwnode_gpiod_get_index(fixed_link_node, > "link", 0, GPIOD_IN, "mdio"); Can fit one line now. > if (IS_ERR(gpiod) && PTR_ERR(gpiod) != -EPROBE_DEFER) { > if (PTR_ERR(gpiod) != -ENOENT) > @@ -212,20 +211,14 @@ static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) > fixed_link_node); > gpiod = NULL; > } > - of_node_put(fixed_link_node); > + fwnode_handle_put(fixed_link_node); > > return gpiod; > } ... > - of_node_get(np); > - phy->mdio.dev.of_node = np; > + fwnode_handle_get(fwnode); > + phy->mdio.dev.fwnode = fwnode; Please, use device_set_node(). ... > + fwnode_handle_put(phy->mdio.dev.fwnode); dev_fwnode()
On Mon, Jun 20, 2022 at 05:02:14PM +0200, Marcin Wojtas wrote: > This patch allows to use fixed_phy driver and its helper > functions without Device Tree dependency, by swtiching from > of_ to fwnode_ API. Do you actually need this? phylink does not use this code, it has its own fixed link implementation. And that implementation is not limited to 1G. Andrew
pon., 20 cze 2022 o 19:59 Andrew Lunn <andrew@lunn.ch> napisał(a): > > On Mon, Jun 20, 2022 at 05:02:14PM +0200, Marcin Wojtas wrote: > > This patch allows to use fixed_phy driver and its helper > > functions without Device Tree dependency, by swtiching from > > of_ to fwnode_ API. > > Do you actually need this? phylink does not use this code, it has its > own fixed link implementation. And that implementation is not limited > to 1G. > Yes, phylink has its own fixed-link handling, however the net/dsa/port.c relies on fixed_phy helpers these are not 1:1 equivalents. I assumed this migration (fixed_phy -> phylink) is not straightforward and IMO should be handled separately. Do you recall justification for not using phylink in this part of net/dsa/*? Thanks, Marcin
On Tue, Jun 21, 2022 at 11:56:06AM +0200, Marcin Wojtas wrote: > pon., 20 cze 2022 o 19:59 Andrew Lunn <andrew@lunn.ch> napisał(a): > > > > On Mon, Jun 20, 2022 at 05:02:14PM +0200, Marcin Wojtas wrote: > > > This patch allows to use fixed_phy driver and its helper > > > functions without Device Tree dependency, by swtiching from > > > of_ to fwnode_ API. > > > > Do you actually need this? phylink does not use this code, it has its > > own fixed link implementation. And that implementation is not limited > > to 1G. > > > > Yes, phylink has its own fixed-link handling, however the > net/dsa/port.c relies on fixed_phy helpers these are not 1:1 > equivalents. I assumed this migration (fixed_phy -> phylink) is not > straightforward and IMO should be handled separately. Do you recall > justification for not using phylink in this part of net/dsa/*? All modern DSA drivers use phylink and not fixed-phy as far as I'm aware - there are a number that still implement the .adjust_link callback, but note in dsa_port_link_register_of(): if (!ds->ops->adjust_link) { ... return 0; } dev_warn(ds->dev, "Using legacy PHYLIB callbacks. Please migrate to PHYLINK!\n"); It's really just that they haven't been migrated.
diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h index 52bc8e487ef7..449a927231ec 100644 --- a/include/linux/phy_fixed.h +++ b/include/linux/phy_fixed.h @@ -19,7 +19,7 @@ extern int fixed_phy_add(unsigned int irq, int phy_id, struct fixed_phy_status *status); extern struct phy_device *fixed_phy_register(unsigned int irq, struct fixed_phy_status *status, - struct device_node *np); + struct fwnode_handle *fwnode); extern struct phy_device * fixed_phy_register_with_gpiod(unsigned int irq, @@ -38,7 +38,7 @@ static inline int fixed_phy_add(unsigned int irq, int phy_id, } static inline struct phy_device *fixed_phy_register(unsigned int irq, struct fixed_phy_status *status, - struct device_node *np) + struct fwnode_handle *fwnode) { return ERR_PTR(-ENODEV); } diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c index 9e3c815a070f..d755fe1ecdda 100644 --- a/drivers/net/mdio/of_mdio.c +++ b/drivers/net/mdio/of_mdio.c @@ -421,7 +421,7 @@ int of_phy_register_fixed_link(struct device_node *np) return -ENODEV; register_phy: - return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, np)); + return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, of_fwnode_handle(np))); } EXPORT_SYMBOL(of_phy_register_fixed_link); diff --git a/drivers/net/phy/fixed_phy.c b/drivers/net/phy/fixed_phy.c index aef739c20ac4..0dbe6c344b05 100644 --- a/drivers/net/phy/fixed_phy.c +++ b/drivers/net/phy/fixed_phy.c @@ -15,9 +15,9 @@ #include <linux/mii.h> #include <linux/phy.h> #include <linux/phy_fixed.h> +#include <linux/property.h> #include <linux/err.h> #include <linux/slab.h> -#include <linux/of.h> #include <linux/gpio/consumer.h> #include <linux/idr.h> #include <linux/netdevice.h> @@ -186,16 +186,15 @@ static void fixed_phy_del(int phy_addr) } } -#ifdef CONFIG_OF_GPIO -static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) +static struct gpio_desc *fixed_phy_get_gpiod(struct fwnode_handle *fwnode) { - struct device_node *fixed_link_node; + struct fwnode_handle *fixed_link_node; struct gpio_desc *gpiod; - if (!np) + if (!fwnode) return NULL; - fixed_link_node = of_get_child_by_name(np, "fixed-link"); + fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link"); if (!fixed_link_node) return NULL; @@ -204,7 +203,7 @@ static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) * Linux device associated with it, we simply have obtain * the GPIO descriptor from the device tree like this. */ - gpiod = fwnode_gpiod_get_index(of_fwnode_handle(fixed_link_node), + gpiod = fwnode_gpiod_get_index(fixed_link_node, "link", 0, GPIOD_IN, "mdio"); if (IS_ERR(gpiod) && PTR_ERR(gpiod) != -EPROBE_DEFER) { if (PTR_ERR(gpiod) != -ENOENT) @@ -212,20 +211,14 @@ static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) fixed_link_node); gpiod = NULL; } - of_node_put(fixed_link_node); + fwnode_handle_put(fixed_link_node); return gpiod; } -#else -static struct gpio_desc *fixed_phy_get_gpiod(struct device_node *np) -{ - return NULL; -} -#endif static struct phy_device *__fixed_phy_register(unsigned int irq, struct fixed_phy_status *status, - struct device_node *np, + struct fwnode_handle *fwnode, struct gpio_desc *gpiod) { struct fixed_mdio_bus *fmb = &platform_fmb; @@ -238,7 +231,7 @@ static struct phy_device *__fixed_phy_register(unsigned int irq, /* Check if we have a GPIO associated with this fixed phy */ if (!gpiod) { - gpiod = fixed_phy_get_gpiod(np); + gpiod = fixed_phy_get_gpiod(fwnode); if (IS_ERR(gpiod)) return ERR_CAST(gpiod); } @@ -269,8 +262,8 @@ static struct phy_device *__fixed_phy_register(unsigned int irq, phy->asym_pause = status->asym_pause; } - of_node_get(np); - phy->mdio.dev.of_node = np; + fwnode_handle_get(fwnode); + phy->mdio.dev.fwnode = fwnode; phy->is_pseudo_fixed_link = true; switch (status->speed) { @@ -299,7 +292,7 @@ static struct phy_device *__fixed_phy_register(unsigned int irq, ret = phy_device_register(phy); if (ret) { phy_device_free(phy); - of_node_put(np); + fwnode_handle_put(fwnode); fixed_phy_del(phy_addr); return ERR_PTR(ret); } @@ -309,9 +302,9 @@ static struct phy_device *__fixed_phy_register(unsigned int irq, struct phy_device *fixed_phy_register(unsigned int irq, struct fixed_phy_status *status, - struct device_node *np) + struct fwnode_handle *fwnode) { - return __fixed_phy_register(irq, status, np, NULL); + return __fixed_phy_register(irq, status, fwnode, NULL); } EXPORT_SYMBOL_GPL(fixed_phy_register); @@ -327,7 +320,7 @@ EXPORT_SYMBOL_GPL(fixed_phy_register_with_gpiod); void fixed_phy_unregister(struct phy_device *phy) { phy_device_remove(phy); - of_node_put(phy->mdio.dev.of_node); + fwnode_handle_put(phy->mdio.dev.fwnode); fixed_phy_del(phy->mdio.addr); } EXPORT_SYMBOL_GPL(fixed_phy_unregister);
This patch allows to use fixed_phy driver and its helper functions without Device Tree dependency, by swtiching from of_ to fwnode_ API. Signed-off-by: Marcin Wojtas <mw@semihalf.com> --- include/linux/phy_fixed.h | 4 +-- drivers/net/mdio/of_mdio.c | 2 +- drivers/net/phy/fixed_phy.c | 37 ++++++++------------ 3 files changed, 18 insertions(+), 25 deletions(-)