[U-Boot] Zoom2 patch set on arm/next

Tom Tom.Rix at windriver.com
Thu May 7 23:26:12 CEST 2009


Initial Zoom2 support rebased on arm/next branch.

Runtested on Zoom2
MAKEALL arm tested.

Patchset to follow.

Tom

My notes on issues and changes.

Jean-Christophe Zoom2 base 4/29
[PATCH 1/5] ZOOM2 Add initial support for Zoom2

Please fix the whitespace present on the patch and split the merge of 
the lds
in a seperated patch

Tom :

OK

lds split out again, in the first patch.

Whitespace changes

include/configs/omap3_zoom2.h

Tabs in trailing comment

#undef CONFIG_CMD_NFS                  /* NFS support                  */

Only tabs in space between variable and comment

#define CONFIG_SYS_LONGHELP            /* undef to save memory */


Other changes
Swap refernce 7 & 8 in README.omap3
Remove ext2 and jffs2 support from config file
environment nand offset in config file matches omapzoom
Remove onenand defines from config file
Remove SDP_3430_V1 and SDP_3430_V2 per upstream changes

------

Jean-Christophe Zoom2 base 4/29
[PATCH 2/5] OMAP3 Port kernel omap gpio interface.

please specify against which kernel commit you import it

Tom : OK
Did in gpio.c and readme.

Other changes

spelling of 'derived'
remove WARNON macro.
remove unused element from gpio_bank structure

----

Jean-Christophe Zoom2 base 4/29
[PATCH 3/5] ZOOM2 Add support for debug board    detection.

 > +#define DEBUG_BOARD_CONNECTED     1
                ^^^^^
whitespace please fix

Tom : OK

 > +    int val = 0;
please add an empty line
 > > +    if (!omap_request_gpio(158)) {
 > > +        /*

Tom : OK

---

Jean-Christophe Zoom2 base 4/29
[PATCH 4/5] ZOOM2 Add serial support.

 > +
 > > +static u32 gpmc_serial_TL16CP754C[GPMC_MAX_REG] = {
 > > +    0x00011000,
 > > +    0x001F1F01,
 > > +    0x00080803,
 > > +    0x1D091D09,
 > > +    0x041D1F1F,
 > > +    0x1D0904C4, 0
 > > +};
please add a few comment about this value

Tom : TBD

 > +extern int zoom2_debug_board_connected (void);
 > > +
 > > +#define SERIAL_TL16CP754C_BASE    0x10000000      /* Zoom2 Serial 
chip address */
                          ^^^^^^
whitespace please fix
 > > +
 > > +#define QUAD_BASE_0      SERIAL_TL16CP754C_BASE
              ^^^^^^
whitespace please fix
 > > +#define QUAD_BASE_1      (SERIAL_TL16CP754C_BASE + 0x100)
              ^^^^^^
whitespace please fix
 > > +#define QUAD_BASE_2      (SERIAL_TL16CP754C_BASE + 0x200)
              ^^^^^^
whitespace please fix
 > > +#define QUAD_BASE_3      (SERIAL_TL16CP754C_BASE + 0x300)
              ^^^^^^
whitespace please fix
 > > +

Tom : OK


+struct serial_device zoom2_serial_device##n =    \
 > +{                        \
 > +    N(n),                    \
 > +    U(n),                    \
 > +    quad_init_##n,                \
 > +    quad_setbrg_##n,            \
 > +    quad_getc_##n,                \
 > +    quad_tstc_##n,                \
 > +    quad_putc_##n,                \
 > +    quad_puts_##n,                \
 > +};

I'm not a fan of this kind of define
a upgrade of the serial API and console device to allow
to provide a struct pointer will be great so we could merge this 2 API

(change not resquested for this patch)

Tom: I agree, this tends to hide what is is really happening.  I did this
after starting down a path similar to cpu/mpc5xxx/serial.c.  4 sets of
very similar functions seemed to me a worse option.

whitespace please fix
 > >  /*
 > > - * select serial console configuration
 > > + * 0 - 1 : first  USB with respect to the left edge of the debug board
 > > + * 2 - 3 : second USB with respect to the left edge of the debug board
 > >   */
 > > -#define CONFIG_CONS_INDEX        3
 > > -#define CONFIG_SYS_NS16550_COM3        OMAP34XX_UART3
 > > -#define CONFIG_SERIAL3            3    /* UART3 */
 > > +#define DEFAULT_ZOOM2_SERIAL_DEVICE (&zoom2_serial_device0)
 > > +
 > > +#define V_NS16550_CLK            (1843200)  /* 1.8432 Mhz */
            ^^^^^^^^^^^^
whitespace please fix
 > > +
 > > +#define CONFIG_SYS_NS16550
 > > +#define CONFIG_SYS_NS16550_REG_SIZE     (-2)
                      ^^^^^
whitespace please fix
 > > +#define CONFIG_SYS_NS16550_CLK          V_NS16550_CLK
                 ^^^^^^^^^^
whitespace please fix
 > > +#define CONFIG_BAUDRATE          115200
              ^^^^^^^^^^
whitespace please fix
 > > +#define CONFIG_SYS_BAUDRATE_TABLE   {115200}
                    ^^^
whitespace please fix
 > >  

Tom : OK
Sorry..
I am starting to emacs blank-mode, it helps.
http://www.emacswiki.org/emacs/BlankMode

---

Jean-Christophe Zoom2 base 4/29
[PATCH 5/5] ZOOM2 Add led support.

 > +COBJS-${CONFIG_STATUS_LED}   += led.o
$() please

Tom: Ok

Other changes,
Redundant CONFIG_STATUS_LED decoration around #include <status_led.h>
#define led gpio numbers



More information about the U-Boot mailing list