2023-01-16 18:12:31

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v4 0/8] DSA: switch to fwnode_/device_

Hi,

After a longer while this is a re-spin of the DSA migration
to fwnode_/device_ API.
It addresses all comments from the previous iteration - the
details are summarized in the changelog section below.
For reference, here is the discussion under v3.
https://lore.kernel.org/netdev/[email protected]/t/

The ultimate goal of these changes is to prepare the DSA to use ACPI in
future, which works locally on my branches [1][2], however, this part
needs to be handled separately on the lists, as discussed in v1.

The patchset is based and tested on top of
pure net-next/main branch. Each commit was checked on:
* On EspressoBIN
* On SolidRun CN913x CEx7 Eval Board

Any comments or remarks will be appreciated.

Best regards,
Marcin

[1] https://github.com/semihalf-wojtas-marcin/Linux-Kernel/commits/dsa-acpi-dev
[2] https://github.com/semihalf-wojtas-marcin/edk2-platforms/commits/dsa-acpi-v2

Changelog v3 -> v4:
1/8:
* Improve commit message.

2/8:
* Improve handling the old 'fixed-link' binding in
fwnode_phy_is_fixed_link()
* Stop shadowing the real error codes in fwnode_phy_register_fixed_link()

3/8
* Make "label" property optional again
* Fix mt7530 compile error reported by kernel test robot <[email protected]>
* Simplify code update in mv88e6xxx

4/8
* Improve commit message

5/8
* Improve commit message

7/8
* Update dev_dbg message

8/8
* Move fwnode declaration
* Simplify obtaining match_data

Changelog v2 -> v3:
1/8:
* Replace forward declaration s/device_node/fwnode_handle/ in
include/linux/phy_fixed.h
* Add Florian's RB

* 3/8:
* Extend lines width in the commit message.
* While dropping dp->dn fields in the drivers, switch to
fwnode_ API in the updated places.

* 5/8:
* Update routine name to fwnode_find_parent_dev_match()
* Improve comment section
* Move the definition adjacent to a group of fwnode
APIs operating on parents

Changelog v1 -> v2:
1/8
* Drop unnecessary check in fixed_phy_get_gpiod()
* Improve line breaking
* Use device_set_node & dev_fwnode

2/8
* Switch to fwnode_property_count_u32 and fix comparison
in if statement.

3/8
* Drop dn usage entirely and use dp->fwnode only. Update
all dependent drivers to use to_of_node.
* Use device_set_node, dev_fwnode & device_get_named_child_node
* Replace '_of' routines suffix with '_fw'

4/8
* Use device_set_node

5/8
* New patch

6/8
* Use device_match_fwnode
* Restore EXPORT_SYMBOL()

7/8
* Get rid of of_mdiobus_register_device

8/8
* Use dev_fwnode in mv88e6xxx_probe
* Simplify condition checks in mv88e6xxx_probe as suggested by Andy

Marcin Wojtas (8):
net: phy: fixed_phy: switch to fwnode_ API
net: mdio: switch fixed-link PHYs API to fwnode_
net: dsa: switch to device_/fwnode_ APIs
net: mvpp2: initialize port fwnode pointer
device property: introduce fwnode_find_parent_dev_match
net: core: switch to fwnode_find_net_device_by_node()
net: mdio: introduce fwnode_mdiobus_register_device()
net: dsa: mv88e6xxx: switch to device_/fwnode_ APIs

include/linux/etherdevice.h | 1 +
include/linux/fwnode_mdio.h | 22 ++++
include/linux/of_net.h | 6 -
include/linux/phy_fixed.h | 6 +-
include/linux/property.h | 1 +
include/net/dsa.h | 2 +-
net/dsa/port.h | 4 +-
drivers/base/property.c | 23 ++++
drivers/net/dsa/mt7530.c | 6 +-
drivers/net/dsa/mv88e6xxx/chip.c | 57 ++++-----
drivers/net/dsa/qca/qca8k-8xxx.c | 2 +-
drivers/net/dsa/realtek/rtl8365mb.c | 2 +-
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
drivers/net/mdio/fwnode_mdio.c | 125 ++++++++++++++++++++
drivers/net/mdio/of_mdio.c | 112 +-----------------
drivers/net/phy/fixed_phy.c | 39 +++---
net/core/net-sysfs.c | 25 ++--
net/dsa/dsa.c | 118 +++++++++---------
net/dsa/port.c | 85 +++++++------
net/dsa/slave.c | 7 +-
20 files changed, 341 insertions(+), 304 deletions(-)

--
2.29.0


2023-01-16 18:16:50

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v4 2/8] 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 | 96 ++++++++++++++++++++
drivers/net/mdio/of_mdio.c | 79 +---------------
3 files changed, 118 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 b782c35c4ac1..56f57381ae69 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>
#include <linux/pse-pd/pse.h>

MODULE_AUTHOR("Calvin Johnson <[email protected]>");
@@ -185,3 +186,98 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
return rc;
}
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;
+
+ /* 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 */
+ return fwnode_property_count_u32(fwnode, "fixed-link") == 5;
+}
+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;
+ int rc;
+
+ 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");
+ rc = fwnode_property_read_u32(fixed_link_node, "speed",
+ &status.speed);
+ if (rc) {
+ fwnode_handle_put(fixed_link_node);
+ return rc;
+ }
+ 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 */
+ rc = fwnode_property_read_u32_array(fwnode, "fixed-link", fixed_link_prop,
+ ARRAY_SIZE(fixed_link_prop));
+ if (rc)
+ return rc;
+
+ 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];
+
+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 ba22b7110cdc..e6b3a4e251a1 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -353,91 +353,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

2023-01-16 18:16:53

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v4 6/8] net: core: switch to fwnode_find_net_device_by_node()

A helper function which allows getting the struct net_device pointer
associated with a given device tree node can be more generic and
also support alternative hardware description. Switch to fwnode_
and update the only existing caller (in the DSA subsystem).
For that purpose use newly added fwnode_find_parent_dev_match routine.

Signed-off-by: Marcin Wojtas <[email protected]>
---
include/linux/etherdevice.h | 1 +
include/linux/of_net.h | 6 -----
net/core/net-sysfs.c | 25 ++++++--------------
net/dsa/dsa.c | 5 ++--
4 files changed, 11 insertions(+), 26 deletions(-)

diff --git a/include/linux/etherdevice.h b/include/linux/etherdevice.h
index a541f0c4f146..3d475ee4a2d7 100644
--- a/include/linux/etherdevice.h
+++ b/include/linux/etherdevice.h
@@ -35,6 +35,7 @@ int nvmem_get_mac_address(struct device *dev, void *addrbuf);
int device_get_mac_address(struct device *dev, char *addr);
int device_get_ethdev_address(struct device *dev, struct net_device *netdev);
int fwnode_get_mac_address(struct fwnode_handle *fwnode, char *addr);
+struct net_device *fwnode_find_net_device_by_node(struct fwnode_handle *fwnode);

