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

[Jagan Teki]

On 2 October 2015 at 00:06, Fabio Estevam <festevam at gmail.com> wrote:
> 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"?

Couldn't understand why would we get an errors for define this in
spi_flash.h Like spi_flash_erase/_read/_write spi_flash_protect is a
command ops which would intern call mtd/spi operations as function
pointers.

>
> 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.
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot



-- 
Jagan | openedev.