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