[U-Boot] [PATCH] Hardware diagnosis for inka4x0 boards.

From: Andreas Pfefferle ap@denx.de
This patch adds diagnosis functions for the buzzer, UARTs and digital IOs on inka4x0 hardware. Signed-off-by: Andreas Pfefferle ap@denx.de --- board/inka4x0/Makefile | 2 +- board/inka4x0/inkadiag.c | 515 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 516 insertions(+), 1 deletions(-) create mode 100644 board/inka4x0/inkadiag.c
diff --git a/board/inka4x0/Makefile b/board/inka4x0/Makefile index 442e2d0..2264dae 100644 --- a/board/inka4x0/Makefile +++ b/board/inka4x0/Makefile @@ -25,7 +25,7 @@ include $(TOPDIR)/config.mk
LIB = $(obj)lib$(BOARD).a
-COBJS := $(BOARD).o +COBJS := $(BOARD).o inkadiag.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS)) diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c new file mode 100644 index 0000000..6194b58 --- /dev/null +++ b/board/inka4x0/inkadiag.c @@ -0,0 +1,515 @@ +/* + * (C) Copyright 2008 + * Andreas Pfefferle, DENX Software Engineering, ap@denx.de. + * + * 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 + */ + +#include <asm/io.h> +#include <common.h> +#include <config.h> +#include <mpc5xxx.h> +#include <pci.h> + +#include <command.h> + +#define TEST_NAME(test, name) static char test##_program_name[] = name +#define TEST_USAGE(test, usage) static char test##_program_usage[] = usage +#define TEST_FUNCTION(test) do_##test +#define PRINT_TEST_USAGE(test) do {\ + printf(test##_program_usage, test##_program_name);\ + return -1;\ +} while (0) + +#define GPIO_BASE (u_char *)0x30400000 + +#define DIGIN_TOUCHSCR_MASK 0x00003000 /* Inputs 12-13 */ +#define DIGIN_KEYB_MASK 0x00010000 /* Input 16 */ + +#define DIGIN_DRAWER_SW1 0x00400000 /* Input 22 */ +#define DIGIN_DRAWER_SW2 0x00800000 /* Input 23 */ + +#define DIGIO_LED0 0x00000001 /* Output 0 */ +#define DIGIO_LED1 0x00000002 /* Output 1 */ +#define DIGIO_LED2 0x00000004 /* Output 2 */ +#define DIGIO_LED3 0x00000008 /* Output 3 */ +#define DIGIO_LED4 0x00000010 /* Output 4 */ +#define DIGIO_LED5 0x00000020 /* Output 5 */ + +#define DIGIO_DRAWER1 0x00000100 /* Output 8 */ +#define DIGIO_DRAWER2 0x00000200 /* Output 9 */ + +static unsigned int inka_digin_get_input(void) +{ + return in_8(GPIO_BASE + 0) << 0 | in_8(GPIO_BASE + 1) << 8 | + in_8(GPIO_BASE + 2) << 16 | in_8(GPIO_BASE + 3) << 24; +} + +#define LED_HIGH(NUM)\ +out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\ + in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) | 0x10) + +#define LED_LOW(NUM)\ +out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\ + in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) & ~0x10) + +#define CHECK_LED(NUM) do {\ + if (state & (1 << NUM)) {\ + LED_HIGH(NUM);\ + } else {\ + LED_LOW(NUM);\ + } \ +} while (0) + +static void inka_digio_set_output(unsigned int state, int which) +{ + if (which == 0) { + CHECK_LED(0); + CHECK_LED(1); + CHECK_LED(2); + CHECK_LED(3); + CHECK_LED(4); + CHECK_LED(5); + } else { + struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO; + if (which == 1) { + if (state & DIGIO_DRAWER1) { + gpio->simple_dvo &= ~0x1000; + udelay(1); + gpio->simple_dvo |= 0x1000; + } else { + gpio->simple_dvo |= 0x1000; + udelay(1); + gpio->simple_dvo &= ~0x1000; + } + } + if (which == 2) { + if (state & DIGIO_DRAWER2) { + gpio->simple_dvo &= ~0x2000; + udelay(1); + gpio->simple_dvo |= 0x2000; + } else { + gpio->simple_dvo |= 0x2000; + udelay(1); + gpio->simple_dvo &= ~0x2000; + } + } + } + udelay(1); +} + +TEST_NAME(io, "inkadiag io"); +TEST_USAGE(io, "Usage: %s\n\ + <which> # get which[drawer1|drawer2|other] input\n\ + <which> <val> # set which[drawer1|drawer2|other] output to val\n\ +"); + +static int io_helper(int argc, char *argv[]) +{ + unsigned int state = inka_digin_get_input(); + if (strcmp(argv[1], "drawer1") == 0) { + printf("exit code: 0x%X\n", + (state & DIGIN_DRAWER_SW1) >> (ffs(DIGIN_DRAWER_SW1) - 1)); + } else if (strcmp(argv[1], "drawer2") == 0) { + printf("exit code: 0x%X\n", + (state & DIGIN_DRAWER_SW2) >> (ffs(DIGIN_DRAWER_SW2) - 1)); + } else if (strcmp(argv[1], "other") == 0) { + printf("exit code: 0x%X\n", + ((state & DIGIN_KEYB_MASK) >> (ffs(DIGIN_KEYB_MASK) - 1)) + | ((state & DIGIN_TOUCHSCR_MASK) >> + (ffs(DIGIN_TOUCHSCR_MASK) - 2))); + } else { + printf("Invalid argument: %s\n", argv[1]); + return -1; + } + return 0; +} + +static int TEST_FUNCTION(io) (cmd_tbl_t *cmdtp, int flag, int argc, + char *argv[]) { + if ((argc < 2) || (argc > 3)) + PRINT_TEST_USAGE(io); + + if (argc == 2) + return io_helper(argc, argv); + + unsigned int val = simple_strtol(argv[2], NULL, 16); + + if (strcmp(argv[1], "drawer1") == 0) { + val <<= (ffs(DIGIO_DRAWER1) - 1); + inka_digio_set_output(val, 1); + } else if (strcmp(argv[1], "drawer2") == 0) { + val <<= (ffs(DIGIO_DRAWER2) - 1); + inka_digio_set_output(val, 2); + } else if (strcmp(argv[1], "other") == 0) + inka_digio_set_output(val, 0); + else { + printf("Invalid argument: %s\n", argv[1]); + return -1; + } + return io_helper(argc, argv); +} + +DECLARE_GLOBAL_DATA_PTR; + +static int ser_init(volatile struct mpc5xxx_psc *psc, int baudrate) +{ + unsigned long baseclk; + int div; + + /* reset PSC */ + psc->command = PSC_SEL_MODE_REG_1; + + /* select clock sources */ + + psc->psc_clock_select = 0; + baseclk = (gd->ipb_clk + 16) / 32; + + /* switch to UART mode */ + psc->sicr = 0; + + /* configure parity, bit length and so on */ + + psc->mode = PSC_MODE_8_BITS | PSC_MODE_PARNONE; + + psc->mode = PSC_MODE_ONE_STOP; + + /* set up UART divisor */ + div = (baseclk + (baudrate / 2)) / baudrate; + psc->ctur = (div >> 8) & 0xff; + psc->ctlr = div & 0xff; + + /* disable all interrupts */ + psc->psc_imr = 0; + + /* reset and enable Rx/Tx */ + psc->command = PSC_RST_RX; + psc->command = PSC_RST_TX; + psc->command = PSC_RX_ENABLE | PSC_TX_ENABLE; + + return 0; +} + +static void ser_putc(volatile struct mpc5xxx_psc *psc, const char c) +{ + /* Wait for last character to go. */ + int i = 0; + while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < CONFIG_SYS_HZ)) + udelay(10); + psc->psc_buffer_8 = c; + +} + +static int ser_getc(volatile struct mpc5xxx_psc *psc) +{ + /* Wait for a character to arrive. */ + int i = 0; + while (!(psc->psc_status & PSC_SR_RXRDY) && (i++ < CONFIG_SYS_HZ)) + udelay(10); + + return psc->psc_buffer_8; +} + +#define SERIAL_PORT_BASE (u_char *)0x80000000 + +#define UART_RX 0 /* In: Receive buffer (DLAB=0) */ +#define UART_TX 0 /* Out: Transmit buffer (DLAB=0) */ +#define UART_DLL 0 /* Out: Divisor Latch Low (DLAB=1) */ + +#define UART_LCR 3 /* Out: Line Control Register */ +#define UART_MCR 4 /* Out: Modem Control Register */ + +#define UART_LSR 5 /* In: Line Status Register */ +#define UART_MSR 6 /* In: Modem Status Register */ + +#define UART_LCR_WLEN8 0x03 /* Wordlength: 8 bits */ +#define UART_LCR_DLAB 0x80 /* Divisor latch access bit */ + +#define UART_LSR_THRE 0x20 /* Transmit-hold-register empty */ +#define UART_LSR_DR 0x01 /* Receiver data ready */ + +#define UART_MCR_LOOP 0x10 /* Enable loopback test mode */ +#define UART_MCR_RTS 0x02 /* RTS complement */ +#define UART_MCR_DTR 0x01 /* DTR complement */ + +#define UART_MSR_DCD 0x80 /* Data Carrier Detect */ +#define UART_MSR_DSR 0x20 /* Data Set Ready */ +#define UART_MSR_CTS 0x10 /* Clear to Send */ +#define PSC_OP1_RTS 0x01 +#define PSC_OP0_RTS 0x01 + +static unsigned char serial_in(unsigned char num, int offset) +{ + return in_8(SERIAL_PORT_BASE + (num << 3) + offset); +} + +static void serial_out(unsigned char num, int offset, unsigned char value) +{ + out_8(SERIAL_PORT_BASE + (num << 3) + offset, value); +} + +TEST_NAME(serial, "inkadiag serial"); +TEST_USAGE(serial, "Usage: %s\n\ + <num> <mode> <baudrate> <msg> # test uart num [0..11] in mode\ + and baudrate with msg\n\ +"); + +static int TEST_FUNCTION(serial) (cmd_tbl_t *cmdtp, int flag, int argc, + char *argv[]) { + if (argc < 5) + PRINT_TEST_USAGE(serial); + + argc--; + argv++; + + unsigned int num = simple_strtol(argv[0], NULL, 0); + if (num < 0 || num > 11) { + printf("invalid argument for num: %d\n", num); + return -1; + } + + unsigned int mode = simple_strtol(argv[1], NULL, 0); + + int combrd = 0; + + if (strcmp(argv[2], "115200") == 0) + combrd = 1; + else if (strcmp(argv[2], "57600") == 0) + combrd = 2; + else if (strcmp(argv[2], "38400") == 0) + combrd = 3; + else if (strcmp(argv[2], "19200") == 0) + combrd = 6; + else if (strcmp(argv[2], "9600") == 0) + combrd = 12; + else if (strcmp(argv[2], "2400") == 0) + combrd = 48; + else if (strcmp(argv[2], "1200") == 0) + combrd = 96; + else if (strcmp(argv[2], "300") == 0) + combrd = 384; + else + printf("Invalid baudrate: %s", argv[2]); + + printf("Testing uart %d.\n\n", num); + + if ((num >= 0) && (num <= 7)) { + if (mode & 1) + /* turn on 'loopback' mode */ + serial_out(num, UART_MCR, UART_MCR_LOOP); + else { + /* establish the UART's operational parameters */ + /* set DLAB=1 */ + serial_out(num, UART_LCR, UART_LCR_DLAB); + /* set baudrate */ + serial_out(num, UART_DLL, combrd); + /* set data-format: 8-N-1 */ + serial_out(num, UART_LCR, UART_LCR_WLEN8); + } + + if (mode & 2) { + /* set request to send */ + serial_out(num, UART_MCR, UART_MCR_RTS); + udelay(10); + /* check clear to send */ + if ((serial_in(num, UART_MSR) & UART_MSR_CTS) == 0x00) + return -1; + } + if (mode & 4) { + /* set data terminal ready */ + serial_out(num, UART_MCR, UART_MCR_DTR); + udelay(10); + /* check data set ready and carrier detect */ + if ((serial_in(num, UART_MSR) & + (UART_MSR_DSR | UART_MSR_DCD)) + != (UART_MSR_DSR | UART_MSR_DCD)) + return -1; + } + + /* write each message-character, read it back, and display it */ + int i, len = strlen(argv[3]); + for (i = 0; i < len; ++i) { + int j = 0; + while ((serial_in(num, UART_LSR) & UART_LSR_THRE) == + 0x00) { + if (j++ > CONFIG_SYS_HZ) + break; + udelay(10); + } + serial_out(num, UART_TX, argv[3][i]); + j = 0; + while ((serial_in(num, UART_LSR) & UART_LSR_DR) == + 0x00) { + if (j++ > CONFIG_SYS_HZ) + break; + udelay(10); + } + unsigned char data = serial_in(num, UART_RX); + printf("%c", data); + } + printf("\n\n"); + serial_out(num, UART_MCR, 0x00); + } else { + int address, irq; + switch (num) { + case 8: + address = MPC5XXX_PSC6; + irq = MPC5XXX_PSC6_IRQ; + break; + case 9: + address = MPC5XXX_PSC3; + irq = MPC5XXX_PSC3_IRQ; + break; + case 10: + address = MPC5XXX_PSC2; + irq = MPC5XXX_PSC2_IRQ; + break; + case 11: + address = MPC5XXX_PSC1; + irq = MPC5XXX_PSC1_IRQ; + break; + } + volatile struct mpc5xxx_psc *psc = + (struct mpc5xxx_psc *)address; + ser_init(psc, simple_strtol(argv[2], NULL, 0)); + if (mode & 2) { + /* set request to send */ + out_8(&psc->op0, PSC_OP0_RTS); + udelay(10); + /* check clear to send */ + if ((in_8(&psc->ip) & PSC_IPCR_CTS) == 0) + return -1; + } + int i, len = strlen(argv[3]); + for (i = 0; i < len; ++i) { + ser_putc(psc, argv[3][i]); + printf("%c", ser_getc(psc)); + } + printf("\n\n"); + } + return 0; +} + +#define BUZZER_GPT (MPC5XXX_GPT + 0x60) /* GPT6 */ +static void buzzer_turn_on(unsigned int freq) +{ + struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)(BUZZER_GPT); + + const u32 prescale = gd->ipb_clk / freq / 128; + const u32 count = 128; + const u32 width = 64; + + gpt->cir = (prescale << 16) | count; + gpt->pwmcr = width << 16; + gpt->emsr = 3; /* Timer enabled for PWM */ +} + +static void buzzer_turn_off(void) +{ + struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)(BUZZER_GPT); + gpt->emsr = 0; +} + +TEST_NAME(buzzer, "inkadiag buzzer"); +TEST_USAGE(buzzer, "Usage: %s\n\ + <period> <freq> # turn buzzer on for period ms with freq hz\n\ +"); + +static int TEST_FUNCTION(buzzer) (cmd_tbl_t *cmdtp, int flag, int argc, + char *argv[]) { + + if (argc != 3) + PRINT_TEST_USAGE(buzzer); + + argc--; + argv++; + + unsigned int period = simple_strtol(argv[0], NULL, 0); + if (!period) + printf("Zero period is senseless\n"); + argc--; + argv++; + + unsigned int freq = simple_strtol(argv[0], NULL, 0); + /* avoid zero prescale in buzzer_turn_on() */ + if (freq > gd->ipb_clk / 128) + printf("%dHz exceeds maximum (%dHz)\n", freq, + gd->ipb_clk / 128); + else if (!freq) + printf("Zero frequency is senseless\n"); + else + buzzer_turn_on(freq); + + clear_ctrlc(); + int prev = disable_ctrlc(0); + + printf("Buzzing for %d ms. Type ^C to abort!\n\n", period); + + int i = 0; + + while (!ctrlc() && (i++ < CONFIG_SYS_HZ)) + udelay(period); + + clear_ctrlc(); + disable_ctrlc(prev); + + buzzer_turn_off(); + + return 0; +} + +TEST_NAME(inkadiag, "inkadiag"); +TEST_USAGE(inkadiag, "Usage: %s\n\ + [buzzer]\n\ + [io]\n\ + [serial]\n\ +"); + +#define IS_TEST(test) \ + (strncmp(&argv[0][0], #test, strlen(#test)) == 0) + +#define CHECK_TEST(test) do {\ + if (IS_TEST(test)) {\ + return TEST_FUNCTION(test) (cmdtp, flag, argc, argv);\ + } \ +} while (0) + +static int TEST_FUNCTION(inkadiag) (cmd_tbl_t *cmdtp, int flag, int argc, + char *argv[]) { + switch (argc) { + case 1: + PRINT_TEST_USAGE(inkadiag); + default: + argc--; + argv++; + CHECK_TEST(io); + CHECK_TEST(serial); + CHECK_TEST(buzzer); + } + PRINT_TEST_USAGE(inkadiag); +} + +U_BOOT_CMD(inkadiag, 6, 1, TEST_FUNCTION(inkadiag), + "inkadiag- inka diagnosis\n", + "[inkadiag what ...]\n" + " - perform a diagnosis on inka hardware\n" + "'inkadiag' performs hardware tests.\n\n" + "Without arguments, inkadiag prints a list of available tests.\n\n" + "To get detailed help information for specific tests you can type\n" + "'inkadiag what' with a test name 'what' as argument.\n");

