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] [day] [month] [year] [list]
Message-ID: <SJ0PR02MB844910CF4F8034D2FD27E882CD229@SJ0PR02MB8449.namprd02.prod.outlook.com>
Date:   Fri, 28 Jan 2022 06:37:54 +0000
From:   "Sajida Bhanu (Temp) (QUIC)" <quic_c_sbhanu@...cinc.com>
To:     Michael Walle <michael@...le.cc>,
        "Sajida Bhanu (Temp) (QUIC)" <quic_c_sbhanu@...cinc.com>
CC:     "dianders@...omium.org" <dianders@...omium.org>,
        "tudor.ambarus@...rochip.com" <tudor.ambarus@...rochip.com>,
        "p.yadav@...com" <p.yadav@...com>,
        "miquel.raynal@...tlin.com" <miquel.raynal@...tlin.com>,
        "richard@....at" <richard@....at>,
        "vigneshr@...com" <vigneshr@...com>,
        "linux-mtd@...ts.infradead.org" <linux-mtd@...ts.infradead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "stummala@...eaurora.org" <stummala@...eaurora.org>,
        "vbadigan@...eaurora.org" <vbadigan@...eaurora.org>,
        "Ram Prakash Gupta (QUIC)" <quic_rampraka@...cinc.com>,
        "Pradeep Pragallapati (QUIC)" <quic_pragalla@...cinc.com>,
        "sartgarg@...eaurora.org" <sartgarg@...eaurora.org>
Subject: RE: [PATCH V4] mtd: spi-nor: winbond: Add support for winbond chip

HI,

Sure thanks for the info .. will add the change and test it.

Thanks,
Sajida
-----Original Message-----
From: Michael Walle <michael@...le.cc> 
Sent: Tuesday, January 25, 2022 9:23 PM
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