[PATCH] mtd: nand_ids: Add device parameters for Toshiba's TC58NVG1S3ETAI0 NAND EEPROM

[PATCH] mtd: nand_ids: Add device parameters for Toshiba's TC58NVG1S3ETAI0 NAND EEPROM

[Stefan Roese]

Toshiba's TC58NVG1S3ETAI0 is a single 3.3V 2 Gbit (2,214,592,512 bits) NAND
Electrically Erasable and Programmable Read-Only Memory (NAND E2PROM)
organized as (2048 + 64) bytes x 64 pages x 2048 blocks.

Signed-off-by: Stefan Roese <sr@denx.de>
Cc: Pekon Gupta <pekon@ti.com>
Cc: Brian Norris <computersforpeace@gmail.com>
---
 drivers/mtd/nand/nand_ids.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
index a87b0a3..cb4ece3 100644
--- a/drivers/mtd/nand/nand_ids.c
+++ b/drivers/mtd/nand/nand_ids.c
@@ -31,6 +31,9 @@ struct nand_flash_dev nand_flash_ids[] = {
 	 * listed by full ID. We list them first so that we can easily identify
 	 * the most specific match.
 	 */
+	{"TC58NVG1S3ETAI0 2G 3.3V 8-bit",
+		{ .id = {0x98, 0xda, 0x90, 0x15, 0x76, 0x00, 0x00, 0x00} },
+		  SZ_2K, SZ_256, SZ_128K, 0, 5, 64, NAND_ECC_INFO(1, SZ_512) },
 	{"TC58NVG2S0F 4G 3.3V 8-bit",
 		{ .id = {0x98, 0xdc, 0x90, 0x26, 0x76, 0x15, 0x01, 0x08} },
 		  SZ_4K, SZ_512, SZ_256K, 0, 8, 224, NAND_ECC_INFO(4, SZ_512) },
-- 
1.8.5.2

Re: [PATCH] mtd: nand_ids: Add device parameters for Toshiba's TC58NVG1S3ETAI0 NAND EEPROM

[Brian Norris]

On Tue, Jan 07, 2014 at 11:04:43AM +0100, Stefan Roese wrote:
> Toshiba's TC58NVG1S3ETAI0 is a single 3.3V 2 Gbit (2,214,592,512 bits) NAND
> Electrically Erasable and Programmable Read-Only Memory (NAND E2PROM)
> organized as (2048 + 64) bytes x 64 pages x 2048 blocks.
> 
> Signed-off-by: Stefan Roese <sr@denx.de>
> Cc: Pekon Gupta <pekon@ti.com>
> Cc: Brian Norris <computersforpeace@gmail.com>
> ---
>  drivers/mtd/nand/nand_ids.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/mtd/nand/nand_ids.c b/drivers/mtd/nand/nand_ids.c
> index a87b0a3..cb4ece3 100644
> --- a/drivers/mtd/nand/nand_ids.c
> +++ b/drivers/mtd/nand/nand_ids.c
> @@ -31,6 +31,9 @@ struct nand_flash_dev nand_flash_ids[] = {
>  	 * listed by full ID. We list them first so that we can easily identify
>  	 * the most specific match.
>  	 */
> +	{"TC58NVG1S3ETAI0 2G 3.3V 8-bit",
> +		{ .id = {0x98, 0xda, 0x90, 0x15, 0x76, 0x00, 0x00, 0x00} },
> +		  SZ_2K, SZ_256, SZ_128K, 0, 5, 64, NAND_ECC_INFO(1, SZ_512) },

I have to NAK this in its current form, for two reasons:

1) This is not a specific enough ID string. It conflicts with another
NAND which was recently supported.

2) This patch may not really be needed at all. The "full ID" listing is
only for NAND which can't be detected via other means (e.g., ONFI, or
traditional extended ID parsing). Is this NAND mis-detected in Linus'
current tree? As I read the code, it should correctly identify this
NAND's device size, page size, OOB size, etc.

See this commit for reference:

commit 60c6738245612df9499b340c15edf48b8f3e7981
Author: Brian Norris <computersforpeace@gmail.com>
Date:   Tue Jun 25 13:17:59 2013 -0700

    mtd: nand: detect OOB size for Toshiba 24nm raw SLC

   https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=60c6738245612df9499b340c15edf48b8f3e7981

It adds support for a class of Toshiba 24nm NAND, including this one:

    24nm SLC 2Gbit TC58NVG1S3HTA00
    ID: 0x98 0xda 0x90 0x15 0x76 0x16 0x08 0x00

Please reconcile your patch with the comments found there.

>  	{"TC58NVG2S0F 4G 3.3V 8-bit",
>  		{ .id = {0x98, 0xdc, 0x90, 0x26, 0x76, 0x15, 0x01, 0x08} },
>  		  SZ_4K, SZ_512, SZ_256K, 0, 8, 224, NAND_ECC_INFO(4, SZ_512) },

Thanks,
Brian

RE: [PATCH] mtd: nand_ids: Add device parameters for Toshiba's TC58NVG1S3ETAI0 NAND EEPROM

[Gupta, Pekon]

Hi Brian,

>From: Brian Norris [mailto:computersforpeace@gmail.com]
>>On Tue, Jan 07, 2014 at 11:04:43AM +0100, Stefan Roese wrote:
>> Toshiba's TC58NVG1S3ETAI0 is a single 3.3V 2 Gbit (2,214,592,512 bits) NAND
>> Electrically Erasable and Programmable Read-Only Memory (NAND E2PROM)
>> organized as (2048 + 64) bytes x 64 pages x 2048 blocks.
>>
>> Signed-off-by: Stefan Roese <sr@denx.de>
[...]
>> @@ -31,6 +31,9 @@ struct nand_flash_dev nand_flash_ids[] = {
>>  	 * listed by full ID. We list them first so that we can easily identify
>>  	 * the most specific match.
>>  	 */
>> +	{"TC58NVG1S3ETAI0 2G 3.3V 8-bit",
>> +		{ .id = {0x98, 0xda, 0x90, 0x15, 0x76, 0x00, 0x00, 0x00} },
>> +		  SZ_2K, SZ_256, SZ_128K, 0, 5, 64, NAND_ECC_INFO(1, SZ_512) },
>
>I have to NAK this in its current form, for two reasons:
>
>1) This is not a specific enough ID string. It conflicts with another
>NAND which was recently supported.
>
>2) This patch may not really be needed at all. The "full ID" listing is
>only for NAND which can't be detected via other means (e.g., ONFI, or
>traditional extended ID parsing). Is this NAND mis-detected in Linus'
>current tree? As I read the code, it should correctly identify this
>NAND's device size, page size, OOB size, etc.
>
>See this commit for reference:
>
>commit 60c6738245612df9499b340c15edf48b8f3e7981
>Author: Brian Norris <computersforpeace@gmail.com>
>Date:   Tue Jun 25 13:17:59 2013 -0700
>
>    mtd: nand: detect OOB size for Toshiba 24nm raw SLC
>
>   https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=60c6738245612df9499b340c15edf48b8f3e7981
>
>It adds support for a class of Toshiba 24nm NAND, including this one:
>
>    24nm SLC 2Gbit TC58NVG1S3HTA00
>    ID: 0x98 0xda 0x90 0x15 0x76 0x16 0x08 0x00
>
>Please reconcile your patch with the comments found there.
>

Yes, I think this is a different technology part..
because though the page-size = 2048B is same, but OOB-size = 64B.
However, with your OOB-size is detected as 128B.

Best part is the datasheet of this part does not mention anything about the 6th byte :-).
So, I think we can first  read 6th-byte (using READID command), and figure out difference
between your part and this one, then based on it append the OOB-calculation to you patch. Ok ?

ID byte 5, bit[7]: 
	1 -> BENAND 
	0 -> raw SLC   <<----- SLC NAND

ID byte 6, bits[2:0]:   <<----- 6th byte has no mention in datasheet
	100b -> 43nm 
	 101b -> 32nm
	 110b -> 24nm
	 111b -> Reserved

