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

[Michal Simek]

On 3/18/24 17:17, James Hilliard wrote:
> On Mon, Mar 18, 2024 at 5:07 AM Michal Simek <michal.simek at 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 at 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 at 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 at 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