[U-Boot] [PATCH 5/5] x86: fsp: Configure SPI opcode registers before SPI is locked down

[Bin Meng]

Hi Simon,

On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 15 August 2017 at 23:38, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Some Intel FSP (like Braswell) does SPI lock-down during the call
>> to fsp_notify(INIT_PHASE_BOOT). But before SPI lock-down is done,
>> it's bootloader's responsibility to configure the SPI controller's
>> opcode registers properly otherwise SPI controller driver doesn't
>> know how to communicate with the SPI flash device.
>>
>> This introduces a Kconfig option CONFIG_FSP_LOCKDOWN_SPI for such
>> FSPs. When it is on, U-Boot will configure the SPI opcode registers
>> before the lock-down.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>> ---
>>
>>  arch/x86/Kconfig              |  9 +++++++++
>>  arch/x86/lib/fsp/fsp_common.c | 24 ++++++++++++++++++++++++
>>  2 files changed, 33 insertions(+)
>>
>> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
>> index c26710b..5373082 100644
>> --- a/arch/x86/Kconfig
>> +++ b/arch/x86/Kconfig
>> @@ -401,6 +401,15 @@ config FSP_BROKEN_HOB
>>           do not overwrite the important boot service data which is used by
>>           FSP, otherwise the subsequent call to fsp_notify() will fail.
>>
>> +config FSP_LOCKDOWN_SPI
>> +       bool
>> +       depends on HAVE_FSP
>> +       help
>> +         Some Intel FSP (like Braswell) does SPI lock-down during the call
>> +         to fsp_notify(INIT_PHASE_BOOT). This option should be turned on
>> +         for such FSP and U-Boot will configure the SPI opcode registers
>> +         before the lock-down.
>> +
>>  config ENABLE_MRC_CACHE
>>         bool "Enable MRC cache"
>>         depends on !EFI && !SYS_COREBOOT
>> diff --git a/arch/x86/lib/fsp/fsp_common.c b/arch/x86/lib/fsp/fsp_common.c
>> index 3397bb8..320d87d 100644
>> --- a/arch/x86/lib/fsp/fsp_common.c
>> +++ b/arch/x86/lib/fsp/fsp_common.c
>> @@ -19,6 +19,8 @@
>>
>>  DECLARE_GLOBAL_DATA_PTR;
>>
>> +extern void ich_spi_config_opcode(struct udevice *dev);
>> +
>>  int checkcpu(void)
>>  {
>>         return 0;
>> @@ -49,6 +51,28 @@ void board_final_cleanup(void)
>>  {
>>         u32 status;
>>
>> +#ifdef CONFIG_FSP_LOCKDOWN_SPI
>> +       struct udevice *dev;
>> +
>> +       /*
>> +        * Some Intel FSP (like Braswell) does SPI lock-down during the call
>> +        * to fsp_notify(INIT_PHASE_BOOT). But before SPI lock-down is done,
>> +        * it's bootloader's responsibility to configure the SPI controller's
>> +        * opcode registers properly otherwise SPI controller driver doesn't
>> +        * know how to communicate with the SPI flash device.
>> +        *
>> +        * Note we cannot do such configuration elsewhere (eg: during the SPI
>> +        * controller driver's probe() routine), becasue:
>> +        *
>> +        * 1). U-Boot SPI controller driver does not set the lock-down bit
>> +        * 2). Any SPI transfer will corrupt the contents of these registers
>> +        *
>> +        * Hence we have to do it right here before SPI lock-down bit is set.
>> +        */
>> +       if (!uclass_first_device_err(UCLASS_SPI, &dev))
>> +               ich_spi_config_opcode(dev);
>
> I wonder if we could do this by using an operation instead of directly
> calling a function in the driver?
>

Do you mean adding one operation to dm_spi_ops?

>> +#endif
>> +
>>         /* call into FspNotify */
>>         debug("Calling into FSP (notify phase INIT_PHASE_BOOT): ");
>>         status = fsp_notify(NULL, INIT_PHASE_BOOT);
>> --

Regards,
Bin