[PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

From: Fabio Estevam <festevam@denx.de>

Add support for the MT25QU01G 128MB Micron Serial NOR Flash Memory
model.
    
Datasheet:
https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_01g_bbb_0.pdf

Tested on a i.MX8MP based board:

 # dmesg | grep spi-nor
[    1.854474] spi-nor spi0.0: mt25qu01g (131072 Kbytes)

 # cat /proc/mtd 
dev:    size   erasesize  name
mtd0: 08000000 00001000 "30bb0000.spi"

Signed-off-by: Fabio Estevam <festevam@denx.de>
---
 drivers/mtd/spi-nor/micron-st.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 8920547c12bf..da2f8992bbc0 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -429,6 +429,13 @@ static const struct flash_info st_nor_parts[] = {
 			 SPI_NOR_BP3_SR_BIT6,
 		.no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
 		.mfr_flags = USE_FSR,
+	}, {
+		.id = SNOR_ID(0x20, 0xbb, 0x21, 0x10, 0x44, 0x00),
+		.name = "mt25qu01g",
+		.size = SZ_128M,
+		.flags = NO_CHIP_ERASE,
+		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
+		.mfr_flags = USE_FSR,
 	}, {
 		.id = SNOR_ID(0x20, 0xbb, 0x21),
 		.name = "n25q00a",
-- 
2.34.1


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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Tudor Ambarus]

On 24.10.2023 00:38, Fabio Estevam wrote:
> From: Fabio Estevam <festevam@denx.de>
> 
> Add support for the MT25QU01G 128MB Micron Serial NOR Flash Memory
> model.
>     
> Datasheet:
> https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_01g_bbb_0.pdf
> 

Hi, Fabio,

I see this is a dual die device. Do both dies work correctly with the
current software?

Cheers,
ta

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

Hi Tudor,

On Tue, Oct 24, 2023 at 3:01 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:

> I see this is a dual die device. Do both dies work correctly with the
> current software?

I am running linux-next  20231023. Is there a way to exercise both
dies to confirm?

Thanks

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Tudor Ambarus]

On 10/24/23 14:57, Fabio Estevam wrote:
> Hi Tudor,
> 
Hi!

> On Tue, Oct 24, 2023 at 3:01 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:
> 
>> I see this is a dual die device. Do both dies work correctly with the
>> current software?
> 
> I am running linux-next  20231023. Is there a way to exercise both
> dies to confirm?
> 

yes, just do some write-erase-verify-erase then a
write-read-back-compare tests at offsets from both dies. You may do a
cross die test if you want too. You can use mtd_utils for that, see an
example at:
https://lore.kernel.org/linux-mtd/20230926131655.51224-1-nicolas.ferre@microchip.com/

cheers,
ta

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Michael Walle]

[+ Takahiro]

> Add support for the MT25QU01G 128MB Micron Serial NOR Flash Memory
> model.
> 
> Datasheet:
> https://www.micron.com/-/media/client/global/documents/products/data-sheet/nor-flash/serial-nor/mt25q/die-rev-b/mt25q_qlkt_u_01g_bbb_0.pdf

As a "Link:" tag please.

Please also include a SFDP dump, see [1].

