[U-Boot] [PATCH v3 1/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime

[Michal Simek]

Hi,

snip.

>>> +$(obj)/pm_cfg_obj.o: $(obj)/pm_cfg_obj.bin
>>> +
>>> +CFLAGS_zynqmp.o += -DZYNQMP_LOAD_PM_CFG_OBJ
>>
>> I am no fan of passing another object. you have
>> CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE already and this can be used instead.
> 
> Not sure I got your point here. I'm not passing an object, just setting
> a define (without value). This is used to enable code under #ifdef in C
> files.

Sorry I meant new config option. It should be enough to use CFG_OBJ_FILE
everywhere and not create another config option which is just used the
same way.

> 
>>> +obj-$(CONFIG_SPL_BUILD) += pm_cfg_obj.o
>>> +endif
>>> +
>>>  obj-$(CONFIG_MMC_SDHCI_ZYNQ) += tap_delays.o
>>>  
>>>  ifndef CONFIG_SPL_BUILD
> ...
>>> diff --git a/board/xilinx/zynqmp/zynqmp.c b/board/xilinx/zynqmp/zynqmp.c
>>> index db272478506f..7fcb3e120688 100644
>>> --- a/board/xilinx/zynqmp/zynqmp.c
>>> +++ b/board/xilinx/zynqmp/zynqmp.c
>>> @@ -327,6 +327,14 @@ int board_early_init_f(void)
>>>  
>>>  int board_init(void)
>>>  {
>>> +#if defined(CONFIG_SPL_BUILD) && defined(ZYNQMP_LOAD_PM_CFG_OBJ)
>>> +	extern const u32 zynqmp_pm_cfg_obj[];
>>> +	extern const int zynqmp_pm_cfg_obj_size;
>>
>> please put these two to header instead.
> 
> This was done on purpose to reduce the amount of #ifdefs, and also to
> not pollute the namespace with two symbols that are not needed outside
> this function. I don't see the added value of moving them in a .h, but I
> might be wrong.

Is checkpatch ok with this? I think that it should error out that you
shouldn't put externs to .c files.

> 
>> Anyway we are almost there. I have tested it on HW and it works.
>> When this is cleanup I think this should also go to zynqmp pmufw command
>> to be able to change it at run time directly from u-boot prompt.
> 
> Sure. Also moving to a mailbox uclass driver is on my todo list. But
> this is progressing in spare time, so it's all probably going to happen
> after this patch is accepted. Ok for you?

Definitely. As I told you I am ok with current approach and these things
should come later.

Thanks,
Michal