RE: [ECOS] HAL_FLASH_CACHES_XXX

[Jay Foster <jay at systech dot com>]

It turns out that this was the cause of the problem I was having.  My
processor (Samsung S3C4510B) is using the unified cache definitions from
flash.h.  The comment in the header file (flash.h) states that the ucache
code has not been tested yet on any target.

The HAL_FLASH_CACHES_OFF() macro for the unified cache model is not correct
for the Samsung processor (probably for any processor).  The problem is that
it invalidates the cache tag lines BEFORE disabling the cache, which creates
a race condition that can (and did in my case) lead to a cache coherency
problem.  The symptom was that the eCos scheduler would just stop running
certain tasks.

My fix was to override the HAL_FLASH_CACHES_OFF() macro in my platform
hal_cache.h file, and reverse the two lines that invalidate the cache lines
and disable the cache.  If the flash driver had indeed disabled the
interrupts as the comments in flash.h state, this race condition would not
have happened either.  By changing my HAL_FLASH_CACHES_OFF() macro, I can
solve the problem without having to disable interrupts (always preferable),
or changing the public code (there are dire warnings about changing anything
in flash.h).

Jay

-----Original Message-----
From: Jonathan Larmour [mailto:jifl@eCosCentric.com]
Sent: Tuesday, July 01, 2003 9:36 PM
To: Jay Foster
Cc: 'ecos-discuss@sources.redhat.com'; 'Jay Foster'
Subject: Re: [ECOS] HAL_FLASH_CACHES_XXX


Jay Foster wrote:
> The comments for the HAL_FLASH_CACHES_ON and HAL_FLASH_CACHES_OFF macros
in
> flash.h state that these macros must be called with interrupts disabled.
> This seems to make sense.  However, code inspection reveals that they are
> called with interrupts enabled.  I see no interrupt disabling in
> flash_dev_query(), flash_erase(), flash_program(), flash_lock() and
> flash_unlock() or in the functions that call these.  Am I missing
something
> here?  It seems unlikely that if this were a real problem that it would
have
> gone unoticed for very long.

Only in as much as that the caller should disable interrupts if needed.

The comment in flash.h is slightly exaggerating - it's generally true in 
most cases people use it, but it's possible to use this safely with 
interrupts enabled if you are running out of RAM or somewhere that isn't 
flash anyway (such that an interrupt can't touch flash, although also that 
it won't go and re-enable the cache either!). Saying you "can't" is safer, 
but actually forcing ints off is unfriendly.

The flash layer was originally only written to be used with RedBoot, not 
ecos, and this is a throwback to then, and something that will have to be 
solved properly throughout the package, not just here. And then we could 
also document the API properly too :-).

Jifl

-- 
Before posting, please read the FAQ: http://sources.redhat.com/fom/ecos
and search the list archive: http://sources.redhat.com/ml/ecos-discuss