[PATCH 3/3] bsps/shared/: Use device tree blob
Padmarao.Begari at microchip.com
Padmarao.Begari at microchip.com
Thu Sep 15 10:28:15 UTC 2022
Hi Alan,
On Wed, 2022-09-14 at 11:51 -0400, Alan Cudmore wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you
> know the content is safe
> Sorry for not thinking this through, but I think I have a more
> portable solution for handling the included DTB:
> 1. In bsps/shared/start/bsp-fdt.c, include the header as you had
> originally proposed.
> 2. Also bsps/shared/start/bsp-fdt.c, change bsp_fdt_get to return
> either &bsp_fdt_blob[0] for the u-boot option, or just system_dtb for
> the DTB option.
> 3. in bsps/riscv/shared/start.S - no change is needed for the fdt
> copy. This is only needed if the device tree is loaded by u-boot and
> needs to be copied. No need to call it for the embedded DTB.
> To make this work, you would need to have the following in config.ini
> for the polarfire (and eventually k210) bsp variant:
> BSP_START_COPY_FDT_FROM_U_BOOT=False
> BSP_DTB_IS_SUPPORTED=True
> I'm not sure if the BSP_DTB_IS_SUPPORTED option should be moved to
> the shared section of the specs, but I think it might - in this case
> the default might be False.
> Same with the BSP_DTB_HEADER_PATH option - but I'm not sure what a
> good default would be for that.
>
I agree with you, the BSP_DTB_IS_SUPPORTED and BSP_DTB_HEADER_PATH will
move into the shared section and the BSP_START_COPY_FDT_FROM_U_BOOT is
not enabled by default for the PolarFire SoC.
> I think this would set up the microblaze_fpga to use the same shared
> code and options, and we could eliminate the
> bsps/microblaze/microblaze_fpga/fdt/bsp_fdt.c file later.
>
> Below are the diffs for bsps/shared/start/bsp-fdt.c that is working
> on my k210 board. Again, sorry for any confusion, what I have now is
> just a slight change to your original proposal. (changing bsp_fdt_get
> instead of bsp_fdt_copy).
ok.. will change bsp_fdt_get().
Thank You for good suggestion. I will update same in the next patch.
Regards
Padmarao
> Thanks,
> Alan
> diff --git a/bsps/shared/start/bsp-fdt.c b/bsps/shared/start/bsp-
> fdt.c
> index 75a1ea41c9..6e2a0b82db 100644
> --- a/bsps/shared/start/bsp-fdt.c
> +++ b/bsps/shared/start/bsp-fdt.c
> @@ -36,6 +36,10 @@
> #warning "BSP FDT support indication not defined"
> #endif
>
> +#ifdef BSP_DTB_IS_SUPPORTED
> + #include BSP_DTB_HEADER_PATH
> +#endif
> +
> #ifndef BSP_FDT_BLOB_SIZE_MAX
> #define BSP_FDT_BLOB_SIZE_MAX 0
> #endif
> @@ -76,5 +80,9 @@ void bsp_fdt_copy(const void *src)
>
> const void *bsp_fdt_get(void)
> {
> +#if BSP_DTB_IS_SUPPORTED
> + return system_dtb;
> +#else
> return &bsp_fdt_blob[0];
> +#endif
> }
>
>
>
> On Wed, Sep 14, 2022 at 9:16 AM Alan Cudmore <alan.cudmore at gmail.com>
> wrote:
> > 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;
> > >
More information about the devel
mailing list