<div dir="ltr"><div>Hi Padmarao,</div><div>Please see my inline comments below</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Sep 14, 2022 at 12:51 AM <<a href="mailto:Padmarao.Begari@microchip.com">Padmarao.Begari@microchip.com</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">Hi Alan,<br>
<br>
> On Tue, 2022-09-13 at 23:57 -0400, Alan Cudmore wrote: <br>
> <br>
> Hi Padmarao,<br>
> I am working on a RISC-V bsp variant for the Kendryte K210 and I am<br>
> also using an included DTB. For my k210 bsp, I changed<br>
> riscv/shared/start/start.S to set the address of the DTB array before<br>
> calling bsp_fdt_copy. If we change start.S to handle the following<br>
> three cases, we would not have to change the bsp_fdt_copy function.<br>
> The three cases are:<br>
> - no FDT, I assume griscv bsp<br>
> - uboot - a0 in contains the address of the u-boot loaded DTB<br>
> - polarfire, k210 and others - set a0 to the address of the included<br>
> DTB array<br>
> I think it's better to keep this function generic and not have to<br>
> conditionally ignore the input parameter.<br>
> <br>
<br>
Yes, Initially I thought same but don't want to include conditional<br>
statements in the startup code so moved to bsp_fdt_copy().<br>
<br>
Can we try like below in the startup code? so that it can support all<br>
three cases.<br>
<br>
#ifdef BSP_DTB_IS_SUPPORTED<br>
#include BSP_DTB_HEADER_PATH<br>
#endif<br></blockquote><div><br></div><div>For including the DTB header, I have a file similar to the microblaze_fpga bsp:</div><div><a href="https://git.rtems.org/rtems/tree/bsps/microblaze/microblaze_fpga/fdt/bsp_fdt.c">https://git.rtems.org/rtems/tree/bsps/microblaze/microblaze_fpga/fdt/bsp_fdt.c</a><br></div><div>Perhaps after your changes are in the tree we can consider promoting this feature and option to bsps/shared for riscv, microblaze and future bsps that need a DTB.</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>
#ifdef BSP_START_COPY_FDT_FROM_U_BOOT<br>
#ifdef BSP_DTB_IS_SUPPORT<br>
LADDR a0, system_dtb<br>
#else<br>
mv a0, a1<br>
#endif<br>
call bsp_fdt_copy<br>
#endif<br>
<br></blockquote><div>I added your dtb options to my build, and tried this code in start.S. It works for me.</div><div>Once your polarfire BSP is in, then it will be much easier for k210 support - in fact it may just be a few lines of code and selection of options.<br></div><div>Thanks!</div><div>Alan</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">
Regards<br>
Padmarao<br>
<br>
> Thanks,<br>
> Alan<br>
> <br>
> <br>
> On Thu, Sep 8, 2022 at 11:44 AM Padmarao Begari <<br>
> <a href="mailto:padmarao.begari@microchip.com" target="_blank">padmarao.begari@microchip.com</a>> wrote:<br>
> > If the bsp is integrated and supported a device tree<br>
> > blob(dtb) then use dtb instead of using it from the U-Boot.<br>
> > ---<br>
> > bsps/shared/start/bsp-fdt.c | 11 ++++++++++-<br>
> > 1 file changed, 10 insertions(+), 1 deletion(-)<br>
> > <br>
> > diff --git a/bsps/shared/start/bsp-fdt.c b/bsps/shared/start/bsp-<br>
> > fdt.c<br>
> > index 75a1ea41c9..7e1a698896 100644<br>
> > --- a/bsps/shared/start/bsp-fdt.c<br>
> > +++ b/bsps/shared/start/bsp-fdt.c<br>
> > @@ -32,6 +32,10 @@<br>
> > #include <bsp/fdt.h><br>
> > #include <bsp/linker-symbols.h><br>
> > <br>
> > +#ifdef BSP_DTB_IS_SUPPORTED<br>
> > +#include BSP_DTB_HEADER_PATH<br>
> > +#endif<br>
> > +<br>
> > #ifndef BSP_FDT_IS_SUPPORTED<br>
> > #warning "BSP FDT support indication not defined"<br>
> > #endif<br>
> > @@ -51,7 +55,12 @@ bsp_fdt_blob[BSP_FDT_BLOB_SIZE_MAX /<br>
> > sizeof(uint32_t)];<br>
> > <br>
> > void bsp_fdt_copy(const void *src)<br>
> > {<br>
> > +#ifdef BSP_DTB_IS_SUPPORTED<br>
> > +const volatile uint32_t *s = (const uint32_t *) system_dtb;<br>
> > +#else<br>
> > const volatile uint32_t *s = (const uint32_t *) src;<br>
> > +#endif<br>
> > +<br>
> > #ifdef BSP_FDT_BLOB_COPY_TO_READ_ONLY_LOAD_AREA<br>
> > uint32_t *d = (uint32_t *) ((uintptr_t) &bsp_fdt_blob[0]<br>
> > - (uintptr_t) bsp_section_rodata_begin<br>
> > @@ -61,7 +70,7 @@ void bsp_fdt_copy(const void *src)<br>
> > #endif<br>
> > <br>
> > if (s != d) {<br>
> > - size_t m = MIN(sizeof(bsp_fdt_blob), fdt_totalsize(src));<br>
> > + size_t m = MIN(sizeof(bsp_fdt_blob), fdt_totalsize(s));<br>
> > size_t aligned_size = roundup2(m, CPU_CACHE_LINE_BYTES);<br>
> > size_t n = (m + sizeof(*d) - 1) / sizeof(*d);<br>
> > size_t i;<br>
</blockquote></div></div>