[PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Mamta Shukla]

mt25qu512a[1] supports locking/unlocking through BP bits in SR.

Tested using mtd-utils- flash_lock/flash_unlock for MT25QU512ABB8E12.

Link: https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211
Signed-off-by: Mamta Shukla <mamta.shukla@leica-geosystems.com>
---
 Changes in v2:
 - add Link tag
 - fix chip part number mt25ql512a->mt25qu512a

 drivers/mtd/spi-nor/micron-st.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 4b919756a205..08e94340ebaa 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -229,6 +229,8 @@ static const struct flash_info st_nor_parts[] = {
 		MFR_FLAGS(USE_FSR)
 	},
 	{ "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024)
+		FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
+		      SPI_NOR_BP3_SR_BIT6)
 		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
 		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
 		MFR_FLAGS(USE_FSR)
-- 
2.25.1


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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Michael Walle]

Am 2023-07-05 17:49, schrieb Mamta Shukla:
> mt25qu512a[1] supports locking/unlocking through BP bits in SR.
> 
> Tested using mtd-utils- flash_lock/flash_unlock for MT25QU512ABB8E12.
> 
> Link: 
> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211
> Signed-off-by: Mamta Shukla <mamta.shukla@leica-geosystems.com>

Reviewed-by: Michael Walle <michael@walle.cc>

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

On 05.07.2023 18:49, Mamta Shukla wrote:
> mt25qu512a[1] supports locking/unlocking through BP bits in SR.
> 
> Tested using mtd-utils- flash_lock/flash_unlock for MT25QU512ABB8E12.
> 
> Link: https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211
> Signed-off-by: Mamta Shukla <mamta.shukla@leica-geosystems.com>
> ---
>  Changes in v2:
>  - add Link tag
>  - fix chip part number mt25ql512a->mt25qu512a
> 
>  drivers/mtd/spi-nor/micron-st.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
> index 4b919756a205..08e94340ebaa 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -229,6 +229,8 @@ static const struct flash_info st_nor_parts[] = {
>  		MFR_FLAGS(USE_FSR)
>  	},
>  	{ "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024)
> +		FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
> +		      SPI_NOR_BP3_SR_BIT6)
>  		NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>  		FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>  		MFR_FLAGS(USE_FSR)

Can you try the following instead? We try to use SFDP parsing whenever
possible.
	{ "mt25qu512a",  INFO6(0x20bb20, 0x104400, 0, 0)
		FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
		SPI_NOR_BP3_SR_BIT6)
		PARSE_SFDP
		MFR_FLAGS(USE_FSR)

You then have to dump the SFDP tables and do a sanity check to make sure
everything's fine after your changes. You can an example on how to do
that on the commit message from
https://lore.kernel.org/linux-mtd/8b7d2420-26b6-405b-8ef7-75e0a0201402@linaro.org/T/#m3550973e0884ec4a288d344fabd4a9c3b64af46e

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

Hello Tudor,

On 13.07.23 05:43, Tudor Ambarus wrote:
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
> 
> 
> On 05.07.2023 18:49, Mamta Shukla wrote:
>> mt25qu512a[1] supports locking/unlocking through BP bits in SR.
>>
>> Tested using mtd-utils- flash_lock/flash_unlock for MT25QU512ABB8E12.
>>
>> Link: https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211
>> Signed-off-by: Mamta Shukla <mamta.shukla@leica-geosystems.com>
>> ---
>>   Changes in v2:
>>   - add Link tag
>>   - fix chip part number mt25ql512a->mt25qu512a
>>
>>   drivers/mtd/spi-nor/micron-st.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>> index 4b919756a205..08e94340ebaa 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -229,6 +229,8 @@ static const struct flash_info st_nor_parts[] = {
>>                MFR_FLAGS(USE_FSR)
>>        },
>>        { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024)
>> +             FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>> +                   SPI_NOR_BP3_SR_BIT6)
>>                NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>>                FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>>                MFR_FLAGS(USE_FSR)
> 
> Can you try the following instead? We try to use SFDP parsing whenever
> possible.
>          { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 0, 0)
>                  FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>                  SPI_NOR_BP3_SR_BIT6)
>                  PARSE_SFDP
>                  MFR_FLAGS(USE_FSR)


I tested with SFDP Parsing Flag. It works fine.

---------------------------------------------------------------------------------
[  214.726090] ACPI: Host-directed Dynamic ACPI Table Load:
[  214.731482] ACPI: SSDT 0xFFFF892882D89800 0000EC (v02 ALASKA MT25QU 
00001000 INTL 20190509)
[  214.766082] spi-nor spi-PRP0001:00: mt25qu512a (65536 Kbytes)


cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/jedec_id
20bb20104400

cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/manufacturer
st

cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/partname
mt25qu512a

xxd -p  /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
53464450060101ff00060110300000ff84000102800000ffffffffffffff
ffffffffffffffffffffffffffffffffffffe520fbffffffff1f29eb276b
273b27bbffffffffffff27bbffff29eb0c2010d80f520000244a99008b8e
03e1ac0127387a757a75fbbdd55c4a0f82ff81bd3d36ffffffffffffffff
ffffffffffffffffffe7ffff21dcffff

md5sum  /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
610efba1647e00ac6db18beb11e84c04 
/sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp

dd if=/dev/urandom of=/tmp/qspi_write  bs=1M count=1
1+0 records in
1+0 records out
1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0221391 s, 47.4 MB/s

mtd_debug write /dev/mtd0 0 1048576 /tmp/qspi_write
Copied 1048576 bytes from /tmp/qspi_write to address 0x00000000 in flash

mtd_debug erase /dev/mtd0 0 1048576
Erased 1048576 bytes from address 0x00000000 in flash

mtd_debug read /dev/mtd0 0 1048576 /tmp/qspi_read
Copied 1048576 bytes from address 0x00000000 in flash to /tmp/qspi_read

hexdump /tmp/qspi_read
0000000 ffff ffff ffff ffff ffff ffff ffff ffff
*
0100000

mtd_debug write /dev/mtd0 0 1048576 /tmp/qspi_write
Copied 1048576 bytes from /tmp/qspi_write to address 0x00000000 in flash

mtd_debug read /dev/mtd0 0 1048576 /tmp/qspi_read
Copied 1048576 bytes from address 0x00000000 in flash to /tmp/qspi_read

sha1sum /tmp/qspi_write  /tmp/qspi_read
4fe4e71b11f44e9672bd49e2e32c0fd94da4feb6  /tmp/qspi_write
4fe4e71b11f44e9672bd49e2e32c0fd94da4feb6  /tmp/qspi_read

----------------------------------------------------------------------------------

Shall I write a new commit for PARSE_SFDP Flag or update this commit as-
"Modify mt25qu512a Flags" ?

> 
> You then have to dump the SFDP tables and do a sanity check to make sure
> everything's fine after your changes. You can an example on how to do
> that on the commit message from
> https://lore.kernel.org/linux-mtd/8b7d2420-26b6-405b-8ef7-75e0a0201402@linaro.org/T/#m3550973e0884ec4a288d344fabd4a9c3b64af46e

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

