| Message ID | 1485481897-6368-1-git-send-email-nobuhiro.iwamatsu.kw@hitachi.com |
|---|---|
| State | Rejected |
| Headers | show |
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) }, >
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) }, >
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 ?
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
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
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 >
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" ?
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
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 ...
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) },
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(-)