This is the mail archive of the ecos-bugs@sourceware.org mailing list for the eCos project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[Bug 1001606] Enable the cache on Kinetis in RAM startup mode


Please do not reply to this email. Use the web interface provided at:
http://bugs.ecos.sourceware.org/show_bug.cgi?id=1001606

Jonathan Larmour <jifl@ecoscentric.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|RESOLVED                    |REOPENED
         Resolution|CURRENTRELEASE              |
         AssignedTo|unassigned@bugs.ecos.source |jifl@ecoscentric.com
                   |ware.org                    |

--- Comment #5 from Jonathan Larmour <jifl@ecoscentric.com> 2012-06-10 01:05:12 BST ---
Hi Ilija,

Looking at this patch, I decided to look a bit closer at the Kinetis. Some
things to do with caches strike me as a bit unusual, but hopefully you can
clarify where my misunderstandings lie:

The cache is split into processor code (PC) and processor system (PS) buses.
Code accesses go through the PC bus, and data through the PS bus. Yet
hal_cache.h treats both as dcache, which doesn't seem right - PC accesses are
equivalent to the icache surely? Shouldn't we be splitting things up
appropriately between the HAL_DCACHE and HAL_ICACHE macros?

Given the division of SRAM memory into PC and PS halves, how are .2ram sections
(such as used by many flash drivers for off-chip flash) supported? They are not
mentioned in the memory layouts. FlexBus does seem to allow for off-chip flash
so these flash drivers could be used on production hardware. Should your
"code_sram" sections be instead changed into something that would work with the
existing .2ram.* section naming for consistency with the rest of eCos?

A minor thing, but does CYGPKG_HAL_KINETIS_CACHE really want to be an option?
It makes things inconsistent with CYGSEM_HAL_ENABLE_(DCACHE|ICACHE)_ON_STARTUP.
I think it is generally a safe assumption that if cache is present, users will
want to use it - the only question might be exactly at what point it becomes
enabled (and of course details about non-cacheable regions, writethru versus
writeback etc.), although developers may temporarily want to disable it if
debugging a problem which may be cache-related, but it would end up being
enabled again for production.

Not related to the particular patch, but still to do with caching
configuration... Some subsystems have their caching configuration set under
CYGPKG_HAL_KINETIS_CACHE, whereas others have them set in their own tree - or
at least SDRAM/DDR does, but I haven't looked more widely for others. And the
two key interfaces CYGINT_HAL_CACHE and CYGINT_HAL_HAS_NONCACHEABLE live under
a separate component CYGHWR_HAL_KINETIS_MEMORY_RESOURCES. I think related
options ought to be kept in the same place. OR if there's a good reason why
not, a note should be placed in the description e.g. of
CYGPKG_HAL_KINETIS_CACHE pointing users in the right direction of other
cache-related configuration options. In line with my thoughts at the top, I
think the two interfaces could be brought under CYGPKG_HAL_KINETIS_CACHE which
would then become "flavor none/no_define", with the active_if then applying to
CYGHWR_HAL_NON_CACHABLE. hal_cortexm_kinetis_conf_cache_regions() can be called
from HAL_DCACHE_ENABLE() and HAL_ICACHE_ENABLE() appropriately, so it's still
only called if actually needed (and linker garbage collection will remove it if
never called).

There are some other issues separate to the cache I have noticed too, and I may
as well mention them here (but I can submit in a new bug if you prefer). Such
as:

 - CYGHWR_HAL_KINETIS_FLEXNVM_FLASH_SIZE has the same CDL display string as
CYGHWR_HAL_KINETIS_FLASH_SIZE.

 - The descriptions of options like CYGHWR_HAL_ENET_TCD_SECTION appear to
conflict with their default settings.

 - CYGINT_HAL_CORTEXM_KINETIS_RTC has no display string.

 - It appears that CYGHWR_HAL_CORTEXM_KINETIS_FPU is effectively redundant
(since it's keyed off CYGHWR_HAL_CORTEXM_FPU), but I suspect this has now been
dealt with in your recent patches. I intend to get to those patches later this
week.

- Incidentally some of the indentation in the kinetis CDL is a bit off, 
e.g. underneath CYGHWR_HAL_KINETIS_FLEXNVM_FLASH_SIZE, at the end of
CYGPKG_HAL_CORTEXM_KINETIS_FLEXBUS and beginning of
CYGHWR_HAL_KINETIS_FLASH_CONF.

I realise it would have been better if issues like these had been raised
earlier, especially during submission of the initial port, and that's my own
fault for not really looking at things much while I was only working part-time
for eCosCentric. I'm trying to improve the balance now.

Jifl

-- 
Configure bugmail: http://bugs.ecos.sourceware.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]