> Tested on a i.MX8MP based board:
> 
>  # dmesg | grep spi-nor
> [    1.854474] spi-nor spi0.0: mt25qu01g (131072 Kbytes)
> 
>  # cat /proc/mtd
> dev:    size   erasesize  name
> mtd0: 08000000 00001000 "30bb0000.spi"
> 
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> ---
>  drivers/mtd/spi-nor/micron-st.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/micron-st.c 
> b/drivers/mtd/spi-nor/micron-st.c
> index 8920547c12bf..da2f8992bbc0 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -429,6 +429,13 @@ static const struct flash_info st_nor_parts[] = {
>  			 SPI_NOR_BP3_SR_BIT6,
>  		.no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
>  		.mfr_flags = USE_FSR,
> +	}, {
> +		.id = SNOR_ID(0x20, 0xbb, 0x21, 0x10, 0x44, 0x00),
> +		.name = "mt25qu01g",
> +		.size = SZ_128M,

Not needed, parsed by SFDP.

> +		.flags = NO_CHIP_ERASE,

Is it fair to assume that this is the usual case for multi die chips?
See also,
https://lore.kernel.org/linux-mtd/cover.1680849425.git.Takahiro.Kuwano@infineon.com/

So "if n_dice > 1" will implicitly be NO_CHIP_ERASE. n_dice should be
parsed from SFDP.

> +		.no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,

.no_sfdp_flags are legacy and shouldn't be used with new flashes.

> +		.mfr_flags = USE_FSR,
>  	}, {
>  		.id = SNOR_ID(0x20, 0xbb, 0x21),
>  		.name = "n25q00a",

-michael

[1] 
https://lore.kernel.org/all/4304e19f3399a0a6e856119d01ccabe0@walle.cc/

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

Hi Michael and Tudor,

On Tue, Oct 24, 2023 at 6:26 AM Michael Walle <michael@walle.cc> wrote:

> > +             .id = SNOR_ID(0x20, 0xbb, 0x21, 0x10, 0x44, 0x00),
> > +             .name = "mt25qu01g",
> > +             .size = SZ_128M,
>
> Not needed, parsed by SFDP.
>
> > +             .flags = NO_CHIP_ERASE,
>
> Is it fair to assume that this is the usual case for multi die chips?
> See also,
> https://lore.kernel.org/linux-mtd/cover.1680849425.git.Takahiro.Kuwano@infineon.com/
>
> So "if n_dice > 1" will implicitly be NO_CHIP_ERASE. n_dice should be
> parsed from SFDP.
>
> > +             .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
>
> .no_sfdp_flags are legacy and shouldn't be used with new flashes.

I tried your suggestion and tested with the change below:

--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -429,6 +429,10 @@ static const struct flash_info st_nor_parts[] = {
                         SPI_NOR_BP3_SR_BIT6,
                .no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
                .mfr_flags = USE_FSR,
+       }, {
+               .id = SNOR_ID(0x20, 0xbb, 0x21, 0x10, 0x44, 0x00),
+               .name = "mt25qu01g",
+               .mfr_flags = USE_FSR,
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x21),
                .name = "n25q00a",

~# hexdump -C -n16 /dev/mtd0
00000000  01 02 03 04 05 ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000010
~# flash_erase /dev/mtd0 0 0
Erasing 131072 Kibyte @ 0 -- 100 % complete

The flash_erase operation completes immediately, but after checking the content
of the flash, I noticed that the original content is still there:

~# hexdump -C -n16 /dev/mtd0
00000000  01 02 03 04 05 ff ff ff  ff ff ff ff ff ff ff ff  |................|
00000010

With the original patch, the erase operation took about 2 minutes to
complete and the erase succeeded.

Below are the requested dumps, thanks.

Please let me know if you have any suggestions.

~# cat /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/jedec_id
20bb21104400
~# cat /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/manufacturer
st
~# cat /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/partname
mt25qu01g
~# xxd -p  /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp
53464450060101ff00060110300000ff84000102800000ffffffffffffff
ffffffffffffffffffffffffffffffffffffe520fbffffffff3f29eb276b
273b27bbffffffffffff27bbffff29eb0c2010d80f520000244a99008b8e
03e1ac0127387a757a75fbbdd55c4a0f82ff81bd3d36ffffffffffffffff
ffffffffffffffffffe7ffff21dcffff
~# md5sum  /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp
9d28d1b11de8b15ba9152644219d9a78
/sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp
~#

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Michael Walle]

Hi,

>> > +             .id = SNOR_ID(0x20, 0xbb, 0x21, 0x10, 0x44, 0x00),
>> > +             .name = "mt25qu01g",
>> > +             .size = SZ_128M,
>> 
>> Not needed, parsed by SFDP.
>> 
>> > +             .flags = NO_CHIP_ERASE,
>> 
>> Is it fair to assume that this is the usual case for multi die chips?
>> See also,
>> https://lore.kernel.org/linux-mtd/cover.1680849425.git.Takahiro.Kuwano@infineon.com/
>> 
>> So "if n_dice > 1" will implicitly be NO_CHIP_ERASE. n_dice should be
>> parsed from SFDP.
>> 
>> > +             .no_sfdp_flags = SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ,
>> 
>> .no_sfdp_flags are legacy and shouldn't be used with new flashes.
> 
> I tried your suggestion and tested with the change below:

You'd need to change the core to assume there is no chip
erase support if "n_dice > 1". So first step would be that
your SPI flash will successfully parse the SFDP and set n_dice
correctly. Then adapt the core to set SNOR_F_NO_OP_CHIP_ERASE
if "n_dice > 1".
The goal here is to avoid any entry in our database at all and
support this flash out of the box by just parsing SFDP correctly.

