[U-Boot] [PATCH v1 0/7] Update for inka4x0 plus some new features

[U-Boot] [PATCH v1 0/7] Update for inka4x0 plus some new features

[Detlev Zundel]

This patchset contains updates for the ink4x0 platform.  Namely it
adds hardware diagnostic funtions and an RTC driver.

For a nice integration by using private command tables, do_help is
extended to support this scenario also.

To successfully convert all the pointer access to accessor macros, a
bunch of new defines is also needed for the 5200 cpu.

Andreas Pfefferle (1):
  rtc: add support for 4543 RTC (manufactured by e.g. EPSON)

Detlev Zundel (6):
  command.c: Expose the core of do_help as _do_help to the rest of
    u-boot.
  mpc5xxx: Add structure definition for several more register blocks.
  drivers/twserial: Add protocol driver for "three wire serial"
    interface.
  inka4x0: Add hardware diagnosis functions for inka4x0
  inka4x0: Add hardware diagnosis and RTC in configuration.
  inka4x0: Use proper accessor macros for memory mapped registers.

 Makefile                    |    3 +-
 board/inka4x0/Makefile      |    4 +-
 board/inka4x0/inka4x0.c     |  162 ++++++++-----
 board/inka4x0/inkadiag.c    |  541 +++++++++++++++++++++++++++++++++++++++++++
 common/command.c            |   20 +-
 drivers/rtc/Makefile        |    1 +
 drivers/rtc/rtc4543.c       |  118 ++++++++++
 drivers/twserial/Makefile   |   46 ++++
 drivers/twserial/soft_tws.c |  110 +++++++++
 include/command.h           |    4 +-
 include/configs/inka4x0.h   |   91 +++++++-
 include/mpc5xxx.h           |  124 ++++++++++
 include/rtc.h               |    4 +
 include/tws.h               |   46 ++++
 14 files changed, 1196 insertions(+), 78 deletions(-)
 create mode 100644 board/inka4x0/inkadiag.c
 create mode 100644 drivers/rtc/rtc4543.c
 create mode 100644 drivers/twserial/Makefile
 create mode 100644 drivers/twserial/soft_tws.c
 create mode 100644 include/tws.h

[U-Boot] [PATCH] command.c: Expose the core of do_help as _do_help to the rest of u-boot.

[Detlev Zundel]

Other commands implementing subcommands can reuse this code nicely.

Signed-off-by: Detlev Zundel <dzu@denx.de>
Signed-off-by: Andreas Pfefferle <ap@denx.de>
---
 common/command.c  |   20 +++++++++++++-------
 include/command.h |    4 +++-
 2 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/common/command.c b/common/command.c
index 3b9ccc9..c9a3f5b 100644
--- a/common/command.c
+++ b/common/command.c
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2000-2003
+ * (C) Copyright 2000-2009
  * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
  *
  * See file CREDITS for list of people who contributed to this
