[PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Nobuhiro Iwamatsu]

Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list
of supported devices. This chip has the same structure as the N25Q256A
but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
name to other n25q chip names.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
CC: Jagan Teki <jagan@openedev.com>
CC: Marek Vasut <marek.vasut@gmail.com>
---
 drivers/mtd/spi-nor/spi-nor.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
index da7cd69d4857..a2a6922e356f 100644
--- a/drivers/mtd/spi-nor/spi-nor.c
+++ b/drivers/mtd/spi-nor/spi-nor.c
@@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
 	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
-	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
+	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
+	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
 	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
 	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
-- 
2.11.0

Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Marek Vasut]

On 01/27/2017 02:51 AM, Nobuhiro Iwamatsu wrote:
> Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list
> of supported devices. This chip has the same structure as the N25Q256A
> but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
> name to other n25q chip names.
> 
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
> CC: Jagan Teki <jagan@openedev.com>
> CC: Marek Vasut <marek.vasut@gmail.com>

Acked-by: Marek Vasut <marek.vasut@gmail.com>

> ---
>  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index da7cd69d4857..a2a6922e356f 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
> -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
> +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
> +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
> 


-- 
Best regards,
Marek Vasut

Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Cyrille Pitchen]

Hi Nobuhiro,

Le 27/01/2017 à 02:51, Nobuhiro Iwamatsu a écrit :
> Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list
> of supported devices. This chip has the same structure as the N25Q256A
> but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
> name to other n25q chip names.
> 
> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
> CC: Jagan Teki <jagan@openedev.com>
> CC: Marek Vasut <marek.vasut@gmail.com>
> ---
>  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index da7cd69d4857..a2a6922e356f 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
> -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
> +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
> +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },

This patch changes the association "n25q256a" <-> 20 ba 19: "n25q256a"
would be now associated to 20 bb 19.
However some device trees use the "micron,n25q256a" as compatible string:
- arch/arm/boot/dts/imx6sx-sbd.dts: compatible = "micron,n25q256a",
"jedec,spi-nor";
- arch/arm/boot/dts/socfpga_arria5_socdk.dts: compatible = "n25q256a";
- arch/arm/boot/dts/socfpga_cyclone5_socrates.dts: compatible = "n25q256a";
- arch/arm/boot/dts/imx6ul-14x14-evk.dts: compatible = "micron,n25q256a";

If the actual JEDEC ID read from the Micron SPI memory of one of these
boards is 20 ba 19 and if you now associate the string "n25q256a" to the
different JEDEC ID 20 bb 19, then spi_nor_scan() is likely to display the
warning message during the boot:

dev_warn(dev, "found %s, expected %s\n", jinfo->name, info->name);


Displaying such a warning during the boot process would be an unwanted side
effect of this patch and users may complain or ask why such warning is now
displayed in the boot log.

You may create a new entry for the 20 bb 19 JEDEC ID but I don't think it
is totally safe to remove the old n25q256a entry.

Best regards,

Cyrille