Which leaves us with this damn USE_FSR..

> ~# cat 
> /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/jedec_id
> 20bb21104400
> ~# cat 
> /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/manufacturer
> st
> ~# cat 
> /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/partname
> mt25qu01g
> ~# xxd -p  
> /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp
> 53464450060101ff00060110300000ff84000102800000ffffffffffffff
> ffffffffffffffffffffffffffffffffffffe520fbffffffff3f29eb276b
> 273b27bbffffffffffff27bbffff29eb0c2010d80f520000244a99008b8e
> 03e1ac0127387a757a75fbbdd55c4a0f82ff81bd3d36ffffffffffffffff
> ffffffffffffffffffe7ffff21dcffff
> ~# md5sum  
> /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp
> 9d28d1b11de8b15ba9152644219d9a78
> /sys/devices/platform/soc@0/30800000.bus/30bb0000.spi/spi_master/spi0/spi0.0/spi-nor/sfdp

thanks!

-michael

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

Hi Michael,

On Tue, Oct 24, 2023 at 10:59 AM Michael Walle <michael@walle.cc> wrote:

> You'd need to change the core to assume there is no chip
> erase support if "n_dice > 1". So first step would be that
> your SPI flash will successfully parse the SFDP and set n_dice
> correctly. Then adapt the core to set SNOR_F_NO_OP_CHIP_ERASE
> if "n_dice > 1".
> The goal here is to avoid any entry in our database at all and
> support this flash out of the box by just parsing SFDP correctly.

Where in SFDP is located the information about the number of dies?

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Tudor Ambarus]

On 10/24/23 16:47, Fabio Estevam wrote:
> Hi Michael,
> 
> On Tue, Oct 24, 2023 at 10:59 AM Michael Walle <michael@walle.cc> wrote:
> 
>> You'd need to change the core to assume there is no chip
>> erase support if "n_dice > 1". So first step would be that
>> your SPI flash will successfully parse the SFDP and set n_dice
>> correctly. Then adapt the core to set SNOR_F_NO_OP_CHIP_ERASE
>> if "n_dice > 1".
>> The goal here is to avoid any entry in our database at all and
>> support this flash out of the box by just parsing SFDP correctly.
> 
> Where in SFDP is located the information about the number of dies?

drivers/mtd/spi-nor/sfdp.c, search for n_dice. spi_nor_parse_sccr_mc()

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

On Tue, Oct 24, 2023 at 12:53 PM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:

> drivers/mtd/spi-nor/sfdp.c, search for n_dice. spi_nor_parse_sccr_mc()

On my tests, spi_nor_parse_sccr_mc() never gets called.

In the 'switch (SFDP_PARAM_HEADER_ID(param_header))' the following
case is reached.

case SFDP_4BAIT_ID:
err = spi_nor_parse_4bait(nor, param_header);
break;

so spi_nor_parse_sccr_mc() does not run.

What should be done to get spi_nor_parse_sccr_mc() called?

Thanks

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Michael Walle]

Am 2023-10-24 18:20, schrieb Fabio Estevam:
> On Tue, Oct 24, 2023 at 12:53 PM Tudor Ambarus 
> <tudor.ambarus@linaro.org> wrote:
> 
>> drivers/mtd/spi-nor/sfdp.c, search for n_dice. spi_nor_parse_sccr_mc()
> 
> On my tests, spi_nor_parse_sccr_mc() never gets called.
> 
> In the 'switch (SFDP_PARAM_HEADER_ID(param_header))' the following
> case is reached.
> 
> case SFDP_4BAIT_ID:
> err = spi_nor_parse_4bait(nor, param_header);
> break;
> 
> so spi_nor_parse_sccr_mc() does not run.
> 
> What should be done to get spi_nor_parse_sccr_mc() called?

Mhh, that's a pity. It seems that your SFDP in the flash doesn't
contain that table. I haven't confirmed it by looking at your dump
as I'm about to go on vacation tomorrow and packing right now.

What do we do in that case, Tudor? I presume a fixup which sets n_dice
(any maybe others) to the correct value?

-michael

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

On Tue, Oct 24, 2023 at 1:36 PM Michael Walle <michael@walle.cc> wrote:

> Mhh, that's a pity. It seems that your SFDP in the flash doesn't
> contain that table. I haven't confirmed it by looking at your dump
> as I'm about to go on vacation tomorrow and packing right now.