@@ -233,20 +233,19 @@ U_BOOT_CMD(
  * Use puts() instead of printf() to avoid printf buffer overflow
  * for long help messages
  */
-int do_help (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+
+int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int
+	      flag, int argc, char *argv[])
 {
 	int i;
 	int rcode = 0;
 
 	if (argc == 1) {	/*show list of commands */
-
-		int cmd_items = &__u_boot_cmd_end -
-				&__u_boot_cmd_start;	/* pointer arith! */
 		cmd_tbl_t *cmd_array[cmd_items];
 		int i, j, swaps;
 
 		/* Make array of commands from .uboot_cmd section */
-		cmdtp = &__u_boot_cmd_start;
+		cmdtp = cmd_start;
 		for (i = 0; i < cmd_items; i++) {
 			cmd_array[i] = cmdtp++;
 		}
@@ -286,7 +285,7 @@ int do_help (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	 * command help (long version)
 	 */
 	for (i = 1; i < argc; ++i) {
-		if ((cmdtp = find_cmd (argv[i])) != NULL) {
+		if ((cmdtp = find_cmd_tbl (argv[i], cmd_start, cmd_items )) != NULL) {
 #ifdef	CONFIG_SYS_LONGHELP
 			/* found - print (long) help info */
 			puts (cmdtp->name);
@@ -313,6 +312,13 @@ int do_help (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
 	return rcode;
 }
 
+int do_help (cmd_tbl_t * cmdtp, int flag, int argc, char *argv[])
+{
+	return _do_help(&__u_boot_cmd_start,
+			&__u_boot_cmd_end - &__u_boot_cmd_start,
+			cmdtp, flag, argc, argv);
+}
+
 
 U_BOOT_CMD(
 	help,	CONFIG_SYS_MAXARGS,	1,	do_help,
diff --git a/include/command.h b/include/command.h
index d7321af..bd47853 100644
--- a/include/command.h
+++ b/include/command.h
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright 2000
+ * (C) Copyright 2000-2009
  * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
  *
  * See file CREDITS for list of people who contributed to this
@@ -66,6 +66,8 @@ extern cmd_tbl_t  __u_boot_cmd_end;
 
 
 /* common/command.c */
+int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp, int
+	      flag, int argc, char *argv[]);
 cmd_tbl_t *find_cmd(const char *cmd);
 cmd_tbl_t *find_cmd_tbl (const char *cmd, cmd_tbl_t *table, int table_len);
 
-- 
1.6.0.6

[U-Boot] [PATCH] mpc5xxx: Add structure definition for several more register blocks.

[Detlev Zundel]

Signed-off-by: Detlev Zundel <dzu@denx.de>
---
 include/mpc5xxx.h |  124 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 124 insertions(+), 0 deletions(-)

diff --git a/include/mpc5xxx.h b/include/mpc5xxx.h
index 6138d45..463d5ae 100644
--- a/include/mpc5xxx.h
+++ b/include/mpc5xxx.h
@@ -30,6 +30,8 @@
 #ifndef __ASMPPC_MPC5XXX_H
 #define __ASMPPC_MPC5XXX_H
 
+#include <asm/types.h>
+
 /* Processor name */
 #if defined(CONFIG_MPC5200)
 #define CPU_ID_STR	"MPC5200"
@@ -217,6 +219,12 @@
 #define MPC5XXX_GPIO_SIMPLE_PSC1_1   0x00000002UL
 #define MPC5XXX_GPIO_SIMPLE_PSC1_0   0x00000001UL
 
+#define MPC5XXX_GPIO_SINT_ETH_16     0x80
+#define MPC5XXX_GPIO_SINT_ETH_15     0x40
+#define MPC5XXX_GPIO_SINT_ETH_14     0x20
+#define MPC5XXX_GPIO_SINT_ETH_13     0x10
+#define MPC5XXX_GPIO_SINT_USB1_9     0x08
+#define MPC5XXX_GPIO_SINT_PSC3_8     0x04
 #define MPC5XXX_GPIO_SINT_PSC3_5     0x02
 #define MPC5XXX_GPIO_SINT_PSC3_4     0x01
 
@@ -454,6 +462,99 @@
 							   IORDY protocol */
 
 #ifndef __ASSEMBLY__
+/* Memory map registers */
+struct mpc5xxx_mmap_ctl {
+	volatile u32	mbar;
+	volatile u32	cs0_start;	/* 0x0004 */
+	volatile u32	cs0_stop;
+	volatile u32	cs1_start;	/* 0x000c */
+	volatile u32	cs1_stop;
+	volatile u32	cs2_start;	/* 0x0014 */
+	volatile u32	cs2_stop;
+	volatile u32	cs3_start;	/* 0x001c */
+	volatile u32	cs3_stop;
+	volatile u32	cs4_start;	/* 0x0024 */
+	volatile u32	cs4_stop;
+	volatile u32	cs5_start;	/* 0x002c */
+	volatile u32	cs5_stop;
+#if defined(CONFIG_MGT5100)
+	volatile u32	sdram_start;	/* 0x0034 */
+	volatile u32	sdram_stop;	/* 0x0038 */
+	volatile u32	pci1_start;	/* 0x003c */
+	volatile u32	pci1_stop;	/* 0x0040 */
+	volatile u32	pci2_start;	/* 0x0044 */
+	volatile u32	pci2_stop;	/* 0x0048 */
+#elif defined(CONFIG_MPC5200)
+	volatile u32	sdram0;		/* 0x0034 */
+	volatile u32	sdram1;		/* 0x0038 */
+	volatile u32	dummy1[4];	/* 0x003c */
+#endif
+	volatile u32	boot_start;	/* 0x004c */
+	volatile u32	boot_stop;
+#if defined(CONFIG_MGT5100)
+	volatile u32	addecr;		/* 0x0054 */
+#elif defined(CONFIG_MPC5200)
+	volatile u32	ipbi_ws_ctrl;	/* 0x0054 */
+#endif
+#if defined(CONFIG_MPC5200)
+	volatile u32	cs6_start;	/* 0x0058 */
+	volatile u32	cs6_stop;
+	volatile u32	cs7_start;	/* 0x0060 */
+	volatile u32	cs7_stop;
+#endif
+};
+
+/* Clock distribution module */
+struct mpc5xxx_cdm {
+	volatile u32	jtagid;		/* 0x0000 */
+	volatile u32	porcfg;
+	volatile u32	brdcrmb;	/* 0x0008 */
+	volatile u32	cfg;
+	volatile u32	fourtyeight_fdc;/* 0x0010 */
+	volatile u32	clock_enable;
+	volatile u32	system_osc;	/* 0x0018 */
+	volatile u32	ccscr;
+	volatile u32	sreset;		/* 0x0020 */
+	volatile u32	pll_status;
+	volatile u32	psc1_mccr;	/* 0x0028 */
+	volatile u32	psc2_mccr;
+	volatile u32	psc3_mccr;	/* 0x0030 */
+	volatile u32	psc6_mccr;
+};
+
+/* SDRAM controller */
+struct mpc5xxx_sdram {
+	volatile u32	mode;
+	volatile u32	ctrl;
+	volatile u32	config1;
+	volatile u32	config2;
+#if defined(CONFIG_MGT5100)
+	volatile u32	xlbsel;
+	volatile u32    dummy[31];
+#else
+	volatile u32	dummy[32];
+#endif
+	volatile u32	sdelay;
+};
+
+struct mpc5xxx_lpb {
+	volatile u32	cs0_cfg;
+	volatile u32	cs1_cfg;
+	volatile u32	cs2_cfg;
+	volatile u32	cs3_cfg;
+	volatile u32	cs4_cfg;
+	volatile u32	cs5_cfg;
+	volatile u32	cs_ctrl;
+	volatile u32	cs_status;
+#if defined(CONFIG_MPC5200)
+	volatile u32	cs6_cfg;
+	volatile u32	cs7_cfg;
+	volatile u32	cs_burst;
+	volatile u32	cs_deadcycle;
+#endif
+};
+
+
 struct mpc5xxx_psc {
 	volatile u8	mode;		/* PSC + 0x00 */
 	volatile u8	reserved0[3];
@@ -596,6 +697,29 @@ struct mpc5xxx_gpio {
 	volatile u8 reserved10;		/* GPIO + 0x3f */
 };
 
+struct mpc5xxx_wu_gpio {
+	volatile u8 enable;		/* WU_GPIO + 0x00 */
+	volatile u8 reserved1[3];	/* WU_GPIO + 0x01 */
+	volatile u8 ode;		/* WU_GPIO + 0x04 */
+	volatile u8 reserved2[3];	/* WU_GPIO + 0x05 */
+	volatile u8 ddr;		/* WU_GPIO + 0x08 */
+	volatile u8 reserved3[3];	/* WU_GPIO + 0x09 */
+	volatile u8 dvo;		/* WU_GPIO + 0x0c */
+	volatile u8 reserved4[3];	/* WU_GPIO + 0x0d */
+	volatile u8 inten;		/* WU_GPIO + 0x10 */
+	volatile u8 reserved5[3];	/* WU_GPIO + 0x11 */
+	volatile u8 iinten;		/* WU_GPIO + 0x14 */
+	volatile u8 reserved6[3];	/* WU_GPIO + 0x15 */
+	volatile u16 itype;		/* WU_GPIO + 0x18 */
+	volatile u8 reserved7[2];	/* WU_GPIO + 0x1a */
+	volatile u8 master_enable;	/* WU_GPIO + 0x1c */
+	volatile u8 reserved8[3];	/* WU_GPIO + 0x1d */
+	volatile u8 ival;		/* WU_GPIO + 0x20 */
+	volatile u8 reserved9[3];	/* WU_GPIO + 0x21 */
+	volatile u8 status;		/* WU_GPIO + 0x24 */
+	volatile u8 reserved10[3];	/* WU_GPIO + 0x25 */
+};
+
 struct mpc5xxx_sdma {
 	volatile u32 taskBar;		/* SDMA + 0x00 */
 	volatile u32 currentPointer;	/* SDMA + 0x04 */
-- 
1.6.0.6

[U-Boot] [PATCH] drivers/twserial: Add protocol driver for "three wire serial" interface.

[Detlev Zundel]

This pretty unintelligent interface is used on some RTC chips.

Signed-off-by: Detlev Zundel <dzu@denx.de>
---
 Makefile                    |    3 +-
 drivers/twserial/Makefile   |   46 ++++++++++++++++++
 drivers/twserial/soft_tws.c |  110 +++++++++++++++++++++++++++++++++++++++++++
 include/tws.h               |   46 ++++++++++++++++++
 4 files changed, 204 insertions(+), 1 deletions(-)
 create mode 100644 drivers/twserial/Makefile
 create mode 100644 drivers/twserial/soft_tws.c
 create mode 100644 include/tws.h

diff --git a/Makefile b/Makefile
index 63b9909..7c37845 100644
--- a/Makefile
+++ b/Makefile
@@ -1,5 +1,5 @@
 #
-# (C) Copyright 2000-2008
+# (C) Copyright 2000-2009
 # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
 #
 # See file CREDITS for list of people who contributed to this
@@ -262,6 +262,7 @@ TAG_SUBDIRS += cpu/mpc8xxx
 endif
 LIBS += drivers/rtc/librtc.a
 LIBS += drivers/serial/libserial.a
+LIBS += drivers/twserial/libtws.a
 LIBS += drivers/usb/libusb.a
 LIBS += drivers/video/libvideo.a
 LIBS += common/libcommon.a
diff --git a/drivers/twserial/Makefile b/drivers/twserial/Makefile
new file mode 100644
index 0000000..0b059f3
--- /dev/null
+++ b/drivers/twserial/Makefile
@@ -0,0 +1,46 @@
+#
+# (C) Copyright 2009
+# Detlev Zundel, DENX Software Engineering, dzu at 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 $(TOPDIR)/config.mk
+
+LIB	:= $(obj)libtws.a
+
+COBJS-$(CONFIG_SOFT_TWS) += soft_tws.o
+
+COBJS	:= $(COBJS-y)
+SRCS	:= $(COBJS:.o=.c)
+OBJS	:= $(addprefix $(obj),$(COBJS))
+
+all:	$(LIB)
+
+$(LIB):	$(obj).depend $(OBJS)
+	$(AR) $(ARFLAGS) $@ $(OBJS)
+
+#########################################################################
+
+# defines $(obj).depend target
+include $(SRCTREE)/rules.mk
+
+sinclude $(obj).depend
+
+#########################################################################
diff --git a/drivers/twserial/soft_tws.c b/drivers/twserial/soft_tws.c
new file mode 100644
index 0000000..f63fc3f
--- /dev/null
+++ b/drivers/twserial/soft_tws.c
@@ -0,0 +1,110 @@
+/*
+ * (C) Copyright 2009
+ * Detlev Zundel, DENX Software Engineering, dzu at 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
+ *
+ */
+
+#define TWS_IMPLEMENTATION
+#include <common.h>
+
+/*=====================================================================*/
+/*                         Public Functions                            */
+/*=====================================================================*/
+
+/*-----------------------------------------------------------------------
+ * Read bits
+ */
+int tws_read(uchar *buffer, int len)
+{
+	int rem = len;
+	uchar accu, shift;
+
+	debug("tws_read: buffer %p len %d\n", buffer, len);
+
+	/* Configure the data pin for input */
+	tws_data_config_output(0);
+
+	/* Disable WR, i.e. setup a read */
+	tws_wr(0);
+	udelay(1);
+
+	/* Rise CE */
+	tws_ce(1);
+	udelay(1);
+
+	for (; rem > 0; ) {
+		for (shift = 0, accu = 0;
+		     (rem > 0) && (shift < 8);
+		     rem--, shift++) {
+			tws_clk(1);
+			udelay(10);
+			accu |= (tws_data_read() << shift); /* LSB first */
+			tws_clk(0);
+			udelay(10);
+		}
+		*buffer++ = accu;
+	}
+
+	/* Lower CE */
+	tws_ce(0);
+
+	return len - rem;
+}
+
+
+/*-----------------------------------------------------------------------
+ * Write bits
+ */
+int tws_write(uchar *buffer, int len)
+{
+	int rem = len;
+	uchar accu, shift;
+	debug("tws_write: buffer %p len %d\n", buffer, len);
+
+	/* Configure the data pin for output */
+	tws_data_config_output(1);
+
+	/* Enable WR, i.e. setup a write */
+	tws_wr(1);
+	udelay(1);
+
+	/* Rise CE */
+	tws_ce(1);
+	udelay(1);
+
+	for (; rem > 0; ) {
+		for (shift = 0, accu = *buffer++;
+		     (rem > 0) && (shift < 8);
+		     rem--, shift++) {
+			tws_data(accu & 0x01); /* LSB first */
+			tws_clk(1);
+			udelay(10);
+			tws_clk(0);
+			udelay(10);
+			accu >>= 1;
+		}
+	}
+
+	/* Lower CE */
+	tws_ce(0);
+
+	return len - rem;
+}
diff --git a/include/tws.h b/include/tws.h
new file mode 100644
index 0000000..35145d7
--- /dev/null
+++ b/include/tws.h
@@ -0,0 +1,46 @@
+/*
+ * (C) Copyright 2009
+ * Detlev Zundel, DENX Software Engineering, dzu at 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
+ *
+ */
+
+#ifndef _TWS_H_
+#define _TWS_H_
+
+#ifndef TWS_SOFT_DECLARATIONS
+# if defined(CONFIG_MPC5xxx)
+#  define TWS_SOFT_DECLARATIONS volatile ioport_t *iop = ioport_addr((immap_t *)CONFIG_SYS_IMMR, I2C_PORT);
+# else
+#  define TWS_SOFT_DECLARATIONS
+# endif
+#endif
+
+/*
+ * Read/Write interface:
+ *   buffer:  Where to read/write the data
+ *   len:     How many bits to read/write
+ *
+ *   Returns: 0 on success, not 0 on failure
+ */
+int tws_read(uchar *buffer, int len);
+int tws_write(uchar *buffer, int len);
+
+#endif	/* _TWS_H_ */
-- 
1.6.0.6

[U-Boot] [PATCH] rtc: add support for 4543 RTC (manufactured by e.g. EPSON)

[Detlev Zundel]

From: Andreas Pfefferle <ap@denx.de>

Signed-off-by: Detlev Zundel <dzu@denx.de>
Signed-off-by: Andreas Pfefferle <ap@denx.de>
---
 drivers/rtc/Makefile  |    1 +
 drivers/rtc/rtc4543.c |  118 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/rtc.h         |    4 ++
 3 files changed, 123 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..242d9bc
--- /dev/null
+++ b/drivers/rtc/rtc4543.c
@@ -0,0 +1,118 @@
+/*
+ * (C) Copyright 2008, 2009
+ * Andreas Pfefferle, DENX Software Engineering, ap at 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>
+#include <tws.h>
+
+#if defined(CONFIG_CMD_DATE)
+
+/*
+ * Note: The acrobatics below is due to the hideously ingenius idea of
+ * the chip designers.  As the chip does not allow register
+ * addressing, all values need to be read and written in one go.  Sure
+ * enough, the 'wday' field (0-6) is transferred using the economic
+ * number of 4 bits right in the middle of the packet.....
+ */
+
+int rtc_get(struct rtc_time *tm)
+{
+	int rel = 0;
+	uchar buffer[7];
+
+	memset(buffer, 0, 7);
+
+	/* Read 52 bits into our buffer */
+	tws_read(buffer, 52);
+
+	tm->tm_sec  = BCD2BIN( buffer[0] & 0x7F);
+	tm->tm_min  = BCD2BIN( buffer[1] & 0x7F);
+	tm->tm_hour = BCD2BIN( buffer[2] & 0x3F);
+	tm->tm_wday = BCD2BIN( buffer[3] & 0x07);
+	tm->tm_mday = BCD2BIN((buffer[3] & 0xF0) >> 4 | (buffer[4] & 0x0F) << 4);
+	tm->tm_mon  = BCD2BIN((buffer[4] & 0x30) >> 4 | (buffer[5] & 0x0F) << 4);
+	tm->tm_year = BCD2BIN((buffer[5] & 0xF0) >> 4 | (buffer[6] & 0x0F) << 4) + 2000;
+	tm->tm_yday = 0;
+	tm->tm_isdst = 0;
+
+	if (tm->tm_sec & 0x80) {
+		puts("### Warning: RTC Low Voltage - date/time not reliable\n");
+		rel = -1;
+	}
+
+	debug("Get DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
+		tm->tm_year, tm->tm_mon, tm->tm_mday, tm->tm_wday,
+		tm->tm_hour, tm->tm_min, tm->tm_sec);
+
+	return rel;
+}
+
+int rtc_set(struct rtc_time *tm)
+{
+	uchar buffer[7];
+	uchar tmp;
+
+	debug("Set DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
+		tm->tm_year, tm->tm_mon, tm->tm_mday, tm->tm_wday,
+		tm->tm_hour, tm->tm_min, tm->tm_sec);
+
+	memset(buffer, 0, 7);
+	buffer[0] = BIN2BCD(tm->tm_sec);
+	buffer[1] = BIN2BCD(tm->tm_min);
+	buffer[2] = BIN2BCD(tm->tm_hour);
+	buffer[3] = BIN2BCD(tm->tm_wday);
+	tmp = BIN2BCD(tm->tm_mday);
+	buffer[3] |= (tmp & 0x0F) << 4;
+	buffer[4] =  (tmp & 0xF0) >> 4;
+	tmp = BIN2BCD(tm->tm_mon);
+	buffer[4] |= (tmp & 0x0F) << 4;
+	buffer[5] =  (tmp & 0xF0) >> 4;
+	tmp = BIN2BCD(tm->tm_year  % 100);
+	buffer[5] |= (tmp & 0x0F) << 4;
+	buffer[6] =  (tmp & 0xF0) >> 4;
+
+	/* Write the resulting 52 bits to device */
+	tws_write(buffer, 52);
+
+	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 = 2000;
+	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_ */
-- 
1.6.0.6

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Detlev Zundel]

This patch adds advanced diagnosis functions for the inka4x0 board.

Signed-off-by: Andreas Pfefferle <ap@denx.de>
Signed-off-by: Detlev Zundel <ap@denx.de>
---
 board/inka4x0/Makefile    |    4 +-
 board/inka4x0/inka4x0.c   |   19 ++-
 board/inka4x0/inkadiag.c  |  541 +++++++++++++++++++++++++++++++++++++++++++++
 include/configs/inka4x0.h |    1 +
 4 files changed, 561 insertions(+), 4 deletions(-)
 create mode 100644 board/inka4x0/inkadiag.c

diff --git a/board/inka4x0/Makefile b/board/inka4x0/Makefile
index 442e2d0..82aa950 100644
--- a/board/inka4x0/Makefile
+++ b/board/inka4x0/Makefile
@@ -1,5 +1,5 @@
 #
-# (C) Copyright 2003-2006
+# (C) Copyright 2003-2009
 # Wolfgang Denk, DENX Software Engineering, wd at denx.de.
 #
 # See file CREDITS for list of people who contributed to this
@@ -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/inka4x0.c b/board/inka4x0/inka4x0.c
index 507196b..ea59db3 100644
--- a/board/inka4x0/inka4x0.c
+++ b/board/inka4x0/inka4x0.c
@@ -1,6 +1,9 @@
 /*
- * (C) Copyright 2003-2004
- * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+ * (C) Copyright 2008-2009
+ * Andreas Pfefferle, DENX Software Engineering, ap at denx.de.
+ *
+ * (C) Copyright 2009
+ * Detlev Zundel, DENX Software Engineering, dzu at denx.de.
  *
  * (C) Copyright 2004
  * Mark Jonas, Freescale Semiconductor, mark.jonas at motorola.com.
@@ -8,6 +11,9 @@
  * (C) Copyright 2004
  * Martin Krause, TQ-Systems GmbH, martin.krause at tqs.de
  *
+ * (C) Copyright 2003-2004
+ * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
+ *
  * See file CREDITS for list of people who contributed to this
  * project.
  *
@@ -171,6 +177,15 @@ void flash_preinit(void)
 	*(vu_long *)MPC5XXX_BOOTCS_CFG &= ~0x1; /* clear RO */
 }
 
+int misc_init_r (void) {
+	extern int inkadiag_init_r (void);
+	/*
+	  * The command table used for the subcommands of inkadiag
+	  * needs to be relocated manually.
+	  */
+	return inkadiag_init_r();
+}
+
 int misc_init_f (void)
 {
 	char tmp[10];
diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c
new file mode 100644
index 0000000..bdbf652
--- /dev/null
+++ b/board/inka4x0/inkadiag.c
@@ -0,0 +1,541 @@
+/*
+ * (C) Copyright 2008, 2009 Andreas Pfefferle,
+ *     DENX Software Engineering, ap at denx.de.
+ * (C) Copyright 2009 Detlev Zundel,
+ *     DENX Software Engineering, dzu at 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 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 */
+
+#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
+
+/*
+ * Table with supported baudrates (defined in inka4x0.h)
+ */
+static const unsigned long baudrate_table[] = CONFIG_SYS_BAUDRATE_TABLE;
+#define	N_BAUDRATES (sizeof(baudrate_table) / sizeof(baudrate_table[0]))
+
+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) \
+    do { \
+	    out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,		\
+		     in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) | 0x10); \
+    } while (0)
+
+#define LED_LOW(NUM) \
+    do { \
+	    out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,		\
+		     in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) & ~0x10); \
+    } while (0)
+
+#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) {
+		/* other */
+		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) {
+			/* drawer1 */
+			if (state) {
+				gpio->simple_dvo &= ~0x1000;
+				udelay(1);
+				gpio->simple_dvo |= 0x1000;
+			} else {
+				gpio->simple_dvo |= 0x1000;
+				udelay(1);
+				gpio->simple_dvo &= ~0x1000;
+			}
+		}
+		if (which == 2) {
+			/* drawer 2 */
+			if (state) {
+				gpio->simple_dvo &= ~0x2000;
+				udelay(1);
+				gpio->simple_dvo |= 0x2000;
+			} else {
+				gpio->simple_dvo |= 0x2000;
+				udelay(1);
+				gpio->simple_dvo &= ~0x2000;
+			}
+		}
+	}
+	udelay(1);
+}
+
+static int do_inkadiag_io(cmd_tbl_t *cmdtp, int flag, int argc,
+			  char *argv[]) {
+	unsigned int state, val;
+
+	switch (argc) {
+	case 3:
+		/* Write a value */
+		val = simple_strtol(argv[2], NULL, 16);
+
+		if (strcmp(argv[1], "drawer1") == 0) {
+			inka_digio_set_output(val, 1);
+		} else if (strcmp(argv[1], "drawer2") == 0) {
+			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;
+		}
+		/* fall through */
+	case 2:
+		/* Read a value */
+		state = inka_digin_get_input();
+
+		if (strcmp(argv[1], "drawer1") == 0) {
+			val = (state & DIGIN_DRAWER_SW1) >> (ffs(DIGIN_DRAWER_SW1) - 1);
+		} else if (strcmp(argv[1], "drawer2") == 0) {
+			val = (state & DIGIN_DRAWER_SW2) >> (ffs(DIGIN_DRAWER_SW2) - 1);
+		} else if (strcmp(argv[1], "other") == 0) {
+			val = ((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;
+		}
+		printf("exit code: 0x%X\n", val);
+		return 0;
+	default:
+		cmd_usage(cmdtp);
+		break;
+	}
+
+	return -1;
+}
+
+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 1 second for last character to go. */
+	int i = 0;
+	while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < 1000000/10))
+		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++ < 1000000/10))
+		udelay(10);
+
+	return psc->psc_buffer_8;
+}
+
+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);
+}
+
+static int do_inkadiag_serial(cmd_tbl_t *cmdtp, int flag, int argc,
+			      char *argv[]) {
+	if (argc < 5) {
+		cmd_usage(cmdtp);
+		return 1;
+	}
+
+	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;
+	int baudrate = simple_strtoul(argv[2], NULL, 10);
+	int i;
+	for (i=0; i<N_BAUDRATES; ++i) {
+		if (baudrate == baudrate_table[i])
+			break;
+	}
+	if (i == N_BAUDRATES) {
+		printf("## Baudrate %d bps not supported\n",
+		       baudrate);
+		return 1;
+	}
+	combrd = 115200 / baudrate;
+
+	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 = 0;
+		int 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;
+}
+
+static int do_inkadiag_buzzer(cmd_tbl_t *cmdtp, int flag, int argc,
+			      char *argv[]) {
+
+	if (argc != 3) {
+		cmd_usage(cmdtp);
+		return 1;
+	}
+
+	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 (%ldHz)\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;
+}
+
+static int do_inkadiag_help(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
+
+cmd_tbl_t cmd_inkadiag_sub[] = {
+	U_BOOT_CMD_MKENT(io, 1, 1, do_inkadiag_io, "read digital input",
+			 "<drawer1|drawer2|other> [value] - get or set specified signal\n"),
+	U_BOOT_CMD_MKENT(serial, 4, 1, do_inkadiag_serial, "test serial port",
+			 "<num> <mode> <baudrate> <msg>  - test uart num [0..11] in mode\n"
+			 "and baudrate with msg\n"),
+	U_BOOT_CMD_MKENT(buzzer, 2, 1, do_inkadiag_buzzer, "activate buzzer",
+			 "<period> <freq> - turn buzzer on for period ms with freq hz\n"),
+	U_BOOT_CMD_MKENT(help, 4, 1, do_inkadiag_help, "get help",
+			 "[command] - get help for command\n"),
+};
+
+static int do_inkadiag_help(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) {
+	extern int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp,
+			     int flag, int argc, char *argv[]);
+	/* do_help prints command name - we prepend inkadiag to our subcommands! */
+#ifdef CONFIG_SYS_LONGHELP
+	puts ("inkadiag ");
+#endif
+	return _do_help(&cmd_inkadiag_sub[0],
+		ARRAY_SIZE(cmd_inkadiag_sub), cmdtp, flag, argc, argv);
+}
+
+static int do_inkadiag(cmd_tbl_t *cmdtp, int flag, int argc,
+		       char *argv[]) {
+	cmd_tbl_t *c;
+
+	c = find_cmd_tbl(argv[1], &cmd_inkadiag_sub[0], ARRAY_SIZE(cmd_inkadiag_sub));
+
+	if (c) {
+		argc--;
+		argv++;
+		return c->cmd(c, flag, argc, argv);
+	} else {
+		/* Unrecognized command */
+		cmd_usage(cmdtp);
+		return 1;
+	}
+}
+
+U_BOOT_CMD(inkadiag, 6, 1, do_inkadiag,
+	   "inkadiag - inka diagnosis\n",
+	   "[inkadiag what ...]\n"
+	   "    - perform a diagnosis on inka hardware\n"
+	   "'inkadiag' performs hardware tests.\n\n");
+
+/* Relocate the command table functino pointers when running in RAM */
+int inkadiag_init_r (void) {
+	cmd_tbl_t *cmdtp;
+
+	for (cmdtp = &cmd_inkadiag_sub[0]; cmdtp !=
+		     &cmd_inkadiag_sub[ARRAY_SIZE(cmd_inkadiag_sub)]; cmdtp++) {
+		ulong addr;
+		addr = (ulong) (cmdtp->cmd) + gd->reloc_off;
+		cmdtp->cmd = (int (*)(struct cmd_tbl_s *, int, int, char *[]))addr;
+
+		addr = (ulong)(cmdtp->name) + gd->reloc_off;
+		cmdtp->name = (char *)addr;
+
+		if (cmdtp->usage) {
+			addr = (ulong)(cmdtp->usage) + gd->reloc_off;
+			cmdtp->usage = (char *)addr;
+		}
+#ifdef CONFIG_SYS_LONGHELP
+		if (cmdtp->help) {
+			addr = (ulong)(cmdtp->help) + gd->reloc_off;
+			cmdtp->help = (char *)addr;
+		}
+#endif
+	}
+	return 0;
+}
diff --git a/include/configs/inka4x0.h b/include/configs/inka4x0.h
index e42fa6d..b80796d 100644
--- a/include/configs/inka4x0.h
+++ b/include/configs/inka4x0.h
@@ -39,6 +39,7 @@
 #define BOOTFLAG_WARM		0x02	/* Software reboot			*/
 
 #define CONFIG_MISC_INIT_F	1	/* Use misc_init_f()			*/