>  	{ "n25q512a",    INFO(0x20bb20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "n25q512ax3",  INFO(0x20ba20, 0, 64 * 1024, 1024, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
>  	{ "n25q00",      INFO(0x20ba21, 0, 64 * 1024, 2048, SECT_4K | USE_FSR | SPI_NOR_QUAD_READ) },
> 

Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Marek Vasut]

On 01/30/2017 02:29 PM, Cyrille Pitchen wrote:
> Hi Nobuhiro,
> 
> Le 27/01/2017 à 02:51, Nobuhiro Iwamatsu a écrit :
>> Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list
>> of supported devices. This chip has the same structure as the N25Q256A
>> but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
>> name to other n25q chip names.
>>
>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
>> CC: Jagan Teki <jagan@openedev.com>
>> CC: Marek Vasut <marek.vasut@gmail.com>
>> ---
>>  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>> index da7cd69d4857..a2a6922e356f 100644
>> --- a/drivers/mtd/spi-nor/spi-nor.c
>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>> @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
>>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>> -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>> +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>> +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
> 
> This patch changes the association "n25q256a" <-> 20 ba 19: "n25q256a"
> would be now associated to 20 bb 19.
> However some device trees use the "micron,n25q256a" as compatible string:
> - arch/arm/boot/dts/imx6sx-sbd.dts: compatible = "micron,n25q256a",
> "jedec,spi-nor";
> - arch/arm/boot/dts/socfpga_arria5_socdk.dts: compatible = "n25q256a";
> - arch/arm/boot/dts/socfpga_cyclone5_socrates.dts: compatible = "n25q256a";
> - arch/arm/boot/dts/imx6ul-14x14-evk.dts: compatible = "micron,n25q256a";
> 
> If the actual JEDEC ID read from the Micron SPI memory of one of these
> boards is 20 ba 19 and if you now associate the string "n25q256a" to the
> different JEDEC ID 20 bb 19, then spi_nor_scan() is likely to display the
> warning message during the boot:
> 
> dev_warn(dev, "found %s, expected %s\n", jinfo->name, info->name);
> 
> 
> Displaying such a warning during the boot process would be an unwanted side
> effect of this patch and users may complain or ask why such warning is now
> displayed in the boot log.
> 
> You may create a new entry for the 20 bb 19 JEDEC ID but I don't think it
> is totally safe to remove the old n25q256a entry.

If the old DTs were lazy and are now broken, then I think the warning is
justified ?

-- 
Best regards,
Marek Vasut

Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Cyrille Pitchen]

Le 30/01/2017 à 21:38, Marek Vasut a écrit :
> On 01/30/2017 02:29 PM, Cyrille Pitchen wrote:
>> Hi Nobuhiro,
>>
>> Le 27/01/2017 à 02:51, Nobuhiro Iwamatsu a écrit :
>>> Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list
>>> of supported devices. This chip has the same structure as the N25Q256A
>>> but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
>>> name to other n25q chip names.
>>>
>>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
>>> CC: Jagan Teki <jagan@openedev.com>
>>> CC: Marek Vasut <marek.vasut@gmail.com>
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>> index da7cd69d4857..a2a6922e356f 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
>>>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>>>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>> -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>> +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>> +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>
>> This patch changes the association "n25q256a" <-> 20 ba 19: "n25q256a"
>> would be now associated to 20 bb 19.
>> However some device trees use the "micron,n25q256a" as compatible string:
>> - arch/arm/boot/dts/imx6sx-sbd.dts: compatible = "micron,n25q256a",
>> "jedec,spi-nor";
>> - arch/arm/boot/dts/socfpga_arria5_socdk.dts: compatible = "n25q256a";
>> - arch/arm/boot/dts/socfpga_cyclone5_socrates.dts: compatible = "n25q256a";
>> - arch/arm/boot/dts/imx6ul-14x14-evk.dts: compatible = "micron,n25q256a";
>>
>> If the actual JEDEC ID read from the Micron SPI memory of one of these
>> boards is 20 ba 19 and if you now associate the string "n25q256a" to the
>> different JEDEC ID 20 bb 19, then spi_nor_scan() is likely to display the
>> warning message during the boot:
>>
>> dev_warn(dev, "found %s, expected %s\n", jinfo->name, info->name);
>>
>>
>> Displaying such a warning during the boot process would be an unwanted side
>> effect of this patch and users may complain or ask why such warning is now
>> displayed in the boot log.
>>
>> You may create a new entry for the 20 bb 19 JEDEC ID but I don't think it
>> is totally safe to remove the old n25q256a entry.
> 
> If the old DTs were lazy and are now broken, then I think the warning is
> justified ?
> 

I'm reading a Micron datasheet for n25q256*:
1 - it states that the JEDEC ID is 20 BA 19.
2 - it explains the part number encoding

Device Type    N25Q: Serial NOR Flash Memory, Multiple Input/Output
                     (Single, Dual, Quad I/O, XIP)
Density        256: 256Mb
Technology     A: 65nm (this is the only value provided by the datasheet)
...

So this datasheet applies to Micron n25q256a memory and its JEDEC ID is 20
BA 19. Hence the current "n25q256a" entry in the spi_nor_ids[] array and
the DTs using this string for the compatible value are all correct and
should not be changed.

Then, I'm sure Nobuhiro is right too when he claims that the JEDEC ID of
his Micron n25q256a sample is 20 BB 19 (and not 20 BA 19). This just means
that Micron has produced different samples of n25q256a with different JEDEC
ID. However this is nothing new with Micron memories.

We should keep the current entry as is since it is perfectly correct but we
can add another entry with a different name for the JEDEC ID 20 BB 19.

If we look at the entries for the other Micron memories, we see for each
memory density there are 2 different ID: 20 BA xx and 20 BB xx.

Besides the "rule" 20 BA xx -> n25q__ , 20 BB xx -> n25q__a is not always
respected and is not relevant to the actual hardware. I guess this is just
a legacy naming scheme to provide 2 different names, one for the 20 BB xx
entry and the other for the 20 BA xx entry but from a hardware point of
view, I think this naming scheme doesn't make sense.

So don't worry about the accuracy of the name you will choose to add a new
entry. For instance let's take "n25q256" since it is available.


Best regards,

Cyrille

Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Marek Vasut]