On 14.07.23 10:15, SHUKLA Mamta Ramendra wrote:
> Hello Tudor,
> 
> On 13.07.23 05:43, Tudor Ambarus wrote:
>> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>>
>>
>> On 05.07.2023 18:49, Mamta Shukla wrote:
>>> mt25qu512a[1] supports locking/unlocking through BP bits in SR.
>>>
>>> Tested using mtd-utils- flash_lock/flash_unlock for MT25QU512ABB8E12.
>>>
>>> Link: https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211
>>> Signed-off-by: Mamta Shukla <mamta.shukla@leica-geosystems.com>
>>> ---
>>>    Changes in v2:
>>>    - add Link tag
>>>    - fix chip part number mt25ql512a->mt25qu512a
>>>
>>>    drivers/mtd/spi-nor/micron-st.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>>> index 4b919756a205..08e94340ebaa 100644
>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>> @@ -229,6 +229,8 @@ static const struct flash_info st_nor_parts[] = {
>>>                 MFR_FLAGS(USE_FSR)
>>>         },
>>>         { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024)
>>> +             FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>>> +                   SPI_NOR_BP3_SR_BIT6)
>>>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>>>                 FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>>>                 MFR_FLAGS(USE_FSR)
>>
>> Can you try the following instead? We try to use SFDP parsing whenever
>> possible.
>>           { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 0, 0)
>>                   FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>>                   SPI_NOR_BP3_SR_BIT6)
>>                   PARSE_SFDP
>>                   MFR_FLAGS(USE_FSR)
> 
> 
> I tested with SFDP Parsing Flag. It works fine.
> 
> ---------------------------------------------------------------------------------
> [  214.726090] ACPI: Host-directed Dynamic ACPI Table Load:
> [  214.731482] ACPI: SSDT 0xFFFF892882D89800 0000EC (v02 ALASKA MT25QU
> 00001000 INTL 20190509)
> [  214.766082] spi-nor spi-PRP0001:00: mt25qu512a (65536 Kbytes)
> 
> 
> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/jedec_id
> 20bb20104400
> 
> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/manufacturer
> st
> 
> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/partname
> mt25qu512a
> 
> xxd -p  /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
> 53464450060101ff00060110300000ff84000102800000ffffffffffffff
> ffffffffffffffffffffffffffffffffffffe520fbffffffff1f29eb276b
> 273b27bbffffffffffff27bbffff29eb0c2010d80f520000244a99008b8e
> 03e1ac0127387a757a75fbbdd55c4a0f82ff81bd3d36ffffffffffffffff
> ffffffffffffffffffe7ffff21dcffff
> 
> md5sum  /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
> 610efba1647e00ac6db18beb11e84c04
> /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
> 
> dd if=/dev/urandom of=/tmp/qspi_write  bs=1M count=1
> 1+0 records in
> 1+0 records out
> 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0221391 s, 47.4 MB/s
> 
> mtd_debug write /dev/mtd0 0 1048576 /tmp/qspi_write
> Copied 1048576 bytes from /tmp/qspi_write to address 0x00000000 in flash
> 
> mtd_debug erase /dev/mtd0 0 1048576
> Erased 1048576 bytes from address 0x00000000 in flash
> 
> mtd_debug read /dev/mtd0 0 1048576 /tmp/qspi_read
> Copied 1048576 bytes from address 0x00000000 in flash to /tmp/qspi_read
> 
> hexdump /tmp/qspi_read
> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff
> *
> 0100000
> 
> mtd_debug write /dev/mtd0 0 1048576 /tmp/qspi_write
> Copied 1048576 bytes from /tmp/qspi_write to address 0x00000000 in flash
> 
> mtd_debug read /dev/mtd0 0 1048576 /tmp/qspi_read
> Copied 1048576 bytes from address 0x00000000 in flash to /tmp/qspi_read
> 
> sha1sum /tmp/qspi_write  /tmp/qspi_read
> 4fe4e71b11f44e9672bd49e2e32c0fd94da4feb6  /tmp/qspi_write
> 4fe4e71b11f44e9672bd49e2e32c0fd94da4feb6  /tmp/qspi_read
> 
> ----------------------------------------------------------------------------------
> 
> Shall I write a new commit for PARSE_SFDP Flag or update this commit as-
> "Modify mt25qu512a Flags" ?


Ping! Just to follow up on this.

>>
>> You then have to dump the SFDP tables and do a sanity check to make sure
>> everything's fine after your changes. You can an example on how to do
>> that on the commit message from
>> https://lore.kernel.org/linux-mtd/8b7d2420-26b6-405b-8ef7-75e0a0201402@linaro.org/T/#m3550973e0884ec4a288d344fabd4a9c3b64af46e
> 
> Thanks,
> Mamta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

Hi, Shukla,

Sorry, this email somehow got off my radar.

On 8/22/23 13:16, SHUKLA Mamta Ramendra wrote:
> 
> On 14.07.23 10:15, SHUKLA Mamta Ramendra wrote:
>> Hello Tudor,
>>
>> On 13.07.23 05:43, Tudor Ambarus wrote:
>>> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>>>
>>>
>>> On 05.07.2023 18:49, Mamta Shukla wrote:
>>>> mt25qu512a[1] supports locking/unlocking through BP bits in SR.
>>>>
>>>> Tested using mtd-utils- flash_lock/flash_unlock for MT25QU512ABB8E12.
>>>>
>>>> Link: https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211
>>>> Signed-off-by: Mamta Shukla <mamta.shukla@leica-geosystems.com>
>>>> ---
>>>>    Changes in v2:
>>>>    - add Link tag
>>>>    - fix chip part number mt25ql512a->mt25qu512a
>>>>
>>>>    drivers/mtd/spi-nor/micron-st.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>>>> index 4b919756a205..08e94340ebaa 100644
>>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>>> @@ -229,6 +229,8 @@ static const struct flash_info st_nor_parts[] = {
>>>>                 MFR_FLAGS(USE_FSR)
>>>>         },
>>>>         { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024)
>>>> +             FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>>>> +                   SPI_NOR_BP3_SR_BIT6)
>>>>                 NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>>>>                 FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>>>>                 MFR_FLAGS(USE_FSR)
>>>
>>> Can you try the following instead? We try to use SFDP parsing whenever
>>> possible.
>>>           { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 0, 0)
>>>                   FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>>>                   SPI_NOR_BP3_SR_BIT6)
>>>                   PARSE_SFDP
>>>                   MFR_FLAGS(USE_FSR)
>>
>>
>> I tested with SFDP Parsing Flag. It works fine.
>>
>> ---------------------------------------------------------------------------------
>> [  214.726090] ACPI: Host-directed Dynamic ACPI Table Load:
>> [  214.731482] ACPI: SSDT 0xFFFF892882D89800 0000EC (v02 ALASKA MT25QU
>> 00001000 INTL 20190509)
>> [  214.766082] spi-nor spi-PRP0001:00: mt25qu512a (65536 Kbytes)
>>
>>
>> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/jedec_id
>> 20bb20104400
>>
>> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/manufacturer
>> st
>>
>> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/partname
>> mt25qu512a
>>
>> xxd -p  /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
>> 53464450060101ff00060110300000ff84000102800000ffffffffffffff
>> ffffffffffffffffffffffffffffffffffffe520fbffffffff1f29eb276b
>> 273b27bbffffffffffff27bbffff29eb0c2010d80f520000244a99008b8e
>> 03e1ac0127387a757a75fbbdd55c4a0f82ff81bd3d36ffffffffffffffff
>> ffffffffffffffffffe7ffff21dcffff
>>
>> md5sum  /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
>> 610efba1647e00ac6db18beb11e84c04
>> /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
>>
>> dd if=/dev/urandom of=/tmp/qspi_write  bs=1M count=1
>> 1+0 records in
>> 1+0 records out
>> 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0221391 s, 47.4 MB/s
>>
>> mtd_debug write /dev/mtd0 0 1048576 /tmp/qspi_write
>> Copied 1048576 bytes from /tmp/qspi_write to address 0x00000000 in flash
>>
>> mtd_debug erase /dev/mtd0 0 1048576
>> Erased 1048576 bytes from address 0x00000000 in flash
>>
>> mtd_debug read /dev/mtd0 0 1048576 /tmp/qspi_read
>> Copied 1048576 bytes from address 0x00000000 in flash to /tmp/qspi_read
>>
>> hexdump /tmp/qspi_read
>> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff
>> *
>> 0100000
>>
>> mtd_debug write /dev/mtd0 0 1048576 /tmp/qspi_write
>> Copied 1048576 bytes from /tmp/qspi_write to address 0x00000000 in flash
>>
>> mtd_debug read /dev/mtd0 0 1048576 /tmp/qspi_read
>> Copied 1048576 bytes from address 0x00000000 in flash to /tmp/qspi_read
>>
>> sha1sum /tmp/qspi_write  /tmp/qspi_read
>> 4fe4e71b11f44e9672bd49e2e32c0fd94da4feb6  /tmp/qspi_write
>> 4fe4e71b11f44e9672bd49e2e32c0fd94da4feb6  /tmp/qspi_read
>>
>> ----------------------------------------------------------------------------------
>>
>> Shall I write a new commit for PARSE_SFDP Flag or update this commit as-
>> "Modify mt25qu512a Flags" ?
> 
> 
> Ping! Just to follow up on this.
> 

Yeah, please make 2 commits so that we can cleanly revert the switching
to SFDP, if ever needed.

So the first commit will look like:
diff --git a/drivers/mtd/spi-nor/micron-st.c
b/drivers/mtd/spi-nor/micron-st.c
index 4afcfc57c896..a8da1f18e335 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -405,9 +405,6 @@ static const struct flash_info st_nor_parts[] = {
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
                .name = "mt25qu512a",
-               .size = SZ_64M,
-               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
SPI_NOR_QUAD_READ,
-               .fixup_flags = SPI_NOR_4B_OPCODES,
                .mfr_flags = USE_FSR,
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x20),

and the second one will add just the BP support, something like:
diff --git a/drivers/mtd/spi-nor/micron-st.c
b/drivers/mtd/spi-nor/micron-st.c
index a8da1f18e335..fdafbfa0f936 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
                .name = "mt25qu512a",
+               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
SPI_NOR_4BIT_BP |
+                        SPI_NOR_BP3_SR_BIT6,
                .mfr_flags = USE_FSR,
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x20),

Of course, I expect you to run again the mtd_debug tests and also verify
the locking. Thanks!

Cheers,
ta


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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

