diff mbox series

[1/1] xilinx: zynq: add FDT_FIXUP_PARTITIONS support

Message ID 20240315192528.464849-1-james.hilliard1@gmail.com
State Superseded
Delegated to: Michal Simek
Headers show
Series [1/1] xilinx: zynq: add FDT_FIXUP_PARTITIONS support | expand

Commit Message

James Hilliard March 15, 2024, 7:25 p.m. UTC
There are situations where we may want to let U-Boot modify the FDT
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.hilliard1@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(-)

Comments

Michal Simek March 18, 2024, 8:26 a.m. UTC | #1
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.hilliard1@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.simek@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.


> 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
James Hilliard March 18, 2024, 8:48 a.m. UTC | #2
On Mon, Mar 18, 2024 at 2:26 AM Michal Simek <michal.simek@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.hilliard1@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.simek@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
Michal Simek March 18, 2024, 11:06 a.m. UTC | #3
On 3/18/24 09:48, James Hilliard wrote:
> On Mon, Mar 18, 2024 at 2:26 AM Michal Simek <michal.simek@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.hilliard1@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.simek@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?

Configuration around xilinx boards are quite generic. Normally nand controller 
is by default connected on Zynq SOC. But because chips are highly configurable 
you can connect it also from Microblaze or Risc-V but

I think you can simply do changes in common location and put there proper ifs 
and it should be fine.
Also keep in mind that you should avoid using #ifdef as much as possible and use
if (IS_ENABLED(...)) instead.

M
James Hilliard March 18, 2024, 4:17 p.m. UTC | #4
On Mon, Mar 18, 2024 at 5:07 AM Michal Simek <michal.simek@amd.com> wrote:
>
>
>
> On 3/18/24 09:48, James Hilliard wrote:
> > On Mon, Mar 18, 2024 at 2:26 AM Michal Simek <michal.simek@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.hilliard1@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.simek@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?
>
> Configuration around xilinx boards are quite generic. Normally nand controller
> is by default connected on Zynq SOC. But because chips are highly configurable
> you can connect it also from Microblaze or Risc-V but
>
> I think you can simply do changes in common location and put there proper ifs
> and it should be fine.
> Also keep in mind that you should avoid using #ifdef as much as possible and use
> if (IS_ENABLED(...)) instead.

Is there a way to do that with the static const struct node_info nodes[] part?

>
> M
>
Michal Simek March 19, 2024, 6:59 a.m. UTC | #5
On 3/18/24 17:17, James Hilliard wrote:
> On Mon, Mar 18, 2024 at 5:07 AM Michal Simek <michal.simek@amd.com> wrote:
>>
>>
>>
>> On 3/18/24 09:48, James Hilliard wrote:
>>> On Mon, Mar 18, 2024 at 2:26 AM Michal Simek <michal.simek@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.hilliard1@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.simek@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?
>>
>> Configuration around xilinx boards are quite generic. Normally nand controller
>> is by default connected on Zynq SOC. But because chips are highly configurable
>> you can connect it also from Microblaze or Risc-V but
>>
>> I think you can simply do changes in common location and put there proper ifs
>> and it should be fine.
>> Also keep in mind that you should avoid using #ifdef as much as possible and use
>> if (IS_ENABLED(...)) instead.
> 
> Is there a way to do that with the static const struct node_info nodes[] part?

Just like this:

int ft_board_setup(void *blob, struct bd_info *bd)
{
	static const struct node_info nodes[] = {
		{ "arm,pl353-nand-r2p1", MTD_DEV_TYPE_NAND, },
	};

	if (IS_ENABLED(FDT_FIXUP_PARTITIONS) && IS_ENABLED(NAND_ZYNQ))
		fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes));

	return ft_common_board_setup(blob, bd);
}

M
diff mbox series

Patch

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.simek@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
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, },
+	};
+
+	fdt_fixup_mtdparts(blob, nodes, ARRAY_SIZE(nodes));
+#endif
+
+	return ft_common_board_setup(blob, bd);
+}
+#endif
diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
index ba49eb7be2..cd06396df0 100644
--- a/board/xilinx/zynqmp/zynqmp.c
+++ b/board/xilinx/zynqmp/zynqmp.c
@@ -700,3 +700,10 @@  unsigned int spl_spi_get_uboot_offs(struct spi_flash *flash)
 	return offset;
 }
 #endif
+
+#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/zynqmp_r5/board.c b/board/xilinx/zynqmp_r5/board.c
index 5c5a2e9386..a512577260 100644
--- a/board/xilinx/zynqmp_r5/board.c
+++ b/board/xilinx/zynqmp_r5/board.c
@@ -6,6 +6,7 @@ 
 #include <common.h>
 #include <fdtdec.h>
 #include <init.h>
+#include "../common/board.h"
 
 int board_init(void)
 {
@@ -24,3 +25,10 @@  int dram_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