From: Andreas Pfefferle ap@denx.de
Signed-off-by: Andreas Pfefferle ap@denx.de --- drivers/rtc/Makefile | 1 + drivers/rtc/rtc4543.c | 107 +++++++++++++++++++++++++++++++++++++++++++++++++ include/rtc.h | 4 ++ 3 files changed, 112 insertions(+), 0 deletions(-) create mode 100644 drivers/rtc/rtc4543.c
diff --git a/drivers/rtc/Makefile b/drivers/rtc/Makefile index 94bc518..6adece2 100644 --- a/drivers/rtc/Makefile +++ b/drivers/rtc/Makefile @@ -55,6 +55,7 @@ COBJS-$(CONFIG_RTC_MPC8xx) += mpc8xx.o COBJS-$(CONFIG_RTC_PCF8563) += pcf8563.o COBJS-$(CONFIG_RTC_PL031) += pl031.o COBJS-$(CONFIG_RTC_RS5C372A) += rs5c372.o +COBJS-$(CONFIG_RTC_RTC4543) += rtc4543.o COBJS-$(CONFIG_RTC_RX8025) += rx8025.o COBJS-$(CONFIG_RTC_S3C24X0) += s3c24x0_rtc.o COBJS-$(CONFIG_RTC_X1205) += x1205.o diff --git a/drivers/rtc/rtc4543.c b/drivers/rtc/rtc4543.c new file mode 100644 index 0000000..d2a6240 --- /dev/null +++ b/drivers/rtc/rtc4543.c @@ -0,0 +1,107 @@ +/* + * (C) Copyright 2008 + * Andreas Pfefferle, DENX Software Engineering, ap@denx.de. + * + * 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 + */ + +#include <asm/io.h> +#include <common.h> +#include <command.h> +#include <config.h> +#include <bcd.h> +#include <rtc.h> + +#if defined(CONFIG_RTC_RTC4543) && defined(CONFIG_CMD_DATE) + +int rtc_get(struct rtc_time *tmp) +{ + int rel = 0; + uchar sec, min, hour, mday, wday, mon, year; + + rtc_read(0x00); + sec = rtc_read(0x01); + min = rtc_read(0x02); + hour = rtc_read(0x03); + wday = rtc_read(0x04); + mday = rtc_read(0x05); + mon = rtc_read(0x06); + year = rtc_read(0x07); + rtc_read(0x08); + + debug("Get RTC year: %02x mon/cent: %02x mday: %02x wday: %02x " + "hr: %02x min: %02x sec: %02x\n", + year, mon, mday, wday, + hour, min, sec); + + if (sec & 0x80) { + puts("### Warning: RTC Low Voltage - date/time not reliable\n"); + rel = -1; + } + + tmp->tm_sec = BCD2BIN(sec & 0x7F); + tmp->tm_min = BCD2BIN(min & 0x7F); + tmp->tm_hour = BCD2BIN(hour & 0x3F); + tmp->tm_mday = BCD2BIN(mday & 0x3F); + tmp->tm_mon = BCD2BIN(mon & 0x1F); + tmp->tm_year = BCD2BIN(year); + tmp->tm_wday = BCD2BIN(wday & 0x07); + tmp->tm_yday = 0; + tmp->tm_isdst = 0; + + debug("Get DATE: %4d-%02d-%02d (wday=%d) TIME: %2d:%02d:%02d\n", + tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday, + tmp->tm_hour, tmp->tm_min, tmp->tm_sec); + + return rel; +} + +int rtc_set(struct rtc_time *tmp) +{ + debug("Set DATE: %4d-%02d-%02d (wday=%d) TIME: %2d:%02d:%02d\n", + tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday, + tmp->tm_hour, tmp->tm_min, tmp->tm_sec); + + rtc_write(0x00, 0); + rtc_write(0x01, BIN2BCD(tmp->tm_sec)); + rtc_write(0x02, BIN2BCD(tmp->tm_min)); + rtc_write(0x03, BIN2BCD(tmp->tm_hour)); + rtc_write(0x04, BIN2BCD(tmp->tm_wday)); + rtc_write(0x05, BIN2BCD(tmp->tm_mday)); + rtc_write(0x06, BIN2BCD(tmp->tm_mon)); + rtc_write(0x07, BIN2BCD(tmp->tm_year % 100)); + rtc_write(0x08, 0); + + return 0; +} + +void rtc_reset(void) +{ + struct rtc_time tmp; + tmp.tm_sec = 0; + tmp.tm_min = 0; + tmp.tm_hour = 0; + tmp.tm_wday = 4; + tmp.tm_mday = 1; + tmp.tm_mon = 1; + tmp.tm_year = 1970; + rtc_set(&tmp); +} + +#endif diff --git a/include/rtc.h b/include/rtc.h index 785fbe3..019c2eb 100644 --- a/include/rtc.h +++ b/include/rtc.h @@ -61,4 +61,8 @@ void to_tm (int, struct rtc_time *); unsigned long mktime (unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int);
+uchar rtc_read(uchar reg) __attribute__((weak)); +void rtc_write(uchar reg, uchar val) __attribute__((weak)); + + #endif /* _RTC_H_ */

