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

[Simon Glass]

Hi Bin,

On 26 August 2017 at 07:58, 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:
>> 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?

Yes I think that would be better.

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

Regards,
Simon