u32 eth_get_headlen(const struct net_device *dev, const void *data, u32 len);
__be16 eth_type_trans(struct sk_buff *skb, struct net_device *dev);
diff --git a/include/linux/of_net.h b/include/linux/of_net.h
index d88715a0b3a5..8a677f44c270 100644
--- a/include/linux/of_net.h
+++ b/include/linux/of_net.h
@@ -16,7 +16,6 @@ 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);
extern int of_get_mac_address_nvmem(struct device_node *np, u8 *mac);
int of_get_ethdev_address(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,
phy_interface_t *interface)
@@ -38,11 +37,6 @@ static inline int of_get_ethdev_address(struct device_node *np, struct net_devic
{
return -ENODEV;
}
-
-static inline struct net_device *of_find_net_device_by_node(struct device_node *np)
-{
- return NULL;
-}
#endif

#endif /* __LINUX_OF_NET_H */
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index ca55dd747d6c..652ba785e6f7 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -6,6 +6,7 @@
*/

#include <linux/capability.h>
+#include <linux/etherdevice.h>
#include <linux/kernel.h>
#include <linux/netdevice.h>
#include <linux/if_arp.h>
@@ -1935,38 +1936,26 @@ static struct class net_class __ro_after_init = {
.get_ownership = net_get_ownership,
};

-#ifdef CONFIG_OF
-static int of_dev_node_match(struct device *dev, const void *data)
-{
- for (; dev; dev = dev->parent) {
- if (dev->of_node == data)
- return 1;
- }
-
- return 0;
-}
-
/*
- * of_find_net_device_by_node - lookup the net device for the device node
- * @np: OF device node
+ * fwnode_find_net_device_by_node - lookup the net device for the device fwnode
+ * @fwnode: firmware node
*
- * Looks up the net_device structure corresponding with the device node.
+ * Looks up the net_device structure corresponding with the fwnode.
* If successful, returns a pointer to the net_device with the embedded
* struct device refcount incremented by one, or NULL on failure. The
* refcount must be dropped when done with the net_device.
*/
-struct net_device *of_find_net_device_by_node(struct device_node *np)
+struct net_device *fwnode_find_net_device_by_node(struct fwnode_handle *fwnode)
{
struct device *dev;

- dev = class_find_device(&net_class, NULL, np, of_dev_node_match);
+ dev = class_find_device(&net_class, NULL, fwnode, fwnode_find_parent_dev_match);
if (!dev)
return NULL;

return to_net_dev(dev);
}
-EXPORT_SYMBOL(of_find_net_device_by_node);
-#endif
+EXPORT_SYMBOL(fwnode_find_net_device_by_node);

/* Delete sysfs entries but hold kobject reference until after all
* netdev references are gone.
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index d1ca3bb03858..55f22a58556b 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -9,6 +9,7 @@

#include <linux/device.h>
#include <linux/err.h>
+#include <linux/etherdevice.h>
#include <linux/list.h>
#include <linux/module.h>
#include <linux/netdevice.h>
@@ -374,7 +375,7 @@ struct net_device *dsa_tree_find_first_master(struct dsa_switch_tree *dst)
if (IS_ERR(ethernet))
return NULL;

- master = of_find_net_device_by_node(to_of_node(ethernet));
+ master = fwnode_find_net_device_by_node(ethernet);
fwnode_handle_put(ethernet);

return master;
@@ -1233,7 +1234,7 @@ static int dsa_port_parse_fw(struct dsa_port *dp, struct fwnode_handle *fwnode)
struct net_device *master;
const char *user_protocol;

- master = of_find_net_device_by_node(to_of_node(ethernet));
+ master = fwnode_find_net_device_by_node(ethernet);
fwnode_handle_put(ethernet);
if (!master)
return -EPROBE_DEFER;
--
2.29.0

2023-01-16 18:21:18

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v4 7/8] net: mdio: introduce fwnode_mdiobus_register_device()

As a preparation patch to extend MDIO capabilities in the ACPI world,
introduce fwnode_mdiobus_register_device() to register non-PHY
devices on the mdiobus.

Use the newly introduced routine instead of of_mdiobus_register_device().

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

diff --git a/include/linux/fwnode_mdio.h b/include/linux/fwnode_mdio.h
index 98755b8c6c8a..39d74c5d1bb0 100644
--- a/include/linux/fwnode_mdio.h
+++ b/include/linux/fwnode_mdio.h
@@ -16,6 +16,9 @@ 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_mdiobus_register_device(struct mii_bus *mdio,
+ 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);
diff --git a/drivers/net/mdio/fwnode_mdio.c b/drivers/net/mdio/fwnode_mdio.c
index 56f57381ae69..4d712d8873d0 100644
--- a/drivers/net/mdio/fwnode_mdio.c
+++ b/drivers/net/mdio/fwnode_mdio.c
@@ -187,6 +187,35 @@ int fwnode_mdiobus_register_phy(struct mii_bus *bus,
}
EXPORT_SYMBOL(fwnode_mdiobus_register_phy);

+int fwnode_mdiobus_register_device(struct mii_bus *mdio,
+ struct fwnode_handle *child, u32 addr)
+{
+ struct mdio_device *mdiodev;
+ int rc;
+
+ mdiodev = mdio_device_create(mdio, addr);
+ if (IS_ERR(mdiodev))
+ return PTR_ERR(mdiodev);
+
+ /* Associate the fwnode with the device structure so it
+ * can be looked up later.
+ */
+ device_set_node(&mdiodev->dev, child);
+
+ /* All data is now stored in the mdiodev struct; register it. */
+ rc = mdio_device_register(mdiodev);
+ if (rc) {
+ device_set_node(&mdiodev->dev, NULL);
+ mdio_device_free(mdiodev);
+ return rc;
+ }
+
+ dev_dbg(&mdio->dev, "registered mdio device %pfw at address %i\n",
+ child, addr);
+ return 0;
+}
+EXPORT_SYMBOL(fwnode_mdiobus_register_device);
+
/*
* fwnode_phy_is_fixed_link() and fwnode_phy_register_fixed_link() must
* support two bindings:
diff --git a/drivers/net/mdio/of_mdio.c b/drivers/net/mdio/of_mdio.c
index e6b3a4e251a1..685ac00f9dee 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -48,37 +48,6 @@ static int of_mdiobus_register_phy(struct mii_bus *mdio,
return fwnode_mdiobus_register_phy(mdio, of_fwnode_handle(child), addr);
}

-static int of_mdiobus_register_device(struct mii_bus *mdio,
- struct device_node *child, u32 addr)
-{
- struct fwnode_handle *fwnode = of_fwnode_handle(child);
- struct mdio_device *mdiodev;
- int rc;
-
- mdiodev = mdio_device_create(mdio, addr);
- if (IS_ERR(mdiodev))
- return PTR_ERR(mdiodev);
-
- /* Associate the OF node with the device structure so it
- * can be looked up later.
- */
- fwnode_handle_get(fwnode);
- device_set_node(&mdiodev->dev, fwnode);
-
- /* All data is now stored in the mdiodev struct; register it. */
- rc = mdio_device_register(mdiodev);
- if (rc) {
- device_set_node(&mdiodev->dev, NULL);
- fwnode_handle_put(fwnode);
- mdio_device_free(mdiodev);
- return rc;
- }
-
- dev_dbg(&mdio->dev, "registered mdio device %pOFn at address %i\n",
- child, addr);
- return 0;
-}
-
/* The following is a list of PHY compatible strings which appear in
* some DTBs. The compatible string is never matched against a PHY
* driver, so is pointless. We only expect devices which are not PHYs
@@ -187,7 +156,7 @@ int of_mdiobus_register(struct mii_bus *mdio, struct device_node *np)
if (of_mdiobus_child_is_phy(child))
rc = of_mdiobus_register_phy(mdio, child, addr);
else
- rc = of_mdiobus_register_device(mdio, child, addr);
+ rc = fwnode_mdiobus_register_device(mdio, of_fwnode_handle(child), addr);

if (rc == -ENODEV)
dev_err(&mdio->dev,
--
2.29.0

2023-01-16 18:45:30

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v4 4/8] net: mvpp2: initialize port fwnode pointer

As a preparation to switch the DSA subsystem from using
of_find_net_device_by_node() to its more generic fwnode_ equivalent,
and later to allow ACPI description, update the port's device structure
also with its fwnode pointer.

Signed-off-by: Marcin Wojtas <[email protected]>
---
drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
index 5f89fcec07b1..a25e90791700 100644
--- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
+++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
@@ -6884,7 +6884,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
dev->min_mtu = ETH_MIN_MTU;
/* 9704 == 9728 - 20 and rounding to 8 */
dev->max_mtu = MVPP2_BM_JUMBO_PKT_SIZE;
- dev->dev.of_node = port_node;
+ device_set_node(&dev->dev, port_fwnode);

