Re: [U-Boot] Fix fsl_elbc_nand driver
On Tue, 2015-05-19 at 17:38 -0500, Scott Wood wrote:
On Tue, 2015-05-19 at 15:29 -0700, Andrei Yakimov wrote:
I did not compiling latest, I still in 2011.9 and 2.6.38. I have go over latest kernel and can see they using NAND_CMD_PARAM with sub command 0x40 - to get JEDEC information, it is 3 mandatory copy by 512 bytes.
3x512 or 3x256?
ONFI - 3x256 sub command 0x0 JEDEC - 3x512 sub command 0x40
Going over kernel divers, figure out some read whole page some 256 bytes. Reading whole page (set fcbr = 0) have some sense - you do not need to know anything about flash, but what to put in to read_bytes ?
You don't want fbcr = 0 here because that will enable ECC which isn't there.
Is it correcting or just generating syndrome? It is working on my board, I would say it only generate or ignored for this command (8313). It should corrupt data if it correcting but it does not.
It looks like for universal patch 2K should be read.
Again, if we're going to do anything beyond s/256/768/ it should be a higher level function where the caller says how much it wants.
It is not normal nand flow: READ_ID and PARAM assuming it know the size.
I have also check other vendor controllers like tegra, there continuous data read trigger additional data transfer from chip.
Can we do (NOP CWO UA RWB RS RS RS RS) wait ltesr (cc) and after that next read_buffer ( RB or RS) all command have to start with NOP, this will effectively terminate previous command. And we do not care about locks in u-boot. kernel will be different store, but again this code executed only during start up - so who care holding CS to long.
there is only 4 places for PARAM: drivers/mtd/nand/mxs_nand_spl.c chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); drivers/mtd/nand/nand_base.c: chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); drivers/mtd/nand/nand_base.c: chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); drivers/mtd/nand/nand_base.c: chip->cmdfunc(mtd, NAND_CMD_PARAM, 0x40, -1);
latest kernel read it like this: chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); for (i = 0; i < 3; i++) { for (j = 0; j < sizeof(*p); j++) ((uint8_t *)p)[j] = chip->read_byte(mtd); if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == le16_to_cpu(p->crc)) { break; } }
Yes, that's how the hardware works, but controllers like eLBC don't directly expose that interface. We need to get all of the command's output at once.
This kind of implementation better, but I did not see how it could be done for this controller.
I wouldn't say it's "better" so much as a closer fit to what the Linux NAND code is expecting.
I am not sure how is small page (512 byte) flash should operate also.
Is there any small-page ONFI flash?
I do not know. ONFI parameter page will tell you page size: 80-83 M Number of data bytes per page 84-85 M Number of spare bytes per page if we drop it, lets set to 2k and forget.
Why did you take this e-mail off-list?
Sorry just forgot.
-Scott
On Tue, 2015-05-19 at 16:42 -0700, Andrei Yakimov wrote:
On Tue, 2015-05-19 at 17:38 -0500, Scott Wood wrote:
On Tue, 2015-05-19 at 15:29 -0700, Andrei Yakimov wrote:
I did not compiling latest, I still in 2011.9 and 2.6.38. I have go over latest kernel and can see they using NAND_CMD_PARAM with sub command 0x40 - to get JEDEC information, it is 3 mandatory copy by 512 bytes.
3x512 or 3x256?
ONFI - 3x256 sub command 0x0 JEDEC - 3x512 sub command 0x40
So then we want 1536 bytes, not 768 (or 786) if we go with the simple fix?
Going over kernel divers, figure out some read whole page some 256 bytes. Reading whole page (set fcbr = 0) have some sense - you do not need to know anything about flash, but what to put in to read_bytes ?
You don't want fbcr = 0 here because that will enable ECC which isn't there.
Is it correcting or just generating syndrome? It is working on my board, I would say it only generate or ignored for this command (8313). It should corrupt data if it correcting but it does not.
Correcting. Perhaps it's working because it's reporting an uncorrectable error (thus not correcting anything) and you're ignoring it?
It looks like for universal patch 2K should be read.
Again, if we're going to do anything beyond s/256/768/ it should be a higher level function where the caller says how much it wants.
It is not normal nand flow: READ_ID and PARAM assuming it know the size.
I'm not sure what you're trying to say here.
I have also check other vendor controllers like tegra, there continuous data read trigger additional data transfer from chip.
Can we do (NOP CWO UA RWB RS RS RS RS) wait ltesr (cc) and after that next read_buffer ( RB or RS) all command have to start with NOP, this will effectively terminate previous command. And we do not care about locks in u-boot. kernel will be different store, but again this code executed only during start up - so who care holding CS to long.
You won't be holding CS that long. It will drop as soon as the current operation completes. And I'm not interested in a solution that only works in U-Boot's single-tasking environment, given that this code is more or less shared with Linux.
I don't see what the objection is to adding a replaceable read_param() method that is not so hostile to high-level controllers.
-Scott
I will make a patch with 1536.
Where should I send linux patch? They have bunch of mail lists for different subsystems. Andrei
On Wed, 2015-05-20 at 17:25 -0500, Scott Wood wrote:
On Tue, 2015-05-19 at 16:42 -0700, Andrei Yakimov wrote:
On Tue, 2015-05-19 at 17:38 -0500, Scott Wood wrote:
On Tue, 2015-05-19 at 15:29 -0700, Andrei Yakimov wrote:
I did not compiling latest, I still in 2011.9 and 2.6.38. I have go over latest kernel and can see they using NAND_CMD_PARAM with sub command 0x40 - to get JEDEC information, it is 3 mandatory copy by 512 bytes.
3x512 or 3x256?
ONFI - 3x256 sub command 0x0 JEDEC - 3x512 sub command 0x40
So then we want 1536 bytes, not 768 (or 786) if we go with the simple fix?
Yes
Going over kernel divers, figure out some read whole page some 256 bytes. Reading whole page (set fcbr = 0) have some sense - you do not need to know anything about flash, but what to put in to read_bytes ?
You don't want fbcr = 0 here because that will enable ECC which isn't there.
Is it correcting or just generating syndrome? It is working on my board, I would say it only generate or ignored for this command (8313). It should corrupt data if it correcting but it does not.
Correcting. Perhaps it's working because it's reporting an uncorrectable error (thus not correcting anything) and you're ignoring it?
may be.
It looks like for universal patch 2K should be read.
Again, if we're going to do anything beyond s/256/768/ it should be a higher level function where the caller says how much it wants.
It is not normal nand flow: READ_ID and PARAM assuming it know the size.
I'm not sure what you're trying to say here.
I meant normal nand flow read/write - propagate size how much to read or write, READ_ID and PARAM regular nand flow assume driver would know how much to read.
I have also check other vendor controllers like tegra, there continuous data read trigger additional data transfer from chip.
Can we do (NOP CWO UA RWB RS RS RS RS) wait ltesr (cc) and after that next read_buffer ( RB or RS) all command have to start with NOP, this will effectively terminate previous command. And we do not care about locks in u-boot. kernel will be different store, but again this code executed only during start up - so who care holding CS to long.
You won't be holding CS that long. It will drop as soon as the current operation completes. And I'm not interested in a solution that only works in U-Boot's single-tasking environment, given that this code is more or less shared with Linux.
Are you saying elbc will drop CS even last fir instruction not 0? You are at Freescale - you should know or can check :). About lock, I was only saying linux will might need a lock for this sequences, depend on nand flash detection can or can not run in parallel if you have multiple chips - but I do not think it can - it is early boot an it is not how nand initializes. MTD doing it at once.
I don't see what the objection is to adding a replaceable read_param() method that is not so hostile to high-level controllers.
Sorry, I has not understand you completely. Are you suggesting add read_param() method to whole nand infrastructure, for NAND_CMD_PARAM method? It is huge changes and this will not change fact some how we should get information about read size. For elbc and imx due to we reading all at once, it can not be stateless, we need to read more and more each time reissuing command or relay on different way to ID chip - and this make it pointless if it can not be done universally.
-Scott
On Wed, 2015-05-20 at 18:27 -0700, Andrei Yakimov wrote:
I will make a patch with 1536.
Where should I send linux patch? They have bunch of mail lists for different subsystems. Andrei
The MAINTAINERS file shows where to send patches for different subsystems. In this case it would be the "MEMORY TECHNOLOGY DEVICES (MTD)" section.
On Wed, 2015-05-20 at 17:25 -0500, Scott Wood wrote:
On Tue, 2015-05-19 at 16:42 -0700, Andrei Yakimov wrote:
On Tue, 2015-05-19 at 17:38 -0500, Scott Wood wrote:
On Tue, 2015-05-19 at 15:29 -0700, Andrei Yakimov wrote:
I did not compiling latest, I still in 2011.9 and 2.6.38. I have go over latest kernel and can see they using NAND_CMD_PARAM with sub command 0x40 - to get JEDEC information, it is 3 mandatory copy by 512 bytes.
3x512 or 3x256?
ONFI - 3x256 sub command 0x0 JEDEC - 3x512 sub command 0x40
So then we want 1536 bytes, not 768 (or 786) if we go with the simple fix?
Yes
Going over kernel divers, figure out some read whole page some 256 bytes. Reading whole page (set fcbr = 0) have some sense - you do not need to know anything about flash, but what to put in to read_bytes ?
You don't want fbcr = 0 here because that will enable ECC which isn't there.
Is it correcting or just generating syndrome? It is working on my board, I would say it only generate or ignored for this command (8313). It should corrupt data if it correcting but it does not.
Correcting. Perhaps it's working because it's reporting an uncorrectable error (thus not correcting anything) and you're ignoring it?
may be.
It looks like for universal patch 2K should be read.
Again, if we're going to do anything beyond s/256/768/ it should be a higher level function where the caller says how much it wants.
It is not normal nand flow: READ_ID and PARAM assuming it know the size.
I'm not sure what you're trying to say here.
I meant normal nand flow read/write - propagate size how much to read or write, READ_ID and PARAM regular nand flow assume driver would know how much to read.
No, they don't assume the driver knows how much to read. They assume the driver doesn't need to tell the hardware how much to read, which isn't true with this controller.
I have also check other vendor controllers like tegra, there continuous data read trigger additional data transfer from chip.
Can we do (NOP CWO UA RWB RS RS RS RS) wait ltesr (cc) and after that next read_buffer ( RB or RS) all command have to start with NOP, this will effectively terminate previous command. And we do not care about locks in u-boot. kernel will be different store, but again this code executed only during start up - so who care holding CS to long.
You won't be holding CS that long. It will drop as soon as the current operation completes. And I'm not interested in a solution that only works in U-Boot's single-tasking environment, given that this code is more or less shared with Linux.
Are you saying elbc will drop CS even last fir instruction not 0? You are at Freescale - you should know or can check :).
That is my understanding of how the hardware works, yes (though I haven't tested or asked someone who knows the implementation). The bit about NOP ending the operation sequence just means that the hardware won't look at any subsequent fields in FIR once it finds a NOP.
About lock, I was only saying linux will might need a lock for this sequences, depend on nand flash detection can or can not run in parallel if you have multiple chips - but I do not think it can - it is early boot an it is not how nand initializes. MTD doing it at once.
What if the user inserts a NAND module after boot, while NOR activity is going on in the background?
In any case, I really don't want to do such things in this driver.
I don't see what the objection is to adding a replaceable read_param() method that is not so hostile to high-level controllers.
Sorry, I has not understand you completely. Are you suggesting add read_param() method to whole nand infrastructure, for NAND_CMD_PARAM method?
Yes.
It is huge changes
It's not. The default implementation would contain the more or less the same code that runs today.
and this will not change fact some how we should get information about read size.
The size to be read would be a parameter to read_param().
For elbc and imx due to we reading all at once, it can not be stateless, we need to read more and more each time
Why do we need to? Why can't we read all three copies at once?
reissuing command or relay on different way to ID chip - and this make it pointless if it can not be done universally.
Or, we can reissue the command. I don't see any big problem either way. This is not performance critical.
-Scott
On Wed, 2015-05-20 at 20:37 -0500, Scott Wood wrote:
On Wed, 2015-05-20 at 18:27 -0700, Andrei Yakimov wrote:
I will make a patch with 1536.
Where should I send linux patch? They have bunch of mail lists for different subsystems. Andrei
The MAINTAINERS file shows where to send patches for different subsystems. In this case it would be the "MEMORY TECHNOLOGY DEVICES (MTD)" section.
On Wed, 2015-05-20 at 17:25 -0500, Scott Wood wrote:
On Tue, 2015-05-19 at 16:42 -0700, Andrei Yakimov wrote:
On Tue, 2015-05-19 at 17:38 -0500, Scott Wood wrote:
On Tue, 2015-05-19 at 15:29 -0700, Andrei Yakimov wrote:
I did not compiling latest, I still in 2011.9 and 2.6.38. I have go over latest kernel and can see they using NAND_CMD_PARAM with sub command 0x40 - to get JEDEC information, it is 3 mandatory copy by 512 bytes.
3x512 or 3x256?
ONFI - 3x256 sub command 0x0 JEDEC - 3x512 sub command 0x40
So then we want 1536 bytes, not 768 (or 786) if we go with the simple fix?
Yes
Going over kernel divers, figure out some read whole page some 256 bytes. Reading whole page (set fcbr = 0) have some sense - you do not need to know anything about flash, but what to put in to read_bytes ?
You don't want fbcr = 0 here because that will enable ECC which isn't there.
Is it correcting or just generating syndrome? It is working on my board, I would say it only generate or ignored for this command (8313). It should corrupt data if it correcting but it does not.
Correcting. Perhaps it's working because it's reporting an uncorrectable error (thus not correcting anything) and you're ignoring it?
may be.
It looks like for universal patch 2K should be read.
Again, if we're going to do anything beyond s/256/768/ it should be a higher level function where the caller says how much it wants.
It is not normal nand flow: READ_ID and PARAM assuming it know the size.
I'm not sure what you're trying to say here.
I meant normal nand flow read/write - propagate size how much to read or write, READ_ID and PARAM regular nand flow assume driver would know how much to read.
No, they don't assume the driver knows how much to read. They assume the driver doesn't need to tell the hardware how much to read, which isn't true with this controller.
I have also check other vendor controllers like tegra, there continuous data read trigger additional data transfer from chip.
Can we do (NOP CWO UA RWB RS RS RS RS) wait ltesr (cc) and after that next read_buffer ( RB or RS) all command have to start with NOP, this will effectively terminate previous command. And we do not care about locks in u-boot. kernel will be different store, but again this code executed only during start up - so who care holding CS to long.
You won't be holding CS that long. It will drop as soon as the current operation completes. And I'm not interested in a solution that only works in U-Boot's single-tasking environment, given that this code is more or less shared with Linux.
Are you saying elbc will drop CS even last fir instruction not 0? You are at Freescale - you should know or can check :).
That is my understanding of how the hardware works, yes (though I haven't tested or asked someone who knows the implementation). The bit about NOP ending the operation sequence just means that the hardware won't look at any subsequent fields in FIR once it finds a NOP.
About lock, I was only saying linux will might need a lock for this sequences, depend on nand flash detection can or can not run in parallel if you have multiple chips - but I do not think it can - it is early boot an it is not how nand initializes. MTD doing it at once.
What if the user inserts a NAND module after boot, while NOR activity is going on in the background?
In any case, I really don't want to do such things in this driver.
I don't see what the objection is to adding a replaceable read_param() method that is not so hostile to high-level controllers.
Sorry, I has not understand you completely. Are you suggesting add read_param() method to whole nand infrastructure, for NAND_CMD_PARAM method?
Yes.
It is huge changes
It's not. The default implementation would contain the more or less the same code that runs today.
and this will not change fact some how we should get information about read size.
The size to be read would be a parameter to read_param().
For elbc and imx due to we reading all at once, it can not be stateless, we need to read more and more each time
Why do we need to? Why can't we read all three copies at once?
reissuing command or relay on different way to ID chip - and this make it pointless if it can not be done universally.
Or, we can reissue the command. I don't see any big problem either way. This is not performance critical.
lets say 1 time you read 256 ( or 512) it go bad, next time you read 512 (or 1024) next time you read 768 ( or 1536). Upper layer can maintain it. Roughly like this:
Was: chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); for (i = 0; i < 3; i++) { chip->read_buf(mtd, (uint8_t *)p, sizeof(*p)); if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == le16_to_cpu(p->crc)) { break; } } if (i == 3) return 0;
new: int read_size, offset; read_size = 256; offset =0; if(!chip->read_param) chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); for (i = 0; i < 3; i++) { if(chip->read_param) chip->read_param( 0, read_size); chip->read_buf(mtd, (uint8_t *)p + offest, sizeof(*p)); if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == le16_to_cpu(p->crc)) { break; } offset = read_size; read_size += 256; /* for ONFI only */ }
if (i == 3) return 0;
I have used old u-boot, but more or less like this. chip structure would have extra member read_param().
-Scott
On Wed, 2015-05-20 at 18:55 -0700, Andrei Yakimov wrote:
On Wed, 2015-05-20 at 20:37 -0500, Scott Wood wrote:
On Wed, 2015-05-20 at 18:27 -0700, Andrei Yakimov wrote:
For elbc and imx due to we reading all at once, it can not be stateless, we need to read more and more each time
Why do we need to? Why can't we read all three copies at once?
reissuing command or relay on different way to ID chip - and this make it pointless if it can not be done universally.
Or, we can reissue the command. I don't see any big problem either way. This is not performance critical.
lets say 1 time you read 256 ( or 512) it go bad, next time you read 512 (or 1024) next time you read 768 ( or 1536).
I was thinking read_param() would take the offset as a parameter and use NAND_CMD_RNDOUT to skip ahead -- but that would change the default flow which I'd rather avoid. Another option is that read_param() just sets up the read for the specified number of bytes, but the caller still uses read_byte() to extract the data. This way the code could specify sizeof(struct)*3 as the size up front without needing three separate buffers.
Note that whatever gets done should first be accepted in Linux, rather than being a local U-Boot change. If you want a short-term fix, just stick 1536 in the eLBC driver.
Upper layer can maintain it. Roughly like this:
Was: chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); for (i = 0; i < 3; i++) { chip->read_buf(mtd, (uint8_t *)p, sizeof(*p));
You're looking at old code. It uses read_byte() now.
if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == le16_to_cpu(p->crc)) { break; }} if (i == 3) return 0;
new: int read_size, offset; read_size = 256; offset =0; if(!chip->read_param) chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
I don't want "if (chip->read_param)" all over the place; there should be a default read_param() that does what the existing code does.
for (i = 0; i < 3; i++) { if(chip->read_param) chip->read_param( 0, read_size); chip->read_buf(mtd, (uint8_t *)p + offest, sizeof(*p));
This isn't going to read the second or third copy; it's going to read the first copy and write beyond the end of your buffer.
-Scott
For now lets stick with 1536 in u-boot. I will send a patch. At least it will not loosing flash over time as nand ages.
I understand what you wish, and will take a look on it inside fresh new kernel. I found one more driver - marvel looks like have same problem. I will check how NAND_CMD_RNDOUT is working. Perhaps we do not need extra read_param(), and use only NAND_CMD_RNDOUT to get next block inside page loop.
Andrei
On Wed, 2015-05-20 at 21:27 -0500, Scott Wood wrote:
On Wed, 2015-05-20 at 18:55 -0700, Andrei Yakimov wrote:
On Wed, 2015-05-20 at 20:37 -0500, Scott Wood wrote:
On Wed, 2015-05-20 at 18:27 -0700, Andrei Yakimov wrote:
For elbc and imx due to we reading all at once, it can not be stateless, we need to read more and more each time
Why do we need to? Why can't we read all three copies at once?
reissuing command or relay on different way to ID chip - and this make it pointless if it can not be done universally.
Or, we can reissue the command. I don't see any big problem either way. This is not performance critical.
lets say 1 time you read 256 ( or 512) it go bad, next time you read 512 (or 1024) next time you read 768 ( or 1536).
I was thinking read_param() would take the offset as a parameter and use NAND_CMD_RNDOUT to skip ahead -- but that would change the default flow which I'd rather avoid. Another option is that read_param() just sets up the read for the specified number of bytes, but the caller still uses read_byte() to extract the data. This way the code could specify sizeof(struct)*3 as the size up front without needing three separate buffers.
Note that whatever gets done should first be accepted in Linux, rather than being a local U-Boot change. If you want a short-term fix, just stick 1536 in the eLBC driver.
Upper layer can maintain it. Roughly like this:
Was: chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1); for (i = 0; i < 3; i++) { chip->read_buf(mtd, (uint8_t *)p, sizeof(*p));
You're looking at old code. It uses read_byte() now.
if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == le16_to_cpu(p->crc)) { break; }} if (i == 3) return 0;
new: int read_size, offset; read_size = 256; offset =0; if(!chip->read_param) chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
I don't want "if (chip->read_param)" all over the place; there should be a default read_param() that does what the existing code does.
for (i = 0; i < 3; i++) { if(chip->read_param) chip->read_param( 0, read_size); chip->read_buf(mtd, (uint8_t *)p + offest, sizeof(*p));
This isn't going to read the second or third copy; it's going to read the first copy and write beyond the end of your buffer.
-Scott
On Wed, 2015-05-20 at 19:42 -0700, Andrei Yakimov wrote:
For now lets stick with 1536 in u-boot. I will send a patch. At least it will not loosing flash over time as nand ages.
I understand what you wish, and will take a look on it inside fresh new kernel. I found one more driver - marvel looks like have same problem. I will check how NAND_CMD_RNDOUT is working. Perhaps we do not need extra read_param(), and use only NAND_CMD_RNDOUT to get next block inside page loop.
Again, I'm a reluctant to use RNDOUT in the default read_param() because that would change the flow for all controllers and chips, and while the chip manual I'm looking at says it's OK, it introduces risk that it doesn't work everywhere (e.g. some controller drivers that provide their own cmdfunc don't implement RNDOUT).
-Scott
On Wed, 2015-05-20 at 21:46 -0500, Scott Wood wrote:
On Wed, 2015-05-20 at 19:42 -0700, Andrei Yakimov wrote:
For now lets stick with 1536 in u-boot. I will send a patch. At least it will not loosing flash over time as nand ages.
I understand what you wish, and will take a look on it inside fresh new kernel. I found one more driver - marvel looks like have same problem. I will check how NAND_CMD_RNDOUT is working. Perhaps we do not need extra read_param(), and use only NAND_CMD_RNDOUT to get next block inside page loop.
Again, I'm a reluctant to use RNDOUT in the default read_param() because that would change the flow for all controllers and chips, and while the chip manual I'm looking at says it's OK, it introduces risk that it doesn't work everywhere (e.g. some controller drivers that provide their own cmdfunc don't implement RNDOUT).
Forget about read_param(), just like this: for (i = 0; i < 3; i++) { for (j = 0; j < sizeof(*p); j++) ((uint8_t *)p)[j] = chip->read_byte(mtd); if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == le16_to_cpu(p->crc)) { break; } chip->cmdfunc(mtd, NAND_CMD_RNDOUT, 0, -1); }
and this is good - will be "no op" or "bad command" error, which could be ignored - so for this drivers operation flow is unchanged.
-Scott
I am still learning git/patman. It will be day or two while I figure out patman. By some reason after "git commit --amend" patman kill my patch. I am missing something. Andrei
On Wed, 2015-05-20 at 20:03 -0700, Andrei Yakimov wrote:
On Wed, 2015-05-20 at 21:46 -0500, Scott Wood wrote:
On Wed, 2015-05-20 at 19:42 -0700, Andrei Yakimov wrote:
For now lets stick with 1536 in u-boot. I will send a patch. At least it will not loosing flash over time as nand ages.
I understand what you wish, and will take a look on it inside fresh new kernel. I found one more driver - marvel looks like have same problem. I will check how NAND_CMD_RNDOUT is working. Perhaps we do not need extra read_param(), and use only NAND_CMD_RNDOUT to get next block inside page loop.
Again, I'm a reluctant to use RNDOUT in the default read_param() because that would change the flow for all controllers and chips, and while the chip manual I'm looking at says it's OK, it introduces risk that it doesn't work everywhere (e.g. some controller drivers that provide their own cmdfunc don't implement RNDOUT).
RNDOUT is already used by nand_flash_detect_ext_param_page(), so this isn't as much of a concern for ONFI, but it could be an issue with nand_flash_detect_jedec().
Forget about read_param(),
Then how will it work on controllers like eLBC/IFC which is the whole point?
just like this: for (i = 0; i < 3; i++) { for (j = 0; j < sizeof(*p); j++) ((uint8_t *)p)[j] = chip->read_byte(mtd); if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == le16_to_cpu(p->crc)) { break; } chip->cmdfunc(mtd, NAND_CMD_RNDOUT, 0, -1); }
and this is good - will be "no op" or "bad command" error, which could be ignored - so for this drivers operation flow is unchanged.
RNDOUT needs to come before read_buf() and it needs to specify the offset you want.
-Scott
On Wed, 2015-05-20 at 22:11 -0500, Scott Wood wrote:
On Wed, 2015-05-20 at 20:03 -0700, Andrei Yakimov wrote:
On Wed, 2015-05-20 at 21:46 -0500, Scott Wood wrote:
On Wed, 2015-05-20 at 19:42 -0700, Andrei Yakimov wrote:
For now lets stick with 1536 in u-boot. I will send a patch. At least it will not loosing flash over time as nand ages.
I understand what you wish, and will take a look on it inside fresh new kernel. I found one more driver - marvel looks like have same problem. I will check how NAND_CMD_RNDOUT is working. Perhaps we do not need extra read_param(), and use only NAND_CMD_RNDOUT to get next block inside page loop.
Again, I'm a reluctant to use RNDOUT in the default read_param() because that would change the flow for all controllers and chips, and while the chip manual I'm looking at says it's OK, it introduces risk that it doesn't work everywhere (e.g. some controller drivers that provide their own cmdfunc don't implement RNDOUT).
RNDOUT is already used by nand_flash_detect_ext_param_page(), so this isn't as much of a concern for ONFI, but it could be an issue with nand_flash_detect_jedec().
Forget about read_param(),
Then how will it work on controllers like eLBC/IFC which is the whole point?
just like this:
I miss this line: chip->cmdfunc(mtd, NAND_CMD_PARAM, 0, -1);
for (i = 0; i < 3; i++) { for (j = 0; j < sizeof(*p); j++) ((uint8_t *)p)[j] = chip->read_byte(mtd); if (onfi_crc16(ONFI_CRC_BASE, (uint8_t *)p, 254) == le16_to_cpu(p->crc)) { break; } chip->cmdfunc(mtd, NAND_CMD_RNDOUT, <offest>, -1); }
and this is good - will be "no op" or "bad command" error, which could be ignored - so for this drivers operation flow is unchanged.
RNDOUT needs to come before read_buf() and it needs to specify the offset you want.
column address - it is exactly offset for RNDOUT. First param read 256/512 bytes, if it fail, we do RNDOUT to get next. we will not do extra RNDOUT - it is for() loop.
I can test it on my board. This is really good solution if it work. it is just 1 line, and only when ONFI mark already read from flash.
And we can leave (NAND_CMD_PARAM, 0x40) (JDEC label) handling for kernel folks.
My problem only jesd230B do not specify PARAM command, ONFI4.0 - do not expect column address for PARAM.
Linux kernel cleary doing (NAND_CMD_PARAM, 0x40).
This is bit annoying.
Question is elbc/ifc old controllers - is it worth the effort?
-Scott
On Wed, 2015-05-20 at 20:54 -0700, Andrei Yakimov wrote:
My problem only jesd230B do not specify PARAM command, ONFI4.0 - do not expect column address for PARAM.
Linux kernel cleary doing (NAND_CMD_PARAM, 0x40).
This is bit annoying.
Question is elbc/ifc old controllers - is it worth the effort?
IFC isn't an old controller.
As for worth the effort, probably not. 1536 is easy. :-)
-Scott
participants (2)
-
Andrei Yakimov -
Scott Wood