[PATCH 0/3] common: Add fdt network helper

[PATCH 0/3] common: Add fdt network helper

[Tony Dinh]

At the moment, there is no common fdt helper function specific to decoding network related
information from FDTs. This new helper functional group fdt_support_net is intended to be used
by board-specific code within U-Boot for various network related chores.

In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
the PHY addess of a specific ethernet device.


Tony Dinh (3):
  Add fdt network helper header file
  Add fdt network helper functions
  Add fdt network helper to Makefile

 common/Makefile           |  2 +-
 common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
 include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
 3 files changed, 86 insertions(+), 1 deletion(-)
 create mode 100644 common/fdt_support_net.c
 create mode 100644 include/fdt_support_net.h

-- 
2.20.1

[PATCH 1/3] Add fdt network helper header file

[Tony Dinh]

Add include header file include/fdt_support_net.h

Signed-off-by: Tony Dinh <mibodhi@gmail.com>
---

 include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 include/fdt_support_net.h

diff --git a/include/fdt_support_net.h b/include/fdt_support_net.h
new file mode 100644
index 0000000000..4fe447f803
--- /dev/null
+++ b/include/fdt_support_net.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0+
+ *
+ * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
+ */
+
+#ifndef __fdt_support_net_h
+#define __fdt_support_net_h
+
+/**
+ * This file contains convenience functions for decoding network related
+ * information from FDTs. It is intended to be used by board-specific code
+ * within U-Boot.
+ */
+
+/*
+ * fdt_get_phy_addr - Return the Ethernet PHY address
+ *
+ * Convenience function to return the PHY address of an
+ * ethernet device given its full path as defined in the device tree
+ *
+ * @path	full path to the network device node
+ * @return	PHY address, or -1 if it does not exist
+ *
+ * Usage examples:
+ *
+ * Get PHY address of eth0 for a Kirkwood board as defined in kirkwood.dtsi
+ *	int phyaddr;
+ *	char *eth0_path = "/ocp@f1000000/ethernet-controller@72000";
+ *	phyaddr = fdt_get_phy_addr(eth0_path);
+ *
+ * Get PHY address of eth1 for a Armada 38x board as defined
+ * in armada-38x.dtsi
+ *	int phyaddr;
+ *	char *eth1_path = "/soc/ethernet@30000";
+ *	phyaddr = fdt_get_phy_addr(eth1_path);
+ */
+int fdt_get_phy_addr(const char *path);
+
+#endif
-- 
2.20.1

[PATCH 2/3] Add fdt network helper functions

[Tony Dinh]

Add fdt network helper functions common/fdt_support_net.c

Signed-off-by: Tony Dinh <mibodhi@gmail.com>
---

 common/fdt_support_net.c | 46 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)
 create mode 100644 common/fdt_support_net.c

diff --git a/common/fdt_support_net.c b/common/fdt_support_net.c
new file mode 100644
index 0000000000..06fa2175b4
--- /dev/null
+++ b/common/fdt_support_net.c
@@ -0,0 +1,46 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
+ */
+
+#include <common.h>
+#include <linux/libfdt.h>
+#include <fdt_support.h>
+#include <asm/global_data.h>
+#include <fdt_support_net.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int fdt_get_phy_addr(const char *path)
+{
+	const void *fdt = gd->fdt_blob;
+	const u32 *reg;
+	const u32 *val;
+	int node, phandle, addr;
+
+	/* Find the node by its full path */
+	node = fdt_path_offset(fdt, path);
+	if (node >= 0) {
+		/* Look up phy-handle */
+		val = fdt_getprop(fdt, node, "phy-handle", NULL);
+		if (!val)
+			/* Look up phy (deprecated property for phy handle) */
+			val = fdt_getprop(fdt, node, "phy", NULL);
+		if (val) {
+			phandle = fdt32_to_cpu(*val);
+			if (!phandle)
+				return -1;
+			/* Follow it to its node */
+			node = fdt_node_offset_by_phandle(fdt, phandle);
+			if (node) {
+				/* Look up reg */
+				reg = fdt_getprop(fdt, node, "reg", NULL);
+				if (reg) {
+					addr = fdt32_to_cpu(*reg);
+					return addr;
+				}
+			}
+		}
+	}
+	return -1;
+}
-- 
2.20.1

[PATCH 3/3] Add fdt network helper to Makefile

[Tony Dinh]

Add fdt_support_net.c to common/Makefile

Signed-off-by: Tony Dinh <mibodhi@gmail.com>
---

 common/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/Makefile b/common/Makefile
index 9063ed9391..94678d26d8 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
 obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
 
 obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
-obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o
+obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o fdt_support_net.o
 obj-$(CONFIG_MII) += miiphyutil.o
 obj-$(CONFIG_CMD_MII) += miiphyutil.o
 obj-$(CONFIG_PHYLIB) += miiphyutil.o
-- 
2.20.1

Re: [PATCH 2/3] Add fdt network helper functions

[Stefan Roese]

Hi Tony,

a few nits...

On 06.08.21 06:49, Tony Dinh wrote:
> Add fdt network helper functions common/fdt_support_net.c
> 
> Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> ---
> 
>   common/fdt_support_net.c | 46 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 46 insertions(+)
>   create mode 100644 common/fdt_support_net.c
> 
> diff --git a/common/fdt_support_net.c b/common/fdt_support_net.c
> new file mode 100644
> index 0000000000..06fa2175b4
> --- /dev/null
> +++ b/common/fdt_support_net.c
> @@ -0,0 +1,46 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
> + */
> +
> +#include <common.h>

Including "common.h" is deprecated. Please remove it and include the
missing other headers instead (if any).

> +#include <linux/libfdt.h>
> +#include <fdt_support.h>
> +#include <asm/global_data.h>
> +#include <fdt_support_net.h>

Sorting would be good as well.

