[U-Boot-Users] flash protection code in cfi_flash

[Tolunay Orkun]

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 at 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...
>>
>>
>>
>>