Just did a hack to force calling  spi_nor_parse_sccr_mc():

diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
index b3b11dfed789..dc61bb5c4f7f 100644
--- a/drivers/mtd/spi-nor/sfdp.c
+++ b/drivers/mtd/spi-nor/sfdp.c
@@ -1320,7 +1320,8 @@ static int spi_nor_parse_sccr_mc(struct spi_nor *nor,
                params->vreg_offset[i] = dwords[SFDP_DWORD(i) * 2];

        params->n_dice = n_dice;
-
+
+       pr_err("***** params->n_dice is %d\n", params->n_dice);
 out:
        kfree(dwords);
        return ret;
@@ -1532,6 +1533,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)

                case SFDP_4BAIT_ID:
                         err = spi_nor_parse_4bait(nor, param_header);
+                       err = spi_nor_parse_sccr_mc(nor, param_header);
                        break;

                case SFDP_PROFILE1_ID:

When booting with this change I get:

[    1.855903] ***** params->n_dice is 2
[    1.859626] spi-nor spi0.0: mt25qu01g (131072 Kbytes)

So spi_nor_parse_sccr_mc() was able to read the number of dies correctly.

> What do we do in that case, Tudor? I presume a fixup which sets n_dice
> (any maybe others) to the correct value?

Maybe I can add a fixup that calls spi_nor_parse_sccr_mc()?

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Tudor Ambarus]

On 24.10.2023 19:43, Fabio Estevam wrote:
> On Tue, Oct 24, 2023 at 1:36 PM Michael Walle <michael@walle.cc> wrote:
> 
>> Mhh, that's a pity. It seems that your SFDP in the flash doesn't
>> contain that table. I haven't confirmed it by looking at your dump
>> as I'm about to go on vacation tomorrow and packing right now.
> 
> Just did a hack to force calling  spi_nor_parse_sccr_mc():
> 
> diff --git a/drivers/mtd/spi-nor/sfdp.c b/drivers/mtd/spi-nor/sfdp.c
> index b3b11dfed789..dc61bb5c4f7f 100644
> --- a/drivers/mtd/spi-nor/sfdp.c
> +++ b/drivers/mtd/spi-nor/sfdp.c
> @@ -1320,7 +1320,8 @@ static int spi_nor_parse_sccr_mc(struct spi_nor *nor,
>                 params->vreg_offset[i] = dwords[SFDP_DWORD(i) * 2];
> 
>         params->n_dice = n_dice;
> -
> +
> +       pr_err("***** params->n_dice is %d\n", params->n_dice);
>  out:
>         kfree(dwords);
>         return ret;
> @@ -1532,6 +1533,7 @@ int spi_nor_parse_sfdp(struct spi_nor *nor)
> 
>                 case SFDP_4BAIT_ID:
>                          err = spi_nor_parse_4bait(nor, param_header);
> +                       err = spi_nor_parse_sccr_mc(nor, param_header);
>                         break;
> 
>                 case SFDP_PROFILE1_ID:
> 
> When booting with this change I get:
> 
> [    1.855903] ***** params->n_dice is 2
> [    1.859626] spi-nor spi0.0: mt25qu01g (131072 Kbytes)
> 
> So spi_nor_parse_sccr_mc() was able to read the number of dies correctly.

you probably just got lucky, the SFDP tables are just 32bit data. You
actually have to have that table defined in the SFDP space.

Check the jesd216 sfdp standard and look into the SFDP Parameter Header
(section 6.3). Each table has a ID LSB & MSB and a parameter table
pointer. If the SCCR ID is not there then the table is not defined, thus
not supported. Let me know what you find. I'll take a look too if you
have troubles decoding the sfdp dump data. We'd like to add a sfdp
decoder in the future to ease the process, but it's not something that I
can allocate time for now.

> 
>> What do we do in that case, Tudor? I presume a fixup which sets n_dice
>> (any maybe others) to the correct value?
> 
> Maybe I can add a fixup that calls spi_nor_parse_sccr_mc()?

I doubt it. You probably need something like s25fs256t_post_sfdp_fixup().

cheers,
ta

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

Hi Tudor,

On Wed, Oct 25, 2023 at 12:34 AM Tudor Ambarus <tudor.ambarus@linaro.org> wrote:

> you probably just got lucky, the SFDP tables are just 32bit data. You
> actually have to have that table defined in the SFDP space.

Oh, I see.

It's unfortunate that SFDP_SCCR_MAP_MC_ID is not defined in this SPI NOR flash.

> Check the jesd216 sfdp standard and look into the SFDP Parameter Header
> (section 6.3). Each table has a ID LSB & MSB and a parameter table
> pointer. If the SCCR ID is not there then the table is not defined, thus
> not supported. Let me know what you find. I'll take a look too if you
> have troubles decoding the sfdp dump data. We'd like to add a sfdp
> decoder in the future to ease the process, but it's not something that I
> can allocate time for now.

Thanks for your explanation.

I have downloaded the  jesd216 spec and will take a close look at it.

> > Maybe I can add a fixup that calls spi_nor_parse_sccr_mc()?
>
> I doubt it. You probably need something like s25fs256t_post_sfdp_fixup().

I need to become familiar with the sfdp internals to be able to come
up with a post_sfdp_fixup()
implementation.

In the meantime, could you please consider the v2 proposal?
https://lore.kernel.org/linux-mtd/20231024135826.2729088-1-festevam@gmail.com/T/#u

mt25qu01g is very similar to the already supported mt25qu02g.

In v2, the .size and .no_sfdp_flag fields were removed, which is an
improvement over mt25qu02g.

Only NO_CHIP_ERASE still needs to be passed, but that can be removed
later when the
mt25qu01g_post_sfdp_fixup() is implemented.

Does this sound reasonable?

Thanks

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

Hi Tudor,

On Wed, Oct 25, 2023 at 8:56 AM Fabio Estevam <festevam@gmail.com> wrote:

> In the meantime, could you please consider the v2 proposal?
> https://lore.kernel.org/linux-mtd/20231024135826.2729088-1-festevam@gmail.com/T/#u
>
> mt25qu01g is very similar to the already supported mt25qu02g.
>
> In v2, the .size and .no_sfdp_flag fields were removed, which is an
> improvement over mt25qu02g.
>
> Only NO_CHIP_ERASE still needs to be passed, but that can be removed
> later when the
> mt25qu01g_post_sfdp_fixup() is implemented.
>
> Does this sound reasonable?

I have also tried adding a mt25qu01g_post_sfdp_fixup().

The changes below should be split into 3 different patches, but just
wanted to share them with you for feedback.

diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
index 1c443fe568cf..f85ea57534ba 100644
--- a/drivers/mtd/spi-nor/core.c
+++ b/drivers/mtd/spi-nor/core.c
@@ -2850,7 +2850,7 @@ static void spi_nor_init_flags(struct spi_nor *nor)
                        nor->flags |= SNOR_F_HAS_SR_BP3_BIT6;
        }

