David,
Please do not top post. Post after the quoted section(s) as I do below.
David Ho wrote:
Hi all,
Maybe if there is someone from Intel on this list they would have clear up this confusion much earlier. Better yet, if someone from Intel can correct me here.
I am not sure if we have anyone from Intel on this list. At least nobody with intel.com address posted to the list in the last couple of years as far as I can remember. U-Boot is not very popular in X86 systems.
Some explanation is in order.
While Intel flashes K3/C3 use the that command sequence as block unlock, the same command sequence (0x60 0xD0) is used to "clear all blocks bits" on the J3. This is restrictly speaking not an intel flash bug. The datasheet had not mentioned there is a change to the lock/unlock command in the datasheet revision history so it appears to have been there in the J3 datasheet from the start. Just that there is an inconsistency in the behaviour of the command sequence. They
I looked at the Intel datasheet for 28F128J3, 28F640J3 and 28F320J3 (Order # 290667-014 December 2003). Upon very careful read, I found on Page 18, Table 4 (Command Bus-Cycle Definitions (Sheet 2 of 2), the small type footnote that reads: "15. The clear block lock-bits operation simultaneously clears all the block lock-bits". Also the while the lock command reads as "Set Block-Lock Bit" the clear command says "Clear Block-Lock Bits" (notice plural).
So, this is indeed not a bug but a feature (or perhaps another example of a design bug being classified as a feature, who knows). So, my earlier comment that it is a bug is not correct. Nevertheless, bug or not this does not change the fact that re-locking is needed as in the current code so removing that functionality is NOT OK. So, the comment "/* Intel's unprotect unprotects all locking */" perhaps needs to be made more specific as /* Intel's Strataflash J3 unlocks all locking , redo remaining locks */
refer to it as Legacy lock/unlock in the Primary Vendor Specifc Exended Query Table. So information can be extracted from CFI attributes.
For J3 flash, "Optional Flash Features and Commands" has offset 36h-39h (4 bytes) bit 3 Legacy lock/unlock supported bit (bit 3). Again, for J3 chips my datasheet says this is set to "1". I see K3 datasheet has this set to 0. If you are absolutely sure that this is foolproof indication that all parts with bit 3 clear do not require re-locking, submit a suitable patch though this will also mean slight increased code base at the expense of saving minor time for K3 parts on unlocking.
BTW, the vendor specific extended query table is unique to each CFI vendor (and might be even to the product family) and might not be implemented by all vendors. So, if you use information from this table you must properly identify vendor, product family before trying to use that information. This might add to the code further.
All indication suggests that Intel is not at fault.
With the evidence gathered thus far, it is simply small detail overlooked by the implementor of the original code, which is perfectly acceptable. Just that I would have hoped this thread elicited discussion that led to this discovery.
Perhaps the code was implemented before K3 families were out. The comment on the code does not indicate this. My earlier comment that this is a bug was indeed wrong but since I did not implement that part of the code my mistake does not necessarily carry to the original author. I am actually happy to another detail that is easy to miss.
I wonder how I can better present myself to make others more co-operative.
Sorry if I sounded not so cooperative but when you simply suggested removing the code, that is obviously not acceptable to me and I raised my objection. Anyhow, through this discussion the need is validated and possibility of improvement is brought out. The question now is that is it worth it? How often do you lock unlock the flash in U-Boot to implement the improvement as the effect is some extra delay in K3 family chips.
Best regards, Tolunay
David
On 3/16/06, Tolunay Orkun listmember@orkun.us wrote:
David Ho wrote:
Okay,
I really didn't mean to rip out someone's code, I know it was there for a reason. In any case, I like to understand which Intel flash is behaving badly. The spec does not say unprotect unlocks all blocks.
First, I am not the author of this part of code. I can tell you the Intel StrataFlash 28F128J3 parts on my Cogent CSB272 board needs this code. Original author must have hit the same issue. This is a chip bug and probably documented on some Intel Errata and it may be applying to certain Revs of the chip. I do not have a list.
If no one has any objection, I will remove the part of the code that relock each sector, for submission.
First, That behavior is required because some Intel flash parts incorrectly unlock all sectors (not just current sector) so after unlocking the current sector we must redo the locking of all others that were supposed to remain locked. Again, the comment in that code reflects and explains this.
I am sorry, my interpretation of your comment led me to think you are suggesting all Intel flashes behave this way. Really my original post
What is not clear about the word "some" in my description of the issue? I am obviously not claiming all Intel parts are this. Your interpretation is incorrect.
came from a confusion of the comment. A better comment may include what you just said above.
Do you know which parts behave this way? Has Intel confirmed this?
I do not need Intel's confirmation. It is a reality for me. Again, it is most likely documented in some Intel Errata (which might not be widely publicly circulated).
Certainly not all flashes need to have this workaround, perhaps it is sensible to option it out? Anyway, I have no problem with this code being there. Since I have not seen the same behaviour you saw, and google did not turn up and evidence to support this, I was just curious if this has been solved since the code was first submitted.
I certainly do not have the capability to test each new rev of every Intel flash. Without a complete list you can only take the safe path...