[PATCH] mtd:spi-nor:Update Winbond SPI NOR Flash device ID

[PATCH] mtd:spi-nor:Update Winbond SPI NOR Flash device ID

[Steam Lin]

This patch is to update Winbond SPI NOR device
ID information.
Add new 3.3V and 1.8V device in the ID table.

Signed-off-by: Steam Lin <Stlin2@winbond.com>
---
 drivers/mtd/spi-nor/winbond.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/mtd/spi-nor/winbond.c b/drivers/mtd/spi-nor/winbond.c
index 9a81c67a60c6..01aa49954793 100644
--- a/drivers/mtd/spi-nor/winbond.c
+++ b/drivers/mtd/spi-nor/winbond.c
@@ -102,6 +102,20 @@ static const struct flash_info winbond_parts[] = {
 			    SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ) },
 	{ "w25q512jvq", INFO(0xef4020, 0, 64 * 1024, 1024,
 			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "w25h512jvm", INFO(0xef9020, 0, 64 * 1024, 1024,
+			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "w25q01jvq", INFO(0xef4021, 0, 64 * 1024, 2048,
+			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "w25q02jvq", INFO(0xef4022, 0, 64 * 1024, 4096,
+			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "w25h01jvm", INFO(0xef9021, 0, 64 * 1024, 2048,
+			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "w25h02jvm", INFO(0xef9022, 0, 64 * 1024, 4096,
+			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024,
+			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
+	{ "w25q512nwm", INFO(0xef8020, 0, 64 * 1024, 1024,
+			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
 };
 
 /**
-- 
2.17.1

Re: [PATCH] mtd:spi-nor:Update Winbond SPI NOR Flash device ID

[Michael Walle]

Hi,

Am 2021-07-07 12:16, schrieb Steam Lin:
> This patch is to update Winbond SPI NOR device
> ID information.
> Add new 3.3V and 1.8V device in the ID table.
> 
> Signed-off-by: Steam Lin <Stlin2@winbond.com>
> ---
>  drivers/mtd/spi-nor/winbond.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/mtd/spi-nor/winbond.c 
> b/drivers/mtd/spi-nor/winbond.c
> index 9a81c67a60c6..01aa49954793 100644
> --- a/drivers/mtd/spi-nor/winbond.c
> +++ b/drivers/mtd/spi-nor/winbond.c
> @@ -102,6 +102,20 @@ static const struct flash_info winbond_parts[] = {
>  			    SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ) },
>  	{ "w25q512jvq", INFO(0xef4020, 0, 64 * 1024, 1024,
>  			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +	{ "w25h512jvm", INFO(0xef9020, 0, 64 * 1024, 1024,
> +			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +	{ "w25q01jvq", INFO(0xef4021, 0, 64 * 1024, 2048,
> +			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +	{ "w25q02jvq", INFO(0xef4022, 0, 64 * 1024, 4096,
> +			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +	{ "w25h01jvm", INFO(0xef9021, 0, 64 * 1024, 2048,
> +			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +	{ "w25h02jvm", INFO(0xef9022, 0, 64 * 1024, 4096,
> +			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +	{ "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024,
> +			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> +	{ "w25q512nwm", INFO(0xef8020, 0, 64 * 1024, 1024,
> +			     SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
>  };

Nice to see patches from the vendor! Did you test these devices? We
only accept new IDs which are actually tested. And are you aware of any
ID collisions of these chips? Eg. sometimes the JWM reused the ID of the
FW variant. How can we distinguish these?

Also, you will have to supply SFDP data for all these chips, please have
a look at [1] how to do that.

[1] 
https://lore.kernel.org/linux-mtd/7038f037de3e224016d269324517400d@walle.cc/


-michael

Re: [PATCH] mtd:spi-nor:Update Winbond SPI NOR Flash device ID

[MODEL WORK LST]

Shame on me, I don't use the plain text to reply.
Please allow me send again.

Dear Michael sir:

Many thanks for your prompt response.
It is my first time update patch for Linux, please advise me if I do wrong.
We are testing these device by the NXP evaluation board.
But the running Linux revision for that board is still 4.x. To update
the latest ID,
should we prepare the system that can work with latest Linux revision?
For the test process, I was wondering to ask mount the device to UBIFS
is a good way for test?

For the ID, this time we add new ID that is not include in the flash_info[].
And we would keep our device who share the same ID have compatible
with each other.
Make sure the FW or SW only need to maintain an unique for each density.
If the same density device have different behavior, Winbond would apply new ID.
Or have application note for customer to aware the difference.

For the SFDP, I would check how to work and fill the information.

Thanks again for your reply and information share.

Sincerely

Steam Lin


Michael Walle <michael@walle.cc> 於 2021年7月7日 週三 下午7:31寫道：
>
> Hi,
>
> Am 2021-07-07 12:16, schrieb Steam Lin:
> > This patch is to update Winbond SPI NOR device
> > ID information.
> > Add new 3.3V and 1.8V device in the ID table.
> >
> > Signed-off-by: Steam Lin <Stlin2@winbond.com>
> > ---
> >  drivers/mtd/spi-nor/winbond.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/mtd/spi-nor/winbond.c
> > b/drivers/mtd/spi-nor/winbond.c
> > index 9a81c67a60c6..01aa49954793 100644
> > --- a/drivers/mtd/spi-nor/winbond.c
> > +++ b/drivers/mtd/spi-nor/winbond.c
> > @@ -102,6 +102,20 @@ static const struct flash_info winbond_parts[] = {
> >                           SECT_4K | SPI_NOR_QUAD_READ | SPI_NOR_DUAL_READ) },
> >       { "w25q512jvq", INFO(0xef4020, 0, 64 * 1024, 1024,
> >                            SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > +     { "w25h512jvm", INFO(0xef9020, 0, 64 * 1024, 1024,
> > +                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > +     { "w25q01jvq", INFO(0xef4021, 0, 64 * 1024, 2048,
> > +                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > +     { "w25q02jvq", INFO(0xef4022, 0, 64 * 1024, 4096,
> > +                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > +     { "w25h01jvm", INFO(0xef9021, 0, 64 * 1024, 2048,
> > +                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > +     { "w25h02jvm", INFO(0xef9022, 0, 64 * 1024, 4096,
> > +                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > +     { "w25q512nwq", INFO(0xef6020, 0, 64 * 1024, 1024,
> > +                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> > +     { "w25q512nwm", INFO(0xef8020, 0, 64 * 1024, 1024,
> > +                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ) },
> >  };
>
> Nice to see patches from the vendor! Did you test these devices? We
> only accept new IDs which are actually tested. And are you aware of any
> ID collisions of these chips? Eg. sometimes the JWM reused the ID of the
> FW variant. How can we distinguish these?
>
> Also, you will have to supply SFDP data for all these chips, please have
> a look at [1] how to do that.
>
> [1]
> https://lore.kernel.org/linux-mtd/7038f037de3e224016d269324517400d@walle.cc/
>
>
> -michael

Re: [PATCH] mtd:spi-nor:Update Winbond SPI NOR Flash device ID

[Michael Walle]

Hi,

Am 2021-07-07 13:58, schrieb MODEL WORK LST:
> Shame on me, I don't use the plain text to reply.
> Please allow me send again.

Please also avoid top-posting ;)

> It is my first time update patch for Linux, please advise me if I do 
> wrong.

No worries!

> We are testing these device by the NXP evaluation board.

Ok, this should then go into the commit message, too. Usually the SoC 
and
the SPI controller which was used for testing go in there.

> But the running Linux revision for that board is still 4.x. To update
> the latest ID,
> should we prepare the system that can work with latest Linux revision?

Yes that would be preferable, either use spi-nor/next or just linux-next
[1]. There, my patches for dumping/reading the SFDP are already 
integrated.

> For the test process, I was wondering to ask mount the device to UBIFS
> is a good way for test?

It depends what flags or features are you plan to support. Ususually,
reading/writing the raw mtd device will be enough. If you also add 
locking
support, for example, you should also test the locking by using the
mtd-utils.

I'd say testing whole UBIFS is overkill. Bascially you will just make
sure reading and writing is possible. And if the results make sense, eg.
does it actually use Quad I/O or Dual I/O.

Also look at dmesg and see if it is probed correctly.

> For the ID, this time we add new ID that is not include in the 
> flash_info[].
> And we would keep our device who share the same ID have compatible
> with each other.
> Make sure the FW or SW only need to maintain an unique for each 
> density.
> If the same density device have different behavior, Winbond would apply 
> new ID.

Oh I wasn't clear here. FW is not firmware but your 'FW' model. Eg.
the W25Q32FW. In my experience it shared the same id with W25Q32JW,
if I remember correctly.

We do have many problems with vendors sharing the same flash id between
different flash models which then have different features; we have
a hard time distinguish them at runtime because they appear to be
the same by looking at the JEDEC id.

So if you are aware of any other winbond flashes which reuse the
IDs which you are adding below, that would be good to know.

And to make you/Winbond aware that this is not really good and
if possible should be avoided for future chips.

> Or have application note for customer to aware the difference.

The problem is not the difference, but the fact that we cannot
detect it on runtime.

> For the SFDP, I would check how to work and fill the information.

Great. Thanks!

-michael