| Message ID | 20170912170931.29974-1-marek.vasut+renesas@gmail.com |
|---|---|
| State | Accepted |
| Commit | 72443c7f7d2174903e73ee88dcb4364e0387bbb2 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | [U-Boot] mtd: cfi: Add support for status register polling | expand |
On 12.09.2017 19:09, Marek Vasut wrote: > The status register is optional in the AMD command sets, but it's > presence can be checked by reading out CFI table entry 0xc bit 0. > If the register is present, prefer using it's bit 7 to determine > if the flash is busy over reading the flash ; this is needed ie. > on Hyperflash memories. > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Applied to u-boot-cfi-flash/master. Thanks, Stefan
On 09/12/2017 10:09 AM, Marek Vasut wrote: > The status register is optional in the AMD command sets, but it's > presence can be checked by reading out CFI table entry 0xc bit 0. > If the register is present, prefer using it's bit 7 to determine > if the flash is busy over reading the flash ; this is needed ie. > on Hyperflash memories. > > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > --- Marek, Some of our boards failed. I traced to this commit. Reverting this commit fixes the issue. I happen to have two boards with slightly different flash chip. One works and the other doesn't. The working board has => fli Bank # 1: CFI conformant flash (16 x 16) Size: 128 MB in 1024 Sectors AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2801 Erase timeout: 2048 ms, write timeout: 1 ms Buffer write timeout: 3 ms, buffer size: 512 bytes The failing board has => fli Bank # 1: CFI conformant flash (16 x 16) Size: 128 MB in 1024 Sectors AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2801 Erase timeout: 4096 ms, write timeout: 1 ms Buffer write timeout: 3 ms, buffer size: 64 bytes Can you investigate? York
On 11/17/2017 05:43 PM, York Sun wrote: > On 09/12/2017 10:09 AM, Marek Vasut wrote: >> The status register is optional in the AMD command sets, but it's >> presence can be checked by reading out CFI table entry 0xc bit 0. >> If the register is present, prefer using it's bit 7 to determine >> if the flash is busy over reading the flash ; this is needed ie. >> on Hyperflash memories. >> >> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >> --- > > Marek, > > Some of our boards failed. I traced to this commit. Reverting this > commit fixes the issue. I happen to have two boards with slightly > different flash chip. One works and the other doesn't. I can't since I don't have the board with such a chip ... Which chip is that ? Mine is Spansion S25KL256 hyperflash. > The working board has > > => fli > > Bank # 1: CFI conformant flash (16 x 16) Size: 128 MB in 1024 Sectors > AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2801 > Erase timeout: 2048 ms, write timeout: 1 ms > Buffer write timeout: 3 ms, buffer size: 512 bytes > > The failing board has > > => fli > > Bank # 1: CFI conformant flash (16 x 16) Size: 128 MB in 1024 Sectors > AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2801 > Erase timeout: 4096 ms, write timeout: 1 ms > Buffer write timeout: 3 ms, buffer size: 64 bytes > > Can you investigate? > > York >
On 11/17/2017 08:02 PM, Marek Vasut wrote: > On 11/17/2017 05:43 PM, York Sun wrote: >> On 09/12/2017 10:09 AM, Marek Vasut wrote: >>> The status register is optional in the AMD command sets, but it's >>> presence can be checked by reading out CFI table entry 0xc bit 0. >>> If the register is present, prefer using it's bit 7 to determine >>> if the flash is busy over reading the flash ; this is needed ie. >>> on Hyperflash memories. >>> >>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >>> --- >> >> Marek, >> >> Some of our boards failed. I traced to this commit. Reverting this >> commit fixes the issue. I happen to have two boards with slightly >> different flash chip. One works and the other doesn't. > > I can't since I don't have the board with such a chip ... > Which chip is that ? Mine is Spansion S25KL256 hyperflash. S26KL256S (26 instead of 25), sorry. > >> The working board has >> >> => fli >> >> Bank # 1: CFI conformant flash (16 x 16) Size: 128 MB in 1024 Sectors >> AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2801 >> Erase timeout: 2048 ms, write timeout: 1 ms >> Buffer write timeout: 3 ms, buffer size: 512 bytes >> >> The failing board has >> >> => fli >> >> Bank # 1: CFI conformant flash (16 x 16) Size: 128 MB in 1024 Sectors >> AMD Standard command set, Manufacturer ID: 0x01, Device ID: 0x227E2801 >> Erase timeout: 4096 ms, write timeout: 1 ms >> Buffer write timeout: 3 ms, buffer size: 64 bytes >> >> Can you investigate? >> >> York >> > >
On Nov 17, 2017, at 11:04, Marek Vasut <marek.vasut@gmail.com<mailto:marek.vasut@gmail.com>> wrote:
On 11/17/2017 08:02 PM, Marek Vasut wrote:
On 11/17/2017 05:43 PM, York Sun wrote:
On 09/12/2017 10:09 AM, Marek Vasut wrote:
The status register is optional in the AMD command sets, but it's
presence can be checked by reading out CFI table entry 0xc bit 0.
If the register is present, prefer using it's bit 7 to determine
if the flash is busy over reading the flash ; this is needed ie.
on Hyperflash memories.
Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com<mailto:marek.vasut+renesas@gmail.com>>
---
Marek,
Some of our boards failed. I traced to this commit. Reverting this
commit fixes the issue. I happen to have two boards with slightly
different flash chip. One works and the other doesn't.
I can't since I don't have the board with such a chip ...
Which chip is that ? Mine is Spansion S25KL256 hyperflash.
S26KL256S (26 instead of 25), sorry.
I have to wait for the team to confirm. The board is remote to me.
York
On 11/17/2017 11:04 AM, Marek Vasut wrote: > On 11/17/2017 08:02 PM, Marek Vasut wrote: >> On 11/17/2017 05:43 PM, York Sun wrote: >>> On 09/12/2017 10:09 AM, Marek Vasut wrote: >>>> The status register is optional in the AMD command sets, but it's >>>> presence can be checked by reading out CFI table entry 0xc bit 0. >>>> If the register is present, prefer using it's bit 7 to determine >>>> if the flash is busy over reading the flash ; this is needed ie. >>>> on Hyperflash memories. >>>> >>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >>>> --- >>> >>> Marek, >>> >>> Some of our boards failed. I traced to this commit. Reverting this >>> commit fixes the issue. I happen to have two boards with slightly >>> different flash chip. One works and the other doesn't. >> >> I can't since I don't have the board with such a chip ... >> Which chip is that ? Mine is Spansion S25KL256 hyperflash. > > S26KL256S (26 instead of 25), sorry. > My local board has chip S29GL01GS11TFIV1. The remote board has identical manufacture ID and device ID. I guess they are the same part (maybe different speed grade though). I haven't heard from the team who can visually check the part number for me. York
On 11/17/2017 12:59 PM, York Sun wrote: > On 11/17/2017 11:04 AM, Marek Vasut wrote: >> On 11/17/2017 08:02 PM, Marek Vasut wrote: >>> On 11/17/2017 05:43 PM, York Sun wrote: >>>> On 09/12/2017 10:09 AM, Marek Vasut wrote: >>>>> The status register is optional in the AMD command sets, but it's >>>>> presence can be checked by reading out CFI table entry 0xc bit 0. >>>>> If the register is present, prefer using it's bit 7 to determine >>>>> if the flash is busy over reading the flash ; this is needed ie. >>>>> on Hyperflash memories. >>>>> >>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >>>>> --- >>>> >>>> Marek, >>>> >>>> Some of our boards failed. I traced to this commit. Reverting this >>>> commit fixes the issue. I happen to have two boards with slightly >>>> different flash chip. One works and the other doesn't. >>> >>> I can't since I don't have the board with such a chip ... >>> Which chip is that ? Mine is Spansion S25KL256 hyperflash. >> >> S26KL256S (26 instead of 25), sorry. >> > > My local board has chip S29GL01GS11TFIV1. The remote board has identical > manufacture ID and device ID. I guess they are the same part (maybe > different speed grade though). I haven't heard from the team who can > visually check the part number for me. > Marek, I got an email from our hardware team. At some point, our vendor had two parts with the same device ID. The other one is S29GL01GP13TFIV1. According to Cypress migration document, the old part doesn't support status register feature. I understand you already check lower software bits. But I am afraid this register is not valid for old parts which don't support this feature. I checked the datasheet of S29GL01GP. It has no description of offset 0xC. Reading from the hardware, I got 0xFF. If there is no "valid" bit to indicate we can use lower software bit regsiter, it is useless. Comparing the logs Failing board Flash: York debug: manuId = 0x1 York debug: lsbits = 0xff York debug: read offset 0xE as 0x28 York debug: read offset 0xF as 0x1 128 MiB Working board Flash: York debug: manuId = 0x1 York debug: lsbits = 0x3 York debug: read offset 0xE as 0x28 York debug: read offset 0xF as 0x1 128 MiB If there is no other way to know the "lower software bits" register is valid, then we have to abandon this feature. York
On 11/17/2017 10:48 PM, York Sun wrote: > On 11/17/2017 12:59 PM, York Sun wrote: >> On 11/17/2017 11:04 AM, Marek Vasut wrote: >>> On 11/17/2017 08:02 PM, Marek Vasut wrote: >>>> On 11/17/2017 05:43 PM, York Sun wrote: >>>>> On 09/12/2017 10:09 AM, Marek Vasut wrote: >>>>>> The status register is optional in the AMD command sets, but it's >>>>>> presence can be checked by reading out CFI table entry 0xc bit 0. >>>>>> If the register is present, prefer using it's bit 7 to determine >>>>>> if the flash is busy over reading the flash ; this is needed ie. >>>>>> on Hyperflash memories. >>>>>> >>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >>>>>> --- >>>>> >>>>> Marek, >>>>> >>>>> Some of our boards failed. I traced to this commit. Reverting this >>>>> commit fixes the issue. I happen to have two boards with slightly >>>>> different flash chip. One works and the other doesn't. >>>> >>>> I can't since I don't have the board with such a chip ... >>>> Which chip is that ? Mine is Spansion S25KL256 hyperflash. >>> >>> S26KL256S (26 instead of 25), sorry. >>> >> >> My local board has chip S29GL01GS11TFIV1. The remote board has identical >> manufacture ID and device ID. I guess they are the same part (maybe >> different speed grade though). I haven't heard from the team who can >> visually check the part number for me. >> > > Marek, > > I got an email from our hardware team. At some point, our vendor had two > parts with the same device ID. The other one is S29GL01GP13TFIV1. > According to Cypress migration document, the old part doesn't support > status register feature. I understand you already check lower software > bits. Which lower software bits ? > But I am afraid this register is not valid for old parts which > don't support this feature. I checked the datasheet of S29GL01GP. It has > no description of offset 0xC. Reading from the hardware, I got 0xFF. If > there is no "valid" bit to indicate we can use lower software bit > regsiter, it is useless. I am happy to dump you whatever you need from the hyperflash (which is in fact a CFI NOR flash with different interface), if that helps us identify how to improve the condition to discern flashes which do and do not support this polling feature. Since CFI is a standard, there must be a way to do it. > Comparing the logs > > Failing board > > Flash: York debug: manuId = 0x1 > York debug: lsbits = 0xff > York debug: read offset 0xE as 0x28 > York debug: read offset 0xF as 0x1 > 128 MiB > > > Working board > > Flash: York debug: manuId = 0x1 > York debug: lsbits = 0x3 > York debug: read offset 0xE as 0x28 > York debug: read offset 0xF as 0x1 > 128 MiB > > If there is no other way to know the "lower software bits" register is > valid, then we have to abandon this feature. We cannot because then that breaks hyperflash support, so we need to figure out how to improve the condition to discern the flashes. > York >
On 11/18/2017 02:24 AM, Marek Vasut wrote: > On 11/17/2017 10:48 PM, York Sun wrote: >> On 11/17/2017 12:59 PM, York Sun wrote: >>> On 11/17/2017 11:04 AM, Marek Vasut wrote: >>>> On 11/17/2017 08:02 PM, Marek Vasut wrote: >>>>> On 11/17/2017 05:43 PM, York Sun wrote: >>>>>> On 09/12/2017 10:09 AM, Marek Vasut wrote: >>>>>>> The status register is optional in the AMD command sets, but it's >>>>>>> presence can be checked by reading out CFI table entry 0xc bit 0. >>>>>>> If the register is present, prefer using it's bit 7 to determine >>>>>>> if the flash is busy over reading the flash ; this is needed ie. >>>>>>> on Hyperflash memories. >>>>>>> >>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >>>>>>> --- >>>>>> >>>>>> Marek, >>>>>> >>>>>> Some of our boards failed. I traced to this commit. Reverting this >>>>>> commit fixes the issue. I happen to have two boards with slightly >>>>>> different flash chip. One works and the other doesn't. >>>>> >>>>> I can't since I don't have the board with such a chip ... >>>>> Which chip is that ? Mine is Spansion S25KL256 hyperflash. >>>> >>>> S26KL256S (26 instead of 25), sorry. >>>> >>> >>> My local board has chip S29GL01GS11TFIV1. The remote board has identical >>> manufacture ID and device ID. I guess they are the same part (maybe >>> different speed grade though). I haven't heard from the team who can >>> visually check the part number for me. >>> >> >> Marek, >> >> I got an email from our hardware team. At some point, our vendor had two >> parts with the same device ID. The other one is S29GL01GP13TFIV1. >> According to Cypress migration document, the old part doesn't support >> status register feature. I understand you already check lower software >> bits. > > Which lower software bits ? Register at offset 0xC. This register is not part of CFI. But there is CFI extention, explained below. > >> But I am afraid this register is not valid for old parts which >> don't support this feature. I checked the datasheet of S29GL01GP. It has >> no description of offset 0xC. Reading from the hardware, I got 0xFF. If >> there is no "valid" bit to indicate we can use lower software bit >> regsiter, it is useless. > > I am happy to dump you whatever you need from the hyperflash (which is > in fact a CFI NOR flash with different interface), if that helps us > identify how to improve the condition to discern flashes which do and do > not support this polling feature. > > Since CFI is a standard, there must be a way to do it. In vendor specific extended query, it has a register at address 53h in this document http://www.cypress.com/file/213771/download. > >> Comparing the logs >> >> Failing board >> >> Flash: York debug: manuId = 0x1 >> York debug: lsbits = 0xff >> York debug: read offset 0xE as 0x28 >> York debug: read offset 0xF as 0x1 >> 128 MiB >> >> >> Working board >> >> Flash: York debug: manuId = 0x1 >> York debug: lsbits = 0x3 >> York debug: read offset 0xE as 0x28 >> York debug: read offset 0xF as 0x1 >> 128 MiB >> >> If there is no other way to know the "lower software bits" register is >> valid, then we have to abandon this feature. > > We cannot because then that breaks hyperflash support, so we need to > figure out how to improve the condition to discern the flashes. > I sent you a patch. Please review. York
On 11/18/2017 08:11 PM, York Sun wrote: > On 11/18/2017 02:24 AM, Marek Vasut wrote: >> On 11/17/2017 10:48 PM, York Sun wrote: >>> On 11/17/2017 12:59 PM, York Sun wrote: >>>> On 11/17/2017 11:04 AM, Marek Vasut wrote: >>>>> On 11/17/2017 08:02 PM, Marek Vasut wrote: >>>>>> On 11/17/2017 05:43 PM, York Sun wrote: >>>>>>> On 09/12/2017 10:09 AM, Marek Vasut wrote: >>>>>>>> The status register is optional in the AMD command sets, but it's >>>>>>>> presence can be checked by reading out CFI table entry 0xc bit 0. >>>>>>>> If the register is present, prefer using it's bit 7 to determine >>>>>>>> if the flash is busy over reading the flash ; this is needed ie. >>>>>>>> on Hyperflash memories. >>>>>>>> >>>>>>>> Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> >>>>>>>> --- >>>>>>> >>>>>>> Marek, >>>>>>> >>>>>>> Some of our boards failed. I traced to this commit. Reverting this >>>>>>> commit fixes the issue. I happen to have two boards with slightly >>>>>>> different flash chip. One works and the other doesn't. >>>>>> >>>>>> I can't since I don't have the board with such a chip ... >>>>>> Which chip is that ? Mine is Spansion S25KL256 hyperflash. >>>>> >>>>> S26KL256S (26 instead of 25), sorry. >>>>> >>>> >>>> My local board has chip S29GL01GS11TFIV1. The remote board has identical >>>> manufacture ID and device ID. I guess they are the same part (maybe >>>> different speed grade though). I haven't heard from the team who can >>>> visually check the part number for me. >>>> >>> >>> Marek, >>> >>> I got an email from our hardware team. At some point, our vendor had two >>> parts with the same device ID. The other one is S29GL01GP13TFIV1. >>> According to Cypress migration document, the old part doesn't support >>> status register feature. I understand you already check lower software >>> bits. >> >> Which lower software bits ? > > Register at offset 0xC. This register is not part of CFI. But there is > CFI extention, explained below. > >> >>> But I am afraid this register is not valid for old parts which >>> don't support this feature. I checked the datasheet of S29GL01GP. It has >>> no description of offset 0xC. Reading from the hardware, I got 0xFF. If >>> there is no "valid" bit to indicate we can use lower software bit >>> regsiter, it is useless. >> >> I am happy to dump you whatever you need from the hyperflash (which is >> in fact a CFI NOR flash with different interface), if that helps us >> identify how to improve the condition to discern flashes which do and do >> not support this polling feature. >> >> Since CFI is a standard, there must be a way to do it. > > In vendor specific extended query, it has a register at address 53h in > this document http://www.cypress.com/file/213771/download. Cool, that works! >> >>> Comparing the logs >>> >>> Failing board >>> >>> Flash: York debug: manuId = 0x1 >>> York debug: lsbits = 0xff >>> York debug: read offset 0xE as 0x28 >>> York debug: read offset 0xF as 0x1 >>> 128 MiB >>> >>> >>> Working board >>> >>> Flash: York debug: manuId = 0x1 >>> York debug: lsbits = 0x3 >>> York debug: read offset 0xE as 0x28 >>> York debug: read offset 0xF as 0x1 >>> 128 MiB >>> >>> If there is no other way to know the "lower software bits" register is >>> valid, then we have to abandon this feature. >> >> We cannot because then that breaks hyperflash support, so we need to >> figure out how to improve the condition to discern the flashes. >> > > I sent you a patch. Please review. Done, thanks > York > >
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index df04a425e2..8a5babea7b 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -544,7 +544,16 @@ static int flash_is_busy (flash_info_t * info, flash_sect_t sect) #ifdef CONFIG_FLASH_CFI_LEGACY case CFI_CMDSET_AMD_LEGACY: #endif - retval = flash_toggle (info, sect, 0, AMD_STATUS_TOGGLE); + if (info->sr_supported) { + flash_write_cmd (info, sect, info->addr_unlock1, + FLASH_CMD_READ_STATUS); + retval = !flash_isset (info, sect, 0, + FLASH_STATUS_DONE); + } else { + retval = flash_toggle (info, sect, 0, + AMD_STATUS_TOGGLE); + } + break; default: retval = 0; @@ -1685,6 +1694,7 @@ static void cmdset_amd_read_jedec_ids(flash_info_t *info) { ushort bankId = 0; uchar manuId; + uchar lsbits; flash_write_cmd(info, 0, 0, AMD_CMD_RESET); flash_unlock_seq(info, 0); @@ -1700,6 +1710,9 @@ static void cmdset_amd_read_jedec_ids(flash_info_t *info) } info->manufacturer_id = manuId; + lsbits = flash_read_uchar(info, FLASH_OFFSET_LOWER_SW_BITS); + info->sr_supported = lsbits & BIT(0); + switch (info->chipwidth){ case FLASH_CFI_8BIT: info->device_id = flash_read_uchar (info, diff --git a/include/flash.h b/include/flash.h index 0eedb1efa8..dc67cb2df6 100644 --- a/include/flash.h +++ b/include/flash.h @@ -42,6 +42,7 @@ typedef struct { ushort cfi_offset; /* offset for cfi query */ ulong addr_unlock1; /* unlock address 1 for AMD flash roms */ ulong addr_unlock2; /* unlock address 2 for AMD flash roms */ + uchar sr_supported; /* status register supported */ const char *name; /* human-readable name */ #endif #ifdef CONFIG_MTD diff --git a/include/mtd/cfi_flash.h b/include/mtd/cfi_flash.h index eade2b3614..095725a805 100644 --- a/include/mtd/cfi_flash.h +++ b/include/mtd/cfi_flash.h @@ -62,6 +62,7 @@ #define FLASH_OFFSET_MANUFACTURER_ID 0x00 #define FLASH_OFFSET_DEVICE_ID 0x01 +#define FLASH_OFFSET_LOWER_SW_BITS 0x0C #define FLASH_OFFSET_DEVICE_ID2 0x0E #define FLASH_OFFSET_DEVICE_ID3 0x0F #define FLASH_OFFSET_CFI 0x55
The status register is optional in the AMD command sets, but it's presence can be checked by reading out CFI table entry 0xc bit 0. If the register is present, prefer using it's bit 7 to determine if the flash is busy over reading the flash ; this is needed ie. on Hyperflash memories. Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> --- drivers/mtd/cfi_flash.c | 15 ++++++++++++++- include/flash.h | 1 + include/mtd/cfi_flash.h | 1 + 3 files changed, 16 insertions(+), 1 deletion(-)