But sincerely, why is Toshiba doing all this ?
It would hit them only, as their devices will lack support on mainline kernels and u-boots.
(Or their customers would have to carry custom patches all along).
I tried convincing Toshiba to get their parts ONFI compliant, but they were too reluctant.


with regards, pekon

Re: [PATCH] mtd: nand_ids: Add device parameters for Toshiba's TC58NVG1S3ETAI0 NAND EEPROM

[Huang Shijie]

On Tue, Jan 21, 2014 at 07:33:04AM +0000, Gupta, Pekon wrote:
> Hi Brian,
> 
> But sincerely, why is Toshiba doing all this ?
> It would hit them only, as their devices will lack support on mainline kernels and u-boots.
> (Or their customers would have to carry custom patches all along).
> I tried convincing Toshiba to get their parts ONFI compliant, but they were too reluctant.
In actullay, Toshiba has began to support the JEDEC standard now.

see my patch set.

thanks
Huang Shijie

RE: [PATCH] mtd: nand_ids: Add device parameters for Toshiba's TC58NVG1S3ETAI0 NAND EEPROM

[Gupta, Pekon]

Hi Shijie,

>From: Huang Shijie [mailto:shijie8@gmail.com]
>>On Tue, Jan 21, 2014 at 07:33:04AM +0000, Gupta, Pekon wrote:
[...]
>> But sincerely, why is Toshiba doing all this ?
>> It would hit them only, as their devices will lack support on mainline kernels and u-boots.
>> (Or their customers would have to carry custom patches all along).
>> I tried convincing Toshiba to get their parts ONFI compliant, but they were too reluctant.
>In actullay, Toshiba has began to support the JEDEC standard now.
>
>see my patch set.
>
Hope you are referring to patch-set given at [1]
Thanks for pointing this out.. I missed it.

Yes, but your patch comment also mentions same issue with Toshiba NANDs 
"	 Tested with Toshiba TH58TEG7DDKTA20(16K + 1280).
	(Unfortuately, this ECC info of its JEDEC parameter page is zero,
	 i hope toshiba can fix it in future.)"

So, I think the basic problem remains the same.. 
However, I let Stefan comment about it more, as his NAND part is different
from the one you tested.
 
[1] http://lists.infradead.org/pipermail/linux-mtd/2013-December/051266.html

with regards, pekon

Re: [PATCH] mtd: nand_ids: Add device parameters for Toshiba's TC58NVG1S3ETAI0 NAND EEPROM

[Huang Shijie]

On Wed, Jan 22, 2014 at 05:56:42AM +0000, Gupta, Pekon wrote:
> Hi Shijie,
> 
> >From: Huang Shijie [mailto:shijie8@gmail.com]
> >>On Tue, Jan 21, 2014 at 07:33:04AM +0000, Gupta, Pekon wrote:
> [...]
> >> But sincerely, why is Toshiba doing all this ?
> >> It would hit them only, as their devices will lack support on mainline kernels and u-boots.
> >> (Or their customers would have to carry custom patches all along).
> >> I tried convincing Toshiba to get their parts ONFI compliant, but they were too reluctant.
> >In actullay, Toshiba has began to support the JEDEC standard now.
> >
> >see my patch set.
> >
> Hope you are referring to patch-set given at [1]
> Thanks for pointing this out.. I missed it.
> 
> Yes, but your patch comment also mentions same issue with Toshiba NANDs 
> "	 Tested with Toshiba TH58TEG7DDKTA20(16K + 1280).
> 	(Unfortuately, this ECC info of its JEDEC parameter page is zero,
> 	 i hope toshiba can fix it in future.)"
> 
> So, I think the basic problem remains the same.. 
My Toshiba nand chips are just samples.

Toshiba's FAE had confirmed that the MP(mass product) nand will fix the issue
above. The ECC info will be filled with the correct value in the MP nand.

> However, I let Stefan comment about it more, as his NAND part is different
> from the one you tested.
Not all the Toshiba nand support the JEDEC, please read the datasheet before
testing the patch set.

thanks
Huang Shijie

Re: [PATCH] mtd: nand_ids: Add device parameters for Toshiba's TC58NVG1S3ETAI0 NAND EEPROM

[Brian Norris]

Hi Pekon,

