<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 12:04 AM Christian Mauderer <<a href="mailto:oss@c-mauderer.de" target="_blank">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">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 directory for files<br>
>> to be placed in. The previous conversation had stopped quite a while ago.<br>
>> Christian suggested I work on the patch since that would also start the<br>
>> conversation again and also refactoring the patch to the correct directory<br>
>> will not be much of work.<br>
>><br>
>> cpukit/libfreebsd was suggested as one of the directories to place the<br>
>> ported<br>
>> FreeBSD files. It is suggested to place all the source files under this<br>
>> directory.<br>
>> Since this will make the sync part easier. But this causes issues when<br>
>> porting<br>
>> BSP dependent files. Since RTEMS currently doesn't allow files in cpukit to<br>
>> reference bsp headers. I faced a similar issue during my porting process<br>
>> also.<br>
>> The OFW init function require bsp_fdt_get function to get a reference to<br>
>> the fdt<br>
>> blob. But I can't include the bsp/fdt.h header file from a source file<br>
>> under cpukit.<br>
>> We must decide the directory quickly because my project will 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 using "git<br>
> send-email". That allows to comment directly on the patches. But 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 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".</blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Will ofw_if.h be included in driver files? As far as I have searched in the</div><div class="gmail_default" style="font-size:small">FreeBSD sources, other than the OFW implementation none of the driver</div><div class="gmail_default" style="font-size:small">files include this header. So this could be placed in the same directory as</div><div class="gmail_default" style="font-size:small">the OFW sources and need not be added to the global include path.</div><div class="gmail_default" style="font-size:small">If there is a case where it is included in some driver please let me know. </div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">And also all the references to this header in the OFW implementations are</div><div class="gmail_default" style="font-size:small">relative so in future, if we add other implementations, having it in the same</div><div class="gmail_default" style="font-size:small">directory would be the right choice.</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">
- 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</blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">I have squashed them into a single port commit.</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"> </blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
- 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></blockquote><div><br></div><div><div class="gmail_default" style="font-size:small">Do we need a separate directory for RTEMS specific handwritten ones depends</div><div class="gmail_default" style="font-size:small">on what we will be importing in the future? Since we intend only to import mostly</div><div class="gmail_default" style="font-size:small">driver related code I don't think there is a need for a separate directory. I think</div></div><div class="gmail_default" style="font-size:small">this way because I assuming we don't need any RTEMS specific file for the drivers.</div><div class="gmail_default" style="font-size:small">But if we plan to include some simple subsystem then having a separate directory</div><div class="gmail_default" style="font-size:small">will be nice.</div><div class="gmail_default" style="font-size:small"><br></div><div class="gmail_default" style="font-size:small">This also makes me think about if having all sources under a single directory under</div><div class="gmail_default" style="font-size:small">cpukit a good idea. Since our main import targets are drivers placing them in the</div><div class="gmail_default" style="font-size:small">cpukit directory or any other directory other than their respective BSP directory is not</div><div class="gmail_default" style="font-size:small">a good idea. This would make writing the sync script harder but this will decrease the</div><div class="gmail_default" style="font-size:small">coupling and dependencies between various folders. The other way around will be</div><div class="gmail_default" style="font-size:small">to have all headers accessible in the global path i.e. files under cpukit should be</div><div class="gmail_default" style="font-size:small">able to access bsp related headers. This would increase the coupling but I guess</div><div class="gmail_default" style="font-size:small">it will make writing the script easier since it has to track only a single directory.</div><div class="gmail_default" style="font-size:small">I am not really sure which is a better tradeoff. If someone could shed some light on</div><div class="gmail_default" style="font-size:small">the complexity of the script it'll help in coming up with a conclusion easier.</div><div class="gmail_default" style="font-size:small">But from a logical perspective, Having the drivers in their respective directories</div><div class="gmail_default" style="font-size:small">seems to be a better idea to me. </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">
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 choosing<br>
>> the correct implementation for OF interface.<br>
>> * ofw_if.h is the replacement for ofw_if.h in FreeBSD. This is an 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 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><br>
> <a href="http://lists.rtems.org/mailman/listinfo/devel" rel="noreferrer" target="_blank">http://lists.rtems.org/mailman/listinfo/devel</a><br>
> <br>
</blockquote></div></div>