On 9/21/23 15:31, Tudor Ambarus wrote:
cut
> 
> So the first commit will look like:
> diff --git a/drivers/mtd/spi-nor/micron-st.c
> b/drivers/mtd/spi-nor/micron-st.c
> index 4afcfc57c896..a8da1f18e335 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -405,9 +405,6 @@ static const struct flash_info st_nor_parts[] = {
>         }, {
>                 .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>                 .name = "mt25qu512a",
> -               .size = SZ_64M,
> -               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
> SPI_NOR_QUAD_READ,
> -               .fixup_flags = SPI_NOR_4B_OPCODES,

since we removed the 4b-opcodes flag here,
>                 .mfr_flags = USE_FSR,
>         }, {
>                 .id = SNOR_ID(0x20, 0xbb, 0x20),
> 
> and the second one will add just the BP support, something like:
> diff --git a/drivers/mtd/spi-nor/micron-st.c
> b/drivers/mtd/spi-nor/micron-st.c
> index a8da1f18e335..fdafbfa0f936 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
>         }, {
>                 .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>                 .name = "mt25qu512a",
> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
> SPI_NOR_4BIT_BP |
> +                        SPI_NOR_BP3_SR_BIT6,
>                 .mfr_flags = USE_FSR,
>         }, {
>                 .id = SNOR_ID(0x20, 0xbb, 0x20),
> 
> Of course, I expect you to run again the mtd_debug tests and also verify
> the locking. Thanks!

would also be good if you can also verify that 4BAIT SFDP table is
present and you still use the 4B opcodes after the change.

> 
> Cheers,
> ta
> 

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

Hello Tudor,


Thanks for your Feedback.

I rebased the patch on spi-nor/next 
[https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=spi-nor/next&id=8f407eda173f1d43466636314c7aa30405e4dd67] 
to align with new flash_info format.

On 21.09.23 16:31, Tudor Ambarus wrote:
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
> 
> 
> Hi, Shukla,
> 
> Sorry, this email somehow got off my radar.
> 
> On 8/22/23 13:16, SHUKLA Mamta Ramendra wrote:
>>
>> On 14.07.23 10:15, SHUKLA Mamta Ramendra wrote:
>>> Hello Tudor,
>>>
>>> On 13.07.23 05:43, Tudor Ambarus wrote:
>>>> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>>>>
>>>>
>>>> On 05.07.2023 18:49, Mamta Shukla wrote:
>>>>> mt25qu512a[1] supports locking/unlocking through BP bits in SR.
>>>>>
>>>>> Tested using mtd-utils- flash_lock/flash_unlock for MT25QU512ABB8E12.
>>>>>
>>>>> Link: https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211
>>>>> Signed-off-by: Mamta Shukla <mamta.shukla@leica-geosystems.com>
>>>>> ---
>>>>>     Changes in v2:
>>>>>     - add Link tag
>>>>>     - fix chip part number mt25ql512a->mt25qu512a
>>>>>
>>>>>     drivers/mtd/spi-nor/micron-st.c | 2 ++
>>>>>     1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>>>>> index 4b919756a205..08e94340ebaa 100644
>>>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>>>> @@ -229,6 +229,8 @@ static const struct flash_info st_nor_parts[] = {
>>>>>                  MFR_FLAGS(USE_FSR)
>>>>>          },
>>>>>          { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 64 * 1024, 1024)
>>>>> +             FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>>>>> +                   SPI_NOR_BP3_SR_BIT6)
>>>>>                  NO_SFDP_FLAGS(SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)
>>>>>                  FIXUP_FLAGS(SPI_NOR_4B_OPCODES)
>>>>>                  MFR_FLAGS(USE_FSR)
>>>>
>>>> Can you try the following instead? We try to use SFDP parsing whenever
>>>> possible.
>>>>            { "mt25qu512a",  INFO6(0x20bb20, 0x104400, 0, 0)
>>>>                    FLAGS(SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | SPI_NOR_4BIT_BP |
>>>>                    SPI_NOR_BP3_SR_BIT6)
>>>>                    PARSE_SFDP
>>>>                    MFR_FLAGS(USE_FSR)
>>>
>>>
>>> I tested with SFDP Parsing Flag. It works fine.
>>>
>>> ---------------------------------------------------------------------------------
>>> [  214.726090] ACPI: Host-directed Dynamic ACPI Table Load:
>>> [  214.731482] ACPI: SSDT 0xFFFF892882D89800 0000EC (v02 ALASKA MT25QU
>>> 00001000 INTL 20190509)
>>> [  214.766082] spi-nor spi-PRP0001:00: mt25qu512a (65536 Kbytes)
>>>
>>>
>>> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/jedec_id
>>> 20bb20104400
>>>
>>> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/manufacturer
>>> st
>>>
>>> cat /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/partname
>>> mt25qu512a
>>>
>>> xxd -p  /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
>>> 53464450060101ff00060110300000ff84000102800000ffffffffffffff
>>> ffffffffffffffffffffffffffffffffffffe520fbffffffff1f29eb276b
>>> 273b27bbffffffffffff27bbffff29eb0c2010d80f520000244a99008b8e
>>> 03e1ac0127387a757a75fbbdd55c4a0f82ff81bd3d36ffffffffffffffff
>>> ffffffffffffffffffe7ffff21dcffff
>>>
>>> md5sum  /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
>>> 610efba1647e00ac6db18beb11e84c04
>>> /sys/bus/spi/devices/spi-PRP0001:00/spi-nor/sfdp
>>>
>>> dd if=/dev/urandom of=/tmp/qspi_write  bs=1M count=1
>>> 1+0 records in
>>> 1+0 records out
>>> 1048576 bytes (1.0 MB, 1.0 MiB) copied, 0.0221391 s, 47.4 MB/s
>>>
>>> mtd_debug write /dev/mtd0 0 1048576 /tmp/qspi_write
>>> Copied 1048576 bytes from /tmp/qspi_write to address 0x00000000 in flash
>>>
>>> mtd_debug erase /dev/mtd0 0 1048576
>>> Erased 1048576 bytes from address 0x00000000 in flash
>>>
>>> mtd_debug read /dev/mtd0 0 1048576 /tmp/qspi_read
>>> Copied 1048576 bytes from address 0x00000000 in flash to /tmp/qspi_read
>>>
>>> hexdump /tmp/qspi_read
>>> 0000000 ffff ffff ffff ffff ffff ffff ffff ffff
>>> *
>>> 0100000
>>>
>>> mtd_debug write /dev/mtd0 0 1048576 /tmp/qspi_write
>>> Copied 1048576 bytes from /tmp/qspi_write to address 0x00000000 in flash
>>>
>>> mtd_debug read /dev/mtd0 0 1048576 /tmp/qspi_read
>>> Copied 1048576 bytes from address 0x00000000 in flash to /tmp/qspi_read
>>>
>>> sha1sum /tmp/qspi_write  /tmp/qspi_read
>>> 4fe4e71b11f44e9672bd49e2e32c0fd94da4feb6  /tmp/qspi_write
>>> 4fe4e71b11f44e9672bd49e2e32c0fd94da4feb6  /tmp/qspi_read
>>>
>>> ----------------------------------------------------------------------------------
>>>
>>> Shall I write a new commit for PARSE_SFDP Flag or update this commit as-
>>> "Modify mt25qu512a Flags" ?
>>
>>
>> Ping! Just to follow up on this.
>>
> 
> Yeah, please make 2 commits so that we can cleanly revert the switching
> to SFDP, if ever needed.
> 
> So the first commit will look like:
> diff --git a/drivers/mtd/spi-nor/micron-st.c
> b/drivers/mtd/spi-nor/micron-st.c
> index 4afcfc57c896..a8da1f18e335 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -405,9 +405,6 @@ static const struct flash_info st_nor_parts[] = {
>          }, {
>                  .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>                  .name = "mt25qu512a",
> -               .size = SZ_64M,
> -               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
> SPI_NOR_QUAD_READ,
> -               .fixup_flags = SPI_NOR_4B_OPCODES,
>                  .mfr_flags = USE_FSR,
>          }, {
>                  .id = SNOR_ID(0x20, 0xbb, 0x20),
> 
> and the second one will add just the BP support, something like:
> diff --git a/drivers/mtd/spi-nor/micron-st.c
> b/drivers/mtd/spi-nor/micron-st.c
> index a8da1f18e335..fdafbfa0f936 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
>          }, {
>                  .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>                  .name = "mt25qu512a",
> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
> SPI_NOR_4BIT_BP |
> +                        SPI_NOR_BP3_SR_BIT6,
>                  .mfr_flags = USE_FSR,
>          }, {
>                  .id = SNOR_ID(0x20, 0xbb, 0x20),
> 
> Of course, I expect you to run again the mtd_debug tests and also verify
> the locking. Thanks!

I applied both changes as mentioned above i.e
1] Switch to SFDP and 2] Using BP Flags.

Case 1: Use BP Flags and Switch to SFDP
With both these changes, the lock/unlock doesn't work.

## x86-64-smarc-evk-uwd0j0007 # uname -r
6.6.0-rc2

# flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x4000000
Lock status: unlocked
Return code: 0
# flash_lock -l /dev/mtd0
flash_lock: error!: could not lock device: /dev/mtd0

             error 5 (Input/output error)


I suspected this is because of miscalculation of BP bits, like the 
possibility mentioned here: 
https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=spi-nor/next&id=3ea3f0ac242c86c0275d347ab8c92bf1eb854b49 


But further checked size, it is correct:

  # mtd_debug info /dev/mtd0
