linux-kernel - Re: [PATCH V4] mtd: spi-nor: winbond: Add support for winbond chip

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ce590c8dc41c64bc56d9c241f4c7dd9a@walle.cc>
Date:   Tue, 25 Jan 2022 16:52:41 +0100
From:   Michael Walle <michael@...le.cc>
To:     "Sajida Bhanu (Temp) (QUIC)" <quic_c_sbhanu@...cinc.com>
Cc:     dianders@...omium.org, tudor.ambarus@...rochip.com, p.yadav@...com,
        miquel.raynal@...tlin.com, richard@....at, vigneshr@...com,
        linux-mtd@...ts.infradead.org, linux-kernel@...r.kernel.org,
        stummala@...eaurora.org, vbadigan@...eaurora.org,
        "Ram Prakash Gupta (QUIC)" <quic_rampraka@...cinc.com>,
        "Pradeep Pragallapati (QUIC)" <quic_pragalla@...cinc.com>,
        sartgarg@...eaurora.org
Subject: Re: [PATCH V4] mtd: spi-nor: winbond: Add support for winbond chip

Hi,

Am 2022-01-25 16:36, schrieb Sajida Bhanu (Temp) (QUIC):
> Am 2022-01-20 10:54, schrieb Shaik Sajida Bhanu:
>> Add support for winbond W25Q512NW-IM chip.
>> 
>> Signed-off-by: Shaik Sajida Bhanu <quic_c_sbhanu@...cinc.com>
>> Reviewed-by: Doug Anderson <dianders@...omium.org>
>> ---
>> 
>> Changes since V1:
>> 	- Added space before name of the flash part as suggested by Doug.
>> 
>> Changes since V2:
>> 	- Updated chip name as w25q512nwm as suggested by Doug.
>> 
>> Changes since V3:
>> 	- Updated flash_info flags passing according to below patch
> 
> Thanks!
> 
> Please note, that you also have to supply a SFDP dump, see [1].
> 
>> https://lore.kernel.org/all/20211207140254.87681-7-tudor.ambarus@microchip.com/
>> 	 As suggested by Tudor Ambarus.
>> ---
>>  drivers/mtd/spi-nor/winbond.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>> 
>> diff --git a/drivers/mtd/spi-nor/winbond.c
>> b/drivers/mtd/spi-nor/winbond.c index 675f32c..c4be225 100644
>> --- a/drivers/mtd/spi-nor/winbond.c
>> +++ b/drivers/mtd/spi-nor/winbond.c
>> @@ -124,6 +124,10 @@ static const struct flash_info winbond_parts[] = 
>> {
>>  	{ "w25m512jv", INFO(0xef7119, 0, 64 * 1024, 1024)
>>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_QUAD_READ |
>>  			      SPI_NOR_DUAL_READ) },
>> +	{ "w25q512nwm", INFO(0xef8020, 0, 64 * 1024, 1024)
>> +		FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>> +		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ |
>> +			      SPI_NOR_QUAD_READ) },
> 
> Could you try adding OTP_INFO(256, 3, 0x1000, 0x1000), this should
> enable OTP support. Could you please test this using the
> flash_otp_{dump,info,erase,write} tools and add that line?
> 
> I've checked ID duplications, because there is the w25qNNjw series.
> There doesn't seem to exist a w25q512jw, so we are safe for now. There
> is only a w25q512jv and it has the id 0xef4020.
> 
> fun fact.. the w25q512nwm describes the OTP lock bit for the first OTP
> region (the one which is not documented and I've found out that its
> used for storing the SFDP) as SFDP lock bit. See ch "7.1.1 Security
> Register Lock Bits (LB3, LB2, LB1, SFDP Lock bit)". So we finally have
> evidence :)
> 
> -michael
> 
> Hi,
> 
> Thanks for the review..
> 
> Can you please share the documentation about OTP_INFO
> settings(Advantages etc..) if any.

I would like to see support for OTP right from the start, and
because you have that specific flash chip, you are in the position
to test it :)

With this line (it should already be exact that line I provided,
I checked the datasheet), you just enable access to a seperate
OTP memory inside that flash. The datasheet calls them Security
Registers, you can have a look there.

> Can you please share more info on
> flash_otp_{dump,info,erase,write} tools.

What do you mean? You can find these tools in the mtd-utils
package or at http://www.linux-mtd.infradead.org/ (couldn't check,
website seems to be down for me at least).

You should make sure to have the latest version, which has a
bugfix for the flash_otp_dump and only newer versions have the
flash_otp_erase tool.

Examples:

# flash_otp_dump -u /dev/mtd0 0
OTP user data for /dev/mtd0
0x0000: 56 01 55 53 44 30 47 30 30 30 32 00 00 00 00 00
0x0010: 00 00 a0 a5 5c 6b 62 7b ff ff ff ff ff ff ff ff
0x0020: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x0030: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x0040: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[..]

# flash_otp_info -u /dev/mtd0
Number of OTP user blocks on /dev/mtd0: 3
block  0:  offset = 0x0000  size = 256 bytes  [locked]
block  1:  offset = 0x0100  size = 256 bytes  [unlocked]
block  2:  offset = 0x0200  size = 256 bytes  [unlocked]

# echo -n hello world | flash_otp_write -u /dev/mtd0 0x100
Writing OTP user data on /dev/mtd0 at offset 0x100
Wrote 11 bytes of OTP user data

# flash_otp_dump -u /dev/mtd0 0x100
OTP user data for /dev/mtd0
0x0100: 68 65 6c 6c 6f 20 77 6f 72 6c 64 ff ff ff ff ff
0x0110: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x0120: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[..]

# flash_otp_erase -u /dev/mtd0 0x100 0x200
# flash_otp_dump -u /dev/mtd0 0x100
OTP user data for /dev/mtd0
0x0100: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x0110: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
0x0120: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff
[..]

For completeness, there is also flash_otp_lock, but that
is not reversible and you'd lock the OTP area forever.

HTH,
-michael