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

[Pali Rohár]

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-0002.pgp