[U-Boot] Generic CFI flash driver is not so generic?

[U-Boot] Generic CFI flash driver is not so generic?

[Choe, Hyun-ho]

I did several tests for my b'd that has CFI Nor erase/write problem.

My main problem is that flash_toggle() function doesn't work properly,
it always returns 0 even while bus writing operation is proceeding.
I mean, toggle bit doesn't toggle for each read operation. (refer to
drivers/mtd/cfi_flash.c)

I found two workarounds.

First, small waiting between each read. So, following code doesn't work.

<SNIP>
retval = flash_read16(addr) != flash_read16(addr);
<SNIP>

But, following code does work.

<SNIP>
u16 d = flash_read16(addr);
udelay(1);
u16 t = flash_read16(addr);
retval = d != t;
<SNIP>

I tried to increase read wait cycle to maximum which my chip allows, but
has no effect. Only waiting small period between each read can solve the
problem.


Second workaround, I defined slightly modified version of
flash_read_word() functions like following:

<SNIP>
static inline ushort flash_read_one_word (flash_info_t * info,
flash_sect_t sect, uint offset)
{
	ushort *addr, retval;

	addr = flash_map (info, sect, offset);
	retval = flash_read16 (addr);
	flash_unmap (info, sect, offset, addr);
	return retval;
}
<SNIP>

And flash_toggle() function is changed like this:

<SNIP>
retval = flash_read_one_word(info, sect, offset) !=
flash_read_one_word(info, sect, offset);
<SNIP>

This trick works perfectly on my board, without any delay.
I don't know exactly that this is caused by specific bus access
mechanism, or simply chip silicon bug.

flash_read_uchar(), flash_read_word() can be modified to get another
parameter for sector. (flash_read_long() already gets sector)
And, flash_read_long_long() function may needs to be defined.

What do you think about this? Is there any side effect between each
platform?

Regards,
Choe, Hyun-ho

[U-Boot] Generic CFI flash driver is not so generic?

[Wolfgang Denk]

Dear "Choe, Hyun-ho",

In message <1227678669.15327.62.camel@hades.legend.co.kr> you wrote:
> I did several tests for my b'd that has CFI Nor erase/write problem.
> 
> My main problem is that flash_toggle() function doesn't work properly,
> it always returns 0 even while bus writing operation is proceeding.
> I mean, toggle bit doesn't toggle for each read operation. (refer to
> drivers/mtd/cfi_flash.c)

Similar problems have been reported before - please see for example
http://lists.denx.de/pipermail/u-boot/2008-October/042036.html

> I found two workarounds.
> 
> First, small waiting between each read. So, following code doesn't work.
> 
> <SNIP>
> retval = flash_read16(addr) != flash_read16(addr);
> <SNIP>

Eventually you are not performing atomic 16 bit read operations.

> But, following code does work.
> 
> <SNIP>
> u16 d = flash_read16(addr);
> udelay(1);
> u16 t = flash_read16(addr);
> retval = d != t;

Hm... a plain delay would probably not change this behaviour unless
you have another problem.

> I don't know exactly that this is caused by specific bus access
> mechanism, or simply chip silicon bug.

I think the problem is that for some reason flash_read16() might  not
result  in  16bit  bus  accesses. Do you have a chance to verify this
suspicion, for example with a logic analyzer?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Never ascribe to malice that which can  adequately  be  explained  by
stupidity.

[U-Boot] Generic CFI flash driver is not so generic?

[Choe, Hyun-ho]

2008-11-26 (?), 10:58 +0100, Wolfgang Denk ???:
> Dear "Choe, Hyun-ho",
> 
> In message <1227678669.15327.62.camel@hades.legend.co.kr> you wrote:
> > I did several tests for my b'd that has CFI Nor erase/write problem.
> > 
> > My main problem is that flash_toggle() function doesn't work properly,
> > it always returns 0 even while bus writing operation is proceeding.
> > I mean, toggle bit doesn't toggle for each read operation. (refer to
> > drivers/mtd/cfi_flash.c)
> 
> Similar problems have been reported before - please see for example
> http://lists.denx.de/pipermail/u-boot/2008-October/042036.html
> 

At first, I also suspected about atomic reading issue. But, IMHO, this
is not the cause of the problem. Please refer to
http://lists.denx.de/pipermail/u-boot/2008-November/043741.html

ldrb/ldrh/ldr is load byte/half-word/word from memory operation in ARM.
I think these operation should be atomic, in all cases.


> > I found two workarounds.
> > 
> > First, small waiting between each read. So, following code doesn't work.
> > 
> > <SNIP>
> > retval = flash_read16(addr) != flash_read16(addr);
> > <SNIP>
> 
> Eventually you are not performing atomic 16 bit read operations.
> 
> > But, following code does work.
> > 
> > <SNIP>
> > u16 d = flash_read16(addr);
> > udelay(1);
> > u16 t = flash_read16(addr);
> > retval = d != t;
> 
> Hm... a plain delay would probably not change this behaviour unless
> you have another problem.
> 

I agree with you. Just a simple delay should not solve this problem.
That is another reason that I think atomic read is not the cause of it.

My second workaround is strange, too.
In ARM code, mapping physical address to logical is actually does
nothing, except re-calculating pointer with sector no. and offset.
It just eating several processor cycles.

> > I don't know exactly that this is caused by specific bus access
> > mechanism, or simply chip silicon bug.
> 
> I think the problem is that for some reason flash_read16() might  not
> result  in  16bit  bus  accesses. Do you have a chance to verify this
> suspicion, for example with a logic analyzer?
> 

Currently, not available. Maybe digital scope is possible after a couple
of days, but it may be impossible to check this kind of problem with it.

> Best regards,
> 
> Wolfgang Denk
> 

[U-Boot] Generic CFI flash driver is not so generic?

[Michael Lawnick]

Choe, Hyun-ho said the following:
> My second workaround is strange, too.
> In ARM code, mapping physical address to logical is actually does
> nothing, except re-calculating pointer with sector no. and offset.
> It just eating several processor cycles.

I actually have no ARM experience, but when you talk about 'mapping' the
word 'cache' is coming to my mind. Any chance that you have not disabled
caching for flash access address space?

-- 

Michael Lawnick
Software Design Engineer

Lise-Meitner-Str. 7/1
89081 Ulm
Tel: +49 731 9533 2066

Michael.Lawnick.ext at nsn.com
http://www.nokiasiemensnetworks.com/global/

Think before you print

[U-Boot] Generic CFI flash driver is not so generic?

[Choe, Hyun-ho]

2008-11-26 (?), 16:10 +0100, Michael Lawnick ???:
> Choe, Hyun-ho said the following:
> > My second workaround is strange, too.
> > In ARM code, mapping physical address to logical is actually does
> > nothing, except re-calculating pointer with sector no. and offset.
> > It just eating several processor cycles.
> 
> I actually have no ARM experience, but when you talk about 'mapping' the
> word 'cache' is coming to my mind. Any chance that you have not disabled
> caching for flash access address space?

If you mean Instruction Cache/Data Cache, yes, ARM have them.
In CPU start-up code, DCache is disabled & ICache is enabled by default.
And always be in the same status all the time.
Turning off ICache has no effect.

(If you are interested, see cpu/arm*/start.S & cpu.c, you may already
know.)

Regards,
Choe, Hyun-ho