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

[<Tudor.Ambarus@xxxxxxxxxxxxx>]

On 03/20/2019 09:06 AM, Jonas Bonn wrote:
> External E-Mail
> 
> 
> 
> On 20/03/2019 07:33, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
>> Jonas,
>>
>> On 03/19/2019 09:13 AM, Jonas Bonn wrote:
>>>
>>> On 19/03/2019 07:59, Tudor.Ambarus@xxxxxxxxxxxxx wrote:
>>>> Jonas, Yong,
>>>>
>>>> On 03/12/2019 09:27 PM, Yong Qin wrote:
>>>>> Hi Tudor,
>>>>>
>>>>> Good question.
>>>>>
>>>>> The WP# function is not available when the Quad mode is enabled (CR[1]=1). The WP# function is replaced by IO2 for input and output during Quad mode. With that said, if CR[1] is set to 1 (Quad mode enabled), the registers will not be protected.
>>>>>
>>>>> Technically default SRWD bit can be set as either 0 or 1. However, as most customers (applications) don't use this feature to protect registers, the default SRWD bit set to 0 might be a better choice, and reserve the option to change to 1 for the applications do need this feature.
>>>>
>>>> I think I found the reason why SRWD bit is configurable, and disabled by
>>>> default: => to allow the installation of the flash in a system with a grounded
>>>> WP# pin while still enabling write to the BP bits.
>>>
>>> I think this is bogus.  Why would you ground the SRWD pin?  That's a design error.
>>
>> I've talked with Microchip hardware team, we have this feature on sst26 flashes too.
>>
>> Grounding WP would not necessarily be a design error. The intention is that some
>> users might choose to populate the Flash chip onto their PCB, program the memory
>> in-circuit, and then lock down the write protection to use the chip like a ROM.
>> Grounding WP allows them to do this. Since SRWD is set to '0' from the factory,
>> so users can program the memory, set the block protection (BP) bits to protect
>> the memory, and then set the SRWD bit to enable the grounded WP input and
>> prevent changing the BP bits.
> 
> Again, this is bogus.  The SRWD bit is non-volatile so just resetting the flash clears the bit and the BP bits can be changed.  This does not magically turn the flash into a ROM.  Grounded or not, the WP# is NOT respected until the SRWD bit is set; what you've described is yet another reason to default the SRWD to set.

SRWD is a non-volatile bit: default at power-up will be the setting prior to
power-down.

Cheers,
ta

> 
> With the BPNV bit (see patch 3 that I posted in the v2 series), at least the flash can be made to start up write-protected; however, there is nothing preventing modification of the PROTection bits until SRWD is set AND WP# is asserted (through grounding or otherwise).  Linux currently defaults SRWD to cleared which is "unprotected", irregardless of the state of WP#.
> 
>>
>> The grounded WP pin + SRWD bit method is an older, legacy approach to the
>> problem. We don't know how many users actually ground the WP pin in this manner,
>> but there are probably some users out there who do.
> 
> If they do so, they are fooling themselves... or they are running a patched kernel! :)
> 
> /Jonas
> 
> 
> 
> 
>>
>> Cheers,
>> ta
>>
>>>
>>> /Jonas
>>>
>>>>
>>>> Jonas, Yong, what do you think?
>>>>
>>>> Cheers,
>>>> ta
>>>>
>>>>>
>>>>> Thanks,
>>>>> Yong
>>>>>
>>>>> -----Original Message-----
>>>>> From: Tudor.Ambarus@xxxxxxxxxxxxx <Tudor.Ambarus@xxxxxxxxxxxxx>
>>>>> Sent: Tuesday, March 12, 2019 5:30 AM
>>>>> To: Yong Qin <Yong.Qin@xxxxxxxxxxx>; jonas@xxxxxxxxxxx; James Tomasetta <James.Tomasetta@xxxxxxxxxxx>
>>>>> Cc: linux-mtd@xxxxxxxxxxxxxxxxxxx; bbrezillon@xxxxxxxxxx; richard@xxxxxx; marek.vasut@xxxxxxxxx; computersforpeace@xxxxxxxxx; dwmw2@xxxxxxxxxxxxx
>>>>> Subject: Re: [PATCH v2 1/3] mtd: spi-nor: always respect write-protect input
>>>>>
>>>>> Hi, Yong,
>>>>>
>>>>> Thank you for the explanation. There are still few things to clarify.
>>>>>
>>>>> On 03/11/2019 10:14 PM, Yong Qin wrote:
>>>>>> SRWD bit (along with WP#) provides a way to protect Status and Configuration Registers from been modified unintendedly or by a malicious actor.
>>>>>>
>>>>>> By default, SRWD bit is 0, which means no protection on registers alternations. Registers can be modified easily by WRR command. (this is most of the application use cases).
>>>>>>
>>>>>> If set SRWD bit to 1, then when WP# is driven low during WRR command, WRR command will be ignored and Registers can't be modified. This provides a way to protect Registers, meanwhile still reserve the capability to modify Registers when necessary by driving WP# to high during WRR command.
>>>>>
>>>>> Does the SRWD bit protect the Status and Configuration Register bits even when in Quad Mode? WP# function is not available in Quad mode. How can one release this protection when in Quad Mode and SRWD set to 1?
>>>>>
>>>>> If SRWD bit is ignored in Quad Mode, then why didn't Cypress enable Status and Configuration Register bits protection by default? I.e., remove SRWD bit from SR1, make BIT(7) a NOP, and consider the Status and Configuration Register bits protection enabled by default when not in Quad Mode.
>>>>>
>>>>> Cheers,
>>>>> ta
>>>>>
>>>>> This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message.
>>>>>
> 
> ______________________________________________________
> Linux MTD discussion mailing list
> http://lists.infradead.org/mailman/listinfo/linux-mtd/
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/