On 01/31/2017 11:18 AM, Cyrille Pitchen wrote:
> Le 30/01/2017 à 21:38, Marek Vasut a écrit :
>> On 01/30/2017 02:29 PM, Cyrille Pitchen wrote:
>>> Hi Nobuhiro,
>>>
>>> Le 27/01/2017 à 02:51, Nobuhiro Iwamatsu a écrit :
>>>> Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list
>>>> of supported devices. This chip has the same structure as the N25Q256A
>>>> but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
>>>> name to other n25q chip names.
>>>>
>>>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
>>>> CC: Jagan Teki <jagan@openedev.com>
>>>> CC: Marek Vasut <marek.vasut@gmail.com>
>>>> ---
>>>>  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>> index da7cd69d4857..a2a6922e356f 100644
>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>> @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
>>>>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>>> -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>> +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>> +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>
>>> This patch changes the association "n25q256a" <-> 20 ba 19: "n25q256a"
>>> would be now associated to 20 bb 19.
>>> However some device trees use the "micron,n25q256a" as compatible string:
>>> - arch/arm/boot/dts/imx6sx-sbd.dts: compatible = "micron,n25q256a",
>>> "jedec,spi-nor";
>>> - arch/arm/boot/dts/socfpga_arria5_socdk.dts: compatible = "n25q256a";
>>> - arch/arm/boot/dts/socfpga_cyclone5_socrates.dts: compatible = "n25q256a";
>>> - arch/arm/boot/dts/imx6ul-14x14-evk.dts: compatible = "micron,n25q256a";
>>>
>>> If the actual JEDEC ID read from the Micron SPI memory of one of these
>>> boards is 20 ba 19 and if you now associate the string "n25q256a" to the
>>> different JEDEC ID 20 bb 19, then spi_nor_scan() is likely to display the
>>> warning message during the boot:
>>>
>>> dev_warn(dev, "found %s, expected %s\n", jinfo->name, info->name);
>>>
>>>
>>> Displaying such a warning during the boot process would be an unwanted side
>>> effect of this patch and users may complain or ask why such warning is now
>>> displayed in the boot log.
>>>
>>> You may create a new entry for the 20 bb 19 JEDEC ID but I don't think it
>>> is totally safe to remove the old n25q256a entry.
>>
>> If the old DTs were lazy and are now broken, then I think the warning is
>> justified ?
>>
> 
> I'm reading a Micron datasheet for n25q256*:
> 1 - it states that the JEDEC ID is 20 BA 19.
> 2 - it explains the part number encoding
> 
> Device Type    N25Q: Serial NOR Flash Memory, Multiple Input/Output
>                      (Single, Dual, Quad I/O, XIP)
> Density        256: 256Mb
> Technology     A: 65nm (this is the only value provided by the datasheet)
> ...
> 
> So this datasheet applies to Micron n25q256a memory and its JEDEC ID is 20
> BA 19. Hence the current "n25q256a" entry in the spi_nor_ids[] array and
> the DTs using this string for the compatible value are all correct and
> should not be changed.
> 
> Then, I'm sure Nobuhiro is right too when he claims that the JEDEC ID of
> his Micron n25q256a sample is 20 BB 19 (and not 20 BA 19). This just means
> that Micron has produced different samples of n25q256a with different JEDEC
> ID. However this is nothing new with Micron memories.