mtd.type = MTD_NORFLASH
mtd.flags = MTD_CAP_NORFLASH
mtd.size = 67108864 (64M)
mtd.erasesize = 4096 (4K)
mtd.writesize = 1
mtd.oobsize = 0
regions = 0

And rest of read/write functions work as expected.

Any suggestions about this?

Case 2: Just added BP flags, rest of the size, mfr_flags, fixup flags 
kept as it is.
Lock/unlock works.

## x86-64-smarc-evk-uwd0j0007 # uname -r
6.6.0-rc2

# flash_lock -i /dev/mtd0
Start: 0
Len: 0x4000000
Lock status: unlocked
Return code: 0

# flash_lock -l /dev/mtd0
# flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x4000000
Lock status: locked
Return code: 1

## x86-64-smarc-evk-uwd0j0007 # mtd_debug erase /dev/mtd0 0 1048576
[  413.472411] spi-nor spi-PRP0001:00: Erase operation failed.
[  413.478084] spi-nor spi-PRP0001:00: Attempted to modify a protected 
sector.
MEMERASE: Input/output error

# flash_lock -u /dev/mtd0
# flash_lock -i /dev/mtd0
Device: /dev/mtd0
Start: 0
Len: 0x4000000
Lock status: unlocked
Return code: 0

## x86-64-smarc-evk-uwd0j0007 #  mtd_debug erase /dev/mtd0 0 1048576
Erased 1048576 bytes from address 0x00000000 in flash


Further I tested on stable 6.5.5 Kernel with old way of Flash Info 
Format and which has forced PARSE_SFDP Flag, no issues with lock/unlock.


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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

On 10/5/23 10:21, SHUKLA Mamta Ramendra wrote:

cut

>>
>> and the second one will add just the BP support, something like:
>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>> b/drivers/mtd/spi-nor/micron-st.c
>> index a8da1f18e335..fdafbfa0f936 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
>>          }, {
>>                  .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>>                  .name = "mt25qu512a",
>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>> SPI_NOR_4BIT_BP |
>> +                        SPI_NOR_BP3_SR_BIT6,
>>                  .mfr_flags = USE_FSR,
>>          }, {
>>                  .id = SNOR_ID(0x20, 0xbb, 0x20),
>>
>> Of course, I expect you to run again the mtd_debug tests and also verify
>> the locking. Thanks!
> 
> I applied both changes as mentioned above i.e
> 1] Switch to SFDP and 2] Using BP Flags.
> 
> Case 1: Use BP Flags and Switch to SFDP
> With both these changes, the lock/unlock doesn't work.
> 
> ## x86-64-smarc-evk-uwd0j0007 # uname -r
> 6.6.0-rc2
> 
> # flash_lock -i /dev/mtd0
> Device: /dev/mtd0
> Start: 0
> Len: 0x4000000
> Lock status: unlocked
> Return code: 0
> # flash_lock -l /dev/mtd0
> flash_lock: error!: could not lock device: /dev/mtd0
> 
>              error 5 (Input/output error)
> 
> 
> I suspected this is because of miscalculation of BP bits, like the 
> possibility mentioned here: 
> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=spi-nor/next&id=3ea3f0ac242c86c0275d347ab8c92bf1eb854b49 
> 
> 
> But further checked size, it is correct:
> 
>   # mtd_debug info /dev/mtd0
> mtd.type = MTD_NORFLASH
> mtd.flags = MTD_CAP_NORFLASH
> mtd.size = 67108864 (64M)
> mtd.erasesize = 4096 (4K)
> mtd.writesize = 1
> mtd.oobsize = 0
> regions = 0
> 
> And rest of read/write functions work as expected.
> 
> Any suggestions about this?
> 
> Case 2: Just added BP flags, rest of the size, mfr_flags, fixup flags 
> kept as it is.

would you please detail what exact definitions you used in case 2? Send
us the diff please.

Cheers,
ta

> Lock/unlock works.
> 
> ## x86-64-smarc-evk-uwd0j0007 # uname -r
> 6.6.0-rc2
> 
> # flash_lock -i /dev/mtd0
> Start: 0
> Len: 0x4000000
> Lock status: unlocked
> Return code: 0
> 
> # flash_lock -l /dev/mtd0
> # flash_lock -i /dev/mtd0
> Device: /dev/mtd0
> Start: 0
> Len: 0x4000000
> Lock status: locked
> Return code: 1
> 
> ## x86-64-smarc-evk-uwd0j0007 # mtd_debug erase /dev/mtd0 0 1048576
> [  413.472411] spi-nor spi-PRP0001:00: Erase operation failed.
> [  413.478084] spi-nor spi-PRP0001:00: Attempted to modify a protected 
> sector.
> MEMERASE: Input/output error
> 
> # flash_lock -u /dev/mtd0
> # flash_lock -i /dev/mtd0
> Device: /dev/mtd0
> Start: 0
> Len: 0x4000000
> Lock status: unlocked
> Return code: 0
> 
> ## x86-64-smarc-evk-uwd0j0007 #  mtd_debug erase /dev/mtd0 0 1048576
> Erased 1048576 bytes from address 0x00000000 in flash
> 
> 
> Further I tested on stable 6.5.5 Kernel with old way of Flash Info 
> Format and which has forced PARSE_SFDP Flag, no issues with lock/unlock.
> 
> 
> ---
> Mamta

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

On 05.10.23 15:51, Tudor Ambarus wrote:
> [Some people who received this message don't often get email from tudor.ambarus@linaro.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
> 
> 
> On 10/5/23 10:21, SHUKLA Mamta Ramendra wrote:
> 
> cut
> 
>>>
>>> and the second one will add just the BP support, something like:
>>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>>> b/drivers/mtd/spi-nor/micron-st.c
>>> index a8da1f18e335..fdafbfa0f936 100644
>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>> @@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
>>>           }, {
>>>                   .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>>>                   .name = "mt25qu512a",
>>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>>> SPI_NOR_4BIT_BP |
>>> +                        SPI_NOR_BP3_SR_BIT6,
>>>                   .mfr_flags = USE_FSR,
>>>           }, {
>>>                   .id = SNOR_ID(0x20, 0xbb, 0x20),
>>>
>>> Of course, I expect you to run again the mtd_debug tests and also verify
>>> the locking. Thanks!
>>
>> I applied both changes as mentioned above i.e
>> 1] Switch to SFDP and 2] Using BP Flags.
>>
>> Case 1: Use BP Flags and Switch to SFDP
>> With both these changes, the lock/unlock doesn't work.
>>
>> ## x86-64-smarc-evk-uwd0j0007 # uname -r
>> 6.6.0-rc2
>>
>> # flash_lock -i /dev/mtd0
>> Device: /dev/mtd0
>> Start: 0
>> Len: 0x4000000
>> Lock status: unlocked
>> Return code: 0
>> # flash_lock -l /dev/mtd0
>> flash_lock: error!: could not lock device: /dev/mtd0
>>
>>               error 5 (Input/output error)
>>
>>
>> I suspected this is because of miscalculation of BP bits, like the
>> possibility mentioned here:
>> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=spi-nor/next&id=3ea3f0ac242c86c0275d347ab8c92bf1eb854b49 
>>
>>
>> But further checked size, it is correct:
>>
>>    # mtd_debug info /dev/mtd0
>> mtd.type = MTD_NORFLASH
>> mtd.flags = MTD_CAP_NORFLASH
>> mtd.size = 67108864 (64M)
>> mtd.erasesize = 4096 (4K)
>> mtd.writesize = 1
>> mtd.oobsize = 0
>> regions = 0
>>
>> And rest of read/write functions work as expected.
>>
>> Any suggestions about this?
>>
>> Case 2: Just added BP flags, rest of the size, mfr_flags, fixup flags
>> kept as it is.
> 
> would you please detail what exact definitions you used in case 2? Send
> us the diff please.
Case 2: Adding Flags for BP

