diff mbox series

[v2,05/13] mtd: spi-nor-ids: add support for Macronix mx25u51245g and mx66u1g45g flash

Message ID 161861628460.298230.15310595411933381824.stgit@localhost
State New
Headers show
Series arm64: synquacer: Add SynQuacer/DeveloperBox support | expand

Commit Message

Masami Hiramatsu April 16, 2021, 11:38 p.m. UTC 
From: Jassi Brar <jaswinder.singh@linaro.org>


Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

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

Comments

Pratyush Yadav April 19, 2021, 8:41 a.m. UTC | #1 
On 17/04/21 08:38AM, Masami Hiramatsu wrote:
> From: Jassi Brar <jaswinder.singh@linaro.org>

> 

> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

> ---

>  drivers/mtd/spi/spi-nor-ids.c |    2 ++

>  1 file changed, 2 insertions(+)

> 

> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c

> index 2b57797954..30a18b4140 100644

> --- a/drivers/mtd/spi/spi-nor-ids.c

> +++ b/drivers/mtd/spi/spi-nor-ids.c

> @@ -160,9 +160,11 @@ const struct flash_info spi_nor_ids[] = {

>  	{ INFO("mx25l12855e", 0xc22618, 0, 64 * 1024, 256, 0) },

>  	{ INFO("mx25l25635e", 0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

>  	{ INFO("mx25u25635f", 0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) },

> +	{ INFO("mx25u51245g", 0xc2253a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },


The flash ID is the same as that of mx66u51235f. Because the ID matching 
function searches through this array in linear fashion, mx66u51235f can 
never be detected. This is a regression.

I am seeing a lot of ID collisions on Macronix flashes recently [0]. Not 
sure how to handle them though. At least in this case both flashes use 
the same set of flags so it should just change the name of the flash 
detected.

[0] https://lore.kernel.org/linux-mtd/CAEyMn7ZEp9f1SuE6umRDWkr8bVT5hdRi-4F3+G-GP9anuGG1Bw@mail.gmail.com/T/#u

>  	{ INFO("mx25l25655e", 0xc22619, 0, 64 * 1024, 512, 0) },

>  	{ INFO("mx66l51235l", 0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

>  	{ INFO("mx66u51235f", 0xc2253a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> +	{ INFO("mx66u1g45g",  0xc2253b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

>  	{ INFO("mx66u2g45g",  0xc2253c, 0, 64 * 1024, 4096, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

>  	{ INFO("mx66l1g45g",  0xc2201b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

>  	{ INFO("mx25l1633e", 0xc22415, 0, 64 * 1024,   32, SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | SECT_4K) },

> 


-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.
Masami Hiramatsu April 21, 2021, 2:39 a.m. UTC | #2 
Hello Pratyush,

2021年4月19日(月) 17:41 Pratyush Yadav <p.yadav@ti.com>:
>

> On 17/04/21 08:38AM, Masami Hiramatsu wrote:

> > From: Jassi Brar <jaswinder.singh@linaro.org>

> >

> > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

> > ---

> >  drivers/mtd/spi/spi-nor-ids.c |    2 ++

> >  1 file changed, 2 insertions(+)

> >

> > diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c

> > index 2b57797954..30a18b4140 100644

> > --- a/drivers/mtd/spi/spi-nor-ids.c

> > +++ b/drivers/mtd/spi/spi-nor-ids.c

> > @@ -160,9 +160,11 @@ const struct flash_info spi_nor_ids[] = {

> >       { INFO("mx25l12855e", 0xc22618, 0, 64 * 1024, 256, 0) },

> >       { INFO("mx25l25635e", 0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

> >       { INFO("mx25u25635f", 0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) },

> > +     { INFO("mx25u51245g", 0xc2253a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

>

> The flash ID is the same as that of mx66u51235f. Because the ID matching

> function searches through this array in linear fashion, mx66u51235f can

> never be detected. This is a regression.


OK

>

> I am seeing a lot of ID collisions on Macronix flashes recently [0]. Not

> sure how to handle them though. At least in this case both flashes use

> the same set of flags so it should just change the name of the flash

> detected.


Would you mean rename the entry as below?

{ INFO("mx66u51235f/mx25u51245g",...

Thank you,

>

> [0] https://lore.kernel.org/linux-mtd/CAEyMn7ZEp9f1SuE6umRDWkr8bVT5hdRi-4F3+G-GP9anuGG1Bw@mail.gmail.com/T/#u

>

> >       { INFO("mx25l25655e", 0xc22619, 0, 64 * 1024, 512, 0) },

> >       { INFO("mx66l51235l", 0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> >       { INFO("mx66u51235f", 0xc2253a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> > +     { INFO("mx66u1g45g",  0xc2253b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> >       { INFO("mx66u2g45g",  0xc2253c, 0, 64 * 1024, 4096, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> >       { INFO("mx66l1g45g",  0xc2201b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

> >       { INFO("mx25l1633e", 0xc22415, 0, 64 * 1024,   32, SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | SECT_4K) },

> >

>

> --

> Regards,

> Pratyush Yadav

> Texas Instruments Inc.




-- 
Masami Hiramatsu
Pratyush Yadav April 21, 2021, 11:15 a.m. UTC | #3 
On 21/04/21 11:39AM, Masami Hiramatsu wrote:
> Hello Pratyush,

> 

> 2021年4月19日(月) 17:41 Pratyush Yadav <p.yadav@ti.com>:

> >

> > On 17/04/21 08:38AM, Masami Hiramatsu wrote:

> > > From: Jassi Brar <jaswinder.singh@linaro.org>

> > >

> > > Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

> > > ---

> > >  drivers/mtd/spi/spi-nor-ids.c |    2 ++

> > >  1 file changed, 2 insertions(+)

> > >

> > > diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c

> > > index 2b57797954..30a18b4140 100644

> > > --- a/drivers/mtd/spi/spi-nor-ids.c

> > > +++ b/drivers/mtd/spi/spi-nor-ids.c

> > > @@ -160,9 +160,11 @@ const struct flash_info spi_nor_ids[] = {

> > >       { INFO("mx25l12855e", 0xc22618, 0, 64 * 1024, 256, 0) },

> > >       { INFO("mx25l25635e", 0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

> > >       { INFO("mx25u25635f", 0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) },

> > > +     { INFO("mx25u51245g", 0xc2253a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> >

> > The flash ID is the same as that of mx66u51235f. Because the ID matching

> > function searches through this array in linear fashion, mx66u51235f can

> > never be detected. This is a regression.

> 

> OK

> 

> >

> > I am seeing a lot of ID collisions on Macronix flashes recently [0]. Not

> > sure how to handle them though. At least in this case both flashes use

> > the same set of flags so it should just change the name of the flash

> > detected.

> 

> Would you mean rename the entry as below?

> 

> { INFO("mx66u51235f/mx25u51245g",...


No, I am saying that your change will make mx66u51235f get detected as 
mx25u51245g, but it won't make any difference in practice because both 
entries have the same flags set.

There have been some ideas on the Linux side about how to handle these 
collisions. One of them being that the SFDP contents can be used to 
differentiate between flashes having the same ID. I would appreciate it 
if you help drive the solution forward on the Linux side, and then it 
can be ported back to U-Boot.

> 

> Thank you,

> 

> >

> > [0] https://lore.kernel.org/linux-mtd/CAEyMn7ZEp9f1SuE6umRDWkr8bVT5hdRi-4F3+G-GP9anuGG1Bw@mail.gmail.com/T/#u

> >

> > >       { INFO("mx25l25655e", 0xc22619, 0, 64 * 1024, 512, 0) },

> > >       { INFO("mx66l51235l", 0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> > >       { INFO("mx66u51235f", 0xc2253a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> > > +     { INFO("mx66u1g45g",  0xc2253b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> > >       { INFO("mx66u2g45g",  0xc2253c, 0, 64 * 1024, 4096, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },

> > >       { INFO("mx66l1g45g",  0xc2201b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },

> > >       { INFO("mx25l1633e", 0xc22415, 0, 64 * 1024,   32, SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | SECT_4K) },

> > >

> >


-- 
Regards,
Pratyush Yadav
Texas Instruments Inc.
diff mbox series

Patch

diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index 2b57797954..30a18b4140 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -160,9 +160,11 @@  const struct flash_info spi_nor_ids[] = {
 	{ INFO("mx25l12855e", 0xc22618, 0, 64 * 1024, 256, 0) },
 	{ INFO("mx25l25635e", 0xc22019, 0, 64 * 1024, 512, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ INFO("mx25u25635f", 0xc22539, 0, 64 * 1024, 512, SECT_4K | SPI_NOR_4B_OPCODES) },
+	{ INFO("mx25u51245g", 0xc2253a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 	{ INFO("mx25l25655e", 0xc22619, 0, 64 * 1024, 512, 0) },
 	{ INFO("mx66l51235l", 0xc2201a, 0, 64 * 1024, 1024, SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 	{ INFO("mx66u51235f", 0xc2253a, 0, 64 * 1024, 1024, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
+	{ INFO("mx66u1g45g",  0xc2253b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 	{ INFO("mx66u2g45g",  0xc2253c, 0, 64 * 1024, 4096, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES) },
 	{ INFO("mx66l1g45g",  0xc2201b, 0, 64 * 1024, 2048, SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 	{ INFO("mx25l1633e", 0xc22415, 0, 64 * 1024,   32, SPI_NOR_QUAD_READ | SPI_NOR_4B_OPCODES | SECT_4K) },