<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-size:small"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, May 27, 2020 at 11:25 PM Christian Mauderer <<a href="mailto:oss@c-mauderer.de">oss@c-mauderer.de</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 27/05/2020 19:32, Niteesh G. S. wrote:<br>
> <br>
> <br>
> On Wed, May 27, 2020 at 12:04 AM Christian Mauderer <<a href="mailto:oss@c-mauderer.de" target="_blank">oss@c-mauderer.de</a><br>
> <mailto:<a href="mailto:oss@c-mauderer.de" target="_blank">oss@c-mauderer.de</a>>> wrote:<br>
> <br>
> Hello Niteesh,<br>
> <br>
> On 26/05/2020 19:56, Christian Mauderer wrote:<br>
> > Hello Niteesh,<br>
> ><br>
> > On 25/05/2020 11:20, Niteesh G. S. wrote:<br>
> >> Hello,<br>
> >><br>
> >> I have completed the porting of the OFW code from FreeBSD to RTEMS.<br>
> >> I do acknowledge the fact that we haven't decided on the<br>
> directory for files<br>
> >> to be placed in. The previous conversation had stopped quite a<br>
> while ago.<br>
> >> Christian suggested I work on the patch since that would also<br>
> start the<br>
> >> conversation again and also refactoring the patch to the correct<br>
> directory<br>
> >> will not be much of work.<br>
> >><br>
> >> cpukit/libfreebsd was suggested as one of the directories to<br>
> place the<br>
> >> ported<br>
> >> FreeBSD files. It is suggested to place all the source files<br>
> under this<br>
> >> directory.<br>
> >> Since this will make the sync part easier. But this causes issues<br>
> when<br>
> >> porting<br>
> >> BSP dependent files. Since RTEMS currently doesn't allow files in<br>
> cpukit to<br>
> >> reference bsp headers. I faced a similar issue during my porting<br>
> process<br>
> >> also.<br>
> >> The OFW init function require bsp_fdt_get function to get a<br>
> reference to<br>
> >> the fdt<br>
> >> blob. But I can't include the bsp/fdt.h header file from a source<br>
> file<br>
> >> under cpukit.<br>
> >> We must decide the directory quickly because my project will<br>
> import other<br>
> >> FreeBSD sources like the pinmux driver for beaglebone into RTEMS.<br>
> ><br>
> > Do you have a suggestion for another directory?<br>
> ><br>
> > If cpukit makes problems (which it clearly does due to the BSP<br>
> > dependencies) maybe something in bsps/shared?<br>
> ><br>
> >><br>
> >> <a href="https://github.com/gs-niteesh/rtems/commits/ofw_branch" rel="noreferrer" target="_blank">https://github.com/gs-niteesh/rtems/commits/ofw_branch</a><br>
> ><br>
> > For small patches it would be better to send them to the list<br>
> using "git<br>
> > send-email". That allows to comment directly on the patches. But<br>
> in this<br>
> > case using a repo is OK because especially the import is quite big.<br>
> ><br>
> > I'll add comments for small stuff directly on github. I hope that<br>
> works ;-)<br>
> <br>
> Finished adding comments for small stuff. Some bigger questions:<br>
> <br>
> - How do you plan to include ofw_if.h in some driver files? RTEMS<br>
> currently puts every file that can be included with <some.h> into a path<br>
> called "include".<br>
> <br>
> <br>
> Will ofw_if.h be included in driver files? As far as I have searched in the<br>
> FreeBSD sources, other than the OFW implementation none of the driver<br>
> files include this header. So this could be placed in the same directory as<br>
> the OFW sources and need not be added to the global include path.<br>
> If there is a case where it is included in some driver please let me know. <br>
> <br>
> And also all the references to this header in the OFW implementations are<br>
> relative so in future, if we add other implementations, having it in the<br>
> same<br>
> directory would be the right choice.<br>
> <br>
<br>
You are right. I didn't have a detailed enough look. The functions like<br>
OFW_GETPROP (declared in ofw_if.h) are only used in openfirm.c. So that<br>
location is OK.<br>
<br>
> <br>
> <br>
> - You created quite some commits. I would suggest to merge all porting<br>
> commits so that you have one import commit, one port commit and maybe<br>
> one commit adding RTEMS specific files<br>
> <br>
> <br>
> I have squashed them into a single port commit.<br>
> <br>
> <br>
> <br>
> <br>
> - You have at least one completely hand written file (ofw_if.h). Maybe<br>
> we should think about whether that is located well in the same directory<br>
> as the imported code or whether a similar structure like used in libbsd<br>
> would be better. One tree for imported files and one for RTEMS specific<br>
> hand written ones.<br>
> <br>
> <br>
> Do we need a separate directory for RTEMS specific handwritten ones depends<br>
> on what we will be importing in the future? Since we intend only to<br>
> import mostly<br>
> driver related code I don't think there is a need for a separate<br>
> directory. I think<br>
> this way because I assuming we don't need any RTEMS specific file for<br>
> the drivers.<br>
> But if we plan to include some simple subsystem then having a separate<br>
> directory<br>
> will be nice.<br>
<br>
Things like that tend to grow. So even if we don't want to import much<br>
for now, it's quite possible that it will be more in the future.<br>
Therefore I would use the future-proof approach even if it is a bit<br>
overkill for now.<br></blockquote><div><div class="gmail_default" style="font-size:small">OK.</div><br></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">
> <br>
> This also makes me think about if having all sources under a single<br>
> directory under<br>
> cpukit a good idea. Since our main import targets are drivers placing<br>
> them in the<br>
> cpukit directory or any other directory other than their respective BSP<br>
> directory is not<br>
> a good idea. This would make writing the sync script harder but this<br>
> will decrease the<br>
> coupling and dependencies between various folders. The other way around<br>
> will be<br>
> to have all headers accessible in the global path i.e. files under<br>
> cpukit should be<br>
> able to access bsp related headers. This would increase the coupling but<br>
> I guess<br>
> it will make writing the script easier since it has to track only a<br>
> single directory.<br>
> I am not really sure which is a better tradeoff. If someone could shed<br>
> some light on<br>
> the complexity of the script it'll help in coming up with a conclusion<br>
> easier.<br>
> But from a logical perspective, Having the drivers in their respective<br>
> directories<br>
> seems to be a better idea to me. <br>
<br>
That really depends. I agree that we only want to import low-level drivers.<br>
<br>
But also note that it's not always easy to say that driver A belongs<br>
exactly to BSP B. For example take a look at some PowerPC chips and some<br>
ARM chips. PowerPC has been used a lot by Freescale. Freescale now<br>
belongs to NXP. NXP started to use the Freescale peripherals but with an<br>
ARM core. Therefore now there are quite some modules that use the same<br>
driver in ARM chips and in Freescale chips. If you import that driver to<br>
an PowerPC you maybe would have to move it later to some shared section<br>
if you want to use it in an ARM BSP.</blockquote><div> </div><div><div class="gmail_default" style="font-size:small">I didn't know this I thought we only share across the same architectures. So I</div><div class="gmail_default" style="font-size:small">didn't take this into consideration.</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">
So why not just put all the drivers imported from FreeBSD in a shared<br>
section right from the start. Keep the directory structure but move it<br>
to bsps. I think I suggested it earlier already: What about<br>
bsps/shared/freebsd or bsps/shared/bsdports?<br></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">We place the shared drivers in the shared section that is okay. But what</div><div class="gmail_default" style="font-size:small">about drivers which are unique something like the Beagle I2C or something</div><div class="gmail_default" style="font-size:small">similar will they be in their respective folders?</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">Why shouldn't we create a separate directory for the ports? Something on</div><div class="gmail_default" style="font-size:small">a top-level. Which can reference headers from both cpukit and bsps.</div><div class="gmail_default" style="font-size:small">This way we can have all imported source files under a single directory and</div><div class="gmail_default" style="font-size:small">can share them across bsps. Is there any disadvantage in using a new directory</div><div class="gmail_default" style="font-size:small">for this?</div><br></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">
Best regards<br>
<br>
Christian<br>
<br>
> <br>
> <br>
> Please see all comments as advises. Feel free to argue against any of<br>
> them if you think different ;-)<br>
> <br>
> Best regards<br>
> <br>
> Christian<br>
> <br>
> ><br>
> > Best regards<br>
> ><br>
> > Christian<br>
> ><br>
> >> Please have a look at the last 6 patches for the ported work.<br>
> >> Below is a short summary of the patch.<br>
> >> * The openfirm.h is the OF interface that will provided to the user.<br>
> >> * The openfirm.c contains the function definition for openfirm.h. The<br>
> >> functions<br>
> >> call the respective OFW_* functions which are responsible for<br>
> choosing<br>
> >> the correct implementation for OF interface.<br>
> >> * ofw_if.h is the replacement for ofw_if.h in FreeBSD. This is an<br>
> auto<br>
> >> generated<br>
> >> header in FreeBSD which choose the correct OF implementation(ofw_fdt,<br>
> >> ofw_std,<br>
> >> ofw_32bit, ofw_real). In RTEMS we directly map to the FDT<br>
> implementation as<br>
> >> suggested by Sebastian.<br>
> >> * ofw_fdt.c contains the fdt implementation of OF interface.<br>
> >><br>
> >> Thanks,<br>
> >> Niteesh.<br>
> >><br>
> > _______________________________________________<br>
> > devel mailing list<br>
> > <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>
> > <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
> ><br>
> <br>
</blockquote></div></div>