+#define CONFIG_MISC_INIT_R	1	/* Use misc_init_r()			*/
 
 #define CONFIG_HIGH_BATS	1	/* High BATs supported			*/
 
-- 
1.6.0.6

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis and RTC in configuration.

[Detlev Zundel]

This patch adds the board specific communication routines needed by
the external 4543 RTC.

Signed-off-by: Detlev Zundel <ap@denx.de>
Signed-off-by: Andreas Pfefferle <ap@denx.de>
---
 board/inka4x0/inka4x0.c   |   20 ++++++++++
 include/configs/inka4x0.h |   90 ++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 105 insertions(+), 5 deletions(-)

diff --git a/board/inka4x0/inka4x0.c b/board/inka4x0/inka4x0.c
index ea59db3..35747ed 100644
--- a/board/inka4x0/inka4x0.c
+++ b/board/inka4x0/inka4x0.c
@@ -33,6 +33,7 @@
  * MA 02111-1307 USA
  */
 
+#include <asm/io.h>
 #include <common.h>
 #include <mpc5xxx.h>
 #include <pci.h>
@@ -188,6 +189,8 @@ int misc_init_r (void) {
 
 int misc_init_f (void)
 {
+	struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
+	struct mpc5xxx_wu_gpio *wu_gpio = (struct mpc5xxx_wu_gpio *)MPC5XXX_WU_GPIO;
 	char tmp[10];
 	int i, br;
 
@@ -235,6 +238,23 @@ 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 three wire serial interface to RTC (PSC1_4,
+	 * PSC2_4, PSC3_4, PSC3_5)
+	 */
+	setbits_8(&wu_gpio->enable,  MPC5XXX_GPIO_WKUP_PSC1_4 |
+				     MPC5XXX_GPIO_WKUP_PSC2_4);
+	setbits_8(&wu_gpio->ddr,     MPC5XXX_GPIO_WKUP_PSC1_4 |
+				     MPC5XXX_GPIO_WKUP_PSC2_4);
+	clrbits_8(&wu_gpio->dvo,     MPC5XXX_GPIO_WKUP_PSC1_4);
+	clrbits_8(&gpio->sint_inten, MPC5XXX_GPIO_SINT_PSC3_4 |
+				     MPC5XXX_GPIO_SINT_PSC3_5);
+	setbits_8(&gpio->sint_gpioe, MPC5XXX_GPIO_SINT_PSC3_4 |
+				     MPC5XXX_GPIO_SINT_PSC3_5);
+	setbits_8(&gpio->sint_ddr,   MPC5XXX_GPIO_SINT_PSC3_5);
+	clrbits_8(&gpio->sint_dvo,   MPC5XXX_GPIO_SINT_PSC3_5);
+
 	return 0;
 }
 
diff --git a/include/configs/inka4x0.h b/include/configs/inka4x0.h
index b80796d..5f06b09 100644
--- a/include/configs/inka4x0.h
+++ b/include/configs/inka4x0.h
@@ -1,4 +1,7 @@
 /*
+ * (C) Copyright 2009
+ * Detlev Zundel, DENX Software Engineering, dzu at denx.de.
+ *
  * (C) Copyright 2003-2005
  * Wolfgang Denk, DENX Software Engineering, wd at denx.de.
  *
@@ -90,16 +93,17 @@
  */
 #include <config_cmd_default.h>
 
+#define CONFIG_CMD_DATE
 #define CONFIG_CMD_DHCP
 #define CONFIG_CMD_EXT2
 #define CONFIG_CMD_FAT
 #define CONFIG_CMD_IDE
 #define CONFIG_CMD_NFS
 #define CONFIG_CMD_PCI
+#define CONFIG_CMD_PING
 #define CONFIG_CMD_SNTP
 #define CONFIG_CMD_USB
 
-
 #define	CONFIG_TIMESTAMP	1	/* Print image info with timestamp */
 
 #if (TEXT_BASE == 0xFFE00000)		/* Boot low */
@@ -240,15 +244,91 @@
  *	01 -> CAN1 on I2C1, CAN2 on Tmr0/1 do not use on TQM5200 with onboard
  *	      EEPROM
  * use PSC1 as UART: Bits 28-31 (mask: 0x00000007): 0100
- * 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.
+ * use PSC2 as UART: Bits 24-27 (mask: 0x00000070): 0100
+ * use PSC3 as UART: Bits 20-23 (mask: 0x00000700): 0100
+ * use PSC6 as UART: Bits  9-11 (mask: 0x00700000): 0101
  */
-#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 */
+
+/*
+ * Software (bit-bang) three wire serial configuration
+ *
+ * Note that we need the ifdefs because otherwise compilation of
+ * mkimage.c fails.
+ */
+#define CONFIG_SOFT_TWS		1
+
+#ifdef TWS_IMPLEMENTATION
+#include <mpc5xxx.h>
+#include <asm/io.h>
+
+#define TWS_CE		MPC5XXX_GPIO_WKUP_PSC1_4 /* GPIO_WKUP_0 */
+#define TWS_WR		MPC5XXX_GPIO_WKUP_PSC2_4 /* GPIO_WKUP_1 */
+#define TWS_DATA	MPC5XXX_GPIO_SINT_PSC3_4 /* GPIO_SINT_0 */
+#define TWS_CLK		MPC5XXX_GPIO_SINT_PSC3_5 /* GPIO_SINT_1 */
+
+static inline void tws_ce(unsigned bit)
+{
+	struct mpc5xxx_wu_gpio *wu_gpio =
+		(struct mpc5xxx_wu_gpio *)MPC5XXX_WU_GPIO;
+	if (bit)
+		setbits_8(&wu_gpio->dvo, TWS_CE);
+	else
+		clrbits_8(&wu_gpio->dvo, TWS_CE);
+}
+
+static inline void tws_wr(unsigned bit)
+{
+	struct mpc5xxx_wu_gpio *wu_gpio =
+		(struct mpc5xxx_wu_gpio *)MPC5XXX_WU_GPIO;
+	if (bit)
+		setbits_8(&wu_gpio->dvo, TWS_WR);
+	else
+		clrbits_8(&wu_gpio->dvo, TWS_WR);
+}
+
+static inline void tws_clk(unsigned bit)
+{
+	struct mpc5xxx_gpio *gpio =
+		(struct mpc5xxx_gpio *)MPC5XXX_GPIO;
+	if (bit)
+		setbits_8(&gpio->sint_dvo, TWS_CLK);
+	else
+		clrbits_8(&gpio->sint_dvo, TWS_CLK);
+}
+
+static inline void tws_data(unsigned bit)
+{
+	struct mpc5xxx_gpio *gpio =
+		(struct mpc5xxx_gpio *)MPC5XXX_GPIO;
+	if (bit)
+		setbits_8(&gpio->sint_dvo, TWS_DATA);
+	else
+		clrbits_8(&gpio->sint_dvo, TWS_DATA);
+}
+
+static inline unsigned tws_data_read(void)
+{
+	struct mpc5xxx_gpio *gpio =
+			(struct mpc5xxx_gpio *)MPC5XXX_GPIO;
+	return !!(in_8(&gpio->sint_ival) & TWS_DATA);
+}
+
+static inline void tws_data_config_output(unsigned output)
+{
+	struct mpc5xxx_gpio *gpio =
+		(struct mpc5xxx_gpio *)MPC5XXX_GPIO;
+	if (output)
+		setbits_8(&gpio->sint_ddr, TWS_DATA);
+	else
+		clrbits_8(&gpio->sint_ddr, TWS_DATA);
+}
+#endif /* TWS_IMPLEMENTATION */
 
 /*
  * Miscellaneous configurable options
-- 
1.6.0.6

[U-Boot] [PATCH] inka4x0: Use proper accessor macros for memory mapped registers.

[Detlev Zundel]

Signed-off-by: Detlev Zundel <dzu@denx.de>
---
 board/inka4x0/inka4x0.c |  131 ++++++++++++++++++++++++-----------------------
 1 files changed, 67 insertions(+), 64 deletions(-)

diff --git a/board/inka4x0/inka4x0.c b/board/inka4x0/inka4x0.c
index 35747ed..2ef770b 100644
--- a/board/inka4x0/inka4x0.c
+++ b/board/inka4x0/inka4x0.c
@@ -55,41 +55,34 @@
 #ifndef CONFIG_SYS_RAMBOOT
 static void sdram_start (int hi_addr)
 {
+	struct mpc5xxx_sdram *sdram = (struct mpc5xxx_sdram *)MPC5XXX_SDRAM;
 	long hi_addr_bit = hi_addr ? 0x01000000 : 0;
 
 	/* unlock mode register */
-	*(vu_long *)MPC5XXX_SDRAM_CTRL = SDRAM_CONTROL | 0x80000000 | hi_addr_bit;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->ctrl, SDRAM_CONTROL | 0x80000000 | hi_addr_bit);
 
 	/* precharge all banks */
