[PATCH 3/3] bsps/shared/: Use device tree blob

[Alan Cudmore]

Hi Padmarao,
Please see my inline comments below

On Wed, Sep 14, 2022 at 12:51 AM <Padmarao.Begari at microchip.com> wrote:

> Hi Alan,
>
> > On Tue, 2022-09-13 at 23:57 -0400, Alan Cudmore wrote:
> >
> > Hi Padmarao,
> > I am working on a RISC-V bsp variant for the Kendryte K210 and I am
> > also using an included DTB. For my k210 bsp, I changed
> > riscv/shared/start/start.S to set the address of the DTB array before
> > calling bsp_fdt_copy. If we change start.S to handle the following
> > three cases, we would not have to change the bsp_fdt_copy function.
> > The three cases are:
> > - no FDT, I assume griscv bsp
> > - uboot - a0 in contains the address of the u-boot loaded DTB
> > - polarfire, k210 and others - set a0 to the address of the included
> > DTB array
> > I think it's better to keep this function generic and not have to
> > conditionally ignore the input parameter.
> >
>
> Yes, Initially I thought same but don't want to include conditional
> statements in the startup code so moved to bsp_fdt_copy().
>
> Can we try like below in the startup code? so that it can support all
> three cases.
>
>         #ifdef BSP_DTB_IS_SUPPORTED
>                 #include BSP_DTB_HEADER_PATH
>         #endif
>

For including the DTB header, I have a file similar to the microblaze_fpga
bsp:
https://git.rtems.org/rtems/tree/bsps/microblaze/microblaze_fpga/fdt/bsp_fdt.c
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.


>
>         #ifdef BSP_START_COPY_FDT_FROM_U_BOOT
>         #ifdef BSP_DTB_IS_SUPPORT
>                 LADDR a0, system_dtb
>         #else
>                 mv      a0, a1
>         #endif
>                 call    bsp_fdt_copy
>         #endif
>
> I added your dtb options to my build, and tried this code in start.S. It
works for me.
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.
Thanks!
Alan


> Regards
> Padmarao
>
> > Thanks,
> > Alan
> >
> >
> > On Thu, Sep 8, 2022 at 11:44 AM Padmarao Begari <
> > padmarao.begari at microchip.com> wrote:
> > > If the bsp is integrated and supported a device tree
> > > blob(dtb) then use dtb instead of using it from the U-Boot.
> > > ---
> > >  bsps/shared/start/bsp-fdt.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/bsps/shared/start/bsp-fdt.c b/bsps/shared/start/bsp-
> > > fdt.c
> > > index 75a1ea41c9..7e1a698896 100644
> > > --- a/bsps/shared/start/bsp-fdt.c
> > > +++ b/bsps/shared/start/bsp-fdt.c
> > > @@ -32,6 +32,10 @@
> > >  #include <bsp/fdt.h>
> > >  #include <bsp/linker-symbols.h>
> > >
> > > +#ifdef BSP_DTB_IS_SUPPORTED
> > > +#include BSP_DTB_HEADER_PATH
> > > +#endif
> > > +
> > >  #ifndef BSP_FDT_IS_SUPPORTED
> > >  #warning "BSP FDT support indication not defined"
> > >  #endif
> > > @@ -51,7 +55,12 @@ bsp_fdt_blob[BSP_FDT_BLOB_SIZE_MAX /
> > > sizeof(uint32_t)];
> > >
> > >  void bsp_fdt_copy(const void *src)
> > >  {
> > > +#ifdef BSP_DTB_IS_SUPPORTED
> > > +const volatile uint32_t *s = (const uint32_t *) system_dtb;
> > > +#else
> > >    const volatile uint32_t *s = (const uint32_t *) src;
> > > +#endif
> > > +
> > >  #ifdef BSP_FDT_BLOB_COPY_TO_READ_ONLY_LOAD_AREA
> > >    uint32_t *d = (uint32_t *) ((uintptr_t) &bsp_fdt_blob[0]
> > >      - (uintptr_t) bsp_section_rodata_begin
> > > @@ -61,7 +70,7 @@ void bsp_fdt_copy(const void *src)
> > >  #endif
> > >
> > >    if (s != d) {
> > > -    size_t m = MIN(sizeof(bsp_fdt_blob), fdt_totalsize(src));
> > > +    size_t m = MIN(sizeof(bsp_fdt_blob), fdt_totalsize(s));
> > >      size_t aligned_size = roundup2(m, CPU_CACHE_LINE_BYTES);
> > >      size_t n = (m + sizeof(*d) - 1) / sizeof(*d);
> > >      size_t i;
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.rtems.org/pipermail/devel/attachments/20220914/9549f74b/attachment.htm>