Re: [PATCH v1 1/3] arm: k3: Fix ft_system_setup so it can be enabled on any SoC

[Andrew Davis <afd@ti.com>]

On 7/12/23 9:32 AM, Francesco Dolcini wrote:
> On Wed, Jul 12, 2023 at 09:15:21AM -0500, Andrew Davis wrote:
>> On 7/12/23 8:47 AM, Francesco Dolcini wrote:
>>> From: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
>>>
>>> ft_system_setup cannot be enabled on SoC without msmc sram otherwise
>>> fdt_fixup_msmc_ram function fails causing system reset.
>>>
>>> Fix by calling fdt_fixup_msmc_ram only on these specific SoC:
>>> - J721S2
>>> - AM654
>>> - J721E
>>>
>>> This change was verified to not change anything on any existing board
>>> (all the J721S2, AM654 and J721E boards requires it,
>>> none of the remaining k3 boards require it).
>>>
>>> Fixes: 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level")
>>> Signed-off-by: Emanuele Ghidoli <emanuele.ghidoli@toradex.com>
>>> Signed-off-by: Francesco Dolcini <francesco.dolcini@toradex.com>
>>> ---
>>>    arch/arm/mach-k3/common.c | 20 ++++++++++++--------
>>>    1 file changed, 12 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
>>> index 34737a43aa08..f86ccaedc94f 100644
>>> --- a/arch/arm/mach-k3/common.c
>>> +++ b/arch/arm/mach-k3/common.c
>>> @@ -433,14 +433,18 @@ int fdt_fixup_msmc_ram(void *blob, char *parent_path, char *node_name)
>>>    #if defined(CONFIG_OF_SYSTEM_SETUP)
>>>    int ft_system_setup(void *blob, struct bd_info *bd)
>>
>> How about we rename this function to something like ft_msmc_sram_setup(), then only
>> call it from board_setup for the platforms that needed it.
> 
> This is partially reverting what you did in commit
> 30e96a240156 ("arm: mach-k3: Move MSMC fixup to SoC level").
> 
> Is this fixup going to be required if tomorrow we have a new board using
> the AM654 SoC (for example) or not? Depending on this answer we can
> decide if ft_board_setup or ft_system_setup is the right function to use.
> 

Maybe the issue is that we have a common ft_system_setup() function for all
of K3, when these should be specific to each SoC.. They could call into
common functions for common tasks like MSMC fixup, but no need to have
one big ft_system_setup() for all K3.

Andrew