-	*(vu_long *)MPC5XXX_SDRAM_CTRL = SDRAM_CONTROL | 0x80000002 | hi_addr_bit;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->ctrl, SDRAM_CONTROL | 0x80000002 | hi_addr_bit);
 
 #if SDRAM_DDR
 	/* set mode register: extended mode */
-	*(vu_long *)MPC5XXX_SDRAM_MODE = SDRAM_EMODE;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->mode, SDRAM_EMODE);
 
 	/* set mode register: reset DLL */
-	*(vu_long *)MPC5XXX_SDRAM_MODE = SDRAM_MODE | 0x04000000;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->mode, SDRAM_MODE | 0x04000000);
 #endif
 
 	/* precharge all banks */
-	*(vu_long *)MPC5XXX_SDRAM_CTRL = SDRAM_CONTROL | 0x80000002 | hi_addr_bit;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->ctrl, SDRAM_CONTROL | 0x80000002 | hi_addr_bit);
 
 	/* auto refresh */
-	*(vu_long *)MPC5XXX_SDRAM_CTRL = SDRAM_CONTROL | 0x80000004 | hi_addr_bit;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->ctrl, SDRAM_CONTROL | 0x80000004 | hi_addr_bit);
 
 	/* set mode register */
-	*(vu_long *)MPC5XXX_SDRAM_MODE = SDRAM_MODE;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->mode, SDRAM_MODE);
 
 	/* normal operation */
-	*(vu_long *)MPC5XXX_SDRAM_CTRL = SDRAM_CONTROL | hi_addr_bit;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->ctrl, SDRAM_CONTROL | hi_addr_bit);
 }
 #endif
 