-       if (flags & NO_CHIP_ERASE)
+       if (flags & NO_CHIP_ERASE || nor->params->n_dice > 1)
                nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;

        if (flags & SPI_NOR_RWW && nor->params->n_banks > 1 &&
diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
index 8920547c12bf..21fef439d5f1 100644
--- a/drivers/mtd/spi-nor/micron-st.c
+++ b/drivers/mtd/spi-nor/micron-st.c
@@ -192,6 +192,20 @@ static struct spi_nor_fixups mt25qu512a_fixups = {
        .post_bfpt = mt25qu512a_post_bfpt_fixup,
 };

+static int mt25qu01g_post_sfdp_fixup(struct spi_nor *nor)
+{
+       struct spi_nor_flash_parameter *params = nor->params;
+
+       /* MT25QU01G does not define the SCCR entry, so pass n_dice
manually.  */
+       params->n_dice = 2;
+
+       return 0;
+}
+
+static struct spi_nor_fixups mt25qu01g_fixups = {
+       .post_sfdp = mt25qu01g_post_sfdp_fixup,
+};
+
 static const struct flash_info st_nor_parts[] = {
        {
                .name = "m25p05-nonjedec",
@@ -429,6 +443,11 @@ static const struct flash_info st_nor_parts[] = {
                         SPI_NOR_BP3_SR_BIT6,
                .no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
                .mfr_flags = USE_FSR,
+       }, {
+               .id = SNOR_ID(0x20, 0xbb, 0x21, 0x10, 0x44, 0x00),
+               .name = "mt25qu01g",
+               .mfr_flags = USE_FSR,
+               .fixups = &mt25qu01g_fixups
        }, {
                .id = SNOR_ID(0x20, 0xbb, 0x21),
                .name = "n25q00a",

Please let me know what is your preference.

Thanks

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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Michael Walle]

Am 25. Oktober 2023 20:35:40 OESZ schrieb Fabio Estevam <festevam@gmail.com>:
>Hi Tudor,
>
>On Wed, Oct 25, 2023 at 8:56 AM Fabio Estevam <festevam@gmail.com> wrote:
>
>> In the meantime, could you please consider the v2 proposal?
>> https://lore.kernel.org/linux-mtd/20231024135826.2729088-1-festevam@gmail.com/T/#u
>>
>> mt25qu01g is very similar to the already supported mt25qu02g.
>>
>> In v2, the .size and .no_sfdp_flag fields were removed, which is an
>> improvement over mt25qu02g.
>>
>> Only NO_CHIP_ERASE still needs to be passed, but that can be removed
>> later when the
>> mt25qu01g_post_sfdp_fixup() is implemented.
>>
>> Does this sound reasonable?
>
>I have also tried adding a mt25qu01g_post_sfdp_fixup().
>
>The changes below should be split into 3 different patches, but just
>wanted to share them with you for feedback.

Two patches. no? one for the no chip erase flag and one for adding the flash. 

And I'd prefer this version. 

>diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c
>index 1c443fe568cf..f85ea57534ba 100644
>--- a/drivers/mtd/spi-nor/core.c
>+++ b/drivers/mtd/spi-nor/core.c
>@@ -2850,7 +2850,7 @@ static void spi_nor_init_flags(struct spi_nor *nor)
>                        nor->flags |= SNOR_F_HAS_SR_BP3_BIT6;
>        }
>
>-       if (flags & NO_CHIP_ERASE)
>+       if (flags & NO_CHIP_ERASE || nor->params->n_dice > 1)
>                nor->flags |= SNOR_F_NO_OP_CHIP_ERASE;
>
>        if (flags & SPI_NOR_RWW && nor->params->n_banks > 1 &&
>diff --git a/drivers/mtd/spi-nor/micron-st.c b/drivers/mtd/spi-nor/micron-st.c
>index 8920547c12bf..21fef439d5f1 100644
>--- a/drivers/mtd/spi-nor/micron-st.c
>+++ b/drivers/mtd/spi-nor/micron-st.c
>@@ -192,6 +192,20 @@ static struct spi_nor_fixups mt25qu512a_fixups = {
>        .post_bfpt = mt25qu512a_post_bfpt_fixup,
> };
>
>+static int mt25qu01g_post_sfdp_fixup(struct spi_nor *nor)
>+{
>+       struct spi_nor_flash_parameter *params = nor->params;
>+
>+       /* MT25QU01G does not define the SCCR entry, so pass n_dice
>manually.  */
>+       params->n_dice = 2;
>+
>+       return 0;
>+}
>+
>+static struct spi_nor_fixups mt25qu01g_fixups = {
>+       .post_sfdp = mt25qu01g_post_sfdp_fixup,
>+};
>+
> static const struct flash_info st_nor_parts[] = {
>        {
>                .name = "m25p05-nonjedec",
>@@ -429,6 +443,11 @@ static const struct flash_info st_nor_parts[] = {
>                         SPI_NOR_BP3_SR_BIT6,
>                .no_sfdp_flags = SECT_4K | SPI_NOR_QUAD_READ,
>                .mfr_flags = USE_FSR,
>+       }, {
>+               .id = SNOR_ID(0x20, 0xbb, 0x21, 0x10, 0x44, 0x00),
>+               .name = "mt25qu01g",
>+               .mfr_flags = USE_FSR,

out of curiosity. does it also work without USE_FSR? I'm not implying to remove it, just want to get a feeling if this 
needed at all. 

-michael

>+               .fixups = &mt25qu01g_fixups
>        }, {
>                .id = SNOR_ID(0x20, 0xbb, 0x21),
>                .name = "n25q00a",
>
>Please let me know what is your preference.
>
>Thanks


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

Re: [PATCH] mtd: spi-nor: micron-st: Add support for mt25qu01g

[Fabio Estevam]

Hi Michael,

On Thu, Oct 26, 2023 at 3:33 AM Michael Walle <michael@walle.cc> wrote:

> Two patches. no? one for the no chip erase flag and one for adding the flash.

You are right. I just sent a v3 with the two patches.

> And I'd prefer this version.

> out of curiosity. does it also work without USE_FSR? I'm not implying to remove it, just want to get a feeling if this
> needed at all.

Yes, it worked without USE_FSR and submitted v3 without it.

Thanks

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