On Tue, Jan 21, 2014 at 07:33:04AM +0000, Pekon Gupta wrote:
> >From: Brian Norris [mailto:computersforpeace@gmail.com]
> >>On Tue, Jan 07, 2014 at 11:04:43AM +0100, Stefan Roese wrote:
> >> Toshiba's TC58NVG1S3ETAI0 is a single 3.3V 2 Gbit (2,214,592,512 bits) NAND
> >> Electrically Erasable and Programmable Read-Only Memory (NAND E2PROM)
> >> organized as (2048 + 64) bytes x 64 pages x 2048 blocks.
> >>
> >> Signed-off-by: Stefan Roese <sr@denx.de>
> [...]
> >> @@ -31,6 +31,9 @@ struct nand_flash_dev nand_flash_ids[] = {
> >>  	 * listed by full ID. We list them first so that we can easily identify
> >>  	 * the most specific match.
> >>  	 */
> >> +	{"TC58NVG1S3ETAI0 2G 3.3V 8-bit",
> >> +		{ .id = {0x98, 0xda, 0x90, 0x15, 0x76, 0x00, 0x00, 0x00} },
> >> +		  SZ_2K, SZ_256, SZ_128K, 0, 5, 64, NAND_ECC_INFO(1, SZ_512) },
> >
> >I have to NAK this in its current form, for two reasons:
> >
> >1) This is not a specific enough ID string. It conflicts with another
> >NAND which was recently supported.
> >
> >2) This patch may not really be needed at all. The "full ID" listing is
> >only for NAND which can't be detected via other means (e.g., ONFI, or
> >traditional extended ID parsing). Is this NAND mis-detected in Linus'
> >current tree? As I read the code, it should correctly identify this
> >NAND's device size, page size, OOB size, etc.

FYI, I meant for there to be a logical break between the above paragraph
and the following commit reference. The following commit is simply a
reference with some information about 1 and 2. It is well documented in
its description, so please read it if you are interested in Toshiba's
SLC NAND.

> >See this commit for reference:
> >
> >commit 60c6738245612df9499b340c15edf48b8f3e7981
> >Author: Brian Norris <computersforpeace@gmail.com>
> >Date:   Tue Jun 25 13:17:59 2013 -0700
> >
> >    mtd: nand: detect OOB size for Toshiba 24nm raw SLC
> >
> >   https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=60c6738245612df9499b340c15edf48b8f3e7981
> >
> >It adds support for a class of Toshiba 24nm NAND, including this one:
> >
> >    24nm SLC 2Gbit TC58NVG1S3HTA00
> >    ID: 0x98 0xda 0x90 0x15 0x76 0x16 0x08 0x00
> >
> >Please reconcile your patch with the comments found there.
> >
> 
> Yes, I think this is a different technology part..

Yes, it is a different technology part. But this patch is not specific
enough to make any distinction between them.

> because though the page-size = 2048B is same, but OOB-size = 64B.
> However, with your OOB-size is detected as 128B.

Stefan should read my patch, then determine the way forward -- either
with a more specific ID string or (as I suspect) by dropping this patch
altogether as it is unnecessary.

For reference, I believe the correct 6-byte ID string would be:

  98, DA 90, 15, 76, 14

And then you would notice that the 6th byte differs from the one I
quoted from my patch. So the 6th ID byte is important.

But again, I don't think this patch is needed at all. This flash (a 43nm
Toshiba SLC) should be detected just fine through ID decoding that is
already present.

> Best part is the datasheet of this part does not mention anything about the 6th byte :-).
> So, I think we can first  read 6th-byte (using READID command), and figure out difference
> between your part and this one, then based on it append the OOB-calculation to you patch. Ok ?

I actually have had several discussions with Toshiba about their SLC. I
have quite a few tables of 6-byte ID strings from them due to these kind
of difficulties, and I believe I convinced them to include the 6th byte
ID information in datasheets for their new parts (not sure if they will
do this retroactively for older ones). Somewhere, I have at least one
doc from Toshiba that they gave the OK for me to post publicly. If you
want it and I can scrounge it up, I'll try to get it out there.

Brian