> +
> +DECLARE_GLOBAL_DATA_PTR;
> +
> +int fdt_get_phy_addr(const char *path)
> +{
> +	const void *fdt = gd->fdt_blob;
> +	const u32 *reg;
> +	const u32 *val;
> +	int node, phandle, addr;
> +
> +	/* Find the node by its full path */
> +	node = fdt_path_offset(fdt, path);
> +	if (node >= 0) {
> +		/* Look up phy-handle */
> +		val = fdt_getprop(fdt, node, "phy-handle", NULL);
> +		if (!val)
> +			/* Look up phy (deprecated property for phy handle) */
> +			val = fdt_getprop(fdt, node, "phy", NULL);

Above is a multi-line statement which is better included in
parentheses.

And I personally would add an empty line here.

> +		if (val) {
> +			phandle = fdt32_to_cpu(*val);
> +			if (!phandle)
> +				return -1;

Could you instead of returning "-1" return some matching error code
defines?

And I personally would add an empty line here.

> +			/* Follow it to its node */
> +			node = fdt_node_offset_by_phandle(fdt, phandle);
> +			if (node) {
> +				/* Look up reg */
> +				reg = fdt_getprop(fdt, node, "reg", NULL);
> +				if (reg) {
> +					addr = fdt32_to_cpu(*reg);
> +					return addr;
> +				}
> +			}
> +		}
> +	}
> +	return -1;

Again, please change to some matching error define here.

Thanks,
Stefan

Re: [PATCH 1/3] Add fdt network helper header file

[Stefan Roese]

On 06.08.21 06:49, Tony Dinh wrote:
> Add include header file include/fdt_support_net.h
> 
> Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> ---
> 
>   include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++++++++
>   1 file changed, 39 insertions(+)
>   create mode 100644 include/fdt_support_net.h
> 
> diff --git a/include/fdt_support_net.h b/include/fdt_support_net.h
> new file mode 100644
> index 0000000000..4fe447f803
> --- /dev/null
> +++ b/include/fdt_support_net.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0+
> + *
> + * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
> + */
> +
> +#ifndef __fdt_support_net_h
> +#define __fdt_support_net_h
> +
> +/**
> + * This file contains convenience functions for decoding network related
> + * information from FDTs. It is intended to be used by board-specific code
> + * within U-Boot.
> + */
> +
> +/*
> + * fdt_get_phy_addr - Return the Ethernet PHY address
> + *
> + * Convenience function to return the PHY address of an
> + * ethernet device given its full path as defined in the device tree
> + *
> + * @path	full path to the network device node
> + * @return	PHY address, or -1 if it does not exist

Please change from "-1" to some meaningful error return code.

Other than that:

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

> + *
> + * Usage examples:
> + *
> + * Get PHY address of eth0 for a Kirkwood board as defined in kirkwood.dtsi
> + *	int phyaddr;
> + *	char *eth0_path = "/ocp@f1000000/ethernet-controller@72000";
> + *	phyaddr = fdt_get_phy_addr(eth0_path);
> + *
> + * Get PHY address of eth1 for a Armada 38x board as defined
> + * in armada-38x.dtsi
> + *	int phyaddr;
> + *	char *eth1_path = "/soc/ethernet@30000";
> + *	phyaddr = fdt_get_phy_addr(eth1_path);
> + */
> +int fdt_get_phy_addr(const char *path);
> +
> +#endif
> 


Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 3/3] Add fdt network helper to Makefile

[Stefan Roese]

On 06.08.21 06:49, Tony Dinh wrote:
> Add fdt_support_net.c to common/Makefile
> 
> Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> ---
> 
>   common/Makefile | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/Makefile b/common/Makefile
> index 9063ed9391..94678d26d8 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -28,7 +28,7 @@ obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
>   obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
>   
>   obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
> -obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o
> +obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o fdt_support_net.o
>   obj-$(CONFIG_MII) += miiphyutil.o
>   obj-$(CONFIG_CMD_MII) += miiphyutil.o
>   obj-$(CONFIG_PHYLIB) += miiphyutil.o
> 

Reviewed-by: Stefan Roese <sr@denx.de>

Thanks,
Stefan

Re: [PATCH 0/3] common: Add fdt network helper

[Stefan Roese]

Hi Ramin, Hi Joe,

On 06.08.21 06:49, Tony Dinh wrote:
> 
> At the moment, there is no common fdt helper function specific to decoding network related
> information from FDTs. This new helper functional group fdt_support_net is intended to be used
> by board-specific code within U-Boot for various network related chores.
> 
> In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> the PHY addess of a specific ethernet device.

Ramon & Joe, could you please also take a look at this patchset? And
let us know if this is okay or if you have some comments?

Thanks,
Stefan

> 
> Tony Dinh (3):
>    Add fdt network helper header file
>    Add fdt network helper functions
>    Add fdt network helper to Makefile
> 
>   common/Makefile           |  2 +-
>   common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
>   include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
>   3 files changed, 86 insertions(+), 1 deletion(-)
>   create mode 100644 common/fdt_support_net.c
>   create mode 100644 include/fdt_support_net.h
> 


Viele Grüße,
Stefan

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr@denx.de

Re: [PATCH 2/3] Add fdt network helper functions

[Tony Dinh]

Hi Stefan,