diff --git a/drivers/mtd/spi-nor/micron-st.c 
b/drivers/mtd/spi-nor/micron-st.c
index 4afcfc57c896..6c8cabbead2e 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
         }, {
                 .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
                 .name = "mt25qu512a",
+               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | 
SPI_NOR_4BIT_BP |
+                       SPI_NOR_BP3_SR_BIT6,
                 .size = SZ_64M,
                 .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | 
SPI_NOR_QUAD_READ,
                 .fixup_flags = SPI_NOR_4B_OPCODES,


-----------------------------------------------------------------
Case 1: BP Flags and removed size, and no_sfdp so by default expecting 
to read SFDP

diff --git a/drivers/mtd/spi-nor/micron-st.c 
b/drivers/mtd/spi-nor/micron-st.c
index 6c8cabbead2e..4feb03ee2d13 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -407,9 +407,6 @@ static const struct flash_info st_nor_parts[] = {
                 .name = "mt25qu512a",
                 .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | 
SPI_NOR_4BIT_BP |
                         SPI_NOR_BP3_SR_BIT6,
-               .size = SZ_64M,
-               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | 
SPI_NOR_QUAD_READ,
-               .fixup_flags = SPI_NOR_4B_OPCODES,
                 .mfr_flags = USE_FSR,
         }, {



> 
> Cheers,
> ta
> 
>> Lock/unlock works.
>>
>> ## x86-64-smarc-evk-uwd0j0007 # uname -r
>> 6.6.0-rc2
>>
>> # flash_lock -i /dev/mtd0
>> Start: 0
>> Len: 0x4000000
>> Lock status: unlocked
>> Return code: 0
>>
>> # flash_lock -l /dev/mtd0
>> # flash_lock -i /dev/mtd0
>> Device: /dev/mtd0
>> Start: 0
>> Len: 0x4000000
>> Lock status: locked
>> Return code: 1
>>
>> ## x86-64-smarc-evk-uwd0j0007 # mtd_debug erase /dev/mtd0 0 1048576
>> [  413.472411] spi-nor spi-PRP0001:00: Erase operation failed.
>> [  413.478084] spi-nor spi-PRP0001:00: Attempted to modify a protected
>> sector.
>> MEMERASE: Input/output error
>>
>> # flash_lock -u /dev/mtd0
>> # flash_lock -i /dev/mtd0
>> Device: /dev/mtd0
>> Start: 0
>> Len: 0x4000000
>> Lock status: unlocked
>> Return code: 0
>>
>> ## x86-64-smarc-evk-uwd0j0007 #  mtd_debug erase /dev/mtd0 0 1048576
>> Erased 1048576 bytes from address 0x00000000 in flash
>>
>>
>> Further I tested on stable 6.5.5 Kernel with old way of Flash Info
>> Format and which has forced PARSE_SFDP Flag, no issues with lock/unlock.
>>
>>

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

On 10/5/23 15:21, SHUKLA Mamta Ramendra wrote:
> 
> 
> On 05.10.23 15:51, Tudor Ambarus wrote:
>> [Some people who received this message don't often get email from tudor.ambarus@linaro.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>>
>> This email is not from Hexagon’s Office 365 instance. Please be careful while clicking links, opening attachments, or replying to this email.
>>
>>
>> On 10/5/23 10:21, SHUKLA Mamta Ramendra wrote:
>>
>> cut
>>
>>>>
>>>> and the second one will add just the BP support, something like:
>>>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>>>> b/drivers/mtd/spi-nor/micron-st.c
>>>> index a8da1f18e335..fdafbfa0f936 100644
>>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>>> @@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
>>>>           }, {
>>>>                   .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>>>>                   .name = "mt25qu512a",
>>>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>>>> SPI_NOR_4BIT_BP |
>>>> +                        SPI_NOR_BP3_SR_BIT6,
>>>>                   .mfr_flags = USE_FSR,
>>>>           }, {
>>>>                   .id = SNOR_ID(0x20, 0xbb, 0x20),
>>>>
>>>> Of course, I expect you to run again the mtd_debug tests and also verify
>>>> the locking. Thanks!
>>>
>>> I applied both changes as mentioned above i.e
>>> 1] Switch to SFDP and 2] Using BP Flags.
>>>
>>> Case 1: Use BP Flags and Switch to SFDP
>>> With both these changes, the lock/unlock doesn't work.
>>>
>>> ## x86-64-smarc-evk-uwd0j0007 # uname -r
>>> 6.6.0-rc2
>>>
>>> # flash_lock -i /dev/mtd0
>>> Device: /dev/mtd0
>>> Start: 0
>>> Len: 0x4000000
>>> Lock status: unlocked
>>> Return code: 0
>>> # flash_lock -l /dev/mtd0
>>> flash_lock: error!: could not lock device: /dev/mtd0
>>>
>>>               error 5 (Input/output error)
>>>
>>>
>>> I suspected this is because of miscalculation of BP bits, like the
>>> possibility mentioned here:
>>> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=spi-nor/next&id=3ea3f0ac242c86c0275d347ab8c92bf1eb854b49 
>>>
>>>
>>> But further checked size, it is correct:
>>>
>>>    # mtd_debug info /dev/mtd0
>>> mtd.type = MTD_NORFLASH
>>> mtd.flags = MTD_CAP_NORFLASH
>>> mtd.size = 67108864 (64M)
>>> mtd.erasesize = 4096 (4K)
>>> mtd.writesize = 1
>>> mtd.oobsize = 0
>>> regions = 0
>>>
>>> And rest of read/write functions work as expected.
>>>
>>> Any suggestions about this?
>>>
>>> Case 2: Just added BP flags, rest of the size, mfr_flags, fixup flags
>>> kept as it is.
>>
>> would you please detail what exact definitions you used in case 2? Send
>> us the diff please.
> Case 2: Adding Flags for BP
> 
> diff --git a/drivers/mtd/spi-nor/micron-st.c 
> b/drivers/mtd/spi-nor/micron-st.c
> index 4afcfc57c896..6c8cabbead2e 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
>          }, {
>                  .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>                  .name = "mt25qu512a",
> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | 
> SPI_NOR_4BIT_BP |
> +                       SPI_NOR_BP3_SR_BIT6,
>                  .size = SZ_64M,
>                  .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | 
> SPI_NOR_QUAD_READ,
>                  .fixup_flags = SPI_NOR_4B_OPCODES,
> 
> 

Thanks. Nothing obvious on a first look. I looked at the sfdp dump, it
looks like 4BAIT table is missing, so you'll probably need:
	.fixup_flags = SPI_NOR_4B_OPCODES,

I don't see how this could affect BP, but it is worth to test
incremental changes and find out what misses.

After you test the above, I'd like you to extend the patch with:
	.size = SZ_64M,
Check if it works and send us the output of mtd_debug info /dev/mtd0 here.

Also you could enable dev_dbg to see where you get -EIO. Probably when
reading the SR back. Also you can use debugfs to check what is set in
the working scenario and what params are different in the non-working
case. See drivers/mtd/spi-nor/debugfs.c

Cheers,
ta


> -----------------------------------------------------------------
> Case 1: BP Flags and removed size, and no_sfdp so by default expecting 
> to read SFDP
> 
> diff --git a/drivers/mtd/spi-nor/micron-st.c 
> b/drivers/mtd/spi-nor/micron-st.c
> index 6c8cabbead2e..4feb03ee2d13 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -407,9 +407,6 @@ static const struct flash_info st_nor_parts[] = {
>                  .name = "mt25qu512a",
>                  .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | 
> SPI_NOR_4BIT_BP |
>                          SPI_NOR_BP3_SR_BIT6,
> -               .size = SZ_64M,
> -               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | 
> SPI_NOR_QUAD_READ,
> -               .fixup_flags = SPI_NOR_4B_OPCODES,
>                  .mfr_flags = USE_FSR,
>          }, {
> 
> 
> 
>>
>> Cheers,
>> ta
>>
>>> Lock/unlock works.
>>>
>>> ## x86-64-smarc-evk-uwd0j0007 # uname -r
>>> 6.6.0-rc2
>>>
>>> # flash_lock -i /dev/mtd0
>>> Start: 0
>>> Len: 0x4000000
>>> Lock status: unlocked
>>> Return code: 0
>>>
>>> # flash_lock -l /dev/mtd0
>>> # flash_lock -i /dev/mtd0
>>> Device: /dev/mtd0
>>> Start: 0
>>> Len: 0x4000000
>>> Lock status: locked
>>> Return code: 1
>>>
>>> ## x86-64-smarc-evk-uwd0j0007 # mtd_debug erase /dev/mtd0 0 1048576
>>> [  413.472411] spi-nor spi-PRP0001:00: Erase operation failed.
>>> [  413.478084] spi-nor spi-PRP0001:00: Attempted to modify a protected
>>> sector.
>>> MEMERASE: Input/output error
>>>
>>> # flash_lock -u /dev/mtd0
>>> # flash_lock -i /dev/mtd0
>>> Device: /dev/mtd0
>>> Start: 0
>>> Len: 0x4000000
>>> Lock status: unlocked
>>> Return code: 0
>>>
>>> ## x86-64-smarc-evk-uwd0j0007 #  mtd_debug erase /dev/mtd0 0 1048576
>>> Erased 1048576 bytes from address 0x00000000 in flash
>>>
>>>
>>> Further I tested on stable 6.5.5 Kernel with old way of Flash Info
>>> Format and which has forced PARSE_SFDP Flag, no issues with lock/unlock.
>>>
>>>
> 
> ---
> Mamta

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

>>> On 10/5/23 10:21, SHUKLA Mamta Ramendra wrote:
>>>
>>> cut
>>>
>>>>>
>>>>> and the second one will add just the BP support, something like:
>>>>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>>>>> b/drivers/mtd/spi-nor/micron-st.c
>>>>> index a8da1f18e335..fdafbfa0f936 100644
>>>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>>>> @@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
>>>>>            }, {
>>>>>                    .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>>>>>                    .name = "mt25qu512a",
>>>>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>>>>> SPI_NOR_4BIT_BP |
>>>>> +                        SPI_NOR_BP3_SR_BIT6,
>>>>>                    .mfr_flags = USE_FSR,
>>>>>            }, {
>>>>>                    .id = SNOR_ID(0x20, 0xbb, 0x20),
>>>>>
>>>>> Of course, I expect you to run again the mtd_debug tests and also verify
>>>>> the locking. Thanks!
>>>>
>>>> I applied both changes as mentioned above i.e
>>>> 1] Switch to SFDP and 2] Using BP Flags.
>>>>
>>>> Case 1: Use BP Flags and Switch to SFDP
>>>> With both these changes, the lock/unlock doesn't work.
>>>>
>>>> ## x86-64-smarc-evk-uwd0j0007 # uname -r
>>>> 6.6.0-rc2
>>>>
>>>> # flash_lock -i /dev/mtd0
>>>> Device: /dev/mtd0
>>>> Start: 0
>>>> Len: 0x4000000
>>>> Lock status: unlocked
>>>> Return code: 0
>>>> # flash_lock -l /dev/mtd0
>>>> flash_lock: error!: could not lock device: /dev/mtd0
>>>>
>>>>                error 5 (Input/output error)
>>>>
>>>>
>>>> I suspected this is because of miscalculation of BP bits, like the
>>>> possibility mentioned here:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/mtd/linux.git/commit/?h=spi-nor/next&id=3ea3f0ac242c86c0275d347ab8c92bf1eb854b49 
>>>>
>>>>
>>>> But further checked size, it is correct:
>>>>
>>>>     # mtd_debug info /dev/mtd0
>>>> mtd.type = MTD_NORFLASH
>>>> mtd.flags = MTD_CAP_NORFLASH
>>>> mtd.size = 67108864 (64M)
>>>> mtd.erasesize = 4096 (4K)
>>>> mtd.writesize = 1
>>>> mtd.oobsize = 0
>>>> regions = 0
>>>>
>>>> And rest of read/write functions work as expected.
>>>>
>>>> Any suggestions about this?
>>>>
>>>> Case 2: Just added BP flags, rest of the size, mfr_flags, fixup flags
>>>> kept as it is.
>>>
>>> would you please detail what exact definitions you used in case 2? Send
>>> us the diff please.
>> Case 2: Adding Flags for BP
>>
>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>> b/drivers/mtd/spi-nor/micron-st.c
>> index 4afcfc57c896..6c8cabbead2e 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -405,6 +405,8 @@ static const struct flash_info st_nor_parts[] = {
>>           }, {
>>                   .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>>                   .name = "mt25qu512a",
>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>> SPI_NOR_4BIT_BP |
>> +                       SPI_NOR_BP3_SR_BIT6,
>>                   .size = SZ_64M,
>>                   .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
>> SPI_NOR_QUAD_READ,
>>                   .fixup_flags = SPI_NOR_4B_OPCODES,
>>
>>
> 
> Thanks. Nothing obvious on a first look. I looked at the sfdp dump, it
> looks like 4BAIT table is missing, so you'll probably need:
>          .fixup_flags = SPI_NOR_4B_OPCODES,

> I don't see how this could affect BP, but it is worth to test
> incremental changes and find out what misses.
> 
> After you test the above, I'd like you to extend the patch with:
>          .size = SZ_64M,
> Check if it works and send us the output of mtd_debug info /dev/mtd0 here.

I started first with params and debug log for working and non-working cases:

1] Non-working case: BP Flags + Parsing SFDP(i.e remove size, fixup flag 
and no_sfdp flag)



