This is the mail archive of the
ecos-patches@sourceware.org
mailing list for the eCos project.
Re: fix AT91 SPI driver
Bart Veer wrote:
"Jifl" == Jonathan Larmour <jifl@eCosCentric.com> writes:
Jifl> Bart Veer wrote:
>> This fixes the AT91 SPI driver in the same way as the CortexM
>> STM32 one.
Jifl> I object to this change. It requires very recent tools (gcc
Jifl> 4.3.0+). It's acceptable for the Cortex HAL to do this as it
Jifl> is a new HAL, but not all the AT91 HALs.
Jifl> Instead I am reinstating the build of spi_at91_init.cxx, but
Jifl> ifdeffing it on the presence of CYGBLD_ATTRIB_C_INIT_PRI,
Jifl> which is only defined if GCC is recent enough. Patch is
Jifl> attached and checked in.
I did think about this. We have already changed the relevant platform
HALs to default to arm-eabi-gcc instead of arm-elf-gcc, so the default
behaviour is already to require an up-to-date compiler that provides
the necessary functionality. I expect some 3.0 testing of AT91 tools
with the new tools, but none with the old tools.
Just because something isn't tested doesn't mean it should be avoidably
broken. It's different from preventing use of old tools from working.
If anybody really does want to keep building AT91 targets with old
tools then they can continue to use old sources: either for all of
eCos, or just for the SPI driver. I really don't see this as a big
problem.
GCC 4.3.x is too recent to mandate. If it turns out there are problems
with some people's sources (maybe not exposed in our own testing) they
have no fallback of an earlier version. And they may not want to change to
the bleeding edge. Or there may be all sorts of reasons why they can't.
Partially reverting the change means there is a still an SPI driver in
the source tree which gets linker garbage collection wrong, albeit
only if built with an old compiler. I really wanted all SPI drivers to
get this right, since any of the existing drivers may get used as the
starting point for a new driver. There are other ways in which it
could have been fixed, e.g. turning spi_at91.c into spi_at91.cxx as
per the LPC driver, or something less pleasant like:
#ifdef CYGBLD_ATTRIB_C_INIT_PRI
...
#else
static void (*spi_at91_init_constructor)(void) __attribute__((unused)) CYGBLD_ATTRIB_SECTION(".ctors.33535") = &spi_at91_bus_init;
#endif
which makes the fairly safe assumption that any pre-4.3 ARM toolchains
use .ctors.
That's incompatible with the eabi. And people may be using eabi tools
older than 4.3.x. And keying off version wouldn't be a solution as we
should still be allowing people to use arm-elf if they want. arm-elf isn't
obsolete. Nor is it deprecated.
If you would like to change it in line with the LPC driver, please feel
free - that seems a fine solution.
So I would have preferred it if you had not reinstated the _init.cxx
module. But if you really think it is important, I'll live with it.
I think allowing GCC prior to 4.3.x is essential. Some people are using
earlier GCC 4. eCosCentric still don't officially ship 4.3.x yet. Some
people are using tools provided by codesourcery (particularly true for
eabi). It's a simple matter of changing a compiler prefix in comparison.
So for the record, any change which breaks older tools can be considered a
bug which wants to be fixed (unless the fix would cause problems with
newer tools and ifdeffing isn't practicable).
Jifl
--
eCosCentric Limited http://www.eCosCentric.com/ The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK. Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["The best things in life aren't things."]------ Opinions==mine