On Wed, Aug 11, 2021 at 11:15 PM Stefan Roese <sr@denx.de> wrote:
>
> Hi Tony,
>
> a few nits...
>
> On 06.08.21 06:49, Tony Dinh wrote:
> > Add fdt network helper functions common/fdt_support_net.c
> >
> > Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> > ---
> >
> >   common/fdt_support_net.c | 46 ++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 46 insertions(+)
> >   create mode 100644 common/fdt_support_net.c
> >
> > diff --git a/common/fdt_support_net.c b/common/fdt_support_net.c
> > new file mode 100644
> > index 0000000000..06fa2175b4
> > --- /dev/null
> > +++ b/common/fdt_support_net.c
> > @@ -0,0 +1,46 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
> > + */
> > +
> > +#include <common.h>
>
> Including "common.h" is deprecated. Please remove it and include the
> missing other headers instead (if any).
>
> > +#include <linux/libfdt.h>
> > +#include <fdt_support.h>
> > +#include <asm/global_data.h>
> > +#include <fdt_support_net.h>
>
> Sorting would be good as well.
>
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +
> > +int fdt_get_phy_addr(const char *path)
> > +{
> > +     const void *fdt = gd->fdt_blob;
> > +     const u32 *reg;
> > +     const u32 *val;
> > +     int node, phandle, addr;
> > +
> > +     /* Find the node by its full path */
> > +     node = fdt_path_offset(fdt, path);
> > +     if (node >= 0) {
> > +             /* Look up phy-handle */
> > +             val = fdt_getprop(fdt, node, "phy-handle", NULL);
> > +             if (!val)
> > +                     /* Look up phy (deprecated property for phy handle) */
> > +                     val = fdt_getprop(fdt, node, "phy", NULL);
>
> Above is a multi-line statement which is better included in
> parentheses.
>
> And I personally would add an empty line here.
>
> > +             if (val) {
> > +                     phandle = fdt32_to_cpu(*val);
> > +                     if (!phandle)
> > +                             return -1;
>
> Could you instead of returning "-1" return some matching error code
> defines?
>
> And I personally would add an empty line here.
>
> > +                     /* Follow it to its node */
> > +                     node = fdt_node_offset_by_phandle(fdt, phandle);
> > +                     if (node) {
> > +                             /* Look up reg */
> > +                             reg = fdt_getprop(fdt, node, "reg", NULL);
> > +                             if (reg) {
> > +                                     addr = fdt32_to_cpu(*reg);
> > +                                     return addr;
> > +                             }
> > +                     }
> > +             }
> > +     }
> > +     return -1;
>
> Again, please change to some matching error define here.
>

Will do all of the above and send in the V2 patch, after Ramon and Joe
had any feedback.

Thanks,
Tony

> Thanks,
> Stefan

Re: [PATCH 1/3] Add fdt network helper header file

[Ramon Fried]

On Fri, Aug 6, 2021 at 7:50 AM Tony Dinh <mibodhi@gmail.com> wrote:
>
> Add include header file include/fdt_support_net.h
>
> Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> ---
>
>  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 include/fdt_support_net.h
>
> diff --git a/include/fdt_support_net.h b/include/fdt_support_net.h
> new file mode 100644
> index 0000000000..4fe447f803
> --- /dev/null
> +++ b/include/fdt_support_net.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: GPL-2.0+
> + *
> + * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
> + */
> +
> +#ifndef __fdt_support_net_h
> +#define __fdt_support_net_h
> +
> +/**
> + * This file contains convenience functions for decoding network related
> + * information from FDTs. It is intended to be used by board-specific code
> + * within U-Boot.
> + */
> +
> +/*
> + * fdt_get_phy_addr - Return the Ethernet PHY address
> + *
> + * Convenience function to return the PHY address of an
> + * ethernet device given its full path as defined in the device tree
> + *
> + * @path       full path to the network device node
> + * @return     PHY address, or -1 if it does not exist
> + *
> + * Usage examples:
> + *
> + * Get PHY address of eth0 for a Kirkwood board as defined in kirkwood.dtsi
> + *     int phyaddr;
> + *     char *eth0_path = "/ocp@f1000000/ethernet-controller@72000";
> + *     phyaddr = fdt_get_phy_addr(eth0_path);
> + *
> + * Get PHY address of eth1 for a Armada 38x board as defined
> + * in armada-38x.dtsi
> + *     int phyaddr;
> + *     char *eth1_path = "/soc/ethernet@30000";
> + *     phyaddr = fdt_get_phy_addr(eth1_path);
> + */
> +int fdt_get_phy_addr(const char *path);
> +
> +#endif
> --
> 2.20.1
>
Acked-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH 2/3] Add fdt network helper functions

[Ramon Fried]