From: Andreas Pfefferle ap@denx.de
This patch adds the board specific communication routines needed by the external 4543 RTC.
Signed-off-by: Andreas Pfefferle ap@denx.de --- board/inka4x0/inka4x0.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 124 insertions(+), 0 deletions(-)
diff --git a/board/inka4x0/inka4x0.c b/board/inka4x0/inka4x0.c index 507196b..6312b4f 100644 --- a/board/inka4x0/inka4x0.c +++ b/board/inka4x0/inka4x0.c @@ -8,6 +8,9 @@ * (C) Copyright 2004 * Martin Krause, TQ-Systems GmbH, martin.krause@tqs.de * + * (C) Copyright 2008 + * Andreas Pfefferle, DENX Software Engineering, ap@denx.de. + * * See file CREDITS for list of people who contributed to this * project. * @@ -27,6 +30,7 @@ * MA 02111-1307 USA */
+#include <asm/io.h> #include <common.h> #include <mpc5xxx.h> #include <pci.h> @@ -45,6 +49,111 @@ #error "INKA4x0 SDRAM: invalid chip type specified!" #endif
+#if defined(CONFIG_RTC_RTC4543) && defined(CONFIG_CMD_DATE) + +#define RTC_CE 0x01000000 +#define RTC_WR 0x02000000 +#define RTC_DATA 0x01 +#define RTC_CLK 0x02 + +#define RTCWRITE(n, data) do {\ + for (i = 0; i < n; i++) {\ + gpio->sint_dvo &= ~RTC_DATA;\ + if (data & (1 << i))\ + gpio->sint_dvo |= RTC_DATA;\ + udelay(10);\ + gpio->sint_dvo |= RTC_CLK;\ + udelay(10);\ + gpio->sint_dvo &= ~RTC_CLK;\ + udelay(10);\ + } \ +} while (0) + +#define RTCREAD(n, data) do {\ + for (i = 0; i < n; i++) {\ + gpio->sint_dvo |= RTC_CLK;\ + udelay(10);\ + data |= ((gpio->sint_ival & RTC_DATA) << i);\ + gpio->sint_dvo &= ~RTC_CLK;\ + udelay(10);\ + } \ +} while (0) + +uchar rtc_read(uchar reg) +{ + struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO; + int i; + uchar val = 0; + switch (reg) { + case 0: + /* PSC3_4 is input */ + gpio->sint_ddr &= ~RTC_DATA; + /* Lower WR */ + out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O, + in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_WR); + udelay(1); + /* Rise CE */ + out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O, + in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_CE); + udelay(1); + break; + case 1: + case 2: + case 3: + case 5: + case 6: + case 7: + RTCREAD(8, val); + break; + case 4: + RTCREAD(4, val); + break; + case 8: + /* Lower CE */ + out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O, + in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_CE); + break; + } + return val; +} + +void rtc_write(uchar reg, uchar val) +{ + struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO; + int i; + switch (reg) { + case 0: + /* PSC3_4 is output */ + gpio->sint_ddr |= RTC_DATA; + /* Rise WR */ + out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O, + in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_WR); + udelay(1); + /* Rise CE */ + out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O, + in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_CE); + udelay(1); + break; + case 1: + case 2: + case 3: + case 5: + case 6: + case 7: + RTCWRITE(8, val); + break; + case 4: + RTCWRITE(4, val); + break; + case 8: + /* Lower CE */ + out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O, + in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_CE); + break; + } +} +#endif + #ifndef CONFIG_SYS_RAMBOOT static void sdram_start (int hi_addr) { @@ -220,6 +329,21 @@ int misc_init_f (void) *(vu_long *) MPC5XXX_WU_GPIO_ENABLE |= GPIO_PSC3_9; *(vu_long *) MPC5XXX_WU_GPIO_DIR |= GPIO_PSC3_9; *(vu_long *) MPC5XXX_WU_GPIO_DATA_O |= GPIO_PSC3_9; + + /* Configure RTC and Display */ + struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO; + out_be32((unsigned *)MPC5XXX_WU_GPIO_ENABLE, + in_be32((unsigned *)MPC5XXX_WU_GPIO_ENABLE) | 0x03000000); + out_be32((unsigned *)MPC5XXX_WU_GPIO_DIR, + in_be32((unsigned *)MPC5XXX_WU_GPIO_DIR) | 0x03000000); + gpio->sint_gpioe |= 0x07; + gpio->sint_ddr |= 0x06; + gpio->sint_inten &= ~0x03; + *(u_long *)MPC5XXX_WU_GPIO_DATA_O &= ~0x01000000; + gpio->sint_dvo &= ~0x02; + gpio->sint_dvo |= 0x04; + /* end of RTC and Display configuration */ + return 0; }

