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

[Simon Glass]

Hi Bin,

On 12 September 2017 at 07:53, Bin Meng  wrote:
> Hi Simon,
>
> On Wed, Sep 6, 2017 at 9:39 AM, Simon Glass  wrote:
>> Hi Bin,
>>
>> On 26 August 2017 at 18:12, Bin Meng  wrote:
>>> Hi Simon,
>>>
>>> On Sun, Aug 27, 2017 at 6:40 AM, Simon Glass  wrote:
 Hi Bin,

 On 26 August 2017 at 07:58, Bin Meng  wrote:
> Hi Simon,
>
> On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass  wrote:
>> Hi Bin,
>>
>> On 15 August 2017 at 23:38, Bin Meng  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 
>>> ---
>>>
>>>  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, ))
>>> +   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 

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

[Bin Meng]

Hi Simon,

On Wed, Sep 6, 2017 at 9:39 AM, Simon Glass  wrote:
> Hi Bin,
>
> On 26 August 2017 at 18:12, Bin Meng  wrote:
>> Hi Simon,
>>
>> On Sun, Aug 27, 2017 at 6:40 AM, Simon Glass  wrote:
>>> Hi Bin,
>>>
>>> On 26 August 2017 at 07:58, Bin Meng  wrote:
 Hi Simon,

 On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass  wrote:
> Hi Bin,
>
> On 15 August 2017 at 23:38, Bin Meng  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 
>> ---
>>
>>  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, ))
>> +   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
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [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 18:12, Bin Meng  wrote:
> Hi Simon,
>
> On Sun, Aug 27, 2017 at 6:40 AM, Simon Glass  wrote:
>> Hi Bin,
>>
>> On 26 August 2017 at 07:58, Bin Meng  wrote:
>>> Hi Simon,
>>>
>>> On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass  wrote:
 Hi Bin,

 On 15 August 2017 at 23:38, Bin Meng  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 
> ---
>
>  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, ))
> +   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?

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Bin Meng]

Hi Simon,

On Sun, Aug 27, 2017 at 6:40 AM, Simon Glass  wrote:
> Hi Bin,
>
> On 26 August 2017 at 07:58, Bin Meng  wrote:
>> Hi Simon,
>>
>> On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass  wrote:
>>> Hi Bin,
>>>
>>> On 15 August 2017 at 23:38, Bin Meng  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 
 ---

  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, ))
 +   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.

Regards,
Bin
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [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  wrote:
> Hi Simon,
>
> On Sat, Aug 26, 2017 at 9:39 PM, Simon Glass  wrote:
>> Hi Bin,
>>
>> On 15 August 2017 at 23:38, Bin Meng  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 
>>> ---
>>>
>>>  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, ))
>>> +   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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

Re: [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  wrote:
> Hi Bin,
>
> On 15 August 2017 at 23:38, Bin Meng  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 
>> ---
>>
>>  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, ))
>> +   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
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Simon Glass]

Hi Bin,

On 15 August 2017 at 23:38, Bin Meng  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 
> ---
>
>  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, ))
> +   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?

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

Regards,
Simon
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Bin Meng]

On Wed, Aug 16, 2017 at 2:21 PM, Stefan Roese  wrote:
> On 16.08.2017 07:38, Bin Meng 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 
>> ---
>>
>>   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:
>
>
> because

Fixed this, and

>
>> +*
>> +* 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, ))
>> +   ich_spi_config_opcode(dev);
>> +#endif
>> +
>> /* call into FspNotify */
>> debug("Calling into FSP (notify phase INIT_PHASE_BOOT): ");
>> status = fsp_notify(NULL, INIT_PHASE_BOOT);
>>
>
> Reviewed-by: Stefan Roese 

applied to u-boot-x86, thanks!
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Stefan Roese]

On 16.08.2017 07:38, Bin Meng 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 
---

  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:


because


+*
+* 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, ))
+   ich_spi_config_opcode(dev);
+#endif
+
/* call into FspNotify */
debug("Calling into FSP (notify phase INIT_PHASE_BOOT): ");
status = fsp_notify(NULL, INIT_PHASE_BOOT);



Reviewed-by: Stefan Roese 

Thanks,
Stefan
___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot

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

[Bin Meng]

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 
---

 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, ))
+   ich_spi_config_opcode(dev);
+#endif
+
/* call into FspNotify */
debug("Calling into FSP (notify phase INIT_PHASE_BOOT): ");
status = fsp_notify(NULL, INIT_PHASE_BOOT);
-- 
2.9.2

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot