[Issue 1000761] eCos support for MPC5xxx MCUs

[bugzilla-daemon at ecoscentric dot com]

http://bugzilla.ecoscentric.com/show_bug.cgi?id=1000761





--- Comment #18 from Nick Garnett <nickg@ecoscentric.com>  2009-08-05 10:54:13 ---
Created an attachment (id=766)
 --> (http://bugzilla.ecoscentric.com/attachment.cgi?id=766)
Fixed vector support

(In reply to comment #16)
> Hi,
> we have now reintegrated our port with the new PowerPC arch. We have also
> merged everything back into one Variant MPC5xxx. However we do require some
> changes to the PowerPC main architecture. Those are mandatory for our
> contribution. In detail the following things:
> 
> - cache Macro: The current arch uses a Macro HAL_UCACHE_ENABLE to differentiate
> between CPUs with unified cache versus Harvard. In all other Architectures
> thise is actually a macro for a function call. Since many of our devices use
> the e200z6 core, which does actually have a unified cache, I have renamed that
> original Macro to HAL_CACHE_IS_UCACHE and use the other Macro as in the other
> architectures

I'm not sure I understand what you are doing here. The whole point of these
macros is that they can be tested for existence and then used if they are
present. We don't need to have a separate macro that indicates whether the
first macro is defined. What a macro is defined to should have no effect on the
ifdef test.


> - default setting for PowerPC Book E - since all devices of the MPC5xxx variant
> are Book E I would like to set this for all devices of that variant

It is not a good idea to put variant specific stuff into the architecture HAL.
The intended use of the BOOK_E option is that the HAL for the appropriate
variant should have a "requires" statement for it. Otherwise we could end up
with twisty mazes of conditions in the architecture HAL.

I also don't like the CYGHWR_HAL_POWERPC_BOOK_E_e200z01 option. This should
appear as an option in the architecture HAL which the variant HALs should
require as necessary. Rather than something that gets imported from another
package. I'm also not too keen on the name of this option or the macro it
controls. Making explicit reference to a specific CPU model is not very
future-proof. I would prefer something more like
CYGHWR_HAL_POWERPC_BOOK_E_FIXED_VECTORS, and a similar change of name for the
macro. 


> 
> - Exception tables: Several of our supported devices use the e200z1, which has
> a reduced set of exception registors (IVOR Registers are missing). I build that
> into the arch.

I think that these vectors should probably go after the call to
hal_fudge_vector_alignment. I would also have defined most of this stuff in
arch.inc and just added a single macro definition in vectors.S. That way
vectors.S remains fairly clean and the option would remain for future variants
to define their own version of the macro if necessary.

I've attached a patch that more closely corresponds to what I would like to
see. Obviously, I have not tried it for any Book E targets, but it does compile
for some existing PPC targets.


-- 
Configure issuemail: http://bugzilla.ecoscentric.com/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the issue.