port->pcs_gmac.ops = &mvpp2_phylink_gmac_pcs_ops;
port->pcs_xlg.ops = &mvpp2_phylink_xlg_pcs_ops;
--
2.29.0

2023-01-16 18:47:10

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v4 3/8] net: dsa: switch to device_/fwnode_ APIs

In order to support both DT and ACPI in future, modify the generic DSA
code to use device_/fwnode_ equivalent routines. Drop using port's 'dn'
field and use only fwnode - update all dependent drivers.

Because support for more generic fwnode is added, replace '_of' suffix
with '_fw' in related routines. No functional change is introduced by
this patch.

Signed-off-by: Marcin Wojtas <[email protected]>
---
include/net/dsa.h | 2 +-
net/dsa/port.h | 4 +-
drivers/net/dsa/mt7530.c | 6 +-
drivers/net/dsa/mv88e6xxx/chip.c | 16 +--
drivers/net/dsa/qca/qca8k-8xxx.c | 2 +-
drivers/net/dsa/realtek/rtl8365mb.c | 2 +-
net/dsa/dsa.c | 117 +++++++++++---------
net/dsa/port.c | 85 +++++++-------
net/dsa/slave.c | 7 +-
9 files changed, 120 insertions(+), 121 deletions(-)

diff --git a/include/net/dsa.h b/include/net/dsa.h
index 96086289aa9b..b933b88ace78 100644
--- a/include/net/dsa.h
+++ b/include/net/dsa.h
@@ -300,7 +300,7 @@ struct dsa_port {

u8 setup:1;

- struct device_node *dn;
+ struct fwnode_handle *fwnode;
unsigned int ageing_time;

struct dsa_bridge *bridge;
diff --git a/net/dsa/port.h b/net/dsa/port.h
index 9c218660d223..5037b8be982e 100644
--- a/net/dsa/port.h
+++ b/net/dsa/port.h
@@ -101,8 +101,8 @@ int dsa_port_mrp_del_ring_role(const struct dsa_port *dp,
const struct switchdev_obj_ring_role_mrp *mrp);
int dsa_port_phylink_create(struct dsa_port *dp);
void dsa_port_phylink_destroy(struct dsa_port *dp);
-int dsa_shared_port_link_register_of(struct dsa_port *dp);
-void dsa_shared_port_link_unregister_of(struct dsa_port *dp);
+int dsa_shared_port_link_register_fw(struct dsa_port *dp);
+void dsa_shared_port_link_unregister_fw(struct dsa_port *dp);
int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr);
void dsa_port_hsr_leave(struct dsa_port *dp, struct net_device *hsr);
int dsa_port_tag_8021q_vlan_add(struct dsa_port *dp, u16 vid, bool broadcast);
diff --git a/drivers/net/dsa/mt7530.c b/drivers/net/dsa/mt7530.c
index 908fa89444c9..35c4c71216fe 100644
--- a/drivers/net/dsa/mt7530.c
+++ b/drivers/net/dsa/mt7530.c
@@ -2235,8 +2235,10 @@ mt7530_setup(struct dsa_switch *ds)