@@ -101,24 +94,24 @@ static void sdram_start (int hi_addr)
 
 phys_size_t initdram (int board_type)
 {
+	struct mpc5xxx_mmap_ctl *mm = (struct mpc5xxx_mmap_ctl *) CONFIG_SYS_MBAR;
+	struct mpc5xxx_cdm     *cdm = (struct mpc5xxx_cdm *)      MPC5XXX_CDM;
+	struct mpc5xxx_sdram *sdram = (struct mpc5xxx_sdram *)    MPC5XXX_SDRAM;
 	ulong dramsize = 0;
 #ifndef CONFIG_SYS_RAMBOOT
 	long test1, test2;
 
 	/* setup SDRAM chip selects */
-	*(vu_long *)MPC5XXX_SDRAM_CS0CFG = 0x0000001c; /* 512MB at 0x0 */
-	*(vu_long *)MPC5XXX_SDRAM_CS1CFG = 0x40000000; /* disabled */
-	__asm__ volatile ("sync");
+	out_be32(&mm->sdram0, 0x0000001c);	/* 512MB at 0x0 */
+	out_be32(&mm->sdram1, 0x40000000);	/* disabled */
 
 	/* setup config registers */
-	*(vu_long *)MPC5XXX_SDRAM_CONFIG1 = SDRAM_CONFIG1;
-	*(vu_long *)MPC5XXX_SDRAM_CONFIG2 = SDRAM_CONFIG2;
-	__asm__ volatile ("sync");
+	out_be32(&sdram->config1, SDRAM_CONFIG1);
+	out_be32(&sdram->config2, SDRAM_CONFIG2);
 
 #if SDRAM_DDR
 	/* set tap delay */
-	*(vu_long *)MPC5XXX_CDM_PORCFG = SDRAM_TAPDELAY;
-	__asm__ volatile ("sync");
+	out_be32(&cdm->porcfg, SDRAM_TAPDELAY);
 #endif
 
 	/* find RAM size using SDRAM CS0 only */
@@ -140,17 +133,17 @@ phys_size_t initdram (int board_type)
 
 	/* set SDRAM CS0 size according to the amount of RAM found */
 	if (dramsize > 0) {
-		*(vu_long *)MPC5XXX_SDRAM_CS0CFG = 0x13 +
-			__builtin_ffs(dramsize >> 20) - 1;
+		out_be32(&mm->sdram0, 0x13 +
+			 __builtin_ffs(dramsize >> 20) - 1);
 	} else {
-		*(vu_long *)MPC5XXX_SDRAM_CS0CFG = 0; /* disabled */
+		out_be32(&mm->sdram0, 0); /* disabled */
 	}
 
-	*(vu_long *)MPC5XXX_SDRAM_CS1CFG = dramsize; /* disabled */
+	out_be32(&mm->sdram1, dramsize); /* disabled */
 #else /* CONFIG_SYS_RAMBOOT */
 
 	/* retrieve size of memory connected to SDRAM CS0 */
-	dramsize = *(vu_long *)MPC5XXX_SDRAM_CS0CFG & 0xFF;
+	dramsize = in_be32(&mm->sdram0) & 0xFF;
 	if (dramsize >= 0x13) {
 		dramsize = (1 << (dramsize - 0x13)) << 20;
 	} else {
@@ -169,13 +162,15 @@ int checkboard (void)
 
 void flash_preinit(void)
 {
+	struct mpc5xxx_lpb *lpb = (struct mpc5xxx_lpb *)MPC5XXX_LPB;
+
 	/*
 	 * Now, when we are in RAM, enable flash write
 	 * access for detection process.
-	 * Note that CS_BOOT cannot be cleared when
+	 * Note that CS_BOOT (CS0) cannot be cleared when
 	 * executing in flash.
 	 */
-	*(vu_long *)MPC5XXX_BOOTCS_CFG &= ~0x1; /* clear RO */
+	clrbits_be32(&lpb->cs0_cfg, 0x1); /* clear RO */
 }
 
 int misc_init_r (void) {
@@ -189,8 +184,9 @@ int misc_init_r (void) {
 
 int misc_init_f (void)
 {
-	struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
-	struct mpc5xxx_wu_gpio *wu_gpio = (struct mpc5xxx_wu_gpio *)MPC5XXX_WU_GPIO;
+	struct mpc5xxx_gpio	*gpio    = (struct mpc5xxx_gpio *)   MPC5XXX_GPIO;
+	struct mpc5xxx_wu_gpio	*wu_gpio = (struct mpc5xxx_wu_gpio *)MPC5XXX_WU_GPIO;
+	struct mpc5xxx_gpt	*gpt;
 	char tmp[10];
 	int i, br;
 
@@ -204,40 +200,43 @@ int misc_init_f (void)
 	/* Initialize GPIO output pins.
 	 */
 	/* Configure GPT as GPIO output (and set them as they control low-active LEDs */
-	*(vu_long *)MPC5XXX_GPT0_ENABLE =
-	*(vu_long *)MPC5XXX_GPT1_ENABLE =
-	*(vu_long *)MPC5XXX_GPT2_ENABLE =
-	*(vu_long *)MPC5XXX_GPT3_ENABLE =
-	*(vu_long *)MPC5XXX_GPT4_ENABLE =
-	*(vu_long *)MPC5XXX_GPT5_ENABLE = 0x34;
+	for (i = 0; i <= 5; i++) {
+		gpt = (struct mpc5xxx_gpt *)(MPC5XXX_GPT + (i * 0x10));
+		out_be32(&gpt->emsr, 0x34);
+	}
 
 	/* Configure GPT7 as PWM timer, 1kHz, no ints. */
-	*(vu_long *)MPC5XXX_GPT7_ENABLE = 0;/* Disable */
-	*(vu_long *)MPC5XXX_GPT7_COUNTER = 0x020000fe;
-	*(vu_long *)MPC5XXX_GPT7_PWMCFG = (br << 16);
-	*(vu_long *)MPC5XXX_GPT7_ENABLE = 0x3;/* Enable PWM mode and start */
+	gpt = (struct mpc5xxx_gpt *)(MPC5XXX_GPT + (7 * 0x10));
+	out_be32(&gpt->emsr,  0);		/* Disable */
+	out_be32(&gpt->cir,   0x020000fe);
+	out_be32(&gpt->pwmcr, (br << 16));
+	out_be32(&gpt->emsr,  0x3);		/* Enable PWM mode and start */
 
 	/* Configure PSC3_6,7 as GPIO output */
-	*(vu_long *)MPC5XXX_GPIO_ENABLE |= 0x00003000;
-	*(vu_long *)MPC5XXX_GPIO_DIR |= 0x00003000;
-
-	/* Configure PSC3_8 as GPIO output, no interrupt */
-	*(vu_long *)MPC5XXX_GPIO_SI_ENABLE |= 0x04000000;
-	*(vu_long *)MPC5XXX_GPIO_SI_DIR |= 0x04000000;
-	*(vu_long *)MPC5XXX_GPIO_SI_IEN &= ~0x04000000;
+	setbits_be32(&gpio->simple_gpioe, MPC5XXX_GPIO_SIMPLE_PSC3_6 |
+					  MPC5XXX_GPIO_SIMPLE_PSC3_7);
+	setbits_be32(&gpio->simple_ddr,   MPC5XXX_GPIO_SIMPLE_PSC3_6 |
+					  MPC5XXX_GPIO_SIMPLE_PSC3_7);
 
 	/* Configure PSC3_9 and GPIO_WKUP6,7 as GPIO output */
-	*(vu_long *)MPC5XXX_WU_GPIO_ENABLE |= 0xc4000000;
-	*(vu_long *)MPC5XXX_WU_GPIO_DIR |= 0xc4000000;
+	setbits_8(&wu_gpio->enable,  MPC5XXX_GPIO_WKUP_6 |
+				     MPC5XXX_GPIO_WKUP_7 |
+				     MPC5XXX_GPIO_WKUP_PSC3_9);
+	setbits_8(&wu_gpio->ddr,     MPC5XXX_GPIO_WKUP_6 |
+				     MPC5XXX_GPIO_WKUP_7 |
+				     MPC5XXX_GPIO_WKUP_PSC3_9);
 
 	/* Set LR mirror bit because it is low-active */
-	*(vu_long *) MPC5XXX_WU_GPIO_DATA_O    |= GPIO_WKUP_7;
-	/*
-	 * Reset Coral-P graphics controller
-	 */
-	*(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;
+	setbits_8(&wu_gpio->dvo,     MPC5XXX_GPIO_WKUP_7);
+
+	/* Reset Coral-P graphics controller */
+	setbits_8(&wu_gpio->dvo,     MPC5XXX_GPIO_WKUP_PSC3_9);
+
+	/* Enable display backlight */
+	clrbits_8(&gpio->sint_inten, MPC5XXX_GPIO_SINT_PSC3_8);
+	setbits_8(&gpio->sint_gpioe, MPC5XXX_GPIO_SINT_PSC3_8);
+	setbits_8(&gpio->sint_ddr,   MPC5XXX_GPIO_SINT_PSC3_8);
+	setbits_8(&gpio->sint_dvo,   MPC5XXX_GPIO_SINT_PSC3_8);
 
 	/*
 	 * Configure three wire serial interface to RTC (PSC1_4,
@@ -273,25 +272,29 @@ void pci_init_board(void)
 
 void init_ide_reset (void)
 {
+	struct mpc5xxx_wu_gpio	*wu_gpio = (struct mpc5xxx_wu_gpio *)MPC5XXX_WU_GPIO;
+
 	debug ("init_ide_reset\n");
 
 	/* Configure PSC1_4 as GPIO output for ATA reset */
-	*(vu_long *) MPC5XXX_WU_GPIO_ENABLE |= GPIO_PSC1_4;
-	*(vu_long *) MPC5XXX_WU_GPIO_DIR    |= GPIO_PSC1_4;
+	setbits_8(&wu_gpio->enable, MPC5XXX_GPIO_WKUP_PSC1_4);
+	setbits_8(&wu_gpio->ddr,    MPC5XXX_GPIO_WKUP_PSC1_4);
 	/* Deassert reset */
-	*(vu_long *) MPC5XXX_WU_GPIO_DATA_O   |= GPIO_PSC1_4;
+	setbits_8(&wu_gpio->dvo,    MPC5XXX_GPIO_WKUP_PSC1_4);
 }
 
 void ide_set_reset (int idereset)
 {
+	struct mpc5xxx_wu_gpio	*wu_gpio = (struct mpc5xxx_wu_gpio *)MPC5XXX_WU_GPIO;
+
 	debug ("ide_reset(%d)\n", idereset);
 
 	if (idereset) {
-		*(vu_long *) MPC5XXX_WU_GPIO_DATA_O &= ~GPIO_PSC1_4;
+		clrbits_8(&wu_gpio->dvo, MPC5XXX_GPIO_WKUP_PSC1_4);
 		/* Make a delay. MPC5200 spec says 25 usec min */
 		udelay(500000);
 	} else {
-		*(vu_long *) MPC5XXX_WU_GPIO_DATA_O |=  GPIO_PSC1_4;
+		setbits_8(&wu_gpio->dvo, MPC5XXX_GPIO_WKUP_PSC1_4);
 	}
 }
 #endif
-- 
1.6.0.6

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis and RTC in configuration.

[Wolfgang Denk]

Dear Detlev Zundel,

In message <1237914158-15693-7-git-send-email-dzu@denx.de> you wrote:
> This patch adds the board specific communication routines needed by
> the external 4543 RTC.
> 
> Signed-off-by: Detlev Zundel <ap@denx.de>
> Signed-off-by: Andreas Pfefferle <ap@denx.de>
...
> @@ -235,6 +238,23 @@ 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;

You don't want to fix the missing accessor functions here, do you? ;-)

> +
> +	/*
> +	 * Configure three wire serial interface to RTC (PSC1_4,
> +	 * PSC2_4, PSC3_4, PSC3_5)
> +	 */
> +	setbits_8(&wu_gpio->enable,  MPC5XXX_GPIO_WKUP_PSC1_4 |
> +				     MPC5XXX_GPIO_WKUP_PSC2_4);
> +	setbits_8(&wu_gpio->ddr,     MPC5XXX_GPIO_WKUP_PSC1_4 |
> +				     MPC5XXX_GPIO_WKUP_PSC2_4);
> +	clrbits_8(&wu_gpio->dvo,     MPC5XXX_GPIO_WKUP_PSC1_4);
> +	clrbits_8(&gpio->sint_inten, MPC5XXX_GPIO_SINT_PSC3_4 |
> +				     MPC5XXX_GPIO_SINT_PSC3_5);
> +	setbits_8(&gpio->sint_gpioe, MPC5XXX_GPIO_SINT_PSC3_4 |
> +				     MPC5XXX_GPIO_SINT_PSC3_5);
> +	setbits_8(&gpio->sint_ddr,   MPC5XXX_GPIO_SINT_PSC3_5);
> +	clrbits_8(&gpio->sint_dvo,   MPC5XXX_GPIO_SINT_PSC3_5);
> +
>  	return 0;
>  }


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: wd at denx.de
Name one thing windows is better than unix in?
                                          Making money for Microsoft?
         -- Randal L. Schwartz in <8cvi5t4c3t.fsf@gadget.cscaper.com>

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis and RTC in configuration.

[Detlev Zundel]

Hi Wolfgang,

> Dear Detlev Zundel,
>
> In message <1237914158-15693-7-git-send-email-dzu@denx.de> you wrote:
>> This patch adds the board specific communication routines needed by
>> the external 4543 RTC.
>> 
>> Signed-off-by: Detlev Zundel <ap@denx.de>
>> Signed-off-by: Andreas Pfefferle <ap@denx.de>
> ...
>> @@ -235,6 +238,23 @@ 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;
>
> You don't want to fix the missing accessor functions here, do you? ;-)

Oh I do - actually, I did - please check my later posting "inka4x0: Use
proper accessor macros for memory mapped registers."

Cheers
  Detlev

-- 
Q: What do you get when you cross an elephant and a banana?
A: |elephant| * |banana| * sin(theta)
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis and RTC in configuration.

[Wolfgang Denk]

Dear Detlev Zundel,

In message <m23ad2rmh2.fsf@ohwell.denx.de> you wrote:
> 
> >> @@ -235,6 +238,23 @@ 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;
> >
> > You don't want to fix the missing accessor functions here, do you? ;-)
> 
> Oh I do - actually, I did - please check my later posting "inka4x0: Use
> proper accessor macros for memory mapped registers."

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis and RTC in configuration.

[Detlev Zundel]

Hi Wolfgang,

> Dear Detlev Zundel,
>
> In message <m23ad2rmh2.fsf@ohwell.denx.de> you wrote:
>> 
>> >> @@ -235,6 +238,23 @@ 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;
>> >
>> > You don't want to fix the missing accessor functions here, do you? ;-)
>> 
>> Oh I do - actually, I did - please check my later posting "inka4x0: Use
>> proper accessor macros for memory mapped registers."
>
>>From the reviewing point of view it would have been easier if that
> other patch would have come earlier in the sequence. As is, it is
> really difficult to see if the lines above are included in your other
> patch, or if they might have been missed. Keep in mind that reviewing
> is a sequential process...

I know.  Software development is *also* a sequential process.  It was of
course only by the time that I had introduced the new stuff that I saw
what else needs to be fixed.  So the order does indeed reflect my
work-flow.  I honestly did not even think about re-ordering these
patches because of your considerations.  Is it written somewhere that I
have to fix all existing problems first before I get to add new stuff?
;)

> But OK, from the end result it makes no difference.

Well, if one uses a threaded news reader, one might have seen the
summaries of all patches before actually diving in...

Cheers
  Detlev

-- 
Lotus Notes (GUI):   Run away from it.
                           -- linux/Documentation/email-clients.txt
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis and RTC in configuration.

[Anatolij Gustschin]

Detlev Zundel wrote:
> This patch adds the board specific communication routines needed by
> the external 4543 RTC.
> 
> Signed-off-by: Detlev Zundel <ap@denx.de>
> Signed-off-by: Andreas Pfefferle <ap@denx.de>

duplicated e-mail address from Andreas in your SoB again.

Best regards,
Anatolij

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Heiko Schocher]

Hello Detlev,

Detlev Zundel wrote:
> This patch adds advanced diagnosis functions for the inka4x0 board.
>
> Signed-off-by: Andreas Pfefferle <ap@denx.de>
> Signed-off-by: Detlev Zundel <ap@denx.de>
> ---
>  board/inka4x0/Makefile    |    4 +-
>  board/inka4x0/inka4x0.c   |   19 ++-
>  board/inka4x0/inkadiag.c  |  541 +++++++++++++++++++++++++++++++++++++++++++++
>  include/configs/inka4x0.h |    1 +
>  4 files changed, 561 insertions(+), 4 deletions(-)
>  create mode 100644 board/inka4x0/inkadiag.c
>   
[...]

> diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c
> new file mode 100644
> index 0000000..bdbf652
> --- /dev/null
> +++ b/board/inka4x0/inkadiag.c
> @@ -0,0 +1,541 @@
> +/*
> + * (C) Copyright 2008, 2009 Andreas Pfefferle,
> + *     DENX Software Engineering, ap at denx.de.
> + * (C) Copyright 2009 Detlev Zundel,
> + *     DENX Software Engineering, dzu at 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 GPIO_BASE  (u_char *)0x30400000
>   

should be

#define GPIO_BASE  (u_char *)CONFIG_SYS_CS3_START


> +
> +#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 */
> +
> +#define SERIAL_PORT_BASE	(u_char *)0x80000000
>   

this should be

#define SERIAL_PORT_BASE	(u_char *)CONFIG_SYS_CS2_START


> +
> +#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 */
>   

Are this not common 16xxx UART registers? If so, I think, they
should go in include/ns16550.h ...

bye
Heiko

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany 

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Wolfgang Denk]

Dear Heiko & Detlev,

In message <49C91E04.2020801@denx.de> you wrote:
...
> > +#define GPIO_BASE  (u_char *)0x30400000
> >   
> 
> should be
> 
> #define GPIO_BASE  (u_char *)CONFIG_SYS_CS3_START

or
	#define GPIO_BASE  ((u_char *)CONFIG_SYS_CS3_START)
?

> > +#define SERIAL_PORT_BASE	(u_char *)0x80000000
> >   
> 
> this should be
> 
> #define SERIAL_PORT_BASE	(u_char *)CONFIG_SYS_CS2_START

or
	#define SERIAL_PORT_BASE	((u_char *)CONFIG_SYS_CS2_START)
? 

etc.

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: wd at denx.de
To be is to program.

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Detlev Zundel]