On Thu, Aug 12, 2021 at 12:12 PM Tony Dinh <mibodhi@gmail.com> wrote:
>
> Hi Stefan,
>
> On Wed, Aug 11, 2021 at 11:15 PM Stefan Roese <sr@denx.de> wrote:
> >
> > Hi Tony,
> >
> > a few nits...
> >
> > On 06.08.21 06:49, Tony Dinh wrote:
> > > Add fdt network helper functions common/fdt_support_net.c
> > >
> > > Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> > > ---
> > >
> > >   common/fdt_support_net.c | 46 ++++++++++++++++++++++++++++++++++++++++
> > >   1 file changed, 46 insertions(+)
> > >   create mode 100644 common/fdt_support_net.c
> > >
> > > diff --git a/common/fdt_support_net.c b/common/fdt_support_net.c
> > > new file mode 100644
> > > index 0000000000..06fa2175b4
> > > --- /dev/null
> > > +++ b/common/fdt_support_net.c
> > > @@ -0,0 +1,46 @@
> > > +// SPDX-License-Identifier: GPL-2.0+
> > > +/*
> > > + * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
> > > + */
> > > +
> > > +#include <common.h>
> >
> > Including "common.h" is deprecated. Please remove it and include the
> > missing other headers instead (if any).
> >
> > > +#include <linux/libfdt.h>
> > > +#include <fdt_support.h>
> > > +#include <asm/global_data.h>
> > > +#include <fdt_support_net.h>
> >
> > Sorting would be good as well.
> >
> > > +
> > > +DECLARE_GLOBAL_DATA_PTR;
> > > +
> > > +int fdt_get_phy_addr(const char *path)
> > > +{
> > > +     const void *fdt = gd->fdt_blob;
> > > +     const u32 *reg;
> > > +     const u32 *val;
> > > +     int node, phandle, addr;
> > > +
> > > +     /* Find the node by its full path */
> > > +     node = fdt_path_offset(fdt, path);
> > > +     if (node >= 0) {
> > > +             /* Look up phy-handle */
> > > +             val = fdt_getprop(fdt, node, "phy-handle", NULL);
> > > +             if (!val)
> > > +                     /* Look up phy (deprecated property for phy handle) */
> > > +                     val = fdt_getprop(fdt, node, "phy", NULL);
> >
> > Above is a multi-line statement which is better included in
> > parentheses.
> >
> > And I personally would add an empty line here.
> >
> > > +             if (val) {
> > > +                     phandle = fdt32_to_cpu(*val);
> > > +                     if (!phandle)
> > > +                             return -1;
> >
> > Could you instead of returning "-1" return some matching error code
> > defines?
> >
> > And I personally would add an empty line here.
> >
> > > +                     /* Follow it to its node */
> > > +                     node = fdt_node_offset_by_phandle(fdt, phandle);
> > > +                     if (node) {
> > > +                             /* Look up reg */
> > > +                             reg = fdt_getprop(fdt, node, "reg", NULL);
> > > +                             if (reg) {
> > > +                                     addr = fdt32_to_cpu(*reg);
> > > +                                     return addr;
> > > +                             }
> > > +                     }
> > > +             }
> > > +     }
> > > +     return -1;
> >
> > Again, please change to some matching error define here.
> >
>
> Will do all of the above and send in the V2 patch, after Ramon and Joe
> had any feedback.
>
> Thanks,
> Tony
>
> > Thanks,
> > Stefan
Acked-by: Ramon Fried <rfried.dev@gmail.com>

Re: [PATCH 3/3] Add fdt network helper to Makefile

[Ramon Fried]

On Fri, Aug 6, 2021 at 7:50 AM Tony Dinh <mibodhi@gmail.com> wrote:
>
> Add fdt_support_net.c to common/Makefile
>
> Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> ---
>
>  common/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/Makefile b/common/Makefile
> index 9063ed9391..94678d26d8 100644
> --- a/common/Makefile
> +++ b/common/Makefile
> @@ -28,7 +28,7 @@ obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
>  obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
>
>  obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
> -obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o
> +obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o fdt_support_net.o
>  obj-$(CONFIG_MII) += miiphyutil.o
>  obj-$(CONFIG_CMD_MII) += miiphyutil.o
>  obj-$(CONFIG_PHYLIB) += miiphyutil.o
> --
> 2.20.1
>
Acked-by: Ramon Fried <rfried.dev@gmail.com>

[PATCH v2 1/3] common: Add fdt network helper

[Tony Dinh]

Add fdt network helper to Makefile

Reviewed-by: Stefan Roese <sr@denx.de>
Acked-by: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Tony Dinh <mibodhi@gmail.com>
---

(no changes since v1)

 common/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/Makefile b/common/Makefile
index 9063ed9391..94678d26d8 100644
--- a/common/Makefile
+++ b/common/Makefile
@@ -28,7 +28,7 @@ obj-$(CONFIG_CMD_BOOTZ) += bootm.o bootm_os.o
 obj-$(CONFIG_CMD_BOOTI) += bootm.o bootm_os.o
 
 obj-$(CONFIG_CMD_BEDBUG) += bedbug.o
-obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o
+obj-$(CONFIG_$(SPL_TPL_)OF_LIBFDT) += fdt_support.o fdt_support_net.o
 obj-$(CONFIG_MII) += miiphyutil.o
 obj-$(CONFIG_CMD_MII) += miiphyutil.o
 obj-$(CONFIG_PHYLIB) += miiphyutil.o
-- 
2.20.1

[PATCH v2 2/3] common: Add fdt network helper header file

[Tony Dinh]

Add include header file fdt_support_net.h

Reviewed-by: Stefan Roese <sr@denx.de>
Acked-by: Ramon Fried <rfried.dev@gmail.com>
Signed-off-by: Tony Dinh <mibodhi@gmail.com>
---

Changes in v2:
- Return FDT_ERR_NOTFOUND if fdt_get_phy_addr failed to find PHY addr

 include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
 create mode 100644 include/fdt_support_net.h

diff --git a/include/fdt_support_net.h b/include/fdt_support_net.h
new file mode 100644
index 0000000000..87852ffc41
--- /dev/null
+++ b/include/fdt_support_net.h
@@ -0,0 +1,39 @@
+/* SPDX-License-Identifier: GPL-2.0+
+ *
+ * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
+ */
+
+#ifndef __fdt_support_net_h
+#define __fdt_support_net_h
+
+/**
+ * This file contains convenience functions for decoding network related
+ * information from FDTs. It is intended to be used by board-specific code
+ * within U-Boot.
+ */
+
+/*
+ * fdt_get_phy_addr - Return the Ethernet PHY address
+ *
+ * Convenience function to return the PHY address of an
+ * ethernet device given its full path as defined in the device tree
+ *
+ * @path	full path to the network device node
+ * @return	PHY address, or -FDT_ERR_NOTFOUND if it does not exist
+ *
+ * Usage examples:
+ *
+ * Get PHY address of eth0 for a Kirkwood board as defined in kirkwood.dtsi
+ *	int phyaddr;
+ *	char *eth0_path = "/ocp@f1000000/ethernet-controller@72000";
+ *	phyaddr = fdt_get_phy_addr(eth0_path);
+ *
+ * Get PHY address of eth1 for a Armada 38x board as defined
+ * in armada-38x.dtsi
+ *	int phyaddr;
+ *	char *eth1_path = "/soc/ethernet@30000";
+ *	phyaddr = fdt_get_phy_addr(eth1_path);
+ */
+int fdt_get_phy_addr(const char *path);
+
+#endif
-- 
2.20.1

[PATCH v2 3/3] common: Add fdt network helper functions

[Tony Dinh]

Add fdt network helper functions fdt_support_net.c

Reviewed-by: Stefan Roese <sr@denx.de>
Acked-by: Ramon Fried <rfried.dev@gmail.com>

Signed-off-by: Tony Dinh <mibodhi@gmail.com>
---

Changes in v2:
- Return FDT_ERR_NOTFOUND if fdt_get_phy_addr failed to find PHY addr
- Coding standards.

 common/fdt_support_net.c | 47 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)
 create mode 100644 common/fdt_support_net.c