## x86-64-smarc-evk-uwd0j0007 # flash_lock -l /dev/mtd0

[  480.230232] spi-nor spi-PRP0001:00: SR: Read back test failed
flash_lock: error!: could not lock device: /dev/mtd0

             error 5 (Input/output error)

and params from debugfs look like:

## x86-64-smarc-evk-uwd0j0007 # cat 
/sys/kernel/debug/spi-nor/spi-PRP0001:00/params
name            mt25qu512a
id              20 bb 20 10 44 00
size            64.0 MiB
write size      1
page size       256
address nbytes  4
flags           HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | 
HAS_16BIT_SR | HAS_4BIT_BP | HAS_SR_BP3_BIT6 | SOFT_RESET


Strange thing here is HAS_16BIT_SR. And it makes sense from debug log:
[  480.230232] spi-nor spi-PRP0001:00: SR: Read back test failed

which comes from spi_nor_write_16bit_sr_and_check () .

It is strange because in micron-st.c, in default init we have:
static void micron_st_nor_default_init(struct spi_nor *nor)
{
	nor->flags |= SNOR_F_HAS_LOCK;
	nor->flags &= ~SNOR_F_HAS_16BIT_SR;
	nor->params->quad_enable = NULL;
}



2] Working case: Just BP flags and no parsing SFDP:

Params for flags look like

flags           HAS_SR_TB | 4B_OPCODES | HAS_4BAIT | HAS_LOCK | 
HAS_4BIT_BP | HAS_SR_BP3_BIT6 | SOFT_RESET


3] Just adding .fixup_flags:
@@ -407,6 +418,7 @@ static const struct flash_info st_nor_parts[] = {
                 .name = "mt25qu512a",
                 .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB | 
SPI_NOR_4BIT_BP |
                         SPI_NOR_BP3_SR_BIT6,
+               .fixup_flags = SPI_NOR_4B_OPCODES,
                 .mfr_flags = USE_FSR,



lock/unlock still doesn't work.

I also compared this with case when we just parse SFDP flag, we get 
4Byte-AIT, so this flag is not required when  parsing SFDP.

This is only required when we don't parse SFDP and use .size=SZ_64M


4] Adding .size flag i.e no SFDP parsing:

lock/unlock works as expected and params:

## x86-64-smarc-evk-uwd0j0007 # cat 
/sys/kernel/debug/spi-nor/spi-PRP0001:00/params
name            mt25qu512a
id              20 bb 20 10 44 00
size            64.0 MiB
write size      1
page size       256
address nbytes  4
flags           HAS_SR_TB | 4B_OPCODES | HAS_LOCK | HAS_4BIT_BP | 
HAS_SR_BP3_BIT6

--------------------------------------------------------

IMO, HAS_16BIT_SR flag is causing lock/unlock failure,
since BP bits are calculated wrong then.

I tested also for a case where I don't parse SFDP and
reverted the condition in  micron_st_nor_default_init()
for 16BIT Status Register Flag. And lock/unlock fails with
same log as Non-working case.

And this mt25qu512 has 8-BIT SR as typical micron-st flash.



> Also you could enable dev_dbg to see where you get -EIO. Probably when
> reading the SR back. Also you can use debugfs to check what is set in
> the working scenario and what params are different in the non-working
> case. See drivers/mtd/spi-nor/debugfs.c
> 
> Cheers,
> ta
> 
> 
>> -----------------------------------------------------------------
>> Case 1: BP Flags and removed size, and no_sfdp so by default expecting
>> to read SFDP
>>
>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>> b/drivers/mtd/spi-nor/micron-st.c
>> index 6c8cabbead2e..4feb03ee2d13 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -407,9 +407,6 @@ static const struct flash_info st_nor_parts[] = {
>>                   .name = "mt25qu512a",
>>                   .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>> SPI_NOR_4BIT_BP |
>>                           SPI_NOR_BP3_SR_BIT6,
>> -               .size = SZ_64M,
>> -               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
>> SPI_NOR_QUAD_READ,
>> -               .fixup_flags = SPI_NOR_4B_OPCODES,
>>                   .mfr_flags = USE_FSR,
>>           }, {
>>
>>
>>
>>>
>>> Cheers,
>>> ta
>>>
>>>> Lock/unlock works.
>>>>
>>>> ## x86-64-smarc-evk-uwd0j0007 # uname -r
>>>> 6.6.0-rc2
>>>>
>>>> # flash_lock -i /dev/mtd0
>>>> Start: 0
>>>> Len: 0x4000000
>>>> Lock status: unlocked
>>>> Return code: 0
>>>>
>>>> # flash_lock -l /dev/mtd0
>>>> # flash_lock -i /dev/mtd0
>>>> Device: /dev/mtd0
>>>> Start: 0
>>>> Len: 0x4000000
>>>> Lock status: locked
>>>> Return code: 1
>>>>
>>>> ## x86-64-smarc-evk-uwd0j0007 # mtd_debug erase /dev/mtd0 0 1048576
>>>> [  413.472411] spi-nor spi-PRP0001:00: Erase operation failed.
>>>> [  413.478084] spi-nor spi-PRP0001:00: Attempted to modify a protected
>>>> sector.
>>>> MEMERASE: Input/output error
>>>>
>>>> # flash_lock -u /dev/mtd0
>>>> # flash_lock -i /dev/mtd0
>>>> Device: /dev/mtd0
>>>> Start: 0
>>>> Len: 0x4000000
>>>> Lock status: unlocked
>>>> Return code: 0
>>>>
>>>> ## x86-64-smarc-evk-uwd0j0007 #  mtd_debug erase /dev/mtd0 0 1048576
>>>> Erased 1048576 bytes from address 0x00000000 in flash
>>>>
>>>>
>>>> Further I tested on stable 6.5.5 Kernel with old way of Flash Info
>>>> Format and which has forced PARSE_SFDP Flag, no issues with lock/unlock.
>>>>
>>>>
>>

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

Hi,

Thanks for the debugging info.

On 10/6/23 11:30, SHUKLA Mamta Ramendra wrote:

cut

> --------------------------------------------------------
> 
> IMO, HAS_16BIT_SR flag is causing lock/unlock failure,
> since BP bits are calculated wrong then.
> 
> I tested also for a case where I don't parse SFDP and
> reverted the condition in  micron_st_nor_default_init()
> for 16BIT Status Register Flag. And lock/unlock fails with
> same log as Non-working case.
> 
> And this mt25qu512 has 8-BIT SR as typical micron-st flash.
> 

Indeed, the problem is that HAS_16BIT_SR gets set when it shouldn't have
to. This means that the BFPT table of the flash is wrong and we should
fix the parsed settings via a post_bfpt hook.

Does the following fix your problem?

diff --git a/drivers/mtd/spi-nor/micron-st.c
b/drivers/mtd/spi-nor/micron-st.c
index 4afcfc57c896..733bbddc6829 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -180,6 +180,17 @@ static const struct flash_info micron_nor_parts[] = {
        },
 };

