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

[Simon Glass]

Hi Jagan,

On 9 May 2017 at 18:47, Bin Meng <bmeng.cn at gmail.com> wrote:
> 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?

This driver is odd in that it tries to decode low-level SPI requests
coming in via spi_xfer() and issue high-level (SPI flash) commands to
the controller. The controller does not actually support generic SPI
operation.

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

Regards,
Simon