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

[Bin Meng]

On Tue, May 9, 2017 at 7:20 PM, Stefan Roese <sr at denx.de> wrote:
> (Added Simon to Cc)

Really added Simon :-)

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

>
> This patch only changes some defines here and passes some allowed
> opcodes via some configuration registers to the Linux driver.
>
> I didn't look closely into this U-Boot driver and how it interacts
> with the SPI NOR. Simon is most likely the best person to answer
> on your questions regarding the usage of spi_flash. Simon could
> you please answer Jagan's questions?

Regards,
Bin