diff --git a/common/fdt_support_net.c b/common/fdt_support_net.c
new file mode 100644
index 0000000000..46fa12dd7c
--- /dev/null
+++ b/common/fdt_support_net.c
@@ -0,0 +1,47 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Copyright (C) 2021 Tony Dinh <mibodhi@gmail.com>
+ */
+
+#include <asm/global_data.h>
+#include <fdt_support.h>
+#include <fdt_support_net.h>
+#include <linux/libfdt.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+int fdt_get_phy_addr(const char *path)
+{
+	const void *fdt = gd->fdt_blob;
+	const u32 *reg;
+	const u32 *val;
+	int node, phandle, addr;
+
+	/* Find the node by its full path */
+	node = fdt_path_offset(fdt, path);
+	if (node >= 0) {
+		/* Look up phy-handle */
+		val = fdt_getprop(fdt, node, "phy-handle", NULL);
+		if (!val) {
+			/* Look up phy (deprecated property for phy handle) */
+			val = fdt_getprop(fdt, node, "phy", NULL);
+		}
+		if (val) {
+			phandle = fdt32_to_cpu(*val);
+			if (!phandle)
+				return -FDT_ERR_NOTFOUND;
+
+			/* Follow it to its node */
+			node = fdt_node_offset_by_phandle(fdt, phandle);
+			if (node) {
+				/* Look up reg */
+				reg = fdt_getprop(fdt, node, "reg", NULL);
+				if (reg) {
+					addr = fdt32_to_cpu(*reg);
+					return addr;
+				}
+			}
+		}
+	}
+	return -FDT_ERR_NOTFOUND;
+}
-- 
2.20.1

Re: [PATCH 0/3] common: Add fdt network helper

[Simon Glass]

Hi Tony,

On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi@gmail.com> wrote:
>
>
> At the moment, there is no common fdt helper function specific to decoding network related
> information from FDTs. This new helper functional group fdt_support_net is intended to be used
> by board-specific code within U-Boot for various network related chores.
>
> In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> the PHY addess of a specific ethernet device.
>
>
> Tony Dinh (3):
>   Add fdt network helper header file
>   Add fdt network helper functions
>   Add fdt network helper to Makefile
>
>  common/Makefile           |  2 +-
>  common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
>  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
>  3 files changed, 86 insertions(+), 1 deletion(-)
>  create mode 100644 common/fdt_support_net.c
>  create mode 100644 include/fdt_support_net.h

Can this use livetre and also have some tests?

Regards,
Simon

Re: [PATCH 0/3] common: Add fdt network helper

[Tony Dinh]

Hi Simon,

On Sun, Aug 15, 2021 at 7:10 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tony,
>
> On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi@gmail.com> wrote:
> >
> >
> > At the moment, there is no common fdt helper function specific to decoding network related
> > information from FDTs. This new helper functional group fdt_support_net is intended to be used
> > by board-specific code within U-Boot for various network related chores.
> >
> > In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> > the PHY addess of a specific ethernet device.
> >
> >
> > Tony Dinh (3):
> >   Add fdt network helper header file
> >   Add fdt network helper functions
> >   Add fdt network helper to Makefile
> >
> >  common/Makefile           |  2 +-
> >  common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
> >  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
> >  3 files changed, 86 insertions(+), 1 deletion(-)
> >  create mode 100644 common/fdt_support_net.c
> >  create mode 100644 include/fdt_support_net.h
>
> Can this use livetre and also have some tests?

I have not enabled livetree for any of the boards I have. So I just
modeled this using the existing ./common/fdt_support.c!

I do agree we should start using livetree in fdt helpers, if I
understood it correctly, it should work for both flattree and
livetree. Perhaps we could have another patch series after this? I am
preparing another Kirkwood board support patch that I could hold off
submitting and enable livetree to use that as a vehicle for testing.

Thanks,
Tony

> Regards,
> Simon

Re: [PATCH 0/3] common: Add fdt network helper

[Tony Dinh]

Hi Simon,

On Sun, Aug 15, 2021 at 2:27 PM Tony Dinh <mibodhi@gmail.com> wrote:
>
> Hi Simon,
>
> On Sun, Aug 15, 2021 at 7:10 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tony,
> >
> > On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi@gmail.com> wrote:
> > >
> > >
> > > At the moment, there is no common fdt helper function specific to decoding network related
> > > information from FDTs. This new helper functional group fdt_support_net is intended to be used
> > > by board-specific code within U-Boot for various network related chores.
> > >
> > > In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> > > the PHY addess of a specific ethernet device.
> > >
> > >
> > > Tony Dinh (3):
> > >   Add fdt network helper header file
> > >   Add fdt network helper functions
> > >   Add fdt network helper to Makefile
> > >
> > >  common/Makefile           |  2 +-
> > >  common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
> > >  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
> > >  3 files changed, 86 insertions(+), 1 deletion(-)
> > >  create mode 100644 common/fdt_support_net.c
> > >  create mode 100644 include/fdt_support_net.h
> >
> > Can this use livetre and also have some tests?
>
> I have not enabled livetree for any of the boards I have. So I just
> modeled this using the existing ./common/fdt_support.c!
>
> I do agree we should start using livetree in fdt helpers, if I
> understood it correctly, it should work for both flattree and
> livetree. Perhaps we could have another patch series after this? I am
> preparing another Kirkwood board support patch that I could hold off
> submitting and enable livetree to use that as a vehicle for testing.
>
> Thanks,
> Tony
>
> > Regards,
> > Simon

So is it OK for this patch to be accepted as is? It'll take a while
for me to switch to the livetree approach and do testing.

Thanks,
Tony

Re: [PATCH 0/3] common: Add fdt network helper

[Simon Glass]

Hi Tony,

