Re: [PATCH 0/3] common: Add fdt network helper
Hi Simon,
On Thu, Sep 2, 2021 at 9:41 AM Simon Glass wrote:
>
> Hi Tony,
>
> On Wed, 1 Sept 2021 at 03:22, Tony Dinh wrote:
> >
> > Hey Simon,
> >
> > On Thu, Aug 26, 2021 at 9:00 PM Tony Dinh wrote:
> > >
> > > Hi Simon,
> > >
> > > On Tue, Aug 17, 2021 at 9:09 AM Simon Glass wrote:
> > > >
> > > > Hi Tony,
> > > >
> > > > On Sun, 15 Aug 2021 at 15:28, Tony Dinh wrote:
> > > > >
> > > > > Hi Simon,
> > > > >
> > > > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass wrote:
> > > > > >
> > > > > > Hi Tony,
> > > > > >
> > > > > > On Thu, 5 Aug 2021 at 22: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.
> > > > > > >
> > > > > > >
> > > > > > > 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
>
Re: [PATCH 0/3] common: Add fdt network helper
Hi Tony,
On Wed, 1 Sept 2021 at 03:22, Tony Dinh wrote:
>
> Hey Simon,
>
> On Thu, Aug 26, 2021 at 9:00 PM Tony Dinh wrote:
> >
> > Hi Simon,
> >
> > On Tue, Aug 17, 2021 at 9:09 AM Simon Glass wrote:
> > >
> > > Hi Tony,
> > >
> > > On Sun, 15 Aug 2021 at 15:28, Tony Dinh wrote:
> > > >
> > > > Hi Simon,
> > > >
> > > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass wrote:
> > > > >
> > > > > Hi Tony,
> > > > >
> > > > > On Thu, 5 Aug 2021 at 22: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.
> > > > > >
> > > > > >
> > > > > > 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
Hey Simon, On Thu, Aug 26, 2021 at 9:00 PM Tony Dinh wrote: > > Hi Simon, > > On Tue, Aug 17, 2021 at 9:09 AM Simon Glass wrote: > > > > Hi Tony, > > > > On Sun, 15 Aug 2021 at 15:28, Tony Dinh wrote: > > > > > > Hi Simon, > > > > > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass wrote: > > > > > > > > Hi Tony, > > > > > > > > On Thu, 5 Aug 2021 at 22: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. > > > > > > > > > > > > > > > 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
Hi Simon, On Tue, Aug 17, 2021 at 9:09 AM Simon Glass wrote: > > Hi Tony, > > On Sun, 15 Aug 2021 at 15:28, Tony Dinh wrote: > > > > Hi Simon, > > > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass wrote: > > > > > > Hi Tony, > > > > > > On Thu, 5 Aug 2021 at 22: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. > > > > > > > > > > > > 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
Hi Tony, On Sun, 15 Aug 2021 at 15:28, Tony Dinh wrote: > > Hi Simon, > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass wrote: > > > > Hi Tony, > > > > On Thu, 5 Aug 2021 at 22: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. > > > > > > > > > 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
Hi Simon, On Sun, Aug 15, 2021 at 2:27 PM Tony Dinh wrote: > > Hi Simon, > > On Sun, Aug 15, 2021 at 7:10 AM Simon Glass wrote: > > > > Hi Tony, > > > > On Thu, 5 Aug 2021 at 22: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. > > > > > > > > > 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
Hi Simon, On Sun, Aug 15, 2021 at 7:10 AM Simon Glass wrote: > > Hi Tony, > > On Thu, 5 Aug 2021 at 22: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. > > > > > > 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
Hi Tony, On Thu, 5 Aug 2021 at 22: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. > > > 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
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: s...@denx.de
