Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 13:08 Mon 09 Feb , Vovk Konstantin wrote:
>> Add support for CPU and serial communication.
>> Interrupt not used for work with Timer0.
>> Serial communication configured for UART0.
>> ---
>> cpu/arm720t/cpu.c | 48 ++++++-
>> cpu/arm720t/interrupts.c | 82 +++++++++++-
>> cpu/arm720t/serial.c | 115 ++++++++++++++-
>> cpu/arm720t/start.S | 30 ++++-
>> include/asm-arm/arch-arm720t/W90P710.h | 240
>> +++++++++++++++++++++++++++++++
>> include/asm-arm/arch-arm720t/hardware.h | 2 +
>> include/asm-arm/io.h | 4 +
>> 7 files changed, 512 insertions(+), 9 deletions(-)
>> create mode 100644 include/asm-arm/arch-arm720t/W90P710.h
>>
>> diff --git a/cpu/arm720t/cpu.c b/cpu/arm720t/cpu.c
>> index 5ac8f59..1cee570 100644
>> --- a/cpu/arm720t/cpu.c
>> +++ b/cpu/arm720t/cpu.c
>> @@ -73,7 +73,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_LPC2292) || defined(CONFIG_W90P710)
> I think it's time to move to CONFIG_ reflecting the functionnality
I do not quite understand what you want to say this.
Please concretize what should I do?
>> disable_interrupts ();
>> /* Nothing more needed */
>> #elif defined(CONFIG_INTEGRATOR) && defined(CONFIG_ARCH_INTEGRATOR)
>> @@ -253,6 +253,52 @@ int dcache_status (void)
>> {
>> }
>> #elif defined(CONFIG_LPC2292) /* just to satisfy the compiler */
>> +
>> +#elif defined(CONFIG_W90P710)
> I think it will be better to put in the soc specific code
> and it's the same for the interrupt.c
This section must be moved to a separate file that describe the W90P710 SoC?
Where this file must be resides?
>> +void icache_enable (void)
>> +{
>> + PUT_REG (REG_CAHCON, 0x05); /* Flush I-cache */
>> + while (GET_REG (REG_CAHCON)); /* Wait for operation progress */
>> +
>> + PUT_REG (REG_CAHCNF, 0x01); /* Enable I-CACHE */
>> +}
>> +
>> +void icache_disable (void)
>> +{
>> + PUT_REG (REG_CAHCON, 0x05); /* Flush I-cache */
>> + while (GET_REG (REG_CAHCON)); /* Wait for operation progress */
>> + /* Disable I-cache */
>> + PUT_REG (REG_CAHCNF, GET_REG (REG_CAHCNF) & (~0x01));
>> +}
>> +{
>> + /* Check if I-Cache enable */
>> + return (GET_REG (REG_CAHCNF) & 0x01) != 0;
>> +}
>> +
> <snip>
>> diff --git a/cpu/arm720t/serial.c b/cpu/arm720t/serial.c
>> index 54a9b31..e9bc652 100644
>> --- a/cpu/arm720t/serial.c
>> +++ b/cpu/arm720t/serial.c
>> @@ -169,13 +169,13 @@ int serial_init (void)
>>
>> void serial_putc (const char c)
>> {
>> - if (c == '\n')
>> - {
>> - while((GET8(U0LSR) & (1<<5)) == 0); /* Wait for empty U0THR */
>> + if (c == '\n') {
>> + /* Wait for empty U0THR */
>> + while((GET8(U0LSR) & (1<<5)) == 0);
>> PUT8(U0THR, '\r');
>> }
> please do not mix coding clean style fix and add new code
This my mistake. I don't understand why i edit LPC code.
Maybe I check the coding style and accidentally change this section.
>> -
>> - while((GET8(U0LSR) & (1<<5)) == 0); /* Wait for empty U0THR */
>> + /* Wait for empty U0THR */
>> + while((GET8(U0LSR) & (1<<5)) == 0);
>> PUT8(U0THR, c);
>> }
>>
>> @@ -199,4 +199,109 @@ int serial_tstc (void)
>> return (GET8(U0LSR) & 1);
>> }
>>
>> +#elif defined(CONFIG_W90P710)
> please think to put this in it's own driver or to reduce the impact of adding
> new code to the current one
If I put this section to the separate serial driver, where
this file must be resides, in current directory as for netarm or in
/drivers/serial?
>> +
>> +DECLARE_GLOBAL_DATA_PTR;
>> +
>> +#include <asm/arch/hardware.h>
>> +
>> +void serial_setbrg (void)
>> +{
>> + unsigned int divider = 0;
>> +
>> + /* use values for 15Mhz quarts */
>> + switch (gd->baudrate) {
> is it not possible to calculate it?
Maybe this is suitabel. But more complex.
If declare this part as universal code, it is necessary to take
into account the current PLL settings, as well as to
know the frequency or the external quartz or generator.
>
>> + case 1200: divider = 779; break;
>> + case 2400: divider = 389; break;
>> + case 4800: divider = 193; break;
>> + case 9600: divider = 96; break;
>> + case 14400: divider = 65; break;
>> + case 19200: divider = 47; break;
>> + case 38400: divider = 22; break;
>> + case 57600: divider = 14; break;
>> + case 115200: divider = 6; break;
>> + default: hang (); break;
> hand? is it not too much?
In your opinion, how much it will be?
>> + }
>> +
>> + /* Reset LCR register and disable Uart interrupt*/
>> + PUT_REG (REG_UART0_LCR, 0x0);
>> + PUT_REG (REG_UART0_IER, 0x0);
>> +
>> + /* Set baud rate to mentioned in divider */
>> + /* Set DLAB to '1' to access Divisor */
>> + PUT_REG (REG_UART0_LCR, GET_REG (REG_UART0_LCR) | (1 << 7));
>> + PUT_REG (REG_UART0_LSB, (divider & 0xFF));
>> + PUT_REG (REG_UART0_MSB, ((divider >> 8) & 0xFF));
>> + /* Reset DLAB to '0' to access RBR, THR, IER */
>> + PUT_REG (REG_UART0_LCR, GET_REG (REG_UART0_LCR) & (~(1 << 7)));
>> +
>> + /* Set port parameters 8 bit, 1 stop, no parity */
>> + PUT_REG (REG_UART0_LCR, GET_REG (REG_UART0_LCR) |
>> + ((1 << 0) | (1 << 1)));
>> +
>> + /*
>> + * Set the RX FIFO trigger level to 8 bytes,
>> + * reset RX, TX FIFO, Enable FIFO
>> + */
>> + PUT_REG (REG_UART0_FCR, ((0x2 << 6) | (1 << 2) | (1 << 1) | (1 << 0)));
>> +
>> + /* Reset timeout comparator */
>> + PUT_REG (REG_UART0_TOR, 0x0);
>> +}
>> +
>> +/* Initialise the serial port with the given baudrate. Settings are 8N1 */
>> +int serial_init (void)
>> +{
>> + unsigned int Temp;
>> +
>> + serial_setbrg ();
>> +
>> + /* Set GPIO_CFG to use pins PORT5_0 as TXD0 and PORT5_1 as RXD0 */
>> + Temp = GET_REG (REG_GPIO_CFG5);
>> + Temp |= 0X00000005;
>> + Temp &= 0XFFFFFFF5;
>> + PUT_REG (REG_GPIO_CFG5, Temp);
>> + return (0);
>> +}
>> +
>> +/* Check for Transmitter holding register is empty */
>> +void CheckForUART0TXReady (void)
>> +{
>> + unsigned int Status;
> please no uppercase in var name or function
>> +
>> + do {
>> + Status = GET_REG (REG_UART0_LSR);
>> + }
>> + while (!(Status & (1 << 6))); /* Check for TE=1*/
>> +}
>> +
>> +/*Put one character to Uart0 TX FIFO. Add 'LF' after 'CR'*/
>> +void serial_putc (const char c)
>> +{
>> + if (c == '\n') {
>> + /* Wait for empty THR */
>> + CheckForUART0TXReady ();
>> + PUT_REG (REG_UART0_TX, '\r');
>> + }
>> + CheckForUART0TXReady ();
>> + PUT_REG (REG_UART0_TX, c);
>> +}
>> +
>> +int serial_getc (void)
>> +{
>> + while ((GET_REG (REG_UART0_LSR) & 1) == 0);
>> + return GET_REG(REG_UART0_RX);
>> +}
>> +
>> +int serial_tstc (void)
>> +{
>> + return (GET_REG (REG_UART0_LSR) & 1);
>> +}
>> +
>> +void serial_puts (const char *s)
>> +{
>> + while (*s) {
>> + serial_putc (*s++);
>> + }
>> +}
>> #endif
>> diff --git a/cpu/arm720t/start.S b/cpu/arm720t/start.S
>> index 022b873..055515a 100644
>> --- a/cpu/arm720t/start.S
>> +++ b/cpu/arm720t/start.S
>> @@ -1,8 +1,8 @@
>> /*
>> * armboot - Startup Code for ARM720 CPU-core
>> *
>> - * Copyright (c) 2001 Marius Gr?ger <m...@sysgo.de>
>> - * Copyright (c) 2002 Alex Z?pke <a...@sysgo.de>
>> + * Copyright (c) 2001 Marius Gr???ger <m...@sysgo.de>
>> + * Copyright (c) 2002 Alex Z???pke <a...@sysgo.de>
> why?
I don't understand this too. I check this in next patch.
>> *
>> * See file CREDITS for list of people who contributed to this
>> * project.
>> @@ -224,6 +224,14 @@ PLLSTAT_ADR: .word PLLSTAT
>> VPBDIV_ADR: .word VPBDIV
>> MEMMAP_ADR: .word MEMMAP
>>
>> +#elif defined(CONFIG_W90P710)
>> +
>> +#define CAHCON 0xFFF02004 /* Cache control register */
>> +#define AIC_MDCR 0xFFF82124 /* AIC Mask Disable Control Register */
>> +#define CAHCNF 0xFFF02000 /* Cache configuration register
>> */
>> +#define IDREG 0xFFF00000 /* W90P710 CHIP ID */
>> +#define CKSKEW 0xFFF01F00 /* Clock Skew Control register
>> */
>> +
>> #endif
>>
>> cpu_init_crit:
>> @@ -368,6 +376,21 @@ lock_loop:
>> ldr r0, VPBDIV_ADR
>> mov r1, #0x01 /* VPB clock is same as process clock */
>> str r1, [r0]
>> +
>> +#elif defined(CONFIG_W90P710)
>> + /* Disable Interrupt */
>> + LDR r0, =AIC_MDCR
>> + LDR r1, =0x7FFFE
>> + STR r1, [r0]
>> + /* Disable FIQ and IRQ */
>> + MRS r0, CPSR
>> + ORR r0, r0, #0xC0
>> + MSR CPSR_c, r0
>> + /* Disable I-Cache and D-cache entirely */
>> + LDR r1,=CAHCNF
>> + MOV r0, #0x0
>> + STR r0, [r1]
>> +
>> #else
>> #error No cpu_init_crit() defined for current CPU type
>> #endif
>> @@ -606,6 +629,9 @@ reset_cpu:
>> .globl reset_cpu
>> reset_cpu:
>> mov pc, r0
>> +#elif defined(CONFIG_W90P710)
>> +/* Nothing done here as reseting the CPU is board specific, depending on
>> external peripherals see lowlevel_init.c*/
>> +
>> #else
>> #error No reset_cpu() defined for current CPU type
>> #endif
>> diff --git a/include/asm-arm/arch-arm720t/W90P710.h
>> b/include/asm-arm/arch-arm720t/W90P710.h
>> new file mode 100644
>> index 0000000..c6be724
>> --- /dev/null
>> +++ b/include/asm-arm/arch-arm720t/W90P710.h
>> @@ -0,0 +1,240 @@
>> +/*
>> + * (C) Copyright 2008
>> + * KSL Embedded Development Team <www.kslemb.com>
>> + * Konstantin Vovk <k...@kslemb.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
>> + *
>> + * Description: Nuvoton W90P710 register layout definition
>> + */
>> +#ifndef _W90P710_H
>> +#define _W90P710_H
>> +
>> +/* L1 8KB on chip SRAM base address */
>> +#define SRAM_BASE 0xFFE00000
>> +
>> +#define GET_REG(reg) (*((volatile unsigned int *)(reg)))
>> +#define PUT_REG(reg, val) (*((volatile unsigned int *)(reg))) = (unsigned
>> int)(val)
> please use readx/writex
> and protect it for the ASM
Ok. I must define this macros in this file as mentioned in /asm-i386/io.h?
Then I must change all intrance of the GET/PUT_REG in all my stuff or
simply redefine PUT_REG as writex? How it will be correct?
>> +
>> +/* System Address Map - Defines the register base address */
>> +#define GCR_BA 0xFFF00000 /* Global Control */
>> +#define EBI_BA 0xFFF01000 /* EBI Control */
>> +#define CACHE_BA 0xFFF02000 /* Cache Control */
>> +#define EMC_BA 0xFFF03000 /* Ethernet MAC */
>> +#define GDMA_BA 0xFFF04000 /* GDMA control */
>> +#define USBH_BA 0xFFF05000 /* USB Host Control */
>> +#define USBD_BA 0xFFF06000 /* USB Device Control */
>> +#define FMI_BA 0xFFF07000 /* Flash Memory Card Interface
>> */
>> +#define LCD_BA 0xFFF08000 /* Display, LCM Interface &
>> Bypass */
>> +#define ADO_BA 0xFFF09000 /* Audio Control */
>> +
>> +#define UART0_BA 0xFFF80000 /* UART0 Control (console) */
>> +#define UART1_BA 0xFFF80100 /* UART1 Control (Bluetooth) */
>> +#define UART2_BA 0xFFF80200 /* UART2 Control (IrDA) */
>> +#define UART3_BA 0xFFF80300 /* UART3 Control
>> (micro-printer) */
>> +#define TMR_BA 0xFFF81000 /* Timer */
>> +#define AIC_BA 0xFFF82000 /* Interrupt Controller */
>> +#define GPIO_BA 0xFFF83000 /* GPIO Control */
>> +#define RTC_BA 0xFFF84000 /* Real Time Clock Control */
>> +#define SCHI0_BA 0xFFF85000 /* Smart Card Host Interface 0
>> Control */
>> +#define SCHI1_BA 0xFFF85800 /* Smart Card Host Interface 1
>> Control */
>> +#define I2C0_BA 0xFFF86000 /* I2C 0 Control */
>> +#define I2C1_BA 0xFFF86100 /* I2C 1 Control */
>> +#define SSP_BA 0xFFF86200 /* Synchronous Serial Port */
>> +#define PWM_BA 0xFFF87000 /* Pulse Width Modulation
>> Control */
>> +#define KPI_BA 0xFFF88000 /* Keypad Interface Control */
>> +#define PS2_BA 0xFFF89000 /* PS2 Interface Control */
>> +
>> +/* System Manager Control Registers */
>> +#define REG_PDID (GCR_BA+0x0000) /* Product Identifier */
> please add a space before and after '+'
> and keep in mind of the 80 chars limit
One question: Comments must be wraped too after 80 chars limit?
I see the large number of uboot code don't wrap the comments.
>> +#define REG_ARBCON (GCR_BA+0x0004) /* Arbitration Control */
>> +#define REG_PLLCON (GCR_BA+0x0008) /* PLL Control */
>> +#define REG_CLKSEL (GCR_BA+0x000C) /* Clock Select */
>> +#define REG_PLLCON2 (GCR_BA+0x0010) /* PLL Control2 */
>> +#define REG_I2SCKCON (GCR_BA+0x0014) /* Audio IIS Clock Control */
>> +#define REG_IRQWAKECON (GCR_BA+0x0020) /* IRQ Wakeup Control */
>> +#define REG_IRQWAKEFLAG (GCR_BA+0x0024) /* IRQ Wakeup Flag */
>> +#define REG_PMCON (GCR_BA+0x0028) /* Power Manager */
>> +#define REG_USBTXRCON (GCR_BA+0x0030) /* USB transceiver */
>> +
>> +/* Memory Control Registers */
>> +#define REG_EBICON (EBI_BA+0x000) /* EBI control */
>> +#define REG_ROMCON (EBI_BA+0x004) /* ROM/FLASH control */
>> +#define REG_SDCONF0 (EBI_BA+0x008) /* SDRAM bank 0 configuration */
>> +#define REG_SDCONF1 (EBI_BA+0x00C) /* SDRAM bank 1 configuration */
>> +#define REG_SDTIME0 (EBI_BA+0x010) /* SDRAM bank 0 timing control
>> */
>> +#define REG_SDTIME1 (EBI_BA+0x014) /* SDRAM bank 1 timing control
>> */
>> +#define REG_EXT0CON (EBI_BA+0x018) /* External I/O 0 control */
>> +#define REG_EXT1CON (EBI_BA+0x01C) /* External I/O 1 control */
>> +#define REG_EXT2CON (EBI_BA+0x020) /* External I/O 2 control */
>> +#define REG_EXT3CON (EBI_BA+0x024) /* External I/O 3 control */
>> +
>> +/* Cache Control Registers */
>> +#define REG_CAHCNF (CACHE_BA+0x000)/* Cache configuration */
>> +#define REG_CAHCON (CACHE_BA+0x004)/* Cache control */
>> +#define REG_CAHADR (CACHE_BA+0x008)/* Cache address */
>> +
>> +/* MAC Control registers*/
>> +#define REG_CAMCMR (EMC_BA+0x000) /* CAM Command */
>> +#define REG_CAMEN (EMC_BA+0x004) /* CAM Enable */
>> +#define REG_CAM0M_Base (EMC_BA+0x008)
>> +#define REG_CAM0L_Base (EMC_BA+0x00c)
> please use uppercase for the MACRO
If I understood correctly, I can't mix small and large letters in the
names of functions, variables and macros? The names of functions
and variables should contain only lowercase letters?
>> +
>> +#define REG_TXDLSA (EMC_BA+0x088) /* Transmit Descriptor Link
>> List Start Address Register */
>> +#define REG_RXDLSA (EMC_BA+0x08C) /* Receive Descriptor Link List
>> Start Address Register */
>> +#define REG_MCMDR (EMC_BA+0x090) /* MAC Command */
>> +#define REG_MIID (EMC_BA+0x094) /* MII Management Data */
>> +#define REG_MIIDA (EMC_BA+0x098) /* MII Management Control and
>> Address */
>> +#define REG_FFTCR (EMC_BA+0x09C) /* FIFO Threshold Control */
>> +#define REG_TSDR (EMC_BA+0x0a0) /* Transmit Start Demand */
>> +#define REG_RSDR (EMC_BA+0x0a4) /* Receive Start Demand */
>> +#define REG_DMARFC (EMC_BA+0x0a8) /* Maximum Receive Frame
>> Control */
>> +#define REG_MIEN (EMC_BA+0x0ac) /* MAC Interrupt Enable */
> v> +
Move this section to the EMC interface code?
>> +/* MAC Status Registers */
>> +#define REG_MISTA (EMC_BA+0x0b0) /* MAC Interrupt Status */
>> +#define REG_MGSTA (EMC_BA+0x0b4) /* MAC General Status */
>> +#define REG_MPCNT (EMC_BA+0x0b8) /* Missed Packet Count */
>> +#define REG_MRPC (EMC_BA+0x0bc) /* MAC Receive Pause Count */
>> +#define REG_MRPCC (EMC_BA+0x0c0) /* MAC Receive Pause Current
>> Count */
>> +#define REG_MREPC (EMC_BA+0x0c4) /* MAC Remote Pause Count */
>> +#define REG_DMARFS (EMC_BA+0x0c8) /* DMA Receive Frame Status */
> <snip>
>> +
>> +/* General-Purpose Input/Output Controller Registers */
>> + #define GPIO_OFFSET 0x10
> ^^^^^^^^^^^^^
> please check you steel have some whitespace in the code
My mistake.
>> +/* Port 0 */
>> +#define REG_GPIO_CFG0 (GPIO_BA+0x0000) /*
>> Configuration */
>> +#define REG_GPIO_DIR0 (GPIO_BA+0x0004) /* Direction
>> control */
>> +#define REG_GPIO_DATAOUT0 (GPIO_BA+0x0008) /* Data out */
>> +#define REG_GPIO_DATAIN0 (GPIO_BA+0x000c) /* Data input */
>
>> +#endif /* _W90P710_H */
>> diff --git a/include/asm-arm/arch-arm720t/hardware.h
>> b/include/asm-arm/arch-arm720t/hardware.h
>> index 3056ca7..7f31b6c 100644
>> --- a/include/asm-arm/arch-arm720t/hardware.h
>> +++ b/include/asm-arm/arch-arm720t/hardware.h
>> @@ -36,6 +36,8 @@
>> /* include armadillo specific hardware file if there was one */
>> #elif defined(CONFIG_INTEGRATOR) && defined(CONFIG_ARCH_INTEGRATOR)
>> /* include IntegratorCP/CM720T specific hardware file if there was one */
>> +#elif defined(CONFIG_W90P710)
>> +#include <asm-arm/arch-arm720t/W90P710.h>
>> #else
>> #error No hardware file defined for this configuration
>> #endif
>> diff --git a/include/asm-arm/io.h b/include/asm-arm/io.h
>> index fec3a7e..4e17d22 100644
>> --- a/include/asm-arm/io.h
>> +++ b/include/asm-arm/io.h
>> @@ -46,6 +46,10 @@ static inline void sync(void)
>> static inline void *
>> map_physmem(phys_addr_t paddr, unsigned long len, unsigned long flags)
>> {
>> + #ifdef CONFIG_W90P710
>> + if (flags == MAP_NOCACHE)
>> + paddr |= 0x80000000;
> why?
This code added because W90P710 SoC has non-cacheable space
start from 0x80000000. We use IDE as memory mapped. When cache enabled
there was an error for read/write operation.
>> + #endif
>> return (void *)paddr;
> Best Regards,
> J.
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
