[U-Boot] [PATCH 1/5] spi_sf: Skip the erasing of protected sectors

[Jagan Teki]

On 2 September 2015 at 20:11, Otavio Salvador <otavio at ossystems.com.br> wrote:
> Many SPI flashes have protection bits (BP2, BP1 and BP0) in the
> status register that can protect selected regions of the SPI NOR.
>
> Take these bits into account when performing erase operations,
> making sure that the protected areas are skipped.
>
> Introduce the CONFIG_SPI_NOR_PROTECTION option that can be selected
> by systems that want to protect selected regions of SPI NOR flash.
>
> Signed-off-by: Otavio Salvador <otavio at ossystems.com.br>
> ---
>
>  README                     |  4 ++
>  drivers/mtd/spi/sf_ops.c   | 91 ++++++++++++++++++++++++++++++++++++++++++++--
>  drivers/mtd/spi/sf_probe.c |  2 +
>  3 files changed, 94 insertions(+), 3 deletions(-)
>
> diff --git a/README b/README
> index 1acc355..18d61a6 100644
> --- a/README
> +++ b/README
> @@ -2751,6 +2751,10 @@ CBFS (Coreboot Filesystem) support
>                 Timeout for waiting until spi transfer completed.
>                 default: (CONFIG_SYS_HZ/100)     /* 10 ms */
>
> +               CONFIG_SPI_NOR_PROTECTION
> +               Enable the built-in protection mechanism provided by the
> +               BP2, BP1 and BP0 bits from the status register.

Does these bits specific for micron flash?

> +
>  - FPGA Support: CONFIG_FPGA
>
>                 Enables FPGA subsystem.
> diff --git a/drivers/mtd/spi/sf_ops.c b/drivers/mtd/spi/sf_ops.c
> index 900ec1f..a7a07f9 100644
> --- a/drivers/mtd/spi/sf_ops.c
> +++ b/drivers/mtd/spi/sf_ops.c
> @@ -264,6 +264,84 @@ int spi_flash_write_common(struct spi_flash *flash, const u8 *cmd,
>         return ret;
>  }
>
> +#ifdef CONFIG_SPI_NOR_PROTECTION
> +static int is_protected(struct spi_flash *flash, u32 offset)
> +{
> +       int total_sector, ret, protected_sector;
> +       bool *protected;
> +       u8 sr;
> +
> +       total_sector = flash->size / flash->erase_size;
> +       protected = calloc(1, total_sector);
> +       if (!protected)
> +               return -ENOMEM;
> +
> +       ret = spi_flash_cmd_read_status(flash, &sr);
> +       if (ret < 0)
> +               goto free_protected;
> +
> +       /* Extract the protection bits: BP2, BP1 and BP0 */
> +       sr = (sr & 0x1c) >> 2;

Does this masking specific to particular flash part or it's generic
way, please elaborate.

> +
> +       switch (sr) {
> +       case 0:
> +               protected_sector = 0;
> +       break;
> +
> +       case 1:
> +               protected_sector = total_sector / 64;
> +       break;
> +
> +       case 2:
> +               protected_sector = total_sector / 32;
> +       break;
> +
> +       case 3:
> +               protected_sector = total_sector / 16;
> +       break;
> +
> +       case 4:
> +               protected_sector = total_sector / 8;
> +       break;
> +
> +       case 5:
> +               protected_sector = total_sector / 4;
> +       break;
> +
> +       case 6:
> +               protected_sector = total_sector / 2;
> +       break;
> +
> +       case 7:
> +               protected_sector = total_sector;
> +       break;
> +
> +       default:
> +               ret = -EINVAL;
> +               goto free_protected;
> +       }
> +
> +       while (protected_sector) {
> +               protected[total_sector - protected_sector] = 1;
> +               protected_sector--;
> +       }
> +
> +       if (protected[offset/flash->erase_size])
> +               return 1;
> +       else
> +               return 0;
> +
> +free_protected:
> +       free(protected);
> +       return ret;
> +}
> +#else
> +static int is_protected(struct spi_flash *flash, u32 offset)
> +{
> +       return 0;
> +}
> +#endif
> +
>  int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
>  {
>         u32 erase_size, erase_addr;
> @@ -294,10 +372,17 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
>                 debug("SF: erase %2x %2x %2x %2x (%x)\n", cmd[0], cmd[1],
>                       cmd[2], cmd[3], erase_addr);
>
> -               ret = spi_flash_write_common(flash, cmd, sizeof(cmd), NULL, 0);
> -               if (ret < 0) {
> -                       debug("SF: erase failed\n");
> +               if (is_protected(flash, offset) > 0) {
> +                       printf("Skipping to erase protected offset 0x%x\n",
> +                              offset);
>                         break;
> +               } else {
> +                       ret = spi_flash_write_common(flash, cmd, sizeof(cmd),
> +                                                    NULL, 0);
> +                       if (ret < 0) {
> +                               debug("SF: erase failed\n");
> +                               break;
> +                       }

So this seems to be locking sector area which is been protected, why
it's needed on erase? why can't we do it while probe? any side
effects. and also do we have any way to unlock the protected sectors?
does that really a need?

>                 }
>
>                 offset += erase_size;
> diff --git a/drivers/mtd/spi/sf_probe.c b/drivers/mtd/spi/sf_probe.c
> index 954376d..49c8b78 100644
> --- a/drivers/mtd/spi/sf_probe.c
> +++ b/drivers/mtd/spi/sf_probe.c
> @@ -256,12 +256,14 @@ static int spi_flash_validate_params(struct spi_slave *spi, u8 *idcode,
>         }
>  #endif
>
> +#if !defined(CONFIG_SPI_NOR_PROTECTION)
>         /* Flash powers up read-only, so clear BP# bits */
>  #if defined(CONFIG_SPI_FLASH_ATMEL) || \
>         defined(CONFIG_SPI_FLASH_MACRONIX) || \
>         defined(CONFIG_SPI_FLASH_SST)
>                 spi_flash_cmd_write_status(flash, 0);
>  #endif
> +#endif
>
>         return 0;
>  }
> --
> 2.5.1

thanks!
-- 
Jagan | openedev.