Isn't the BA a 3V3 part and the BB a 1V8 part ? I recall something like
that.

> We should keep the current entry as is since it is perfectly correct but we
> can add another entry with a different name for the JEDEC ID 20 BB 19.
> 
> If we look at the entries for the other Micron memories, we see for each
> memory density there are 2 different ID: 20 BA xx and 20 BB xx.
> 
> Besides the "rule" 20 BA xx -> n25q__ , 20 BB xx -> n25q__a is not always
> respected and is not relevant to the actual hardware. I guess this is just
> a legacy naming scheme to provide 2 different names, one for the 20 BB xx
> entry and the other for the 20 BA xx entry but from a hardware point of
> view, I think this naming scheme doesn't make sense.

The micron ID/naming scheme does not make sense, it's complete chaos,
that is no news.

> So don't worry about the accuracy of the name you will choose to add a new
> entry. For instance let's take "n25q256" since it is available.

Or we can add "n25q256ax3" ?

-- 
Best regards,
Marek Vasut

Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Cyrille Pitchen]

Le 04/02/2017 à 22:32, Marek Vasut a écrit :
> On 01/31/2017 11:18 AM, Cyrille Pitchen wrote:
>> Le 30/01/2017 à 21:38, Marek Vasut a écrit :
>>> On 01/30/2017 02:29 PM, Cyrille Pitchen wrote:
>>>> Hi Nobuhiro,
>>>>
>>>> Le 27/01/2017 à 02:51, Nobuhiro Iwamatsu a écrit :
>>>>> Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list
>>>>> of supported devices. This chip has the same structure as the N25Q256A
>>>>> but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
>>>>> name to other n25q chip names.
>>>>>
>>>>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
>>>>> CC: Jagan Teki <jagan@openedev.com>
>>>>> CC: Marek Vasut <marek.vasut@gmail.com>
>>>>> ---
>>>>>  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>>> index da7cd69d4857..a2a6922e356f 100644
>>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>>> @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
>>>>>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>> -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>> +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>> +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>
>>>> This patch changes the association "n25q256a" <-> 20 ba 19: "n25q256a"
>>>> would be now associated to 20 bb 19.
>>>> However some device trees use the "micron,n25q256a" as compatible string:
>>>> - arch/arm/boot/dts/imx6sx-sbd.dts: compatible = "micron,n25q256a",
>>>> "jedec,spi-nor";
>>>> - arch/arm/boot/dts/socfpga_arria5_socdk.dts: compatible = "n25q256a";
>>>> - arch/arm/boot/dts/socfpga_cyclone5_socrates.dts: compatible = "n25q256a";
>>>> - arch/arm/boot/dts/imx6ul-14x14-evk.dts: compatible = "micron,n25q256a";
>>>>
>>>> If the actual JEDEC ID read from the Micron SPI memory of one of these
>>>> boards is 20 ba 19 and if you now associate the string "n25q256a" to the
>>>> different JEDEC ID 20 bb 19, then spi_nor_scan() is likely to display the
>>>> warning message during the boot:
>>>>
>>>> dev_warn(dev, "found %s, expected %s\n", jinfo->name, info->name);
>>>>
>>>>
>>>> Displaying such a warning during the boot process would be an unwanted side
>>>> effect of this patch and users may complain or ask why such warning is now
>>>> displayed in the boot log.
>>>>
>>>> You may create a new entry for the 20 bb 19 JEDEC ID but I don't think it
>>>> is totally safe to remove the old n25q256a entry.
>>>
>>> If the old DTs were lazy and are now broken, then I think the warning is
>>> justified ?
>>>
>>
>> I'm reading a Micron datasheet for n25q256*:
>> 1 - it states that the JEDEC ID is 20 BA 19.
>> 2 - it explains the part number encoding
>>
>> Device Type    N25Q: Serial NOR Flash Memory, Multiple Input/Output
>>                      (Single, Dual, Quad I/O, XIP)
>> Density        256: 256Mb
>> Technology     A: 65nm (this is the only value provided by the datasheet)
>> ...
>>
>> So this datasheet applies to Micron n25q256a memory and its JEDEC ID is 20
>> BA 19. Hence the current "n25q256a" entry in the spi_nor_ids[] array and
>> the DTs using this string for the compatible value are all correct and
>> should not be changed.
>>
>> Then, I'm sure Nobuhiro is right too when he claims that the JEDEC ID of
>> his Micron n25q256a sample is 20 BB 19 (and not 20 BA 19). This just means
>> that Micron has produced different samples of n25q256a with different JEDEC
>> ID. However this is nothing new with Micron memories.
> 
> Isn't the BA a 3V3 part and the BB a 1V8 part ? I recall something like
> that.
>

