Soon I will be posting a rebase of omap3 zoom2 board with fixes and enhancements per comments.
Tom
Big changes.
GPIO Interface Ported from gpio code from linux. Used in zoom2 debug_board and led code. Add documenation to doc/README.omap3
LED Interface Use existing status_led interface. Add blue led to color led's. Add documentation to doc/README.LED
OMAP u-boot.lds Move this from board to cpu.
Comments
Dirk on 4/4
...
+static void zoom2_debug_board_detect (void) +{
- unsigned int val;
- /*
* GPIO to query for debug board* 158 db board query, bank 5, index 30*/- gpio_t *gpio5_base = (gpio_t *) OMAP34XX_GPIO5_BASE;
- val = __raw_readl (&gpio5_base->datain);
Is there any special reason why you use __raw_readl() instead of just readl() here? Looking at ARM's io.h, they seem to map to the same macro?
Tom: New gpio interface is used, so this is hidden. BUT looking closely at interface and __raw_readl is still being used. This was kept because I wanted the keep code as close to kernel as possible.
------------------------------------------------------------------------ Dirk 4/4
I'm not sure if I overlooked it in one of the other patches, but if not: Do you like to update
doc/README.omap3
Tom: Zoom2 information added to README.omap3 similar to other targets.
------------------------------------------------------------------------
Jean-Christophe Serial 4/2
COBJS := zoom2.o \
- debug_board.o
- debug_board.o \
- zoom2_serial.o
it could be nice to disactivate it
A : Suprising hard to do. Logic added to do this for LED. COBJS-${CONFIG_STATUS_LED} += led.o Doing something similar leaves the serial_* functions undefined. Defining a set of functions to be weak aliases for these functions does not work cleanly.
+#include "zoom2_serial.h"
+int zoom2_debug_board_connected (void);
please move to a header
A : Moved to zoom2_serial.h
- if (zoom2_debug_board_connected ()) {
NS16550_t com_port = (NS16550_t) base;int baud_divisor = CONFIG_SYS_NS16550_CLK / 16 /CONFIG_BAUDRATE;
aligning it with CONFIG_SYS_NS16550_CLK could be nice please add an empty line
A : Done
com_port->mcr = (MCR_DTR | MCR_RTS);
com_port->fcr = (FCR_FIFO_EN | FCR_RXSR | FCR_TXSR);com_port->lcr = LCR_BKSE | LCR_8N1;com_port->dll = baud_divisor & 0xff;com_port->dlm = (baud_divisor >> 8) & 0xff;com_port->lcr = LCR_8N1;
clould you explain a fe more what you do here?
A : Added a comment that this was generic setup code.
- if (zoom2_debug_board_connected ()) {
NS16550_t port = (NS16550_t) base;if (c == '\n')quad_putc_dev (base, '\r');NS16550_putc (port, c);
why not NS16550_putc ((NS16550_t) base, c);
A : Done in all the port vs. base functions.
void quad_setbrg_dev (unsigned long base)
+{
- if (zoom2_debug_board_connected ()) {
NS16550_t port = (NS16550_t) base;int clock_divisor = CONFIG_SYS_NS16550_CLK / 16 /CONFIG_BAUDRATE;NS16550_reinit (port, clock_divisor);
why don't you use the same in in the imit?
Tom: No good reason. I was using drivers/serial/serial.c as a guide and this is how it does it, with the clock_divisor function inlined.
+#define QUAD_BASE_0 SERIAL_TL16CP754C_BASE +#define QUAD_BASE_1 (SERIAL_TL16CP754C_BASE + 0x100) +#define QUAD_BASE_2 (SERIAL_TL16CP754C_BASE + 0x200) +#define QUAD_BASE_3 (SERIAL_TL16CP754C_BASE + 0x300)
why not #define QUAD_BASE(x) (SERIAL_TL16CP754C_BASE + (x << 8))
Tom: That would work too. I would prefer to keep it as is becuase the register base is less obscured.
+#define QUAD_INIT(n) \
^^^^^^^^^^^^^^^^^^^^^^^^^^^ white space please fix
+int quad_init_##n(void)
Tom: Done, converted
- enable_gpmc_config(gpmc_config,
serial_cs_base,SERIAL_TL16CP754C_BASE,GPMC_SIZE_16M);+#endif
it's board specific please move to board
Tom: Done.
-#ifdef CONFIG_OMAP
+#if defined(CONFIG_OMAP) && !defined(CONFIG_OMAP3_ZOOM2)
a config could be better
Tom: I do not use this code, the ifdef is because other omap's do.
#define ONENAND_MAP 0x20000000 /* OneNand addr */
/* (actual size small port) */+#define SERIAL_TL16CP754C_BASE 0x10000000 /* Zoom2 Serial
chip address */ it's not cpu specfic please move this to the serial driver
Tom: Done, moved to zoom2_serial.h
-#define CONFIG_CONS_INDEX 3
-#define CONFIG_SYS_NS16550_COM3 OMAP34XX_UART3 -#define CONFIG_SERIAL3 3 /* UART3 */
is the removed serial will work on the zoom2?
Tom: This, and the other UARTS, are internally connected and should not be used.
------------------------------------------------------------------------
Jean-Christophe Zoom2 base 4/2
From
- .ARM.extab : { *(.ARM.extab* .gnu.linkonce.armextab.*) }
- __exidx_start = .;
- .ARM.exidx : { *(.ARM.exidx* .gnu.linkonce.armexidx.*) }
- __exidx_end = .;
no need please remove
- . = ALIGN(4);
- .data : { *(.data) }
all the omap3 share the same lds it wil be nice to regroup it as done for the at91
Tom: Done. See OMAP-Consolidate-common-u-boot.lds-to-cpu-layer.patch
+#define CONFIG_CMD_EXT2 /* EXT2 Support */
^^^^^^^^^^^^^^^^^ white space please fix it
+#define CONFIG_CMD_FAT /* FAT support */
ditto and the followiing configs too
+#define CONFIG_CMD_JFFS2 /* JFFS2 Support */
Tom: Done, tab-ified.
+#define CONFIG_JFFS2_PART_OFFSET 0x680000
+#define CONFIG_JFFS2_PART_SIZE 0xf980000 /* size of jffs2 */
you do not active the MTD_PARTS it could be usefull
Tom: NAND related changes to follow this patch set.
+#define CONFIG_EXTRA_ENV_SETTINGS \
- "loadaddr=0x82000000\0" \
load_addr?
Tom: This whole section was removed. It came from Zoom1 which came from beagle and does not really apply to zoom2.
- */
+#define V_PROMPT "OMAP3 Zoom2# "
why? why not only define CONFIG_SYS_PROMPT?
Tom: Yes. Good point. Done.
+/*
- 2430 has 12 GP timers, they can be driven by the SysClk
(12/13/19.2) or by
- 32KHz clk, or from external sig. This rate is divided by a
local divisor.
- */
+#define V_PVT 7
as said precedently us the 12Mhz as imput with a PVT at 1 will give us a better timer
Tom: OK. This code is no longer in the config file.
------------------------------------------------------------------------
Jean-Christophe debug board 4/2
+COBJS := zoom2.o \
- debug_board.o
you do not plan to activate it via a CONFIG only?
Tom: No. The debug board is ment to be determined at run time. The board is easily detatched and single binary is needed.
+static int debug_board_connected = DEBUG_BOARD_CONNECTED;
why not set it as -1; and then avoid the first_time int?
Tom: No. I looked at this and the tradeoff would be more code, I would prefer to leave it as it but will change it if you really want this.
-------------------------------------------------------------------------
Scott Wood Zoom2 base
+#define NAND_NO_RB 1
+#define CONFIG_SYS_NAND_WP
More legacy NAND stuff. You should probably go through the config and clean out anything that isn't verifiably necessary.
Tom: I Cleaned out a lot more of the config file.