[U-Boot] [PATCH v3] spi: ich: Configure SPI BIOS parameters

[Bin Meng]

Hi Stefan,

On Thu, Feb 9, 2017 at 5:26 PM, Stefan Roese <sr at denx.de> wrote:
> Without configuring these registers in the SPI controller, the Linux
> MTD device driver is not able to correctly read/write to the SPI
> NOR chip at all. In fact, the chip is not detected at all.
>
> Signed-off-by: Stefan Roese <sr at denx.de>
> Cc: Bin Meng <bmeng.cn at gmail.com>
> Cc: Simon Glass <sjg at chromium.org>
> Cc: Jagan Teki <jteki at openedev.com>
> ---
> v3:
> - Moved defines from C-file into header file
> - Changed some enum's into macros in ich.h for consistency
> - Used these newly introduced defines
>
> v2:
> - Moved code into the ICH SPI driver as suggested by Simon and Bin
>
>  drivers/spi/ich.c | 18 ++++++++++++++++++
>  drivers/spi/ich.h | 45 ++++++++++++++++++++++++++++++++++++++-------
>  2 files changed, 56 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/spi/ich.c b/drivers/spi/ich.c
> index caf0103dc3..0418455a79 100644
> --- a/drivers/spi/ich.c
> +++ b/drivers/spi/ich.c
> @@ -111,6 +111,17 @@ static int ich9_can_do_33mhz(struct udevice *dev)
>         return speed == 1;
>  }
>
> +/*
> + * Configure SPI controller so that the Linux MTD driver can fully
> + * access the SPI NOR chip
> + */
> +static void spi_controller_config(struct ich_spi_priv *ctlr)
> +{
> +       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));
> +}
> +
>  static int ich_init_controller(struct udevice *dev,
>                                struct ich_spi_platdata *plat,
>                                struct ich_spi_priv *ctlr)
> @@ -172,6 +183,13 @@ static int ich_init_controller(struct udevice *dev,
>
>         ich_set_bbar(ctlr, 0);
>
> +       /*
> +        * Configure the SPI-NOR controller in a way that the Linux
> +        * MTD SPI-NOR device driver has full read-write access to
> +        * the SPI-NOR chips
> +        */
> +       spi_controller_config(ctlr);
> +
>         return 0;
>  }
>
> diff --git a/drivers/spi/ich.h b/drivers/spi/ich.h
> index bd0a820809..7282181247 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,
>  };
>
> @@ -127,6 +120,44 @@ struct spi_trans {
>  #define SPI_OPCODE_WREN                0x06
>  #define SPI_OPCODE_FAST_READ   0x0b
>
> +#define SPI_OPCODE_TYPE_READ_NO_ADDRESS                0
> +#define SPI_OPCODE_TYPE_WRITE_NO_ADDRESS       1
> +#define SPI_OPCODE_TYPE_READ_WITH_ADDRESS      2
> +#define SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS     3
> +

Thanks for changing this.

> +#define SPI_OPMENU_0   0x01 /* WRSR: Write Status Register */
> +#define SPI_OPTYPE_0   SPI_OPCODE_TYPE_WRITE_NO_ADDRESS
> +
> +#define SPI_OPMENU_1   0x02 /* BYPR: Byte Program */
> +#define SPI_OPTYPE_1   SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS
> +
> +#define SPI_OPMENU_2   0x03 /* READ: Read Data */
> +#define SPI_OPTYPE_2   SPI_OPCODE_TYPE_READ_WITH_ADDRESS
> +
> +#define SPI_OPMENU_3   0x05 /* RDSR: Read Status Register */
> +#define SPI_OPTYPE_3   SPI_OPCODE_TYPE_READ_NO_ADDRESS
> +
> +#define SPI_OPMENU_4   0x20 /* SE20: Sector Erase 0x20 */
> +#define SPI_OPTYPE_4   SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS
> +
> +#define SPI_OPMENU_5   0x9f /* RDID: Read ID */
> +#define SPI_OPTYPE_5   SPI_OPCODE_TYPE_READ_NO_ADDRESS
> +
> +#define SPI_OPMENU_6   0xd8 /* BED8: Block Erase 0xd8 */
> +#define SPI_OPTYPE_6   SPI_OPCODE_TYPE_WRITE_WITH_ADDRESS
> +
> +#define SPI_OPMENU_7   0x0b /* FAST: Fast Read */
> +#define SPI_OPTYPE_7   SPI_OPCODE_TYPE_READ_WITH_ADDRESS
> +

These are SPI flash commands. Should we name them similarly like
SPI_OPCODE_WREN and SPI_OPCODE_FAST_READ?

> +#define SPI_OPTYPE ((SPI_OPTYPE_7 << 14) | (SPI_OPTYPE_6 << 12) | \
> +                   (SPI_OPTYPE_5 << 10) | (SPI_OPTYPE_4 <<  8) | \
> +                   (SPI_OPTYPE_3 <<  6) | (SPI_OPTYPE_2 <<  4) | \
> +                   (SPI_OPTYPE_1 <<  2) | (SPI_OPTYPE_0 <<  0))
> +#define SPI_OPMENU_UPPER ((SPI_OPMENU_7 << 24) | (SPI_OPMENU_6 << 16) | \
> +                         (SPI_OPMENU_5 <<  8) | (SPI_OPMENU_4 <<  0))
> +#define SPI_OPMENU_LOWER ((SPI_OPMENU_3 << 24) | (SPI_OPMENU_2 << 16) | \
> +                         (SPI_OPMENU_1 <<  8) | (SPI_OPMENU_0 <<  0))
> +

What if the board mounts a flash with a different SPI flash command
set? Will this work?

>  enum ich_version {
>         ICHV_7,
>         ICHV_9,
> --

Regards,
Bin