devel Digest, Vol 110, Issue 38
Niteesh G. S.
niteesh.gs at gmail.com
Fri Jan 29 03:57:10 UTC 2021
Hello,
I have answered your questions below, but I myself have few questions.
1) This patch is based on the fact that libBSD OFW API is hardwired to the
FDT implementation
if in future this changes this patch will create problems. Can this happen?
2) We should also remove ofw_fdt.c which I have missed in this patch.
ofw_fdt.c:
https://github.com/RTEMS/rtems-libbsd/blob/master/freebsd/sys/dev/ofw/ofw_fdt.c
It is only used to provide the FDT implementation for OFW API. Since we use
the RTEMS one
this is no longer needed. But this also does some registering stuff which I
quite don't understand.
https://github.com/RTEMS/rtems-libbsd/blob/33bfaee89aa71d2252eb48d6b9a9ec17183faced/freebsd/sys/dev/ofw/ofw_fdt.c#L126
Will removing these cause some issues?
Just to summarize the goal is to make libBSD use the RTEMS OFW FDT
implementation but here we do
something different. I think the right approach should be to remove
ofw_fdt.c or modify ofw_fdt.c to use
the RTEMS implementation, this will also fix the problem mentioned in
question 1.
But then can't we just leave the libBSD as it is and use the RTEMS
implementation for RTEMS
alone when not used with libBSD? Won't this be a good approach?
On Thu, Jan 28, 2021 at 11:10 AM Chris Johns <chrisj at rtems.org> wrote:
> On 28/1/21 4:25 am, Niteesh G. S. wrote:
> > Date: Fri, 22 Jan 2021 04:28:37 +1100
> > From: Chris Johns <chrisj at rtems.org <mailto:chrisj at rtems.org>>
> > To: devel at rtems.org <mailto:devel at rtems.org>
> > Subject: Re: [patch libBSD] dev/ofw: Use RTEMS OFW FDT implementation
> > Message-ID: <d2cca4d0-f1fc-b93f-c30e-0c95ce4b410b at rtems.org
> > <mailto:d2cca4d0-f1fc-b93f-c30e-0c95ce4b410b at rtems.org>>
> > Content-Type: text/plain; charset=utf-8
> >
> > On 20/1/21 7:31 pm, G S Niteesh Babu wrote:
> > > This commit modifies the OFW interface to the RTEMS FDT
> > > implementation instead of the default FreeBSD.
> >
> > Which branch?
> >
> > Did you mean which commit? If not can you please rephrase your question?
> > RTEMS OFW API implemented in this commit:
> >
> https://git.rtems.org/rtems/commit/?id=9d2ed41fcb1dc635ce7d689c60cdf374f2394dbd
> > <
> https://git.rtems.org/rtems/commit/?id=9d2ed41fcb1dc635ce7d689c60cdf374f2394dbd
> >
>
> This is for the rtems.git repo and the patch being reviewed is for the
> rtems-libbsd.git repo. We have 2 active branches in that repo. Which
> branch is
> this patch for?
>
The master branch, at least that what I built and tested this patch on.
BTW what is the difference between and why do we need those two branches?
>
> > Is this patch dependent on the changes to rtems.git?
> >
> >
> > Yes. This patch depends on the previously committed patch
> > https://lists.rtems.org/pipermail/devel/2020-December/063644.html
> > <https://lists.rtems.org/pipermail/devel/2020-December/063644.html>
>
> OK.
>
> > Are the kernel versions ofthe functions built for all bsps?
> >
> > Which BSPs have been built to test the changes?
> >
> >
> > I tested the changes with Beagle BSP and xilinx_zynq_a9_qemu.
>
> Great and thanks however libbsd builds for powerpc etc and this is not
> covered
> by that testing. I have not looked into the rtems.git patch to know if it
> is
> common to all bsps.
Yes, it is common to all BSPS at least as far as I understand.
The open firmware(OFW) provides many ways through which the software can
learn about
the hardware. Device Trees is one of the ways. The OFW API provides a
common API for all these
different methods. In RTEMS, we have implemented the OFW API for device
trees so I think
it should be common for all BSPs using device trees.
And in libBSD, the OFW API is hardwired to use the FDT implementation only.
So we currently
don't have to worry about other implementations.
>
>
> My concnern is these calls are currently available to all BSPs in
> libbsd and may
> > not be in rtems.git.
> >
> >
> > If I understand the question correctly, my previous answer should have
> answered
> > this.
>
> Hmm, well I must have missed it or I did not understand because I have
> asked again.
>
> > But once again, An RTEMS OFW API is already available in the master
> branch. This
> > patch removes the functions which have been implemented in RTEMS. We
> provide
> > FreeBSD compatibility using ofw_compat.h.
> > https://git.rtems.org/rtems/tree/bsps/include/ofw/ofw_compat.h
> > <https://git.rtems.org/rtems/tree/bsps/include/ofw/ofw_compat.h>
>
> Yeap I understand this ...
>
> >
> > > ---
> > > freebsd/sys/dev/ofw/openfirm.c | 2 ++
> > > freebsd/sys/dev/ofw/openfirm.h | 9 ++++++++
> >
> > Why do we need to keep these files in libbsd?
> >
> >
> > Some functions cannot be implemented in RTEMS since they depend
> > on the FreeBSD driver infrastructure. So I have left them here. All
> functions
> > which are independent of the FreeBSD stuff are moved into RTEMS from
> > FreeBSD.
>
> For this change to proceed I need you to:
>
> 1. Post here the name of a BSP, if any exist, that does not build the
> rtems.git
> version of the OFW functions. If all BSPs build these functions please say
> so.
>
I think all BSPs build with the patch committed in rtems.git. I can say so
because
we tried a BSP that supports FDT(Beagle) and a BSP that
doesn't(Xilinx_zynq_a9_qemu), it builds in both
cases so I guess it should be the case for all other BSPs.
Christian, please correct me if I am wrong.
> 2. A report from you that the BSP listed in 1, if there is one, builds
> _and_
> libbsd for that BSP also builds with the patch we are reviewing.
>
This patch along with the one in RTEMS has been tested on Beagleboneblack
BSP
and xilinx_zynq_a9_qemu. I haven't tried building other BSPs though. Does
this
suffice? I'll test this with PowerPC as I get some free time.
>
> > > .../machine/rtems-bsd-kernel-namespace.h | 22
> -------------------
> > > 3 files changed, 11 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/freebsd/sys/dev/ofw/openfirm.c
> b/freebsd/sys/dev/ofw/openfirm.c
> > > index 9cc7dbdc..30513ab2 100644
> > > --- a/freebsd/sys/dev/ofw/openfirm.c
> > > +++ b/freebsd/sys/dev/ofw/openfirm.c
> > > @@ -333,6 +333,7 @@ OF_interpret(const char *cmd, int nreturns,
> ...)
> > > * Device tree functions
> > > */
> > >
> > > +#ifndef __rtems__
> > > /* Return the next sibling of this node or 0. */
> > > phandle_t
> > > OF_peer(phandle_t node)
> > > @@ -672,6 +673,7 @@ OF_xref_from_node(phandle_t node)
> > > return (node);
> > > return (xref);
> > > }
> > > +#endif /* __rtems__ */
> > >
> > > device_t
> > > OF_device_from_xref(phandle_t xref)
> > > diff --git a/freebsd/sys/dev/ofw/openfirm.h
> b/freebsd/sys/dev/ofw/openfirm.h
> > > index f043197a..5df07258 100644
> > > --- a/freebsd/sys/dev/ofw/openfirm.h
> > > +++ b/freebsd/sys/dev/ofw/openfirm.h
> > > @@ -64,7 +64,11 @@
> > >
> > > #include <sys/types.h>
> > > #include <machine/_bus.h>
> > > +#ifdef __rtems__
> > > +#include <ofw/ofw_compat.h>
> > > +#endif /* __rtems__ */
> > >
> > > +#ifndef __rtems__
> > > /*
> > > * Prototypes for Open Firmware Interface Routines
> > > */
> > > @@ -72,6 +76,7 @@
> > > typedef uint32_t ihandle_t;
> > > typedef uint32_t phandle_t;
> > > typedef uint32_t pcell_t;
> > > +#endif /* __rtems__ */
> > >
> > > #ifdef _KERNEL
> > > #include <sys/malloc.h>
> > > @@ -102,6 +107,7 @@ int OF_test(const char *name);
> > > void OF_printf(const char *fmt, ...);
> > >
> > > /* Device tree functions */
> > > +#ifndef __rtems__
> > > phandle_t OF_peer(phandle_t node);
> > > phandle_t OF_child(phandle_t node);
> > > phandle_t OF_parent(phandle_t node);
> > > @@ -140,6 +146,7 @@ ssize_t OF_package_to_path(phandle_t
> node,
> > char *buf, size_t len);
> > > */
> > > phandle_t OF_node_from_xref(phandle_t xref);
> > > phandle_t OF_xref_from_node(phandle_t node);
> > > +#endif /* __rtems__ */
> > >
> > > /*
> > > * When properties contain references to other nodes using xref
> handles it is
> > > @@ -159,8 +166,10 @@ ssize_t OF_read(ihandle_t instance,
> void
> > *buf, size_t len);
> > > ssize_t OF_write(ihandle_t instance, const void
> *buf, size_t
> > len);
> > > int OF_seek(ihandle_t instance, uint64_t where);
> > >
> > > +#ifndef __rtems__
> > > phandle_t OF_instance_to_package(ihandle_t instance);
> > > ssize_t OF_instance_to_path(ihandle_t instance, char
> *buf,
> > size_t len);
> > > +#endif /* __rtems__ */
> > > int OF_call_method(const char *method, ihandle_t
> instance,
> > > int nargs, int nreturns, ...);
> > >
> > > diff --git a/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> > b/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> > > index 75b744a4..53944393 100644
> > > --- a/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> > > +++ b/rtemsbsd/include/machine/rtems-bsd-kernel-namespace.h
> > > @@ -3044,42 +3044,20 @@
> > > #define null_filtops _bsd_null_filtops
> > > #define nullop _bsd_nullop
> > > #define OF_call_method _bsd_OF_call_method
> > > -#define OF_canon _bsd_OF_canon
> > > -#define OF_child _bsd_OF_child
> > > #define OF_claim _bsd_OF_claim
> > > #define OF_close _bsd_OF_close
> > > #define OF_device_from_xref _bsd_OF_device_from_xref
> > > #define OF_device_register_xref _bsd_OF_device_register_xref
> > > #define OF_enter _bsd_OF_enter
> > > #define OF_exit _bsd_OF_exit
> > > -#define OF_finddevice _bsd_OF_finddevice
> > > -#define OF_getencprop _bsd_OF_getencprop
> > > -#define OF_getencprop_alloc _bsd_OF_getencprop_alloc
> > > -#define OF_getencprop_alloc_multi
> _bsd_OF_getencprop_alloc_multi
> > > -#define OF_getprop _bsd_OF_getprop
> > > -#define OF_getprop_alloc _bsd_OF_getprop_alloc
> > > -#define OF_getprop_alloc_multi _bsd_OF_getprop_alloc_multi
> > > -#define OF_getproplen _bsd_OF_getproplen
> > > -#define OF_hasprop _bsd_OF_hasprop
> > > #define OF_init _bsd_OF_init
> > > #define OF_install _bsd_OF_install
> > > -#define OF_instance_to_package _bsd_OF_instance_to_package
> > > -#define OF_instance_to_path _bsd_OF_instance_to_path
> > > #define OF_interpret _bsd_OF_interpret
> > > -#define OF_nextprop _bsd_OF_nextprop
> > > -#define OF_node_from_xref _bsd_OF_node_from_xref
> > > #define OF_open _bsd_OF_open
> > > -#define OF_package_to_path _bsd_OF_package_to_path
> > > -#define OF_parent _bsd_OF_parent
> > > -#define OF_peer _bsd_OF_peer
> > > #define OF_printf _bsd_OF_printf
> > > -#define OF_prop_free _bsd_OF_prop_free
> > > #define OF_read _bsd_OF_read
> > > #define OF_release _bsd_OF_release
> > > -#define OF_searchencprop _bsd_OF_searchencprop
> > > -#define OF_searchprop _bsd_OF_searchprop
> > > #define OF_seek _bsd_OF_seek
> > > -#define OF_setprop _bsd_OF_setprop
> > > #define OF_test _bsd_OF_test
> > > #define ofw_bus_assigned_addresses_to_rl
> > _bsd_ofw_bus_assigned_addresses_to_rl
> > > #define ofwbus_driver _bsd_ofwbus_driver
> >
> > How did you create this changes?
> >
> > In RTEMS, there is ofw_compat.h
> > https://git.rtems.org/rtems/tree/bsps/include/ofw/ofw_compat.h
> > <https://git.rtems.org/rtems/tree/bsps/include/ofw/ofw_compat.h>
> > to provide compatibility between FreeBSD OFW API and RTEMS OFW API.
>
> Huh? The segment of the patch I commented under is for
> rtems-bsd-kernel-namespace.h. I am specifically asking how that fragment
> of the
> posted patch was created.
>
I manually edited the file. I removed the defines for all functions that
have been
defined in ofw_compat.h.
Does this answer your question? or have I again misunderstood your question?
Thanks,
Niteesh.
> Chris
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20210129/52867a04/attachment-0001.html>
More information about the devel
mailing list