if (!dsa_is_unused_port(ds, 5)) {
priv->p5_intf_sel = P5_INTF_SEL_GMAC5;
- ret = of_get_phy_mode(dsa_to_port(ds, 5)->dn, &interface);
- if (ret && ret != -ENODEV)
+ ret = fwnode_get_phy_mode(dsa_to_port(ds, 5)->fwnode);
+ if (ret >= 0)
+ interface = ret;
+ else if (ret != -ENODEV)
return ret;
} else {
/* Scan the ethernet nodes. look for GMAC1, lookup used phy */
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 1168ea75f5f5..6731597bded0 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3285,7 +3285,7 @@ static int mv88e6xxx_setup_upstream_port(struct mv88e6xxx_chip *chip, int port)

static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
{
- struct device_node *phy_handle = NULL;
+ struct fwnode_handle *phy_handle = NULL;
struct dsa_switch *ds = chip->ds;
phy_interface_t mode;
struct dsa_port *dp;
@@ -3509,18 +3509,14 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)

if (chip->info->ops->serdes_set_tx_amplitude) {
if (dp)
- phy_handle = of_parse_phandle(dp->dn, "phy-handle", 0);
+ phy_handle = fwnode_find_reference(dp->fwnode, "phy-handle", 0);

- if (phy_handle && !of_property_read_u32(phy_handle,
- "tx-p2p-microvolt",
- &tx_amp))
+ if (!fwnode_property_read_u32(phy_handle, "tx-p2p-microvolt", &tx_amp))
err = chip->info->ops->serdes_set_tx_amplitude(chip,
port, tx_amp);
- if (phy_handle) {
- of_node_put(phy_handle);
- if (err)
- return err;
- }
+ fwnode_handle_put(phy_handle);
+ if (err)
+ return err;
}

/* Port based VLAN map: give each port the same default address
diff --git a/drivers/net/dsa/qca/qca8k-8xxx.c b/drivers/net/dsa/qca/qca8k-8xxx.c
index 2f224b166bbb..6ceb9478309f 100644
--- a/drivers/net/dsa/qca/qca8k-8xxx.c
+++ b/drivers/net/dsa/qca/qca8k-8xxx.c
@@ -1088,7 +1088,7 @@ qca8k_parse_port_config(struct qca8k_priv *priv)
continue;

dp = dsa_to_port(priv->ds, port);
- port_dn = dp->dn;
+ port_dn = to_of_node(dp->fwnode);
cpu_port_index++;

if (!of_device_is_available(port_dn))
diff --git a/drivers/net/dsa/realtek/rtl8365mb.c b/drivers/net/dsa/realtek/rtl8365mb.c
index da31d8b839ac..d61da012451f 100644
--- a/drivers/net/dsa/realtek/rtl8365mb.c
+++ b/drivers/net/dsa/realtek/rtl8365mb.c
@@ -887,7 +887,7 @@ static int rtl8365mb_ext_config_rgmii(struct realtek_priv *priv, int port,
return -ENODEV;

dp = dsa_to_port(priv->ds, port);
- dn = dp->dn;
+ dn = to_of_node(dp->fwnode);

/* Set the RGMII TX/RX delay
*
diff --git a/net/dsa/dsa.c b/net/dsa/dsa.c
index e5f156940c67..d1ca3bb03858 100644
--- a/net/dsa/dsa.c
+++ b/net/dsa/dsa.c
@@ -273,12 +273,12 @@ static void dsa_tree_put(struct dsa_switch_tree *dst)
}

static struct dsa_port *dsa_tree_find_port_by_node(struct dsa_switch_tree *dst,
- struct device_node *dn)
+ struct fwnode_handle *fwnode)
{
struct dsa_port *dp;

list_for_each_entry(dp, &dst->ports, list)
- if (dp->dn == dn)
+ if (dp->fwnode == fwnode)
return dp;

return NULL;
@@ -314,14 +314,13 @@ static bool dsa_port_setup_routing_table(struct dsa_port *dp)
{
struct dsa_switch *ds = dp->ds;
struct dsa_switch_tree *dst = ds->dst;
- struct device_node *dn = dp->dn;
struct of_phandle_iterator it;
struct dsa_port *link_dp;
struct dsa_link *dl;
int err;

- of_for_each_phandle(&it, err, dn, "link", NULL, 0) {
- link_dp = dsa_tree_find_port_by_node(dst, it.node);
+ of_for_each_phandle(&it, err, to_of_node(dp->fwnode), "link", NULL, 0) {
+ link_dp = dsa_tree_find_port_by_node(dst, of_fwnode_handle(it.node));
if (!link_dp) {
of_node_put(it.node);
return false;
@@ -366,14 +365,17 @@ static struct dsa_port *dsa_tree_find_first_cpu(struct dsa_switch_tree *dst)

struct net_device *dsa_tree_find_first_master(struct dsa_switch_tree *dst)
{
- struct device_node *ethernet;
+ struct fwnode_handle *ethernet;
struct net_device *master;
struct dsa_port *cpu_dp;

cpu_dp = dsa_tree_find_first_cpu(dst);
- ethernet = of_parse_phandle(cpu_dp->dn, "ethernet", 0);
- master = of_find_net_device_by_node(ethernet);
- of_node_put(ethernet);
+ ethernet = fwnode_find_reference(cpu_dp->fwnode, "ethernet", 0);
+ if (IS_ERR(ethernet))
+ return NULL;
+
+ master = of_find_net_device_by_node(to_of_node(ethernet));
+ fwnode_handle_put(ethernet);

return master;
}
@@ -457,8 +459,8 @@ static int dsa_port_setup(struct dsa_port *dp)
dsa_port_disable(dp);
break;
case DSA_PORT_TYPE_CPU:
- if (dp->dn) {
- err = dsa_shared_port_link_register_of(dp);
+ if (dp->fwnode) {
+ err = dsa_shared_port_link_register_fw(dp);
if (err)
break;
dsa_port_link_registered = true;
@@ -475,8 +477,8 @@ static int dsa_port_setup(struct dsa_port *dp)

break;
case DSA_PORT_TYPE_DSA:
- if (dp->dn) {
- err = dsa_shared_port_link_register_of(dp);
+ if (dp->fwnode) {
+ err = dsa_shared_port_link_register_fw(dp);
if (err)
break;
dsa_port_link_registered = true;
@@ -493,7 +495,7 @@ static int dsa_port_setup(struct dsa_port *dp)

break;
case DSA_PORT_TYPE_USER:
- of_get_mac_address(dp->dn, dp->mac);
+ fwnode_get_mac_address(dp->fwnode, dp->mac);
err = dsa_slave_create(dp);
break;
}
@@ -501,7 +503,7 @@ static int dsa_port_setup(struct dsa_port *dp)
if (err && dsa_port_enabled)
dsa_port_disable(dp);
if (err && dsa_port_link_registered)
- dsa_shared_port_link_unregister_of(dp);
+ dsa_shared_port_link_unregister_fw(dp);
if (err) {
dsa_port_devlink_teardown(dp);
return err;
@@ -522,13 +524,13 @@ static void dsa_port_teardown(struct dsa_port *dp)
break;
case DSA_PORT_TYPE_CPU:
dsa_port_disable(dp);
- if (dp->dn)
- dsa_shared_port_link_unregister_of(dp);
+ if (dp->fwnode)
+ dsa_shared_port_link_unregister_fw(dp);
break;
case DSA_PORT_TYPE_DSA:
dsa_port_disable(dp);
- if (dp->dn)
- dsa_shared_port_link_unregister_of(dp);
+ if (dp->fwnode)
+ dsa_shared_port_link_unregister_fw(dp);
break;
case DSA_PORT_TYPE_USER:
if (dp->slave) {
@@ -603,7 +605,7 @@ static void dsa_switch_teardown_tag_protocol(struct dsa_switch *ds)

static int dsa_switch_setup(struct dsa_switch *ds)
{
- struct device_node *dn;
+ struct fwnode_handle *fwnode;
int err;

if (ds->setup)
@@ -643,10 +645,10 @@ static int dsa_switch_setup(struct dsa_switch *ds)

dsa_slave_mii_bus_init(ds);

- dn = of_get_child_by_name(ds->dev->of_node, "mdio");
+ fwnode = device_get_named_child_node(ds->dev, "mdio");

- err = of_mdiobus_register(ds->slave_mii_bus, dn);
- of_node_put(dn);
+ err = of_mdiobus_register(ds->slave_mii_bus, to_of_node(fwnode));
+ fwnode_handle_put(fwnode);
if (err < 0)
goto free_slave_mii_bus;
}
@@ -1216,24 +1218,31 @@ static int dsa_port_parse_cpu(struct dsa_port *dp, struct net_device *master,
return 0;
}

-static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
+static int dsa_port_parse_fw(struct dsa_port *dp, struct fwnode_handle *fwnode)
{
- struct device_node *ethernet = of_parse_phandle(dn, "ethernet", 0);
- const char *name = of_get_property(dn, "label", NULL);
- bool link = of_property_read_bool(dn, "link");
+ struct fwnode_handle *ethernet = fwnode_find_reference(fwnode, "ethernet", 0);
+ bool link = fwnode_property_present(fwnode, "link");
+ const char *name;
+ int ret;

- dp->dn = dn;
+ fwnode_property_read_string(fwnode, "label", &name);

- if (ethernet) {
+ dp->fwnode = fwnode;
+
+ if (!IS_ERR(ethernet)) {
struct net_device *master;
const char *user_protocol;

- master = of_find_net_device_by_node(ethernet);
- of_node_put(ethernet);
+ master = of_find_net_device_by_node(to_of_node(ethernet));
+ fwnode_handle_put(ethernet);
if (!master)
return -EPROBE_DEFER;

- user_protocol = of_get_property(dn, "dsa-tag-protocol", NULL);
+ ret = fwnode_property_read_string(fwnode, "dsa-tag-protocol",
+ &user_protocol);
+ if (ret)
+ user_protocol = NULL;
+
return dsa_port_parse_cpu(dp, master, user_protocol);
}

@@ -1243,61 +1252,61 @@ static int dsa_port_parse_of(struct dsa_port *dp, struct device_node *dn)
return dsa_port_parse_user(dp, name);
}

-static int dsa_switch_parse_ports_of(struct dsa_switch *ds,
- struct device_node *dn)
+static int dsa_switch_parse_ports_fw(struct dsa_switch *ds,
+ struct fwnode_handle *fwnode)
{
- struct device_node *ports, *port;
+ struct fwnode_handle *ports, *port;
struct dsa_port *dp;
int err = 0;
u32 reg;

- ports = of_get_child_by_name(dn, "ports");
+ ports = fwnode_get_named_child_node(fwnode, "ports");
if (!ports) {
/* The second possibility is "ethernet-ports" */
- ports = of_get_child_by_name(dn, "ethernet-ports");
+ ports = fwnode_get_named_child_node(fwnode, "ethernet-ports");
if (!ports) {
dev_err(ds->dev, "no ports child node found\n");
return -EINVAL;
}
}

