[U-Boot] [PATCH 3/3] sf: parse Serial Flash Discoverable Parameters (SFDP) tables
Prabhakar Kushwaha
prabhakar.kushwaha at nxp.com
Thu Dec 21 11:37:09 UTC 2017
Thanks Cyrille for taking time and reviewing this patch.
Please find reply in-lined.
> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen at wedev4u.fr]
> Sent: Wednesday, December 20, 2017 7:36 PM
> To: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>; u-
> boot at lists.denx.de
> Cc: jagannadh.teki at gmail.com; Poonam Aggrwal
> <poonam.aggrwal at nxp.com>; Suresh Gupta <suresh.gupta at nxp.com>;
> marek.vasut at gmail.com
> Subject: Re: [PATCH 3/3] sf: parse Serial Flash Discoverable Parameters (SFDP)
> tables
>
> Hi Prabhakar,
>
> Le 20/12/2017 à 11:32, Prabhakar Kushwaha a écrit :
> > This patch adds support to the JESD216 rev B standard and parses the
> > SFDP tables to dynamically initialize the 'struct spi_nor_flash_parameter'.
> >
> > It has been ported from Linux commit "mtd: spi-nor: parse Serial Flash
> > Discoverable Parameters (SFDP) tables". It Also ports all modifications
> > done on top of the mentioned commit.
> >
> > This feature is enabled by defining FLASH_SFDP in spi_flash_info's flag field.
> >
> > Signed-off-by: Prabhakar Kushwaha <prabhakar.kushwaha at nxp.com>
> > CC: Cyrille Pitchen <cyrille.pitchen at wedev4u.fr>
> > CC: Marek Vasut <marek.vasut at gmail.com>
> > ---
> > drivers/mtd/spi/sf_internal.h | 204 +++++++++++++++++++++
> > drivers/mtd/spi/spi_flash.c | 416
> +++++++++++++++++++++++++++++++++++++++++-
> > 2 files changed, 619 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
> > index 164b0ea..73fe207 100644
> > --- a/drivers/mtd/spi/sf_internal.h
> > +++ b/drivers/mtd/spi/sf_internal.h
> > @@ -12,6 +12,7 @@
> >
> > #include <linux/types.h>
> > #include <linux/compiler.h>
> > +#include <linux/sizes.h>
> >
> > /* Dual SPI flash memories - see SPI_COMM_DUAL_... */
> > enum spi_dual_flash {
> > @@ -82,6 +83,7 @@ enum spi_nor_option_flags {
> > #define CMD_FLAG_STATUS 0x70
> > #define CMD_EN4B 0xB7
> > #define CMD_EX4B 0xE9
> > +#define CMD_READ_SFDP 0x5a
> >
> > /* Bank addr access commands */
> > #ifdef CONFIG_SPI_FLASH_BAR
> > @@ -155,9 +157,211 @@ struct spi_flash_info {
> > * Use dedicated 4byte address op
> codes
> > * to support memory size above
> 128Mib.
> > */
> > +#define FLASH_SFDP BIT(9) /* Parse SFDP tables */
> > #define RD_FULL (RD_QUAD | RD_DUAL | RD_QUADIO |
> RD_DUALIO)
> > };
> >
> > +struct sfdp_parameter_header {
> > + u8 id_lsb;
> > + u8 minor;
> > + u8 major;
> > + u8 length; /* in double words */
> > + u8 parameter_table_pointer[3]; /* byte address */
> > + u8 id_msb;
> > +};
> > +
> > +#define SFDP_PARAM_HEADER_ID(p) (((p)->id_msb << 8) | (p)->id_lsb)
> > +#define SFDP_PARAM_HEADER_PTP(p) \
> > + (((p)->parameter_table_pointer[2] << 16) | \
> > + ((p)->parameter_table_pointer[1] << 8) | \
> > + ((p)->parameter_table_pointer[0] << 0))
> > +
> > +#define SFDP_BFPT_ID 0xff00 /* Basic Flash Parameter Table
> */
> > +#define SFDP_SECTOR_MAP_ID 0xff81 /* Sector Map Table */
> > +
> > +#define SFDP_SIGNATURE 0x50444653U
> > +#define SFDP_JESD216_MAJOR 1
> > +#define SFDP_JESD216_MINOR 0
> > +#define SFDP_JESD216A_MINOR 5
> > +#define SFDP_JESD216B_MINOR 6
> > +
> > +struct sfdp_header {
> > + u32 signature; /* Ox50444653U <=> "SFDP" */
> > + u8 minor;
> > + u8 major;
> > + u8 nph; /* 0-base number of parameter headers */
> > + u8 unused;
> > +
> > + /* Basic Flash Parameter Table. */
> > + struct sfdp_parameter_header bfpt_header;
> > +};
> > +
> > +/* Basic Flash Parameter Table */
> > +
> > +/*
> > + * JESD216 rev B defines a Basic Flash Parameter Table of 16 DWORDs.
> > + * They are indexed from 1 but C arrays are indexed from 0.
> > + */
> > +#define BFPT_DWORD(i) ((i) - 1)
> > +#define BFPT_DWORD_MAX 16
> > +
> > +/* The first version of JESB216 defined only 9 DWORDs. */
> > +#define BFPT_DWORD_MAX_JESD216 9
> > +
> > +/* 1st DWORD. */
> > +#define BFPT_DWORD1_FAST_READ_1_1_2 BIT(16)
> > +#define BFPT_DWORD1_ADDRESS_BYTES_MASK GENMASK(18,
> 17)
> > +#define BFPT_DWORD1_ADDRESS_BYTES_3_ONLY (0x0UL << 17)
> > +#define BFPT_DWORD1_ADDRESS_BYTES_3_OR_4 (0x1UL << 17)
> > +#define BFPT_DWORD1_ADDRESS_BYTES_4_ONLY (0x2UL << 17)
> > +#define BFPT_DWORD1_DTR BIT(19)
> > +#define BFPT_DWORD1_FAST_READ_1_2_2 BIT(20)
> > +#define BFPT_DWORD1_FAST_READ_1_4_4 BIT(21)
> > +#define BFPT_DWORD1_FAST_READ_1_1_4 BIT(22)
> > +
> > +/* 5th DWORD. */
> > +#define BFPT_DWORD5_FAST_READ_2_2_2 BIT(0)
> > +#define BFPT_DWORD5_FAST_READ_4_4_4 BIT(4)
> > +
> > +/* 11th DWORD. */
> > +#define BFPT_DWORD11_PAGE_SIZE_SHIFT 4
> > +#define BFPT_DWORD11_PAGE_SIZE_MASK GENMASK(7, 4)
> > +
> > +/* 15th DWORD. */
> > +
> > +/*
> > + * (from JESD216 rev B)
> > + * Quad Enable Requirements (QER):
> > + * - 000b: Device does not have a QE bit. Device detects 1-1-4 and 1-4-4
> > + * reads based on instruction. DQ3/HOLD# functions are hold during
> > + * instruction phase.
> > + * - 001b: QE is bit 1 of status register 2. It is set via Write Status with
> > + * two data bytes where bit 1 of the second byte is one.
> > + * [...]
> > + * Writing only one byte to the status register has the side-effect of
> > + * clearing status register 2, including the QE bit. The 100b code is
> > + * used if writing one byte to the status register does not modify
> > + * status register 2.
> > + * - 010b: QE is bit 6 of status register 1. It is set via Write Status with
> > + * one data byte where bit 6 is one.
> > + * [...]
> > + * - 011b: QE is bit 7 of status register 2. It is set via Write status
> > + * register 2 instruction 3Eh with one data byte where bit 7 is one.
> > + * [...]
> > + * The status register 2 is read using instruction 3Fh.
> > + * - 100b: QE is bit 1 of status register 2. It is set via Write Status with
> > + * two data bytes where bit 1 of the second byte is one.
> > + * [...]
> > + * In contrast to the 001b code, writing one byte to the status
> > + * register does not modify status register 2.
> > + * - 101b: QE is bit 1 of status register 2. Status register 1 is read using
> > + * Read Status instruction 05h. Status register2 is read using
> > + * instruction 35h. QE is set via Writ Status instruction 01h with
> > + * two data bytes where bit 1 of the second byte is one.
> > + * [...]
> > + */
> > +#define BFPT_DWORD15_QER_MASK GENMASK(22, 20)
> > +#define BFPT_DWORD15_QER_NONE (0x0UL << 20) /*
> Micron */
> > +#define BFPT_DWORD15_QER_SR2_BIT1_BUGGY (0x1UL << 20)
> > +#define BFPT_DWORD15_QER_SR1_BIT6 (0x2UL << 20) /*
> Macronix */
> > +#define BFPT_DWORD15_QER_SR2_BIT7 (0x3UL << 20)
> > +#define BFPT_DWORD15_QER_SR2_BIT1_NO_RD (0x4UL << 20)
> > +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /*
> Spansion */
> > +
> > +struct sfdp_bfpt {
> > + u32 dwords[BFPT_DWORD_MAX];
> > +};
> > +
> > +/* Fast Read settings. */
> > +
> > +struct sfdp_bfpt_read {
> > + /*
> > + * The <supported_bit> bit in <supported_dword> BFPT DWORD tells us
> > + * whether the Fast Read x-y-z command is supported.
> > + */
> > + u32 supported_dword;
> > + u32 supported_bit;
> > +
> > + /*
> > + * The half-word at offset <setting_shift> in <setting_dword> BFPT
> DWORD
>
> Should be <settings_dwoard> and <settings_dword>. I should fix in linux too.
>
Sure let me update it.
>
> > + * encodes the op code, the number of mode clocks and the number of
> wait
> > + * states to be used by Fast Read x-y-z command.
> > + */
> > + u32 settings_dword;
> > + u32 settings_shift;
> > +
> > + /* The SPI Read x-y-z command. */
> > + u8 read_cmd;
> > +
> > +};
> > +
> > +static const struct sfdp_bfpt_read sfdp_bfpt_reads[] = {
> > + /* Fast Read 1-1-2 */
> > + {
> > + BFPT_DWORD(1), BIT(16), /* Supported bit */
> > + BFPT_DWORD(4), 0, /* Settings */
> > + CMD_READ_DUAL_OUTPUT_FAST,
> > + },
> > +
> > + /* Fast Read 1-2-2 */
> > + {
> > + BFPT_DWORD(1), BIT(20), /* Supported bit */
> > + BFPT_DWORD(4), 16, /* Settings */
> > + CMD_READ_DUAL_IO_FAST,
> > + },
> > +
> > + /* Fast Read 2-2-2 */
> > + {
> > + BFPT_DWORD(5), BIT(0), /* Supported bit */
> > + BFPT_DWORD(6), 16, /* Settings */
> > + 0xFF, /* Not supported cmd */
> > + },
> > +
> > + /* Fast Read 1-1-4 */
> > + {
> > + BFPT_DWORD(1), BIT(22), /* Supported bit */
> > + BFPT_DWORD(3), 16, /* Settings */
> > + CMD_READ_QUAD_OUTPUT_FAST,
> > + },
> > +
> > + /* Fast Read 1-4-4 */
> > + {
> > + BFPT_DWORD(1), BIT(21), /* Supported bit */
> > + BFPT_DWORD(3), 0, /* Settings */
> > + CMD_READ_QUAD_IO_FAST,
> > + },
> > +
> > + /* Fast Read 4-4-4 */
> > + {
> > + BFPT_DWORD(5), BIT(4), /* Supported bit */
> > + BFPT_DWORD(7), 16, /* Settings */
> > + 0xFF, /* Not supported cmd */
> > + },
> > +};
> > +
> > +struct sfdp_bfpt_erase {
> > + /*
> > + * The half-word at offset <shift> in DWORD <dwoard> encodes the
>
> s/dwoard/dword/
> I should fix in linux too ;)
Let me update here.
>
>
> > + * op code and erase sector size to be used by Sector Erase commands.
> > + */
> > + u32 dword;
> > + u32 shift;
> > +};
> > +
> > +static const struct sfdp_bfpt_erase sfdp_bfpt_erases[] = {
> > + /* Erase Type 1 in DWORD8 bits[15:0] */
> > + {BFPT_DWORD(8), 0},
> > +
> > + /* Erase Type 2 in DWORD8 bits[31:16] */
> > + {BFPT_DWORD(8), 16},
> > +
> > + /* Erase Type 3 in DWORD9 bits[15:0] */
> > + {BFPT_DWORD(9), 0},
> > +
> > + /* Erase Type 4 in DWORD9 bits[31:16] */
> > + {BFPT_DWORD(9), 16},
> > +};
> > +
> > extern const struct spi_flash_info spi_flash_ids[];
> >
> > /* Send a single-byte command to the device and read the response */
> > diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
> > index 4d2e58e..2bc3af8 100644
> > --- a/drivers/mtd/spi/spi_flash.c
> > +++ b/drivers/mtd/spi/spi_flash.c
> > @@ -1107,6 +1107,417 @@ int spi_flash_decode_fdt(struct spi_flash *flash)
> > }
> > #endif /* CONFIG_IS_ENABLED(OF_CONTROL) */
> >
> > +/*
> > + * Serial Flash Discoverable Parameters (SFDP) parsing.
> > + */
> > +
> > +/**
> > + * spi_flash_read_sfdp() - read Serial Flash Discoverable Parameters.
> > + * @flash: pointer to a 'struct spi_flash'
> > + * @addr: offset in the SFDP area to start reading data from
> > + * @len: number of bytes to read
> > + * @buf: buffer where the SFDP data are copied into (dma-safe memory)
> > + *
> > + * Whatever the actual numbers of bytes for address and dummy cycles are
> > + * for (Fast) Read commands, the Read SFDP (5Ah) instruction is always
> > + * followed by a 3-byte address and 8 dummy clock cycles.
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spi_flash_read_sfdp(struct spi_flash *flash, u32 addr,
> > + size_t len, void *buf)
> > +{
> > + u8 addr_width, read_cmd, dummy_byte;
> > + int ret;
> > +
> > + ret = spi_claim_bus(flash->spi);
> > + if (ret) {
> > + debug("SF: Unable to claim SPI bus\n");
> > + return ret;
> > + }
> > +
> > + read_cmd = flash->read_cmd;
> > + addr_width = flash->addr_width;
> > + dummy_byte = flash->dummy_byte;
> > +
> > + flash->read_cmd = CMD_READ_SFDP;
> > + flash->addr_width = 3;
> > + flash->dummy_byte = 2;
>
> As written in the comment above, the Read SFDP instruction is always
> followed by a 3-byte address and 8 dummy cloc cycles, whatever SPI x-y-z
> protocol is used. So using 2 dummy bytes looks wrong to me.
>
> Using 8 dummy clock cycles means:
> * 1 byte for SPI 1-1-1
> * 2 bytes for SPI x-2-2
> * 4 bytes for SPI x-4-4
>
> More precisely, for SPI x-y-z and n dummy clock cycles, the number of dummy
> bits is y * n, hence the number of dummy bytes is (y * n) / 8.
>
You are right. It should be 1 byte.
Somehow, I was able to read SFDP data structure. May be I was lucky :)
> > +
> > + ret = spi_flash_cmd_read_ops(flash, addr, len, (u8 *)buf);
> > + if (ret < 0)
> > + goto read_err;
> > + ret = 0;
> > +
> > +read_err:
> > + flash->read_cmd = read_cmd;
> > + flash->addr_width = addr_width;
> > + flash->dummy_byte = dummy_byte;
> > +
> > + spi_release_bus(flash->spi);
> > + return ret;
> > +}
> > +
> > +/**
> > + * spi_flash_read_sfdp_dma_unsafe() - read Serial Flash Discoverable
> Parameters.
> > + * @flash: pointer to a 'struct spi_flash'
> > + * @addr: offset in the SFDP area to start reading data from
> > + * @len: number of bytes to read
> > + * @buf: buffer where the SFDP data are copied into
> > + *
> > + * Wrap spi_flash_read_sfdp() using a kmalloc'ed bounce buffer as @buf is
> now
> > + * not guaranteed to be dma-safe.
> > + *
> > + * Return: -ENOMEM if kmalloc() fails, the return code of
> spi_flash_read_sfdp()
> > + * otherwise.
> > + */
> > +static int spi_flash_read_sfdp_dma_unsafe(struct spi_flash *flash, u32 addr,
> > + size_t len, void *buf)
> > +{
>
> I don't know u-boot very well so I'm not sure whether this function is
> actually needed. In linux, both the MMU and the data cache are enabled on
> most architectures, then you have to be aware of non-contiguous buffers in
> physical memory or of the cache aliases issue before using DMA transfers.
> These issues can be addressed under Linux using kmalloc'ed memory.
>
> However, if I don't make a mistake, the MMU as well as the data cache are
> still disabled in u-boot, so is kmalloc() still needed here?
>
>
Not sure about other platforms.
But for NXP ARM platforms both MMU and data cache are enabled in u-boot.
So I believe kmalloc should be used.
> > + void *dma_safe_buf;
> > + int ret;
> > +
> > + dma_safe_buf = kmalloc(len, GFP_KERNEL);
> > + if (!dma_safe_buf)
> > + return -ENOMEM;
> > +
> > + ret = spi_flash_read_sfdp(flash, addr, len, dma_safe_buf);
> > + memcpy(buf, dma_safe_buf, len);
> > + kfree(dma_safe_buf);
> > +
> > + return ret;
> > +}
> > +
> > +/**
> > + * spi_flash_parse_bfpt() - read and parse the Basic Flash Parameter Table.
> > + * @flash: pointer to a 'struct spi_flash'
> > + * @info: pointer to a 'struct spi_flash_info'
> > + * @bfpt_header: pointer to the 'struct sfdp_parameter_header'
> describing
> > + * the Basic Flash Parameter Table length and version
> > + *
> > + * The Basic Flash Parameter Table is the main and only mandatory table as
> > + * defined by the SFDP (JESD216) specification.
> > + * It provides us with the total size (memory density) of the data array and
> > + * the number of address bytes for Fast Read, Page Program and Sector Erase
> > + * commands.
> > + * For Fast READ commands, it also gives the number of mode clock cycles
> and
> > + * wait states (regrouped in the number of dummy clock cycles) for each
> > + * supported instruction op code.
> > + * For Page Program, the page size is now available since JESD216 rev A,
> however
> > + * the supported instruction op codes are still not provided.
> > + * For Sector Erase commands, this table stores the supported instruction op
> > + * codes and the associated sector sizes.
> > + * Finally, the Quad Enable Requirements (QER) are also available since
> JESD216
> > + * rev A. The QER bits encode the manufacturer dependent procedure to be
> > + * executed to set the Quad Enable (QE) bit in some internal register of the
> > + * Quad SPI memory. Indeed the QE bit, when it exists, must be set before
> > + * sending any Quad SPI command to the memory. Actually, setting the QE bit
> > + * tells the memory to reassign its WP# and HOLD#/RESET# pins to functions
> IO2
> > + * and IO3 hence enabling 4 (Quad) I/O lines.
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spi_flash_parse_bfpt(struct spi_flash *flash,
> > + const struct sfdp_parameter_header
> *bfpt_header)
> > +{
> > + struct spi_slave *spi = flash->spi;
> > + struct sfdp_bfpt bfpt;
> > + size_t len;
> > + int i, err;
> > + u32 addr;
> > + u16 half;
> > +
> > + /* JESD216 Basic Flash Parameter Table length is at least 9 DWORDs. */
> > + if (bfpt_header->length < BFPT_DWORD_MAX_JESD216)
> > + return -EINVAL;
> > +
> > + /* Read the Basic Flash Parameter Table. */
> > + len = min_t(size_t, sizeof(bfpt),
> > + bfpt_header->length * sizeof(u32));
> > + addr = SFDP_PARAM_HEADER_PTP(bfpt_header);
> > + memset(&bfpt, 0, sizeof(bfpt));
> > + err = spi_flash_read_sfdp_dma_unsafe(flash, addr, len, &bfpt);
> > + if (err < 0)
> > + return err;
> > +
> > + /* Fix endianness of the BFPT DWORDs. */
> > + for (i = 0; i < BFPT_DWORD_MAX; i++)
> > + bfpt.dwords[i] = le32_to_cpu(bfpt.dwords[i]);
> > +
> > + /* Number of address bytes. */
> > + switch (bfpt.dwords[BFPT_DWORD(1)] &
> BFPT_DWORD1_ADDRESS_BYTES_MASK) {
> > + case BFPT_DWORD1_ADDRESS_BYTES_3_ONLY:
> > + flash->addr_width = 3;
> > + break;
> > +
> > + case BFPT_DWORD1_ADDRESS_BYTES_4_ONLY:
> > + flash->addr_width = 4;
> > + break;
> > +
> > + case BFPT_DWORD1_ADDRESS_BYTES_3_OR_4:
> > + flash->addr_width = 3;
> > + break;
> > +
> > + default: /* Reserved */
> > + break;
> > + }
> > +
> > + /* Flash Memory Density (in bits). */
> > + flash->size = bfpt.dwords[BFPT_DWORD(2)];
> > + if (flash->size & BIT(31)) {
> > + flash->size &= ~BIT(31);
> > +
> > + /*
> > + * Prevent overflows on params->size. Anyway, a NOR of 2^64
> > + * bits is unlikely to exist so this error probably means
> > + * the BFPT we are reading is corrupted/wrong.
> > + */
> > + if (flash->size > 63)
> > + return -EINVAL;
> > +
> > + flash->size = 1ULL << flash->size;
> > + } else {
> > + flash->size++;
> > + }
> > + flash->size >>= 3; /* Convert to bytes. */
> > + flash->size <<= flash->shift;
> > +
> > + /* Fast Read settings. */
> > + for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_reads); i++) {
> > + const struct sfdp_bfpt_read *rd = &sfdp_bfpt_reads[i];
> > + u32 mode_dummy_cycle;
> > +
> > + if (!(bfpt.dwords[rd->supported_dword] & rd->supported_bit))
> > + continue;
> > +
> > + half = bfpt.dwords[rd->settings_dword] >> rd->settings_shift;
> > + mode_dummy_cycle = ((half >> 5) & 0x07) + ((half >> 0) & 0x1f);
> > +
> > + flash->dummy_byte = mode_dummy_cycle / 8;
> > +
> > + switch (rd->read_cmd) {
> > + case CMD_READ_QUAD_OUTPUT_FAST:
>
> I don't agree: I guess CMD_READ_QUAD_OUTPUT_FAST is actually Fast Read
> 1-1-4, then y = 1, not 4. The formula below computing the number of dummy
> bytes should be applied for Fast Read x-4-4 only.
>
I will try to modify code.
> > + case CMD_READ_QUAD_IO_FAST:
> > + if (spi->mode & SPI_RX_QUAD) {
>
> AFAIK, testing SPI_RX_QUAD alone in spi->mode only tells that the
> SPI controller can read data on four I/O lines but it doesn't tell us
> whether this controller can also write data (op code and/or
> address,mode,dummy data) on four I/O lines too.
>
> So SPI_RX_QUAD only allow SPI 1-1-4 but you should test SPI_TX_QUAD to
> allow SPI 1-4-4 or SPI 4-4-4.
>
Let me update it.
> > + flash->read_cmd = rd->read_cmd;
> > +
> > + if (flash->read_cmd ==
> CMD_READ_QUAD_IO_FAST)
> > + flash->dummy_byte =
> > + (mode_dummy_cycle * 4) / 8;
> > + }
> > + break;
> > +
> > + case CMD_READ_DUAL_OUTPUT_FAST:
>
> Same mistake here: the formula only applies to Fast Read x-2-2.
> All Fast Read 1-1-z use directly flash->dummy_byte = mode_dummy_cycle / 8;
>
Let me update the code
> > + case CMD_READ_DUAL_IO_FAST:
> > + if (spi->mode & SPI_RX_DUAL &&
> > + !(spi->mode & SPI_RX_QUAD)) {
>
> Please check SPI_TX_DUAL as well.
sure
>
> > + flash->read_cmd = rd->read_cmd;
> > +
> > + if (flash->read_cmd ==
> CMD_READ_DUAL_IO_FAST)
> > + flash->dummy_byte =
> > + (mode_dummy_cycle * 2) / 8;
> > + }
> > +
> > + break;
> > +
> > + default:
> > + debug("read_cmd=0x%x is not supported\n", rd-
> >read_cmd);
> > + break;
> > + }
> > + }
> > +
> > + if (spi->mode & SPI_RX_SLOW &&
> > + !(spi->mode & (SPI_RX_QUAD | SPI_RX_DUAL)))
> > + flash->read_cmd = CMD_READ_ARRAY_SLOW;
> > + else
> > + flash->read_cmd = CMD_READ_ARRAY_FAST;
>
> I don't really understand the above chunk.
>
I was trying to set read_cmd as "READ i.e. 0x03" instead of FAST READ i.e. 0x0b.
It may be required for controller which want to support slow read.
Logic should be something like this
if (flash->cmd == CMD_READ_ARRAY_FAST && spi->mode & SPI_RX_SLOW)
flash->read_cmd = CMD_READ_ARRAY_SLOW;
> > +
> > + /* Sector Erase settings. */
> > + for (i = 0; i < ARRAY_SIZE(sfdp_bfpt_erases); i++) {
> > + const struct sfdp_bfpt_erase *er = &sfdp_bfpt_erases[i];
> > + u32 erasesize;
> > + u8 opcode;
> > +
> > + half = bfpt.dwords[er->dword] >> er->shift;
> > + erasesize = half & 0xff;
> > +
> > + /* erasesize == 0 means this Erase Type is not supported. */
> > + if (!erasesize)
> > + continue;
> > +
> > + erasesize = 1U << erasesize;
> > + opcode = (half >> 8) & 0xff;
> > +#ifdef CONFIG_SPI_FLASH_USE_4K_SECTORS
> > + if (erasesize == SZ_4K) {
> > + flash->erase_cmd = opcode;
> > + flash->erase_size = erasesize;
> > + break;
> > + }
> > +#endif
> > + if (!flash->erase_size || flash->erase_size < erasesize) {
> > + flash->erase_cmd = opcode;
> > + flash->erase_size = erasesize;
> > + }
> > + }
> > +
> > + flash->erase_size <<= flash->shift;
> > +
> > + flash->sector_size = flash->erase_size;
> > +
> > + /* Stop here if not JESD216 rev A or later. */
> > + if (bfpt_header->length < BFPT_DWORD_MAX)
> > + return 0;
> > +
> > + /* Page size: this field specifies 'N' so the page size = 2^N bytes. */
> > + flash->page_size = bfpt.dwords[BFPT_DWORD(11)];
> > + flash->page_size &= BFPT_DWORD11_PAGE_SIZE_MASK;
> > + flash->page_size >>= BFPT_DWORD11_PAGE_SIZE_SHIFT;
> > + flash->page_size = 1U << flash->page_size;
> > + flash->page_size <<= flash->shift;
> > +
> > + if ((flash->read_cmd == CMD_READ_QUAD_OUTPUT_FAST) ||
> > + (flash->read_cmd == CMD_READ_QUAD_IO_FAST) ||
> > + (flash->write_cmd == CMD_QUAD_PAGE_PROGRAM)) {
> > + /* Quad Enable Requirements. */
> > + switch (bfpt.dwords[BFPT_DWORD(15)] &
> BFPT_DWORD15_QER_MASK) {
> > + case BFPT_DWORD15_QER_NONE:
> > + break;
> > +
> > + case BFPT_DWORD15_QER_SR2_BIT1_BUGGY:
> > + case BFPT_DWORD15_QER_SR2_BIT1_NO_RD:
> > + break;
> > +
> > + case BFPT_DWORD15_QER_SR1_BIT6:
> > +#ifdef CONFIG_SPI_FLASH_MACRONIX
> > + macronix_quad_enable(flash);
> > +#endif
> > + break;
> > +
> > + case BFPT_DWORD15_QER_SR2_BIT7:
> > +
> > + break;
> > +
> > + case BFPT_DWORD15_QER_SR2_BIT1:
> > +#if defined(CONFIG_SPI_FLASH_SPANSION) ||
> defined(CONFIG_SPI_FLASH_WINBOND)
> > + spansion_quad_enable(flash);
> > +#endif
> > + break;
> > +
> > + default:
> > + return -EINVAL;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +/**
> > + * spi_flash_parse_sfdp() - parse the Serial Flash Discoverable Parameters.
> > + * @flash: pointer to a 'struct spi_flash'
> > + *
> > + * The Serial Flash Discoverable Parameters are described by the JEDEC
> JESD216
> > + * specification. This is a standard which tends to supported by almost all
> > + * (Q)SPI memory manufacturers. Those hard-coded tables allow us to learn at
> > + * runtime the main parameters needed to perform basic SPI flash operations
> such
> > + * as Fast Read, Page Program or Sector Erase commands.
> > + *
> > + * Return: 0 on success, -errno otherwise.
> > + */
> > +static int spi_flash_parse_sfdp(struct spi_flash *flash)
> > +{
> > + const struct sfdp_parameter_header *param_header, *bfpt_header;
> > + struct sfdp_parameter_header *param_headers = NULL;
> > + struct sfdp_header header;
> > + size_t psize;
> > + int i, err;
> > +
> > + /* Get the SFDP header. */
> > + err = spi_flash_read_sfdp_dma_unsafe(flash, 0, sizeof(header),
> &header);
> > + if (err < 0)
> > + return err;
> > +
> > + /* Check the SFDP header version. */
> > + if (le32_to_cpu(header.signature) != SFDP_SIGNATURE ||
> > + header.major != SFDP_JESD216_MAJOR ||
> > + header.minor < SFDP_JESD216_MINOR)
> > + return -EINVAL;
> > +
> > + /*
> > + * Verify that the first and only mandatory parameter header is a
> > + * Basic Flash Parameter Table header as specified in JESD216.
> > + */
> > + bfpt_header = &header.bfpt_header;
> > + if (SFDP_PARAM_HEADER_ID(bfpt_header) != SFDP_BFPT_ID ||
> > + bfpt_header->major != SFDP_JESD216_MAJOR)
> > + return -EINVAL;
> > +
> > + /*
> > + * Allocate memory then read all parameter headers with a single
> > + * Read SFDP command. These parameter headers will actually be parsed
> > + * twice: a first time to get the latest revision of the basic flash
> > + * parameter table, then a second time to handle the supported optional
> > + * tables.
> > + * Hence we read the parameter headers once for all to reduce the
> > + * processing time. Also we use kmalloc() instead of devm_kmalloc()
> > + * because we don't need to keep these parameter headers: the
> allocated
> > + * memory is always released with kfree() before exiting this function.
> > + */
> > + if (header.nph) {
> > + psize = header.nph * sizeof(*param_headers);
> > +
> > + param_headers = kmalloc(psize, GFP_KERNEL);
> > + if (!param_headers)
> > + return -ENOMEM;
> > +
> > + err = spi_flash_read_sfdp(flash, sizeof(header),
> > + psize, param_headers);
> > + if (err < 0) {
> > + debug("SF: failed to read SFDP parameter headers\n");
> > + goto exit;
> > + }
> > + }
> > +
> > + /*
> > + * Check other parameter headers to get the latest revision of
> > + * the basic flash parameter table.
> > + */
> > + for (i = 0; i < header.nph; i++) {
> > + param_header = ¶m_headers[i];
> > +
> > + if (SFDP_PARAM_HEADER_ID(param_header) == SFDP_BFPT_ID
> &&
> > + param_header->major == SFDP_JESD216_MAJOR &&
> > + (param_header->minor > bfpt_header->minor ||
> > + (param_header->minor == bfpt_header->minor &&
> > + param_header->length > bfpt_header->length)))
> > + bfpt_header = param_header;
> > + }
> > +
> > + err = spi_flash_parse_bfpt(flash, bfpt_header);
> > + if (err)
> > + goto exit;
> > +
> > + /* Parse other parameter headers. */
> > + for (i = 0; i < header.nph; i++) {
> > + param_header = ¶m_headers[i];
> > +
> > + switch (SFDP_PARAM_HEADER_ID(param_header)) {
> > + case SFDP_SECTOR_MAP_ID:
> > + debug("non-uniform erase sector maps not
> supported\n");
> > + break;
> > +
> > + default:
> > + break;
> > + }
> > +
> > + if (err)
> > + goto exit;
> > + }
> > +
> > +exit:
> > + kfree(param_headers);
> > + return err;
> > +}
> > +
> > int spi_flash_scan(struct spi_flash *flash)
> > {
> > struct spi_slave *spi = flash->spi;
> > @@ -1294,9 +1705,12 @@ int spi_flash_scan(struct spi_flash *flash)
> > puts(" Full access #define CONFIG_SPI_FLASH_BAR\n");
> > }
> > #endif
> > -
> > flash->addr_width = SPI_FLASH_3B_ADDR_LEN;
> >
> > + /* Override the parameters with data read from SFDP tables. */
> > + if (info->flags & (FLASH_SFDP))
> > + spi_flash_parse_sfdp(flash);
> > +
>
> What if the SFDP tables are corrupted or invalid in some but not all memory
> parts? You should recover and use the legacy settings then.
>
> Some SPI NOR manufacturers share the JEDEC ID between many revisions of
> their memory. Sometime, early revisions have non JESD216B compliant tables
> programmed in their SFDP area then the issue is fixed in later revisions.
>
Agree. Let me fix it.
--pk
More information about the U-Boot
mailing list