[U-Boot] [PATCH 11/13] x86: braswell: Add FSP configuration

[Simon Glass]

Hi Bin,

On 26 August 2017 at 18:10, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Sun, Aug 27, 2017 at 6:39 AM, Simon Glass <sjg at chromium.org> wrote:
>> Hi Bin,
>>
>> On 26 August 2017 at 07:56, Bin Meng <bmeng.cn at gmail.com> wrote:
>>> Hi Simon,
>>>
>>> On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass <sjg at chromium.org> wrote:
>>>> On 15 August 2017 at 23:42, Bin Meng <bmeng.cn at gmail.com> wrote:
>>>>> Add FSP related configuration for Braswell.
>>>>>
>>>>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>>>> ---
>>>>>
>>>>>  arch/x86/cpu/braswell/Makefile                     |   2 +-
>>>>>  arch/x86/cpu/braswell/fsp_configs.c                | 158 ++++++++++++++
>>>>>  .../include/asm/arch-braswell/fsp/fsp_configs.h    |  89 ++++++++
>>>>>  arch/x86/include/asm/arch-braswell/fsp/fsp_vpd.h   | 172 +++++++++++++++
>>>>>  arch/x86/include/asm/arch-braswell/gpio.h          | 234 +++++++++++++++++++++
>>>>>  5 files changed, 654 insertions(+), 1 deletion(-)
>>>>>  create mode 100644 arch/x86/cpu/braswell/fsp_configs.c
>>>>>  create mode 100644 arch/x86/include/asm/arch-braswell/fsp/fsp_configs.h
>>>>>  create mode 100644 arch/x86/include/asm/arch-braswell/fsp/fsp_vpd.h
>>>>>  create mode 100644 arch/x86/include/asm/arch-braswell/gpio.h
>>>>>
>>>>
>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>>
>>>> Can this use drivers instead of manual device-tree access?
>>>
>>> Which part?
>>
>> Well you have intel,braswell-fsp for example. You could create a
>> driver with the two compatible strings and have it read the platdata
>> from the DT in the ofdata_to_platdata() method.
>
> I thought this before. We discussed the possibility of adding a new
> FSP uclass long time ago. When I added the Braswell support, I wanted
> to have a try since Braswell's FSP is v1.1 spec complaint and if we
> have a uclass for FSP we can put the common stuff in the uclass
> driver. But in the end I did not do it because:
>
> 1. FSP's initialization sequence is just a one time initialization and
> we don't do anything after the initialization completes.

That's not a very good reason though. There will be several drivers like that.

> 2. Making a uclass for FSP means we have to delay fsp_init() to after
> initf_dm().But after fsp_init(), we will return to board_init_f()
> again and do the initialization for the second time. So all previous
> platdata of FSP that is set up by DM gets lost during this process.

Yes, although this is in the nature of the broken FSP API that we hope
Intel will fix. As I understand it we already do the init twice, this
is just a case of knowing what stage we are at.

> 3. There are some other architecture-dependent stuff in the
> arch_fsp_init() that is not suitable to be put in a FSP driver.

But I am suggesting having a driver specific to the arch, not a
generic one, so this should not be a problem.

Regards,
Simon