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

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

[bugzilla-daemon]

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

           Summary: Enable the cache on Kinetis in RAM startup mode
           Product: eCos
           Version: CVS
          Platform: All
        OS/Version: Cortex-M
            Status: NEW
          Severity: enhancement
          Priority: low
         Component: HAL
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: ilijak@siva.com.mk
                CC: ecos-bugs@ecos.sourceware.org
             Class: Advice Request


Created an attachment (id=1783)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1783)
Kinetis cache fixes 120602

Currently Kinetis HAL enables the cache only in ROM startup mode, leaving RAM
mode to Redboot. After some time I find convenient to enable the cache in RAM
startup as well. Attached patch provides for this as well as some code
robustness fixes.

What should be right behavior in RAM startup? I would appreciate comments on
this issue.

Ilija

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

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

[bugzilla-daemon]

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
----------------------------------------------------------------------------
                 CC|                            |jifl@ecoscentric.com

--- Comment #1 from Jonathan Larmour <jifl@ecoscentric.com> 2012-06-03 05:03:26 BST ---
You should use the existing standard defines in hal/common:
CYGSEM_HAL_ENABLE_DCACHE_ON_STARTUP and CYGSEM_HAL_ENABLE_ICACHE_ON_STARTUP.
Those options correctly default to 1 - we should always use caches if we can
unless the user configures otherwise.

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

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

[bugzilla-daemon]

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

--- Comment #2 from Ilija Kocho <ilijak@siva.com.mk> 2012-06-03 21:09:58 BST ---
(In reply to comment #1)
> You should use the existing standard defines in hal/common:
> CYGSEM_HAL_ENABLE_DCACHE_ON_STARTUP and CYGSEM_HAL_ENABLE_ICACHE_ON_STARTUP.
> Those options correctly default to 1 - we should always use caches if we can
> unless the user configures otherwise.

Thank you for clarification Jifl. Then, that will be the case with this patch
so I am going to commit it accompanied with appropriate ChangeLog entry.

Ilija.

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

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

[bugzilla-daemon]

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

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Attachment #1783|0                           |1
        is obsolete|                            |

--- Comment #3 from Ilija Kocho <ilijak@siva.com.mk> 2012-06-06 20:31:16 BST ---
Created an attachment (id=1795)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1795)
Kinetis cache fixes 120606-2129

Updated patch together with ChangeLog to be committed.

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

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

[bugzilla-daemon]

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

Ilija Kocho <ilijak@siva.com.mk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |CURRENTRELEASE

--- Comment #4 from Ilija Kocho <ilijak@siva.com.mk> 2012-06-09 10:34:31 BST ---
Checked in.

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

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

[bugzilla-daemon]

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.

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

[bugzilla-daemon]

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

           Summary: Enable the cache on Kinetis in RAM startup mode
           Product: eCos
           Version: CVS
          Platform: All
        OS/Version: Cortex-M
            Status: NEW
          Severity: enhancement
          Priority: low
         Component: HAL
        AssignedTo: unassigned@bugs.ecos.sourceware.org
        ReportedBy: ilijak@siva.com.mk
                CC: ecos-bugs@ecos.sourceware.org
             Class: Advice Request


Created an attachment (id=1783)
 --> (http://bugs.ecos.sourceware.org/attachment.cgi?id=1783)
Kinetis cache fixes 120602

Currently Kinetis HAL enables the cache only in ROM startup mode, leaving RAM
mode to Redboot. After some time I find convenient to enable the cache in RAM
startup as well. Attached patch provides for this as well as some code
robustness fixes.

What should be right behavior in RAM startup? I would appreciate comments on
this issue.

Ilija

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