| Message ID | 20210411184745.8388-1-marek.vasut+renesas@gmail.com |
|---|---|
| State | Accepted |
| Commit | 94657482f3fb64de124cc68b14c869a6836eaaf3 |
| Delegated to: | Stefan Roese |
| Headers | show |
| Series | mtd: cfi: Fix PPB lock status readout | expand |
On 11.04.21 20:47, Marek Vasut wrote: > According to S26KL512S datasheet [1] and S29GL01GS datasheet [2], > the procedure to read out PPB lock bits is to send the PPB Entry, > PPB Read, Reset/ASO Exit. Currently, the code does send incorrect > PPB Entry, PPB Read and Reset/ASO Exit is completely missing. > > The PPB Entry sent is implemented by sending flash_unlock_seq() > and flash_write_cmd(..., FLASH_CMD_READ_ID). This translates to > sequence 0x555:0xaa, 0x2aa:0x55, 0x555:0x90=FLASH_CMD_READ_ID. > However, both [1] and [2] specify the last byte of PPB Entry as > 0xc0=AMD_CMD_SET_PPB_ENTRY instead of 0x90=FLASH_CMD_READ_ID, > that is 0x555:0xaa, 0x2aa:0x55, 0x555:0xc0=AMD_CMD_SET_PPB_ENTRY. > Since this does make sense, this patch fixes it and thus also > aligns the code in flash_get_size() with flash_real_protect(). > > The PPB Read returns 00h in case of Protected state and 01h in case > of Unprotected state, according to [1] Note 83 and [2] Note 17, so > invert the result. Moreover, align the arguments with similar code > in flash_real_protect(). > > Finally, Reset/ASO Exit command should be executed to exit the PPB > mode, so add the missing reset. > > [1] https://www.cypress.com/file/213346/download > Document Number: 001-99198 Rev. *M > Table 40. Command Definitions, Nonvolatile Sector Protection > Command Set Definitions > [2] https://www.cypress.com/file/177976/download > Document Number: 001-98285 Rev. *R > Table 7.1 Command Definitions, Nonvolatile Sector Protection > Command Set Definitions > > Fixes: 03deff433e ("cfi_flash: Read PPB sector protection from device for AMD/Spansion chips") > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Stefan Roese <sr@denx.de> Reviewed-by: Stefan Roese <sr@denx.de> Thanks, Stefan > --- > drivers/mtd/cfi_flash.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c > index 9642d7c7dc..9c27fea5d8 100644 > --- a/drivers/mtd/cfi_flash.c > +++ b/drivers/mtd/cfi_flash.c > @@ -2276,12 +2276,12 @@ ulong flash_get_size(phys_addr_t base, int banknum) > flash_unlock_seq(info, 0); > flash_write_cmd(info, 0, > info->addr_unlock1, > - FLASH_CMD_READ_ID); > + AMD_CMD_SET_PPB_ENTRY); > info->protect[sect_cnt] = > - flash_isset( > - info, sect_cnt, > - FLASH_OFFSET_PROTECT, > - FLASH_STATUS_PROTECT); > + !flash_isset(info, sect_cnt, > + 0, 0x01); > + flash_write_cmd(info, 0, 0, > + info->cmd_reset); > break; > default: > /* default: not protected */ > Viele Grüße, Stefan
On Thu, 2021-04-15 at 07:25 +0200, Stefan Roese wrote: > On 11.04.21 20:47, Marek Vasut wrote: > > According to S26KL512S datasheet [1] and S29GL01GS datasheet [2], > > the procedure to read out PPB lock bits is to send the PPB Entry, > > PPB Read, Reset/ASO Exit. Currently, the code does send incorrect > > PPB Entry, PPB Read and Reset/ASO Exit is completely missing. > > > > The PPB Entry sent is implemented by sending flash_unlock_seq() > > and flash_write_cmd(..., FLASH_CMD_READ_ID). This translates to > > sequence 0x555:0xaa, 0x2aa:0x55, 0x555:0x90=FLASH_CMD_READ_ID. > > However, both [1] and [2] specify the last byte of PPB Entry as > > 0xc0=AMD_CMD_SET_PPB_ENTRY instead of 0x90=FLASH_CMD_READ_ID, > > that is 0x555:0xaa, 0x2aa:0x55, 0x555:0xc0=AMD_CMD_SET_PPB_ENTRY. > > Since this does make sense, this patch fixes it and thus also > > aligns the code in flash_get_size() with flash_real_protect(). > > > > The PPB Read returns 00h in case of Protected state and 01h in case > > of Unprotected state, according to [1] Note 83 and [2] Note 17, so > > invert the result. Moreover, align the arguments with similar code > > in flash_real_protect(). > > Just saw the PPB lock words passing by and remembered the problems we had with u-boot using PPB to lock/unlock flash. This method is slow and persistent(unlike the Intel method) and this carries all the way to fw_setenv. PPB will write to internal flash bits(same bits every time) and this causes wear on said flash. We had disable flash lock/unlock here due to this wear. The DYB method is a much better fit for u-boot/linux flash locking needs. Jocke
On 11.04.21 20:47, Marek Vasut wrote: > According to S26KL512S datasheet [1] and S29GL01GS datasheet [2], > the procedure to read out PPB lock bits is to send the PPB Entry, > PPB Read, Reset/ASO Exit. Currently, the code does send incorrect > PPB Entry, PPB Read and Reset/ASO Exit is completely missing. > > The PPB Entry sent is implemented by sending flash_unlock_seq() > and flash_write_cmd(..., FLASH_CMD_READ_ID). This translates to > sequence 0x555:0xaa, 0x2aa:0x55, 0x555:0x90=FLASH_CMD_READ_ID. > However, both [1] and [2] specify the last byte of PPB Entry as > 0xc0=AMD_CMD_SET_PPB_ENTRY instead of 0x90=FLASH_CMD_READ_ID, > that is 0x555:0xaa, 0x2aa:0x55, 0x555:0xc0=AMD_CMD_SET_PPB_ENTRY. > Since this does make sense, this patch fixes it and thus also > aligns the code in flash_get_size() with flash_real_protect(). > > The PPB Read returns 00h in case of Protected state and 01h in case > of Unprotected state, according to [1] Note 83 and [2] Note 17, so > invert the result. Moreover, align the arguments with similar code > in flash_real_protect(). > > Finally, Reset/ASO Exit command should be executed to exit the PPB > mode, so add the missing reset. > > [1] https://www.cypress.com/file/213346/download > Document Number: 001-99198 Rev. *M > Table 40. Command Definitions, Nonvolatile Sector Protection > Command Set Definitions > [2] https://www.cypress.com/file/177976/download > Document Number: 001-98285 Rev. *R > Table 7.1 Command Definitions, Nonvolatile Sector Protection > Command Set Definitions > > Fixes: 03deff433e ("cfi_flash: Read PPB sector protection from device for AMD/Spansion chips") > Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> > Cc: Stefan Roese <sr@denx.de> Applied to u-boot-cfi-flash/master Thanks, Stefan > --- > drivers/mtd/cfi_flash.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c > index 9642d7c7dc..9c27fea5d8 100644 > --- a/drivers/mtd/cfi_flash.c > +++ b/drivers/mtd/cfi_flash.c > @@ -2276,12 +2276,12 @@ ulong flash_get_size(phys_addr_t base, int banknum) > flash_unlock_seq(info, 0); > flash_write_cmd(info, 0, > info->addr_unlock1, > - FLASH_CMD_READ_ID); > + AMD_CMD_SET_PPB_ENTRY); > info->protect[sect_cnt] = > - flash_isset( > - info, sect_cnt, > - FLASH_OFFSET_PROTECT, > - FLASH_STATUS_PROTECT); > + !flash_isset(info, sect_cnt, > + 0, 0x01); > + flash_write_cmd(info, 0, 0, > + info->cmd_reset); > break; > default: > /* default: not protected */ > Viele Grüße, Stefan
diff --git a/drivers/mtd/cfi_flash.c b/drivers/mtd/cfi_flash.c index 9642d7c7dc..9c27fea5d8 100644 --- a/drivers/mtd/cfi_flash.c +++ b/drivers/mtd/cfi_flash.c @@ -2276,12 +2276,12 @@ ulong flash_get_size(phys_addr_t base, int banknum) flash_unlock_seq(info, 0); flash_write_cmd(info, 0, info->addr_unlock1, - FLASH_CMD_READ_ID); + AMD_CMD_SET_PPB_ENTRY); info->protect[sect_cnt] = - flash_isset( - info, sect_cnt, - FLASH_OFFSET_PROTECT, - FLASH_STATUS_PROTECT); + !flash_isset(info, sect_cnt, + 0, 0x01); + flash_write_cmd(info, 0, 0, + info->cmd_reset); break; default: /* default: not protected */
According to S26KL512S datasheet [1] and S29GL01GS datasheet [2], the procedure to read out PPB lock bits is to send the PPB Entry, PPB Read, Reset/ASO Exit. Currently, the code does send incorrect PPB Entry, PPB Read and Reset/ASO Exit is completely missing. The PPB Entry sent is implemented by sending flash_unlock_seq() and flash_write_cmd(..., FLASH_CMD_READ_ID). This translates to sequence 0x555:0xaa, 0x2aa:0x55, 0x555:0x90=FLASH_CMD_READ_ID. However, both [1] and [2] specify the last byte of PPB Entry as 0xc0=AMD_CMD_SET_PPB_ENTRY instead of 0x90=FLASH_CMD_READ_ID, that is 0x555:0xaa, 0x2aa:0x55, 0x555:0xc0=AMD_CMD_SET_PPB_ENTRY. Since this does make sense, this patch fixes it and thus also aligns the code in flash_get_size() with flash_real_protect(). The PPB Read returns 00h in case of Protected state and 01h in case of Unprotected state, according to [1] Note 83 and [2] Note 17, so invert the result. Moreover, align the arguments with similar code in flash_real_protect(). Finally, Reset/ASO Exit command should be executed to exit the PPB mode, so add the missing reset. [1] https://www.cypress.com/file/213346/download Document Number: 001-99198 Rev. *M Table 40. Command Definitions, Nonvolatile Sector Protection Command Set Definitions [2] https://www.cypress.com/file/177976/download Document Number: 001-98285 Rev. *R Table 7.1 Command Definitions, Nonvolatile Sector Protection Command Set Definitions Fixes: 03deff433e ("cfi_flash: Read PPB sector protection from device for AMD/Spansion chips") Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com> Cc: Stefan Roese <sr@denx.de> --- drivers/mtd/cfi_flash.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)