Re: [PATCH v2 1/3] mtd: spi-nor: always respect write-protect input

[<Tudor.Ambarus@xxxxxxxxxxxxx>]

Hi, Jonas,

On 03/10/2019 12:42 PM, Jonas Bonn wrote:
> Hi,
> 
> On 10/03/2019 10:58, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
>> + James and Yong from Cypress.
>>
>> Hi, Jonas, James, Yong,
>>
>> On 01/30/2019 12:07 AM, Jonas Bonn wrote:
>>> The status register bit SRWD (status register write disable) is
>>> described in many words in the datasheets but effectively boils down to:
>>>
>>> i) if set, respect WP# when trying to change protection bits;
>>> ii) if unset, ignore WP# when trying to change protection bits
>>>
>>> In short, the bit determines whether the WP# signal is honored or not.
>>>
>>> It's difficult to imagine the use-case where the WP# is connected and
>>> asserted but the user doesn't want to respect its setting.  As such,
>>> this patch sets the SRWD bit unconditionally so that the WP# is _always_
>>> respected; hardware that doesn't care about WP# normally won't even have
>>> it connected.
>>>
>>
>> Always setting SRWD to 1, dismisses the need of a SRWD bit in the first place.
>> Do you know why Cypress made this bit configurable and didn't enable it by default?
> 
> I suspect it has some historical relevance; as things currently stand, I don't see how it's useful in its current form.
> 
> We initially thought that the WP# signal turned on/off block protection but that's not the case; the block protection bits BP0-BP2 turn on/off block protection and the WP# signal only protects the BP[0-2] bits from being modified.  Turning off the WP# functionality via the SRWD bits means that the BP[0-2] bits are always modifiable and therefore the flash device is never really protected from writes by a malicious actor.
> 
> The key question here is:  if the WP# signal is connected, why would you ever want its state to be ignored by the device?  De-asserting the signal has the same effect as setting the SRWD bit.  And the default state is un-asserted if the signal is not connected, so that case is covered, too.  I see no reason not to just always set SRWD and thereby make the WP# signal do what one expects of it, always.
> 

I tend to agree with you. Let's wait for a few days, maybe James or Yong will
tell us the Cypress's reasons of making SRWD configurable. In the meantime I'll
check the datasheets of the flashes that have SPI_NOR_HAS_LOCK declared, see if
SRWD is configurable and what were the reasons of making it so.

Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/