Antwort: Re: [RFC PATCH 2/2] arch: x86: apl: Use devicetree for FSP configuration

[Bernhard Messerklinger]

Hi Simon,

>Hi Bernhard,
>
>On Thu, 30 Apr 2020 at 03:16, Bernhard Messerklinger
><bernhard.messerklinger at br-automation.com> wrote:
>>
>> A the moment the FSP configuration is a mix of hard coded values
>and
>> devicetree properties.
>> This patch makes FSP-M and FSP-S full configurable from devicetree
>by
>> adding binding properties for all FSP parameters.
>>
>> Co-developed-by: Wolfgang Wallner
><wolfgang.wallner at br-automation.com>
>> Signed-off-by: Wolfgang Wallner
><wolfgang.wallner at br-automation.com>
>> Signed-off-by: Bernhard Messerklinger
><bernhard.messerklinger at br-automation.com>
>>
>> ---
>>
>>  arch/x86/cpu/apollolake/Makefile              |    1 +
>>  arch/x86/cpu/apollolake/fsp_bindings.c        | 2096
>+++++++++++++++++
>>  arch/x86/cpu/apollolake/fsp_m.c               |  164 +-
>>  arch/x86/cpu/apollolake/fsp_s.c               |  382 +--
>>  arch/x86/dts/chromebook_coral.dts             |   72 +-
>>  .../asm/arch-apollolake/fsp/fsp_m_upd.h       |  168 ++
>>  .../asm/arch-apollolake/fsp/fsp_s_upd.h       |  202 ++
>>  .../asm/arch-apollolake/fsp_bindings.h        |   74 +
>>  .../fsp/fsp2/apollolake/fsp-m.txt             |  320 +++
>>  .../fsp/fsp2/apollolake/fsp-s.txt             |  483 ++++
>>  10 files changed, 3422 insertions(+), 540 deletions(-)
>>  create mode 100644 arch/x86/cpu/apollolake/fsp_bindings.c
>>  create mode 100644
>arch/x86/include/asm/arch-apollolake/fsp_bindings.h
>>  create mode 100644
>doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-m.txt
>>  create mode 100644
>doc/device-tree-bindings/fsp/fsp2/apollolake/fsp-s.txt
>>
>
>Tested on coral:
>Tested-by: Simon Glass <sjg at chromium.org>
>
>This looks good to me. I wonder if one day the binding table could be
>created from the binding .txt file, or compared with it
>programmatically?
Yes that's true. But at the moment its just copy paste.
I generated the binding table from the fsp_s and fsp_m config struct
with a python script. But this script is also far from being finished.
> ...
>> +#if defined(CONFIG_SPL_BUILD)
>
>Do you need these #ifs? I would hope the compiler would only include
>them if needed.
Without the #ifs the SPL size stays the same but the u-boot proper size
increases by about 2 kb.
> ...
>> +void fsp_s_update_config_from_dtb(ofnode node, struct fsp_s_config
>*cfg);
>
>function comments
>
>Regards,
>Simon

Regards,
Bernhard