I've just looked at 2 Micron's datasheets, the first one for 3V3 and the
other for some 1V8 memory and I guess you're right.

About the part numbers, voltage and JEDEC ID:
N25Q*Ax1 -> 1V8, 20 BB __
N25Q*Ax3 -> 3V3, 20 BA __

>> We should keep the current entry as is since it is perfectly correct but we
>> can add another entry with a different name for the JEDEC ID 20 BB 19.
>>
>> If we look at the entries for the other Micron memories, we see for each
>> memory density there are 2 different ID: 20 BA xx and 20 BB xx.
>>
>> Besides the "rule" 20 BA xx -> n25q__ , 20 BB xx -> n25q__a is not always
>> respected and is not relevant to the actual hardware. I guess this is just
>> a legacy naming scheme to provide 2 different names, one for the 20 BB xx
>> entry and the other for the 20 BA xx entry but from a hardware point of
>> view, I think this naming scheme doesn't make sense.
> 
> The micron ID/naming scheme does not make sense, it's complete chaos,
> that is no news.
> 
>> So don't worry about the accuracy of the name you will choose to add a new
>> entry. For instance let's take "n25q256" since it is available.
> 
> Or we can add "n25q256ax3" ?
> 

Indeed, it sounds like a good idea, so I vote for it :)

Just a small comment: the JEDEC ID 20 BA 19 is already associated to the
string "n25q256a" and we have to keep this string for DT backward
compatibility purpose. However the JEDEC ID 20 BB 19 is not handled yet so
we can associate it to the string "n25q256ax1".

So if we all agree, let the "n25q256a" entry as is and just add a new
"n25q256ax1" entry for the JEDEC ID 20 BB 19.

In the future, for new Micron memories we could use the patterns "n25q*ax1"
for 1V8 and "n25q*ax3" for 3V3 memories.

Best regards,

Cyrille

Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Marek Vasut]