On Sun, 15 Aug 2021 at 15:28, Tony Dinh <mibodhi@gmail.com> wrote:
>
> Hi Simon,
>
> On Sun, Aug 15, 2021 at 7:10 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tony,
> >
> > On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi@gmail.com> wrote:
> > >
> > >
> > > At the moment, there is no common fdt helper function specific to decoding network related
> > > information from FDTs. This new helper functional group fdt_support_net is intended to be used
> > > by board-specific code within U-Boot for various network related chores.
> > >
> > > In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> > > the PHY addess of a specific ethernet device.
> > >
> > >
> > > Tony Dinh (3):
> > >   Add fdt network helper header file
> > >   Add fdt network helper functions
> > >   Add fdt network helper to Makefile
> > >
> > >  common/Makefile           |  2 +-
> > >  common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
> > >  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
> > >  3 files changed, 86 insertions(+), 1 deletion(-)
> > >  create mode 100644 common/fdt_support_net.c
> > >  create mode 100644 include/fdt_support_net.h
> >
> > Can this use livetre and also have some tests?
>
> I have not enabled livetree for any of the boards I have. So I just
> modeled this using the existing ./common/fdt_support.c!
>
> I do agree we should start using livetree in fdt helpers, if I
> understood it correctly, it should work for both flattree and

OK good, yes that's right.

> livetree. Perhaps we could have another patch series after this? I am
> preparing another Kirkwood board support patch that I could hold off
> submitting and enable livetree to use that as a vehicle for testing.

I think it is better to use livetree in this patch. For testing, you
can use sandbox for testing (see for example test/dm/ofnode.c)

Regards,
Simon

Re: [PATCH 0/3] common: Add fdt network helper

[Tony Dinh]

Hi Simon,

On Tue, Aug 17, 2021 at 9:09 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tony,
>
> On Sun, 15 Aug 2021 at 15:28, Tony Dinh <mibodhi@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Tony,
> > >
> > > On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi@gmail.com> wrote:
> > > >
> > > >
> > > > At the moment, there is no common fdt helper function specific to decoding network related
> > > > information from FDTs. This new helper functional group fdt_support_net is intended to be used
> > > > by board-specific code within U-Boot for various network related chores.
> > > >
> > > > In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> > > > the PHY addess of a specific ethernet device.
> > > >
> > > >
> > > > Tony Dinh (3):
> > > >   Add fdt network helper header file
> > > >   Add fdt network helper functions
> > > >   Add fdt network helper to Makefile
> > > >
> > > >  common/Makefile           |  2 +-
> > > >  common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
> > > >  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
> > > >  3 files changed, 86 insertions(+), 1 deletion(-)
> > > >  create mode 100644 common/fdt_support_net.c
> > > >  create mode 100644 include/fdt_support_net.h
> > >
> > > Can this use livetre and also have some tests?
> >
> > I have not enabled livetree for any of the boards I have. So I just
> > modeled this using the existing ./common/fdt_support.c!
> >
> > I do agree we should start using livetree in fdt helpers, if I
> > understood it correctly, it should work for both flattree and
>
> OK good, yes that's right.
>
> > livetree. Perhaps we could have another patch series after this? I am
> > preparing another Kirkwood board support patch that I could hold off
> > submitting and enable livetree to use that as a vehicle for testing.
>
> I think it is better to use livetree in this patch. For testing, you
> can use sandbox for testing (see for example test/dm/ofnode.c)
>
> Regards,
> Simon

It seems it is too time consuming to implement this using livetree
calls (with my limited understanding about livetree). I spent a few
hours reading ./include/dm/read.h and ./include/dm/ofnode.h, and it is
not apparent to me which functions to use. I see that we have
eth_get_dev_by_name(), that's a start.

Do you have any objection if I submit this function as a patch to
./common/fdt_support.c? fdt_support.c file is all flatree
implementation. And by the way, this new function fdt_get_phy_addr()
has been tested with several Kirkwood boards that I have been
converting to DM Ethernet.

When the time comes that it's mandatory to convert all to livetree
calls, I'll be glad to help.

Thanks,
Tony

Re: [PATCH 0/3] common: Add fdt network helper

[Tony Dinh]

Hey Simon,

On Thu, Aug 26, 2021 at 9:00 PM Tony Dinh <mibodhi@gmail.com> wrote:
>
> Hi Simon,
>
> On Tue, Aug 17, 2021 at 9:09 AM Simon Glass <sjg@chromium.org> wrote:
> >
> > Hi Tony,
> >
> > On Sun, 15 Aug 2021 at 15:28, Tony Dinh <mibodhi@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Tony,
> > > >
> > > > On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi@gmail.com> wrote:
> > > > >
> > > > >
> > > > > At the moment, there is no common fdt helper function specific to decoding network related
> > > > > information from FDTs. This new helper functional group fdt_support_net is intended to be used
> > > > > by board-specific code within U-Boot for various network related chores.
> > > > >
> > > > > In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> > > > > the PHY addess of a specific ethernet device.
> > > > >
> > > > >
> > > > > Tony Dinh (3):
> > > > >   Add fdt network helper header file
> > > > >   Add fdt network helper functions
> > > > >   Add fdt network helper to Makefile
> > > > >
> > > > >  common/Makefile           |  2 +-
> > > > >  common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
> > > > >  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
> > > > >  3 files changed, 86 insertions(+), 1 deletion(-)
> > > > >  create mode 100644 common/fdt_support_net.c
> > > > >  create mode 100644 include/fdt_support_net.h
> > > >
> > > > Can this use livetre and also have some tests?
> > >
> > > I have not enabled livetree for any of the boards I have. So I just
> > > modeled this using the existing ./common/fdt_support.c!
> > >
> > > I do agree we should start using livetree in fdt helpers, if I
> > > understood it correctly, it should work for both flattree and
> >
> > OK good, yes that's right.
> >
> > > livetree. Perhaps we could have another patch series after this? I am
> > > preparing another Kirkwood board support patch that I could hold off
> > > submitting and enable livetree to use that as a vehicle for testing.
> >
> > I think it is better to use livetree in this patch. For testing, you
> > can use sandbox for testing (see for example test/dm/ofnode.c)
> >
> > Regards,
> > Simon
>
> It seems it is too time consuming to implement this using livetree
> calls (with my limited understanding about livetree). I spent a few
> hours reading ./include/dm/read.h and ./include/dm/ofnode.h, and it is
> not apparent to me which functions to use. I see that we have
> eth_get_dev_by_name(), that's a start.
>
> Do you have any objection if I submit this function as a patch to
> ./common/fdt_support.c? fdt_support.c file is all flatree
> implementation. And by the way, this new function fdt_get_phy_addr()
> has been tested with several Kirkwood boards that I have been
> converting to DM Ethernet.
>
> When the time comes that it's mandatory to convert all to livetree
> calls, I'll be glad to help.