- for_each_available_child_of_node(ports, port) {
- err = of_property_read_u32(port, "reg", &reg);
+ fwnode_for_each_available_child_node(ports, port) {
+ err = fwnode_property_read_u32(port, "reg", &reg);
if (err) {
- of_node_put(port);
+ fwnode_handle_put(port);
goto out_put_node;
}

if (reg >= ds->num_ports) {
dev_err(ds->dev, "port %pOF index %u exceeds num_ports (%u)\n",
port, reg, ds->num_ports);
- of_node_put(port);
+ fwnode_handle_put(port);
err = -EINVAL;
goto out_put_node;
}

dp = dsa_to_port(ds, reg);

- err = dsa_port_parse_of(dp, port);
+ err = dsa_port_parse_fw(dp, port);
if (err) {
- of_node_put(port);
+ fwnode_handle_put(port);
goto out_put_node;
}
}

out_put_node:
- of_node_put(ports);
+ fwnode_handle_put(ports);
return err;
}

-static int dsa_switch_parse_member_of(struct dsa_switch *ds,
- struct device_node *dn)
+static int dsa_switch_parse_member_fw(struct dsa_switch *ds,
+ struct fwnode_handle *fwnode)
{
u32 m[2] = { 0, 0 };
int sz;

/* Don't error out if this optional property isn't found */
- sz = of_property_read_variable_u32_array(dn, "dsa,member", m, 2, 2);
+ sz = fwnode_property_read_u32_array(fwnode, "dsa,member", m, 2);
if (sz < 0 && sz != -EINVAL)
return sz;

@@ -1334,11 +1343,11 @@ static int dsa_switch_touch_ports(struct dsa_switch *ds)
return 0;
}

-static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
+static int dsa_switch_parse_fw(struct dsa_switch *ds, struct fwnode_handle *fwnode)
{
int err;

- err = dsa_switch_parse_member_of(ds, dn);
+ err = dsa_switch_parse_member_fw(ds, fwnode);
if (err)
return err;

@@ -1346,7 +1355,7 @@ static int dsa_switch_parse_of(struct dsa_switch *ds, struct device_node *dn)
if (err)
return err;

- return dsa_switch_parse_ports_of(ds, dn);
+ return dsa_switch_parse_ports_fw(ds, fwnode);
}

