* Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
@ 2021-07-27 7:09 ` Michael Walle
0 siblings, 0 replies; 20+ messages in thread
From: Michael Walle @ 2021-07-27 7:09 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Am 2021-07-23 13:27, schrieb Matthias Schiffer:
> All mt25q variants have the same features.
>
> Unlike the smaller variants, no n25q with 2G exists, so we don't need
> to
> match on the extended ID to distinguish n25q and mt25q series for these
> models.
But why shouldn't we? What if there will be another flash with
the same first three id bytes?
> Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> ---
> drivers/mtd/spi-nor/micron-st.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/micron-st.c
> b/drivers/mtd/spi-nor/micron-st.c
> index c224e59820a1..d5baa8762c8d 100644
> --- a/drivers/mtd/spi-nor/micron-st.c
> +++ b/drivers/mtd/spi-nor/micron-st.c
> @@ -181,11 +181,11 @@ static const struct flash_info st_parts[] = {
> SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> NO_CHIP_ERASE) },
> { "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096,
> - SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> - NO_CHIP_ERASE) },
This bothers me. I'm not sure how this will work. I see that
chip erase is command 0xc7, but both the new and the old flash
just supports 0xc3 (DIE ERASE). Did you test these changes?
> + SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> { "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096,
> SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> - SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
> + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
>
> { "m25p05", INFO(0x202010, 0, 32 * 1024, 2, 0) },
> { "m25p10", INFO(0x202011, 0, 32 * 1024, 4, 0) },
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
2021-07-27 7:09 ` Michael Walle
@ 2021-07-27 10:45 ` Matthias Schiffer
-1 siblings, 0 replies; 20+ messages in thread
From: Matthias Schiffer @ 2021-07-27 10:45 UTC (permalink / raw)
To: Michael Walle
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
> Am 2021-07-23 13:27, schrieb Matthias Schiffer:
> > All mt25q variants have the same features.
> >
> > Unlike the smaller variants, no n25q with 2G exists, so we don't need
> > to
> > match on the extended ID to distinguish n25q and mt25q series for these
> > models.
>
> But why shouldn't we? What if there will be another flash with
> the same first three id bytes?
That makes sense, I'll update my patch accordingly.
It looked to me like the current ID list only checks the extended ID
when necessary to distinguish two known flash models.
>
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > ---
> > drivers/mtd/spi-nor/micron-st.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/micron-st.c
> > b/drivers/mtd/spi-nor/micron-st.c
> > index c224e59820a1..d5baa8762c8d 100644
> > --- a/drivers/mtd/spi-nor/micron-st.c
> > +++ b/drivers/mtd/spi-nor/micron-st.c
> > @@ -181,11 +181,11 @@ static const struct flash_info st_parts[] = {
> > SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> > NO_CHIP_ERASE) },
> > { "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096,
> > - SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> > - NO_CHIP_ERASE) },
>
> This bothers me. I'm not sure how this will work. I see that
> chip erase is command 0xc7, but both the new and the old flash
> just supports 0xc3 (DIE ERASE). Did you test these changes?
Thanks for catching this. I overlooked that the 1G and 2G variants
don't support the same erase commands as the smaller versions after
all... It is possible that I only tested this with partitioned MTD, so
I didn't hit the whole-chip erase case.
Which command should I use to test the chip erase? Will a `flash_erase
/dev/mtdX 0 0` trigger the correct operation?
>
> > + SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> > + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> > { "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096,
> > SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> > - SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
> > + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> >
> > { "m25p05", INFO(0x202010, 0, 32 * 1024, 2, 0) },
> > { "m25p10", INFO(0x202011, 0, 32 * 1024, 4, 0) },
>
> -michael
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
@ 2021-07-27 10:45 ` Matthias Schiffer
0 siblings, 0 replies; 20+ messages in thread
From: Matthias Schiffer @ 2021-07-27 10:45 UTC (permalink / raw)
To: Michael Walle
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
> Am 2021-07-23 13:27, schrieb Matthias Schiffer:
> > All mt25q variants have the same features.
> >
> > Unlike the smaller variants, no n25q with 2G exists, so we don't need
> > to
> > match on the extended ID to distinguish n25q and mt25q series for these
> > models.
>
> But why shouldn't we? What if there will be another flash with
> the same first three id bytes?
That makes sense, I'll update my patch accordingly.
It looked to me like the current ID list only checks the extended ID
when necessary to distinguish two known flash models.
>
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > ---
> > drivers/mtd/spi-nor/micron-st.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/micron-st.c
> > b/drivers/mtd/spi-nor/micron-st.c
> > index c224e59820a1..d5baa8762c8d 100644
> > --- a/drivers/mtd/spi-nor/micron-st.c
> > +++ b/drivers/mtd/spi-nor/micron-st.c
> > @@ -181,11 +181,11 @@ static const struct flash_info st_parts[] = {
> > SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> > NO_CHIP_ERASE) },
> > { "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096,
> > - SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> > - NO_CHIP_ERASE) },
>
> This bothers me. I'm not sure how this will work. I see that
> chip erase is command 0xc7, but both the new and the old flash
> just supports 0xc3 (DIE ERASE). Did you test these changes?
Thanks for catching this. I overlooked that the 1G and 2G variants
don't support the same erase commands as the smaller versions after
all... It is possible that I only tested this with partitioned MTD, so
I didn't hit the whole-chip erase case.
Which command should I use to test the chip erase? Will a `flash_erase
/dev/mtdX 0 0` trigger the correct operation?
>
> > + SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> > + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> > { "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096,
> > SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> > - SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
> > + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> >
> > { "m25p05", INFO(0x202010, 0, 32 * 1024, 2, 0) },
> > { "m25p10", INFO(0x202011, 0, 32 * 1024, 4, 0) },
>
> -michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
2021-07-27 10:45 ` Matthias Schiffer
@ 2021-07-28 10:00 ` Michael Walle
-1 siblings, 0 replies; 20+ messages in thread
From: Michael Walle @ 2021-07-28 10:00 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Am 2021-07-27 12:45, schrieb Matthias Schiffer:
> On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
[..]
>> > --- a/drivers/mtd/spi-nor/micron-st.c
>> > +++ b/drivers/mtd/spi-nor/micron-st.c
>> > @@ -181,11 +181,11 @@ static const struct flash_info st_parts[] = {
>> > SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>> > NO_CHIP_ERASE) },
>> > { "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096,
>> > - SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>> > - NO_CHIP_ERASE) },
>>
>> This bothers me. I'm not sure how this will work. I see that
>> chip erase is command 0xc7, but both the new and the old flash
>> just supports 0xc3 (DIE ERASE). Did you test these changes?
>
> Thanks for catching this. I overlooked that the 1G and 2G variants
> don't support the same erase commands as the smaller versions after
> all... It is possible that I only tested this with partitioned MTD, so
> I didn't hit the whole-chip erase case.
>
> Which command should I use to test the chip erase? Will a `flash_erase
> /dev/mtdX 0 0` trigger the correct operation?
I guess so. Looking at
http://git.infradead.org/mtd-utils.git/blob/HEAD:/misc-utils/flash_erase.c#l226
It seems you should see a different output for either erasing individual
sectors or the whole chip (as long as the kernel doesn't the invidual
block erase itself).
-michael
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
@ 2021-07-28 10:00 ` Michael Walle
0 siblings, 0 replies; 20+ messages in thread
From: Michael Walle @ 2021-07-28 10:00 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Am 2021-07-27 12:45, schrieb Matthias Schiffer:
> On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
[..]
>> > --- a/drivers/mtd/spi-nor/micron-st.c
>> > +++ b/drivers/mtd/spi-nor/micron-st.c
>> > @@ -181,11 +181,11 @@ static const struct flash_info st_parts[] = {
>> > SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>> > NO_CHIP_ERASE) },
>> > { "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096,
>> > - SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
>> > - NO_CHIP_ERASE) },
>>
>> This bothers me. I'm not sure how this will work. I see that
>> chip erase is command 0xc7, but both the new and the old flash
>> just supports 0xc3 (DIE ERASE). Did you test these changes?
>
> Thanks for catching this. I overlooked that the 1G and 2G variants
> don't support the same erase commands as the smaller versions after
> all... It is possible that I only tested this with partitioned MTD, so
> I didn't hit the whole-chip erase case.
>
> Which command should I use to test the chip erase? Will a `flash_erase
> /dev/mtdX 0 0` trigger the correct operation?
I guess so. Looking at
http://git.infradead.org/mtd-utils.git/blob/HEAD:/misc-utils/flash_erase.c#l226
It seems you should see a different output for either erasing individual
sectors or the whole chip (as long as the kernel doesn't the invidual
block erase itself).
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: (EXT) Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
2021-07-27 7:09 ` Michael Walle
@ 2021-10-06 12:32 ` Matthias Schiffer
-1 siblings, 0 replies; 20+ messages in thread
From: Matthias Schiffer @ 2021-10-06 12:32 UTC (permalink / raw)
To: Michael Walle
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
> Am 2021-07-23 13:27, schrieb Matthias Schiffer:
> > All mt25q variants have the same features.
> >
> > Unlike the smaller variants, no n25q with 2G exists, so we don't need
> > to
> > match on the extended ID to distinguish n25q and mt25q series for these
> > models.
>
> But why shouldn't we? What if there will be another flash with
> the same first three id bytes?
How do you suggest we proceed here? At the moment there are entries
matching on 0x20b[ab]22 (ignoring the extended ID) with the name
mt25q[lu]02g.
Should I change these entries to match on on the extended ID
0x20b[ab]22 / 0x104400 instead when I add the bits for the features
specific to the variant, removing support for other 0x20b[ab]22
variants that may or may not actually exist? Keeping both entries (with
and without extended ID match) would preserve compatiblity with such
variants, but this approach seems problematic to me as well, as I can't
even give a name to the more generic entries (and there is no natural
extension of the n25q naming scheme to a 2G variant).
>
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > ---
> > drivers/mtd/spi-nor/micron-st.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/micron-st.c
> > b/drivers/mtd/spi-nor/micron-st.c
> > index c224e59820a1..d5baa8762c8d 100644
> > --- a/drivers/mtd/spi-nor/micron-st.c
> > +++ b/drivers/mtd/spi-nor/micron-st.c
> > @@ -181,11 +181,11 @@ static const struct flash_info st_parts[] = {
> > SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> > NO_CHIP_ERASE) },
> > { "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096,
> > - SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> > - NO_CHIP_ERASE) },
>
> This bothers me. I'm not sure how this will work. I see that
> chip erase is command 0xc7, but both the new and the old flash
> just supports 0xc3 (DIE ERASE). Did you test these changes?
I finally got my hands on hardware with this flash again (well, a
mt25qu01g, I don't think we have the 2G variants anywhere) and I can
confirm that the chip erase does not work with my patch. I will send an
updated version that keeps NO_CHIP_ERASE.
>
> > + SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> > + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> > { "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096,
> > SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> > - SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
> > + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> >
> > { "m25p05", INFO(0x202010, 0, 32 * 1024, 2, 0) },
> > { "m25p10", INFO(0x202011, 0, 32 * 1024, 4, 0) },
>
> -michael
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: (EXT) Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
@ 2021-10-06 12:32 ` Matthias Schiffer
0 siblings, 0 replies; 20+ messages in thread
From: Matthias Schiffer @ 2021-10-06 12:32 UTC (permalink / raw)
To: Michael Walle
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
> Am 2021-07-23 13:27, schrieb Matthias Schiffer:
> > All mt25q variants have the same features.
> >
> > Unlike the smaller variants, no n25q with 2G exists, so we don't need
> > to
> > match on the extended ID to distinguish n25q and mt25q series for these
> > models.
>
> But why shouldn't we? What if there will be another flash with
> the same first three id bytes?
How do you suggest we proceed here? At the moment there are entries
matching on 0x20b[ab]22 (ignoring the extended ID) with the name
mt25q[lu]02g.
Should I change these entries to match on on the extended ID
0x20b[ab]22 / 0x104400 instead when I add the bits for the features
specific to the variant, removing support for other 0x20b[ab]22
variants that may or may not actually exist? Keeping both entries (with
and without extended ID match) would preserve compatiblity with such
variants, but this approach seems problematic to me as well, as I can't
even give a name to the more generic entries (and there is no natural
extension of the n25q naming scheme to a 2G variant).
>
> > Signed-off-by: Matthias Schiffer <matthias.schiffer@ew.tq-group.com>
> > ---
> > drivers/mtd/spi-nor/micron-st.c | 6 +++---
> > 1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/mtd/spi-nor/micron-st.c
> > b/drivers/mtd/spi-nor/micron-st.c
> > index c224e59820a1..d5baa8762c8d 100644
> > --- a/drivers/mtd/spi-nor/micron-st.c
> > +++ b/drivers/mtd/spi-nor/micron-st.c
> > @@ -181,11 +181,11 @@ static const struct flash_info st_parts[] = {
> > SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> > NO_CHIP_ERASE) },
> > { "mt25ql02g", INFO(0x20ba22, 0, 64 * 1024, 4096,
> > - SECT_4K | USE_FSR | SPI_NOR_QUAD_READ |
> > - NO_CHIP_ERASE) },
>
> This bothers me. I'm not sure how this will work. I see that
> chip erase is command 0xc7, but both the new and the old flash
> just supports 0xc3 (DIE ERASE). Did you test these changes?
I finally got my hands on hardware with this flash again (well, a
mt25qu01g, I don't think we have the 2G variants anywhere) and I can
confirm that the chip erase does not work with my patch. I will send an
updated version that keeps NO_CHIP_ERASE.
>
> > + SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> > + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> > { "mt25qu02g", INFO(0x20bb22, 0, 64 * 1024, 4096,
> > SECT_4K | USE_FSR | SPI_NOR_DUAL_READ |
> > - SPI_NOR_QUAD_READ | NO_CHIP_ERASE) },
> > + SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
> >
> > { "m25p05", INFO(0x202010, 0, 32 * 1024, 2, 0) },
> > { "m25p10", INFO(0x202011, 0, 32 * 1024, 4, 0) },
>
> -michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: (EXT) Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
2021-10-06 12:32 ` Matthias Schiffer
@ 2021-10-07 7:08 ` Michael Walle
-1 siblings, 0 replies; 20+ messages in thread
From: Michael Walle @ 2021-10-07 7:08 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Am 2021-10-06 14:32, schrieb Matthias Schiffer:
> On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
>> Am 2021-07-23 13:27, schrieb Matthias Schiffer:
>> > All mt25q variants have the same features.
>> >
>> > Unlike the smaller variants, no n25q with 2G exists, so we don't need
>> > to
>> > match on the extended ID to distinguish n25q and mt25q series for these
>> > models.
>>
>> But why shouldn't we? What if there will be another flash with
>> the same first three id bytes?
>
> How do you suggest we proceed here? At the moment there are entries
> matching on 0x20b[ab]22 (ignoring the extended ID) with the name
> mt25q[lu]02g.
>
> Should I change these entries to match on on the extended ID
> 0x20b[ab]22 / 0x104400 instead when I add the bits for the features
> specific to the variant, removing support for other 0x20b[ab]22
> variants that may or may not actually exist? Keeping both entries (with
> and without extended ID match) would preserve compatiblity with such
> variants, but this approach seems problematic to me as well, as I can't
> even give a name to the more generic entries (and there is no natural
> extension of the n25q naming scheme to a 2G variant).
Mh, what do you think of adding three entries and make the last one,
the one with the short id, as a fallback so to speak. This should
retrain backwards compatibility, right? It should probably have a
comment because the order will matter then.
-michael
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: (EXT) Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
@ 2021-10-07 7:08 ` Michael Walle
0 siblings, 0 replies; 20+ messages in thread
From: Michael Walle @ 2021-10-07 7:08 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Am 2021-10-06 14:32, schrieb Matthias Schiffer:
> On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
>> Am 2021-07-23 13:27, schrieb Matthias Schiffer:
>> > All mt25q variants have the same features.
>> >
>> > Unlike the smaller variants, no n25q with 2G exists, so we don't need
>> > to
>> > match on the extended ID to distinguish n25q and mt25q series for these
>> > models.
>>
>> But why shouldn't we? What if there will be another flash with
>> the same first three id bytes?
>
> How do you suggest we proceed here? At the moment there are entries
> matching on 0x20b[ab]22 (ignoring the extended ID) with the name
> mt25q[lu]02g.
>
> Should I change these entries to match on on the extended ID
> 0x20b[ab]22 / 0x104400 instead when I add the bits for the features
> specific to the variant, removing support for other 0x20b[ab]22
> variants that may or may not actually exist? Keeping both entries (with
> and without extended ID match) would preserve compatiblity with such
> variants, but this approach seems problematic to me as well, as I can't
> even give a name to the more generic entries (and there is no natural
> extension of the n25q naming scheme to a 2G variant).
Mh, what do you think of adding three entries and make the last one,
the one with the short id, as a fallback so to speak. This should
retrain backwards compatibility, right? It should probably have a
comment because the order will matter then.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: (EXT) Re: (EXT) Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
2021-10-07 7:08 ` Michael Walle
@ 2021-10-07 7:18 ` Matthias Schiffer
-1 siblings, 0 replies; 20+ messages in thread
From: Matthias Schiffer @ 2021-10-07 7:18 UTC (permalink / raw)
To: Michael Walle
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
On Thu, 2021-10-07 at 09:08 +0200, Michael Walle wrote:
> Am 2021-10-06 14:32, schrieb Matthias Schiffer:
> > On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
> > > Am 2021-07-23 13:27, schrieb Matthias Schiffer:
> > > > All mt25q variants have the same features.
> > > >
> > > > Unlike the smaller variants, no n25q with 2G exists, so we don't need
> > > > to
> > > > match on the extended ID to distinguish n25q and mt25q series for these
> > > > models.
> > >
> > > But why shouldn't we? What if there will be another flash with
> > > the same first three id bytes?
> >
> > How do you suggest we proceed here? At the moment there are entries
> > matching on 0x20b[ab]22 (ignoring the extended ID) with the name
> > mt25q[lu]02g.
> >
> > Should I change these entries to match on on the extended ID
> > 0x20b[ab]22 / 0x104400 instead when I add the bits for the features
> > specific to the variant, removing support for other 0x20b[ab]22
> > variants that may or may not actually exist? Keeping both entries (with
> > and without extended ID match) would preserve compatiblity with such
> > variants, but this approach seems problematic to me as well, as I can't
> > even give a name to the more generic entries (and there is no natural
> > extension of the n25q naming scheme to a 2G variant).
>
> Mh, what do you think of adding three entries and make the last one,
> the one with the short id, as a fallback so to speak. This should
> retrain backwards compatibility, right? It should probably have a
> comment because the order will matter then.
>
> -michael
Is it okay for multiple entries to use the same value for the "name"
field? In the existing definitions I couldn't find any example of
different ID matches mapping to the same name.
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: (EXT) Re: (EXT) Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
@ 2021-10-07 7:18 ` Matthias Schiffer
0 siblings, 0 replies; 20+ messages in thread
From: Matthias Schiffer @ 2021-10-07 7:18 UTC (permalink / raw)
To: Michael Walle
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
On Thu, 2021-10-07 at 09:08 +0200, Michael Walle wrote:
> Am 2021-10-06 14:32, schrieb Matthias Schiffer:
> > On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
> > > Am 2021-07-23 13:27, schrieb Matthias Schiffer:
> > > > All mt25q variants have the same features.
> > > >
> > > > Unlike the smaller variants, no n25q with 2G exists, so we don't need
> > > > to
> > > > match on the extended ID to distinguish n25q and mt25q series for these
> > > > models.
> > >
> > > But why shouldn't we? What if there will be another flash with
> > > the same first three id bytes?
> >
> > How do you suggest we proceed here? At the moment there are entries
> > matching on 0x20b[ab]22 (ignoring the extended ID) with the name
> > mt25q[lu]02g.
> >
> > Should I change these entries to match on on the extended ID
> > 0x20b[ab]22 / 0x104400 instead when I add the bits for the features
> > specific to the variant, removing support for other 0x20b[ab]22
> > variants that may or may not actually exist? Keeping both entries (with
> > and without extended ID match) would preserve compatiblity with such
> > variants, but this approach seems problematic to me as well, as I can't
> > even give a name to the more generic entries (and there is no natural
> > extension of the n25q naming scheme to a 2G variant).
>
> Mh, what do you think of adding three entries and make the last one,
> the one with the short id, as a fallback so to speak. This should
> retrain backwards compatibility, right? It should probably have a
> comment because the order will matter then.
>
> -michael
Is it okay for multiple entries to use the same value for the "name"
field? In the existing definitions I couldn't find any example of
different ID matches mapping to the same name.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: (EXT) Re: (EXT) Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
2021-10-07 7:18 ` Matthias Schiffer
@ 2021-10-07 7:37 ` Michael Walle
-1 siblings, 0 replies; 20+ messages in thread
From: Michael Walle @ 2021-10-07 7:37 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Am 2021-10-07 09:18, schrieb Matthias Schiffer:
> On Thu, 2021-10-07 at 09:08 +0200, Michael Walle wrote:
>> Am 2021-10-06 14:32, schrieb Matthias Schiffer:
>> > On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
>> > > Am 2021-07-23 13:27, schrieb Matthias Schiffer:
>> > > > All mt25q variants have the same features.
>> > > >
>> > > > Unlike the smaller variants, no n25q with 2G exists, so we don't need
>> > > > to
>> > > > match on the extended ID to distinguish n25q and mt25q series for these
>> > > > models.
>> > >
>> > > But why shouldn't we? What if there will be another flash with
>> > > the same first three id bytes?
>> >
>> > How do you suggest we proceed here? At the moment there are entries
>> > matching on 0x20b[ab]22 (ignoring the extended ID) with the name
>> > mt25q[lu]02g.
>> >
>> > Should I change these entries to match on on the extended ID
>> > 0x20b[ab]22 / 0x104400 instead when I add the bits for the features
>> > specific to the variant, removing support for other 0x20b[ab]22
>> > variants that may or may not actually exist? Keeping both entries (with
>> > and without extended ID match) would preserve compatiblity with such
>> > variants, but this approach seems problematic to me as well, as I can't
>> > even give a name to the more generic entries (and there is no natural
>> > extension of the n25q naming scheme to a 2G variant).
>>
>> Mh, what do you think of adding three entries and make the last one,
>> the one with the short id, as a fallback so to speak. This should
>> retrain backwards compatibility, right? It should probably have a
>> comment because the order will matter then.
>>
>> -michael
>
> Is it okay for multiple entries to use the same value for the "name"
> field? In the existing definitions I couldn't find any example of
> different ID matches mapping to the same name.
You're right, thats probably not good. Ok, if there is no objection
from Tudor, I'd say we change the entry of the mt25 variant to the
longer one. If there is a flash chip out there which matched the
shorter one, but not the longer one, thats probably not a mt25 anyway.
-michael
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: (EXT) Re: (EXT) Re: [PATCH 1/2] mtd: spi-nor: micron-st: sync flags of mt25ql02g and mt25qu02g with other mt25q
@ 2021-10-07 7:37 ` Michael Walle
0 siblings, 0 replies; 20+ messages in thread
From: Michael Walle @ 2021-10-07 7:37 UTC (permalink / raw)
To: Matthias Schiffer
Cc: Tudor Ambarus, Pratyush Yadav, Miquel Raynal, Richard Weinberger,
Vignesh Raghavendra, linux-mtd, linux-kernel
Am 2021-10-07 09:18, schrieb Matthias Schiffer:
> On Thu, 2021-10-07 at 09:08 +0200, Michael Walle wrote:
>> Am 2021-10-06 14:32, schrieb Matthias Schiffer:
>> > On Tue, 2021-07-27 at 09:09 +0200, Michael Walle wrote:
>> > > Am 2021-07-23 13:27, schrieb Matthias Schiffer:
>> > > > All mt25q variants have the same features.
>> > > >
>> > > > Unlike the smaller variants, no n25q with 2G exists, so we don't need
>> > > > to
>> > > > match on the extended ID to distinguish n25q and mt25q series for these
>> > > > models.
>> > >
>> > > But why shouldn't we? What if there will be another flash with
>> > > the same first three id bytes?
>> >
>> > How do you suggest we proceed here? At the moment there are entries
>> > matching on 0x20b[ab]22 (ignoring the extended ID) with the name
>> > mt25q[lu]02g.
>> >
>> > Should I change these entries to match on on the extended ID
>> > 0x20b[ab]22 / 0x104400 instead when I add the bits for the features
>> > specific to the variant, removing support for other 0x20b[ab]22
>> > variants that may or may not actually exist? Keeping both entries (with
>> > and without extended ID match) would preserve compatiblity with such
>> > variants, but this approach seems problematic to me as well, as I can't
>> > even give a name to the more generic entries (and there is no natural
>> > extension of the n25q naming scheme to a 2G variant).
>>
>> Mh, what do you think of adding three entries and make the last one,
>> the one with the short id, as a fallback so to speak. This should
>> retrain backwards compatibility, right? It should probably have a
>> comment because the order will matter then.
>>
>> -michael
>
> Is it okay for multiple entries to use the same value for the "name"
> field? In the existing definitions I couldn't find any example of
> different ID matches mapping to the same name.
You're right, thats probably not good. Ok, if there is no objection
from Tudor, I'd say we change the entry of the mt25 variant to the
longer one. If there is a flash chip out there which matched the
shorter one, but not the longer one, thats probably not a mt25 anyway.
-michael
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply [flat|nested] 20+ messages in thread