I know you're as busy as always ;) But I would appreciate either an
ACK or NACK on my proposal to submit this as a patch to
./common/fdt_support.c.

Thanks,
Tony

Re: [PATCH 0/3] common: Add fdt network helper

[Simon Glass]

Hi Tony,

On Wed, 1 Sept 2021 at 03:22, Tony Dinh <mibodhi@gmail.com> wrote:
>
> Hey Simon,
>
> On Thu, Aug 26, 2021 at 9:00 PM Tony Dinh <mibodhi@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Aug 17, 2021 at 9:09 AM Simon Glass <sjg@chromium.org> wrote:
> > >
> > > Hi Tony,
> > >
> > > On Sun, 15 Aug 2021 at 15:28, Tony Dinh <mibodhi@gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass <sjg@chromium.org> wrote:
> > > > >
> > > > > Hi Tony,
> > > > >
> > > > > On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi@gmail.com> wrote:
> > > > > >
> > > > > >
> > > > > > At the moment, there is no common fdt helper function specific to decoding network related
> > > > > > information from FDTs. This new helper functional group fdt_support_net is intended to be used
> > > > > > by board-specific code within U-Boot for various network related chores.
> > > > > >
> > > > > > In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> > > > > > the PHY addess of a specific ethernet device.
> > > > > >
> > > > > >
> > > > > > Tony Dinh (3):
> > > > > >   Add fdt network helper header file
> > > > > >   Add fdt network helper functions
> > > > > >   Add fdt network helper to Makefile
> > > > > >
> > > > > >  common/Makefile           |  2 +-
> > > > > >  common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
> > > > > >  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
> > > > > >  3 files changed, 86 insertions(+), 1 deletion(-)
> > > > > >  create mode 100644 common/fdt_support_net.c
> > > > > >  create mode 100644 include/fdt_support_net.h
> > > > >
> > > > > Can this use livetre and also have some tests?
> > > >
> > > > I have not enabled livetree for any of the boards I have. So I just
> > > > modeled this using the existing ./common/fdt_support.c!
> > > >
> > > > I do agree we should start using livetree in fdt helpers, if I
> > > > understood it correctly, it should work for both flattree and
> > >
> > > OK good, yes that's right.
> > >
> > > > livetree. Perhaps we could have another patch series after this? I am
> > > > preparing another Kirkwood board support patch that I could hold off
> > > > submitting and enable livetree to use that as a vehicle for testing.
> > >
> > > I think it is better to use livetree in this patch. For testing, you
> > > can use sandbox for testing (see for example test/dm/ofnode.c)
> > >
> > > Regards,
> > > Simon
> >
> > It seems it is too time consuming to implement this using livetree
> > calls (with my limited understanding about livetree). I spent a few
> > hours reading ./include/dm/read.h and ./include/dm/ofnode.h, and it is
> > not apparent to me which functions to use. I see that we have
> > eth_get_dev_by_name(), that's a start.
> >
> > Do you have any objection if I submit this function as a patch to
> > ./common/fdt_support.c? fdt_support.c file is all flatree
> > implementation. And by the way, this new function fdt_get_phy_addr()
> > has been tested with several Kirkwood boards that I have been
> > converting to DM Ethernet.
> >
> > When the time comes that it's mandatory to convert all to livetree
> > calls, I'll be glad to help.
>
> I know you're as busy as always ;) But I would appreciate either an
> ACK or NACK on my proposal to submit this as a patch to
> ./common/fdt_support.c.

We should not be adding new functions that use flattree. If you are
unsure which functions to use, please ask. But some examples:

int ofnode_read_u32(ofnode node, const char *propname, u32 *outp);

u32 ofnode_read_u32_default(ofnode ref, const char *propname, u32 def);

const void *ofnode_read_prop(ofnode node, const char *propname, int *sizep);

const char *ofnode_read_string(ofnode node, const char *propname);

It is easy enough to convert an offset into an ofnode and vice versa.

For adding tests, see test/dm/ofnode.c for examples.

https://u-boot.readthedocs.io/en/latest/develop/tests_sandbox.html

To run a particular test, a quick way is:

make sandbox_defconfig
make
./u-boot -T -c "ut dm ofnode"
./u-boot -T -c "ut dm dm_test_ofnode_get_by_phandle"

This bash function will run tests which match a particular string:

function pyt {
test/py/test.py -B sandbox --build-dir /tmp/b/sandbox ${1:+"-k $1"}
}

e.g to run all ofnode tests:

$ pyt ofnode
=============================== test session starts
================================
platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0
rootdir: /home/sjg/c/src/third_party/u-boot/files/test/py, configfile:
pytest.ini
plugins: forked-1.3.0, hypothesis-5.43.3, xdist-2.2.0
collected 843 items / 828 deselected / 15 selected

test/py/tests/test_ut.py ...............
      [100%]

======================== 15 passed, 828 deselected in 0.67s
========================


Regards,
Simon

Re: [PATCH 0/3] common: Add fdt network helper

[Tony Dinh]

Hi Simon,

