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 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).
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.cudm...@gmail.com> wrote:
> Hi Padmarao,
> Please see my inline comments below
>
> On Wed, Sep 14, 2022 at 12:51 AM <padmarao.beg...@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.beg...@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;
>>
>
_______________________________________________
devel mailing list
devel@rtems.org
http://lists.rtems.org/mailman/listinfo/devel