Hi Wolfgang,

> Dear Heiko & Detlev,
>
> In message <49C91E04.2020801@denx.de> you wrote:
> ...
>> > +#define GPIO_BASE  (u_char *)0x30400000
>> >   
>> 
>> should be
>> 
>> #define GPIO_BASE  (u_char *)CONFIG_SYS_CS3_START
>
> or
> 	#define GPIO_BASE  ((u_char *)CONFIG_SYS_CS3_START)
> ?
>
>> > +#define SERIAL_PORT_BASE	(u_char *)0x80000000
>> >   
>> 
>> this should be
>> 
>> #define SERIAL_PORT_BASE	(u_char *)CONFIG_SYS_CS2_START
>
> or
> 	#define SERIAL_PORT_BASE	((u_char *)CONFIG_SYS_CS2_START)
> ? 

Yes, I agree.

Thanks
  Detlev

-- 
Ftpd never switches uid and euid, it uses setfsuid(2) instead. The
main reason is that uid switching has been exploited in several
breakins, but the sheer ugliness of uid switching counts too.
                                     -- pure-ftpd(8)
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Detlev Zundel]

Hi Heiko,

>> +#define GPIO_BASE  (u_char *)0x30400000
>>   
>
> should be
>
> #define GPIO_BASE  (u_char *)CONFIG_SYS_CS3_START

Fixed, thanks.

>> +#define SERIAL_PORT_BASE	(u_char *)0x80000000
>>   
>
> this should be
>
> #define SERIAL_PORT_BASE	(u_char *)CONFIG_SYS_CS2_START

Fixed, thanks.


[...]

>> +#define UART_MSR_DCD	0x80	/* Data Carrier Detect */
>> +#define UART_MSR_DSR	0x20	/* Data Set Ready */
>> +#define UART_MSR_CTS	0x10	/* Clear to Send */
>>   
>
> Are this not common 16xxx UART registers? If so, I think, they
> should go in include/ns16550.h ...

Ok, done.

I'm glad I posted this series of patches again with my name added.  On
the previous posting, the code unfortunately did not get this quality
review ;)

Cheers
  Detlev

-- 
Wenn ein Kopf und ein Buch zusammenstossen und es klingt hohl; ist
denn das allemal im Buche?
                               - Lichtenberg
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Heiko Schocher]

Hello Detlev,

Detlev Zundel wrote:
> Hi Heiko,
>
>   
>>> +#define GPIO_BASE  (u_char *)0x30400000
>>>   
>>>       
>> should be
>>
>> #define GPIO_BASE  (u_char *)CONFIG_SYS_CS3_START
>>     
>
> Fixed, thanks.
>
>   
>>> +#define SERIAL_PORT_BASE	(u_char *)0x80000000
>>>   
>>>       
>> this should be
>>
>> #define SERIAL_PORT_BASE	(u_char *)CONFIG_SYS_CS2_START
>>     
>
> Fixed, thanks.
>
>
> [...]
>
>   
>>> +#define UART_MSR_DCD	0x80	/* Data Carrier Detect */
>>> +#define UART_MSR_DSR	0x20	/* Data Set Ready */
>>> +#define UART_MSR_CTS	0x10	/* Clear to Send */
>>>   
>>>       
>> Are this not common 16xxx UART registers? If so, I think, they
>> should go in include/ns16550.h ...
>>     
>
> Ok, done.
>
> I'm glad I posted this series of patches again with my name added.  On
> the previous posting, the code unfortunately did not get this quality
> review ;)
>   

Think that has nothing to do with your name, more with time ;-)

bye
Heiko

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany 

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Wolfgang Denk]

Dear Detlev Zundel,

In message <1237914158-15693-6-git-send-email-dzu@denx.de> you wrote:
> This patch adds advanced diagnosis functions for the inka4x0 board.
> 
> Signed-off-by: Andreas Pfefferle <ap@denx.de>
> Signed-off-by: Detlev Zundel <ap@denx.de>
...
> +	extern int inkadiag_init_r (void);
> +	/*
> +	  * The command table used for the subcommands of inkadiag
> +	  * needs to be relocated manually.
> +	  */

Incorrect multiline comment style?

> +}
> +
>  int misc_init_f (void)
>  {
>  	char tmp[10];
> diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c
> new file mode 100644
> index 0000000..bdbf652
...
> +#define LED_HIGH(NUM) \
> +    do { \
> +	    out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,		\
> +		     in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) | 0x10); \
> +    } while (0)

