[U-Boot] [PATCH 03/15] Nokia RX-51 aka N900 support

Pali Rohár
2011-10-09T00:20:47Z

On Thursday 01 September 2011 09:57:13 you wrote:
> On Thursday, September 01, 2011 07:34:29 Pali Roh?r wrote:
> > --- /dev/null
> > +++ b/board/nokia/rx51/Makefile
> >
> > +LIB = $(obj)lib$(BOARD).a
> > +$(LIB): $(obj).depend $(OBJS)
> > + $(AR) $(ARFLAGS) $@ $(OBJS)
>
> your makefiles are old. the LIB is supposed to be a .o not .a now, and you
> need to call the link command rather than use $(AR). look at board/bf537-
> stamp/Makefile as a simple example.

ok, now I have updated makefile

>
> > --- /dev/null
> > +++ b/board/nokia/rx51/config.mk
> >
> > +TEXT_BASE = 0x80e80000
>
> pretty sure this is old and you should use a CONFIG_xxx in your board config
> header. then you can delete this config.mk file.

deleted

>
> > --- /dev/null
> > +++ b/board/nokia/rx51/rx51.c
> >
> > +GraphicDevice gdev;
>
> no camel case

GraphicDevice is defined in include/video_fb.h

>
> > +char keymap[] = {
> > + /* normal */
> > + 'q', 'o', 'p', ',', '\b', 0, 'a', 's',
> > + 'w', 'd', 'f', 'g', 'h', 'j', 'k', 'l',
> > + 'e', '.', 0, '\r', 0, 'z', 'x', 'c',
> > + 'r', 'v', 'b', 'n', 'm', ' ', 0, 0,
> > + 't', 0, 0, 0, 0, 0, 0, 0,
> > + 'y', 0, 0, 0, 0, 0, 0, 0,
> > + 'u', 0, 0, 0, 0, 0, 0, 0,
> > + 'i', 0, 0, 0, 0, 0, 0, 0,
> > + /* fn */
> > + '1', '9', '0', '=', '\b', 0, '*', '+',
> > + '2', '#', '-', '_', '(', ')', '&', '!',
> > + '3', '?', 0, '\r', 0, 0, '$', 0,
> > + '4', '/', '\\', '"', '\'', '@', 0, 0,
> > + '5', 0, 0, 0, 0, 0, 0, 0,
> > + '6', 0, 0, 0, 0, 0, 0, 0,
> > + '7', 0, 0, 0, 0, 0, 0, 0,
> > + '8', 0, 0, 0, 0, 0, 0, 0,
> > +};
> > +
> > +u8 keys[8];
> > +u8 old_keys[8] = {0, 0, 0, 0, 0, 0, 0, 0};
> > +#define KEYBUF_SIZE 4
> > +u8 keybuf[KEYBUF_SIZE];
> > +u8 keybuf_head = 0, keybuf_tail = 0;
>
> these probably all should be static. and "keymap[]" should probably also be
> const.

done

>
> > --- /dev/null
> > +++ b/board/nokia/rx51/rx51.h
> >
> > +const omap3_sysinfo sysinfo = {
>
> you should not be defining variable storage in header files

this code was derivated from board/ti/beagle/beagle.h and beagle.h still has
this code

>
> > +/*
> > + * IEN - Input Enable
> > + * IDIS - Input Disable
> > + * PTD - Pull type Down
> > + * PTU - Pull type Up
> > + * DIS - Pull type selection is inactive
> > + * EN - Pull type selection is active
> > + * M0 - Mode 0
> > + * The commented string gives the final mux configuration for that pin
> > + */
>
> this looks like SoC info and not board specific. should this be in an SoC
> header somewhere instead ?

all boards in board/ti/ has this code

> -mike

--
Pali Roh?r
pali.rohar at gmail.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20111009/3b130db6/attachment.pgp