Hi Bin, On 12 September 2017 at 07:53, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Wed, Sep 6, 2017 at 9:39 AM, Simon Glass <s...@chromium.org> wrote: >> Hi Bin, >> >> On 26 August 2017 at 18:12, Bin Meng <bmeng...@gmail.com> wrote: >>> Hi Simon, >>> >>> On Sun, Aug 27, 2017 at 6:40 AM, Simon Glass <s...@chromium.org> wrote: >>>> Hi Bin, >>>> >>>> On 26 August 2017 at 07:58, Bin Meng <bmeng...@gmail.com> wrote: >>>>> Hi Simon, >>>>> >>>>> On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass <s...@chromium.org> wrote: >>>>>> Hi Bin, >>>>>> >>>>>> On 15 August 2017 at 23:38, Bin Meng <bmeng...@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...@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. >>>> >>> >>> 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.
OK, sounds good so long as it knows when to call it. Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot