All of lore.kernel.org
 help / color / mirror / Atom feed
From: Tom <Tom.Rix@windriver.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] Zoom2 patch set on arm/next
Date: Thu, 07 May 2009 16:26:12 -0500	[thread overview]
Message-ID: <4A0351F4.4080007@windriver.com> (raw)

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

                 reply	other threads:[~2009-05-07 21:26 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4A0351F4.4080007@windriver.com \
    --to=tom.rix@windriver.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.