From: Andreas Pfefferle ap@denx.de
This patch supports added diagnosis functions and the use of the external RTC. Signed-off-by: Andreas Pfefferle ap@denx.de --- include/configs/inka4x0.h | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/configs/inka4x0.h b/include/configs/inka4x0.h index 405234c..5caa405 100644 --- a/include/configs/inka4x0.h +++ b/include/configs/inka4x0.h @@ -97,7 +97,8 @@ #define CONFIG_CMD_PCI #define CONFIG_CMD_SNTP #define CONFIG_CMD_USB - +#define CONFIG_CMD_DATE +#define CONFIG_CMD_PING
#define CONFIG_TIMESTAMP 1 /* Print image info with timestamp */
@@ -241,13 +242,13 @@ * use PSC6_1 and PSC6_3 as GPIO: Bits 9:11 (mask: 0x07000000): * 011 -> PSC6 could not be used as UART or CODEC. IrDA still possible. */ -#define CONFIG_SYS_GPS_PORT_CONFIG 0x01001004 +#define CONFIG_SYS_GPS_PORT_CONFIG 0x01501444
/* * RTC configuration */ -#define CONFIG_RTC_MPC5200 1 /* use internal MPC5200 RTC */
+#define CONFIG_RTC_RTC4543 1 /* use external RTC */ /* * Miscellaneous configurable options */

On 21:58 Thu 20 Nov , ap@denx.de wrote:
From: Andreas Pfefferle ap@denx.de
This patch supports added diagnosis functions and the
diagnosis where?
use of the external RTC. Signed-off-by: Andreas Pfefferle ap@denx.de
include/configs/inka4x0.h | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
diff --git a/include/configs/inka4x0.h b/include/configs/inka4x0.h index 405234c..5caa405 100644 --- a/include/configs/inka4x0.h +++ b/include/configs/inka4x0.h @@ -97,7 +97,8 @@ #define CONFIG_CMD_PCI #define CONFIG_CMD_SNTP #define CONFIG_CMD_USB
+#define CONFIG_CMD_DATE +#define CONFIG_CMD_PING
not mention in the commit message
#define CONFIG_TIMESTAMP 1 /* Print image info with timestamp */
@@ -241,13 +242,13 @@
- use PSC6_1 and PSC6_3 as GPIO: Bits 9:11 (mask: 0x07000000):
- 011 -> PSC6 could not be used as UART or CODEC. IrDA still possible.
*/ -#define CONFIG_SYS_GPS_PORT_CONFIG 0x01001004 +#define CONFIG_SYS_GPS_PORT_CONFIG 0x01501444
why do yuo change it?
/*
- RTC configuration
*/ -#define CONFIG_RTC_MPC5200 1 /* use internal MPC5200 RTC */
+#define CONFIG_RTC_RTC4543 1 /* use external RTC */
^ whitespace please fix
why not make configurable via if def as ifndef CONFIG_RTC_RTC4543 #define CONFIG_RTC_MPC5200 #endif
Best Regards, J.

Dear Jean-Christophe PLAGNIOL-VILLARD,
In message 20081120213415.GH20436@game.jcrosoft.org you wrote:
why not make configurable via if def as > ifndef CONFIG_RTC_RTC4543 #define CONFIG_RTC_MPC5200 #endif
Because it doesn't make sense?
Best regards,
Wolfgang Denk

Dear ap@denx.de,
In message 1227214731-19542-4-git-send-email-ap@denx.de you wrote:
RnJvbTogQW5kcmVhcyBQZmVmZmVybGUgPGFwQGRlbnguZGU+CgpUaGlzIHBhdGNoIHN1cHBvcnRz IGFkZGVkIGRpYWdub3NpcyBmdW5jdGlvbnMgYW5kIHRoZQp1c2Ugb2YgdGhlIGV4dGVybmFsIFJU Qy4KwqAgwqAKU2lnbmVkLW9mZi1ieTogQW5kcmVhcyBQZmVmZmVybGUgPGFwQGRlbnguZGU+Ci0t LQogaW5jbHVkZS9jb25maWdzL2lua2E0eDAuaCB8ICAgIDcgKysrKy0tLQogMSBmaWxlcyBjaGFu Z2VkLCA0IGluc2VydGlvbnMoKyksIDMgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvaW5jbHVk ZS9jb25maWdzL2lua2E0eDAuaCBiL2luY2x1ZGUvY29uZmlncy9pbmthNHgwLmgKaW5kZXggNDA1 MjM0Yy4uNWNhYTQwNSAxMDA2NDQKLS0tIGEvaW5jbHVkZS9jb25maWdzL2lua2E0eDAuaAorKysg Yi9pbmNsdWRlL2NvbmZpZ3MvaW5rYTR4MC5oCkBAIC05Nyw3ICs5Nyw4IEBACiAjZGVmaW5lIENP TkZJR19DTURfUENJCiAjZGVmaW5lIENPTkZJR19DTURfU05UUAogI2RlZmluZSBDT05GSUdfQ01E X1VTQgotCisjZGVmaW5lIENPTkZJR19DTURfREFURQorI2RlZmluZSBDT05GSUdfQ01EX1BJTkcK IAogI2RlZmluZQlDT05GSUdfVElNRVNUQU1QCTEJLyogUHJpbnQgaW1hZ2UgaW5mbyB3aXRoIHRp bWVzdGFtcCAqLwogCkBAIC0yNDEsMTMgKzI0MiwxMyBAQAogICogdXNlIFBTQzZfMSBhbmQgUFND Nl8zIGFzIEdQSU86IEJpdHMgOToxMSAobWFzazogMHgwNzAwMDAwMCk6CiAgKgkwMTEgLT4gUFND NiBjb3VsZCBub3QgYmUgdXNlZCBhcyBVQVJUIG9yIENPREVDLiBJckRBIHN0aWxsIHBvc3NpYmxl LgogICovCi0jZGVmaW5lIENPTkZJR19TWVNfR1BTX1BPUlRfQ09ORklHCTB4MDEwMDEwMDQKKyNk ZWZpbmUgQ09ORklHX1NZU19HUFNfUE9SVF9DT05GSUcJMHgwMTUwMTQ0NAogCiAvKgogICogUlRD IGNvbmZpZ3VyYXRpb24KICAqLwotI2RlZmluZSBDT05GSUdfUlRDX01QQzUyMDAJMQkvKiB1c2Ug aW50ZXJuYWwgTVBDNTIwMCBSVEMgKi8KIAorI2RlZmluZSBDT05GSUdfUlRDX1JUQzQ1NDMgCTEJ LyogdXNlIGV4dGVybmFsIFJUQyAqLwogLyoKICAqIE1pc2NlbGxhbmVvdXMgY29uZmlndXJhYmxl IG9wdGlvbnMKICAqLwotLSAKMS42LjAuNAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KVS1Cb290IG1haWxpbmcgbGlzdApVLUJvb3RAbGlzdHMuZGVueC5k ZQpodHRwOi8vbGlzdHMuZGVueC5kZS9tYWlsbWFuL2xpc3RpbmZvL3UtYm9vdAo=
Please do not send base64 encoded messages.
diff --git a/include/configs/inka4x0.h b/include/configs/inka4x0.h index 405234c..5caa405 100644 --- a/include/configs/inka4x0.h +++ b/include/configs/inka4x0.h @@ -97,7 +97,8 @@ #define CONFIG_CMD_PCI #define CONFIG_CMD_SNTP #define CONFIG_CMD_USB
+#define CONFIG_CMD_DATE +#define CONFIG_CMD_PING
Please keep lists sorted.
@@ -241,13 +242,13 @@
- use PSC6_1 and PSC6_3 as GPIO: Bits 9:11 (mask: 0x07000000):
- 011 -> PSC6 could not be used as UART or CODEC. IrDA still possible.
*/ -#define CONFIG_SYS_GPS_PORT_CONFIG 0x01001004 +#define CONFIG_SYS_GPS_PORT_CONFIG 0x01501444
Don't we need a change to the comments above, too?
/*
- RTC configuration
*/ -#define CONFIG_RTC_MPC5200 1 /* use internal MPC5200 RTC */
+#define CONFIG_RTC_RTC4543 1 /* use external RTC */ /*
- Miscellaneous configurable options
*/
Please keep white space as it was before.
There is no such thing as CONFIG_RTC_RTC4543 in U-Boot. Eventually your patch depends on othere patches that need to be applied before? You should then mark this by submitting a numbered series of patches.
Best regards,
Wolfgang Denk

