Re: [ECOS] PowerPC HAL patches.

[Sergei Organov <osv at javad dot ru>]

Jesper,

Thank you. What you did seems to be fine for my target. I didn't try your
patch because I've already moved a little bit further in my development (FPU
support), and it seems to be simpler for me to wait until the changes appear
in anon cvs to merge them semi-automatically.

BTW, I don't think my platform is so special. The MPC50x processors have a pin 
that defines whether the vector table after reset is at 0 or at 0xfff00000, so 
designers can put ROM at 0 or at 0xfff00000. I mean that unlike other
supported PowerPC families for MPC50x we can't simply assume ROM is at
0xfff0000.

I have nothing to say against your plan for further HAL improvements. It
matches what I'd do myself. Actually I didn't implement it this way only
because I wanted to minimize required changes to the existing
variants/platforms to don't break something by mistake.

The only comment is about MMU support. Maybe we better distinguish between
two situations. The first one is when processor has MMU but we need to
disable it (the case with SIM). The second one is when the processor just
doesn't have MMU (MPC50x). For the latter case all the MMU related calls from
the HAL could be just removed. That's not very important though.

Recently I've also found a few things in the HAL that I don't understand. In
fact for me they seem to be bugs. Here are they:

1. Function 'hal_variant_init()' that is defined in most variants isn't
   actually get called. The bug doesn't show itself only because all these
   routines are currently empty.

2. 'hal_variant_load' macro isn't called in exception/interrupt management
   code while 'hal_variant_store' is. Again currently all of them are just
   empty. 

BR,
Sergei.

Jesper Skov <jskov@redhat.com> writes:
> >>>>> "Sergei" == Sergei Organov <osv@javad.ru> writes:
> 
> Sergei> Please let me know if the patch is acceptable and whether it will be
> Sergei>applied to the eCos sources.
> 
> Sergei,
> 
> I have included the essence of your patch - I rewrote it a bit:
> 
>  o Change mode of CYGHWR_HAL_POWERPC_DISABLE_MMU (so the code
>    now uses CYGHWR_HAL_POWERPC_ENABLE_MMU, while platform HALs can
>    override using CYGHWR_HAL_POWERPC_DISABLE_MMU)
> 
>  o Added CYGHWR_HAL_POWERPC_VECTOR_BASE which is either 0 or
>    0xfff00000.
> 
>  o Have variant CDL define VSR table location based on
>    CYGHWR_HAL_POWERPC_VECTOR_BASE.
> 
> While it's an improvement over what was before, I'm still not happy with
> it. IMO we want:
> 
>  o platform option which controls where RAM vector space is (i.e., the
>    place where we COPY vectors to - this is normally 0, but 0xfff00000
>    on your platform).
> 
>  o option which for ROM startup selects whether to copy vectors to RAM
>    (for performance reasons).
> 
>  o option which for RAM startup selects whether to include&copy
>    vectors to RAM (to gain full control of the target).
> 
>  o CYGHWR_HAL_POWERPC_VECTOR_BASE gets computed according to settings of
>    the above.
> 
> There's probably other variations that we'd want to cater for.
> 
> Anyway, the present CDL & vectors.S code is better than before. Except that it
> only makes sense for handling the present eCos platforms and your rather
> special platform. It deserves a proper & generic rewrite so it's capable of
> handling all cases, while at the same time making it easy for the user to
> configure...
> 
> Jesper
> 
> [diff is against the internal eCos tree so it may not apply cleanly to the CVS
> tree]