New port for ARM Industrial Module AIM 711 - Checked by AntiVir DEM

[Roland Caßebohm]

On Dienstag, 6. April 2004 16:37, Andrew Lunn wrote:
> On Tue, Apr 06, 2004 at 04:10:13PM +0200, Roland Ca?ebohm wrote:
> > Hi Andrew,
> >
> > thanks for looking.
> >
> > On Dienstag, 6. April 2004 15:02, Andrew Lunn wrote:
> > > 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.
> >
> > Oh yes, I always lost it. :-(
> >
> > > 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
> >
> > It is for having a delay between POST codes to see where it stops
> > if there is a failure.
> > I think I make a cdl option for it.
>
> OK. I just don't like dead code. It tends to accumulate with time
> unless you remove it.

Yes I understand that. :-)

>
> > > 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 _.
> >
> > All that code is a copy of the snds and e7t hal which very semilar to
> > the aim711. My intention was to make one variant hal for the s3c4510
> > controller, which are used from the different platforms.
> > But till now I haven't had the time to do this, so I have just made
> > a copy of the snds hal and changed the things which are needed for
> > the aim711.
>
> OK leave it as it is. Changing it will make it harder for somebody
> else to do a variant. Its easier to see a function is common if its
> identical.

Yes.

>
> > > 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.
> >
> > I needed memcpy(). Should I make a small loop to copy instead?
>
> I missed that. The infra package implements memcpy, so it should
> always be available. What i cannot find is a prototype in the infra
> header files. I'm not sure what you are supposed to do!

It seems like the isoinfra package which includes the string.h file
is allways used. Also if the libc package is not used.
I have searched how other hal packages make it, but most of them don't
need memcpy(), some have there own and some have just a prototype. 
I don't know what whould be the best?

>
> > I will change these things and what John suggested
> > as soon as possible and send it again to the list.
>
> For the moment please just send the HAL. We can work on the others
> once thats committed.
>
>      Andrew

OK.

Roland
-- 

___________________________________________________

VS Vision Systems GmbH, Industrial Image Processing
Dipl.-Ing. Roland Caßebohm
Aspelohe 27A, D-22848 Norderstedt, Germany
http://www.visionsystems.de
___________________________________________________