On 21:58 Thu 20 Nov , ap@denx.de wrote:
From: Andreas Pfefferle ap@denx.de
This patch adds the board specific communication routines needed by the external 4543 RTC.
Signed-off-by: Andreas Pfefferle ap@denx.de
board/inka4x0/inka4x0.c | 124 +++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 124 insertions(+), 0 deletions(-)
diff --git a/board/inka4x0/inka4x0.c b/board/inka4x0/inka4x0.c index 507196b..6312b4f 100644 --- a/board/inka4x0/inka4x0.c +++ b/board/inka4x0/inka4x0.c @@ -8,6 +8,9 @@
- (C) Copyright 2004
- Martin Krause, TQ-Systems GmbH, martin.krause@tqs.de
- (C) Copyright 2008
- Andreas Pfefferle, DENX Software Engineering, ap@denx.de.
- See file CREDITS for list of people who contributed to this
- project.
@@ -27,6 +30,7 @@
- MA 02111-1307 USA
*/
+#include <asm/io.h> #include <common.h> #include <mpc5xxx.h> #include <pci.h> @@ -45,6 +49,111 @@ #error "INKA4x0 SDRAM: invalid chip type specified!" #endif
+#if defined(CONFIG_RTC_RTC4543) && defined(CONFIG_CMD_DATE)
please create a rtc.c file and move the compile condition to Makefile
+#define RTC_CE 0x01000000
^^^^ please use tab instead of whitespcase and so on
+#define RTC_WR 0x02000000 +#define RTC_DATA 0x01 +#define RTC_CLK 0x02
+#define RTCWRITE(n, data) do {\
^^ please use tab
- for (i = 0; i < n; i++) {\
^^ please use tab
- gpio->sint_dvo &= ~RTC_DATA;\
- if (data & (1 << i))\
gpio->sint_dvo |= RTC_DATA;\
- udelay(10);\
- gpio->sint_dvo |= RTC_CLK;\
- udelay(10);\
- gpio->sint_dvo &= ~RTC_CLK;\
- udelay(10);\
- } \
^^ please use tab
+} while (0)
why not a inline function?
+#define RTCREAD(n, data) do {\
^^ please use tab
- for (i = 0; i < n; i++) {\
^^ please use tab
- gpio->sint_dvo |= RTC_CLK;\
- udelay(10);\
- data |= ((gpio->sint_ival & RTC_DATA) << i);\
- gpio->sint_dvo &= ~RTC_CLK;\
- udelay(10);\
- } \
^^ please use tab
+} while (0)
ditto
+uchar rtc_read(uchar reg) +{
- struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
- int i;
- uchar val = 0;
please add a blank line
- switch (reg) {
- case 0:
/* PSC3_4 is input */
gpio->sint_ddr &= ~RTC_DATA;
/* Lower WR */
out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_WR);
udelay(1);
/* Rise CE */
out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_CE);
udelay(1);
break;
- case 1:
- case 2:
- case 3:
- case 5:
- case 6:
- case 7:
RTCREAD(8, val);
break;
- case 4:
RTCREAD(4, val);
break;
- case 8:
/* Lower CE */
out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_CE);
break;
- }
- return val;
+}
+void rtc_write(uchar reg, uchar val) +{
- struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
- int i;
please add a blank line
- switch (reg) {
- case 0:
/* PSC3_4 is output */
gpio->sint_ddr |= RTC_DATA;
/* Rise WR */
out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_WR);
udelay(1);
/* Rise CE */
out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_CE);
udelay(1);
break;
- case 1:
- case 2:
- case 3:
- case 5:
- case 6:
- case 7:
RTCWRITE(8, val);
break;
- case 4:
RTCWRITE(4, val);
break;
- case 8:
/* Lower CE */
out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_CE);
break;
- }
+} +#endif
#ifndef CONFIG_SYS_RAMBOOT static void sdram_start (int hi_addr) { @@ -220,6 +329,21 @@ int misc_init_f (void) *(vu_long *) MPC5XXX_WU_GPIO_ENABLE |= GPIO_PSC3_9; *(vu_long *) MPC5XXX_WU_GPIO_DIR |= GPIO_PSC3_9; *(vu_long *) MPC5XXX_WU_GPIO_DATA_O |= GPIO_PSC3_9;
- /* Configure RTC and Display */
- struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
- out_be32((unsigned *)MPC5XXX_WU_GPIO_ENABLE,
in_be32((unsigned *)MPC5XXX_WU_GPIO_ENABLE) | 0x03000000);
- out_be32((unsigned *)MPC5XXX_WU_GPIO_DIR,
in_be32((unsigned *)MPC5XXX_WU_GPIO_DIR) | 0x03000000);
- gpio->sint_gpioe |= 0x07;
- gpio->sint_ddr |= 0x06;
- gpio->sint_inten &= ~0x03;
- *(u_long *)MPC5XXX_WU_GPIO_DATA_O &= ~0x01000000;
please do not use diect access
- gpio->sint_dvo &= ~0x02;
- gpio->sint_dvo |= 0x04;
- /* end of RTC and Display configuration */
- return 0;
}
Best Regards, J.

Dear ap@denx.de,
In message 1227214731-19542-3-git-send-email-ap@denx.de you wrote:
From: Andreas Pfefferle ap@denx.de
This patch adds the board specific communication routines needed by the external 4543 RTC.
No, that's broken by design. An RTC is not board-dependent, but a generic piece of hardware. Code for it does not belong into any board-specific file, but into drivers/rtc/
Best regards,
Wolfgang Denk

Hi Wolfgang,
Dear ap@denx.de,
In message 1227214731-19542-3-git-send-email-ap@denx.de you wrote:
From: Andreas Pfefferle ap@denx.de
This patch adds the board specific communication routines needed by the external 4543 RTC.
No, that's broken by design. An RTC is not board-dependent, but a generic piece of hardware. Code for it does not belong into any board-specific file, but into drivers/rtc/
Did you actually check what he does?
He does exactly what you request, he puts all the 4543 code into drivers/rtc, but this cannot globally encode on how to access the chip from a specific hardware. So he factored that out of the rtc driver (correctly in my opinion) and has only this glue code in the board files.
What *exactly* is broken by design in this approach?
Cheers Detlev

- 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
- */
+#include <asm/io.h> +#include <common.h> +#include <command.h> +#include <config.h> +#include <bcd.h> +#include <rtc.h>
+#if defined(CONFIG_RTC_RTC4543) && defined(CONFIG_CMD_DATE)
please pove this to Makefile
+int rtc_get(struct rtc_time *tmp) +{
- int rel = 0;
- uchar sec, min, hour, mday, wday, mon, year;
- rtc_read(0x00);
- sec = rtc_read(0x01);
- min = rtc_read(0x02);
- hour = rtc_read(0x03);
- wday = rtc_read(0x04);
- mday = rtc_read(0x05);
- mon = rtc_read(0x06);
- year = rtc_read(0x07);
- rtc_read(0x08);
- debug("Get RTC year: %02x mon/cent: %02x mday: %02x wday: %02x "
"hr: %02x min: %02x sec: %02x\n",
year, mon, mday, wday,
hour, min, sec);
- if (sec & 0x80) {
puts("### Warning: RTC Low Voltage - date/time not reliable\n");
rel = -1;
- }
- tmp->tm_sec = BCD2BIN(sec & 0x7F);
^^ please use tab instead of whitespace
- tmp->tm_min = BCD2BIN(min & 0x7F);
- tmp->tm_hour = BCD2BIN(hour & 0x3F);
- tmp->tm_mday = BCD2BIN(mday & 0x3F);
- tmp->tm_mon = BCD2BIN(mon & 0x1F);
- tmp->tm_year = BCD2BIN(year);
- tmp->tm_wday = BCD2BIN(wday & 0x07);
- tmp->tm_yday = 0;
- tmp->tm_isdst = 0;
- debug("Get DATE: %4d-%02d-%02d (wday=%d) TIME: %2d:%02d:%02d\n",
tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,
tmp->tm_hour, tmp->tm_min, tmp->tm_sec);
- return rel;
+}
+int rtc_set(struct rtc_time *tmp) +{
- debug("Set DATE: %4d-%02d-%02d (wday=%d) TIME: %2d:%02d:%02d\n",
tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,
tmp->tm_hour, tmp->tm_min, tmp->tm_sec);
- rtc_write(0x00, 0);
- rtc_write(0x01, BIN2BCD(tmp->tm_sec));
- rtc_write(0x02, BIN2BCD(tmp->tm_min));
- rtc_write(0x03, BIN2BCD(tmp->tm_hour));
- rtc_write(0x04, BIN2BCD(tmp->tm_wday));
- rtc_write(0x05, BIN2BCD(tmp->tm_mday));
- rtc_write(0x06, BIN2BCD(tmp->tm_mon));
- rtc_write(0x07, BIN2BCD(tmp->tm_year % 100));
^^ whitespace please fix
- rtc_write(0x08, 0);
- return 0;
+}
+void rtc_reset(void) +{
- struct rtc_time tmp;
please add an empty line and use tab for indentation
- tmp.tm_sec = 0;
- tmp.tm_min = 0;
- tmp.tm_hour = 0;
- tmp.tm_wday = 4;
- tmp.tm_mday = 1;
- tmp.tm_mon = 1;
- tmp.tm_year = 1970;
- rtc_set(&tmp);
+}
+#endif diff --git a/include/rtc.h b/include/rtc.h index 785fbe3..019c2eb 100644 --- a/include/rtc.h +++ b/include/rtc.h @@ -61,4 +61,8 @@ void to_tm (int, struct rtc_time *); unsigned long mktime (unsigned int, unsigned int, unsigned int, unsigned int, unsigned int, unsigned int);
+uchar rtc_read(uchar reg) __attribute__((weak)); +void rtc_write(uchar reg, uchar val) __attribute__((weak));
please private default function
and if you add it please fix the other rtc drivers too
Best Regards, J.