+static int mt25qu512a_post_bfpt_fixup(struct spi_nor *nor,
+                               const struct sfdp_parameter_header
*bfpt_header,
+                               const struct sfdp_bfpt *bfpt)
+{
+       nor->flags &= ~SNOR_F_HAS_16BIT_SR;
+}
+
+static struct spi_nor_fixups mt25qu512a_fixups = {
+       .post_bfpt = mt25qu512a_post_bfpt_fixup,
+}
+
 static const struct flash_info st_nor_parts[] = {
        {
                .name = "m25p05-nonjedec",
@@ -405,10 +416,10 @@ static const struct flash_info st_nor_parts[] = {
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
                .name = "mt25qu512a",
-               .size = SZ_64M,
-               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
SPI_NOR_QUAD_READ,
-               .fixup_flags = SPI_NOR_4B_OPCODES,
+               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
SPI_NOR_4BIT_BP |
+                        SPI_NOR_BP3_SR_BIT6,
                .mfr_flags = USE_FSR,
+               .fixups = &mt25qu512a_fixups,
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x20),
                .name = "n25q512a",


If yes, please add some prints in sfdp.c to determine where it's set,
either in BFPT_DWORD15_QER_SR2_BIT1 or BFPT_DWORD15_QER_SR2_BIT1_NO_RD

Is the datasheet for this flash public? Would you send me a link to it
please?

Cheers,
ta

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

On 10/6/23 14:33, Tudor Ambarus wrote:
> 
> Hi,
> 
> Thanks for the debugging info.
> 
> On 10/6/23 11:30, SHUKLA Mamta Ramendra wrote:
> 
> cut
> 
>> --------------------------------------------------------
>>
>> IMO, HAS_16BIT_SR flag is causing lock/unlock failure,
>> since BP bits are calculated wrong then.
>>
>> I tested also for a case where I don't parse SFDP and
>> reverted the condition in  micron_st_nor_default_init()
>> for 16BIT Status Register Flag. And lock/unlock fails with
>> same log as Non-working case.
>>
>> And this mt25qu512 has 8-BIT SR as typical micron-st flash.
>>
> 
> Indeed, the problem is that HAS_16BIT_SR gets set when it shouldn't have
> to. This means that the BFPT table of the flash is wrong and we should
> fix the parsed settings via a post_bfpt hook.
> 
> Does the following fix your problem?
> 

here it is again, this time compile tested:

diff --git a/drivers/mtd/spi-nor/micron-st.c
b/drivers/mtd/spi-nor/micron-st.c
index 4afcfc57c896..20f76e278095 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -180,6 +180,18 @@ static const struct flash_info micron_nor_parts[] = {
        },
 };

+static int mt25qu512a_post_bfpt_fixup(struct spi_nor *nor,
+                               const struct sfdp_parameter_header
*bfpt_header,
+                               const struct sfdp_bfpt *bfpt)
+{
+       nor->flags &= ~SNOR_F_HAS_16BIT_SR;
+       return 0;
+}
+
+static struct spi_nor_fixups mt25qu512a_fixups = {
+       .post_bfpt = mt25qu512a_post_bfpt_fixup,
+};
+
 static const struct flash_info st_nor_parts[] = {
        {
                .name = "m25p05-nonjedec",
@@ -405,10 +417,10 @@ static const struct flash_info st_nor_parts[] = {
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
                .name = "mt25qu512a",
-               .size = SZ_64M,
-               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
SPI_NOR_QUAD_READ,
-               .fixup_flags = SPI_NOR_4B_OPCODES,
+               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
SPI_NOR_4BIT_BP |
+                        SPI_NOR_BP3_SR_BIT6,
                .mfr_flags = USE_FSR,
+               .fixups = &mt25qu512a_fixups,
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x20),
                .name = "n25q512a",

cut

> If yes, please add some prints in sfdp.c to determine where it's set,
> either in BFPT_DWORD15_QER_SR2_BIT1 or BFPT_DWORD15_QER_SR2_BIT1_NO_RD
> 
> Is the datasheet for this flash public? Would you send me a link to it
> please?
> 
> Cheers,
> ta

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

Hello Tudor,


> On 10/6/23 14:33, Tudor Ambarus wrote:
>>
>> Hi,
>>
>> Thanks for the debugging info.
>>
>> On 10/6/23 11:30, SHUKLA Mamta Ramendra wrote:
>>
>> cut
>>
>>> --------------------------------------------------------
>>>
>>> IMO, HAS_16BIT_SR flag is causing lock/unlock failure,
>>> since BP bits are calculated wrong then.
>>>
>>> I tested also for a case where I don't parse SFDP and
>>> reverted the condition in  micron_st_nor_default_init()
>>> for 16BIT Status Register Flag. And lock/unlock fails with
>>> same log as Non-working case.
>>>
>>> And this mt25qu512 has 8-BIT SR as typical micron-st flash.
>>>
>>
>> Indeed, the problem is that HAS_16BIT_SR gets set when it shouldn't have
>> to. This means that the BFPT table of the flash is wrong and we should
>> fix the parsed settings via a post_bfpt hook.
>>
>> Does the following fix your problem?
>>
> 
> here it is again, this time compile tested:
> 
> diff --git a/drivers/mtd/spi-nor/micron-st.c
> b/drivers/mtd/spi-nor/micron-st.c
> index 4afcfc57c896..20f76e278095 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -180,6 +180,18 @@ static const struct flash_info micron_nor_parts[] = {
>          },
>   };
> 
> +static int mt25qu512a_post_bfpt_fixup(struct spi_nor *nor,
> +                               const struct sfdp_parameter_header
> *bfpt_header,
> +                               const struct sfdp_bfpt *bfpt)
> +{
> +       nor->flags &= ~SNOR_F_HAS_16BIT_SR;
> +       return 0;
> +}
> +
> +static struct spi_nor_fixups mt25qu512a_fixups = {
> +       .post_bfpt = mt25qu512a_post_bfpt_fixup,
> +};
> +
>   static const struct flash_info st_nor_parts[] = {
>          {
>                  .name = "m25p05-nonjedec",
> @@ -405,10 +417,10 @@ static const struct flash_info st_nor_parts[] = {
>          }, {
>                  .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>                  .name = "mt25qu512a",
> -               .size = SZ_64M,
> -               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
> SPI_NOR_QUAD_READ,
> -               .fixup_flags = SPI_NOR_4B_OPCODES,
> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
> SPI_NOR_4BIT_BP |
> +                        SPI_NOR_BP3_SR_BIT6,
>                  .mfr_flags = USE_FSR,
> +               .fixups = &mt25qu512a_fixups,
>          }, {
>                  .id = SNOR_ID(0x20, 0xbb, 0x20),
>                  .name = "n25q512a",
> 
> cut


Thanks. Yes, this post  BFPT fixup resolves lock/unlock issue and
I don't see HAS_16BIT_SR flag in params.


Datasheet is public: 
https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211 


>> If yes, please add some prints in sfdp.c to determine where it's set,
>> either in BFPT_DWORD15_QER_SR2_BIT1 or BFPT_DWORD15_QER_SR2_BIT1_NO_RD
>> Is the datasheet for this flash public? Would you send me a link to it
>> please?
>>
>> Cheers,
>> ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

On 10/6/23 17:09, SHUKLA Mamta Ramendra wrote:
> 
> 
> Hello Tudor,
> 
Hi!

> 
>> On 10/6/23 14:33, Tudor Ambarus wrote:
>>>
>>> Hi,
>>>
>>> Thanks for the debugging info.
>>>
>>> On 10/6/23 11:30, SHUKLA Mamta Ramendra wrote:
>>>
>>> cut
>>>
>>>> --------------------------------------------------------
>>>>
>>>> IMO, HAS_16BIT_SR flag is causing lock/unlock failure,
>>>> since BP bits are calculated wrong then.
>>>>
>>>> I tested also for a case where I don't parse SFDP and
>>>> reverted the condition in  micron_st_nor_default_init()
>>>> for 16BIT Status Register Flag. And lock/unlock fails with
>>>> same log as Non-working case.
>>>>
>>>> And this mt25qu512 has 8-BIT SR as typical micron-st flash.
>>>>
>>>
>>> Indeed, the problem is that HAS_16BIT_SR gets set when it shouldn't have
>>> to. This means that the BFPT table of the flash is wrong and we should
>>> fix the parsed settings via a post_bfpt hook.
>>>
>>> Does the following fix your problem?
>>>
>>
>> here it is again, this time compile tested:
>>
>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>> b/drivers/mtd/spi-nor/micron-st.c
>> index 4afcfc57c896..20f76e278095 100644
>> --- a/drivers/mtd/spi-nor/micron-st.c
>> +++ b/drivers/mtd/spi-nor/micron-st.c
>> @@ -180,6 +180,18 @@ static const struct flash_info micron_nor_parts[] = {
>>          },
>>   };
>>
>> +static int mt25qu512a_post_bfpt_fixup(struct spi_nor *nor,
>> +                               const struct sfdp_parameter_header
>> *bfpt_header,
>> +                               const struct sfdp_bfpt *bfpt)
>> +{
>> +       nor->flags &= ~SNOR_F_HAS_16BIT_SR;
>> +       return 0;
>> +}
>> +
>> +static struct spi_nor_fixups mt25qu512a_fixups = {
>> +       .post_bfpt = mt25qu512a_post_bfpt_fixup,
>> +};
>> +
>>   static const struct flash_info st_nor_parts[] = {
>>          {
>>                  .name = "m25p05-nonjedec",
>> @@ -405,10 +417,10 @@ static const struct flash_info st_nor_parts[] = {
>>          }, {
>>                  .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>>                  .name = "mt25qu512a",
>> -               .size = SZ_64M,
>> -               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
>> SPI_NOR_QUAD_READ,
>> -               .fixup_flags = SPI_NOR_4B_OPCODES,
>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>> SPI_NOR_4BIT_BP |
>> +                        SPI_NOR_BP3_SR_BIT6,
>>                  .mfr_flags = USE_FSR,
>> +               .fixups = &mt25qu512a_fixups,
>>          }, {
>>                  .id = SNOR_ID(0x20, 0xbb, 0x20),
>>                  .name = "n25q512a",
>>
>> cut
> 
> 
> Thanks. Yes, this post  BFPT fixup resolves lock/unlock issue and
> I don't see HAS_16BIT_SR flag in params.

Good. Please send some patches addressing all that we discussed until now.
> 
> 
> Datasheet is public: 
> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211 
> 
I'll take a look later today.

Cheers,
ta
> 
>>> If yes, please add some prints in sfdp.c to determine where it's set,
>>> either in BFPT_DWORD15_QER_SR2_BIT1 or BFPT_DWORD15_QER_SR2_BIT1_NO_RD
>>> Is the datasheet for this flash public? Would you send me a link to it
>>> please?
>>>
>>> Cheers,
>>> ta

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

Hello,

> On 10/6/23 17:09, SHUKLA Mamta Ramendra wrote:
>>
>>
>> Hello Tudor,
>>
> Hi!
> 
>>
>>> On 10/6/23 14:33, Tudor Ambarus wrote:
>>>>
>>>> Hi,
>>>>
>>>> Thanks for the debugging info.
>>>>
>>>> On 10/6/23 11:30, SHUKLA Mamta Ramendra wrote:
>>>>
>>>> cut
>>>>
>>>>> --------------------------------------------------------
>>>>>
>>>>> IMO, HAS_16BIT_SR flag is causing lock/unlock failure,
>>>>> since BP bits are calculated wrong then.
>>>>>
>>>>> I tested also for a case where I don't parse SFDP and
>>>>> reverted the condition in  micron_st_nor_default_init()
>>>>> for 16BIT Status Register Flag. And lock/unlock fails with
>>>>> same log as Non-working case.
>>>>>
>>>>> And this mt25qu512 has 8-BIT SR as typical micron-st flash.
>>>>>
>>>>
>>>> Indeed, the problem is that HAS_16BIT_SR gets set when it shouldn't have
>>>> to. This means that the BFPT table of the flash is wrong and we should
>>>> fix the parsed settings via a post_bfpt hook.
>>>>
>>>> Does the following fix your problem?
>>>>
>>>
>>> here it is again, this time compile tested:
>>>
>>> diff --git a/drivers/mtd/spi-nor/micron-st.c
>>> b/drivers/mtd/spi-nor/micron-st.c
>>> index 4afcfc57c896..20f76e278095 100644
>>> --- a/drivers/mtd/spi-nor/micron-st.c
>>> +++ b/drivers/mtd/spi-nor/micron-st.c
>>> @@ -180,6 +180,18 @@ static const struct flash_info micron_nor_parts[] = {
>>>           },
>>>    };
>>>
>>> +static int mt25qu512a_post_bfpt_fixup(struct spi_nor *nor,
>>> +                               const struct sfdp_parameter_header
>>> *bfpt_header,
>>> +                               const struct sfdp_bfpt *bfpt)
>>> +{
>>> +       nor->flags &= ~SNOR_F_HAS_16BIT_SR;
>>> +       return 0;
>>> +}
>>> +
>>> +static struct spi_nor_fixups mt25qu512a_fixups = {
>>> +       .post_bfpt = mt25qu512a_post_bfpt_fixup,
>>> +};
>>> +
>>>    static const struct flash_info st_nor_parts[] = {
>>>           {
>>>                   .name = "m25p05-nonjedec",
>>> @@ -405,10 +417,10 @@ static const struct flash_info st_nor_parts[] = {
>>>           }, {
>>>                   .id = SNOR_ID(0x20, 0xbb, 0x20, 0x10, 0x44, 0x00),
>>>                   .name = "mt25qu512a",
>>> -               .size = SZ_64M,
>>> -               .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ |
>>> SPI_NOR_QUAD_READ,
>>> -               .fixup_flags = SPI_NOR_4B_OPCODES,
>>> +               .flags = SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB |
>>> SPI_NOR_4BIT_BP |
>>> +                        SPI_NOR_BP3_SR_BIT6,
>>>                   .mfr_flags = USE_FSR,
>>> +               .fixups = &mt25qu512a_fixups,
>>>           }, {
>>>                   .id = SNOR_ID(0x20, 0xbb, 0x20),
>>>                   .name = "n25q512a",
>>>
>>> cut
>>
>>
>> Thanks. Yes, this post  BFPT fixup resolves lock/unlock issue and
>> I don't see HAS_16BIT_SR flag in params.
> 
> Good. Please send some patches addressing all that we discussed until now.

v3: 
https://patchwork.ozlabs.org/project/linux-mtd/patch/6b89ae4e4d7a381050746458cb000cd3c60f7a42.1696849423.git.mamta.shukla@leica-geosystems.com/ 


>>
>> Datasheet is public:
>> https://media-www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_512_abb_0.pdf?rev=b259aadc3bea49ea8210a41c9ad58211 
>>
> I'll take a look later today.
> 
> Cheers,
> ta
>>
>>>> If yes, please add some prints in sfdp.c to determine where it's set,
>>>> either in BFPT_DWORD15_QER_SR2_BIT1 or BFPT_DWORD15_QER_SR2_BIT1_NO_RD
>>>> Is the datasheet for this flash public? Would you send me a link to it
>>>> please?
>>>>
>>>> Cheers,
>>>> ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[Tudor Ambarus]

Hi, Shukla,


On 16.10.2023 12:39, SHUKLA Mamta Ramendra wrote:
> v3:
> https://patchwork.ozlabs.org/project/linux-mtd/patch/6b89ae4e4d7a381050746458cb000cd3c60f7a42.1696849423.git.mamta.shukla@leica-geosystems.com/

I've just sent v4 on your behalf to speed up the process. Please check
what I did and if it looks good to you let me know.

https://lore.kernel.org/linux-mtd/20231017074711.12167-1-tudor.ambarus@linaro.org/T/#t

Cheers,
ta

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

Re: [PATCH v2] mtd: micron-st: enable lock/unlock for mt25qu512a

[SHUKLA Mamta Ramendra]

Hello Tudor,

On 17.10.23 09:53, Tudor Ambarus wrote:
> 
> Hi, Shukla,
> 
> 
> On 16.10.2023 12:39, SHUKLA Mamta Ramendra wrote:
>> v3:
>> https://patchwork.ozlabs.org/project/linux-mtd/patch/6b89ae4e4d7a381050746458cb000cd3c60f7a42.1696849423.git.mamta.shukla@leica-geosystems.com/
> 
> I've just sent v4 on your behalf to speed up the process. Please check
> what I did and if it looks good to you let me know.
> 
> https://lore.kernel.org/linux-mtd/20231017074711.12167-1-tudor.ambarus@linaro.org/T/#t

This looks good to me!

Thanks,
Mamta


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