Hi,
Unfortunately thats a problem with U-Boot spi_flash layer today. Many newer flashes like Micron's mt35x series dont support BAR registers. Accessing above 16MB is broken for such flashes today.
Also, initially I wanted to make SFDP optional so that none of the drivers
are affected
and one could enable it using a flag as per requirement. Shall we get back
to making
this optional (similar to v1 of this patch)?
How does that help? With opt-in approach you mark some flash devices as SFDP capable, but you still don't update opcodes, thus breaking boards that have such flash devices. So, porting full bfpt parsing and always using opcode to 4 byte addressing opcodes when available will be needed. Thus you would end up with SFDP as default instead of opt-in.
Hi Vignesh
Porting full bftp parsing requires time. I am seeing a conflict with a patch of yours https://patchwork.ozlabs.org/cover/1012146/
Yes, I ended up porting all of Linux SPI NOR features including SFDP and 4 byte addressing.
It seem above RFC patch will bring SFDP to U-boot. Do you still want me to put effort in porting SFDP in current version?
No need to post this series as my patches including this change. I would appreciate any review/test of my series.
You can find Patch v1 here: https://patchwork.ozlabs.org/cover/1012146/ (Please drop patch 9/16 while testing). Thanks!
Regards Vignesh
Regards Rajat
Regards Rajat
Additionally, there are flash chips with a 3-/4-byte mode change
instruction.
But using 4-byte opcodes should be preferred.
Simon
In particular you are not parsing entire bfpt like in [1] and will break flash operations
[1]https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2F
elixir.bootlin.com%2Flinux%2Flatest%2Fsource%2Fdrivers%2Fmtd%2Fspi-
nor
%2Fspi-
nor.c%23L2259&data=02%7C01%7Crajat.srivastava%40nxp.com%7Ca
cee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92cd99c5c30163 5
%7C0
%7C0%7C636779606107591743&sdata=UACl919%2BMQmdtBo5GUTAy
M428u1bT1bU
rXuX2GsSjb0%3D&reserved=0
Regards Vignesh
Regards Rajat
> > Regards > Vigensh > > >> + default: >> + 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 flash->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. */ >> + >> + /* 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; >> + >> + 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. >> + * >> + * 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(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) >> + 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 >> + */ >> + if (header.nph) { >> + psize = header.nph * sizeof(*param_headers); >> + >> + param_headers = malloc(psize); >> + if (!param_headers) >> + return -ENOMEM; >> + >> + err = spi_flash_read_sfdp(flash, sizeof(header), >> + psize, param_headers); >> + if (err < 0) { >> + dev_err(dev, "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; >> + >> +exit: >> + free(param_headers); >> + return err; >> +} >> + >> static int set_quad_mode(struct spi_flash *flash, >> const struct spi_flash_info *info) { @@ >> -1130,7 > +1366,7 @@ int >> spi_flash_scan(struct spi_flash *flash) { >> struct spi_slave *spi = flash->spi; >> const struct spi_flash_info *info = NULL; >> - int ret; >> + int ret, sfdp = 0; >> >> info = spi_flash_read_id(flash); >> if (IS_ERR_OR_NULL(info)) >> @@ -1196,9 +1432,28 @@ int spi_flash_scan(struct spi_flash *flash) >> } >> #endif >> >> + spi->flash = flash; >> + flash->addrwd_3_in_use = false; >> + >> + /* Read Serial Flash Discoverable Parameters and initialize >> + * the following parameters of flash: >> + * 1. Flash size >> + * 2. Page size >> + * 3. Address width to be used for commands >> + */ >> + if (!(info->flags & SPI_FLASH_SKIP_SFDP)) { >> + flash->size = 0; >> + sfdp = spi_flash_parse_sfdp(flash); >> + if (sfdp < 0) >> + debug("Unable to get SFDP information\n"); >> + } >> + >> /* Compute the flash size */ >> flash->shift = (flash->dual_flash & SF_DUAL_PARALLEL_FLASH) ? 1 : > 0; >> - flash->page_size = info->page_size; >> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) { >> + flash->page_size = info->page_size; >> + flash->addr_width = SPI_FLASH_3B_ADDR_LEN; >> + } >> /* >> * The Spansion S25FS512S, S25FL032P and S25FL064P have 256b > pages, >> * yet use the 0x4d00 Extended JEDEC code. The rest of the >> Spansion @@ -1213,7 +1468,10 @@ int spi_flash_scan(struct
spi_flash
*flash)
>> } >> flash->page_size <<= flash->shift; >> flash->sector_size = info->sector_size << flash->shift; >> - flash->size = flash->sector_size * info->n_sectors << flash->shift; >> + if (info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) { >> + flash->size = flash->sector_size * info->n_sectors << >> + flash->shift; >> + } >> #ifdef CONFIG_SF_DUAL_FLASH >> if (flash->dual_flash & SF_DUAL_STACKED_FLASH) >> flash->size <<= 1; >> @@ -1312,9 +1570,10 @@ int spi_flash_scan(struct spi_flash *flash) >> #endif >> >> #ifndef CONFIG_SPI_FLASH_BAR >> - if (((flash->dual_flash == SF_SINGLE_FLASH) && >> - (flash->size > SPI_FLASH_16MB_BOUN)) || >> - ((flash->dual_flash > SF_SINGLE_FLASH) && >> + if ((info->flags & SPI_FLASH_SKIP_SFDP || sfdp < 0) && >> + (flash->dual_flash == SF_SINGLE_FLASH && >> + flash->size > SPI_FLASH_16MB_BOUN) || >> + (flash->dual_flash > SF_SINGLE_FLASH && >> (flash->size > SPI_FLASH_16MB_BOUN << 1))) { >> puts("SF: Warning - Only lower 16MiB accessible,"); >> puts(" Full access #define CONFIG_SPI_FLASH_BAR\n"); >> diff - > -git >> a/include/spi.h b/include/spi.h index 938627bc01..7189e60581 >> 100644 >> --- a/include/spi.h >> +++ b/include/spi.h >> @@ -10,6 +10,7 @@ >> #define _SPI_H_ >> >> #include <common.h> >> +#include <spi_flash.h> >> >> /* SPI mode flags */ >> #define SPI_CPHA BIT(0) /* clock phase */ >> @@ -103,6 +104,7 @@ struct spi_slave { >> unsigned int bus; >> unsigned int cs; >> #endif >> + struct spi_flash *flash; >> uint mode; >> unsigned int wordlen; >> unsigned int max_read_size; >> diff --git a/include/spi_flash.h b/include/spi_flash.h index >> 0ec98fb55d..6558a4a1d5 100644 >> --- a/include/spi_flash.h >> +++ b/include/spi_flash.h >> @@ -47,6 +47,9 @@ struct spi_slave; >> * @read_cmd: Read cmd - Array Fast, Extn read and quad > read. >> * @write_cmd: Write cmd - page and quad program. >> * @dummy_byte: Dummy cycles for read operation. >> + * @cmd_len: Total length of command. >> + * @addr_width: Number of address bytes. >> + * @addrwd_3_in_use: Flag to send command in 3-byte address > mode. >> * @memory_map: Address of read-only SPI flash access >> * @flash_lock: lock a region of the SPI Flash >> * @flash_unlock: unlock a region of the SPI Flash @@ -82,6 >> +85,9 @@ struct spi_flash { >> u8 read_cmd; >> u8 write_cmd; >> u8 dummy_byte; >> + u8 cmd_len; >> + u8 addr_width; >> + bool addrwd_3_in_use; >> >> void *memory_map; >> >> @@ -107,6 +113,120 @@ struct spi_flash { #endif }; >> >> +/* >> + * Serial Flash Discoverable Parameter Headers */ 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; >> +}; >> + >> +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; >> +}; >> + >> +#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 >> + >> +/* 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 <<
>> +#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 <<
>> +#define BFPT_DWORD15_QER_SR2_BIT1 (0x5UL << 20) /* > Spansion */ >> + >> +struct sfdp_bfpt { >> + u32 dwords[BFPT_DWORD_MAX]; >> +}; >> + >> struct dm_spi_flash_ops { >> int (*read)(struct udevice *dev, u32 offset, size_t len, void *buf); >> int (*write)(struct udevice *dev, u32 offset, size_t len, >>
-- Regards Vignesh _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de
https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flis
ts.denx.de%2Flistinfo%2Fu-
boot&data=02%7C01%7Crajat.srivastava%40n
xp.com%7Cacee153f50194bba71c808d64bad8d2e%7C686ea1d3bc2b4c6fa92c
d99c5c
301635%7C0%7C0%7C636779606107591743&sdata=u9H5PMJiRNVKyyFI7
y5eCmDx
0KPmAHXvF8kusaJVUdM%3D&reserved=0