On 21:58 Thu 20 Nov , ap@denx.de wrote:
From: Andreas Pfefferle ap@denx.de
This patch adds diagnosis functions for the buzzer, UARTs and digital IOs on inka4x0 hardware. Signed-off-by: Andreas Pfefferle ap@denx.de
board/inka4x0/Makefile | 2 +- board/inka4x0/inkadiag.c | 515 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 516 insertions(+), 1 deletions(-) create mode 100644 board/inka4x0/inkadiag.c
why not make this diagnostic active via CONFIG_?
diff --git a/board/inka4x0/Makefile b/board/inka4x0/Makefile index 442e2d0..2264dae 100644 --- a/board/inka4x0/Makefile +++ b/board/inka4x0/Makefile @@ -25,7 +25,7 @@ include $(TOPDIR)/config.mk
LIB = $(obj)lib$(BOARD).a
-COBJS := $(BOARD).o +COBJS := $(BOARD).o inkadiag.o
SRCS := $(SOBJS:.o=.S) $(COBJS:.o=.c) OBJS := $(addprefix $(obj),$(COBJS)) diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c new file mode 100644 index 0000000..6194b58 --- /dev/null +++ b/board/inka4x0/inkadiag.c @@ -0,0 +1,515 @@ +/*
- (C) Copyright 2008
- Andreas Pfefferle, DENX Software Engineering, ap@denx.de.
- 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
- */
+#include <asm/io.h> +#include <common.h> +#include <config.h> +#include <mpc5xxx.h> +#include <pci.h>
+#include <command.h>
+#define TEST_NAME(test, name) static char test##_program_name[] = name +#define TEST_USAGE(test, usage) static char test##_program_usage[] = usage +#define TEST_FUNCTION(test) do_##test +#define PRINT_TEST_USAGE(test) do {\
- printf(test##_program_usage, test##_program_name);\
- return -1;\
+} while (0)
+#define GPIO_BASE (u_char *)0x30400000
^^ whitespace please remove
+#define DIGIN_TOUCHSCR_MASK 0x00003000 /* Inputs 12-13 */ +#define DIGIN_KEYB_MASK 0x00010000 /* Input 16 */
+#define DIGIN_DRAWER_SW1 0x00400000 /* Input 22 */ +#define DIGIN_DRAWER_SW2 0x00800000 /* Input 23 */
+#define DIGIO_LED0 0x00000001 /* Output 0 */ +#define DIGIO_LED1 0x00000002 /* Output 1 */ +#define DIGIO_LED2 0x00000004 /* Output 2 */ +#define DIGIO_LED3 0x00000008 /* Output 3 */ +#define DIGIO_LED4 0x00000010 /* Output 4 */ +#define DIGIO_LED5 0x00000020 /* Output 5 */
+#define DIGIO_DRAWER1 0x00000100 /* Output 8 */ +#define DIGIO_DRAWER2 0x00000200 /* Output 9 */
+static unsigned int inka_digin_get_input(void) +{
- return in_8(GPIO_BASE + 0) << 0 | in_8(GPIO_BASE + 1) << 8 |
in_8(GPIO_BASE + 2) << 16 | in_8(GPIO_BASE + 3) << 24;
+}
+#define LED_HIGH(NUM)\ +out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\
in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) | 0x10)
why not inline?
+#define LED_LOW(NUM)\ +out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\
in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) & ~0x10)
why not inline?
+#define CHECK_LED(NUM) do {\
- if (state & (1 << NUM)) {\
LED_HIGH(NUM);\
- } else {\
LED_LOW(NUM);\
- } \
+} while (0)
why not inline?
+static void inka_digio_set_output(unsigned int state, int which) +{
- if (which == 0) {
CHECK_LED(0);
CHECK_LED(1);
CHECK_LED(2);
CHECK_LED(3);
CHECK_LED(4);
CHECK_LED(5);
- } else {
struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
please add a blank line
if (which == 1) {
if (state & DIGIO_DRAWER1) {
gpio->simple_dvo &= ~0x1000;
udelay(1);
gpio->simple_dvo |= 0x1000;
} else {
gpio->simple_dvo |= 0x1000;
udelay(1);
gpio->simple_dvo &= ~0x1000;
}
}
if (which == 2) {
if (state & DIGIO_DRAWER2) {
gpio->simple_dvo &= ~0x2000;
udelay(1);
gpio->simple_dvo |= 0x2000;
} else {
gpio->simple_dvo |= 0x2000;
udelay(1);
gpio->simple_dvo &= ~0x2000;
}
}
- }
- udelay(1);
+}
+TEST_NAME(io, "inkadiag io"); +TEST_USAGE(io, "Usage: %s\n\
<which> # get which[drawer1|drawer2|other] input\n\
<which> <val> # set which[drawer1|drawer2|other] output to val\n\
+");
+static int io_helper(int argc, char *argv[]) +{
- unsigned int state = inka_digin_get_input();
please add a blank line
- if (strcmp(argv[1], "drawer1") == 0) {
printf("exit code: 0x%X\n",
(state & DIGIN_DRAWER_SW1) >> (ffs(DIGIN_DRAWER_SW1) - 1));
- } else if (strcmp(argv[1], "drawer2") == 0) {
printf("exit code: 0x%X\n",
(state & DIGIN_DRAWER_SW2) >> (ffs(DIGIN_DRAWER_SW2) - 1));
- } else if (strcmp(argv[1], "other") == 0) {
printf("exit code: 0x%X\n",
((state & DIGIN_KEYB_MASK) >> (ffs(DIGIN_KEYB_MASK) - 1))
| ((state & DIGIN_TOUCHSCR_MASK) >>
(ffs(DIGIN_TOUCHSCR_MASK) - 2)));
- } else {
printf("Invalid argument: %s\n", argv[1]);
return -1;
- }
- return 0;
+}
+static int TEST_FUNCTION(io) (cmd_tbl_t *cmdtp, int flag, int argc,
char *argv[]) {
- if ((argc < 2) || (argc > 3))
PRINT_TEST_USAGE(io);
- if (argc == 2)
return io_helper(argc, argv);
why not instead if (argc == 2) return io_helper(argc, argv); else PRINT_TEST_USAGE(io);
- unsigned int val = simple_strtol(argv[2], NULL, 16);
declaration must at the beginning of the function
- if (strcmp(argv[1], "drawer1") == 0) {
val <<= (ffs(DIGIO_DRAWER1) - 1);
inka_digio_set_output(val, 1);
- } else if (strcmp(argv[1], "drawer2") == 0) {
val <<= (ffs(DIGIO_DRAWER2) - 1);
inka_digio_set_output(val, 2);
- } else if (strcmp(argv[1], "other") == 0)
inka_digio_set_output(val, 0);
- else {
printf("Invalid argument: %s\n", argv[1]);
return -1;
- }
- return io_helper(argc, argv);
+}
+DECLARE_GLOBAL_DATA_PTR;
+static int ser_init(volatile struct mpc5xxx_psc *psc, int baudrate) +{
- unsigned long baseclk;
- int div;
- /* reset PSC */
- psc->command = PSC_SEL_MODE_REG_1;
- /* select clock sources */
- psc->psc_clock_select = 0;
- baseclk = (gd->ipb_clk + 16) / 32;
- /* switch to UART mode */
- psc->sicr = 0;
- /* configure parity, bit length and so on */
- psc->mode = PSC_MODE_8_BITS | PSC_MODE_PARNONE;
- psc->mode = PSC_MODE_ONE_STOP;
- /* set up UART divisor */
- div = (baseclk + (baudrate / 2)) / baudrate;
- psc->ctur = (div >> 8) & 0xff;
- psc->ctlr = div & 0xff;
- /* disable all interrupts */
- psc->psc_imr = 0;
- /* reset and enable Rx/Tx */
- psc->command = PSC_RST_RX;
- psc->command = PSC_RST_TX;
- psc->command = PSC_RX_ENABLE | PSC_TX_ENABLE;
- return 0;
+}
+static void ser_putc(volatile struct mpc5xxx_psc *psc, const char c) +{
- /* Wait for last character to go. */
- int i = 0;
please add a blank line
- while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < CONFIG_SYS_HZ))
udelay(10);
- psc->psc_buffer_8 = c;
+}
+static int ser_getc(volatile struct mpc5xxx_psc *psc) +{
- /* Wait for a character to arrive. */
- int i = 0;
please add a blank line
- while (!(psc->psc_status & PSC_SR_RXRDY) && (i++ < CONFIG_SYS_HZ))
udelay(10);
- return psc->psc_buffer_8;
+}
+#define SERIAL_PORT_BASE (u_char *)0x80000000
+#define UART_RX 0 /* In: Receive buffer (DLAB=0) */ +#define UART_TX 0 /* Out: Transmit buffer (DLAB=0) */ +#define UART_DLL 0 /* Out: Divisor Latch Low (DLAB=1) */
+#define UART_LCR 3 /* Out: Line Control Register */ +#define UART_MCR 4 /* Out: Modem Control Register */
+#define UART_LSR 5 /* In: Line Status Register */ +#define UART_MSR 6 /* In: Modem Status Register */
+#define UART_LCR_WLEN8 0x03 /* Wordlength: 8 bits */
^^ please use tab
+#define UART_LCR_DLAB 0x80 /* Divisor latch access bit */
+#define UART_LSR_THRE 0x20 /* Transmit-hold-register empty */ +#define UART_LSR_DR 0x01 /* Receiver data ready */
+#define UART_MCR_LOOP 0x10 /* Enable loopback test mode */ +#define UART_MCR_RTS 0x02 /* RTS complement */ +#define UART_MCR_DTR 0x01 /* DTR complement */
+#define UART_MSR_DCD 0x80 /* Data Carrier Detect */ +#define UART_MSR_DSR 0x20 /* Data Set Ready */ +#define UART_MSR_CTS 0x10 /* Clear to Send */ +#define PSC_OP1_RTS 0x01 +#define PSC_OP0_RTS 0x01
+static unsigned char serial_in(unsigned char num, int offset) +{
- return in_8(SERIAL_PORT_BASE + (num << 3) + offset);
+}
+static void serial_out(unsigned char num, int offset, unsigned char value) +{
- out_8(SERIAL_PORT_BASE + (num << 3) + offset, value);
+}
+TEST_NAME(serial, "inkadiag serial"); +TEST_USAGE(serial, "Usage: %s\n\
<num> <mode> <baudrate> <msg> # test uart num [0..11] in mode\
- and baudrate with msg\n\
+");
+static int TEST_FUNCTION(serial) (cmd_tbl_t *cmdtp, int flag, int argc,
char *argv[]) {
- if (argc < 5)
PRINT_TEST_USAGE(serial);
- argc--;
- argv++;
- unsigned int num = simple_strtol(argv[0], NULL, 0);
declaration must at the beginning of the function
- if (num < 0 || num > 11) {
printf("invalid argument for num: %d\n", num);
return -1;
- }
- unsigned int mode = simple_strtol(argv[1], NULL, 0);
declaration must at the beginning of the function
- int combrd = 0;
declaration must at the beginning of the function
- if (strcmp(argv[2], "115200") == 0)
combrd = 1;
- else if (strcmp(argv[2], "57600") == 0)
combrd = 2;
- else if (strcmp(argv[2], "38400") == 0)
combrd = 3;
- else if (strcmp(argv[2], "19200") == 0)
combrd = 6;
- else if (strcmp(argv[2], "9600") == 0)
combrd = 12;
- else if (strcmp(argv[2], "2400") == 0)
combrd = 48;
- else if (strcmp(argv[2], "1200") == 0)
combrd = 96;
- else if (strcmp(argv[2], "300") == 0)
combrd = 384;
- else
printf("Invalid baudrate: %s", argv[2]);
- printf("Testing uart %d.\n\n", num);
- if ((num >= 0) && (num <= 7)) {
if (mode & 1)
please add {}
/* turn on 'loopback' mode */
serial_out(num, UART_MCR, UART_MCR_LOOP);
else {
/* establish the UART's operational parameters */
/* set DLAB=1 */
serial_out(num, UART_LCR, UART_LCR_DLAB);
/* set baudrate */
serial_out(num, UART_DLL, combrd);
/* set data-format: 8-N-1 */
serial_out(num, UART_LCR, UART_LCR_WLEN8);
}
if (mode & 2) {
/* set request to send */
serial_out(num, UART_MCR, UART_MCR_RTS);
udelay(10);
/* check clear to send */
if ((serial_in(num, UART_MSR) & UART_MSR_CTS) == 0x00)
return -1;
}
if (mode & 4) {
/* set data terminal ready */
serial_out(num, UART_MCR, UART_MCR_DTR);
udelay(10);
/* check data set ready and carrier detect */
if ((serial_in(num, UART_MSR) &
(UART_MSR_DSR | UART_MSR_DCD))
!= (UART_MSR_DSR | UART_MSR_DCD))
return -1;
}
/* write each message-character, read it back, and display it */
int i, len = strlen(argv[3]);
declaration must at the beginning of the function
for (i = 0; i < len; ++i) {
int j = 0;
please add a blank line
while ((serial_in(num, UART_LSR) & UART_LSR_THRE) ==
0x00) {
if (j++ > CONFIG_SYS_HZ)
break;
udelay(10);
}
serial_out(num, UART_TX, argv[3][i]);
j = 0;
while ((serial_in(num, UART_LSR) & UART_LSR_DR) ==
0x00) {
if (j++ > CONFIG_SYS_HZ)
break;
udelay(10);
}
unsigned char data = serial_in(num, UART_RX);
printf("%c", data);
}
printf("\n\n");
serial_out(num, UART_MCR, 0x00);
- } else {
int address, irq;
please add a blank line
switch (num) {
case 8:
address = MPC5XXX_PSC6;
irq = MPC5XXX_PSC6_IRQ;
break;
case 9:
address = MPC5XXX_PSC3;
irq = MPC5XXX_PSC3_IRQ;
break;
case 10:
address = MPC5XXX_PSC2;
irq = MPC5XXX_PSC2_IRQ;
break;
case 11:
address = MPC5XXX_PSC1;
irq = MPC5XXX_PSC1_IRQ;
break;
}
volatile struct mpc5xxx_psc *psc =
(struct mpc5xxx_psc *)address;
ser_init(psc, simple_strtol(argv[2], NULL, 0));
if (mode & 2) {
/* set request to send */
out_8(&psc->op0, PSC_OP0_RTS);
udelay(10);
/* check clear to send */
if ((in_8(&psc->ip) & PSC_IPCR_CTS) == 0)
return -1;
}
int i, len = strlen(argv[3]);
declaration must at the beginning of the function
for (i = 0; i < len; ++i) {
ser_putc(psc, argv[3][i]);
printf("%c", ser_getc(psc));
}
printf("\n\n");
- }
- return 0;
+}
+#define BUZZER_GPT (MPC5XXX_GPT + 0x60) /* GPT6 */ +static void buzzer_turn_on(unsigned int freq) +{
- struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)(BUZZER_GPT);
- const u32 prescale = gd->ipb_clk / freq / 128;
- const u32 count = 128;
- const u32 width = 64;
- gpt->cir = (prescale << 16) | count;
- gpt->pwmcr = width << 16;
- gpt->emsr = 3; /* Timer enabled for PWM */
+}
+static void buzzer_turn_off(void) +{
- struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)(BUZZER_GPT);
please add a blank line
- gpt->emsr = 0;
+}
+TEST_NAME(buzzer, "inkadiag buzzer"); +TEST_USAGE(buzzer, "Usage: %s\n\
<period> <freq> # turn buzzer on for period ms with freq hz\n\
+");
+static int TEST_FUNCTION(buzzer) (cmd_tbl_t *cmdtp, int flag, int argc,
char *argv[]) {
- if (argc != 3)
PRINT_TEST_USAGE(buzzer);
- argc--;
- argv++;
- unsigned int period = simple_strtol(argv[0], NULL, 0);
declaration must at the beginning of the function
- if (!period)
printf("Zero period is senseless\n");
- argc--;
- argv++;
- unsigned int freq = simple_strtol(argv[0], NULL, 0);
declaration must at the beginning of the function
- /* avoid zero prescale in buzzer_turn_on() */
- if (freq > gd->ipb_clk / 128)
printf("%dHz exceeds maximum (%dHz)\n", freq,
gd->ipb_clk / 128);
- else if (!freq)
printf("Zero frequency is senseless\n");
- else
buzzer_turn_on(freq);
- clear_ctrlc();
- int prev = disable_ctrlc(0);
declaration must at the beginning of the function
- printf("Buzzing for %d ms. Type ^C to abort!\n\n", period);
- int i = 0;
- while (!ctrlc() && (i++ < CONFIG_SYS_HZ))
udelay(period);
- clear_ctrlc();
- disable_ctrlc(prev);
- buzzer_turn_off();
- return 0;
+}
Best Regards, J.

