[U-Boot] sf: proposed status register protect

[U-Boot] sf: proposed status register protect

[George McCollister]

I'm looking into adding a sub-command to sf to protect status
registers on Winbond SPI flash parts via the status register protect
bits SRP0, SRP1. I propose the sub-command be named sr-protect.

My use case is simple, I want to make the SPI flash read-only when the
/WP pin is low. To do this I must do 'sf protect lock 0 $size' then
set SRP0 to 1 and hold the /WP pin low. Although my use case doesn't
require it, setting SRP1 to 1 allows either Power Supply Lock-Down or
One Time Program (if supported by hardware) and I would probably add
support for these as well.

All of the current Winbond SPI NOR flash part datasheets that I've
looked at show that the parts support the same locking implemented in
stm_lock() even though SPI_FLASH_CFI_MFR_WINBOND is not in the list of
vendors which use the function. I'll change this to make
SPI_FLASH_CFI_MFR_WINBOND parts use stm_lock(), stm_unlock(),
stm_is_locked().

spi_flash_protect() (which ultimately calls either stm_lock() or
stm_unlock()) is not part of the driver model (dm_spi_flash_ops).
Is there any issue with adding another function (let's call it
spi_flash_sr_protect) that is not part of the driver model?

Is anyone aware of any vendors other than Winbond that use this SRP
mechanism or should only SPI_FLASH_CFI_MFR_WINBOND parts use it?
For details see "7.1.7 Status Register Protect Bit (SRP1, SRP0)" on
page 15 of http://www.winbond.com/resource-files/w25q64cv_revh_052214[2].pdf

Cheers,
George McCollister

[U-Boot] sf: proposed status register protect

[Jagan Teki]

On Thu, Oct 6, 2016 at 8:32 PM, George McCollister
<george.mccollister@gmail.com> wrote:
> I'm looking into adding a sub-command to sf to protect status
> registers on Winbond SPI flash parts via the status register protect
> bits SRP0, SRP1. I propose the sub-command be named sr-protect.
>
> My use case is simple, I want to make the SPI flash read-only when the
> /WP pin is low. To do this I must do 'sf protect lock 0 $size' then
> set SRP0 to 1 and hold the /WP pin low. Although my use case doesn't
> require it, setting SRP1 to 1 allows either Power Supply Lock-Down or
> One Time Program (if supported by hardware) and I would probably add
> support for these as well.
>
> All of the current Winbond SPI NOR flash part datasheets that I've
> looked at show that the parts support the same locking implemented in
> stm_lock() even though SPI_FLASH_CFI_MFR_WINBOND is not in the list of
> vendors which use the function. I'll change this to make
> SPI_FLASH_CFI_MFR_WINBOND parts use stm_lock(), stm_unlock(),
> stm_is_locked().

Does stm_lock can't protect the Winbond? how different is in the
process of doing when compared to stm?

>
> spi_flash_protect() (which ultimately calls either stm_lock() or
> stm_unlock()) is not part of the driver model (dm_spi_flash_ops).
> Is there any issue with adding another function (let's call it
> spi_flash_sr_protect) that is not part of the driver model?

Why spi_flash_sr_protect, why can't existing stm_lock?

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] sf: proposed status register protect

[George McCollister]

On Thu, Oct 6, 2016 at 10:27 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Thu, Oct 6, 2016 at 8:32 PM, George McCollister
> <george.mccollister@gmail.com> wrote:
>> I'm looking into adding a sub-command to sf to protect status
>> registers on Winbond SPI flash parts via the status register protect
>> bits SRP0, SRP1. I propose the sub-command be named sr-protect.
>>
>> My use case is simple, I want to make the SPI flash read-only when the
>> /WP pin is low. To do this I must do 'sf protect lock 0 $size' then
>> set SRP0 to 1 and hold the /WP pin low. Although my use case doesn't
>> require it, setting SRP1 to 1 allows either Power Supply Lock-Down or
>> One Time Program (if supported by hardware) and I would probably add
>> support for these as well.
>>
>> All of the current Winbond SPI NOR flash part datasheets that I've
>> looked at show that the parts support the same locking implemented in
>> stm_lock() even though SPI_FLASH_CFI_MFR_WINBOND is not in the list of
>> vendors which use the function. I'll change this to make
>> SPI_FLASH_CFI_MFR_WINBOND parts use stm_lock(), stm_unlock(),
>> stm_is_locked().
>
> Does stm_lock can't protect the Winbond? how different is in the
> process of doing when compared to stm?

stm_lock() does work with the Winbond parts. The issue is that you can
simply unlock from software (stm_unlock). To make it impossible to
write to the flash from software you must use stm_lock() then use the
status register protect bits to prevent an unlock.

>
>>
>> spi_flash_protect() (which ultimately calls either stm_lock() or
>> stm_unlock()) is not part of the driver model (dm_spi_flash_ops).
>> Is there any issue with adding another function (let's call it
>> spi_flash_sr_protect) that is not part of the driver model?
>
> Why spi_flash_sr_protect, why can't existing stm_lock?

