2022-06-20 15:14:11

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH 02/12] net: mdio: switch fixed-link PHYs API to fwnode_

fixed-link PHYs API is used by DSA and a number of drivers
and was depending on of_. Switch to fwnode_ so to make it
hardware description agnostic and allow to be used in ACPI
world as well.

Signed-off-by: Marcin Wojtas <[email protected]>
---
include/linux/fwnode_mdio.h | 19 ++++
drivers/net/mdio/fwnode_mdio.c | 100 ++++++++++++++++++++
drivers/net/mdio/of_mdio.c | 79 +---------------
3 files changed, 122 insertions(+), 76 deletions(-)

diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
index faf603c48c86..98755b8c6c8a 100644
--- a/include/linux/fwnode_mdio.h
+++ b/include/linux/fwnode_mdio.h
@@ -16,6 +16,11 @@ int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
int fwnode_mdiobus_register_phy(struct mii_bus *bus,
struct fwnode_handle *child, u32 addr);

+int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode);
+
+void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode);
+
+bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode);
#else /* CONFIG_FWNODE_MDIO */
int fwnode_mdiobus_phy_device_register(struct mii_bus *mdio,
struct phy_device *phy,
@@ -30,6 +35,20 @@ static inline int fwnode_mdiobus_register_phy(struct mii_bus *bus,
{
return -EINVAL;
}
+
+static inline int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode)
+{
+ return -ENODEV;
+}
+
+static inline void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode)
+{
+}
+
+static inline bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
+{
+ return false;
+}
#endif

#endif /* __LINUX_FWNODE_MDIO_H */
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 1c1584fca632..b1c20c48b6cb 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -10,6 +10,7 @@
#include <linux/fwnode_mdio.h>
#include <linux/of.h>
#include <linux/phy.h>
+#include <linux/phy_fixed.h>

MODULE_AUTHOR("Calvin Johnson <[email protected]>");
MODULE_LICENSE("GPL");
@@ -147,3 +148,102 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
return 0;
}
EXPORT_SYMBOL(fwnode_mdiobus_register_phy);
+
+/*
+ * fwnode_phy_is_fixed_link() and fwnode_phy_register_fixed_link() must
+ * support two bindings:
+ * - the old binding, where 'fixed-link' was a property with 5
+ * cells encoding various information about the fixed PHY
+ * - the new binding, where 'fixed-link' is a sub-node of the
+ * Ethernet device.
+ */
+bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
+{
+ struct fwnode_handle *fixed_link_node;
+ const char *managed;
+ int len;
+
+ /* New binding */
+ fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
+ if (fixed_link_node) {
+ fwnode_handle_put(fixed_link_node);
+ return true;
+ }
+
+ if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
+ strcmp(managed, "auto") != 0)
+ return true;
+
+ /* Old binding */
+ len = fwnode_property_read_u32_array(fwnode, "fixed-link", NULL, 0);
+ if (len == (5 * sizeof(u32)))
+ return true;
+
+ return false;
+}
+EXPORT_SYMBOL(fwnode_phy_is_fixed_link);
+
+int fwnode_phy_register_fixed_link(struct fwnode_handle *fwnode)
+{
+ struct fixed_phy_status status = {};
+ struct fwnode_handle *fixed_link_node;
+ u32 fixed_link_prop[5];
+ const char *managed;
+
+ if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
+ strcmp(managed, "in-band-status") == 0) {
+ /* status is zeroed, namely its .link member */
+ goto register_phy;
+ }
+
+ /* New binding */
+ fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
+ if (fixed_link_node) {
+ status.link = 1;
+ status.duplex = fwnode_property_present(fixed_link_node,
+ "full-duplex");
+ if (fwnode_property_read_u32(fixed_link_node, "speed",
+ &status.speed)) {
+ fwnode_handle_put(fixed_link_node);
+ return -EINVAL;
+ }
+ status.pause = fwnode_property_present(fixed_link_node, "pause");
+ status.asym_pause = fwnode_property_present(fixed_link_node,
+ "asym-pause");
+ fwnode_handle_put(fixed_link_node);
+
+ goto register_phy;
+ }
+
+ /* Old binding */
+ if (fwnode_property_read_u32_array(fwnode, "fixed-link", fixed_link_prop,
+ ARRAY_SIZE(fixed_link_prop)) == 0) {
+ status.link = 1;
+ status.duplex = fixed_link_prop[1];
+ status.speed = fixed_link_prop[2];
+ status.pause = fixed_link_prop[3];
+ status.asym_pause = fixed_link_prop[4];
+ goto register_phy;
+ }
+
+ return -ENODEV;
+
+register_phy:
+ return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, fwnode));
+}
+EXPORT_SYMBOL(fwnode_phy_register_fixed_link);
+
+void fwnode_phy_deregister_fixed_link(struct fwnode_handle *fwnode)
+{
+ struct phy_device *phydev;
+
+ phydev = fwnode_phy_find_device(fwnode);
+ if (!phydev)
+ return;
+
+ fixed_phy_unregister(phydev);
+
+ put_device(&phydev->mdio.dev); /* fwnode_phy_find_device() */
+ phy_device_free(phydev); /* fixed_phy_register() */
+}
+EXPORT_SYMBOL(fwnode_phy_deregister_fixed_link);
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index d755fe1ecdda..409da6e92f7d 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -351,91 +351,18 @@ EXPORT_SYMBOL(of_phy_get_and_connect);
*/
bool of_phy_is_fixed_link(struct device_node *np)
{
- struct device_node *dn;
- int len, err;
- const char *managed;
-
- /* New binding */
- dn = of_get_child_by_name(np, "fixed-link");
- if (dn) {
- of_node_put(dn);
- return true;
- }
-
- err = of_property_read_string(np, "managed", &managed);
- if (err == 0 && strcmp(managed, "auto") != 0)
- return true;
-
- /* Old binding */
- if (of_get_property(np, "fixed-link", &len) &&
- len == (5 * sizeof(__be32)))
- return true;
-
- return false;
+ return fwnode_phy_is_fixed_link(of_fwnode_handle(np));
}
EXPORT_SYMBOL(of_phy_is_fixed_link);