Dear ap@denx.de,
In message 1227214731-19542-1-git-send-email-ap@denx.de you wrote:
RnJvbTogQW5kcmVhcyBQZmVmZmVybGUgPGFwQGRlbnguZGU+CgpUaGlzIHBhdGNoIGFkZHMgZGlh Z25vc2lzIGZ1bmN0aW9ucyBmb3IgdGhlIGJ1enplciwgVUFSVHMgYW5kIGRpZ2l0YWwKSU9zIG9u IGlua2E0eDAgaGFyZHdhcmUuCsKgIMKgClNpZ25lZC1vZmYtYnk6IEFuZHJlYXMgUGZlZmZlcmxl IDxhcEBkZW54LmRlPgotLS0KIGJvYXJkL2lua2E0eDAvTWFrZWZpbGUgICB8ICAgIDIgKy0KIGJv YXJkL2lua2E0eDAvaW5rYWRpYWcuYyB8ICA1MTUgKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrKysrKwogMiBmaWxlcyBjaGFuZ2VkLCA1MTYgaW5zZXJ0aW9ucygrKSwg MSBkZWxldGlvbnMoLSkKIGNyZWF0ZSBtb2RlIDEwMDY0NCBib2FyZC9pbmthNHgwL2lua2FkaWFn LmMKCmRpZmYgLS1naXQgYS9ib2FyZC9pbmthNHgwL01ha2VmaWxlIGIvYm9hcmQvaW5rYTR4MC9N YWtlZmlsZQppbmRleCA0NDJlMmQwLi4yMjY0ZGFlIDEwMDY0NAotLS0gYS9ib2FyZC9pbmthNHgw L01ha2VmaWxlCisrKyBiL2JvYXJkL2lua2E0eDAvTWFrZWZpbGUKQEAgLTI1LDcgKzI1LDcgQEAg aW5jbHVkZSAkKFRPUERJUikvY29uZmlnLm1rCiAKIExJQgk9ICQob2JqKWxpYiQoQk9BUkQpLmEK IAotQ09CSlMJOj0gJChCT0FSRCkubworQ09CSlMJOj0gJChCT0FSRCkubyAgaW5rYWRpYWcubwog CiBTUkNTCTo9ICQoU09CSlM6Lm89LlMpICQoQ09CSlM6Lm89LmMpCiBPQkpTCTo9ICQoYWRkcHJl Zml4ICQob2JqKSwkKENPQkpTKSkKZGlmZiAtLWdpdCBhL2JvYXJkL2lua2E0eDAvaW5rYWRpYWcu
...
Please do not post base64 encoded messages!
Any further such postings will be ignored.
This patch adds diagnosis functions for the buzzer, UARTs and digital
Such hardware test code should be implemented as part of the POST system, i. e. please create a post/board/inka4x0/ directory for it, and make it part of the POST.
diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c new file mode 100644 index 0000000..6194b58 --- /dev/null +++ b/board/inka4x0/inkadiag.c
...
+#define LED_HIGH(NUM)\ +out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\
in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) | 0x10)
+#define LED_LOW(NUM)\ +out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\
in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) & ~0x10)
Please use "do { ...} while (0)" wrappers for these macros.
+static int io_helper(int argc, char *argv[]) +{
- unsigned int state = inka_digin_get_input();
- if (strcmp(argv[1], "drawer1") == 0) {
printf("exit code: 0x%X\n",
(state & DIGIN_DRAWER_SW1) >> (ffs(DIGIN_DRAWER_SW1) - 1));
- } else if (strcmp(argv[1], "drawer2") == 0) {
printf("exit code: 0x%X\n",
(state & DIGIN_DRAWER_SW2) >> (ffs(DIGIN_DRAWER_SW2) - 1));
- } else if (strcmp(argv[1], "other") == 0) {
printf("exit code: 0x%X\n",
((state & DIGIN_KEYB_MASK) >> (ffs(DIGIN_KEYB_MASK) - 1))
| ((state & DIGIN_TOUCHSCR_MASK) >>
(ffs(DIGIN_TOUCHSCR_MASK) - 2)));
- } else {
printf("Invalid argument: %s\n", argv[1]);
return -1;
- }
- return 0;
Please use the existing command parser code here and below.
+DECLARE_GLOBAL_DATA_PTR;
+static int ser_init(volatile struct mpc5xxx_psc *psc, int baudrate) +{
Maybe there should be some comments what these functions are doing?
+static void ser_putc(volatile struct mpc5xxx_psc *psc, const char c) +{
- /* Wait for last character to go. */
- int i = 0;
- while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < CONFIG_SYS_HZ))
udelay(10);
- psc->psc_buffer_8 = c;
What exactly has CONFIG_SYS_HZ to do with that? I cannot understand this sort of magic.
+static int ser_getc(volatile struct mpc5xxx_psc *psc) +{
- /* Wait for a character to arrive. */
- int i = 0;
- while (!(psc->psc_status & PSC_SR_RXRDY) && (i++ < CONFIG_SYS_HZ))
udelay(10);
Ditto here.
- return psc->psc_buffer_8;
+}
+#define SERIAL_PORT_BASE (u_char *)0x80000000
+#define UART_RX 0 /* In: Receive buffer (DLAB=0) */ +#define UART_TX 0 /* Out: Transmit buffer (DLAB=0) */ +#define UART_DLL 0 /* Out: Divisor Latch Low (DLAB=1) */
+#define UART_LCR 3 /* Out: Line Control Register */ +#define UART_MCR 4 /* Out: Modem Control Register */
...
Lots of #defines right in the middle of a C file? That's uygly. Please move these in a separate header file (or at least at the begin of the C file).
+static unsigned char serial_in(unsigned char num, int offset) +{
- return in_8(SERIAL_PORT_BASE + (num << 3) + offset);
+}
+static void serial_out(unsigned char num, int offset, unsigned char value) +{
- out_8(SERIAL_PORT_BASE + (num << 3) + offset, value);
+}
The amount of comments in this file sucessfully avoids all risks of fatiguing the reader...
- if (strcmp(argv[2], "115200") == 0)
combrd = 1;
- else if (strcmp(argv[2], "57600") == 0)
combrd = 2;
- else if (strcmp(argv[2], "38400") == 0)
combrd = 3;
- else if (strcmp(argv[2], "19200") == 0)
combrd = 6;
- else if (strcmp(argv[2], "9600") == 0)
combrd = 12;
- else if (strcmp(argv[2], "2400") == 0)
combrd = 48;
- else if (strcmp(argv[2], "1200") == 0)
combrd = 96;
- else if (strcmp(argv[2], "300") == 0)
combrd = 384;
- else
printf("Invalid baudrate: %s", argv[2]);
Instead of calling strcmp() again and again, you could convert the number just once and use a switch. Or table lookup. See existing code.
if (mode & 1)
/* turn on 'loopback' mode */
serial_out(num, UART_MCR, UART_MCR_LOOP);
else {
Multiline statements require braces.
/* establish the UART's operational parameters */
/* set DLAB=1 */
Multiline comment style is
/* * establish the UART's operational parameters * set DLAB=1 */
volatile struct mpc5xxx_psc *psc =
(struct mpc5xxx_psc *)address;
Indentation not by TAB.
- if (freq > gd->ipb_clk / 128)
printf("%dHz exceeds maximum (%dHz)\n", freq,
gd->ipb_clk / 128);
- else if (!freq)
Multiline statements require braces.
+static int TEST_FUNCTION(inkadiag) (cmd_tbl_t *cmdtp, int flag, int argc,
char *argv[]) {
- switch (argc) {
- case 1:
PRINT_TEST_USAGE(inkadiag);
"break;" missing?
- default:
argc--;
argv++;
CHECK_TEST(io);
CHECK_TEST(serial);
CHECK_TEST(buzzer);
- }
- PRINT_TEST_USAGE(inkadiag);
+}
+U_BOOT_CMD(inkadiag, 6, 1, TEST_FUNCTION(inkadiag),
"inkadiag- inka diagnosis\n",
"[inkadiag what ...]\n"
" - perform a diagnosis on inka hardware\n"
"'inkadiag' performs hardware tests.\n\n"
"Without arguments, inkadiag prints a list of available tests.\n\n"
"To get detailed help information for specific tests you can type\n"
"'inkadiag what' with a test name 'what' as argument.\n");
Please no prose here - be terse, please.
Viele Grüße,
Wolfgang Denk

Hi Wolfgang,
This patch adds diagnosis functions for the buzzer, UARTs and digital
Such hardware test code should be implemented as part of the POST system, i. e. please create a post/board/inka4x0/ directory for it, and make it part of the POST.
But these tests cannot be part of any Power On Self Test as some of them are interactive. They were designed with the serial port being scripted - thus yielding an automated setup as a whole, but still such scripting is needed.
So I still see these tests to belong into the board directory.
Cheers Detlev
participants (4)
-
ap@denx.de
-
Detlev Zundel
-
Jean-Christophe PLAGNIOL-VILLARD
-
Wolfgang Denk