diff mbox series

[u-boot-net,03/14] net: introduce helpers to get PHY ofnode from MAC

Message ID 20220317124958.27921-4-kabel@kernel.org
State Superseded
Delegated to: Ramon Fried
Headers show
Series u-boot-net refactors, fixes, cleanups | expand

Commit Message

Marek Behún March 17, 2022, 12:49 p.m. UTC
From: Marek Behún <marek.behun@nic.cz>

Add helpers ofnode_get_phy_node() and dev_get_phy_node() and use it in
net/mdio-uclass.c function dm_eth_connect_phy_handle().

This is useful because other part's of U-Boot may want to get PHY ofnode
without connecting a PHY.

Signed-off-by: Marek Behún <marek.behun@nic.cz>
---
 drivers/core/ofnode.c | 21 +++++++++++++++++++++
 drivers/core/read.c   |  5 +++++
 include/dm/ofnode.h   | 14 ++++++++++++++
 include/dm/read.h     | 19 +++++++++++++++++++
 net/mdio-uclass.c     | 24 ++++++------------------
 5 files changed, 65 insertions(+), 18 deletions(-)

Comments

Ramon Fried March 18, 2022, 8:02 a.m. UTC | #1
On Thu, Mar 17, 2022 at 2:50 PM Marek Behún <kabel@kernel.org> wrote:
>
> From: Marek Behún <marek.behun@nic.cz>
>
> Add helpers ofnode_get_phy_node() and dev_get_phy_node() and use it in
> net/mdio-uclass.c function dm_eth_connect_phy_handle().
>
> This is useful because other part's of U-Boot may want to get PHY ofnode
> without connecting a PHY.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>  drivers/core/ofnode.c | 21 +++++++++++++++++++++
>  drivers/core/read.c   |  5 +++++
>  include/dm/ofnode.h   | 14 ++++++++++++++
>  include/dm/read.h     | 19 +++++++++++++++++++
>  net/mdio-uclass.c     | 24 ++++++------------------
>  5 files changed, 65 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> index 709bea272a..eaad2c989b 100644
> --- a/drivers/core/ofnode.c
> +++ b/drivers/core/ofnode.c
> @@ -1162,3 +1162,24 @@ const char *ofnode_conf_read_str(const char *prop_name)
>
>         return ofnode_read_string(node, prop_name);
>  }
> +
> +ofnode ofnode_get_phy_node(ofnode node)
> +{
> +       /* DT node properties that reference a PHY node */
> +       static const char * const phy_handle_str[] = {
> +               "phy-handle", "phy", "phy-device",
> +       };
> +       struct ofnode_phandle_args args = {
> +               .node = ofnode_null()
> +       };
> +       int i;
> +
> +       assert(ofnode_valid(node));
> +
> +       for (i = 0; i < ARRAY_SIZE(phy_handle_str); i++)
> +               if (!ofnode_parse_phandle_with_args(node, phy_handle_str[i],
> +                                                   NULL, 0, 0, &args))
> +                       break;
> +
> +       return args.node;
> +}
> diff --git a/drivers/core/read.c b/drivers/core/read.c
> index 31f9e78a06..7ff100218d 100644
> --- a/drivers/core/read.c
> +++ b/drivers/core/read.c
> @@ -398,3 +398,8 @@ int dev_decode_display_timing(const struct udevice *dev, int index,
>  {
>         return ofnode_decode_display_timing(dev_ofnode(dev), index, config);
>  }
> +
> +ofnode dev_get_phy_node(const struct udevice *dev)
> +{
> +       return ofnode_get_phy_node(dev_ofnode(dev));
> +}
> diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
> index 0cb324c8b0..8164386043 100644
> --- a/include/dm/ofnode.h
> +++ b/include/dm/ofnode.h
> @@ -1204,4 +1204,18 @@ int ofnode_conf_read_int(const char *prop_name, int default_val);
>   */
>  const char *ofnode_conf_read_str(const char *prop_name);
>
> +/**
> + * ofnode_get_phy_node() - Get PHY node for a MAC (if not fixed-link)
> + *
> + * This function parses PHY handle from the Ethernet controller's ofnode
> + * (trying all possible PHY handle property names), and returns the PHY ofnode.
> + *
> + * Before this is used, ofnode_phy_is_fixed_link() should be checked first, and
> + * if the result to that is true, this function should not be called.
> + *
> + * @eth_node:  ofnode belonging to the Ethernet controller
> + * Return: ofnode of the PHY, if it exists, otherwise an invalid ofnode
> + */
> +ofnode ofnode_get_phy_node(ofnode eth_node);
> +
>  #endif
> diff --git a/include/dm/read.h b/include/dm/read.h
> index 233af3c063..899eb813fd 100644
> --- a/include/dm/read.h
> +++ b/include/dm/read.h
> @@ -743,6 +743,20 @@ int dev_read_pci_bus_range(const struct udevice *dev, struct resource *res);
>  int dev_decode_display_timing(const struct udevice *dev, int index,
>                               struct display_timing *config);
>
> +/**
> + * dev_get_phy_node() - Get PHY node for a MAC (if not fixed-link)
> + *
> + * This function parses PHY handle from the Ethernet controller's ofnode
> + * (trying all possible PHY handle property names), and returns the PHY ofnode.
> + *
> + * Before this is used, ofnode_phy_is_fixed_link() should be checked first, and
> + * if the result to that is true, this function should not be called.
> + *
> + * @dev: device representing the MAC
> + * Return: ofnode of the PHY, if it exists, otherwise an invalid ofnode
> + */
> +ofnode dev_get_phy_node(const struct udevice *dev);
> +
>  #else /* CONFIG_DM_DEV_READ_INLINE is enabled */
>  #include <asm/global_data.h>
>
> @@ -1092,6 +1106,11 @@ static inline int dev_decode_display_timing(const struct udevice *dev,
>         return ofnode_decode_display_timing(dev_ofnode(dev), index, config);
>  }
>
> +static inline ofnode dev_get_phy_node(const struct udevice *dev)
> +{
> +       return ofnode_get_phy_node(dev_ofnode(dev));
> +}
> +
>  #endif /* CONFIG_DM_DEV_READ_INLINE */
>
>  /**
> diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
> index 649dc60f73..233b70171b 100644
> --- a/net/mdio-uclass.c
> +++ b/net/mdio-uclass.c
> @@ -20,11 +20,6 @@ static const char * const phy_mode_str[] = {
>         "phy-mode", "phy-connection-type"
>  };
>
> -/* DT node properties that reference a PHY node */
> -static const char * const phy_handle_str[] = {
> -       "phy-handle", "phy", "phy-device"
> -};
> -
>  void dm_mdio_probe_devices(void)
>  {
>         struct udevice *it;
> @@ -137,23 +132,16 @@ static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
>         u32 phy_addr;
>         struct udevice *mdiodev;
>         struct phy_device *phy;
> -       struct ofnode_phandle_args phandle = {.node = ofnode_null()};
>         ofnode phynode;
> -       int i;
>
>         if (CONFIG_IS_ENABLED(PHY_FIXED) &&
>             ofnode_phy_is_fixed_link(dev_ofnode(ethdev), &phynode)) {
>                 phy = phy_connect(NULL, 0, ethdev, interface);
> -               phandle.node = phynode;
>                 goto out;
>         }
>
> -       for (i = 0; i < ARRAY_SIZE(phy_handle_str); i++)
> -               if (!dev_read_phandle_with_args(ethdev, phy_handle_str[i], NULL,
> -                                               0, 0, &phandle))
> -                       break;
> -
> -       if (!ofnode_valid(phandle.node)) {
> +       phynode = dev_get_phy_node(ethdev);
> +       if (!ofnode_valid(phynode)) {
>                 dev_dbg(ethdev, "can't find PHY node\n");
>                 return NULL;
>         }
> @@ -162,16 +150,16 @@ static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
>          * reading 'reg' directly should be fine.  This is a PHY node, the
>          * address is always size 1 and requires no translation
>          */
> -       if (ofnode_read_u32(phandle.node, "reg", &phy_addr)) {
> +       if (ofnode_read_u32(phynode, "reg", &phy_addr)) {
>                 dev_dbg(ethdev, "missing reg property in phy node\n");
>                 return NULL;
>         }
>
>         if (uclass_get_device_by_ofnode(UCLASS_MDIO,
> -                                       ofnode_get_parent(phandle.node),
> +                                       ofnode_get_parent(phynode),
>                                         &mdiodev)) {
>                 dev_dbg(ethdev, "can't find MDIO bus for node %s\n",
> -                       ofnode_get_name(ofnode_get_parent(phandle.node)));
> +                       ofnode_get_name(ofnode_get_parent(phynode)));
>                 return NULL;
>         }
>
> @@ -179,7 +167,7 @@ static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
>
>  out:
>         if (phy)
> -               phy->node = phandle.node;
> +               phy->node = phynode;
>
>         return phy;
>  }
> --
> 2.34.1
>
Reviewed-by: Ramon Fried <rfried.dev@gmail.com>
Simon Glass March 28, 2022, 6:35 a.m. UTC | #2
Hi Marek,

On Thu, 17 Mar 2022 at 06:50, Marek Behún <kabel@kernel.org> wrote:
>
> From: Marek Behún <marek.behun@nic.cz>
>
> Add helpers ofnode_get_phy_node() and dev_get_phy_node() and use it in
> net/mdio-uclass.c function dm_eth_connect_phy_handle().
>
> This is useful because other part's of U-Boot may want to get PHY ofnode
> without connecting a PHY.
>
> Signed-off-by: Marek Behún <marek.behun@nic.cz>
> ---
>  drivers/core/ofnode.c | 21 +++++++++++++++++++++
>  drivers/core/read.c   |  5 +++++
>  include/dm/ofnode.h   | 14 ++++++++++++++
>  include/dm/read.h     | 19 +++++++++++++++++++
>  net/mdio-uclass.c     | 24 ++++++------------------
>  5 files changed, 65 insertions(+), 18 deletions(-)

Please add a test for your new function. With that:

Reviewed-by: Simon Glass <sjg@chromium.org>
Marek Behún March 28, 2022, 9:51 p.m. UTC | #3
On Mon, 28 Mar 2022 00:35:00 -0600
Simon Glass <sjg@chromium.org> wrote:

> Hi Marek,
> 
> On Thu, 17 Mar 2022 at 06:50, Marek Behún <kabel@kernel.org> wrote:
> >
> > From: Marek Behún <marek.behun@nic.cz>
> >
> > Add helpers ofnode_get_phy_node() and dev_get_phy_node() and use it in
> > net/mdio-uclass.c function dm_eth_connect_phy_handle().
> >
> > This is useful because other part's of U-Boot may want to get PHY ofnode
> > without connecting a PHY.
> >
> > Signed-off-by: Marek Behún <marek.behun@nic.cz>
> > ---
> >  drivers/core/ofnode.c | 21 +++++++++++++++++++++
> >  drivers/core/read.c   |  5 +++++
> >  include/dm/ofnode.h   | 14 ++++++++++++++
> >  include/dm/read.h     | 19 +++++++++++++++++++
> >  net/mdio-uclass.c     | 24 ++++++------------------
> >  5 files changed, 65 insertions(+), 18 deletions(-)  
> 
> Please add a test for your new function. With that:
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>

Dear Simon,

OK, I added test for ofnode_get_phy_node(). Should I also add one for
dev_get_phy_node() ? That one is implemented in drivers/core/read.c or
include/dm/read.h, and it just calls ofnode_get_phy_node()...

Also adding test for ofnode_get_phy_mode() (node -> mode) which is
implemented in patch 07/14. Can I add your reviewde-by for that patch
also? That patch is rather big, touches many files treewide...

Marek
diff mbox series

Patch

diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
index 709bea272a..eaad2c989b 100644
--- a/drivers/core/ofnode.c
+++ b/drivers/core/ofnode.c
@@ -1162,3 +1162,24 @@  const char *ofnode_conf_read_str(const char *prop_name)
 
 	return ofnode_read_string(node, prop_name);
 }
+
+ofnode ofnode_get_phy_node(ofnode node)
+{
+	/* DT node properties that reference a PHY node */
+	static const char * const phy_handle_str[] = {
+		"phy-handle", "phy", "phy-device",
+	};
+	struct ofnode_phandle_args args = {
+		.node = ofnode_null()
+	};
+	int i;
+
+	assert(ofnode_valid(node));
+
+	for (i = 0; i < ARRAY_SIZE(phy_handle_str); i++)
+		if (!ofnode_parse_phandle_with_args(node, phy_handle_str[i],
+						    NULL, 0, 0, &args))
+			break;
+
+	return args.node;
+}
diff --git a/drivers/core/read.c b/drivers/core/read.c
index 31f9e78a06..7ff100218d 100644
--- a/drivers/core/read.c
+++ b/drivers/core/read.c
@@ -398,3 +398,8 @@  int dev_decode_display_timing(const struct udevice *dev, int index,
 {
 	return ofnode_decode_display_timing(dev_ofnode(dev), index, config);
 }
+
+ofnode dev_get_phy_node(const struct udevice *dev)
+{
+	return ofnode_get_phy_node(dev_ofnode(dev));
+}
diff --git a/include/dm/ofnode.h b/include/dm/ofnode.h
index 0cb324c8b0..8164386043 100644
--- a/include/dm/ofnode.h
+++ b/include/dm/ofnode.h
@@ -1204,4 +1204,18 @@  int ofnode_conf_read_int(const char *prop_name, int default_val);
  */
 const char *ofnode_conf_read_str(const char *prop_name);
 
+/**
+ * ofnode_get_phy_node() - Get PHY node for a MAC (if not fixed-link)
+ *
+ * This function parses PHY handle from the Ethernet controller's ofnode
+ * (trying all possible PHY handle property names), and returns the PHY ofnode.
+ *
+ * Before this is used, ofnode_phy_is_fixed_link() should be checked first, and
+ * if the result to that is true, this function should not be called.
+ *
+ * @eth_node:	ofnode belonging to the Ethernet controller
+ * Return: ofnode of the PHY, if it exists, otherwise an invalid ofnode
+ */
+ofnode ofnode_get_phy_node(ofnode eth_node);
+
 #endif
diff --git a/include/dm/read.h b/include/dm/read.h
index 233af3c063..899eb813fd 100644
--- a/include/dm/read.h
+++ b/include/dm/read.h
@@ -743,6 +743,20 @@  int dev_read_pci_bus_range(const struct udevice *dev, struct resource *res);
 int dev_decode_display_timing(const struct udevice *dev, int index,
 			      struct display_timing *config);
 
+/**
+ * dev_get_phy_node() - Get PHY node for a MAC (if not fixed-link)
+ *
+ * This function parses PHY handle from the Ethernet controller's ofnode
+ * (trying all possible PHY handle property names), and returns the PHY ofnode.
+ *
+ * Before this is used, ofnode_phy_is_fixed_link() should be checked first, and
+ * if the result to that is true, this function should not be called.
+ *
+ * @dev: device representing the MAC
+ * Return: ofnode of the PHY, if it exists, otherwise an invalid ofnode
+ */
+ofnode dev_get_phy_node(const struct udevice *dev);
+
 #else /* CONFIG_DM_DEV_READ_INLINE is enabled */
 #include <asm/global_data.h>
 
@@ -1092,6 +1106,11 @@  static inline int dev_decode_display_timing(const struct udevice *dev,
 	return ofnode_decode_display_timing(dev_ofnode(dev), index, config);
 }
 
+static inline ofnode dev_get_phy_node(const struct udevice *dev)
+{
+	return ofnode_get_phy_node(dev_ofnode(dev));
+}
+
 #endif /* CONFIG_DM_DEV_READ_INLINE */
 
 /**
diff --git a/net/mdio-uclass.c b/net/mdio-uclass.c
index 649dc60f73..233b70171b 100644
--- a/net/mdio-uclass.c
+++ b/net/mdio-uclass.c
@@ -20,11 +20,6 @@  static const char * const phy_mode_str[] = {
 	"phy-mode", "phy-connection-type"
 };
 
-/* DT node properties that reference a PHY node */
-static const char * const phy_handle_str[] = {
-	"phy-handle", "phy", "phy-device"
-};
-
 void dm_mdio_probe_devices(void)
 {
 	struct udevice *it;
@@ -137,23 +132,16 @@  static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
 	u32 phy_addr;
 	struct udevice *mdiodev;
 	struct phy_device *phy;
-	struct ofnode_phandle_args phandle = {.node = ofnode_null()};
 	ofnode phynode;
-	int i;
 
 	if (CONFIG_IS_ENABLED(PHY_FIXED) &&
 	    ofnode_phy_is_fixed_link(dev_ofnode(ethdev), &phynode)) {
 		phy = phy_connect(NULL, 0, ethdev, interface);
-		phandle.node = phynode;
 		goto out;
 	}
 
-	for (i = 0; i < ARRAY_SIZE(phy_handle_str); i++)
-		if (!dev_read_phandle_with_args(ethdev, phy_handle_str[i], NULL,
-						0, 0, &phandle))
-			break;
-
-	if (!ofnode_valid(phandle.node)) {
+	phynode = dev_get_phy_node(ethdev);
+	if (!ofnode_valid(phynode)) {
 		dev_dbg(ethdev, "can't find PHY node\n");
 		return NULL;
 	}
@@ -162,16 +150,16 @@  static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
 	 * reading 'reg' directly should be fine.  This is a PHY node, the
 	 * address is always size 1 and requires no translation
 	 */
-	if (ofnode_read_u32(phandle.node, "reg", &phy_addr)) {
+	if (ofnode_read_u32(phynode, "reg", &phy_addr)) {
 		dev_dbg(ethdev, "missing reg property in phy node\n");
 		return NULL;
 	}
 
 	if (uclass_get_device_by_ofnode(UCLASS_MDIO,
-					ofnode_get_parent(phandle.node),
+					ofnode_get_parent(phynode),
 					&mdiodev)) {
 		dev_dbg(ethdev, "can't find MDIO bus for node %s\n",
-			ofnode_get_name(ofnode_get_parent(phandle.node)));
+			ofnode_get_name(ofnode_get_parent(phynode)));
 		return NULL;
 	}
 
@@ -179,7 +167,7 @@  static struct phy_device *dm_eth_connect_phy_handle(struct udevice *ethdev,
 
 out:
 	if (phy)
-		phy->node = phandle.node;
+		phy->node = phynode;
 
 	return phy;
 }