On Mon, Mar 18, 2024 at 2:26 AM Michal Simek <michal.si...@amd.com> wrote:
>
>
>
> On 3/15/24 20:25, James Hilliard wrote:
> > There are situations where we may want to let U-Boot modify the FDT
>
> please use imperative mood.
>
> > nand partitions for the kernel, such as when supporting multiple
> > sizes of NAND chips.
> >
> > Lets also refactor xilinx common board support to have a
> > ft_common_board_setup which gets called by the ft_board_setup for each
> > specific board so that we can add non-common functionality to each
> > ft_board_setup like FDT_FIXUP_PARTITIONS as needed.
> >
> > This pattern is modeled after the one used by tdx-common.c.
> >
> > Signed-off-by: James Hilliard <james.hillia...@gmail.com>
> > ---
> > board/xilinx/common/board.c | 2 +-
> > board/xilinx/common/board.h | 2 ++
> > board/xilinx/mbv/board.c | 9 +++++++++
> > board/xilinx/versal-net/board.c | 7 +++++++
> > board/xilinx/versal/board.c | 7 +++++++
> > board/xilinx/zynq/board.c | 17 +++++++++++++++++
> > board/xilinx/zynqmp/zynqmp.c | 7 +++++++
> > board/xilinx/zynqmp_r5/board.c | 8 ++++++++
> > 8 files changed, 58 insertions(+), 1 deletion(-)
> >
> > diff --git a/board/xilinx/common/board.c b/board/xilinx/common/board.c
> > index 9641ed307b..629ba2b902 100644
> > --- a/board/xilinx/common/board.c
> > +++ b/board/xilinx/common/board.c
> > @@ -686,7 +686,7 @@ phys_addr_t board_get_usable_ram_top(phys_size_t
> > total_size)
> >
> > #ifdef CONFIG_OF_BOARD_SETUP
> > #define MAX_RAND_SIZE 8
> > -int ft_board_setup(void *blob, struct bd_info *bd)
> > +int ft_common_board_setup(void *blob, struct bd_info *bd)
> > {
> > size_t n = MAX_RAND_SIZE;
> > struct udevice *dev;
> > diff --git a/board/xilinx/common/board.h b/board/xilinx/common/board.h
> > index 64d657673e..73f576952a 100644
> > --- a/board/xilinx/common/board.h
> > +++ b/board/xilinx/common/board.h
> > @@ -18,4 +18,6 @@ bool board_detection(void);
> > char *soc_name_decode(void);
> >
> > bool soc_detection(void);
> > +
> > +int ft_common_board_setup(void *blob, struct bd_info *bd);
> > #endif /* BOARD_XILINX_COMMON_BOARD_H */
> > diff --git a/board/xilinx/mbv/board.c b/board/xilinx/mbv/board.c
> > index ccf4395d6a..d8af1eaa90 100644
> > --- a/board/xilinx/mbv/board.c
> > +++ b/board/xilinx/mbv/board.c
> > @@ -5,7 +5,16 @@
> > * Michal Simek <michal.si...@amd.com>
> > */
> >
> > +#include "../common/board.h"
> > +
> > int board_init(void)
> > {
> > return 0;
> > }
> > +
> > +#ifdef CONFIG_OF_BOARD_SETUP
> > +int ft_board_setup(void *blob, struct bd_info *bd)
> > +{
> > + return ft_common_board_setup(blob, bd);
> > +}
> > +#endif
>
> Any reason not to put directly to board/xilinx/common/board.c?
> All xilinx boards are kept in sync from user perspective that's why generic
> pieces should be added to common location.
I assumed the device tree node "arm,pl353-nand-r2p1" was specific to boards
using xilinx/zynq/board.c since it seemed only those were the only ones to use
that node. Have any idea if that assumption is accurate?
>
>
> > diff --git a/board/xilinx/versal-net/board.c
> > b/board/xilinx/versal-net/board.c
> > index 990ca1650a..bd674e6739 100644
> > --- a/board/xilinx/versal-net/board.c
> > +++ b/board/xilinx/versal-net/board.c
> > @@ -371,3 +371,10 @@ int dram_init(void)
> > void reset_cpu(void)
> > {
> > }
> > +
> > +#ifdef CONFIG_OF_BOARD_SETUP
> > +int ft_board_setup(void *blob, struct bd_info *bd)
> > +{
> > + return ft_common_board_setup(blob, bd);
> > +}
> > +#endif
> > diff --git a/board/xilinx/versal/board.c b/board/xilinx/versal/board.c
> > index 8c2e614ad8..944ef2d822 100644
> > --- a/board/xilinx/versal/board.c
> > +++ b/board/xilinx/versal/board.c
> > @@ -320,3 +320,10 @@ enum env_location env_get_location(enum env_operation
> > op, int prio)
> > return ENVL_NOWHERE;
> > }
> > }
> > +
> > +#ifdef CONFIG_OF_BOARD_SETUP
> > +int ft_board_setup(void *blob, struct bd_info *bd)
> > +{
> > + return ft_common_board_setup(blob, bd);
> > +}
> > +#endif
> > diff --git a/board/xilinx/zynq/board.c b/board/xilinx/zynq/board.c
> > index 6c36591001..1ca1984c49 100644
> > --- a/board/xilinx/zynq/board.c
> > +++ b/board/xilinx/zynq/board.c
> > @@ -13,10 +13,12 @@
> > #include <env.h>
> > #include <env_internal.h>
> > #include <fdtdec.h>
> > +#include <fdt_support.h>
> > #include <fpga.h>
> > #include <malloc.h>
> > #include <memalign.h>
> > #include <mmc.h>
> > +#include <mtd_node.h>
> > #include <watchdog.h>
> > #include <wdt.h>
> > #include <zynqpl.h>
> > @@ -201,3 +203,18 @@ void set_dfu_alt_info(char *interface, char *devstr)
> > puts("DFU alt info setting: done\n");
> > }
> > #endif
> > +
> > +#ifdef CONFIG_OF_BOARD_SETUP
> > +int ft_board_setup(void *blob, struct bd_info *bd)
> > +{
> > +#ifdef CONFIG_FDT_FIXUP_PARTITIONS
> > + static const struct node_info nodes[] = {
> > + { "arm,pl353-nand-r2p1", MTD_DEV_TYPE_NAND, },
>
> Very likely also depends on CONFIG_NAND_ZYNQ. It means pretty much you can
> move
> this code directly to common/board.c because only Zynq uses this symbol.
>
> Thanks,
> Michal