Please use:

	setbits_be32 ((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,  0x10); 

instead (and so on in the rest of the code).

> +		struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
> +		if (which == 1) {
> +			/* drawer1 */
> +			if (state) {
> +				gpio->simple_dvo &= ~0x1000;
> +				udelay(1);
> +				gpio->simple_dvo |= 0x1000;
> +			} else {
> +				gpio->simple_dvo |= 0x1000;
> +				udelay(1);
> +				gpio->simple_dvo &= ~0x1000;
> +			}
> +		}
> +		if (which == 2) {
> +			/* drawer 2 */
> +			if (state) {
> +				gpio->simple_dvo &= ~0x2000;
> +				udelay(1);
> +				gpio->simple_dvo |= 0x2000;
> +			} else {
> +				gpio->simple_dvo |= 0x2000;
> +				udelay(1);
> +				gpio->simple_dvo &= ~0x2000;

Please use accessor functions to write to I/O ports.

...
> +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;

Should we not use accessor function to access the device registers
(here and in the following code) ?

> +static int do_inkadiag_serial(cmd_tbl_t *cmdtp, int flag, int argc,
> +			      char *argv[]) {
> +	if (argc < 5) {
> +		cmd_usage(cmdtp);
> +		return 1;
> +	}
> +
> +	argc--;
> +	argv++;
> +
> +	unsigned int num = simple_strtol(argv[0], NULL, 0);

Please NEVER declare variables right in the middle of the code!!

Did this actually pass a compile test?

> +	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;
> +	int baudrate = simple_strtoul(argv[2], NULL, 10);
> +	int i;

Ditto.

...
> +		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]);

Ditto. Please make sure to fix this globally.

> +		for (i = 0; i < len; ++i) {
> +			int j = 0;
> +			while ((serial_in(num, UART_LSR) & UART_LSR_THRE) ==
> +				0x00) {

This is actually difficult to read. Please reconsider indentation /
line wrapping / adding a blank line.

> +				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) {

Ditto.

> +				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 = 0;
> +		int 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;
> +		}

"irq" seems to be unused (accept for the assignments) in  this  code.
Wasn't there a compiler warning?

> +		volatile struct mpc5xxx_psc *psc =
> +			(struct mpc5xxx_psc *)address;

Variable declaration in the middle of the code - see above.

> +		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 */

Accessor functions?

> +}
> +
> +static void buzzer_turn_off(void)
> +{
> +	struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)(BUZZER_GPT);
> +	gpt->emsr = 0;

Accessor functions?

> +}
> +
> +static int do_inkadiag_buzzer(cmd_tbl_t *cmdtp, int flag, int argc,
> +			      char *argv[]) {
> +
> +	if (argc != 3) {
> +		cmd_usage(cmdtp);
> +		return 1;
> +	}
> +
> +	argc--;
> +	argv++;
> +
> +	unsigned int period = simple_strtol(argv[0], NULL, 0);

Declaration in the middle of the code.

> +	if (!period)
> +		printf("Zero period is senseless\n");
> +	argc--;
> +	argv++;
> +
> +	unsigned int freq = simple_strtol(argv[0], NULL, 0);

Declaration in the middle of the code.

> +	/* avoid zero prescale in buzzer_turn_on() */
> +	if (freq > gd->ipb_clk / 128) {
> +		printf("%dHz exceeds maximum (%ldHz)\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;

Declaration in the middle of the code.

> +	while (!ctrlc() && (i++ < CONFIG_SYS_HZ))
> +		udelay(period);
> +
> +	clear_ctrlc();
> +	disable_ctrlc(prev);
> +
> +	buzzer_turn_off();
> +
> +	return 0;
> +}
> +
> +static int do_inkadiag_help(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
> +
> +cmd_tbl_t cmd_inkadiag_sub[] = {
> +	U_BOOT_CMD_MKENT(io, 1, 1, do_inkadiag_io, "read digital input",
> +			 "<drawer1|drawer2|other> [value] - get or set specified signal\n"),
> +	U_BOOT_CMD_MKENT(serial, 4, 1, do_inkadiag_serial, "test serial port",
> +			 "<num> <mode> <baudrate> <msg>  - test uart num [0..11] in mode\n"
> +			 "and baudrate with msg\n"),
> +	U_BOOT_CMD_MKENT(buzzer, 2, 1, do_inkadiag_buzzer, "activate buzzer",
> +			 "<period> <freq> - turn buzzer on for period ms with freq hz\n"),
> +	U_BOOT_CMD_MKENT(help, 4, 1, do_inkadiag_help, "get help",
> +			 "[command] - get help for command\n"),
> +};

Lines too long.

> +static int do_inkadiag_help(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) {
> +	extern int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp,
> +			     int flag, int argc, char *argv[]);
> +	/* do_help prints command name - we prepend inkadiag to our subcommands! */

Lines too long.


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: wd at denx.de
Substitute "damn" every time you're inclined to write "very"; your
editor will delete it and the writing will be just as it should be.
                - Mark Twain

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Scott Wood]

Wolfgang Denk wrote:
>> +static int do_inkadiag_serial(cmd_tbl_t *cmdtp, int flag, int argc,
>> +			      char *argv[]) {
>> +	if (argc < 5) {
>> +		cmd_usage(cmdtp);
>> +		return 1;
>> +	}
>> +
>> +	argc--;
>> +	argv++;
>> +
>> +	unsigned int num = simple_strtol(argv[0], NULL, 0);
> 
> Please NEVER declare variables right in the middle of the code!!
> 
> Did this actually pass a compile test?

Probably -- it's been legal C for about a decade.

Style is another matter, but the compiler's not going to enforce that. :-)

-Scott

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Detlev Zundel]

Hi Wolfgang,

> Dear Detlev Zundel,
>
> In message <1237914158-15693-6-git-send-email-dzu@denx.de> you wrote:
>> This patch adds advanced diagnosis functions for the inka4x0 board.
>> 
>> Signed-off-by: Andreas Pfefferle <ap@denx.de>
>> Signed-off-by: Detlev Zundel <ap@denx.de>
> ...
>> +	extern int inkadiag_init_r (void);
>> +	/*
>> +	  * The command table used for the subcommands of inkadiag
>> +	  * needs to be relocated manually.
>> +	  */
>
> Incorrect multiline comment style?

Nothing escapes the eye of an eagle ;)

>> +}
>> +
>>  int misc_init_f (void)
>>  {
>>  	char tmp[10];
>> diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c
>> new file mode 100644
>> index 0000000..bdbf652
> ...
>> +#define LED_HIGH(NUM) \
>> +    do { \
>> +	    out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,		\
>> +		     in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) | 0x10); \
>> +    } while (0)
>
> Please use:
>
> 	setbits_be32 ((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,  0x10); 
>
> instead (and so on in the rest of the code).

Done.

>> +		struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
>> +		if (which == 1) {
>> +			/* drawer1 */
>> +			if (state) {
>> +				gpio->simple_dvo &= ~0x1000;
>> +				udelay(1);
>> +				gpio->simple_dvo |= 0x1000;
>> +			} else {
>> +				gpio->simple_dvo |= 0x1000;
>> +				udelay(1);
>> +				gpio->simple_dvo &= ~0x1000;
>> +			}
>> +		}
>> +		if (which == 2) {
>> +			/* drawer 2 */
>> +			if (state) {
>> +				gpio->simple_dvo &= ~0x2000;
>> +				udelay(1);
>> +				gpio->simple_dvo |= 0x2000;
>> +			} else {
>> +				gpio->simple_dvo |= 0x2000;
>> +				udelay(1);
>> +				gpio->simple_dvo &= ~0x2000;
>
> Please use accessor functions to write to I/O ports.

Fixed.

>> +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;
>
> Should we not use accessor function to access the device registers
> (here and in the following code) ?

Indeed we should.  But here we go, I can understand, why this wasn't
done the first time around.  The new code looks like this:

	/* reset PSC */
	out_8(&psc->command, PSC_SEL_MODE_REG_1);

	/* select clock sources */

	out_be16(&psc->psc_clock_select, 0);
	baseclk = (gd->ipb_clk + 16) / 32;

	/* switch to UART mode */
	out_be32(&psc->sicr, 0);

	/* configure parity, bit length and so on */

	out_8(&psc->mode, PSC_MODE_8_BITS | PSC_MODE_PARNONE);
	out_8(&psc->mode, PSC_MODE_ONE_STOP);

	/* set up UART divisor */
	div = (baseclk + (baudrate / 2)) / baudrate;
	out_8(&psc->ctur, (div >> 8) & 0xff);
	out_8(&psc->ctlr, div & 0xff);

	/* disable all interrupts */
	out_be16(&psc->psc_imr, 0);

	/* reset and enable Rx/Tx */
	out_8(&psc->command, PSC_RST_RX);
	out_8(&psc->command, PSC_RST_TX);
	out_8(&psc->command, PSC_RX_ENABLE | PSC_TX_ENABLE);


Look at the wonderful mix of out_{8,be16,be32}.  What a heart refreshing
break from the monotonic coding routine :(

Actully this is due to to mpc5xxx.h:

struct mpc5xxx_psc {
	volatile u8	mode;		/* PSC + 0x00 */
	volatile u8	reserved0[3];
	union {				/* PSC + 0x04 */
		volatile u16	status;
		volatile u16	clock_select;
	} sr_csr;
#define psc_status	sr_csr.status
#define psc_clock_select sr_csr.clock_select
	volatile u16	reserved1;
	volatile u8	command;	/* PSC + 0x08 */
	volatile u8	reserved2[3];
	union {				/* PSC + 0x0c */
		volatile u8	buffer_8;
		volatile u16	buffer_16;
		volatile u32	buffer_32;
	} buffer;
#define psc_buffer_8	buffer.buffer_8
#define psc_buffer_16	buffer.buffer_16
#define psc_buffer_32	buffer.buffer_32
	union {				/* PSC + 0x10 */
		volatile u8	ipcr;
		volatile u8	acr;
	} ipcr_acr;
#define psc_ipcr	ipcr_acr.ipcr
#define psc_acr		ipcr_acr.acr
	volatile u8	reserved3[3];
	union {				/* PSC + 0x14 */
		volatile u16	isr;
		volatile u16	imr;
	} isr_imr;


Git-blame points the finger to someone called 'wd'.  Maybe he knows
about the advantages of such structure definitions?  

On second check the corresponding header file in Linux (mpc52xx_uart.h) has
stuff like this introduced by Kumar Gala, maybe he can enlighten me?

To be honest, I wouldn't even dare accessing memory mapped registers in
the non-native size, but maybe I'm missing something here.

>> +static int do_inkadiag_serial(cmd_tbl_t *cmdtp, int flag, int argc,
>> +			      char *argv[]) {
>> +	if (argc < 5) {
>> +		cmd_usage(cmdtp);
>> +		return 1;
>> +	}
>> +
>> +	argc--;
>> +	argv++;
>> +
>> +	unsigned int num = simple_strtol(argv[0], NULL, 0);
>
> Please NEVER declare variables right in the middle of the code!!

I won't, I PROMISE.

> Did this actually pass a compile test?

Heck, do you think I dare send a patch without compiling?

>> +		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]);
>
> Ditto. Please make sure to fix this globally.

I believe I succeeded.

>> +		for (i = 0; i < len; ++i) {
>> +			int j = 0;
>> +			while ((serial_in(num, UART_LSR) & UART_LSR_THRE) ==
>> +				0x00) {
>
> This is actually difficult to read. Please reconsider indentation /
> line wrapping / adding a blank line.

The switch to using struct NS16650 allowed for nicer layout without
violating line length.

>> +				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 = 0;
>> +		int 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;
>> +		}
>
> "irq" seems to be unused (accept for the assignments) in  this  code.
> Wasn't there a compiler warning?

Nope.

>> +#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 */
>
> Accessor functions?

Yep.

>> +}
>> +
>> +static void buzzer_turn_off(void)
>> +{
>> +	struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)(BUZZER_GPT);
>> +	gpt->emsr = 0;
>
> Accessor functions?

Again, not a question.

>> +}
>> +
>> +static int do_inkadiag_buzzer(cmd_tbl_t *cmdtp, int flag, int argc,
>> +			      char *argv[]) {
>> +
>> +	if (argc != 3) {
>> +		cmd_usage(cmdtp);
>> +		return 1;
>> +	}
>> +
>> +	argc--;
>> +	argv++;
>> +
>> +	unsigned int period = simple_strtol(argv[0], NULL, 0);
>
> Declaration in the middle of the code.

Fixed.

>> +	if (!period)
>> +		printf("Zero period is senseless\n");
>> +	argc--;
>> +	argv++;
>> +
>> +	unsigned int freq = simple_strtol(argv[0], NULL, 0);
>
> Declaration in the middle of the code.

What insolence!  Fixed.

>> +	/* avoid zero prescale in buzzer_turn_on() */
>> +	if (freq > gd->ipb_clk / 128) {
>> +		printf("%dHz exceeds maximum (%ldHz)\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;
>
> Declaration in the middle of the code.

Fixed.

>> +	while (!ctrlc() && (i++ < CONFIG_SYS_HZ))
>> +		udelay(period);
>> +
>> +	clear_ctrlc();
>> +	disable_ctrlc(prev);
>> +
>> +	buzzer_turn_off();
>> +
>> +	return 0;
>> +}
>> +
>> +static int do_inkadiag_help(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]);
>> +
>> +cmd_tbl_t cmd_inkadiag_sub[] = {
>> +	U_BOOT_CMD_MKENT(io, 1, 1, do_inkadiag_io, "read digital input",
>> +			 "<drawer1|drawer2|other> [value] - get or set specified signal\n"),
>> +	U_BOOT_CMD_MKENT(serial, 4, 1, do_inkadiag_serial, "test serial port",
>> +			 "<num> <mode> <baudrate> <msg>  - test uart num [0..11] in mode\n"
>> +			 "and baudrate with msg\n"),
>> +	U_BOOT_CMD_MKENT(buzzer, 2, 1, do_inkadiag_buzzer, "activate buzzer",
>> +			 "<period> <freq> - turn buzzer on for period ms with freq hz\n"),
>> +	U_BOOT_CMD_MKENT(help, 4, 1, do_inkadiag_help, "get help",
>> +			 "[command] - get help for command\n"),
>> +};
>
> Lines too long.

Personally I do not think that the new version is any more readable.  To
fix the "problem", I can split the string or violate indentation.  As
the first seems very unattractive from a reading point of view, I
actually willingly decided for the latter.  I hope you don't criticize
that in the next round.

>> +static int do_inkadiag_help(cmd_tbl_t *cmdtp, int flag, int argc, char *argv[]) {
>> +	extern int _do_help (cmd_tbl_t *cmd_start, int cmd_items, cmd_tbl_t * cmdtp,
>> +			     int flag, int argc, char *argv[]);
>> +	/* do_help prints command name - we prepend inkadiag to our subcommands! */
>
> Lines too long.

Again I believe the new version is harder to read, but I don't really
care.

Cheers
  Detlev

-- 
We have a live-manual.  It's called emacs-devel at gnu.org.
You can stick to just reading it, but you can skip to a specific chapter
by simply sending an email asking for it ;-)
                                    -- Stefan Monnier
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Wolfgang Denk]

Dear Detlev & Kumar,

In message <m2ljqtr68m.fsf@ohwell.denx.de> you wrote:
> 
...
> >> +	/* reset PSC */
> >> +	psc->command = PSC_SEL_MODE_REG_1;
> >
> > Should we not use accessor function to access the device registers
> > (here and in the following code) ?
> 
> Indeed we should.  But here we go, I can understand, why this wasn't
> done the first time around.  The new code looks like this:
> 
> 	/* reset PSC */
> 	out_8(&psc->command, PSC_SEL_MODE_REG_1);
> 
> 	/* select clock sources */
> 
> 	out_be16(&psc->psc_clock_select, 0);
> 	baseclk = (gd->ipb_clk + 16) / 32;
> 
> 	/* switch to UART mode */
> 	out_be32(&psc->sicr, 0);
...
> Look at the wonderful mix of out_{8,be16,be32}.  What a heart refreshing
> break from the monotonic coding routine :(

Indeed, what a mess, especially given the fact that these are all 32
bit registers, or am I wrong?

> Actully this is due to to mpc5xxx.h:
> 
> struct mpc5xxx_psc {
> 	volatile u8	mode;		/* PSC + 0x00 */
> 	volatile u8	reserved0[3];
> 	union {				/* PSC + 0x04 */
> 		volatile u16	status;
> 		volatile u16	clock_select;
> 	} sr_csr;
> #define psc_status	sr_csr.status
> #define psc_clock_select sr_csr.clock_select
> 	volatile u16	reserved1;
> 	volatile u8	command;	/* PSC + 0x08 */
> 	volatile u8	reserved2[3];
> 	union {				/* PSC + 0x0c */
> 		volatile u8	buffer_8;
> 		volatile u16	buffer_16;
> 		volatile u32	buffer_32;
> 	} buffer;
> #define psc_buffer_8	buffer.buffer_8
> #define psc_buffer_16	buffer.buffer_16
> #define psc_buffer_32	buffer.buffer_32
> 	union {				/* PSC + 0x10 */
> 		volatile u8	ipcr;
> 		volatile u8	acr;
> 	} ipcr_acr;
> #define psc_ipcr	ipcr_acr.ipcr
> #define psc_acr		ipcr_acr.acr
> 	volatile u8	reserved3[3];
> 	union {				/* PSC + 0x14 */
> 		volatile u16	isr;
> 		volatile u16	imr;
> 	} isr_imr;
> 
> 
> Git-blame points the finger to someone called 'wd'.  Maybe he knows
> about the advantages of such structure definitions?  

Sorry, I don;t. I'm pretty sure that I didn't write that code.

> On second check the corresponding header file in Linux (mpc52xx_uart.h) has
> stuff like this introduced by Kumar Gala, maybe he can enlighten me?
> 
> To be honest, I wouldn't even dare accessing memory mapped registers in
> the non-native size, but maybe I'm missing something here.

Kumar, do you have any explanation for this strange design?

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: wd at denx.de
Die meisten Menschen pflegen im Kindesalter vom Zeigen auf Gegenst?n-
de (Mausbewegung) und "ga" sagen  (Mausklick)  abzukommen,  zugunsten
eines  m?chtigeren  und langwierig zu erlernenden Tools (Sprache).
                             -- Achim Linder in de.comp.os.linux.misc

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Anatolij Gustschin]

Hi Detlev,

Detlev Zundel wrote:
> This patch adds advanced diagnosis functions for the inka4x0 board.
> 
> Signed-off-by: Andreas Pfefferle <ap@denx.de>
> Signed-off-by: Detlev Zundel <ap@denx.de>

Something went wrong here, duplicated e-mail address from
Andreas in your SoB. Also please see some style comments below.

> diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c
> new file mode 100644
> index 0000000..bdbf652
> --- /dev/null
> +++ b/board/inka4x0/inkadiag.c
...
> +
> +static void inka_digio_set_output(unsigned int state, int which)
> +{
> +	if (which == 0) {
> +		/* other */
> +		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) {

maybe we should add blank line before 'if' statement here?

...
> +static void ser_putc(volatile struct mpc5xxx_psc *psc, const char c)
> +{
> +	/* Wait 1 second for last character to go. */
> +	int i = 0;
> +	while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < 1000000/10))

add blank line before while().

...
> +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++ < 1000000/10))
> +		udelay(10);

add blank line before while().

...
> +U_BOOT_CMD(inkadiag, 6, 1, do_inkadiag,
> +	   "inkadiag - inka diagnosis\n",
> +	   "[inkadiag what ...]\n"
> +	   "    - perform a diagnosis on inka hardware\n"
> +	   "'inkadiag' performs hardware tests.\n\n");
> +
> +/* Relocate the command table functino pointers when running in RAM */

Typo, s/functino/function/

> +int inkadiag_init_r (void) {
> +	cmd_tbl_t *cmdtp;
> +
> +	for (cmdtp = &cmd_inkadiag_sub[0]; cmdtp !=
> +		     &cmd_inkadiag_sub[ARRAY_SIZE(cmd_inkadiag_sub)]; cmdtp++) {
> +		ulong addr;
> +		addr = (ulong) (cmdtp->cmd) + gd->reloc_off;

add blank line after declaration.

Best regards,
Anatolij

[U-Boot] [PATCH] inka4x0: Add hardware diagnosis functions for inka4x0

[Detlev Zundel]

Hi Anatolij,

> Hi Detlev,
>
> Detlev Zundel wrote:
>> This patch adds advanced diagnosis functions for the inka4x0 board.
>> 
>> Signed-off-by: Andreas Pfefferle <ap@denx.de>
>> Signed-off-by: Detlev Zundel <ap@denx.de>
>
> Something went wrong here, duplicated e-mail address from
> Andreas in your SoB. Also please see some style comments below.

Yes indeed, something went wrong here ;)  Thanks for catching this.
Actually I *knew* there was a problem because the automatically
generated coverletter had Andreas the originator of one of the patches
which I believed was markes as mine.  I simply didn't see it.  Thanks
for catching this!

>> diff --git a/board/inka4x0/inkadiag.c b/board/inka4x0/inkadiag.c
>> new file mode 100644
>> index 0000000..bdbf652
>> --- /dev/null
>> +++ b/board/inka4x0/inkadiag.c
> ...
>> +
>> +static void inka_digio_set_output(unsigned int state, int which)
>> +{
>> +	if (which == 0) {
>> +		/* other */
>> +		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) {
>
> maybe we should add blank line before 'if' statement here?

To be honest - I don't care. Fixed.

> ...
>> +static void ser_putc(volatile struct mpc5xxx_psc *psc, const char c)
>> +{
>> +	/* Wait 1 second for last character to go. */
>> +	int i = 0;
>> +	while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < 1000000/10))
>
> add blank line before while().

As you wish.

> ...
>> +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++ < 1000000/10))
>> +		udelay(10);
>
> add blank line before while().

Sure we do.

> ...
>> +U_BOOT_CMD(inkadiag, 6, 1, do_inkadiag,
>> +	   "inkadiag - inka diagnosis\n",
>> +	   "[inkadiag what ...]\n"
>> +	   "    - perform a diagnosis on inka hardware\n"
>> +	   "'inkadiag' performs hardware tests.\n\n");
>> +
>> +/* Relocate the command table functino pointers when running in RAM */
>
> Typo, s/functino/function/

Ah, thanks!

>> +int inkadiag_init_r (void) {
>> +	cmd_tbl_t *cmdtp;
>> +
>> +	for (cmdtp = &cmd_inkadiag_sub[0]; cmdtp !=
>> +		     &cmd_inkadiag_sub[ARRAY_SIZE(cmd_inkadiag_sub)]; cmdtp++) {
>> +		ulong addr;
>> +		addr = (ulong) (cmdtp->cmd) + gd->reloc_off;
>
> add blank line after declaration.

Fixed.

Thanks for taking the time to look over it!

Cheers
  Detlev

-- 
If you currently have a 32-bit UNIX system, you are advised to
trade it in for a 64-bit one sometime before the year 2106.
 -- Andrew S. Tanenbaum: Modern Operating Systems, 2nd Edition
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] drivers/twserial: Add protocol driver for "three wire serial" interface.

[Wolfgang Denk]

Dear Detlev Zundel,

In message <1237914158-15693-4-git-send-email-dzu@denx.de> you wrote:
> This pretty unintelligent interface is used on some RTC chips.
> 
> Signed-off-by: Detlev Zundel <dzu@denx.de>
...
> +#ifndef TWS_SOFT_DECLARATIONS
> +# if defined(CONFIG_MPC5xxx)
> +#  define TWS_SOFT_DECLARATIONS volatile ioport_t *iop = ioport_addr((immap_t *)CONFIG_SYS_IMMR, I2C_PORT);

Line too long.

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: wd at denx.de
How can you tell when sour cream goes bad?

[U-Boot] [PATCH] drivers/twserial: Add protocol driver for "three wire serial" interface.

[Detlev Zundel]

Hi Wolfgang,

> Dear Detlev Zundel,
>
> In message <1237914158-15693-4-git-send-email-dzu@denx.de> you wrote:
>> This pretty unintelligent interface is used on some RTC chips.
>> 
>> Signed-off-by: Detlev Zundel <dzu@denx.de>
> ...
>> +#ifndef TWS_SOFT_DECLARATIONS
>> +# if defined(CONFIG_MPC5xxx)
>> +#  define TWS_SOFT_DECLARATIONS volatile ioport_t *iop = ioport_addr((immap_t *)CONFIG_SYS_IMMR, I2C_PORT);
>
> Line too long.

Fixed.

Thanks
  Detlev

-- 
Paul Simon sang "Fifty Ways to Lose Your Lover", because quite
frankly, "A Million Ways to Tell a Developer He Is a D*ckhead"
doesn't scan nearly as well.  But I'm sure he thought about it.
                            -- linux/Documentation/ManagementStyle
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de

[U-Boot] [PATCH] mpc5xxx: Add structure definition for several more register blocks.

[Wolfgang Denk]

Dear Detlev Zundel,

In message <1237914158-15693-3-git-send-email-dzu@denx.de> you wrote:
> Signed-off-by: Detlev Zundel <dzu@denx.de>
> ---
>  include/mpc5xxx.h |  124 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 files changed, 124 insertions(+), 0 deletions(-)

Applied, thanks.

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: wd at denx.de
I still miss my ex-wife, but my aim is getting better.

[U-Boot] [PATCH v1 0/7] Update for inka4x0 plus some new features

[Wolfgang Denk]

Dear Detlev Zundel,

In message <1237914158-15693-1-git-send-email-dzu@denx.de> you wrote:
> This patchset contains updates for the ink4x0 platform.  Namely it
> adds hardware diagnostic funtions and an RTC driver.

It would have been nice if your patches gave *any* hint about the
order they in which they should be applied...

Can you please provide something like a sequence file or so?


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: wd at denx.de
When choosing between two evils, I always like to take the  one  I've
never tried before.                     -- Mae West, "Klondike Annie"

[U-Boot] [PATCH v1 0/7] Update for inka4x0 plus some new features

[Detlev Zundel]

Hi Wolfgang,

> Dear Detlev Zundel,
>
> In message <1237914158-15693-1-git-send-email-dzu@denx.de> you wrote:
>> This patchset contains updates for the ink4x0 platform.  Namely it
>> adds hardware diagnostic funtions and an RTC driver.
>
> It would have been nice if your patches gave *any* hint about the
> order they in which they should be applied...
>
> Can you please provide something like a sequence file or so?

Aah, big operator trouble.  Regenerating the patch series, without the
(already written cover letter, I inadvertently forgot "-n" for
git-format-patch, which escaped my attention as the cover-letter from
the previous version indeed *was* numbered :(

Will fix in next round.

Cheers
  Detlev

-- 
Less talking -- more hacking
                                        -- Olin Shivers
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de