Explained above.

>
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.

[U-Boot] sf: proposed status register protect

[Jagan Teki]

On Thu, Oct 6, 2016 at 9:21 PM, George McCollister
<george.mccollister@gmail.com> wrote:
> On Thu, Oct 6, 2016 at 10:27 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>> On Thu, Oct 6, 2016 at 8:32 PM, George McCollister
>> <george.mccollister@gmail.com> wrote:
>>> I'm looking into adding a sub-command to sf to protect status
>>> registers on Winbond SPI flash parts via the status register protect
>>> bits SRP0, SRP1. I propose the sub-command be named sr-protect.
>>>
>>> My use case is simple, I want to make the SPI flash read-only when the
>>> /WP pin is low. To do this I must do 'sf protect lock 0 $size' then
>>> set SRP0 to 1 and hold the /WP pin low. Although my use case doesn't
>>> require it, setting SRP1 to 1 allows either Power Supply Lock-Down or
>>> One Time Program (if supported by hardware) and I would probably add
>>> support for these as well.
>>>
>>> All of the current Winbond SPI NOR flash part datasheets that I've
>>> looked at show that the parts support the same locking implemented in
>>> stm_lock() even though SPI_FLASH_CFI_MFR_WINBOND is not in the list of
>>> vendors which use the function. I'll change this to make
>>> SPI_FLASH_CFI_MFR_WINBOND parts use stm_lock(), stm_unlock(),
>>> stm_is_locked().
>>
>> Does stm_lock can't protect the Winbond? how different is in the
>> process of doing when compared to stm?
>
> stm_lock() does work with the Winbond parts. The issue is that you can
> simply unlock from software (stm_unlock). To make it impossible to
> write to the flash from software you must use stm_lock() then use the
> status register protect bits to prevent an unlock.

So, you need even if stm_unlock triggered from cmd flash still lock?
then what is the point of using unlock?
Just describe me what is the diff b/w winbond vs other stm flash in
case of lock and unlocking.

thanks!
-- 
Jagan Teki
Free Software Engineer | www.openedev.com
U-Boot, Linux | Upstream Maintainer
Hyderabad, India.

[U-Boot] sf: proposed status register protect

[George McCollister]

On Thu, Oct 6, 2016 at 11:55 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
> On Thu, Oct 6, 2016 at 9:21 PM, George McCollister
> <george.mccollister@gmail.com> wrote:
>> On Thu, Oct 6, 2016 at 10:27 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote:
>>> On Thu, Oct 6, 2016 at 8:32 PM, George McCollister
>>> <george.mccollister@gmail.com> wrote:
>>>> I'm looking into adding a sub-command to sf to protect status
>>>> registers on Winbond SPI flash parts via the status register protect
>>>> bits SRP0, SRP1. I propose the sub-command be named sr-protect.
>>>>
>>>> My use case is simple, I want to make the SPI flash read-only when the
>>>> /WP pin is low. To do this I must do 'sf protect lock 0 $size' then
>>>> set SRP0 to 1 and hold the /WP pin low. Although my use case doesn't
>>>> require it, setting SRP1 to 1 allows either Power Supply Lock-Down or
>>>> One Time Program (if supported by hardware) and I would probably add
>>>> support for these as well.
>>>>
>>>> All of the current Winbond SPI NOR flash part datasheets that I've
>>>> looked at show that the parts support the same locking implemented in
>>>> stm_lock() even though SPI_FLASH_CFI_MFR_WINBOND is not in the list of
>>>> vendors which use the function. I'll change this to make
>>>> SPI_FLASH_CFI_MFR_WINBOND parts use stm_lock(), stm_unlock(),
>>>> stm_is_locked().
>>>
>>> Does stm_lock can't protect the Winbond? how different is in the
>>> process of doing when compared to stm?
>>
>> stm_lock() does work with the Winbond parts. The issue is that you can
>> simply unlock from software (stm_unlock). To make it impossible to
>> write to the flash from software you must use stm_lock() then use the
>> status register protect bits to prevent an unlock.
>
> So, you need even if stm_unlock triggered from cmd flash still lock?
> then what is the point of using unlock?

stm_lock()/stm_unlock() change the block protect bits in the status
register to lock/unlock flash blocks. The status register protect bits
allow changes to the status register to be blocked under certain
conditions (if /WP pin is low, until re-powered or permanently).

> Just describe me what is the diff b/w winbond vs other stm flash in
> case of lock and unlocking.

I think it may be the same but if you have a part number this code is
known to work with I'll get the datasheet and check. The comment above
stm_lock() actually mentions a Winbond part! As far as I can tell
STMicroelectronics no longer manufactures SPI Flash.

>
> thanks!
> --
> Jagan Teki
> Free Software Engineer | www.openedev.com
> U-Boot, Linux | Upstream Maintainer
> Hyderabad, India.