int of_phy_register_fixed_link(struct device_node *np)
{
- struct fixed_phy_status status = {};
- struct device_node *fixed_link_node;
- u32 fixed_link_prop[5];
- const char *managed;
-
- if (of_property_read_string(np, "managed", &managed) == 0 &&
- strcmp(managed, "in-band-status") == 0) {
- /* status is zeroed, namely its .link member */
- goto register_phy;
- }
-
- /* New binding */
- fixed_link_node = of_get_child_by_name(np, "fixed-link");
- if (fixed_link_node) {
- status.link = 1;
- status.duplex = of_property_read_bool(fixed_link_node,
- "full-duplex");
- if (of_property_read_u32(fixed_link_node, "speed",
- &status.speed)) {
- of_node_put(fixed_link_node);
- return -EINVAL;
- }
- status.pause = of_property_read_bool(fixed_link_node, "pause");
- status.asym_pause = of_property_read_bool(fixed_link_node,
- "asym-pause");
- of_node_put(fixed_link_node);
-
- goto register_phy;
- }
-
- /* Old binding */
- if (of_property_read_u32_array(np, "fixed-link", fixed_link_prop,
- ARRAY_SIZE(fixed_link_prop)) == 0) {
- status.link = 1;
- status.duplex = fixed_link_prop[1];
- status.speed = fixed_link_prop[2];
- status.pause = fixed_link_prop[3];
- status.asym_pause = fixed_link_prop[4];
- goto register_phy;
- }
-
- return -ENODEV;
-
-register_phy:
- return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status, of_fwnode_handle(np)));
+ return fwnode_phy_register_fixed_link(of_fwnode_handle(np));
}
EXPORT_SYMBOL(of_phy_register_fixed_link);

void of_phy_deregister_fixed_link(struct device_node *np)
{
- struct phy_device *phydev;
-
- phydev = of_phy_find_device(np);
- if (!phydev)
- return;
-
- fixed_phy_unregister(phydev);
-
- put_device(&phydev->mdio.dev); /* of_phy_find_device() */
- phy_device_free(phydev); /* fixed_phy_register() */
+ fwnode_phy_deregister_fixed_link(of_fwnode_handle(np));
}
EXPORT_SYMBOL(of_phy_deregister_fixed_link);
--
2.29.0


2022-06-20 17:40:34

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [net-next: PATCH 02/12] net: mdio: switch fixed-link PHYs API to fwnode_

On Mon, Jun 20, 2022 at 05:02:15PM +0200, Marcin Wojtas wrote:
> fixed-link PHYs API is used by DSA and a number of drivers
> and was depending on of_. Switch to fwnode_ so to make it
> hardware description agnostic and allow to be used in ACPI
> world as well.

...

> +bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
> +{
> + struct fwnode_handle *fixed_link_node;
> + const char *managed;
> + int len;
> +
> + /* New binding */
> + fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
> + if (fixed_link_node) {
> + fwnode_handle_put(fixed_link_node);
> + return true;
> + }
> +
> + if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
> + strcmp(managed, "auto") != 0)
> + return true;
> +
> + /* Old binding */
> + len = fwnode_property_read_u32_array(fwnode, "fixed-link", NULL, 0);


fwnode_property_count_u32()

> + if (len == (5 * sizeof(u32)))

I'm not sure how to interpret this. len will return a count of u32 elements.
What does the sizeof(u32) mean here?

> + return true;
> +
> + return false;
> +}

--
With Best Regards,
Andy Shevchenko


2022-06-21 09:26:01

by Marcin Wojtas

[permalink] [raw]
Subject: Re: [net-next: PATCH 02/12] net: mdio: switch fixed-link PHYs API to fwnode_

pon., 20 cze 2022 o 19:32 Andy Shevchenko
<[email protected]> napisaƂ(a):
>
> On Mon, Jun 20, 2022 at 05:02:15PM +0200, Marcin Wojtas wrote:
> > fixed-link PHYs API is used by DSA and a number of drivers
> > and was depending on of_. Switch to fwnode_ so to make it
> > hardware description agnostic and allow to be used in ACPI
> > world as well.
>
> ...
>
> > +bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
> > +{
> > + struct fwnode_handle *fixed_link_node;
> > + const char *managed;
> > + int len;
> > +
> > + /* New binding */
> > + fixed_link_node = fwnode_get_named_child_node(fwnode, "fixed-link");
> > + if (fixed_link_node) {
> > + fwnode_handle_put(fixed_link_node);
> > + return true;
> > + }
> > +
> > + if (fwnode_property_read_string(fwnode, "managed", &managed) == 0 &&
> > + strcmp(managed, "auto") != 0)
> > + return true;
> > +
> > + /* Old binding */
> > + len = fwnode_property_read_u32_array(fwnode, "fixed-link", NULL, 0);
>
>
> fwnode_property_count_u32()
>
> > + if (len == (5 * sizeof(u32)))
>
> I'm not sure how to interpret this. len will return a count of u32 elements.
> What does the sizeof(u32) mean here?
>

You are right, thanks for spotting. The total byte count remained
after migrating from of_get_property ->
fwnode_property_read_u32_array.

Best regards,
Marcin