On 02/06/2017 06:53 PM, Cyrille Pitchen wrote:
> Le 04/02/2017 à 22:32, Marek Vasut a écrit :
>> On 01/31/2017 11:18 AM, Cyrille Pitchen wrote:
>>> Le 30/01/2017 à 21:38, Marek Vasut a écrit :
>>>> On 01/30/2017 02:29 PM, Cyrille Pitchen wrote:
>>>>> Hi Nobuhiro,
>>>>>
>>>>> Le 27/01/2017 à 02:51, Nobuhiro Iwamatsu a écrit :
>>>>>> Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list
>>>>>> of supported devices. This chip has the same structure as the N25Q256A
>>>>>> but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
>>>>>> name to other n25q chip names.
>>>>>>
>>>>>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
>>>>>> CC: Jagan Teki <jagan@openedev.com>
>>>>>> CC: Marek Vasut <marek.vasut@gmail.com>
>>>>>> ---
>>>>>>  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
>>>>>> index da7cd69d4857..a2a6922e356f 100644
>>>>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>>>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>>>>> @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
>>>>>>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>>>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>>>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>>> -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>>> +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>>> +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K | SPI_NOR_QUAD_READ) },
>>>>>
>>>>> This patch changes the association "n25q256a" <-> 20 ba 19: "n25q256a"
>>>>> would be now associated to 20 bb 19.
>>>>> However some device trees use the "micron,n25q256a" as compatible string:
>>>>> - arch/arm/boot/dts/imx6sx-sbd.dts: compatible = "micron,n25q256a",
>>>>> "jedec,spi-nor";
>>>>> - arch/arm/boot/dts/socfpga_arria5_socdk.dts: compatible = "n25q256a";
>>>>> - arch/arm/boot/dts/socfpga_cyclone5_socrates.dts: compatible = "n25q256a";
>>>>> - arch/arm/boot/dts/imx6ul-14x14-evk.dts: compatible = "micron,n25q256a";
>>>>>
>>>>> If the actual JEDEC ID read from the Micron SPI memory of one of these
>>>>> boards is 20 ba 19 and if you now associate the string "n25q256a" to the
>>>>> different JEDEC ID 20 bb 19, then spi_nor_scan() is likely to display the
>>>>> warning message during the boot:
>>>>>
>>>>> dev_warn(dev, "found %s, expected %s\n", jinfo->name, info->name);
>>>>>
>>>>>
>>>>> Displaying such a warning during the boot process would be an unwanted side
>>>>> effect of this patch and users may complain or ask why such warning is now
>>>>> displayed in the boot log.
>>>>>
>>>>> You may create a new entry for the 20 bb 19 JEDEC ID but I don't think it
>>>>> is totally safe to remove the old n25q256a entry.
>>>>
>>>> If the old DTs were lazy and are now broken, then I think the warning is
>>>> justified ?
>>>>
>>>
>>> I'm reading a Micron datasheet for n25q256*:
>>> 1 - it states that the JEDEC ID is 20 BA 19.
>>> 2 - it explains the part number encoding
>>>
>>> Device Type    N25Q: Serial NOR Flash Memory, Multiple Input/Output
>>>                      (Single, Dual, Quad I/O, XIP)
>>> Density        256: 256Mb
>>> Technology     A: 65nm (this is the only value provided by the datasheet)
>>> ...
>>>
>>> So this datasheet applies to Micron n25q256a memory and its JEDEC ID is 20
>>> BA 19. Hence the current "n25q256a" entry in the spi_nor_ids[] array and
>>> the DTs using this string for the compatible value are all correct and
>>> should not be changed.
>>>
>>> Then, I'm sure Nobuhiro is right too when he claims that the JEDEC ID of
>>> his Micron n25q256a sample is 20 BB 19 (and not 20 BA 19). This just means
>>> that Micron has produced different samples of n25q256a with different JEDEC
>>> ID. However this is nothing new with Micron memories.
>>
>> Isn't the BA a 3V3 part and the BB a 1V8 part ? I recall something like
>> that.
>>
> 
> I've just looked at 2 Micron's datasheets, the first one for 3V3 and the
> other for some 1V8 memory and I guess you're right.

Good !

> About the part numbers, voltage and JEDEC ID:
> N25Q*Ax1 -> 1V8, 20 BB __
> N25Q*Ax3 -> 3V3, 20 BA __
> 
>>> We should keep the current entry as is since it is perfectly correct but we
>>> can add another entry with a different name for the JEDEC ID 20 BB 19.
>>>
>>> If we look at the entries for the other Micron memories, we see for each
>>> memory density there are 2 different ID: 20 BA xx and 20 BB xx.
>>>
>>> Besides the "rule" 20 BA xx -> n25q__ , 20 BB xx -> n25q__a is not always
>>> respected and is not relevant to the actual hardware. I guess this is just
>>> a legacy naming scheme to provide 2 different names, one for the 20 BB xx
>>> entry and the other for the 20 BA xx entry but from a hardware point of
>>> view, I think this naming scheme doesn't make sense.
>>
>> The micron ID/naming scheme does not make sense, it's complete chaos,
>> that is no news.
>>
>>> So don't worry about the accuracy of the name you will choose to add a new
>>> entry. For instance let's take "n25q256" since it is available.
>>
>> Or we can add "n25q256ax3" ?
>>
> 
> Indeed, it sounds like a good idea, so I vote for it :)

I like where this is going, hehehe.

> Just a small comment: the JEDEC ID 20 BA 19 is already associated to the
> string "n25q256a" and we have to keep this string for DT backward
> compatibility purpose. However the JEDEC ID 20 BB 19 is not handled yet so
> we can associate it to the string "n25q256ax1".
> 
> So if we all agree, let the "n25q256a" entry as is and just add a new
> "n25q256ax1" entry for the JEDEC ID 20 BB 19.

Fine

> In the future, for new Micron memories we could use the patterns "n25q*ax1"
> for 1V8 and "n25q*ax3" for 3V3 memories.

