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

[Simon Glass]

Hi Tony,

On Wed, 1 Sept 2021 at 03:22, Tony Dinh <mibodhi at gmail.com> wrote:
>
> Hey Simon,
>
> On Thu, Aug 26, 2021 at 9:00 PM Tony Dinh <mibodhi at gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Tue, Aug 17, 2021 at 9:09 AM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Tony,
> > >
> > > On Sun, 15 Aug 2021 at 15:28, Tony Dinh <mibodhi at gmail.com> wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass <sjg at chromium.org> wrote:
> > > > >
> > > > > Hi Tony,
> > > > >
> > > > > On Thu, 5 Aug 2021 at 22:49, Tony Dinh <mibodhi at 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