[U-Boot] [PATCH v3 1/2] arm64: zynqmp: spl: install a PMU firmware config object at runtime
Michal Simek
michal.simek at xilinx.com
Mon May 6 15:56:30 UTC 2019
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
More information about the U-Boot
mailing list