Indeed ... At least until Micron damagement has another big drinking
party and decides to re-randomize the JEDEC ID assignments ...

-- 
Best regards,
Marek Vasut

RE: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[岩松信洋 / IWAMATSU，NOBUHIRO]

Hi,

Thanks for your review.

> -----Original Message-----
> From: Cyrille Pitchen [mailto:cyrille.pitchen@atmel.com]
> Sent: Monday, January 30, 2017 10:29 PM
> To: 岩松信洋 / IWAMATSU，NOBUHIRO; linux-mtd@lists.infradead.org
> Cc: Marek Vasut; Jagan Teki
> Subject: Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A
> 
> Hi Nobuhiro,
> 
> Le 27/01/2017 à 02:51, Nobuhiro Iwamatsu a écrit :
> > Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list of
> > supported devices. This chip has the same structure as the N25Q256A
> > but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
> > name to other n25q chip names.
> >
> > Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
> > CC: Jagan Teki <jagan@openedev.com>
> > CC: Marek Vasut <marek.vasut@gmail.com>
> > ---
> >  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/mtd/spi-nor/spi-nor.c
> > b/drivers/mtd/spi-nor/spi-nor.c index da7cd69d4857..a2a6922e356f
> > 100644
> > --- a/drivers/mtd/spi-nor/spi-nor.c
> > +++ b/drivers/mtd/spi-nor/spi-nor.c
> > @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
> >  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K |
> SPI_NOR_QUAD_READ) },
> >  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K |
> SPI_NOR_QUAD_READ) },
> >  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K |
> SPI_NOR_QUAD_READ) },
> > -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
> SPI_NOR_QUAD_READ) },
> > +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
> SPI_NOR_QUAD_READ) },
> > +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K |
> SPI_NOR_QUAD_READ) },
> 
> This patch changes the association "n25q256a" <-> 20 ba 19: "n25q256a"
> would be now associated to 20 bb 19.
> However some device trees use the "micron,n25q256a" as compatible string:
> - arch/arm/boot/dts/imx6sx-sbd.dts: compatible = "micron,n25q256a",
> "jedec,spi-nor";
> - arch/arm/boot/dts/socfpga_arria5_socdk.dts: compatible = "n25q256a";
> - arch/arm/boot/dts/socfpga_cyclone5_socrates.dts: compatible =
> "n25q256a";
> - arch/arm/boot/dts/imx6ul-14x14-evk.dts: compatible =
> "micron,n25q256a";
> 
> If the actual JEDEC ID read from the Micron SPI memory of one of these boards
> is 20 ba 19 and if you now associate the string "n25q256a" to the different
> JEDEC ID 20 bb 19, then spi_nor_scan() is likely to display the warning
> message during the boot:
> 
> dev_warn(dev, "found %s, expected %s\n", jinfo->name, info->name);
> 
> 
> Displaying such a warning during the boot process would be an unwanted side
> effect of this patch and users may complain or ask why such warning is now
> displayed in the boot log.
> 

I see.

> You may create a new entry for the 20 bb 19 JEDEC ID but I don't think it
> is totally safe to remove the old n25q256a entry.

We can not solve this problem if we use an old DTB, but I think that this
problem can be solved by updating DTS.
In order not to output this warning, I think that it is necessary to change the
DTS of the target boards.
Is this right?

> 
> Best regards,
> 
> Cyrille

Best regards,
  Nobuhiro

Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A

[Cyrille Pitchen]

