On Wed, Apr 13, 2022 at 05:46:11PM -0400, Tom Rini wrote:
> On Thu, Apr 07, 2022 at 12:32:57AM +0200, Marek Behún wrote:
> 
> > From: Marek Behún <marek.be...@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(). Also add
> > corresponding UT test.
> > 
> > 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.be...@nic.cz>
> > Reviewed-by: Ramon Fried <rfried....@gmail.com>
> > Reviewed-by: Simon Glass <s...@chromium.org>
> > ---
> >  arch/sandbox/dts/test.dts | 13 +++++++++++++
> >  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 ++++++------------------
> >  test/dm/ofnode.c          | 18 ++++++++++++++++++
> >  7 files changed, 96 insertions(+), 18 deletions(-)
> > 
> > diff --git a/arch/sandbox/dts/test.dts b/arch/sandbox/dts/test.dts
> > index 05c1cd5e1a..e536943503 100644
> > --- a/arch/sandbox/dts/test.dts
> > +++ b/arch/sandbox/dts/test.dts
> > @@ -530,6 +530,13 @@
> >             fake-host-hwaddr = [00 00 66 44 22 22];
> >     };
> >  
> > +   phy_eth0: phy-test-eth {
> > +           compatible = "sandbox,eth";
> > +           reg = <0x10007000 0x1000>;
> > +           fake-host-hwaddr = [00 00 66 44 22 77];
> > +           phy-handle = <&ethphy1>;
> > +   };
> > +
> >     dsa_eth0: dsa-test-eth {
> >             compatible = "sandbox,eth";
> >             reg = <0x10006000 0x1000>;
> > @@ -1555,6 +1562,12 @@
> >  
> >     mdio: mdio-test {
> >             compatible = "sandbox,mdio";
> > +           #address-cells = <1>;
> > +           #size-cells = <0>;
> > +
> > +           ethphy1: ethernet-phy@1 {
> > +                   reg = <1>;
> > +           };
> >     };
> >  
> >     pm-bus-test {
> > diff --git a/drivers/core/ofnode.c b/drivers/core/ofnode.c
> > index 8042847f3c..445b7ad5ad 100644
> > --- a/drivers/core/ofnode.c
> > +++ b/drivers/core/ofnode.c
> > @@ -1198,3 +1198,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 744dffe0a2..429aee2812 100644
> > --- a/include/dm/ofnode.h
> > +++ b/include/dm/ofnode.h
> > @@ -1217,4 +1217,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;
> >  }
> > diff --git a/test/dm/ofnode.c b/test/dm/ofnode.c
> > index dab0480a42..500c63aef5 100644
> > --- a/test/dm/ofnode.c
> > +++ b/test/dm/ofnode.c
> > @@ -447,3 +447,21 @@ static int dm_test_ofnode_string_err(struct 
> > unit_test_state *uts)
> >     return 0;
> >  }
> >  DM_TEST(dm_test_ofnode_string_err, UT_TESTF_LIVE_TREE);
> > +
> > +static int dm_test_ofnode_get_phy(struct unit_test_state *uts)
> > +{
> > +   ofnode eth_node, phy_node;
> > +   u32 reg;
> > +
> > +   eth_node = ofnode_path("/phy-test-eth");
> > +   ut_assert(ofnode_valid(eth_node));
> > +
> > +   phy_node = ofnode_get_phy_node(eth_node);
> > +   ut_assert(ofnode_valid(phy_node));
> > +
> > +   reg = ofnode_read_u32_default(phy_node, "reg", -1U);
> > +   ut_asserteq_64(0x1, reg);
> > +
> > +   return 0;
> > +}
> > +DM_TEST(dm_test_ofnode_get_phy, 0);
> 
> So this patch consistently leads to:
> https://source.denx.de/u-boot/u-boot/-/jobs/423690#L222
> on current master.  I did have to manually merge a few changes, but I
> think I did that correctly.  I don't see _why_ this leads to failure.
> Simon, any ideas?

This is addressed with:
https://patchwork.ozlabs.org/project/uboot/patch/20220414153613.3015932-1-tr...@konsulko.com/

-- 
Tom

Attachment: signature.asc
Description: PGP signature

Reply via email to