All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID
@ 2022-03-09  3:18 Marek Vasut
  2022-03-09  4:33 ` Tudor.Ambarus
  2022-04-07 19:20 ` Marek Vasut
  0 siblings, 2 replies; 6+ messages in thread
From: Marek Vasut @ 2022-03-09  3:18 UTC (permalink / raw)
  To: u-boot
  Cc: Marek Vasut, Horatiu Vultur, Jagan Teki, Simon Goldschmidt,
	Stefan Roese, Vignesh R

Add ID for Winbond W25Q128JW device. This is a 128 Mbit QSPI NOR.
Tested on W25Q128JWPIM part.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
Cc: Stefan Roese <sr@denx.de>
Cc: Vignesh R <vigneshr@ti.com>
---
 drivers/mtd/spi/spi-nor-ids.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
index b551ebd75ef..e2d09fc747d 100644
--- a/drivers/mtd/spi/spi-nor-ids.c
+++ b/drivers/mtd/spi/spi-nor-ids.c
@@ -345,6 +345,11 @@ const struct flash_info spi_nor_ids[] = {
 			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
 			SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
 	},
+	{
+		INFO("w25q128jw", 0xef8018, 0, 64 * 1024, 256,
+			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
+			SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
+	},
 	{
 		INFO("w25q256fw", 0xef6019, 0, 64 * 1024, 512,
 			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID
  2022-03-09  3:18 [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID Marek Vasut
@ 2022-03-09  4:33 ` Tudor.Ambarus
  2022-03-16  1:29   ` Marek Vasut
  2022-04-07 19:20 ` Marek Vasut
  1 sibling, 1 reply; 6+ messages in thread
From: Tudor.Ambarus @ 2022-03-09  4:33 UTC (permalink / raw)
  To: marex, u-boot
  Cc: Horatiu.Vultur, jagan, simon.k.r.goldschmidt, sr, vigneshr, michael

On 3/9/22 05:18, Marek Vasut wrote:

Hi!

> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Add ID for Winbond W25Q128JW device. This is a 128 Mbit QSPI NOR.
> Tested on W25Q128JWPIM part.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Vignesh R <vigneshr@ti.com>
> ---
>  drivers/mtd/spi/spi-nor-ids.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index b551ebd75ef..e2d09fc747d 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -345,6 +345,11 @@ const struct flash_info spi_nor_ids[] = {
>                         SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>                         SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>         },
> +       {
> +               INFO("w25q128jw", 0xef8018, 0, 64 * 1024, 256,

I'm glad I can discuss with you about the name of these winbond flashes,
you can help with an advice. In linux we name this flash "w25q128jwm",
but maybe we should change it.

Winbond names this flash "W25Q128JW_DTR", you can find the name on the
top right of each page from [1].

What I propose:
1/ according to [1] W25Q128JW_IM/JM is W25Q128JW_DTR, let's use "-dtr"
   for the M flavor.
2/ according to [2] W25Q128JW has two flavors: W25Q128JW-IQ/JQ and
   W25Q128JW-IM/JM*. Since W25Q128JW_IM/JM is called W25Q128JW_DTR, let's
   use just "W25Q128JW" for W25Q128JW-IQ/JQ, as winbond uses too.

Looking at [3], I see that W25Q128JV and W25Q128FV use the same flash ID,
I wonder what's the difference between them. I made a proposal in linux on
how to handle flash collisions, a v5 will follow.

Cheers,
ta
[1] https://www.winbond.com/resource-files/W25Q128JW_DTR%20RevF1%2005212021.pdf
[2] https://www.winbond.com/resource-files/W25Q128JW_RevG_07292021%20Plus.pdf
[3] https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/?__locale=en&selected=128Mb#Density

> +                       SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> +       },
>         {
>                 INFO("w25q256fw", 0xef6019, 0, 64 * 1024, 512,
>                         SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> --
> 2.34.1
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID
  2022-03-09  4:33 ` Tudor.Ambarus
@ 2022-03-16  1:29   ` Marek Vasut
  2022-03-16  3:55     ` Tudor.Ambarus
  0 siblings, 1 reply; 6+ messages in thread
From: Marek Vasut @ 2022-03-16  1:29 UTC (permalink / raw)
  To: Tudor.Ambarus, u-boot
  Cc: Horatiu.Vultur, jagan, simon.k.r.goldschmidt, sr, vigneshr, michael

On 3/9/22 05:33, Tudor.Ambarus@microchip.com wrote:

Hi,

[...]

>>   drivers/mtd/spi/spi-nor-ids.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
>> index b551ebd75ef..e2d09fc747d 100644
>> --- a/drivers/mtd/spi/spi-nor-ids.c
>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>> @@ -345,6 +345,11 @@ const struct flash_info spi_nor_ids[] = {
>>                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>                          SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>>          },
>> +       {
>> +               INFO("w25q128jw", 0xef8018, 0, 64 * 1024, 256,
> 
> I'm glad I can discuss with you about the name of these winbond flashes,
> you can help with an advice. In linux we name this flash "w25q128jwm",
> but maybe we should change it.

How did you arrive at "jwm" suffix ? Per both [1] and [2] "11.1 Valid 
Part Numbers and Top Side Marking" there is no "M" PACKAGE TYPE, so that 
seems wrong.

> Winbond names this flash "W25Q128JW_DTR", you can find the name on the
> top right of each page from [1].

Per 8.1 the IDs are the same for both _DTR [1] and non-DTR [2] variants.

> What I propose:
> 1/ according to [1] W25Q128JW_IM/JM is W25Q128JW_DTR, let's use "-dtr"
>     for the M flavor.
> 2/ according to [2] W25Q128JW has two flavors: W25Q128JW-IQ/JQ and
>     W25Q128JW-IM/JM*. Since W25Q128JW_IM/JM is called W25Q128JW_DTR, let's
>     use just "W25Q128JW" for W25Q128JW-IQ/JQ, as winbond uses too.

1/ That looks wrong to me. The W25Q128JW and W25Q128JW_DTR are two 
different products of Winbond, see "Features" column in:

https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/

Density -> 128Mb

W25Q128JW
1.7V - 1.95V
133MHz
SPI 4 I/O Fixed <------

W25Q128JW_DTR
1.7V - 1.95V
133MHz
SPI / QPI / DTR <------

Then, compare the datasheets [1] and [2], notice how the _DTR variant 
supports more functionality compared to the non-DTR variant:

8.1.4 Instruction Set Table 3 (QPI Instructions)
8.1.5 Instruction Set Table 4 (DTR with SPI Instructions)
8.1.6 Instruction Set Table 5 (DTR with QPI Instructions)

So my counter-proposal would be to call it just "w25q128jw" , that's the 
lowest common denominator of the DTR and non-DTR variant. And then, use 
SFDP to determine whether the QPI/DTR extras are supported by the flash 
(and thus whether it is the DTR variant or not).

The only downside is, I only have the non-DTR variant, so I cannot dump 
the SFDP tables of the DTR variant and check whether it differs from the 
non-DTR variant. I would expect it does.

2/ That looks wrong to me too. Look at [1] for the meaning of -xM/-xQ 
suffix of the flash, it has nothing to do with the DTR support. It just 
says what is the QE bit setting in SR2:

11. ORDERING INFORMATION
Q ... with QE = 1 (fixed) in Status register-2.
N ... with QE = 1 (fixed) in Status register-2 & DRV=75%. Backward 
compatible to FW family.
M ... with QE = 0 (programmable) in Status register-2. New device ID is 
used to identify FW family

> Looking at [3], I see that W25Q128JV and W25Q128FV use the same flash ID,
> I wonder what's the difference between them. I made a proposal in linux on
> how to handle flash collisions, a v5 will follow.

https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/

Density -> 128Mb

W25Q128FV
2.7V - 3.6V
104MHz <-----------
SPI / QPI <--------

W25Q128JV
2.7V - 3.6V
133MHz <-----------
SPI 4 I/O Fixed <--

Unlike the -JV, the -FV has
Instruction Set Table 3 (QPI Instructions)

See my suggestion above, use the lowest common denominator flash (in 
this case JV) as a base and then use SFDP to determine whether extra 
features are available. Would that work ?

> Cheers,
> ta
> [1] https://www.winbond.com/resource-files/W25Q128JW_DTR%20RevF1%2005212021.pdf
> [2] https://www.winbond.com/resource-files/W25Q128JW_RevG_07292021%20Plus.pdf
> [3] https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/?__locale=en&selected=128Mb#Density
> 
>> +                       SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>> +       },
>>          {
>>                  INFO("w25q256fw", 0xef6019, 0, 64 * 1024, 512,
>>                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>> --
>> 2.34.1
>>
> 

[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID
  2022-03-16  1:29   ` Marek Vasut
@ 2022-03-16  3:55     ` Tudor.Ambarus
  2022-03-16 10:07       ` Marek Vasut
  0 siblings, 1 reply; 6+ messages in thread
From: Tudor.Ambarus @ 2022-03-16  3:55 UTC (permalink / raw)
  To: marex, u-boot
  Cc: Horatiu.Vultur, jagan, simon.k.r.goldschmidt, sr, vigneshr, michael

On 3/16/22 03:29, Marek Vasut wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 3/9/22 05:33, Tudor.Ambarus@microchip.com wrote:
> 
> Hi,

Hi,

> 
> [...]
> 
>>>   drivers/mtd/spi/spi-nor-ids.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
>>> index b551ebd75ef..e2d09fc747d 100644
>>> --- a/drivers/mtd/spi/spi-nor-ids.c
>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>>> @@ -345,6 +345,11 @@ const struct flash_info spi_nor_ids[] = {
>>>                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>>                          SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>>>          },
>>> +       {
>>> +               INFO("w25q128jw", 0xef8018, 0, 64 * 1024, 256,
>>
>> I'm glad I can discuss with you about the name of these winbond flashes,
>> you can help with an advice. In linux we name this flash "w25q128jwm",
>> but maybe we should change it.
> 
> How did you arrive at "jwm" suffix ? Per both [1] and [2] "11.1 Valid

by checking sections "8.1.1 Manufacturer and Device Identification" and
"11. ORDERING INFORMATION" from the datasheets.

Package type and temperature info doesn't matter for naming convention,
but seems that the last suffix does: when M, QE = 0 (and it is programmable),
when Q or N, Quad Enable is fixed and always enabled (QE = 1).

> Part Numbers and Top Side Marking" there is no "M" PACKAGE TYPE, so that
> seems wrong.

mm, ok.

> 
>> Winbond names this flash "W25Q128JW_DTR", you can find the name on the
>> top right of each page from [1].
> 
> Per 8.1 the IDs are the same for both _DTR [1] and non-DTR [2] variants.
> 
>> What I propose:
>> 1/ according to [1] W25Q128JW_IM/JM is W25Q128JW_DTR, let's use "-dtr"
>>     for the M flavor.
>> 2/ according to [2] W25Q128JW has two flavors: W25Q128JW-IQ/JQ and
>>     W25Q128JW-IM/JM*. Since W25Q128JW_IM/JM is called W25Q128JW_DTR, let's
>>     use just "W25Q128JW" for W25Q128JW-IQ/JQ, as winbond uses too.
> 
> 1/ That looks wrong to me. The W25Q128JW and W25Q128JW_DTR are two
> different products of Winbond, see "Features" column in:
> 
> https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/
> 
> Density -> 128Mb
> 
> W25Q128JW
> 1.7V - 1.95V
> 133MHz
> SPI 4 I/O Fixed <------
> 
> W25Q128JW_DTR
> 1.7V - 1.95V
> 133MHz
> SPI / QPI / DTR <------
> 
> Then, compare the datasheets [1] and [2], notice how the _DTR variant
> supports more functionality compared to the non-DTR variant:
> 
> 8.1.4 Instruction Set Table 3 (QPI Instructions)
> 8.1.5 Instruction Set Table 4 (DTR with SPI Instructions)
> 8.1.6 Instruction Set Table 5 (DTR with QPI Instructions)
> 
> So my counter-proposal would be to call it just "w25q128jw" , that's the
> lowest common denominator of the DTR and non-DTR variant. And then, use
> SFDP to determine whether the QPI/DTR extras are supported by the flash
> (and thus whether it is the DTR variant or not).
> 
> The only downside is, I only have the non-DTR variant, so I cannot dump
> the SFDP tables of the DTR variant and check whether it differs from the
> non-DTR variant. I would expect it does.
> 
> 2/ That looks wrong to me too. Look at [1] for the meaning of -xM/-xQ
> suffix of the flash, it has nothing to do with the DTR support. It just
> says what is the QE bit setting in SR2:
> 
> 11. ORDERING INFORMATION
> Q ... with QE = 1 (fixed) in Status register-2.
> N ... with QE = 1 (fixed) in Status register-2 & DRV=75%. Backward
> compatible to FW family.
> M ... with QE = 0 (programmable) in Status register-2. New device ID is
> used to identify FW family
> 
>> Looking at [3], I see that W25Q128JV and W25Q128FV use the same flash ID,
>> I wonder what's the difference between them. I made a proposal in linux on
>> how to handle flash collisions, a v5 will follow.
> 
> https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/
> 
> Density -> 128Mb
> 
> W25Q128FV
> 2.7V - 3.6V
> 104MHz <-----------
> SPI / QPI <--------
> 
> W25Q128JV
> 2.7V - 3.6V
> 133MHz <-----------
> SPI 4 I/O Fixed <--
> 
> Unlike the -JV, the -FV has
> Instruction Set Table 3 (QPI Instructions)
> 
> See my suggestion above, use the lowest common denominator flash (in
> this case JV) as a base and then use SFDP to determine whether extra
> features are available. Would that work ?

I don't think it will work because the -DTR and non-DTR have different flash
IDs.

ta
> 
>> Cheers,
>> ta
>> [1] https://www.winbond.com/resource-files/W25Q128JW_DTR%20RevF1%2005212021.pdf
>> [2] https://www.winbond.com/resource-files/W25Q128JW_RevG_07292021%20Plus.pdf
>> [3] https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/?__locale=en&selected=128Mb#Density
>>
>>> +                       SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>> +                       SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>>> +       },
>>>          {
>>>                  INFO("w25q256fw", 0xef6019, 0, 64 * 1024, 512,
>>>                          SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>> -- 
>>> 2.34.1
>>>
>>
> 
> [...]


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID
  2022-03-16  3:55     ` Tudor.Ambarus
@ 2022-03-16 10:07       ` Marek Vasut
  0 siblings, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2022-03-16 10:07 UTC (permalink / raw)
  To: Tudor.Ambarus, u-boot
  Cc: Horatiu.Vultur, jagan, simon.k.r.goldschmidt, sr, vigneshr, michael

On 3/16/22 04:55, Tudor.Ambarus@microchip.com wrote:

Hi,

>> [...]
>>
>>>>    drivers/mtd/spi/spi-nor-ids.c | 5 +++++
>>>>    1 file changed, 5 insertions(+)
>>>>
>>>> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
>>>> index b551ebd75ef..e2d09fc747d 100644
>>>> --- a/drivers/mtd/spi/spi-nor-ids.c
>>>> +++ b/drivers/mtd/spi/spi-nor-ids.c
>>>> @@ -345,6 +345,11 @@ const struct flash_info spi_nor_ids[] = {
>>>>                           SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>>>>                           SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>>>>           },
>>>> +       {
>>>> +               INFO("w25q128jw", 0xef8018, 0, 64 * 1024, 256,
>>>
>>> I'm glad I can discuss with you about the name of these winbond flashes,
>>> you can help with an advice. In linux we name this flash "w25q128jwm",
>>> but maybe we should change it.
>>
>> How did you arrive at "jwm" suffix ? Per both [1] and [2] "11.1 Valid
> 
> by checking sections "8.1.1 Manufacturer and Device Identification" and
> "11. ORDERING INFORMATION" from the datasheets.
> 
> Package type and temperature info doesn't matter for naming convention,
> but seems that the last suffix does: when M, QE = 0 (and it is programmable),
> when Q or N, Quad Enable is fixed and always enabled (QE = 1).

Oh, so that would be w25q128jw--m / w25q128jw--q then ?

>> Part Numbers and Top Side Marking" there is no "M" PACKAGE TYPE, so that
>> seems wrong.
> 
> mm, ok.

See above.

>>> Winbond names this flash "W25Q128JW_DTR", you can find the name on the
>>> top right of each page from [1].
>>
>> Per 8.1 the IDs are the same for both _DTR [1] and non-DTR [2] variants.
>>
>>> What I propose:
>>> 1/ according to [1] W25Q128JW_IM/JM is W25Q128JW_DTR, let's use "-dtr"
>>>      for the M flavor.
>>> 2/ according to [2] W25Q128JW has two flavors: W25Q128JW-IQ/JQ and
>>>      W25Q128JW-IM/JM*. Since W25Q128JW_IM/JM is called W25Q128JW_DTR, let's
>>>      use just "W25Q128JW" for W25Q128JW-IQ/JQ, as winbond uses too.
>>
>> 1/ That looks wrong to me. The W25Q128JW and W25Q128JW_DTR are two
>> different products of Winbond, see "Features" column in:
>>
>> https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/
>>
>> Density -> 128Mb
>>
>> W25Q128JW
>> 1.7V - 1.95V
>> 133MHz
>> SPI 4 I/O Fixed <------
>>
>> W25Q128JW_DTR
>> 1.7V - 1.95V
>> 133MHz
>> SPI / QPI / DTR <------
>>
>> Then, compare the datasheets [1] and [2], notice how the _DTR variant
>> supports more functionality compared to the non-DTR variant:
>>
>> 8.1.4 Instruction Set Table 3 (QPI Instructions)
>> 8.1.5 Instruction Set Table 4 (DTR with SPI Instructions)
>> 8.1.6 Instruction Set Table 5 (DTR with QPI Instructions)
>>
>> So my counter-proposal would be to call it just "w25q128jw" , that's the
>> lowest common denominator of the DTR and non-DTR variant. And then, use
>> SFDP to determine whether the QPI/DTR extras are supported by the flash
>> (and thus whether it is the DTR variant or not).
>>
>> The only downside is, I only have the non-DTR variant, so I cannot dump
>> the SFDP tables of the DTR variant and check whether it differs from the
>> non-DTR variant. I would expect it does.
>>
>> 2/ That looks wrong to me too. Look at [1] for the meaning of -xM/-xQ
>> suffix of the flash, it has nothing to do with the DTR support. It just
>> says what is the QE bit setting in SR2:
>>
>> 11. ORDERING INFORMATION
>> Q ... with QE = 1 (fixed) in Status register-2.
>> N ... with QE = 1 (fixed) in Status register-2 & DRV=75%. Backward
>> compatible to FW family.
>> M ... with QE = 0 (programmable) in Status register-2. New device ID is
>> used to identify FW family
>>
>>> Looking at [3], I see that W25Q128JV and W25Q128FV use the same flash ID,
>>> I wonder what's the difference between them. I made a proposal in linux on
>>> how to handle flash collisions, a v5 will follow.
>>
>> https://www.winbond.com/hq/product/code-storage-flash-memory/serial-nor-flash/
>>
>> Density -> 128Mb
>>
>> W25Q128FV
>> 2.7V - 3.6V
>> 104MHz <-----------
>> SPI / QPI <--------
>>
>> W25Q128JV
>> 2.7V - 3.6V
>> 133MHz <-----------
>> SPI 4 I/O Fixed <--
>>
>> Unlike the -JV, the -FV has
>> Instruction Set Table 3 (QPI Instructions)
>>
>> See my suggestion above, use the lowest common denominator flash (in
>> this case JV) as a base and then use SFDP to determine whether extra
>> features are available. Would that work ?
> 
> I don't think it will work because the -DTR and non-DTR have different flash
> IDs.

Be careful, I think you are conflating -xM/-xQ and _DTR and non-DTR 
variants of this W25Q128 flash. Look at [1] and [2] again, the ID 0x8018:

8.1.1 Manufacturer and Device Identification

Device ID       | (ID7 - ID0)        | (ID15 - ID0)
Instruction     | ABh, 90h, 92h, 94h | 9Fh

(non-DTR variant)
W25Q128JW-IQ/JQ | 17h                | 6018h
W25Q128JW-IM/JM*| 17h                | 8018h

-DTR variant:
W25Q128JW_IM/JM | 17h                | 8018h

So you should be able to tell the non-DTR variant of w25q128jw based 
purely on flash ID . You should also be able to tell whether the 0x8018 
flash is in fact DTR variant based on the presence of the extra DTR 
opcodes in SFDP. This:

if (id == 0x6018) {
   return W25Q128JWxxQ with QE=1
} else if (id == 0x8018) {
   if (SFDP indicates DTR commands present)
     return W25Q128JWxxM_DTR;
   else
     return W25Q128JWxxM (non-DTR);
}

Same thing applies to W25Q128JV/FV with id 0x4018:

Device ID            | (ID7 - ID0)        | (ID15 - ID0)
Instruction          | ABh, 90h, 92h, 94h | 9Fh
W25Q128FV (SPI Mode) | 17h                | 4018h
W25Q128FV (QPI Mode) | 17h                | 6018h

Device ID            | (ID7 - ID0)        | (ID15 - ID0)
Instruction          | ABh, 90h, 92h, 94h | 9Fh
W25Q128JV-IN/IQ/JQ   | 17h                | 4018h
W25Q128JV-IM*/JM*    | 17h                | 7018h

There the lowest common denominator is w25q128jv and we should be able 
to tell whether it is really w25q128fv by checking SFDP and see whether 
the QPI opcodes are supported.

[...]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID
  2022-03-09  3:18 [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID Marek Vasut
  2022-03-09  4:33 ` Tudor.Ambarus
@ 2022-04-07 19:20 ` Marek Vasut
  1 sibling, 0 replies; 6+ messages in thread
From: Marek Vasut @ 2022-04-07 19:20 UTC (permalink / raw)
  To: u-boot
  Cc: Horatiu Vultur, Jagan Teki, Simon Goldschmidt, Stefan Roese,
	Vignesh R, Tom Rini

On 3/9/22 04:18, Marek Vasut wrote:
> Add ID for Winbond W25Q128JW device. This is a 128 Mbit QSPI NOR.
> Tested on W25Q128JWPIM part.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Horatiu Vultur <horatiu.vultur@microchip.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
> Cc: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> Cc: Stefan Roese <sr@denx.de>
> Cc: Vignesh R <vigneshr@ti.com>
> ---
>   drivers/mtd/spi/spi-nor-ids.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/mtd/spi/spi-nor-ids.c b/drivers/mtd/spi/spi-nor-ids.c
> index b551ebd75ef..e2d09fc747d 100644
> --- a/drivers/mtd/spi/spi-nor-ids.c
> +++ b/drivers/mtd/spi/spi-nor-ids.c
> @@ -345,6 +345,11 @@ const struct flash_info spi_nor_ids[] = {
>   			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
>   			SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
>   	},
> +	{
> +		INFO("w25q128jw", 0xef8018, 0, 64 * 1024, 256,
> +			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |
> +			SPI_NOR_HAS_LOCK | SPI_NOR_HAS_TB)
> +	},
>   	{
>   		INFO("w25q256fw", 0xef6019, 0, 64 * 1024, 512,
>   			SECT_4K | SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ |

It would be good to apply this patch.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-04-07 19:20 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09  3:18 [PATCH] mtd: spi-nor-ids: Add Winbond W25Q128JW ID Marek Vasut
2022-03-09  4:33 ` Tudor.Ambarus
2022-03-16  1:29   ` Marek Vasut
2022-03-16  3:55     ` Tudor.Ambarus
2022-03-16 10:07       ` Marek Vasut
2022-04-07 19:20 ` Marek Vasut

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.