Le 31/01/2017 à 05:51, 岩松信洋 / IWAMATSU，NOBUHIRO a écrit :
> Hi,
> 
> Thanks for your review.
> 
>> -----Original Message-----
>> From: Cyrille Pitchen [mailto:cyrille.pitchen@atmel.com]
>> Sent: Monday, January 30, 2017 10:29 PM
>> To: 岩松信洋 / IWAMATSU，NOBUHIRO; linux-mtd@lists.infradead.org
>> Cc: Marek Vasut; Jagan Teki
>> Subject: Re: [PATCH v2] mtd: spi-nor: Add support for N25Q256A13 as N25Q256A
>>
>> Hi Nobuhiro,
>>
>> Le 27/01/2017 à 02:51, Nobuhiro Iwamatsu a écrit :
>>> Add new Micron N25Q256A (N25Q256A13) 256Mbit NOR Flash in the list of
>>> supported devices. This chip has the same structure as the N25Q256A
>>> but ID is different. And this fixes N25Q256A to N25Q256 to fit chip
>>> name to other n25q chip names.
>>>
>>> Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.kw@hitachi.com>
>>> CC: Jagan Teki <jagan@openedev.com>
>>> CC: Marek Vasut <marek.vasut@gmail.com>
>>> ---
>>>  drivers/mtd/spi-nor/spi-nor.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/spi-nor.c
>>> b/drivers/mtd/spi-nor/spi-nor.c index da7cd69d4857..a2a6922e356f
>>> 100644
>>> --- a/drivers/mtd/spi-nor/spi-nor.c
>>> +++ b/drivers/mtd/spi-nor/spi-nor.c
>>> @@ -887,7 +887,8 @@ static const struct flash_info spi_nor_ids[] = {
>>>  	{ "n25q064a",    INFO(0x20bb17, 0, 64 * 1024,  128, SECT_4K |
>> SPI_NOR_QUAD_READ) },
>>>  	{ "n25q128a11",  INFO(0x20bb18, 0, 64 * 1024,  256, SECT_4K |
>> SPI_NOR_QUAD_READ) },
>>>  	{ "n25q128a13",  INFO(0x20ba18, 0, 64 * 1024,  256, SECT_4K |
>> SPI_NOR_QUAD_READ) },
>>> -	{ "n25q256a",    INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
>> SPI_NOR_QUAD_READ) },
>>> +	{ "n25q256",     INFO(0x20ba19, 0, 64 * 1024,  512, SECT_4K |
>> SPI_NOR_QUAD_READ) },
>>> +	{ "n25q256a",    INFO(0x20bb19, 0, 64 * 1024,  512, SECT_4K |
>> SPI_NOR_QUAD_READ) },
>>
>> This patch changes the association "n25q256a" <-> 20 ba 19: "n25q256a"
>> would be now associated to 20 bb 19.
>> However some device trees use the "micron,n25q256a" as compatible string:
>> - arch/arm/boot/dts/imx6sx-sbd.dts: compatible = "micron,n25q256a",
>> "jedec,spi-nor";
>> - arch/arm/boot/dts/socfpga_arria5_socdk.dts: compatible = "n25q256a";
>> - arch/arm/boot/dts/socfpga_cyclone5_socrates.dts: compatible =
>> "n25q256a";
>> - arch/arm/boot/dts/imx6ul-14x14-evk.dts: compatible =
>> "micron,n25q256a";
>>
>> If the actual JEDEC ID read from the Micron SPI memory of one of these boards
>> is 20 ba 19 and if you now associate the string "n25q256a" to the different
>> JEDEC ID 20 bb 19, then spi_nor_scan() is likely to display the warning
>> message during the boot:
>>
>> dev_warn(dev, "found %s, expected %s\n", jinfo->name, info->name);
>>
>>
>> Displaying such a warning during the boot process would be an unwanted side
>> effect of this patch and users may complain or ask why such warning is now
>> displayed in the boot log.
>>
> 
> I see.
> 
>> You may create a new entry for the 20 bb 19 JEDEC ID but I don't think it
>> is totally safe to remove the old n25q256a entry.
> 
> We can not solve this problem if we use an old DTB, but I think that this
> problem can be solved by updating DTS.
> In order not to output this warning, I think that it is necessary to change the
> DTS of the target boards.
> Is this right?
>

As I explain in the mail I've just sent, those DTS are actually correct so
there is no valid reason to change them.

Besides, when possible, "the usage of the Device Tree on ARM also came with
a requirement: it should be a stable ABI. An old Device Tree for a given
hardware platform must continue to work with newer kernel versions." [1]

This is not always so easy to respect this rule but in our case we can,
especially since the device trees we are talking about are actually right.


[1]
http://free-electrons.com/pub/conferences/2015/elc/petazzoni-dt-as-stable-abi-fairy-tale/petazzoni-dt-as-stable-abi-fairy-tale.pdf


>>
>> Best regards,
>>
>> Cyrille
> 
> Best regards,
>   Nobuhiro
>