Hi Bin,
On 12 September 2017 at 07:53, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Wed, Sep 6, 2017 at 9:39 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 August 2017 at 18:12, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sun, Aug 27, 2017 at 6:40 AM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 26 August 2017 at 07:58, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass sjg@chromium.org wrote:
Hi Bin,
On 15 August 2017 at 23:38, Bin Meng bmeng.cn@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@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