2022-06-20 15:24:28

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH 01/12] net: phy: fixed_phy: switch to fwnode_ API

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 <[email protected]>
---
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(-)

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);
--
2.29.0


2022-06-20 17:43:29

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [net-next: PATCH 01/12] net: phy: fixed_phy: switch to fwnode_ API

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()

--
With Best Regards,
Andy Shevchenko


2022-06-20 18:14:51

by Andrew Lunn

[permalink] [raw]
Subject: Re: [net-next: PATCH 01/12] net: phy: fixed_phy: switch to fwnode_ API

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

2022-06-21 10:09:18

by Marcin Wojtas

[permalink] [raw]
Subject: Re: [net-next: PATCH 01/12] net: phy: fixed_phy: switch to fwnode_ API

pon., 20 cze 2022 o 19:59 Andrew Lunn <[email protected]> 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

2022-06-21 10:10:44

by Russell King (Oracle)

[permalink] [raw]
Subject: Re: [net-next: PATCH 01/12] net: phy: fixed_phy: switch to fwnode_ API

On Tue, Jun 21, 2022 at 11:56:06AM +0200, Marcin Wojtas wrote:
> pon., 20 cze 2022 o 19:59 Andrew Lunn <[email protected]> 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.

--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!