
Dear Prafulla Wadaskar,
+unsigned char get_random_hex(void) +{
- int i;
- u32 inbuf[16];
- u8 outbuf[16];
- /*
* in case of 88F6281/88F6192 A0,
* Bit7 need to reset to generate random values in KW_REG_UNDOC_0x1470
* Soc reg offsets KW_REG_UNDOC_0x1470 and KW_REG_UNDOC_0x1478 are reserved regs and
* Does not have names at this moment (no errata available)
*/
- writel(readl(KW_REG_UNDOC_0x1478) & ~(1 << 7), KW_REG_UNDOC_0x1478);
- for (i = 0; i < 16; i++) {
either use a #define'd name instead of the repeated 16, or use sizeof(inbuf), but don't hard-code the same constant in multiple places. This is calling for inconsistency.
--- /dev/null +++ b/drivers/serial/kirkwood_serial.c @@ -0,0 +1,129 @@
... ...
+static void kw_uart_putc(u8 c) +{
- while ((p_uart_port->lsr & LSR_THRE) == 0) ;
- p_uart_port->thr = c;
- return;
+}
+void serial_putc(const char c) +{
- if (c == '\n')
kw_uart_putc('\r');
- kw_uart_putc(c);
+}
+int serial_getc(void) +{
- while ((p_uart_port->lsr & LSR_DR) == 0) ;
- return (p_uart_port->rbr);
+}
+int serial_tstc(void) +{
- return ((p_uart_port->lsr & LSR_DR) != 0);
+}
+void serial_setbrg(void) +{
- DECLARE_GLOBAL_DATA_PTR;
- int clock_divisor = (CONFIG_SYS_TCLK / 16) / gd->baudrate;
- p_uart_port->ier = 0x00;
- p_uart_port->lcr = LCR_DIVL_EN; /* Access baud rate */
- p_uart_port->dll = clock_divisor & 0xff; /* 9600 baud */
- p_uart_port->dlm = (clock_divisor >> 8) & 0xff;
- p_uart_port->lcr = LCR_8N1; /* 8 data, 1 stop, no parity */
- /* Clear & enable FIFOs */
- p_uart_port->fcr = FCR_FIFO_EN | FCR_RXSR | FCR_TXSR;
- return;
Please use I/O accessors instead of all trhese raw register accesses.
+/* SOC specific definations */ +#define INTREG_BASE 0xd0000000 +#define KW_REGISTER(x) (KW_REGS_PHY_BASE | x) +#define KW_OFFSET_REG (INTREG_BASE | 0x20080)
You want to perform a "base + offset" addition, so please write this as an addition and not as a logical operation! But...
+/* undocumented registers */ +#define KW_REG_UNDOC_0x1470 (KW_REGISTER(0x1470)) +#define KW_REG_UNDOC_0x1478 (KW_REGISTER(0x1478))
+#define KW_UART0_BASE (KW_REGISTER(0x12000)) /* UArt 0 */ +#define KW_UART1_BASE (KW_REGISTER(0x13000)) /* UArt 1 */
+/* Controler environment registers offsets */ +#define KW_REG_MPP_CONTROL0 (KW_REGISTER(0x10000)) +#define KW_REG_MPP_CONTROL1 (KW_REGISTER(0x10004)) +#define KW_REG_MPP_CONTROL2 (KW_REGISTER(0x10008)) +#define KW_REG_MPP_CONTROL3 (KW_REGISTER(0x1000C)) +#define KW_REG_MPP_CONTROL4 (KW_REGISTER(0x10010)) +#define KW_REG_MPP_CONTROL5 (KW_REGISTER(0x10014)) +#define KW_REG_MPP_CONTROL6 (KW_REGISTER(0x10018)) +#define KW_REG_MPP_SMPL_AT_RST (KW_REGISTER(0x10030)) +#define KW_REG_DEVICE_ID (KW_REGISTER(0x10034)) +#define KW_REG_MPP_OUT_DRV_REG (KW_REGISTER(0x100E0))
+#define KW_REG_GPP0_DATA_OUT (KW_REGISTER(0x10100)) +#define KW_REG_GPP0_DATA_OUT_EN (KW_REGISTER(0x10104)) +#define KW_REG_GPP0_BLINK_EN (KW_REGISTER(0x10108)) +#define KW_REG_GPP0_DATA_IN_POL (KW_REGISTER(0x1010C)) +#define KW_REG_GPP0_DATA_IN (KW_REGISTER(0x10110)) +#define KW_REG_GPP0_INT_CAUSE (KW_REGISTER(0x10114)) +#define KW_REG_GPP0_INT_MASK (KW_REGISTER(0x10118)) +#define KW_REG_GPP0_INT_LVL (KW_REGISTER(0x1011c))
+#define KW_REG_GPP1_DATA_OUT (KW_REGISTER(0x10140)) +#define KW_REG_GPP1_DATA_OUT_EN (KW_REGISTER(0x10144)) +#define KW_REG_GPP1_BLINK_EN (KW_REGISTER(0x10148)) +#define KW_REG_GPP1_DATA_IN_POL (KW_REGISTER(0x1014C)) +#define KW_REG_GPP1_DATA_IN (KW_REGISTER(0x10150)) +#define KW_REG_GPP1_INT_CAUSE (KW_REGISTER(0x10154)) +#define KW_REG_GPP1_INT_MASK (KW_REGISTER(0x10158)) +#define KW_REG_GPP1_INT_LVL (KW_REGISTER(0x1015c))
+#define KW_REG_NAND_READ_PARAM (KW_REGISTER(0x10418)) +#define KW_REG_NAND_WRITE_PARAM (KW_REGISTER(0x1041c)) +#define KW_REG_NAND_CTRL (KW_REGISTER(0x10470))
+#define KW_REG_WIN_CTRL(x) (KW_REGISTER((0x20000 + (x * 0x10)))) +#define KW_REG_WIN_BASE(x) (KW_REGISTER((0x20004 + (x * 0x10)))) +#define KW_REG_WIN_REMAP_LOW(x) (KW_REGISTER((0x20008 + (x * 0x10)))) +#define KW_REG_WIN_REMAP_HIGH(x) (KW_REGISTER((0x2000c + (x * 0x10))))
+#define KW_REG_CPU_CONFIG (KW_REGISTER(0x20100)) +#define KW_REG_CPU_CTRL_STAT (KW_REGISTER(0x20104)) +#define KW_REG_CPU_RSTOUTN_MASK (KW_REGISTER(0x20108)) +#define KW_REG_CPU_SYS_SOFT_RST (KW_REGISTER(0x2010C)) +#define KW_REG_CPU_AHB_MBUS_CAUSE_INT (KW_REGISTER(0x20110)) +#define KW_REG_CPU_AHB_MBUS_MASK_INT (KW_REGISTER(0x20114)) +#define KW_REG_CPU_FTDLL_CONFIG (KW_REGISTER(0x20120)) +#define KW_REG_CPU_L2_CONFIG (KW_REGISTER(0x20128)) +#define KW_REG_L2_RAM_TIMING0 (KW_REGISTER(0x20134)) +#define KW_REG_L2_RAM_TIMING1 (KW_REGISTER(0x20138))
+#define KW_REG_TMR_CTRL (KW_REGISTER(0x20300)) +#define KW_REG_TMR_RELOAD (KW_REGISTER(0x20310)) +#define KW_REG_TMR_VAL (KW_REGISTER(0x20314))
+#define KW_REG_PCIE_BASE (KW_REGISTER(0x40000))
+#define KW_EGIGA0_BASE (KW_REGISTER(0x72000)) +#define KW_EGIGA1_BASE (KW_REGISTER(0x76000))
... please get rid of all these register offsets and use C structs instead, as has been discussed here a couple of times recently.
...
+++ b/include/asm-arm/arch-kirkwood/serial.h @@ -0,0 +1,89 @@
...
+/* This structure describes the registers offsets for one UART port/channel */ +struct kw_uart_port {
- u8 rbr; /* 0 = 0-3 */
- u8 pad1[3];
- u8 ier; /* 1 = 4-7 */
- u8 pad2[3];
- u8 fcr; /* 2 = 8-b */
- u8 pad3[3];
- u8 lcr; /* 3 = c-f */
- u8 pad4[3];
- u8 mcr; /* 4 = 10-13 */
- u8 pad5[3];
- u8 lsr; /* 5 = 14-17 */
- u8 pad6[3];
- u8 msr; /* 6 =18-1b */
- u8 pad7[3];
- u8 scr; /* 7 =1c-1f */
- u8 pad8[3];
+};
Come on. Why does this have to be a separate header file? Does this not look very much the same like a zillion of other UARTs Why cannot you simply use "include/ns16550.h" instead?
Heck. Why do you need your own serial driver at all? I bet the standard 16550 driver just works fine for you, too.
Best regards,
Wolfgang Denk