[U-Boot] [PATCH 5/5 v2] spi: ich: Configure SPI BIOS parameters for Linux upon U-Boot exit

[Stefan Roese]

Hi Bin,

On 10.05.2017 02:47, Bin Meng wrote:
> On Tue, May 9, 2017 at 7:20 PM, Stefan Roese <sr at denx.de> wrote:
>> (Added Simon to Cc)
>
> Really added Simon :-)

I did in my mail as well. Sometimes the ML removes recipients from
the list as it seems.

>>
>>
>> On 09.05.2017 13:14, Jagan Teki wrote:
>>>
>>> On Mon, Apr 24, 2017 at 1:18 PM, Stefan Roese <sr at denx.de> wrote:
>>>>
>>>> This patch adds a remove function to the Intel ICH SPI driver, that will
>>>> be called upon U-Boot exit, directly before the OS (Linux) is started.
>>>> This function takes care of configuring the BIOS registers in the SPI
>>>> controller (similar to what a "standard" BIOS or coreboot does), so that
>>>> the Linux MTD device driver is able to correctly read/write to the SPI
>>>> NOR chip. Without this, the chip is not detected at all.
>>>>
>>>> Signed-off-by: Stefan Roese <sr at denx.de>
>>>> Reviewed-by: Simon Glass <sjg at chromium.org>
>>>> Cc: Bin Meng <bmeng.cn at gmail.com>
>>>> Cc: Jagan Teki <jteki at openedev.com>
>>>> ---
>>>> v2:
>>>> - Added Simons RB line
>>>>
>>>>  drivers/spi/ich.c | 18 ++++++++++++++++++
>>>>  drivers/spi/ich.h | 54
>>>> +++++++++++++++++++++++++++++++++++++++++++++++-------
>>>>  2 files changed, 65 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
>>>> index 893fe33b66..bf2e99b5cc 100644
>>>> --- a/drivers/spi/ich.c
>>>> +++ b/drivers/spi/ich.c
>>>> @@ -617,6 +617,22 @@ static int ich_spi_probe(struct udevice *dev)
>>>>         return 0;
>>>>  }
>>>>
>>>> +static int ich_spi_remove(struct udevice *bus)
>>>> +{
>>>> +       struct ich_spi_priv *ctlr = dev_get_priv(bus);
>>>> +
>>>> +       /*
>>>> +        * Configure SPI controller so that the Linux MTD driver can
>>>> fully
>>>> +        * access the SPI NOR chip
>>>> +        */
>>>> +       ich_writew(ctlr, SPI_OPPREFIX, ctlr->preop);
>>>> +       ich_writew(ctlr, SPI_OPTYPE, ctlr->optype);
>>>> +       ich_writel(ctlr, SPI_OPMENU_LOWER, ctlr->opmenu);
>>>> +       ich_writel(ctlr, SPI_OPMENU_UPPER, ctlr->opmenu + sizeof(u32));
>>>> +
>>>> +       return 0;
>>>> +}
>>>> +
>>>>  static int ich_spi_set_speed(struct udevice *bus, uint speed)
>>>>  {
>>>>         struct ich_spi_priv *priv = dev_get_priv(bus);
>>>> @@ -700,4 +716,6 @@ U_BOOT_DRIVER(ich_spi) = {
>>>>         .priv_auto_alloc_size = sizeof(struct ich_spi_priv),
>>>>         .child_pre_probe = ich_spi_child_pre_probe,
>>>>         .probe  = ich_spi_probe,
>>>> +       .remove = ich_spi_remove,
>>>> +       .flags  = DM_FLAG_OS_PREPARE,
>>>>  };
>>>> diff --git a/drivers/spi/ich.h b/drivers/spi/ich.h
>>>> index bd0a820809..dcb8a9048f 100644
>>>> --- a/drivers/spi/ich.h
>>>> +++ b/drivers/spi/ich.h
>>>> @@ -102,13 +102,6 @@ enum {
>>>>  };
>>>>
>>>>  enum {
>>>> -       SPI_OPCODE_TYPE_READ_NO_ADDRESS =       0,
>>>> -       SPI_OPCODE_TYPE_WRITE_NO_ADDRESS =      1,
>>>> -       SPI_OPCODE_TYPE_READ_WITH_ADDRESS =     2,
>>>> -       SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS =    3
>>>> -};
>>>> -
>>>> -enum {
>>>>         ICH_MAX_CMD_LEN         = 5,
>>>>  };
>>>>
>>>> @@ -124,8 +117,55 @@ struct spi_trans {
>>>>         uint32_t offset;
>>>>  };
>>>>
>>>> +#define SPI_OPCODE_WRSR                0x01
>>>> +#define SPI_OPCODE_PAGE_PROGRAM        0x02
>>>> +#define SPI_OPCODE_READ                0x03
>>>> +#define SPI_OPCODE_WRDIS       0x04
>>>> +#define SPI_OPCODE_RDSR                0x05
>>>>  #define SPI_OPCODE_WREN                0x06
>>>>  #define SPI_OPCODE_FAST_READ   0x0b
>>>> +#define SPI_OPCODE_ERASE_SECT  0x20
>>>> +#define SPI_OPCODE_READ_ID     0x9f
>>>> +#define SPI_OPCODE_ERASE_BLOCK 0xd8
>>>
>>>
>>> Wonder why the flash part should be part of SPI, can't we use existing
>>> spi_flash through command interface if there is specific stuff like
>>> this?
>>
>
> These flash commands need to be programmed to SPI controller register,
> by Intel's design, to make Linux MTD driver happy. Possibly we may do
> like this, like get such value dynamically from spi_flash to program
> this to SPI controller? I once asked a question about this: if some
> other flash part that does not have the exact same command set as what
> was programmed to the SPI controller, will Linux MTD driver still
> work?

Yes, we discussed this a few months before already. I've not seen
any other values used in coreboot - so these values seem to be
quite "safe" for now. Once we have such an issue with incompatible
SPI NOR chips on x86, we will find a way to solve this in a board
specific way. But for now, its very helpful to have access from the
Linux MTD subsystem to the NOR chips by using these patches.

Thanks,
Stefan