Hi,
On 06/05/19 17:56, Michal Simek wrote:
> 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.
I totally agree. But I would need to check whether
CONFIG_ZYNQMP_LOAD_PM_CFG_OBJ_FILE is empty or not, and I don't think
the C preprocessor can do that.
>>>> 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.
Uhm, no, checkpatch is not ok with this. I disagree, but for sake of
peace in the world I'll move it to a .h file.
--
Luca
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