static int dev_is_class(struct device *dev, void *class)
@@ -1475,20 +1484,20 @@ static int dsa_switch_probe(struct dsa_switch *ds)
{
struct dsa_switch_tree *dst;
struct dsa_chip_data *pdata;
- struct device_node *np;
+ struct fwnode_handle *fwnode;
int err;

if (!ds->dev)
return -ENODEV;

pdata = ds->dev->platform_data;
- np = ds->dev->of_node;
+ fwnode = dev_fwnode(ds->dev);

if (!ds->num_ports)
return -EINVAL;

- if (np) {
- err = dsa_switch_parse_of(ds, np);
+ if (fwnode) {
+ err = dsa_switch_parse_fw(ds, fwnode);
if (err)
dsa_switch_release_ports(ds);
} else if (pdata) {
diff --git a/net/dsa/port.c b/net/dsa/port.c
index 67ad1adec2a2..8f5793f87f40 100644
--- a/net/dsa/port.c
+++ b/net/dsa/port.c
@@ -6,11 +6,10 @@
* Vivien Didelot <[email protected]>
*/

+#include <linux/fwnode_mdio.h>
#include <linux/if_bridge.h>
#include <linux/netdevice.h>
#include <linux/notifier.h>
-#include <linux/of_mdio.h>
-#include <linux/of_net.h>

#include "dsa.h"
#include "port.h"
@@ -1535,20 +1534,20 @@ void dsa_port_set_tag_protocol(struct dsa_port *cpu_dp,

static struct phy_device *dsa_port_get_phy_device(struct dsa_port *dp)
{
- struct device_node *phy_dn;
+ struct fwnode_handle *phy_handle;
struct phy_device *phydev;

- phy_dn = of_parse_phandle(dp->dn, "phy-handle", 0);
- if (!phy_dn)
+ phy_handle = fwnode_find_reference(dp->fwnode, "phy-handle", 0);
+ if (IS_ERR(phy_handle))
return NULL;

- phydev = of_phy_find_device(phy_dn);
+ phydev = fwnode_phy_find_device(phy_handle);
if (!phydev) {
- of_node_put(phy_dn);
+ fwnode_handle_put(phy_handle);
return ERR_PTR(-EPROBE_DEFER);
}

- of_node_put(phy_dn);
+ fwnode_handle_put(phy_handle);
return phydev;
}

@@ -1678,12 +1677,11 @@ static const struct phylink_mac_ops dsa_port_phylink_mac_ops = {
int dsa_port_phylink_create(struct dsa_port *dp)
{
struct dsa_switch *ds = dp->ds;
- phy_interface_t mode;
struct phylink *pl;
- int err;
+ int mode;

- err = of_get_phy_mode(dp->dn, &mode);
- if (err)
+ mode = fwnode_get_phy_mode(dp->fwnode);
+ if (mode < 0)
mode = PHY_INTERFACE_MODE_NA;

/* Presence of phylink_mac_link_state or phylink_mac_an_restart is
@@ -1696,7 +1694,7 @@ int dsa_port_phylink_create(struct dsa_port *dp)
if (ds->ops->phylink_get_caps)
ds->ops->phylink_get_caps(ds, dp->index, &dp->pl_config);

- pl = phylink_create(&dp->pl_config, of_fwnode_handle(dp->dn),
+ pl = phylink_create(&dp->pl_config, dp->fwnode,
mode, &dsa_port_phylink_mac_ops);
if (IS_ERR(pl)) {
pr_err("error creating PHYLINK: %ld\n", PTR_ERR(pl));
@@ -1714,7 +1712,7 @@ void dsa_port_phylink_destroy(struct dsa_port *dp)
dp->pl = NULL;
}

-static int dsa_shared_port_setup_phy_of(struct dsa_port *dp, bool enable)
+static int dsa_shared_port_setup_phy_fw(struct dsa_port *dp, bool enable)
{
struct dsa_switch *ds = dp->ds;
struct phy_device *phydev;
@@ -1752,16 +1750,15 @@ static int dsa_shared_port_setup_phy_of(struct dsa_port *dp, bool enable)
return err;
}

-static int dsa_shared_port_fixed_link_register_of(struct dsa_port *dp)
+static int dsa_shared_port_fixed_link_register_fw(struct dsa_port *dp)
{
- struct device_node *dn = dp->dn;
struct dsa_switch *ds = dp->ds;
struct phy_device *phydev;
int port = dp->index;
- phy_interface_t mode;
+ int mode;
int err;

- err = of_phy_register_fixed_link(dn);
+ err = fwnode_phy_register_fixed_link(dp->fwnode);
if (err) {
dev_err(ds->dev,
"failed to register the fixed PHY of port %d\n",
@@ -1769,10 +1766,10 @@ static int dsa_shared_port_fixed_link_register_of(struct dsa_port *dp)
return err;
}

- phydev = of_phy_find_device(dn);
+ phydev = fwnode_phy_find_device(dp->fwnode);

- err = of_get_phy_mode(dn, &mode);
- if (err)
+ mode = fwnode_get_phy_mode(dp->fwnode);
+ if (mode < 0)
mode = PHY_INTERFACE_MODE_NA;
phydev->interface = mode;

@@ -1789,7 +1786,6 @@ static int dsa_shared_port_fixed_link_register_of(struct dsa_port *dp)
static int dsa_shared_port_phylink_register(struct dsa_port *dp)
{
struct dsa_switch *ds = dp->ds;
- struct device_node *port_dn = dp->dn;
int err;

dp->pl_config.dev = ds->dev;
@@ -1799,7 +1795,7 @@ static int dsa_shared_port_phylink_register(struct dsa_port *dp)
if (err)
return err;

- err = phylink_of_phy_connect(dp->pl, port_dn, 0);
+ err = phylink_fwnode_phy_connect(dp->pl, dp->fwnode, 0);
if (err && err != -ENODEV) {
pr_err("could not attach to PHY: %d\n", err);
goto err_phy_connect;
@@ -1926,51 +1922,50 @@ static const char * const dsa_switches_apply_workarounds[] = {
NULL,
};

-static void dsa_shared_port_validate_of(struct dsa_port *dp,
+static void dsa_shared_port_validate_fw(struct dsa_port *dp,
bool *missing_phy_mode,
bool *missing_link_description)
{
- struct device_node *dn = dp->dn, *phy_np;
+ struct fwnode_handle *phy_handle;
struct dsa_switch *ds = dp->ds;
- phy_interface_t mode;

*missing_phy_mode = false;
*missing_link_description = false;

- if (of_get_phy_mode(dn, &mode)) {
+ if (fwnode_get_phy_mode(dp->fwnode) < 0) {
*missing_phy_mode = true;
dev_err(ds->dev,
- "OF node %pOF of %s port %d lacks the required \"phy-mode\" property\n",
- dn, dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index);
+ "FW node %p of %s port %d lacks the required \"phy-mode\" property\n",
+ dp->fwnode, dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index);
}

- /* Note: of_phy_is_fixed_link() also returns true for
+ /* Note: fwnode_phy_is_fixed_link() also returns true for
* managed = "in-band-status"
*/
- if (of_phy_is_fixed_link(dn))
+ if (fwnode_phy_is_fixed_link(dp->fwnode))
return;

- phy_np = of_parse_phandle(dn, "phy-handle", 0);
- if (phy_np) {
- of_node_put(phy_np);
+ phy_handle = fwnode_find_reference(dp->fwnode, "phy-handle", 0);
+ if (!IS_ERR(phy_handle)) {
+ fwnode_handle_put(phy_handle);
return;
}

*missing_link_description = true;

dev_err(ds->dev,
- "OF node %pOF of %s port %d lacks the required \"phy-handle\", \"fixed-link\" or \"managed\" properties\n",
- dn, dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index);
+ "FW node %p of %s port %d lacks the required \"phy-handle\", \"fixed-link\" or \"managed\" properties\n",
+ dp->fwnode, dsa_port_is_cpu(dp) ? "CPU" : "DSA", dp->index);
}

-int dsa_shared_port_link_register_of(struct dsa_port *dp)
+int dsa_shared_port_link_register_fw(struct dsa_port *dp)
{
struct dsa_switch *ds = dp->ds;
bool missing_link_description;
bool missing_phy_mode;
int port = dp->index;

- dsa_shared_port_validate_of(dp, &missing_phy_mode,
+ dsa_shared_port_validate_fw(dp, &missing_phy_mode,
&missing_link_description);

if ((missing_phy_mode || missing_link_description) &&
@@ -1996,13 +1991,13 @@ int dsa_shared_port_link_register_of(struct dsa_port *dp)
dev_warn(ds->dev,
"Using legacy PHYLIB callbacks. Please migrate to PHYLINK!\n");

- if (of_phy_is_fixed_link(dp->dn))
- return dsa_shared_port_fixed_link_register_of(dp);
+ if (fwnode_phy_is_fixed_link(dp->fwnode))
+ return dsa_shared_port_fixed_link_register_fw(dp);
else
- return dsa_shared_port_setup_phy_of(dp, true);
+ return dsa_shared_port_setup_phy_fw(dp, true);
}

-void dsa_shared_port_link_unregister_of(struct dsa_port *dp)
+void dsa_shared_port_link_unregister_fw(struct dsa_port *dp)
{
struct dsa_switch *ds = dp->ds;

@@ -2014,10 +2009,10 @@ void dsa_shared_port_link_unregister_of(struct dsa_port *dp)
return;
}

- if (of_phy_is_fixed_link(dp->dn))
- of_phy_deregister_fixed_link(dp->dn);
+ if (fwnode_phy_is_fixed_link(dp->fwnode))
+ fwnode_phy_deregister_fixed_link(dp->fwnode);
else
- dsa_shared_port_setup_phy_of(dp, false);
+ dsa_shared_port_setup_phy_fw(dp, false);
}

int dsa_port_hsr_join(struct dsa_port *dp, struct net_device *hsr)
diff --git a/net/dsa/slave.c b/net/dsa/slave.c
index aab79c355224..fb150a543c30 100644
--- a/net/dsa/slave.c
+++ b/net/dsa/slave.c
@@ -10,8 +10,6 @@
#include <linux/phy.h>
#include <linux/phy_fixed.h>
#include <linux/phylink.h>
-#include <linux/of_net.h>
-#include <linux/of_mdio.h>
#include <linux/mdio.h>
#include <net/rtnetlink.h>
#include <net/pkt_cls.h>
@@ -2309,7 +2307,6 @@ static int dsa_slave_phy_connect(struct net_device *slave_dev, int addr,
static int dsa_slave_phy_setup(struct net_device *slave_dev)
{
struct dsa_port *dp = dsa_slave_to_port(slave_dev);
- struct device_node *port_dn = dp->dn;
struct dsa_switch *ds = dp->ds;
u32 phy_flags = 0;
int ret;
@@ -2333,7 +2330,7 @@ static int dsa_slave_phy_setup(struct net_device *slave_dev)
if (ds->ops->get_phy_flags)
phy_flags = ds->ops->get_phy_flags(ds, dp->index);

- ret = phylink_of_phy_connect(dp->pl, port_dn, phy_flags);
+ ret = phylink_fwnode_phy_connect(dp->pl, dp->fwnode, phy_flags);
if (ret == -ENODEV && ds->slave_mii_bus) {
/* We could not connect to a designated PHY or SFP, so try to
* use the switch internal MDIO bus instead
@@ -2455,7 +2452,7 @@ int dsa_slave_create(struct dsa_port *port)

SET_NETDEV_DEV(slave_dev, port->ds->dev);
SET_NETDEV_DEVLINK_PORT(slave_dev, &port->devlink_port);
- slave_dev->dev.of_node = port->dn;
+ device_set_node(&slave_dev->dev, port->fwnode);
slave_dev->vlan_features = master->vlan_features;

p = netdev_priv(slave_dev);
--
2.29.0

2023-01-16 18:48:00

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [net-next: PATCH v4 4/8] net: mvpp2: initialize port fwnode pointer

On Mon, Jan 16, 2023 at 06:34:16PM +0100, Marcin Wojtas wrote:
> As a preparation to switch the DSA subsystem from using
> of_find_net_device_by_node() to its more generic fwnode_ equivalent,
> and later to allow ACPI description, update the port's device structure
> also with its fwnode pointer.

I believe this patch worth to be submitted even before core of this series.
FWIW,
Reviewed-by: Andy Shevchenko <[email protected]>

> Signed-off-by: Marcin Wojtas <[email protected]>
> ---
> drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> index 5f89fcec07b1..a25e90791700 100644
> --- a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> +++ b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> @@ -6884,7 +6884,7 @@ static int mvpp2_port_probe(struct platform_device *pdev,
> dev->min_mtu = ETH_MIN_MTU;
> /* 9704 == 9728 - 20 and rounding to 8 */
> dev->max_mtu = MVPP2_BM_JUMBO_PKT_SIZE;
> - dev->dev.of_node = port_node;
> + device_set_node(&dev->dev, port_fwnode);
>
> port->pcs_gmac.ops = &mvpp2_phylink_gmac_pcs_ops;
> port->pcs_xlg.ops = &mvpp2_phylink_xlg_pcs_ops;
> --
> 2.29.0
>

--
With Best Regards,
Andy Shevchenko


2023-01-16 18:49:16

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v4 1/8] net: phy: fixed_phy: switch to fwnode_ API

Allow 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]>
Reviewed-by: Florian Fainelli <[email protected]>
---
include/linux/phy_fixed.h | 6 +--
drivers/net/mdio/of_mdio.c | 2 +-
drivers/net/phy/fixed_phy.c | 39 +++++++-------------
3 files changed, 18 insertions(+), 29 deletions(-)

diff --git a/include/linux/phy_fixed.h b/include/linux/phy_fixed.h
index 1acafd86ab13..b3edf0c7c687 100644
--- a/include/linux/phy_fixed.h
+++ b/include/linux/phy_fixed.h
@@ -12,7 +12,7 @@ struct fixed_phy_status {
int asym_pause;
};

-struct device_node;
+struct fwnode_handle;
struct gpio_desc;
struct net_device;

@@ -22,7 +22,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,
@@ -41,7 +41,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 510822d6d0d9..ba22b7110cdc 100644
--- a/drivers/net/mdio/of_mdio.c
+++ b/drivers/net/mdio/of_mdio.c
@@ -423,7 +423,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..e59d186f78e6 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,12 @@ 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)
- 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,28 +200,21 @@ 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),
- "link", 0, GPIOD_IN, "mdio");
+ 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)
pr_err("error getting GPIO for fixed link %pOF, proceed without\n",
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 +227,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 +258,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);
+ device_set_node(&phy->mdio.dev, fwnode);
phy->is_pseudo_fixed_link = true;

switch (status->speed) {
@@ -299,7 +288,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 +298,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 +316,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(dev_fwnode(&phy->mdio.dev));
fixed_phy_del(phy->mdio.addr);
}
EXPORT_SYMBOL_GPL(fixed_phy_unregister);
--
2.29.0

2023-01-16 18:53:48

by Marcin Wojtas

[permalink] [raw]
Subject: [net-next: PATCH v4 8/8] net: dsa: mv88e6xxx: switch to device_/fwnode_ APIs

In order to support both DT and ACPI in future, modify the
mv88e6xx driver code to use device_/fwnode_ equivalent routines.
No functional change is introduced by this patch.

Signed-off-by: Marcin Wojtas <[email protected]>
---
drivers/net/dsa/mv88e6xxx/chip.c | 41 +++++++++-----------
1 file changed, 19 insertions(+), 22 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 6731597bded0..1f1dd3dd4012 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -3932,10 +3932,11 @@ static int mv88e6xxx_mdio_write_c45(struct mii_bus *bus, int phy, int devad,
}

static int mv88e6xxx_mdio_register(struct mv88e6xxx_chip *chip,
- struct device_node *np,
+ struct fwnode_handle *fwnode,
bool external)
{
static int index;
+ struct device_node *np = to_of_node(fwnode);
struct mv88e6xxx_mdio_bus *mdio_bus;
struct mii_bus *bus;
int err;
@@ -4016,18 +4017,18 @@ static void mv88e6xxx_mdios_unregister(struct mv88e6xxx_chip *chip)
}

static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip,
- struct device_node *np)
+ struct fwnode_handle *fwnode)
{
- struct device_node *child;
+ struct fwnode_handle *child;
int err;

/* Always register one mdio bus for the internal/default mdio
* bus. This maybe represented in the device tree, but is
* optional.
*/
- child = of_get_child_by_name(np, "mdio");
+ child = fwnode_get_named_child_node(fwnode, "mdio");
err = mv88e6xxx_mdio_register(chip, child, false);
- of_node_put(child);
+ fwnode_handle_put(child);
if (err)
return err;

@@ -4035,13 +4036,13 @@ static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip,
* which say they are compatible with the external mdio
* bus.
*/
- for_each_available_child_of_node(np, child) {
- if (of_device_is_compatible(
- child, "marvell,mv88e6xxx-mdio-external")) {
+ fwnode_for_each_available_child_node(fwnode, child) {
+ if (fwnode_property_match_string(child, "compatible",
+ "marvell,mv88e6xxx-mdio-external") == 0) {
err = mv88e6xxx_mdio_register(chip, child, true);
if (err) {
mv88e6xxx_mdios_unregister(chip);
- of_node_put(child);
+ fwnode_handle_put(child);
return err;
}
}
@@ -7096,18 +7097,14 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
struct dsa_mv88e6xxx_pdata *pdata = mdiodev->dev.platform_data;
const struct mv88e6xxx_info *compat_info = NULL;
struct device *dev = &mdiodev->dev;
- struct device_node *np = dev->of_node;
+ struct fwnode_handle *fwnode = dev_fwnode(dev);
struct mv88e6xxx_chip *chip;
int port;
int err;

- if (!np && !pdata)
- return -EINVAL;
-
- if (np)
- compat_info = of_device_get_match_data(dev);
+ compat_info = device_get_match_data(dev);

- if (pdata) {
+ if (!compat_info && pdata) {
compat_info = pdata_device_get_match_data(dev);

if (!pdata->netdev)
@@ -7164,9 +7161,9 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
mv88e6xxx_phy_init(chip);

if (chip->info->ops->get_eeprom) {
- if (np)
- of_property_read_u32(np, "eeprom-length",
- &chip->eeprom_len);
+ if (fwnode)
+ device_property_read_u32(dev, "eeprom-length",
+ &chip->eeprom_len);
else
chip->eeprom_len = pdata->eeprom_len;
}
@@ -7177,8 +7174,8 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (err)
goto out;

- if (np) {
- chip->irq = of_irq_get(np, 0);
+ if (fwnode) {
+ chip->irq = fwnode_irq_get(fwnode, 0);
if (chip->irq == -EPROBE_DEFER) {
err = chip->irq;
goto out;
@@ -7216,7 +7213,7 @@ static int mv88e6xxx_probe(struct mdio_device *mdiodev)
if (err)
goto out_g1_atu_prob_irq;

- err = mv88e6xxx_mdios_register(chip, np);
+ err = mv88e6xxx_mdios_register(chip, fwnode);
if (err)
goto out_g1_vtu_prob_irq;

--
2.29.0

2023-01-16 23:18:09

by Andrew Lunn

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

> +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;
> + int rc;
> +
> + 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");
> + rc = fwnode_property_read_u32(fixed_link_node, "speed",
> + &status.speed);
> + if (rc) {
> + fwnode_handle_put(fixed_link_node);
> + return rc;
> + }
> + 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 */
> + rc = fwnode_property_read_u32_array(fwnode, "fixed-link", fixed_link_prop,
> + ARRAY_SIZE(fixed_link_prop));
> + if (rc)
> + return rc;
> +
> + 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];

This is one example of the issue i just pointed out. The "Old binding"
has been deprecated for years. Maybe a decade? There is no reason it
should be used in ACPI.

Andrew

2023-01-17 14:38:55

by Arun Ramadoss

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

Hi Marcin,

On Mon, 2023-01-16 at 18:34 +0100, 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.
>
> Signed-off-by: Marcin Wojtas <[email protected]>
> ---
> include/linux/fwnode_mdio.h | 19 ++++
> drivers/net/mdio/fwnode_mdio.c | 96 ++++++++++++++++++++
> drivers/net/mdio/of_mdio.c | 79 +---------------
> 3 files changed, 118 insertions(+), 76 deletions(-)
>
> #endif /* __LINUX_FWNODE_MDIO_H */
> diff --git a/drivers/net/mdio/fwnode_mdio.c
> b/drivers/net/mdio/fwnode_mdio.c
> index b782c35c4ac1..56f57381ae69 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>
> #include <linux/pse-pd/pse.h>
>
> MODULE_AUTHOR("Calvin Johnson <[email protected]>");
> @@ -185,3 +186,98 @@ int fwnode_mdiobus_register_phy(struct mii_bus
> *bus,
> return rc;
> }
> 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.
> + */

networking comment style

> +bool fwnode_phy_is_fixed_link(struct fwnode_handle *fwnode)
> +{
> + struct fwnode_handle *fixed_link_node;
> + const char *managed;
> +
> + /* 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 */
> + return fwnode_property_count_u32(fwnode, "fixed-link") == 5;
> +}
> +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;

Reverse christmas tree

> + u32 fixed_link_prop[5];
> + const char *managed;
> + int rc;
> +
> + 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");
> + rc = fwnode_property_read_u32(fixed_link_node, "speed",
> + &status.speed);
> + if (rc) {
> + fwnode_handle_put(fixed_link_node);
> + return rc;
> + }
> + 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 */
> + rc = fwnode_property_read_u32_array(fwnode, "fixed-link",
> fixed_link_prop,
> + ARRAY_SIZE(fixed_link_prop)
> );
> + if (rc)
> + return rc;
> +
> + 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];
> +
> +register_phy:
> + return PTR_ERR_OR_ZERO(fixed_phy_register(PHY_POLL, &status,
> fwnode));
> +}
> +EXPORT_SYMBOL(fwnode_phy_register_fixed_link);
> +
>
>

2023-01-17 19:05:41

by Russell King (Oracle)

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

Hi Marcin,

On Tue, Jan 17, 2023 at 05:20:01PM +0100, Marcin Wojtas wrote:
> Hi Russell,
>
>
> pon., 16 sty 2023 o 18:50 Russell King (Oracle) <[email protected]>
> napisał(a):
> >
> > On Mon, Jan 16, 2023 at 06:34:14PM +0100, 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.
> >
> > Would it be better to let the fixed-link PHY die, and have everyone use
> > the more flexible fixed link implementation in phylink?
> >
> ,
> This patchset did not intend to introduce any functional change, simply
> switch to a more generic HW description abstraction. Killing
> of/fwnode_phy_(de)register_fixed_link entirely seems to be a challenge, as
> there are a lot of users beyond the DSA. Otoh I see a value in having
> of_/fwnode_phy_is_fixed_link check, afaik there is no equivalent in
> phylink...

Phylink provides a much improved implementation of fixed-link that is
way more flexible than the phylib approach - it can implement speeds
in excess of 1G. DSA already supports phylink with modern updated
drivers that do not use the "adjust_link" implementation.

What I'm proposing is that we don't bring the baggage of the phylib
based fixed link forwards into fwnode, and leave this to be DT-only.
I think this is what Andrew and Vladimir have also said.

Thanks.

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

2023-01-17 20:39:18

by Marcin Wojtas

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

wt., 17 sty 2023 o 18:58 Russell King (Oracle) <[email protected]>
napisał(a):
>
> Hi Marcin,
>
> On Tue, Jan 17, 2023 at 05:20:01PM +0100, Marcin Wojtas wrote:
> > Hi Russell,
> >
> >
> > pon., 16 sty 2023 o 18:50 Russell King (Oracle) <[email protected]>
> > napisał(a):
> > >
> > > On Mon, Jan 16, 2023 at 06:34:14PM +0100, 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.
> > >
> > > Would it be better to let the fixed-link PHY die, and have everyone use
> > > the more flexible fixed link implementation in phylink?
> > >
> > ,
> > This patchset did not intend to introduce any functional change, simply
> > switch to a more generic HW description abstraction. Killing
> > of/fwnode_phy_(de)register_fixed_link entirely seems to be a challenge, as
> > there are a lot of users beyond the DSA. Otoh I see a value in having
> > of_/fwnode_phy_is_fixed_link check, afaik there is no equivalent in
> > phylink...
>
> Phylink provides a much improved implementation of fixed-link that is
> way more flexible than the phylib approach - it can implement speeds
> in excess of 1G. DSA already supports phylink with modern updated
> drivers that do not use the "adjust_link" implementation.
>
> What I'm proposing is that we don't bring the baggage of the phylib
> based fixed link forwards into fwnode, and leave this to be DT-only.
> I think this is what Andrew and Vladimir have also said.
>

Ok, thanks for clarifying.

Best regards,
Marcin