[U-Boot] [PATCH v4 3/3] spi: Add SPI NOR protection mechanism

[Fabio Estevam]

On Thu, Oct 1, 2015 at 10:48 AM, Jagan Teki <jteki at openedev.com> wrote:

> To be in-line with mtd lock utils, how about this notation
> sf protect lock
> sf protect unlock
> sf protect islocked

Ok.

>> diff --git a/drivers/mtd/spi/Kconfig b/drivers/mtd/spi/Kconfig
>> index 3f7433c..2ee1089 100644
>> --- a/drivers/mtd/spi/Kconfig
>> +++ b/drivers/mtd/spi/Kconfig
>> @@ -101,6 +101,21 @@ config SPI_FLASH_USE_4K_SECTORS
>>           Please note that some tools/drivers/filesystems may not work with
>>           4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
>>
>> +config SPI_FLASH_STM_PROTECT
>
> Remove this macro, just use existing CONFIG_SPI_FLASH_STMICRO as all
> these code is specific to these parts.

Ok.

>> diff --git a/drivers/mtd/spi/sf_internal.h b/drivers/mtd/spi/sf_internal.h
>> index 9c95d56..8fb1b18 100644
>> --- a/drivers/mtd/spi/sf_internal.h
>> +++ b/drivers/mtd/spi/sf_internal.h
>> @@ -162,12 +162,6 @@ int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len,
>>  /* Flash erase(sectors) operation, support all possible erase commands */
>>  int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len);
>>
>> -/* Read the status register */
>> -int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs);
>> -
>> -/* Program the status register */
>> -int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws);
>
> Please don't remove this stuff - this is mtd/spi

Ok.

>> --- a/drivers/mtd/spi/sf_ops.c
>> +++ b/drivers/mtd/spi/sf_ops.c
>> @@ -276,6 +276,11 @@ int spi_flash_cmd_erase_ops(struct spi_flash *flash, u32 offset, size_t len)
>>                 return -1;
>>         }
>>
>> +       if (flash->is_locked(flash, offset, len) > 0) {
>> +               printf("offset 0x%x is protected and cannot be erased\n", offset);
>> +               return -EINVAL;
>> +       }
>
> Can we handle - if the part of the sectors were protected and
> remaining were not, so it will erase only unprotected sectors and show
> "these many sectors were locked and unable to erase"?

We can add this feature in subsequent patches.

>> +int spi_flash_protect(struct spi_flash *nor, loff_t ofs, u32 len, bool prot)
>> +{
>> +       if (prot)
>> +               return nor->lock(nor, ofs, len);
>> +       else
>> +               return nor->unlock(nor, ofs, len);
>
> %s/nor/flash/g

Ok.

> Define this spi_flash_protect in include/spi_flash not in ops.

Cannot do this as include/spi_flash.h is included by several files,
and then we would get errors about funcion being redefined.

>> +/* Read the status register */
>> +int spi_flash_cmd_read_status(struct spi_flash *flash, u8 *rs);
>> +
>> +/* Program the status register */
>> +int spi_flash_cmd_write_status(struct spi_flash *flash, u8 ws);
>> +
>> +int stm_is_locked(struct spi_flash *nor, loff_t ofs, u32 len);
>> +int stm_lock(struct spi_flash *nor, u32 ofs, u32 len);
>> +int stm_unlock(struct spi_flash *nor, u32 ofs, u32 len);
>> +int spi_flash_protect(struct spi_flash *nor, loff_t ofs, u32 len, bool prot);
>
> Move these into mtd/spi

Ok.