Hi Simon, On Wed, Sep 6, 2017 at 9:39 AM, Simon Glass <[email protected]> wrote: > Hi Bin, > > On 26 August 2017 at 18:12, Bin Meng <[email protected]> wrote: >> Hi Simon, >> >> On Sun, Aug 27, 2017 at 6:40 AM, Simon Glass <[email protected]> wrote: >>> Hi Bin, >>> >>> On 26 August 2017 at 07:58, Bin Meng <[email protected]> wrote: >>>> Hi Simon, >>>> >>>> On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass <[email protected]> wrote: >>>>> Hi Bin, >>>>> >>>>> On 15 August 2017 at 23:38, Bin Meng <[email protected]> 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 <[email protected]> >>>>>> --- >>>>>> >>>>>> 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. >>> >> >> Since this is x86-specific, I would hesitate to add one. >> > > Yes I understand that. Still I don't think we should call directly > into drivers. What do you suggest? > > - add some sort of DM event system to which drivers can attach for > notifications > - add an ioctl() method to the SPI uclass where we can put random > calls like this > - set up a new MISC device as a child of SPI which includes an ioctl > for this operation > - something else? >
These are maybe too complicated to solve this little specific issue. I've thought about this, and looks we can add a simple DTS property "intel,spi-lock-down" and let the driver call this function. Regards, Bin _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
