Re: [U-Boot] [PATCH 1/2] LPC2468 support
Hi Wolfgang, On Tuesday 28 April 2009, Wolfgang Denk wrote: I took a quick look at the flash driver. The main functions lpc24xx_flash_erase() and lpc24xx_write_buff() are not even referenced somewhere in this patch. They seem to be used in the 2nd patch (2/2) though. It's hard to really understand what's going on here. So this flash driver part really needs some sort of documentation how it's supposed to work. And I also think this driver (as all FLASH related drivers) should be moved into drivers/mtd instead. Into drivers/mtd? Even if it's not a MTD driver? This doesn't make mnuch sense to me. In the end this driver will be used by the common FLASH user interface (cmd_flash.c, cmd_mem.c). So I prefer to have the driver for this interface in a common place for a better overview. And the only common place I can think of is drivers/mtd right now. We should probably add a subdirectory for NOR flash drivers: drivers/mtd/nor (or drivers/mtd/maps as done in Linux). BTW: This common place will prevent those multiple platform/board specific flash drivers we have right now as well. Best regards, Stefan = DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Hi Wolfgang, On Tuesday 28 April 2009, Wolfgang Denk wrote: Into drivers/mtd? Even if it's not a MTD driver? This doesn't make mnuch sense to me. In the end this driver will be used by the common FLASH user interface (cmd_flash.c, cmd_mem.c). So I prefer to have the driver for this interface Yes. But MTD is a description of a specific interface. Even now we have two versions of the CFI driver: drivers/mtd/cfi_flash.c and drivers/mtd/cfi_mtd.c, No, we don't have two versions of the CFI driver. cfi_mtd.c is not really a driver but only a wrapper implementing the MTD infrastructure on top of the U-Boot NOR driver (cfi_flash or others). and if we look closer at it only cfi_mtd.c makes sense in this directory. cfi_flash.c is *not* MTD conformant and should be somewhere else. in a common place for a better overview. And the only common place I can think of is drivers/mtd right now. We should probably add a subdirectory for Maybe we should create drivers/flash for those non-MTD flash drivers? Then we should also move cfi_flash.c there. Yes, this could be done. Best regards, Stefan = DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
This patch includes support for the LPC2468 processor from NXP.
Signed-off-by: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
---
It now also includes support for the ethernet interface.
It does not include any changes to flash related code, until it's known
where to put it. Stefan was right that the unreferenced function are needed
by the board code.
Neither does it contain any significant changes to the interrupt code,
waiting for the patch to clean it up.
I would be gratefull if the patch can be applied before then, I can then help
clean up the code, instead of staying behind.
Concerning thumb code: As explained earlier, it is needed to call the internal
IAP functions. These are seldom used, so there is no performance penalty.
From 10ae52e03e99a2567f4b74434ba346b45c24ac02 Mon Sep 17 00:00:00 2001
From: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
Date: Fri, 24 Apr 2009 12:18:21 +0200
Subject: [PATCH] Support for LPC2468 processor from NXP
---
Makefile|3 +
cpu/arm720t/cpu.c |4 +-
cpu/arm720t/interrupts.c| 38 ++-
cpu/arm720t/lpc24xx/Makefile| 50 +++
cpu/arm720t/lpc24xx/flash.c | 236 +++
cpu/arm720t/lpc24xx/iap_entry.S |7 +
cpu/arm720t/start.S | 12 +-
drivers/net/Makefile|1 +
drivers/net/lpc2468_eth.c | 494 +++
drivers/net/lpc2468_eth.h | 159 ++
drivers/serial/Makefile |1 +
drivers/serial/serial_lpc2468.c | 119
include/asm-arm/arch-lpc24xx/hardware.h | 32 ++
include/asm-arm/arch-lpc24xx/immap.h| 406 +
include/asm-arm/config.h|4 +
include/flash.h |1 +
16 files changed, 1558 insertions(+), 9 deletions(-)
create mode 100644 cpu/arm720t/lpc24xx/Makefile
create mode 100644 cpu/arm720t/lpc24xx/flash.c
create mode 100644 cpu/arm720t/lpc24xx/iap_entry.S
create mode 100644 drivers/net/lpc2468_eth.c
create mode 100644 drivers/net/lpc2468_eth.h
create mode 100644 drivers/serial/serial_lpc2468.c
create mode 100644 include/asm-arm/arch-lpc24xx/hardware.h
create mode 100644 include/asm-arm/arch-lpc24xx/immap.h
diff --git a/Makefile b/Makefile
index e91c051..fb23ee6 100644
--- a/Makefile
+++ b/Makefile
@@ -2940,6 +2940,9 @@ B2_config : unconfig
## ARM720T Systems
#
+LPC2468_config:unconfig
+ @$(MKCONFIG) $(@:_config=) arm arm720t LPC2468 NULL lpc24xx
+
armadillo_config: unconfig
@$(MKCONFIG) $(@:_config=) arm arm720t armadillo
diff --git a/cpu/arm720t/cpu.c b/cpu/arm720t/cpu.c
index 6c40903..29e13d0 100644
--- a/cpu/arm720t/cpu.c
+++ b/cpu/arm720t/cpu.c
@@ -75,7 +75,9 @@ int cleanup_before_linux (void)
/* go to high speed */
IO_SYSCON3 = (IO_SYSCON3 ~CLKCTL) | CLKCTL_73;
#endif
-#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) ||
defined(CONFIG_LPC2292)
+#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) ||\
+ defined(CONFIG_LPC2000)
+
disable_interrupts ();
/* Nothing more needed */
#elif defined(CONFIG_INTEGRATOR) defined(CONFIG_ARCH_INTEGRATOR)
diff --git a/cpu/arm720t/interrupts.c b/cpu/arm720t/interrupts.c
index 39ed345..8671b63 100644
--- a/cpu/arm720t/interrupts.c
+++ b/cpu/arm720t/interrupts.c
@@ -29,17 +29,26 @@
#include common.h
#include clps7111.h
#include asm/proc-armv/ptrace.h
+#include asm/io.h
+#if defined(CONFIG_LPC2468)
+#include asm/arch/immap.h
+#else
#include asm/hardware.h
+#endif
#ifndef CONFIG_NETARM
+
+#if defined(CONFIG_LPC2292)
+#define TIMER_LOAD_VAL 0x
+#define READ_TIMER (0x - GET32(T0TC))
+#elif defined(CONFIG_LPC2468)
+#define TIMER_LOAD_VAL 0
+#define READ_TIMER (0x - 0xE0004008)
+#else
/* we always count down the max. */
#define TIMER_LOAD_VAL 0x
/* macro to read the 16 bit timer */
#define READ_TIMER (IO_TC1D 0x)
-
-#ifdef CONFIG_LPC2292
-#undef READ_TIMER
-#define READ_TIMER (0x - GET32(T0TC))
#endif
#else
@@ -80,6 +89,14 @@ void do_irq (struct pt_regs *pt_regs)
pfnct = (void (*)(void))VICVectAddr;
(*pfnct)();
+#elif defined(CONFIG_LPC2468)
+ void (*pfnct) (void);
+ vic_2468_t *vic = (((immap_t *)CONFIG_SYS_IMMAP)-ahb.vic);
+
+ pfnct = (void (*)(void))((vic-vicaddr));
+
+ (*pfnct) ();
+
#else
#error do_irq() not defined for this CPU type
#endif
@@ -112,6 +129,9 @@ static ulong lastdec;
int interrupt_init (void)
{
+#if defined(CONFIG_LPC2468)
+ timer_2468_t *timer0=(((immap_t *)CONFIG_SYS_IMMAP)-apb.timer0);
+#endif
#if defined(CONFIG_NETARM)
/* disable all interrupts */
@@ -185,6 +205,13 @@ int interrupt_init (void)
PUT32(T0MCR, 0);
PUT32(T0TC, 0);
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Hi Remco,
Remco Poelstra wrote:
This patch includes support for the LPC2468 processor from NXP.
Signed-off-by: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
---
It now also includes support for the ethernet interface.
It does not include any changes to flash related code, until it's known
where to put it. Stefan was right that the unreferenced function are needed
by the board code.
Neither does it contain any significant changes to the interrupt code,
waiting for the patch to clean it up.
I would be gratefull if the patch can be applied before then, I can then help
clean up the code, instead of staying behind.
Concerning thumb code: As explained earlier, it is needed to call the internal
IAP functions. These are seldom used, so there is no performance penalty.
Please break this up into orthogonal patches by functionality. I'll
comment on the net driver here, but will likely have more comments once
the major issues are taken care of :) :
snip
diff --git a/drivers/net/lpc2468_eth.c b/drivers/net/lpc2468_eth.c
new file mode 100644
index 000..b16e404
--- /dev/null
+++ b/drivers/net/lpc2468_eth.c
@@ -0,0 +1,494 @@
+/*
+ * (C) Copyright 2009 Duran Audio B.V. www.duran-audio.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ * 18-03-2009 Updated for U-boot 2009.3
+ * by Remco Poelstra remco.poelstra+u-b...@duran-audio.com
+ * Based on sample code from NXP
+ */
+
+#include config.h
+#include common.h
+#include net.h
+#include asm/io.h
+#include asm/arch/immap.h
+#include lpc2468_eth.h
+
+/**
+ * Local variables
+
*/
+
+static unsigned char macAddr[6];
+static volatile int phyAddr = 0x1000;
+static int eth_initialized = 0;
+mac_2468_t *mac=(((immap_t *)CONFIG_SYS_IMMAP)-ahb.mac);
Please encapsulate these in a 'priv' data structure. More on that later...
+
+/**
+ * Local functions
+
*/
+
+static void writePhy (u32 phyReg, u32 phyData)
+{
+
+ /* write command */
+ writel (0x0, (mac-mcmd));
+
+ /* [12:8] == PHY addr, [4:0]=0x00(BMCR) register addr */
+ writel ((phyAddr | phyReg), (mac-madr));
+ writel (phyData, (mac-mwtd));
I don't know if 'writel' and 'readl' are the correct accessors for ARM.
Hopefully somebody'll chime in on that.
+ while (readl ((mac-mind)) != 0) ;
+}
+
+static u32 readPhy (u32 phyReg)
+{
+
+ /* read command */
+ writel (0x1, (mac-mcmd));
+
+ /* [12:8] == PHY addr, [4:0]=0x00(BMCR) register addr */
+ writel ((phyAddr | phyReg), (mac-madr));
+
+ while (readl ((mac-mind)) != 0);
+
+ writel (0x0, (mac-mcmd));
+
+ return readl ((mac-mrdd));
+}
+
+static int emac_start_xmit (volatile void *buf, int length)
+{
+
+ u32 txProduceIndex = 0;
+ u32 txConsumeIndex = 0;
+ u8 *pData = 0;
+ u32 len = length;
+ u32 sendLen = 0;
+ u32 *tx_desc_addr = NULL;
+
+ txProduceIndex = readl ((mac-txproduceindex));
+ txConsumeIndex = readl ((mac-txconsumeindex));
+
+ if (txConsumeIndex != txProduceIndex) {
+ /* TODO why return here? This just means that the transmit
array isn't empty */
Good question... Please resolve before submitting.
+ printf (emac: emac_tx transmit array isn't empty\n);
+ return -1;
+ }
+
+ if (txProduceIndex == EMAC_TX_DESCRIPTOR_COUNT) {
+ /* should never happen */
+ printf (emac: emac_tx produce index == count\n);
+ }
+
+ if (len 0) {
+ pData = (u8 *) EMAC_TX_BUFFER_ADDR;
+ memcpy (pData, (void *)buf, length);
+
+ do {
+ tx_desc_addr =
+ (u32 *) (TX_DESCRIPTOR_ADDR + txProduceIndex * 8);
+
+ sendLen = len;
+ if (sendLen EMAC_BLOCK_SIZE) {
+ sendLen = EMAC_BLOCK_SIZE;
+ } else {
+ /* last fragment */
+
Re: [U-Boot] [PATCH 1/2] LPC2468 support
On Saturday 25 April 2009, Jean-Christophe PLAGNIOL-VILLARD wrote: In message 20090424215804.gc10...@game.jcrosoft.org you wrote: ... +#define COPY_BUFFER_LOCATION 0x4000fde0 evenif it's soc specific flash support I think they need to be store with the other flash and need to have the ack of Stefan No, not really. Stefan cares only about the CFI driver, and this is clearly not a CFI conformant flash type. ok but I steel wish to have Stefan opinion I took a quick look at the flash driver. The main functions lpc24xx_flash_erase() and lpc24xx_write_buff() are not even referenced somewhere in this patch. They seem to be used in the 2nd patch (2/2) though. It's hard to really understand what's going on here. So this flash driver part really needs some sort of documentation how it's supposed to work. And I also think this driver (as all FLASH related drivers) should be moved into drivers/mtd instead. Best regards, Stefan = DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-0 Fax: +49-8142-66989-80 Email: off...@denx.de = ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Dear Stefan Roese, In message 200904270927.29232...@denx.de you wrote: I took a quick look at the flash driver. The main functions lpc24xx_flash_erase() and lpc24xx_write_buff() are not even referenced somewhere in this patch. They seem to be used in the 2nd patch (2/2) though. It's hard to really understand what's going on here. So this flash driver part really needs some sort of documentation how it's supposed to work. And I also think this driver (as all FLASH related drivers) should be moved into drivers/mtd instead. Into drivers/mtd? Even if it's not a MTD driver? This doesn't make mnuch sense to me. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Politics: A strife of interests masquerading as a contest of principles. The conduct of public affairs for private advantage. - Ambrose Bierce ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
This patch includes support for the LPC2468 processor from NXP.
Signed-off-by: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
---
A working board example will be submitted when this patch is found to be OK.
This patch is against latest git.
The previous problem with PUTx vs. writex is solved.
From 75361079ed78fb40c2840b3bd74687153e486620 Mon Sep 17 00:00:00 2001
From: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
Date: Fri, 24 Apr 2009 12:18:21 +0200
Subject: [PATCH] Support for LPC2468 processor from NXP
---
Makefile|3 +
cpu/arm720t/cpu.c |2 +-
cpu/arm720t/interrupts.c| 37 +++-
cpu/arm720t/lpc24xx/Makefile| 50 +
cpu/arm720t/lpc24xx/flash.c | 233
cpu/arm720t/lpc24xx/iap_entry.S |7 +
cpu/arm720t/start.S | 11 +-
drivers/serial/Makefile |1 +
drivers/serial/serial_lpc2468.c | 119 +++
include/asm-arm/arch-lpc24xx/hardware.h | 32 +++
include/asm-arm/arch-lpc24xx/immap.h| 351 +++
include/asm-arm/config.h|4 +
include/flash.h |1 +
13 files changed, 842 insertions(+), 9 deletions(-)
create mode 100644 cpu/arm720t/lpc24xx/Makefile
create mode 100644 cpu/arm720t/lpc24xx/flash.c
create mode 100644 cpu/arm720t/lpc24xx/iap_entry.S
create mode 100644 drivers/serial/serial_lpc2468.c
create mode 100644 include/asm-arm/arch-lpc24xx/hardware.h
create mode 100644 include/asm-arm/arch-lpc24xx/immap.h
diff --git a/Makefile b/Makefile
index e91c051..fb23ee6 100644
--- a/Makefile
+++ b/Makefile
@@ -2940,6 +2940,9 @@ B2_config : unconfig
## ARM720T Systems
#
+LPC2468_config:unconfig
+ @$(MKCONFIG) $(@:_config=) arm arm720t LPC2468 NULL lpc24xx
+
armadillo_config: unconfig
@$(MKCONFIG) $(@:_config=) arm arm720t armadillo
diff --git a/cpu/arm720t/cpu.c b/cpu/arm720t/cpu.c
index 6c40903..b3a2853 100644
--- a/cpu/arm720t/cpu.c
+++ b/cpu/arm720t/cpu.c
@@ -75,7 +75,7 @@ int cleanup_before_linux (void)
/* go to high speed */
IO_SYSCON3 = (IO_SYSCON3 ~CLKCTL) | CLKCTL_73;
#endif
-#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) ||
defined(CONFIG_LPC2292)
+#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) ||
defined(CONFIG_LPC2000)
disable_interrupts ();
/* Nothing more needed */
#elif defined(CONFIG_INTEGRATOR) defined(CONFIG_ARCH_INTEGRATOR)
diff --git a/cpu/arm720t/interrupts.c b/cpu/arm720t/interrupts.c
index 39ed345..d7aec27 100644
--- a/cpu/arm720t/interrupts.c
+++ b/cpu/arm720t/interrupts.c
@@ -29,17 +29,26 @@
#include common.h
#include clps7111.h
#include asm/proc-armv/ptrace.h
+#include asm/io.h
+#if defined(CONFIG_LPC2468)
+#include asm/arch/immap.h
+#else
#include asm/hardware.h
+#endif
#ifndef CONFIG_NETARM
+
+#if defined(CONFIG_LPC2292)
+#define TIMER_LOAD_VAL 0x
+#define READ_TIMER (0x - GET32(T0TC))
+#elif defined(CONFIG_LPC2468)
+#define TIMER_LOAD_VAL 0
+#define READ_TIMER (0x - 0xE0004008)
+#else
/* we always count down the max. */
#define TIMER_LOAD_VAL 0x
/* macro to read the 16 bit timer */
#define READ_TIMER (IO_TC1D 0x)
-
-#ifdef CONFIG_LPC2292
-#undef READ_TIMER
-#define READ_TIMER (0x - GET32(T0TC))
#endif
#else
@@ -80,6 +89,14 @@ void do_irq (struct pt_regs *pt_regs)
pfnct = (void (*)(void))VICVectAddr;
(*pfnct)();
+#elif defined(CONFIG_LPC2468)
+ void (*pfnct) (void);
+ vic_2468_t *vic = (((immap_t *)CONFIG_SYS_IMMAP)-ahb.vic);
+
+ pfnct = (void (*)(void))((vic-vicaddr));
+
+ (*pfnct) ();
+
#else
#error do_irq() not defined for this CPU type
#endif
@@ -112,6 +129,9 @@ static ulong lastdec;
int interrupt_init (void)
{
+#if defined(CONFIG_LPC2468)
+ timer_2468_t *timer0=(((immap_t *)CONFIG_SYS_IMMAP)-apb.timer0);
+#endif
#if defined(CONFIG_NETARM)
/* disable all interrupts */
@@ -185,6 +205,13 @@ int interrupt_init (void)
PUT32(T0MCR, 0);
PUT32(T0TC, 0);
PUT32(T0TCR, 1);/* enable timer0 */
+#elif defined(CONFIG_LPC2468)
+ writel (0, (timer0-ir)); /*disable all timer0 interupts
*/
+ writel (0, (timer0-tcr)); /*disable timer0 */
+ writel (CFG_SYS_CLK_FREQ / CONFIG_SYS_HZ - 1, (timer0-pr));
+ writel (0, (timer0-mcr));
+ writel (0, (timer0-tc));
+ writel (1, (timer0-tcr));
#else
#error No interrupt_init() defined for this CPU type
@@ -201,7 +228,7 @@ int interrupt_init (void)
*/
-#if defined(CONFIG_IMPA7) || defined(CONFIG_EP7312) || defined(CONFIG_NETARM)
|| defined(CONFIG_ARMADILLO) || defined(CONFIG_LPC2292)
+#if defined(CONFIG_IMPA7) || defined(CONFIG_EP7312) ||
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Dear Remco Poelstra,
In message 49f1a92f.9070...@duran-audio.com you wrote:
This patch includes support for the LPC2468 processor from NXP.
Thanks.
Here a few comments:
--- a/cpu/arm720t/cpu.c
+++ b/cpu/arm720t/cpu.c
@@ -75,7 +75,7 @@ int cleanup_before_linux (void)
/* go to high speed */
IO_SYSCON3 = (IO_SYSCON3 ~CLKCTL) | CLKCTL_73;
#endif
-#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) ||
defined(CONFIG_LPC2292)
+#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) ||
defined(CONFIG_LPC2000)
Line too long.
diff --git a/cpu/arm720t/interrupts.c b/cpu/arm720t/interrupts.c
index 39ed345..d7aec27 100644
--- a/cpu/arm720t/interrupts.c
+++ b/cpu/arm720t/interrupts.c
@@ -29,17 +29,26 @@
#include common.h
#include clps7111.h
#include asm/proc-armv/ptrace.h
+#include asm/io.h
+#if defined(CONFIG_LPC2468)
+#include asm/arch/immap.h
+#else
#include asm/hardware.h
+#endif
Would it be possible to keep a generic ``#include asm/hardware.h''
here and add the processor specific stuff there?
#ifndef CONFIG_NETARM
+
+#if defined(CONFIG_LPC2292)
+#define TIMER_LOAD_VAL 0x
+#define READ_TIMER (0x - GET32(T0TC))
+#elif defined(CONFIG_LPC2468)
+#define TIMER_LOAD_VAL 0
+#define READ_TIMER (0x - 0xE0004008)
+#else
Can we please move this out of a common source file?
-#if defined(CONFIG_IMPA7) || defined(CONFIG_EP7312) ||
defined(CONFIG_NETARM) || defined(CONFIG_ARMADILLO) || defined(CONFIG_LPC2292)
+#if defined(CONFIG_IMPA7) || defined(CONFIG_EP7312) ||
defined(CONFIG_NETARM) || defined(CONFIG_ARMADILLO) || defined(CONFIG_LPC2000)
Line too long.
--- a/cpu/arm720t/start.S
+++ b/cpu/arm720t/start.S
...
@@ -368,6 +368,11 @@ lock_loop:
ldr r0, VPBDIV_ADR
mov r1, #0x01 /* VPB clock is same as process clock */
str r1, [r0]
+#elif defined(CONFIG_LPC2468)
+ldr r0, =0x40008000 /*0x4000 is internal SRAM,
+ 0x4000 is end of SRAM*/
+mov sp,r0
+sub sl,sp,#0x2000
Indentation not by TAB
diff --git a/drivers/serial/serial_lpc2468.c b/drivers/serial/serial_lpc2468.c
new file mode 100644
index 000..4b8f241
--- /dev/null
+++ b/drivers/serial/serial_lpc2468.c
@@ -0,0 +1,119 @@
Do we really need a new file here?
diff --git a/include/asm-arm/arch-lpc24xx/immap.h
b/include/asm-arm/arch-lpc24xx/immap.h
new file mode 100644
index 000..d37e4f1
--- /dev/null
+++ b/include/asm-arm/arch-lpc24xx/immap.h
@@ -0,0 +1,351 @@
+/*
+ * (C) Copyright 2009 Duran Audio B.V.
+ *
+ * LPC2468 Internal Memory Map
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ *
+ */
+
+#ifndef __LPC24XX_IMMAP_H
+#define __LPC24XX_IMMAP_H
+
+#include asm/types.h
+#include config.h
+
+typedef struct watchdog_2468 {
+u8 fixme[0x4000];
+} watchdog2468_t;
+
+typedef struct timer_2468 {
+u32 ir;
+u32 tcr;
+u32 tc;
+u32 pr;
+u32 pc;
+u32 mcr;
+u32 mr0;
+u32 mr1;
+u32 mr2;
+u32 mr3;
+u32 ccr;
+u32 cr0;
+u32 cr1;
+u32 cr2;
+u32 cr3;
+u32 emr;
+u32 ctcr;
+u8 notused[0x3fbc];
+} timer_2468_t;
Indentation not by TAB (whole file!).
How about adding some comments to explain what all these fields mean?
(whole file!)
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Of course there's no reason for it, it's just our policy.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
On 13:57 Fri 24 Apr , Remco Poelstra wrote:
This patch includes support for the LPC2468 processor from NXP.
Signed-off-by: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
---
A working board example will be submitted when this patch is found to be OK.
This patch is against latest git.
The previous problem with PUTx vs. writex is solved.
two general news
I'm preparing some patch to cleanup the arm720t include LPC irq support
I'll send it asap
please take a look and test it
From 75361079ed78fb40c2840b3bd74687153e486620 Mon Sep 17 00:00:00 2001
From: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
Date: Fri, 24 Apr 2009 12:18:21 +0200
Subject: [PATCH] Support for LPC2468 processor from NXP
---
Makefile|3 +
cpu/arm720t/cpu.c |2 +-
cpu/arm720t/interrupts.c| 37 +++-
cpu/arm720t/lpc24xx/Makefile| 50 +
cpu/arm720t/lpc24xx/flash.c | 233
cpu/arm720t/lpc24xx/iap_entry.S |7 +
cpu/arm720t/start.S | 11 +-
drivers/serial/Makefile |1 +
drivers/serial/serial_lpc2468.c | 119 +++
include/asm-arm/arch-lpc24xx/hardware.h | 32 +++
include/asm-arm/arch-lpc24xx/immap.h| 351
+++
include/asm-arm/config.h|4 +
include/flash.h |1 +
13 files changed, 842 insertions(+), 9 deletions(-)
create mode 100644 cpu/arm720t/lpc24xx/Makefile
create mode 100644 cpu/arm720t/lpc24xx/flash.c
create mode 100644 cpu/arm720t/lpc24xx/iap_entry.S
create mode 100644 drivers/serial/serial_lpc2468.c
create mode 100644 include/asm-arm/arch-lpc24xx/hardware.h
create mode 100644 include/asm-arm/arch-lpc24xx/immap.h
diff --git a/Makefile b/Makefile
index e91c051..fb23ee6 100644
--- a/Makefile
+++ b/Makefile
@@ -2940,6 +2940,9 @@ B2_config : unconfig
## ARM720T Systems
#
+LPC2468_config: unconfig
+ @$(MKCONFIG) $(@:_config=) arm arm720t LPC2468 NULL lpc24xx
+
armadillo_config: unconfig
@$(MKCONFIG) $(@:_config=) arm arm720t armadillo
diff --git a/cpu/arm720t/cpu.c b/cpu/arm720t/cpu.c
index 6c40903..b3a2853 100644
--- a/cpu/arm720t/cpu.c
+++ b/cpu/arm720t/cpu.c
@@ -75,7 +75,7 @@ int cleanup_before_linux (void)
/* go to high speed */
IO_SYSCON3 = (IO_SYSCON3 ~CLKCTL) | CLKCTL_73;
#endif
-#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) ||
defined(CONFIG_LPC2292)
+#elif defined(CONFIG_NETARM) || defined(CONFIG_S3C4510B) ||
defined(CONFIG_LPC2000)
disable_interrupts ();
/* Nothing more needed */
#elif defined(CONFIG_INTEGRATOR) defined(CONFIG_ARCH_INTEGRATOR)
diff --git a/cpu/arm720t/interrupts.c b/cpu/arm720t/interrupts.c
index 39ed345..d7aec27 100644
--- a/cpu/arm720t/interrupts.c
+++ b/cpu/arm720t/interrupts.c
@@ -29,17 +29,26 @@
#include common.h
#include clps7111.h
#include asm/proc-armv/ptrace.h
+#include asm/io.h
+#if defined(CONFIG_LPC2468)
+#include asm/arch/immap.h
+#else
#include asm/hardware.h
+#endif
#ifndef CONFIG_NETARM
+
+#if defined(CONFIG_LPC2292)
+#define TIMER_LOAD_VAL 0x
+#define READ_TIMER (0x - GET32(T0TC))
+#elif defined(CONFIG_LPC2468)
+#define TIMER_LOAD_VAL 0
+#define READ_TIMER (0x - 0xE0004008)
+#else
/* we always count down the max. */
#define TIMER_LOAD_VAL 0x
/* macro to read the 16 bit timer */
#define READ_TIMER (IO_TC1D 0x)
-
-#ifdef CONFIG_LPC2292
-#undef READ_TIMER
-#define READ_TIMER (0x - GET32(T0TC))
#endif
#else
@@ -80,6 +89,14 @@ void do_irq (struct pt_regs *pt_regs)
pfnct = (void (*)(void))VICVectAddr;
(*pfnct)();
+#elif defined(CONFIG_LPC2468)
+ void (*pfnct) (void);
+ vic_2468_t *vic = (((immap_t *)CONFIG_SYS_IMMAP)-ahb.vic);
+
+ pfnct = (void (*)(void))((vic-vicaddr));
+
+ (*pfnct) ();
+
#else
#error do_irq() not defined for this CPU type
#endif
@@ -112,6 +129,9 @@ static ulong lastdec;
int interrupt_init (void)
{
+#if defined(CONFIG_LPC2468)
+ timer_2468_t *timer0=(((immap_t *)CONFIG_SYS_IMMAP)-apb.timer0);
+#endif
#if defined(CONFIG_NETARM)
/* disable all interrupts */
@@ -185,6 +205,13 @@ int interrupt_init (void)
PUT32(T0MCR, 0);
PUT32(T0TC, 0);
PUT32(T0TCR, 1);/* enable timer0 */
+#elif defined(CONFIG_LPC2468)
+ writel (0, (timer0-ir)); /*disable all timer0 interupts
*/
+ writel (0, (timer0-tcr)); /*disable timer0 */
+ writel (CFG_SYS_CLK_FREQ / CONFIG_SYS_HZ - 1, (timer0-pr));
+ writel (0, (timer0-mcr));
+ writel (0, (timer0-tc));
+ writel (1, (timer0-tcr));
#else
#error No
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Dear Jean-Christophe PLAGNIOL-VILLARD, In message 20090424215804.gc10...@game.jcrosoft.org you wrote: ... +#define COPY_BUFFER_LOCATION 0x4000fde0 evenif it's soc specific flash support I think they need to be store with the other flash and need to have the ack of Stefan No, not really. Stefan cares only about the CFI driver, and this is clearly not a CFI conformant flash type. Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de The typical page layout program is nothing more than an electronic light table for cutting and pasting documents. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Jean-Christophe PLAGNIOL-VILLARD schreef:
No, thumb code is less efficient in terms of performance, but this
single file needs thumb code. See LPC2292.
what is the Difference?
until a real big gap please do not use thumb
IAP entries need thumb code. This is not a problem, they are only used
for programming the internal flash of the processor, so there is no
performance penalty for normal operation.
Yes I do. They are straight from the LPC2292 code, so once they were
considered OK. I checked out the the write{s,l,b} functions in asm/io.h,
but although they look similar, for some reason they simply don't work.
Given the similarities between the write{s,l,b} and the PUT* functions,
what is the problem with those? Furthermore, the ARM architecture
doesn't use any kind of special instructions for accessing registers,
everything is memory mapped.
please use readx/writex
Thanks for the pointer, I'll look into those.
I do understand that you want the best code for U-boot, but I do not
entirely agree on all points. Certainly when I look at the code already
in place in U-boot.
I'm preparing a patch series to clean the arm720t to seprate it as arch
and avoid this borring #ifdef
Can you explain what you mean here?
Kind regards,
Remco Poelstra
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Jean-Christophe PLAGNIOL-VILLARD schreef: I'm not an expert in assembly, but at first I had immap.h included in start.S and it complained about invalid instructions, so if I combine hardware.h and immap.h, then there must be some way of making sure that the assembler ignores the C code in immap.h. Do you know of any such thing? immap.h? immap.h is like hardware.h for the LPC2292, but it contains C structures to define the registers instead of #defines. I based it on the ppc code. See the patch for an explanation. I can remove the file, but than I need to put an #ifdef construct in start.S to only exclude it in the lpc2468 case. The file is used by the other ARM ports. I can also simply empty it, but in this way it is more similar to the other ports. What would you like? no please do not I'll prefer to separate arch file I do not understand what you mean with the last part of your comment. Kind regards, Remco Poelstra ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Remco Poelstra schreef:
Yes I do. They are straight from the LPC2292 code, so once they were
considered OK. I checked out the the write{s,l,b} functions in asm/io.h,
but although they look similar, for some reason they simply don't work.
Given the similarities between the write{s,l,b} and the PUT* functions,
what is the problem with those? Furthermore, the ARM architecture
doesn't use any kind of special instructions for accessing registers,
everything is memory mapped.
please use readx/writex
Thanks for the pointer, I'll look into those.
I can't find these functions/macros, but I think you mean the
write{b,s,l} macros. As I already indicated in a previous e-mail, they
do not work in my code. If I replace, e.g. the PUT32 with writel than
the code doesn't run. I will look into that problem after I've finished
another project at work.
Kind regards,
Remco Poelstra
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
--- u-boot-orig/cpu/arm720t/lpc24xx/Makefile 1970-01-01
01:00:00.0 +0100
+++ u-boot-cleanup/cpu/arm720t/lpc24xx/Makefile2009-03-19
10:56:53.0 +0100
...
+$(SOBJS):
+ $(CC) $(AFLAGS) -march=armv4t -c -o $(SOBJS) iap_entry.S
Such compile options hsould probably be set globally, not just for
this single source file?
No, thumb code is less efficient in terms of performance, but this
single file needs thumb code. See LPC2292.
what is the Difference?
until a real big gap please do not use thumb
diff -upNr u-boot-orig/cpu/arm720t/start.S
u-boot-cleanup/cpu/arm720t/start.S
--- u-boot-orig/cpu/arm720t/start.S2009-03-18 00:42:12.0
+0100
+++ u-boot-cleanup/cpu/arm720t/start.S 2009-03-24 11:52:35.0
+0100
@@ -127,7 +127,7 @@ reset:
bl cpu_init_crit
#endif
-#ifdef CONFIG_LPC2292
+#if defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
I'm sorry, it is combined in this case, no?
#else
#error No cpu_init_crit() defined for current CPU type
#endif
@@ -383,7 +387,7 @@ lock_loop:
str r1, [r0]
#endif
-#ifndef CONFIG_LPC2292
+#if !defined(CONFIG_LPC2292) !defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
Same here.
mov ip, lr
/*
* before relocating, we have to setup RAM timing
@@ -601,7 +605,7 @@ reset_cpu:
* on external peripherals such as watchdog timers, etc. */
#elif defined(CONFIG_INTEGRATOR) defined(CONFIG_ARCH_INTEGRATOR)
/* No specific reset actions for IntegratorAP/CM720T as yet */
-#elif defined(CONFIG_LPC2292)
+#elif defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
Same here.
.align 5
.globl reset_cpu
reset_cpu:
diff -upNr u-boot-orig/include/asm-arm/arch-lpc24xx/hardware.h
u-boot-cleanup/include/asm-arm/arch-lpc24xx/hardware.h
--- u-boot-orig/include/asm-arm/arch-lpc24xx/hardware.h1970-01-01
01:00:00.0 +0100
+++ u-boot-cleanup/include/asm-arm/arch-lpc24xx/hardware.h 2009-03-24
11:54:44.0 +0100
@@ -0,0 +1,32 @@
+#ifndef __ASM_ARCH_HARDWARE_H
+#define __ASM_ARCH_HARDWARE_H
+
+/*
+ * Copyright (c) 2004 Cucy Systems (http://www.cucy.com)
+ * Curt Brune c...@cucy.com
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#if defined(CONFIG_LPC2468)
+#else
+#error No hardware file defined for this configuration
+#endif
+
+#endif /* __ASM_ARCH_HARDWARE_H */
Ummm... What exactly is this file needed for?
I don't need it, but start.S wants to include it. See my comment about
the #ifdef's. Other architectures left it empty too, so it seemed the
best option to me.
+/* Macros for reading/writing registers */
+#define PUT8(reg, value) (*(volatile unsigned char*)(reg) = (value))
+#define PUT16(reg, value) (*(volatile unsigned short*)(reg) = (value))
+#define PUT32(reg, value) (*(volatile unsigned int*)(reg) = (value))
+#define GET8(reg) (*(volatile unsigned char*)(reg))
+#define GET16(reg) (*(volatile unsigned short*)(reg))
+#define GET32(reg) (*(volatile unsigned int*)(reg))
Do you clain these are proper accessor functions for your processor?
Yes I do. They are straight from the LPC2292 code, so once they were
considered OK. I checked out the the write{s,l,b} functions in asm/io.h,
but although they look similar, for some reason they simply don't work.
Given the similarities between the write{s,l,b} and the PUT* functions,
what is the problem with those? Furthermore, the ARM architecture
doesn't use any kind of special instructions for accessing registers,
everything is memory mapped.
please use readx/writex
I do understand that you want the best code for U-boot, but I do not
entirely agree on all points. Certainly when I look at the code already
in place in U-boot.
I'm preparing a patch series to clean the arm720t to seprate it as arch
and avoid this borring #ifdef
Best Regards,
J.
Re: [U-Boot] [PATCH 1/2] LPC2468 support
On 11:06 Wed 25 Mar , Remco Poelstra wrote: Wolfgang Denk schreef: Dear Remco Poelstra, Is there no way we can do without such a #ifdef here? The problem is that start.S needs hardware.h, but the code in immap.h should not be included in start.S, so I can not merge hardware.h and immap.h Why not? I'm not aware of such a restriction? I'm not an expert in assembly, but at first I had immap.h included in start.S and it complained about invalid instructions, so if I combine hardware.h and immap.h, then there must be some way of making sure that the assembler ignores the C code in immap.h. Do you know of any such thing? immap.h? I would like to avoid the ever growing list of #if defined(this) || defined(that) || defined(...) || ... Maybe we can have a common #define that covers the common case? I could add something like #if defined(CONFIG_LPC2922) || defined(CONFIG_LPC2468) #define (CONFIG_LPC2000) #endif in a general place and then use CONFIG_LPC2000 at the common places. The problem I then have is: What would be the best place to put such define? Preferably it is automatically included also for the LPC2292 code. If that's not possible, it can be defined in the board config, but I think that leads to confusion. What's your opinion? Maybe in include/asm-arm/config.h Ummm... What exactly is this file needed for? I don't need it, but start.S wants to include it. See my comment about the #ifdef's. Other architectures left it empty too, so it seemed the best option to me. Hm... that doesn't really make sense to me. Also, the error checking in this file makes little sense to me. I can remove the file, but than I need to put an #ifdef construct in start.S to only exclude it in the lpc2468 case. The file is used by the other ARM ports. I can also simply empty it, but in this way it is more similar to the other ports. What would you like? no please do not I'll prefer to separate arch file Best Regards; J. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Wolfgang Denk schreef: Dear Remco Poelstra, In message 49c8be7a.10...@duran-audio.com you wrote: This patch includes support for the LPC2468 processor from NXP. The example board will follow when this patch is OK. Such a comment does not belong into the commit message. Please mode it below the --- line. I see, I thought nothing else was allowed below the --- Signed-off-by: Remco Poelstra remco.poelstra+u-b...@duran-audio.com --- diff -upNr u-boot-orig/cpu/arm720t/interrupts.c u-boot-cleanup/cpu/arm720t/interrupts.c --- u-boot-orig/cpu/arm720t/interrupts.c 2009-03-18 00:42:12.0 +0100 +++ u-boot-cleanup/cpu/arm720t/interrupts.c 2009-03-24 11:48:50.0 +0100 @@ -29,7 +29,11 @@ #include common.h #include clps7111.h #include asm/proc-armv/ptrace.h +#if defined(CONFIG_LPC2468) +#include asm/arch/immap.h +#else #include asm/hardware.h +#endif Is there no way we can do without such a #ifdef here? The problem is that start.S needs hardware.h, but the code in immap.h should not be included in start.S, so I can not merge hardware.h and immap.h #ifndef CONFIG_NETARM /* we always count down the max. */ @@ -40,6 +44,11 @@ #ifdef CONFIG_LPC2292 #undef READ_TIMER #define READ_TIMER (0x - GET32(T0TC)) +#elif defined(CONFIG_LPC2468) +#undef TIMER_LOAD_VAL +#define TIMER_LOAD_VAL 0 +#undef READ_TIMER +#define READ_TIMER (0x - 0xE0004008) NAK. When you have to #unifdef existing variable definitions, then ther eis something fundamentally wrong. Please fix this problem at the cause, i. e. where the wroing values are defined. I'll look into this. #endif #else @@ -80,6 +89,14 @@ void do_irq (struct pt_regs *pt_regs) pfnct = (void (*)(void))VICVectAddr; (*pfnct)(); +#elif defined(CONFIG_LPC2468) +void (*pfnct) (void); +vic_2468_t *vic = (((immap_t *)CONFIG_SYS_IMMAP)-ahb.vic); + +pfnct = (void (*)(void))((vic-vicaddr)); + +(*pfnct) (); Is there no way to combine this code with the one for the LPC2292? It doesn't look that different to me... Interesting point. I was wondering the same. The problem lies in the fact that you want this patch to use C data structures, while the LPC2292 code uses offset lists. I can not convert the LPC2292 code to C structures, since a) I can not test the code b) I get paid to design hardware, not getting my ports published. I'm doing this to give something back to the community, since I really appreciate the work done by other OSS developers. But I can not spend time on converting complete other architectures. I leave that to other (the original LPC2292?) developers. --- u-boot-orig/cpu/arm720t/lpc24xx/Makefile 1970-01-01 01:00:00.0 +0100 +++ u-boot-cleanup/cpu/arm720t/lpc24xx/Makefile 2009-03-19 10:56:53.0 +0100 ... +$(SOBJS): +$(CC) $(AFLAGS) -march=armv4t -c -o $(SOBJS) iap_entry.S Such compile options hsould probably be set globally, not just for this single source file? No, thumb code is less efficient in terms of performance, but this single file needs thumb code. See LPC2292. diff -upNr u-boot-orig/cpu/arm720t/start.S u-boot-cleanup/cpu/arm720t/start.S --- u-boot-orig/cpu/arm720t/start.S 2009-03-18 00:42:12.0 +0100 +++ u-boot-cleanup/cpu/arm720t/start.S 2009-03-24 11:52:35.0 +0100 @@ -127,7 +127,7 @@ reset: bl cpu_init_crit #endif -#ifdef CONFIG_LPC2292 +#if defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468) Is there no way to combine this code with the one for the LPC2292? I'm sorry, it is combined in this case, no? #else #error No cpu_init_crit() defined for current CPU type #endif @@ -383,7 +387,7 @@ lock_loop: str r1, [r0] #endif -#ifndef CONFIG_LPC2292 +#if !defined(CONFIG_LPC2292) !defined(CONFIG_LPC2468) Is there no way to combine this code with the one for the LPC2292? Same here. mov ip, lr /* * before relocating, we have to setup RAM timing @@ -601,7 +605,7 @@ reset_cpu: * on external peripherals such as watchdog timers, etc. */ #elif defined(CONFIG_INTEGRATOR) defined(CONFIG_ARCH_INTEGRATOR) /* No specific reset actions for IntegratorAP/CM720T as yet */ -#elif defined(CONFIG_LPC2292) +#elif defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468) Is there no way to combine this code with the one for the LPC2292? Same here. .align 5 .globl reset_cpu reset_cpu: diff -upNr u-boot-orig/include/asm-arm/arch-lpc24xx/hardware.h u-boot-cleanup/include/asm-arm/arch-lpc24xx/hardware.h --- u-boot-orig/include/asm-arm/arch-lpc24xx/hardware.h 1970-01-01 01:00:00.0 +0100 +++ u-boot-cleanup/include/asm-arm/arch-lpc24xx/hardware.h 2009-03-24 11:54:44.0 +0100 @@ -0,0 +1,32 @@ +#ifndef __ASM_ARCH_HARDWARE_H +#define __ASM_ARCH_HARDWARE_H + +/* + * Copyright (c) 2004 Cucy Systems (http://www.cucy.com) + * Curt
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Dear Remco Poelstra,
In message 49c9eb4d.5050...@duran-audio.com you wrote:
--- u-boot-orig/cpu/arm720t/interrupts.c 2009-03-18 00:42:12.0
+0100
+++ u-boot-cleanup/cpu/arm720t/interrupts.c2009-03-24
11:48:50.0 +0100
@@ -29,7 +29,11 @@
#include common.h
#include clps7111.h
#include asm/proc-armv/ptrace.h
+#if defined(CONFIG_LPC2468)
+#include asm/arch/immap.h
+#else
#include asm/hardware.h
+#endif
Is there no way we can do without such a #ifdef here?
The problem is that start.S needs hardware.h, but the code in immap.h
should not be included in start.S, so I can not merge hardware.h and immap.h
Why not? I'm not aware of such a restriction?
diff -upNr u-boot-orig/cpu/arm720t/start.S
u-boot-cleanup/cpu/arm720t/start.S
--- u-boot-orig/cpu/arm720t/start.S2009-03-18 00:42:12.0
+0100
+++ u-boot-cleanup/cpu/arm720t/start.S 2009-03-24 11:52:35.0
+0100
@@ -127,7 +127,7 @@ reset:
bl cpu_init_crit
#endif
-#ifdef CONFIG_LPC2292
+#if defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
I'm sorry, it is combined in this case, no?
I would like to avoid the ever growing list of
#if defined(this) || defined(that) || defined(...) || ...
Maybe we can have a common #define that covers the common case?
diff -upNr u-boot-orig/include/asm-arm/arch-lpc24xx/hardware.h
u-boot-cleanup/include/asm-arm/arch-lpc24xx/hardware.h
--- u-boot-orig/include/asm-arm/arch-lpc24xx/hardware.h1970-01-01
01:00:00.0 +0100
+++ u-boot-cleanup/include/asm-arm/arch-lpc24xx/hardware.h 2009-03-24
11:54:44.0 +0100
@@ -0,0 +1,32 @@
+#ifndef __ASM_ARCH_HARDWARE_H
+#define __ASM_ARCH_HARDWARE_H
+
+/*
...
+ */
+
+#if defined(CONFIG_LPC2468)
+#else
+#error No hardware file defined for this configuration
+#endif
+
+#endif /* __ASM_ARCH_HARDWARE_H */
Ummm... What exactly is this file needed for?
I don't need it, but start.S wants to include it. See my comment about
the #ifdef's. Other architectures left it empty too, so it seemed the
best option to me.
Hm... that doesn't really make sense to me. Also, the error checking
in this file makes little sense to me.
+/* Macros for reading/writing registers */
+#define PUT8(reg, value) (*(volatile unsigned char*)(reg) = (value))
+#define PUT16(reg, value) (*(volatile unsigned short*)(reg) = (value))
+#define PUT32(reg, value) (*(volatile unsigned int*)(reg) = (value))
+#define GET8(reg) (*(volatile unsigned char*)(reg))
+#define GET16(reg) (*(volatile unsigned short*)(reg))
+#define GET32(reg) (*(volatile unsigned int*)(reg))
Do you clain these are proper accessor functions for your processor?
Yes I do. They are straight from the LPC2292 code, so once they were
considered OK. I checked out the the write{s,l,b} functions in asm/io.h,
but although they look similar, for some reason they simply don't work.
Given the similarities between the write{s,l,b} and the PUT* functions,
what is the problem with those? Furthermore, the ARM architecture
doesn't use any kind of special instructions for accessing registers,
everything is memory mapped.
I'm not an expert for this processor, but I wonder it there might be
some form of sync instruction (or memory barrier or similar) needed?
Best regards,
Wolfgang Denk
--
DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de
Given a choice between two theories, take the one which is funnier.
___
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Wolfgang Denk schreef: Dear Remco Poelstra, Is there no way we can do without such a #ifdef here? The problem is that start.S needs hardware.h, but the code in immap.h should not be included in start.S, so I can not merge hardware.h and immap.h Why not? I'm not aware of such a restriction? I'm not an expert in assembly, but at first I had immap.h included in start.S and it complained about invalid instructions, so if I combine hardware.h and immap.h, then there must be some way of making sure that the assembler ignores the C code in immap.h. Do you know of any such thing? I would like to avoid the ever growing list of #if defined(this) || defined(that) || defined(...) || ... Maybe we can have a common #define that covers the common case? I could add something like #if defined(CONFIG_LPC2922) || defined(CONFIG_LPC2468) #define (CONFIG_LPC2000) #endif in a general place and then use CONFIG_LPC2000 at the common places. The problem I then have is: What would be the best place to put such define? Preferably it is automatically included also for the LPC2292 code. If that's not possible, it can be defined in the board config, but I think that leads to confusion. What's your opinion? Ummm... What exactly is this file needed for? I don't need it, but start.S wants to include it. See my comment about the #ifdef's. Other architectures left it empty too, so it seemed the best option to me. Hm... that doesn't really make sense to me. Also, the error checking in this file makes little sense to me. I can remove the file, but than I need to put an #ifdef construct in start.S to only exclude it in the lpc2468 case. The file is used by the other ARM ports. I can also simply empty it, but in this way it is more similar to the other ports. What would you like? I'm not an expert for this processor, but I wonder it there might be some form of sync instruction (or memory barrier or similar) needed? No, none at all. There is only a single linear memory region for the processor and a write to a location has immediate effect, whether it be a register or just RAM. I suppose leaving parts of the 2468 and 2292 code separated is OK for the time being (considering offset lists and data structures), in the hope someone will upgrade the 2292 code? I'm willing to look into the PUT32() vs. writel() problem in about half a year or so. That will at least cleanup part of the code. Kind regards, Remco Poelstra ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Dear Remco Poelstra,
In message 49c8be7a.10...@duran-audio.com you wrote:
This patch includes support for the LPC2468 processor from NXP.
The example board will follow when this patch is OK.
Such a comment does not belong into the commit message. Please mode it
below the --- line.
Signed-off-by: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
---
diff -upNr u-boot-orig/cpu/arm720t/interrupts.c
u-boot-cleanup/cpu/arm720t/interrupts.c
--- u-boot-orig/cpu/arm720t/interrupts.c 2009-03-18 00:42:12.0
+0100
+++ u-boot-cleanup/cpu/arm720t/interrupts.c 2009-03-24 11:48:50.0
+0100
@@ -29,7 +29,11 @@
#include common.h
#include clps7111.h
#include asm/proc-armv/ptrace.h
+#if defined(CONFIG_LPC2468)
+#include asm/arch/immap.h
+#else
#include asm/hardware.h
+#endif
Is there no way we can do without such a #ifdef here?
#ifndef CONFIG_NETARM
/* we always count down the max. */
@@ -40,6 +44,11 @@
#ifdef CONFIG_LPC2292
#undef READ_TIMER
#define READ_TIMER (0x - GET32(T0TC))
+#elif defined(CONFIG_LPC2468)
+#undef TIMER_LOAD_VAL
+#define TIMER_LOAD_VAL 0
+#undef READ_TIMER
+#define READ_TIMER (0x - 0xE0004008)
NAK. When you have to #unifdef existing variable definitions, then
ther eis something fundamentally wrong. Please fix this problem at the
cause, i. e. where the wroing values are defined.
#endif
#else
@@ -80,6 +89,14 @@ void do_irq (struct pt_regs *pt_regs)
pfnct = (void (*)(void))VICVectAddr;
(*pfnct)();
+#elif defined(CONFIG_LPC2468)
+ void (*pfnct) (void);
+ vic_2468_t *vic = (((immap_t *)CONFIG_SYS_IMMAP)-ahb.vic);
+
+ pfnct = (void (*)(void))((vic-vicaddr));
+
+ (*pfnct) ();
Is there no way to combine this code with the one for the LPC2292? It
doesn't look that different to me...
#else
#error do_irq() not defined for this CPU type
#endif
@@ -112,6 +129,9 @@ static ulong lastdec;
int interrupt_init (void)
{
+#if defined(CONFIG_LPC2468)
+ timer_2468_t *timer0=(((immap_t *)CONFIG_SYS_IMMAP)-apb.timer0);
+#endif
#if defined(CONFIG_NETARM)
/* disable all interrupts */
@@ -185,6 +205,13 @@ int interrupt_init (void)
PUT32(T0MCR, 0);
PUT32(T0TC, 0);
PUT32(T0TCR, 1);/* enable timer0 */
+#elif defined(CONFIG_LPC2468)
+ PUT32 ((timer0-ir), 0); /*disable all timer0 interupts
*/
+ PUT32 ((timer0-tcr), 0); /*disable timer0 */
+ PUT32 ((timer0-pr), CFG_SYS_CLK_FREQ / CONFIG_SYS_HZ - 1);
+ PUT32 ((timer0-mcr), 0);
+ PUT32 ((timer0-tc), 0);
+ PUT32 ((timer0-tcr), 1);
Again: Is there no way to combine this code with the one for the
LPC2292? It doesn't look that different to me...
...
--- u-boot-orig/cpu/arm720t/lpc24xx/Makefile 1970-01-01 01:00:00.0
+0100
+++ u-boot-cleanup/cpu/arm720t/lpc24xx/Makefile 2009-03-19
10:56:53.0 +0100
...
+$(SOBJS):
+ $(CC) $(AFLAGS) -march=armv4t -c -o $(SOBJS) iap_entry.S
Such compile options hsould probably be set globally, not just for
this single source file?
...
+int serial_tstc (void)
+{
+ uart_2468_t *uart0=(((immap_t *)CONFIG_SYS_IMMAP)-apb.uart0);
+
+ return (GET8 ((uart0-lsr)) 1);
+}
+
+
+
#endif
Get rid of all these empty lines, please.
diff -upNr u-boot-orig/cpu/arm720t/start.S u-boot-cleanup/cpu/arm720t/start.S
--- u-boot-orig/cpu/arm720t/start.S 2009-03-18 00:42:12.0 +0100
+++ u-boot-cleanup/cpu/arm720t/start.S2009-03-24 11:52:35.0
+0100
@@ -127,7 +127,7 @@ reset:
bl cpu_init_crit
#endif
-#ifdef CONFIG_LPC2292
+#if defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
bl lowlevel_init
#endif
@@ -368,6 +368,10 @@ lock_loop:
ldr r0, VPBDIV_ADR
mov r1, #0x01 /* VPB clock is same as process clock */
str r1, [r0]
+#elif defined(CONFIG_LPC2468)
+ldr r0, =0x40008000 /*0x4000 is internal SRAM, 0x4000 is end of
SRAM*/
Line too long.
+mov sp,r0
+sub sl,sp,#0x2000
Indentation by TAB, please.
#else
#error No cpu_init_crit() defined for current CPU type
#endif
@@ -383,7 +387,7 @@ lock_loop:
str r1, [r0]
#endif
-#ifndef CONFIG_LPC2292
+#if !defined(CONFIG_LPC2292) !defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
mov ip, lr
/*
* before relocating, we have to setup RAM timing
@@ -601,7 +605,7 @@ reset_cpu:
* on external peripherals such as watchdog timers, etc. */
#elif defined(CONFIG_INTEGRATOR) defined(CONFIG_ARCH_INTEGRATOR)
/* No specific reset actions for IntegratorAP/CM720T as yet */
-#elif defined(CONFIG_LPC2292)
+#elif defined(CONFIG_LPC2292) || defined(CONFIG_LPC2468)
Is there no way to combine this code with the one for the LPC2292?
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Dear Remco Poelstra, In message 49c25f75.3080...@duran-audio.com you wrote: Wolfgang Denk schreef: +/* Vectored Interrupt Controller (VIC) */ +#define VIC_BASE_ADDR 0xF000 +#define VICIRQStatus (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x000)) +#define VICFIQStatus (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x004)) +#define VICRawIntr (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x008)) +#define VICIntSelect (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x00C)) +#define VICIntEnable (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x010)) +#define VICIntEnClr(*(volatile unsigned long *)(VIC_BASE_ADDR + 0x014)) +#define VICSoftInt (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x018)) +#define VICSoftIntClr (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x01C)) +#define VICProtection (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x020)) +#define VICSWPrioMask (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x024)) Please do not use offset lists, but define a proper C data structure instead. And never ever access device regiters through simple volatile pointers. Use proper accessor functions. Here and elsewhere. All examples I checked use the same syntax for defining registers, so I left it in the code. If that is a problem, can you indicate an example which does The Right Thing (tm)? See for example the structure declarations in include/asm-ppc/*immap* I think the patch now matches the other criteria. The patch is a bit bigger, since other code did not follow the Coding Styles either. I used indent to fix my code and it fixed the other code as well. I hope you didn't dop this in a single step. Codin style cleanup and adding new stuff must be done separately. The second patch will follow when this patch is OK. Regards, Remco Poelstra --- u-boot-orig/cpu/arm720t/cpu.c 2009-03-18 00:42:12.0 +0100 +++ u-boot/cpu/arm720t/cpu.c 2009-03-19 16:00:04.0 +0100 @@ -41,7 +41,9 @@ int cpu_init (void) * setup up stacks if necessary */ #ifdef CONFIG_USE_IRQ - IRQ_STACK_START = _armboot_start - CONFIG_SYS_MALLOC_LEN - CONFIG_SYS_GBL_DATA_SIZE - 4; + IRQ_STACK_START = + _armboot_start - CONFIG_SYS_MALLOC_LEN - CONFIG_SYS_GBL_DATA_SIZE - + 4; FIQ_STACK_START = IRQ_STACK_START - CONFIG_STACKSIZE_IRQ; #endif return 0; @@ -63,17 +65,17 @@ int cleanup_before_linux (void) disable_interrupts (); /* turn off I-cache */ - asm (mrc p15, 0, %0, c1, c0, 0:=r (i)); + asm (mrc p15, 0, %0, c1, c0, 0:=r (i)); i = ~0x1000; - asm (mcr p15, 0, %0, c1, c0, 0: :r (i)); + asm (mcr p15, 0, %0, c1, c0, 0: :r (i)); What sort of patch is this? There is no commit message, no explanation what it is, no SoB, ... ? Best regards, Wolfgang Denk -- DENX Software Engineering GmbH, MD: Wolfgang Denk Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: w...@denx.de Deliver yesterday, code today, think tomorrow. ___ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Re: [U-Boot] [PATCH 1/2] LPC2468 support
Dear Remco Poelstra,
In message 49c10b37.1070...@duran-audio.com you wrote:
This patch includes support for the LPC2468 processor from NXP.
Signed-off-by: Remco Poelstra remco.poelstra+u-b...@duran-audio.com
General comment: There are lots of coding style issues: trailing
white space, incorrect brace style, lines too long, C++ comments,
incorrect indentation, indentation not by TAB, etc.
Please clean up.
diff -upNr u-boot-orig/cpu/arm720t/cpu.c u-boot/cpu/arm720t/cpu.c
--- u-boot-orig/cpu/arm720t/cpu.c 2009-03-18 00:42:12.0 +0100
+++ u-boot/cpu/arm720t/cpu.c 2009-03-18 09:54:58.0 +0100
@@ -78,6 +78,23 @@ int cleanup_before_linux (void)
/* Nothing more needed */
#elif defined(CONFIG_INTEGRATOR) defined(CONFIG_ARCH_INTEGRATOR)
/* No cleanup before linux for IntegratorAP/CM720T as yet */
+#elif defined(CONFIG_LPC2468)
+ disable_interrupts ();
+ {
+ volatile unsigned char dummy,i;
Please do not use arbitrary blocks to declare variables right in the
middle of the function.
+ U0IER = 0;
+ U1IER = 0;
+ for(i=0; i16; i++)
+ {
Incorrect brace style, here and in many other places.
Indetation not by TAB, here and in many other places.
+ dummy=U0RBR;
+ dummy=U0LSR;
+ dummy=U0IIR;
+ dummy=U1RBR;
+ dummy=U1LSR;
+ dummy=U1IIR;
+ }
What sort of magic is this code supposed to perform?
diff -upNr u-boot-orig/cpu/arm720t/serial.c u-boot/cpu/arm720t/serial.c
--- u-boot-orig/cpu/arm720t/serial.c 2009-03-18 00:42:12.0 +0100
+++ u-boot/cpu/arm720t/serial.c 2009-03-18 12:29:00.0 +0100
@@ -199,4 +199,91 @@ int serial_tstc (void)
return (GET8(U0LSR) 1);
}
+#elif defined(CONFIG_LPC2468)
+#include asm/arch/hardware.h
+
+int serial_init (void)
+{
+ unsigned long pinsel0;
+
+ //enable uart #0 pins in GPIO (P0.2 = TxD0, P0.3 = RxD0)
+ pinsel0 = PINSEL0;
+ pinsel0 = ~(0x00f0);
+ pinsel0 |= 0x0050;
+ PINSEL0 = pinsel0;
Please use proper accessor functions to access device registers, here
and everywhere else.
+void serial_setbrg (void)
+{
+ DECLARE_GLOBAL_DATA_PTR;
+
+ unsigned short divisor = 0;
+ unsigned long fractional = 0;
+
+ switch (gd-baudrate) {
+ case 1200: divisor = 3000; fractional = 0xF0; break;
+ case 9600: divisor = 375; fractional = 0xF0; break;
+// case 19200: divisor = 188; fractional = 0; break;
+ case 19200: divisor = 175; fractional = 0xAF; break;
+ case 38400: divisor = 94; fractional = 0; break;
+// case 38400: divisor = 75; fractional = 0xC3; break;
+// case 57600: divisor = 63; fractional = 0; break;
+ case 57600: divisor = 50; fractional = 0xC3; break;
+// case 115200: divisor = 31; fractional = 0; break;
...
+//#if 0
...
No such dead code, please!
diff -upNr u-boot-orig/include/asm-arm/arch-lpc24xx/lpc24xx_registers.h
u-boot/include/asm-arm/arch-lpc24xx/lpc24xx_registers.h
--- u-boot-orig/include/asm-arm/arch-lpc24xx/lpc24xx_registers.h
1970-01-01 01:00:00.0 +0100
+++ u-boot/include/asm-arm/arch-lpc24xx/lpc24xx_registers.h 2009-03-18
15:43:41.0 +0100
@@ -0,0 +1,1123 @@
+#ifndef __LPC24XX_REGISTERS_H
+#define __LPC24XX_REGISTERS_H
+
+#include config.h
+
+/* Macros for reading/writing registers */
+//#define PUT8(reg, value) (*(volatile unsigned char*)(reg) = (value))
+//#define PUT16(reg, value) (*(volatile unsigned short*)(reg) = (value))
+//#define PUT32(reg, value) (*(volatile unsigned int*)(reg) = (value))
+//#define GET8(reg) (*(volatile unsigned char*)(reg))
+//#define GET16(reg) (*(volatile unsigned short*)(reg))
+//#define GET32(reg) (*(volatile unsigned int*)(reg))
No dead code, here and elsewhere, please.
+
+/* Vectored Interrupt Controller (VIC) */
+#define VIC_BASE_ADDR0xF000
+#define VICIRQStatus (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x000))
+#define VICFIQStatus (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x004))
+#define VICRawIntr (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x008))
+#define VICIntSelect (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x00C))
+#define VICIntEnable (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x010))
+#define VICIntEnClr(*(volatile unsigned long *)(VIC_BASE_ADDR + 0x014))
+#define VICSoftInt (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x018))
+#define VICSoftIntClr (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x01C))
+#define VICProtection (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x020))
+#define VICSWPrioMask (*(volatile unsigned long *)(VIC_BASE_ADDR + 0x024))
Please do not use offset lists, but define a proper C data structure
instead. And never ever access device regiters through simple
volatile pointers. Use proper accessor functions. Here and elsewhere.
The whole code