On Thu, Sep 2, 2021 at 9:41 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Tony,
>
> On Wed, 1 Sept 2021 at 03:22, Tony Dinh <mibodhi@gmail.com> wrote:
> >
> > Hey Simon,
> >
> > On Thu, Aug 26, 2021 at 9:00 PM Tony Dinh <mibodhi@gmail.com> wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Aug 17, 2021 at 9:09 AM Simon Glass <sjg@chromium.org> wrote:
> > > >
> > > > Hi Tony,
> > > >
> > > > On Sun, 15 Aug 2021 at 15:28, Tony Dinh <mibodhi@gmail.com> wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass <sjg@chromium.org> wrote:
> > > > > >
> > > > > > Hi Tony,
> > > > > >
> > > > > > On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi@gmail.com> wrote:
> > > > > > >
> > > > > > >
> > > > > > > At the moment, there is no common fdt helper function specific to decoding network related
> > > > > > > information from FDTs. This new helper functional group fdt_support_net is intended to be used
> > > > > > > by board-specific code within U-Boot for various network related chores.
> > > > > > >
> > > > > > > In this patch, create the 1st function fdt_get_phy_addr to parse the device tree to find
> > > > > > > the PHY addess of a specific ethernet device.
> > > > > > >
> > > > > > >
> > > > > > > Tony Dinh (3):
> > > > > > >   Add fdt network helper header file
> > > > > > >   Add fdt network helper functions
> > > > > > >   Add fdt network helper to Makefile
> > > > > > >
> > > > > > >  common/Makefile           |  2 +-
> > > > > > >  common/fdt_support_net.c  | 46 +++++++++++++++++++++++++++++++++++++++
> > > > > > >  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++
> > > > > > >  3 files changed, 86 insertions(+), 1 deletion(-)
> > > > > > >  create mode 100644 common/fdt_support_net.c
> > > > > > >  create mode 100644 include/fdt_support_net.h
> > > > > >
> > > > > > Can this use livetre and also have some tests?
> > > > >
> > > > > I have not enabled livetree for any of the boards I have. So I just
> > > > > modeled this using the existing ./common/fdt_support.c!
> > > > >
> > > > > I do agree we should start using livetree in fdt helpers, if I
> > > > > understood it correctly, it should work for both flattree and
> > > >
> > > > OK good, yes that's right.
> > > >
> > > > > livetree. Perhaps we could have another patch series after this? I am
> > > > > preparing another Kirkwood board support patch that I could hold off
> > > > > submitting and enable livetree to use that as a vehicle for testing.
> > > >
> > > > I think it is better to use livetree in this patch. For testing, you
> > > > can use sandbox for testing (see for example test/dm/ofnode.c)
> > > >
> > > > Regards,
> > > > Simon
> > >
> > > It seems it is too time consuming to implement this using livetree
> > > calls (with my limited understanding about livetree). I spent a few
> > > hours reading ./include/dm/read.h and ./include/dm/ofnode.h, and it is
> > > not apparent to me which functions to use. I see that we have
> > > eth_get_dev_by_name(), that's a start.
> > >
> > > Do you have any objection if I submit this function as a patch to
> > > ./common/fdt_support.c? fdt_support.c file is all flatree
> > > implementation. And by the way, this new function fdt_get_phy_addr()
> > > has been tested with several Kirkwood boards that I have been
> > > converting to DM Ethernet.
> > >
> > > When the time comes that it's mandatory to convert all to livetree
> > > calls, I'll be glad to help.
> >
> > I know you're as busy as always ;) But I would appreciate either an
> > ACK or NACK on my proposal to submit this as a patch to
> > ./common/fdt_support.c.
>
> We should not be adding new functions that use flattree. If you are
> unsure which functions to use, please ask. But some examples:

Really appreciate this quick howto. I'll document this in the header
prologue when I can get back to work on this patch.

Thanks,
Tony

>
> int ofnode_read_u32(ofnode node, const char *propname, u32 *outp);
>
> u32 ofnode_read_u32_default(ofnode ref, const char *propname, u32 def);
>
> const void *ofnode_read_prop(ofnode node, const char *propname, int *sizep);
>
> const char *ofnode_read_string(ofnode node, const char *propname);
>
> It is easy enough to convert an offset into an ofnode and vice versa.
>
> For adding tests, see test/dm/ofnode.c for examples.
>
> https://u-boot.readthedocs.io/en/latest/develop/tests_sandbox.html
>
> To run a particular test, a quick way is:
>
> make sandbox_defconfig
> make
> ./u-boot -T -c "ut dm ofnode"
> ./u-boot -T -c "ut dm dm_test_ofnode_get_by_phandle"
>
> This bash function will run tests which match a particular string:
>
> function pyt {
> test/py/test.py -B sandbox --build-dir /tmp/b/sandbox ${1:+"-k $1"}
> }
>
> e.g to run all ofnode tests:
>
> $ pyt ofnode
> =============================== test session starts
> ================================
> platform linux -- Python 3.9.2, pytest-6.0.2, py-1.10.0, pluggy-0.13.0
> rootdir: /home/sjg/c/src/third_party/u-boot/files/test/py, configfile:
> pytest.ini
> plugins: forked-1.3.0, hypothesis-5.43.3, xdist-2.2.0
> collected 843 items / 828 deselected / 15 selected
>
> test/py/tests/test_ut.py ...............
>       [100%]
>
> ======================== 15 passed, 828 deselected in 0.67s
> ========================
>
>
> Regards,
> Simon

Re: [PATCH 1/3] Add fdt network helper header file

[Simon Glass]

Hi Tony,

On Thu, 5 Aug 2021 at 22:50, Tony Dinh <mibodhi@gmail.com> wrote:
>
> Add include header file include/fdt_support_net.h
>
> Signed-off-by: Tony Dinh <mibodhi@gmail.com>
> ---
>
>  include/fdt_support_net.h | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>  create mode 100644 include/fdt_support_net.h

Sorry for the delay but I am only now coming to this after being away
for a month and finding it in my queue.

Can you please
- move this to drivers/core/of_extra.c and include/dm/ofnode.h
- use the livetree interface (ofnode) instead of fdt
- merge everything into a single patch since the first patch creates a
build error

Regards,
Simon