This is the mail archive of the
ecos-patches@sources.redhat.com
mailing list for the eCos project.
Re: New port for ARM Industrial Module AIM 711
- From: Andrew Lunn <andrew at lunn dot ch>
- To: Roland Ca?ebohm <roland dot cassebohm at VisionSystems dot de>
- Cc: John Dallaway <jld at ecoscentric dot com>, ecos-patches at ecos dot sourceware dot org
- Date: Tue, 6 Apr 2004 15:02:55 +0200
- Subject: Re: New port for ARM Industrial Module AIM 711
- References: <200403031753.44381.roland.cassebohm@visionsystems.de> <200403311503.13774.roland.cassebohm@visionsystems.de>
On Wed, Mar 31, 2004 at 03:03:13PM +0200, Roland Ca?ebohm wrote:
> Hello,
>
> does anybody had time to look at the code or tested the
> run-time code?
> Or could somebody tell me how could I help to get the
> port integrated in the public cvs tree?
Hi Roland
I've started to look at this now. I will start with the HAL
hal_arm_aim711.cdl:
A few places you are missing the e from module.
hal_platform_setup.h:
Remove the commented out instruction in CYGHWR_LED_MACRO
There is some dead code inside #if 0 which should be removed
// FIXME: Is that good?
#if CYGNUM_HAL_COMMON_INTERRUPTS_STACK_SIZE==4096
// Override default to a more sensible value
#undef CYGNUM_HAL_COMMON_INTERRUPTS_STACK_SIZE
#define CYGNUM_HAL_COMMON_INTERRUPTS_STACK_SIZE 2048
#endif
Im not so sure about this. Do other HAL do this? It seems better to do
this in CDL.
This is the only HAL with a hal_aux.h file. Is this file named
correctly? Where do other HAL put auxiliary information? Maybe in
plf_io.h?
aim711_misc.c
More code inside #if 0 to be deleted.
I don't particularly like the nameing of _period. The _ suggests is a
global variable you want to hide when in fact its a static. I also
don't get what _period is being used for. I would rename it something
more understandable, and without the _.
In hal_IRQ_handler it looks funny dividing a status by 4. A shift
operator would be more readable.
You include <string.h> but do not use anything from it as far as i can
see. You should not have this, it means you HAL is dependant on libc.
redboot_ROMRAM.ecm:
CYGDAT_REDBOOT_CUSTOM_VERSION should probably not be there. Same for
the IP address and BOOTP configuration. It seems this is your redboot
configuration, not a generic configuration a user would start from.
I've not tried to actually compile it yet.
Andrew