[PATCH] mtd: cfi: Fix PPB lock status readout

[PATCH] mtd: cfi: Fix PPB lock status readout

[Marek Vasut]

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(-)

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 */
-- 
2.30.2

[PATCH] mtd: cfi: Fix PPB lock status readout

[Stefan Roese]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de

[PATCH] mtd: cfi: Fix PPB lock status readout

[Joakim Tjernlund]

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

[PATCH] mtd: cfi: Fix PPB lock status readout

[Stefan Roese]

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

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: sr at denx.de