| Message ID | 1389089083-29694-1-git-send-email-sr@denx.de |
|---|---|
| State | Rejected |
| Headers | show |
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
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
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
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
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
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
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) },
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(+)