Skip to contentSkip to conversation

Hardware diagnosis for inka4x0 boards.

PATCH New ap at denx.de 4 participants 14 messages Nov 20, 2008 – Dec 11, 2008 Real time clock (RTC)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Hardware diagnosis for inka4x0 boards.

From: Andreas Pfefferle <[email protected]>

This patch adds diagnosis functions for the buzzer, UARTs and digital IOs on inka4x0 hardware.
? ?

---
 board/inka4x0/Makefile   |    2 +-
 board/inka4x0/inkadiag.c |  515 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 516 insertions(+), 1 deletions(-)
 create mode 100644 board/inka4x0/inkadiag.c
board/inka4x0/Makefile +1 -1
index 442e2d0..2264dae 100644
@@ -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))
board/inka4x0/inkadiag.c new file +515 -0

515 lines not on this page · raw message

Signature · 1 line
1.6.0.4
#1 lore raw

PATCH 1/1 =?utf-8?q?rtc:=20add=20support=20for=204543=20RTC=20(manufactured=20by=20e.g.=20EPSON)

From: Andreas Pfefferle <[email protected]>

---
 drivers/rtc/Makefile  |    1 +
 drivers/rtc/rtc4543.c |  107 +++++++++++++++++++++++++++++++++++++++++++++++++
 include/rtc.h         |    4 ++
 3 files changed, 112 insertions(+), 0 deletions(-)
 create mode 100644 drivers/rtc/rtc4543.c
drivers/rtc/Makefile +1 -0
index 94bc518..6adece2 100644
@@ -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
drivers/rtc/rtc4543.c new file +107 -0

107 lines not on this page · raw message

include/rtc.h +4 -0
index 785fbe3..019c2eb 100644
@@ -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_ */
Signature · 1 line
1.6.0.4
#2 lore raw

PATCH 1/1 inka4x0: board specific RTC support.

From: Andreas Pfefferle <[email protected]>

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

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

PATCH 1/1 inka4x0: hardware diagnosis and RTC.

From: Andreas Pfefferle <[email protected]>

This patch supports added diagnosis functions and the use of the external RTC.
? ?

---
 include/configs/inka4x0.h |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
include/configs/inka4x0.h +4 -3
index 405234c..5caa405 100644
@@ -97,7 +97,8 @@
 #define CONFIG_CMD_PCI
 #define CONFIG_CMD_SNTP
 #define CONFIG_CMD_USB
-
+#define CONFIG_CMD_DATE
+#define CONFIG_CMD_PING
 
 #define	CONFIG_TIMESTAMP	1	/* Print image info with timestamp */
 
@@ -241,13 +242,13 @@
  * use PSC6_1 and PSC6_3 as GPIO: Bits 9:11 (mask: 0x07000000):
  *	011 -> PSC6 could not be used as UART or CODEC. IrDA still possible.
  */
-#define CONFIG_SYS_GPS_PORT_CONFIG	0x01001004
+#define CONFIG_SYS_GPS_PORT_CONFIG	0x01501444
 
 /*
  * RTC configuration
  */
-#define CONFIG_RTC_MPC5200	1	/* use internal MPC5200 RTC */
 
+#define CONFIG_RTC_RTC4543 	1	/* use external RTC */
 /*
  * Miscellaneous configurable options
  */
Signature · 1 line
1.6.0.4
#4 lore raw

PATCH 1/1 inka4x0: hardware diagnosis and RTC.

Quoted 3 lines · ap at denx.de

From: Andreas Pfefferle <[email protected]>

This patch supports added diagnosis functions and the

diagnosis where?

Quoted 12 lines

use of the external RTC.
? ?

---
 include/configs/inka4x0.h |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)
include/configs/inka4x0.h +0 -0
index 405234c..5caa405 100644

include/configs/inka4x0.h line 101

Quoted 6 lines
include/configs/inka4x0.h +2 -1
@@ -97,7 +97,8 @@
 #define CONFIG_CMD_PCI
 #define CONFIG_CMD_SNTP
 #define CONFIG_CMD_USB
-
+#define CONFIG_CMD_DATE
+#define CONFIG_CMD_PING

not mention in the commit message

include/configs/inka4x0.h line 245

Quoted 9 lines
include/configs/inka4x0.h +1 -1
 #define	CONFIG_TIMESTAMP	1	/* Print image info with timestamp */
 
@@ -241,13 +242,13 @@
  * use PSC6_1 and PSC6_3 as GPIO: Bits 9:11 (mask: 0x07000000):
  *	011 -> PSC6 could not be used as UART or CODEC. IrDA still possible.
  */
-#define CONFIG_SYS_GPS_PORT_CONFIG	0x01001004
+#define CONFIG_SYS_GPS_PORT_CONFIG	0x01501444

why do yuo change it?

include/configs/inka4x0.h line 251

Quoted 7 lines
include/configs/inka4x0.h +1 -1
 /*
  * RTC configuration
  */
-#define CONFIG_RTC_MPC5200	1	/* use internal MPC5200 RTC */
 
+#define CONFIG_RTC_RTC4543 	1	/* use external RTC */
			     ^

whitespace please fix

why not make configurable via if def
as
ifndef CONFIG_RTC_RTC4543
#define CONFIG_RTC_MPC5200
#endif

Best Regards,
J.

#5 lore raw

PATCH 1/1 inka4x0: hardware diagnosis and RTC.

Dear Jean-Christophe PLAGNIOL-VILLARD,

Quoted 6 lines · In message <[email protected]> you wrote:

why not make configurable via if def
as >
ifndef CONFIG_RTC_RTC4543
#define CONFIG_RTC_MPC5200
#endif

Because it doesn't make sense?

Best regards,

Wolfgang Denk

Signature · 7 lines
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
Carelessly planned projects take three times longer to complete  than
expected.  Carefully  planned  projects  take  four  times  longer to
complete than expected, mostly  because  the  planners  expect  their
planning to reduce the time it takes.
#6 lore raw

PATCH 1/1 inka4x0: hardware diagnosis and RTC.

In reply to ap at denx.de

Dear ap at denx.de,

Quoted 14 lines · In message <[email protected]> you wrote:

RnJvbTogQW5kcmVhcyBQZmVmZmVybGUgPGFwQGRlbnguZGU+CgpUaGlzIHBhdGNoIHN1cHBvcnRz IGFkZGVkIGRpYWdub3NpcyBmdW5jdGlvbnMgYW5kIHRoZQp1c2Ugb2YgdGhlIGV4dGVybmFsIFJU Qy4KwqAgwqAKU2lnbmVkLW9mZi1ieTogQW5kcmVhcyBQZmVmZmVybGUgPGFwQGRlbnguZGU+Ci0t LQogaW5jbHVkZS9jb25maWdzL2lua2E0eDAuaCB8ICAgIDcgKysrKy0tLQogMSBmaWxlcyBjaGFu Z2VkLCA0IGluc2VydGlvbnMoKyksIDMgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvaW5jbHVk ZS9jb25maWdzL2lua2E0eDAuaCBiL2luY2x1ZGUvY29uZmlncy9pbmthNHgwLmgKaW5kZXggNDA1 MjM0Yy4uNWNhYTQwNSAxMDA2NDQKLS0tIGEvaW5jbHVkZS9jb25maWdzL2lua2E0eDAuaAorKysg Yi9pbmNsdWRlL2NvbmZpZ3MvaW5rYTR4MC5oCkBAIC05Nyw3ICs5Nyw4IEBACiAjZGVmaW5lIENP TkZJR19DTURfUENJCiAjZGVmaW5lIENPTkZJR19DTURfU05UUAogI2RlZmluZSBDT05GSUdfQ01E X1VTQgotCisjZGVmaW5lIENPTkZJR19DTURfREFURQorI2RlZmluZSBDT05GSUdfQ01EX1BJTkcK IAogI2RlZmluZQlDT05GSUdfVElNRVNUQU1QCTEJLyogUHJpbnQgaW1hZ2UgaW5mbyB3aXRoIHRp bWVzdGFtcCAqLwogCkBAIC0yNDEsMTMgKzI0MiwxMyBAQAogICogdXNlIFBTQzZfMSBhbmQgUFND Nl8zIGFzIEdQSU86IEJpdHMgOToxMSAobWFzazogMHgwNzAwMDAwMCk6CiAgKgkwMTEgLT4gUFND NiBjb3VsZCBub3QgYmUgdXNlZCBhcyBVQVJUIG9yIENPREVDLiBJckRBIHN0aWxsIHBvc3NpYmxl

Quoted 8 lines

LgogICovCi0jZGVmaW5lIENPTkZJR19TWVNfR1BTX1BPUlRfQ09ORklHCTB4MDEwMDEwMDQKKyNk ZWZpbmUgQ09ORklHX1NZU19HUFNfUE9SVF9DT05GSUcJMHgwMTUwMTQ0NAogCiAvKgogICogUlRD IGNvbmZpZ3VyYXRpb24KICAqLwotI2RlZmluZSBDT05GSUdfUlRDX01QQzUyMDAJMQkvKiB1c2Ug aW50ZXJuYWwgTVBDNTIwMCBSVEMgKi8KIAorI2RlZmluZSBDT05GSUdfUlRDX1JUQzQ1NDMgCTEJ LyogdXNlIGV4dGVybmFsIFJUQyAqLwogLyoKICAqIE1pc2NlbGxhbmVvdXMgY29uZmlndXJhYmxl IG9wdGlvbnMKICAqLwotLSAKMS42LjAuNAoKX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX18KVS1Cb290IG1haWxpbmcgbGlzdApVLUJvb3RAbGlzdHMuZGVueC5k ZQpodHRwOi8vbGlzdHMuZGVueC5kZS9tYWlsbWFuL2xpc3RpbmZvL3UtYm9vdAo=

Please do not send base64 encoded messages.

include/configs/inka4x0.h line 101

Quoted 11 lines
include/configs/inka4x0.h +2 -1
index 405234c..5caa405 100644
@@ -97,7 +97,8 @@
 #define CONFIG_CMD_PCI
 #define CONFIG_CMD_SNTP
 #define CONFIG_CMD_USB
-
+#define CONFIG_CMD_DATE
+#define CONFIG_CMD_PING

Please keep lists sorted.

include/configs/inka4x0.h line 245

Quoted 6 lines
include/configs/inka4x0.h +1 -1
@@ -241,13 +242,13 @@
  * use PSC6_1 and PSC6_3 as GPIO: Bits 9:11 (mask: 0x07000000):
  *	011 -> PSC6 could not be used as UART or CODEC. IrDA still possible.
  */
-#define CONFIG_SYS_GPS_PORT_CONFIG	0x01001004
+#define CONFIG_SYS_GPS_PORT_CONFIG	0x01501444

Don't we need a change to the comments above, too?

include/configs/inka4x0.h line 254

Quoted 9 lines
include/configs/inka4x0.h +1 -1
 /*
  * RTC configuration
  */
-#define CONFIG_RTC_MPC5200	1	/* use internal MPC5200 RTC */
 
+#define CONFIG_RTC_RTC4543 	1	/* use external RTC */
 /*
  * Miscellaneous configurable options
  */

Please keep white space as it was before.

There is no such thing as CONFIG_RTC_RTC4543 in U-Boot. Eventually your patch depends on othere patches that need to be applied before? You should then mark this by submitting a numbered series of patches.

Best regards,

Wolfgang Denk

Signature · 5 lines
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 think trash is the most important manifestation of culture we have
in my lifetime."                                      - Johnny Legend
#7 lore raw

PATCH 1/1 inka4x0: board specific RTC support.

In reply to ap at denx.de
Quoted 33 lines · ap at denx.de

From: Andreas Pfefferle <[email protected]>

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

---
 board/inka4x0/inka4x0.c |  124 +++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 124 insertions(+), 0 deletions(-)
board/inka4x0/inka4x0.c +4 -0
index 507196b..6312b4f 100644
@@ -8,6 +8,9 @@
  * (C) Copyright 2004
  * Martin Krause, TQ-Systems GmbH, martin.krause at tqs.de
  *
+ * (C) Copyright 2008
+ * Andreas Pfefferle, DENX Software Engineering, ap at denx.de.
+ *
  * See file CREDITS for list of people who contributed to this
  * project.
  *
@@ -27,6 +30,7 @@
  * MA 02111-1307 USA
  */
 
+#include <asm/io.h>
 #include <common.h>
 #include <mpc5xxx.h>
 #include <pci.h>

board/inka4x0/inka4x0.c line 52

Quoted 4 lines
board/inka4x0/inka4x0.c +1 -0
@@ -45,6 +49,111 @@
 #error "INKA4x0 SDRAM: invalid chip type specified!"
 #endif
 
+#if defined(CONFIG_RTC_RTC4543) && defined(CONFIG_CMD_DATE)

please create a rtc.c file and move the compile condition to Makefile

board/inka4x0/inka4x0.c line 54

Quoted 2 lines
board/inka4x0/inka4x0.c +2 -0
+
+#define RTC_CE    0x01000000
                 ^^^^

please use tab instead of whitespcase and so on

board/inka4x0/inka4x0.c line 59

Quoted 5 lines
board/inka4x0/inka4x0.c +5 -0
+#define RTC_WR    0x02000000
+#define RTC_DATA  0x01
+#define RTC_CLK   0x02
+
+#define RTCWRITE(n, data) do {\
                           ^^

please use tab

board/inka4x0/inka4x0.c line 60

Quoted 1 line
board/inka4x0/inka4x0.c +1 -0
+  for (i = 0; i < n; i++) {\

^^
please use tab

board/inka4x0/inka4x0.c line 69

Quoted 9 lines
board/inka4x0/inka4x0.c +9 -0
+	gpio->sint_dvo &= ~RTC_DATA;\
+	if (data & (1 << i))\
+		gpio->sint_dvo |= RTC_DATA;\
+	udelay(10);\
+	gpio->sint_dvo |= RTC_CLK;\
+	udelay(10);\
+	gpio->sint_dvo &= ~RTC_CLK;\
+	udelay(10);\
+  } \

^^
please use tab

board/inka4x0/inka4x0.c line 70

Quoted 1 line
board/inka4x0/inka4x0.c +1 -0
+} while (0)

why not a inline function?

board/inka4x0/inka4x0.c line 72

Quoted 2 lines
board/inka4x0/inka4x0.c +2 -0
+
+#define RTCREAD(n, data)  do {\
                           ^^

please use tab

board/inka4x0/inka4x0.c line 73

Quoted 1 line
board/inka4x0/inka4x0.c +1 -0
+  for (i = 0; i < n; i++) {\

^^
please use tab

board/inka4x0/inka4x0.c line 79

Quoted 6 lines
board/inka4x0/inka4x0.c +6 -0
+	gpio->sint_dvo |= RTC_CLK;\
+	udelay(10);\
+	data |= ((gpio->sint_ival & RTC_DATA) << i);\
+	gpio->sint_dvo &= ~RTC_CLK;\
+	udelay(10);\
+  } \

^^
please use tab

board/inka4x0/inka4x0.c line 80

Quoted 1 line
board/inka4x0/inka4x0.c +1 -0
+} while (0)

ditto

board/inka4x0/inka4x0.c line 86

Quoted 6 lines
board/inka4x0/inka4x0.c +6 -0
+
+uchar rtc_read(uchar reg)
+{
+	struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
+	int i;
+	uchar val = 0;

please add a blank line

Quoted 27 lines
board/inka4x0/inka4x0.c +27 -0
+	switch (reg) {
+	case 0:
+		/* PSC3_4 is input */
+		gpio->sint_ddr &= ~RTC_DATA;
+		/* Lower WR */
+		out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
+			in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_WR);
+		udelay(1);
+		/* Rise CE */
+		out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
+			in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_CE);
+		udelay(1);
+		break;
+	case 1:
+	case 2:
+	case 3:
+	case 5:
+	case 6:
+	case 7:
+		RTCREAD(8, val);
+		break;
+	case 4:
+		RTCREAD(4, val);
+		break;
+	case 8:
+		/* Lower CE */
+		out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,

board/inka4x0/inka4x0.c line 123

Quoted 10 lines
board/inka4x0/inka4x0.c +10 -0
+			in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_CE);
+		break;
+	}
+	return val;
+}
+
+void rtc_write(uchar reg, uchar val)
+{
+	struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
+	int i;

please add a blank line

Quoted 41 lines
board/inka4x0/inka4x0.c +34 -0
+	switch (reg) {
+	case 0:
+		/* PSC3_4 is output */
+		gpio->sint_ddr |= RTC_DATA;
+		/* Rise WR */
+		out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
+			in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_WR);
+		udelay(1);
+		/* Rise CE */
+		out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
+			in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) | RTC_CE);
+		udelay(1);
+		break;
+	case 1:
+	case 2:
+	case 3:
+	case 5:
+	case 6:
+	case 7:
+		RTCWRITE(8, val);
+		break;
+	case 4:
+		RTCWRITE(4, val);
+		break;
+	case 8:
+		/* Lower CE */
+		out_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O,
+			in_be32((unsigned *)MPC5XXX_WU_GPIO_DATA_O) & ~RTC_CE);
+		break;
+	}
+}
+#endif
+
 #ifndef CONFIG_SYS_RAMBOOT
 static void sdram_start (int hi_addr)
 {
@@ -220,6 +329,21 @@ int misc_init_f (void)
 	*(vu_long *) MPC5XXX_WU_GPIO_ENABLE |= GPIO_PSC3_9;
 	*(vu_long *) MPC5XXX_WU_GPIO_DIR    |= GPIO_PSC3_9;
 	*(vu_long *) MPC5XXX_WU_GPIO_DATA_O   |= GPIO_PSC3_9;
+

board/inka4x0/inka4x0.c int misc_init_f (void) · line 342

Quoted 10 lines
board/inka4x0/inka4x0.c +10 -0
+	/* Configure RTC and Display */
+	struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;
+	out_be32((unsigned *)MPC5XXX_WU_GPIO_ENABLE,
+		  in_be32((unsigned *)MPC5XXX_WU_GPIO_ENABLE) | 0x03000000);
+	out_be32((unsigned *)MPC5XXX_WU_GPIO_DIR,
+		  in_be32((unsigned *)MPC5XXX_WU_GPIO_DIR) | 0x03000000);
+	gpio->sint_gpioe |= 0x07;
+	gpio->sint_ddr |= 0x06;
+	gpio->sint_inten &= ~0x03;
+	*(u_long *)MPC5XXX_WU_GPIO_DATA_O &= ~0x01000000;

please do not use diect access

board/inka4x0/inka4x0.c int misc_init_f (void) · line 348

Quoted 6 lines
board/inka4x0/inka4x0.c +4 -0
+	gpio->sint_dvo &= ~0x02;
+	gpio->sint_dvo |= 0x04;
+	/* end of RTC and Display configuration */
+
 	return 0;
 }

Best Regards,
J.

#8 lore raw

PATCH 1/1 inka4x0: board specific RTC support.

In reply to ap at denx.de

Dear ap at denx.de,

Quoted 4 lines · In message <[email protected]> you wrote:

From: Andreas Pfefferle <[email protected]>

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

No, that's broken by design. An RTC is not board-dependent, but a generic piece of hardware. Code for it does not belong into any board-specific file, but into drivers/rtc/

Best regards,

Wolfgang Denk

Signature · 5 lines
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
Wenn das dann in die Hose geht, nehme ich es auf meine Kappe.
                                         -- Rudi V?ller, 15. Nov 2003
#9 lore raw

PATCH 1/1 inka4x0: board specific RTC support.

Hi Wolfgang,

Quoted 11 lines

Dear ap at denx.de,

Quoted 4 lines · In message <[email protected]> you wrote:

From: Andreas Pfefferle <[email protected]>

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

No, that's broken by design. An RTC is not board-dependent, but a generic piece of hardware. Code for it does not belong into any board-specific file, but into drivers/rtc/

Did you actually check what he does?

He does exactly what you request, he puts all the 4543 code into drivers/rtc, but this cannot globally encode on how to access the chip from a specific hardware. So he factored that out of the rtc driver (correctly in my opinion) and has only this glue code in the board files.

What *exactly* is broken by design in this approach?

Cheers
Detlev

Signature · 8 lines
FORTRAN's tragic fate has been its wide acceptance, mentally chaining thousands
and thousands of programmers to our past mistakes. I pray daily that more of my
fellow-programmers  may find the means of  freeing themselves from the curse of
compatibility.                          -- Edsger W. Dijkstra
--
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
#10 lore raw

PATCH 1/1 =?utf-8?q?rtc:=20add=20support=20for=204543=20RTC=20(manufactured=20by=20e.g.=20EPSON)

In reply to ap at denx.de
Quoted 7 lines

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

drivers/rtc/rtc4543.c line 31

Quoted 9 lines
drivers/rtc/rtc4543.c new file +9 -0
+
+#include <asm/io.h>
+#include <common.h>
+#include <command.h>
+#include <config.h>
+#include <bcd.h>
+#include <rtc.h>
+
+#if defined(CONFIG_RTC_RTC4543) && defined(CONFIG_CMD_DATE)

please pove this to Makefile

Quoted 17 lines
drivers/rtc/rtc4543.c new file +17 -0
+
+int rtc_get(struct rtc_time *tmp)
+{
+	int rel = 0;
+	uchar sec, min, hour, mday, wday, mon, year;
+
+	rtc_read(0x00);
+	sec	= rtc_read(0x01);
+	min	= rtc_read(0x02);
+	hour	= rtc_read(0x03);
+	wday	= rtc_read(0x04);
+	mday	= rtc_read(0x05);
+	mon	= rtc_read(0x06);
+	year	= rtc_read(0x07);
+	rtc_read(0x08);
+
+	debug("Get RTC year: %02x mon/cent: %02x mday: %02x wday: %02x "

drivers/rtc/rtc4543.c line 58

Quoted 10 lines
drivers/rtc/rtc4543.c new file +10 -0
+		"hr: %02x min: %02x sec: %02x\n",
+		year, mon, mday, wday,
+		hour, min, sec);
+
+	if (sec & 0x80) {
+		puts("### Warning: RTC Low Voltage - date/time not reliable\n");
+		rel = -1;
+	}
+
+	tmp->tm_sec  = BCD2BIN(sec  & 0x7F);
                   ^^

please use tab instead of whitespace

Quoted 20 lines
drivers/rtc/rtc4543.c new file +20 -0
+	tmp->tm_min  = BCD2BIN(min  & 0x7F);
+	tmp->tm_hour = BCD2BIN(hour & 0x3F);
+	tmp->tm_mday = BCD2BIN(mday & 0x3F);
+	tmp->tm_mon  = BCD2BIN(mon & 0x1F);
+	tmp->tm_year = BCD2BIN(year);
+	tmp->tm_wday = BCD2BIN(wday & 0x07);
+	tmp->tm_yday = 0;
+	tmp->tm_isdst = 0;
+
+	debug("Get DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
+		tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,
+		tmp->tm_hour, tmp->tm_min, tmp->tm_sec);
+
+	return rel;
+}
+
+int rtc_set(struct rtc_time *tmp)
+{
+	debug("Set DATE: %4d-%02d-%02d (wday=%d)  TIME: %2d:%02d:%02d\n",
+		tmp->tm_year, tmp->tm_mon, tmp->tm_mday, tmp->tm_wday,

drivers/rtc/rtc4543.c line 88

Quoted 10 lines
drivers/rtc/rtc4543.c new file +10 -0
+		tmp->tm_hour, tmp->tm_min, tmp->tm_sec);
+
+	rtc_write(0x00, 0);
+	rtc_write(0x01, BIN2BCD(tmp->tm_sec));
+	rtc_write(0x02, BIN2BCD(tmp->tm_min));
+	rtc_write(0x03, BIN2BCD(tmp->tm_hour));
+	rtc_write(0x04, BIN2BCD(tmp->tm_wday));
+	rtc_write(0x05, BIN2BCD(tmp->tm_mday));
+	rtc_write(0x06, BIN2BCD(tmp->tm_mon));
+	rtc_write(0x07, BIN2BCD(tmp->tm_year  % 100));
					    ^^

whitespace please fix

drivers/rtc/rtc4543.c line 96

Quoted 8 lines
drivers/rtc/rtc4543.c new file +8 -0
+	rtc_write(0x08, 0);
+
+	return 0;
+}
+
+void rtc_reset(void)
+{
+  struct rtc_time tmp;

please add an empty line and use tab for indentation

Quoted 16 lines
drivers/rtc/rtc4543.c new file +11 -0
+  tmp.tm_sec = 0;
+  tmp.tm_min = 0;
+  tmp.tm_hour = 0;
+  tmp.tm_wday = 4;
+  tmp.tm_mday = 1;
+  tmp.tm_mon = 1;
+  tmp.tm_year = 1970;
+  rtc_set(&tmp);
+}
+
+#endif
include/rtc.h +0 -0
index 785fbe3..019c2eb 100644

include/rtc.h void to_tm (int, struct rtc_time *); · line 65

Quoted 5 lines
include/rtc.h +2 -0
@@ -61,4 +61,8 @@ void to_tm (int, struct rtc_time *);
 unsigned long mktime (unsigned int, unsigned int, unsigned int,
 		      unsigned int, unsigned int, unsigned int);
 
+uchar rtc_read(uchar reg) __attribute__((weak));
+void  rtc_write(uchar reg, uchar val) __attribute__((weak));

please private default function

and if you add it please fix the other rtc drivers too

Best Regards,
J.

#11 lore raw

PATCH 1/1 Hardware diagnosis for inka4x0 boards.

In reply to ap at denx.de
Quoted 11 lines · ap at denx.de

From: Andreas Pfefferle <[email protected]>

This patch adds diagnosis functions for the buzzer, UARTs and digital IOs on inka4x0 hardware.
? ?

---
 board/inka4x0/Makefile   |    2 +-
 board/inka4x0/inkadiag.c |  515 ++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 516 insertions(+), 1 deletions(-)
 create mode 100644 board/inka4x0/inkadiag.c

why not make this diagnostic active via CONFIG_?

Quoted 50 lines
board/inka4x0/Makefile +1 -1
index 442e2d0..2264dae 100644
@@ -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))
board/inka4x0/inkadiag.c new file +30 -0
new file mode 100644
index 0000000..6194b58
@@ -0,0 +1,515 @@
+/*
+ * (C) Copyright 2008
+ * 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 <config.h>
+#include <mpc5xxx.h>
+#include <pci.h>
+
+#include <command.h>

board/inka4x0/inkadiag.c line 40

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+
+#define TEST_NAME(test, name) static char test##_program_name[] = name
+#define TEST_USAGE(test, usage) static char test##_program_usage[] = usage
+#define TEST_FUNCTION(test) do_##test
+#define PRINT_TEST_USAGE(test) do {\
+	printf(test##_program_usage, test##_program_name);\
+	return -1;\
+} while (0)
+
+#define GPIO_BASE  (u_char *)0x30400000
                    ^^

whitespace please remove

Quoted 16 lines
board/inka4x0/inkadiag.c new file +16 -0
+
+#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 */

board/inka4x0/inkadiag.c line 66

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -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)\
+out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\
+	  in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) | 0x10)

why not inline?

board/inka4x0/inkadiag.c line 70

Quoted 4 lines
board/inka4x0/inkadiag.c new file +4 -0
+
+#define LED_LOW(NUM)\
+out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\
+	  in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) & ~0x10)

why not inline?

board/inka4x0/inkadiag.c line 78

Quoted 9 lines
board/inka4x0/inkadiag.c new file +9 -0
+
+
+#define CHECK_LED(NUM) do {\
+	if (state & (1 << NUM)) {\
+		LED_HIGH(NUM);\
+	} else {\
+		LED_LOW(NUM);\
+	} \
+} while (0)

why not inline?

board/inka4x0/inkadiag.c line 90

Quoted 13 lines
board/inka4x0/inkadiag.c new file +13 -0
+
+
+static void inka_digio_set_output(unsigned int state, int which)
+{
+	if (which == 0) {
+		CHECK_LED(0);
+		CHECK_LED(1);
+		CHECK_LED(2);
+		CHECK_LED(3);
+		CHECK_LED(4);
+		CHECK_LED(5);
+	} else {
+		struct mpc5xxx_gpio *gpio = (struct mpc5xxx_gpio *)MPC5XXX_GPIO;

please add a blank line

Quoted 25 lines
board/inka4x0/inkadiag.c new file +25 -0
+		if (which == 1) {
+			if (state & DIGIO_DRAWER1) {
+				gpio->simple_dvo &= ~0x1000;
+				udelay(1);
+				gpio->simple_dvo |= 0x1000;
+			} else {
+				gpio->simple_dvo |= 0x1000;
+				udelay(1);
+				gpio->simple_dvo &= ~0x1000;
+			}
+		}
+		if (which == 2) {
+			if (state & DIGIO_DRAWER2) {
+				gpio->simple_dvo &= ~0x2000;
+				udelay(1);
+				gpio->simple_dvo |= 0x2000;
+			} else {
+				gpio->simple_dvo |= 0x2000;
+				udelay(1);
+				gpio->simple_dvo &= ~0x2000;
+			}
+		}
+	}
+	udelay(1);
+}

board/inka4x0/inkadiag.c line 125

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+
+TEST_NAME(io, "inkadiag io");
+TEST_USAGE(io, "Usage: %s\n\
+       <which> # get which[drawer1|drawer2|other] input\n\
+       <which> <val> # set which[drawer1|drawer2|other] output to val\n\
+");
+
+static int io_helper(int argc, char *argv[])
+{
+	unsigned int state = inka_digin_get_input();

please add a blank line

Quoted 15 lines
board/inka4x0/inkadiag.c new file +15 -0
+	if (strcmp(argv[1], "drawer1") == 0) {
+		printf("exit code: 0x%X\n",
+		    (state & DIGIN_DRAWER_SW1) >> (ffs(DIGIN_DRAWER_SW1) - 1));
+	} else if (strcmp(argv[1], "drawer2") == 0) {
+		printf("exit code: 0x%X\n",
+		    (state & DIGIN_DRAWER_SW2) >> (ffs(DIGIN_DRAWER_SW2) - 1));
+	} else if (strcmp(argv[1], "other") == 0) {
+		printf("exit code: 0x%X\n",
+		      ((state & DIGIN_KEYB_MASK) >> (ffs(DIGIN_KEYB_MASK) - 1))
+		      | ((state & DIGIN_TOUCHSCR_MASK) >>
+		      (ffs(DIGIN_TOUCHSCR_MASK) - 2)));
+	} else {
+		printf("Invalid argument: %s\n", argv[1]);
+		return -1;
+	}

board/inka4x0/inkadiag.c line 150

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+	return 0;
+}
+
+static int TEST_FUNCTION(io) (cmd_tbl_t *cmdtp, int flag, int argc,
+			      char *argv[]) {
+	if ((argc < 2) || (argc > 3))
+		PRINT_TEST_USAGE(io);
+
+	if (argc == 2)
+		return io_helper(argc, argv);

why not instead

	if (argc == 2)
		return io_helper(argc, argv);
	else
		PRINT_TEST_USAGE(io);

board/inka4x0/inkadiag.c line 154

Quoted 2 lines
board/inka4x0/inkadiag.c new file +2 -0
+
+	unsigned int val = simple_strtol(argv[2], NULL, 16);

declaration must at the beginning of the function

Quoted 49 lines
board/inka4x0/inkadiag.c new file +49 -0
+	if (strcmp(argv[1], "drawer1") == 0) {
+		val <<= (ffs(DIGIO_DRAWER1) - 1);
+		inka_digio_set_output(val, 1);
+	} else if (strcmp(argv[1], "drawer2") == 0) {
+		val <<= (ffs(DIGIO_DRAWER2) - 1);
+		inka_digio_set_output(val, 2);
+	} else if (strcmp(argv[1], "other") == 0)
+		inka_digio_set_output(val, 0);
+	else {
+		printf("Invalid argument: %s\n", argv[1]);
+		return -1;
+	}
+	return io_helper(argc, argv);
+}
+
+DECLARE_GLOBAL_DATA_PTR;
+
+static int ser_init(volatile struct mpc5xxx_psc *psc, int baudrate)
+{
+	unsigned long baseclk;
+	int div;
+
+	/* reset PSC */
+	psc->command = PSC_SEL_MODE_REG_1;
+
+	/* select clock sources */
+
+	psc->psc_clock_select = 0;
+	baseclk = (gd->ipb_clk + 16) / 32;
+
+	/* switch to UART mode */
+	psc->sicr = 0;
+
+	/* configure parity, bit length and so on */
+
+	psc->mode = PSC_MODE_8_BITS | PSC_MODE_PARNONE;
+
+	psc->mode = PSC_MODE_ONE_STOP;
+
+	/* set up UART divisor */
+	div = (baseclk + (baudrate / 2)) / baudrate;
+	psc->ctur = (div >> 8) & 0xff;
+	psc->ctlr = div & 0xff;
+
+	/* disable all interrupts */
+	psc->psc_imr = 0;
+
+	/* reset and enable Rx/Tx */
+	psc->command = PSC_RST_RX;

board/inka4x0/inkadiag.c line 212

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+	psc->command = PSC_RST_TX;
+	psc->command = PSC_RX_ENABLE | PSC_TX_ENABLE;
+
+	return 0;
+}
+
+static void ser_putc(volatile struct mpc5xxx_psc *psc, const char c)
+{
+	/* Wait for last character to go. */
+	int i = 0;

please add a blank line

board/inka4x0/inkadiag.c line 222

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+	while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < CONFIG_SYS_HZ))
+		udelay(10);
+	psc->psc_buffer_8 = c;
+
+}
+
+static int ser_getc(volatile struct mpc5xxx_psc *psc)
+{
+	/* Wait for a character to arrive. */
+	int i = 0;

please add a blank line

Quoted 9 lines
board/inka4x0/inkadiag.c new file +9 -0
+	while (!(psc->psc_status & PSC_SR_RXRDY) && (i++ < CONFIG_SYS_HZ))
+		udelay(10);
+
+	return psc->psc_buffer_8;
+}
+
+#define SERIAL_PORT_BASE	(u_char *)0x80000000
+
+#define UART_RX		0	/* In:  Receive buffer (DLAB=0) */

board/inka4x0/inkadiag.c line 241

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+#define UART_TX		0	/* Out: Transmit buffer (DLAB=0) */
+#define UART_DLL	0	/* Out: Divisor Latch Low (DLAB=1) */
+
+#define UART_LCR	3	/* Out: Line Control Register */
+#define UART_MCR	4	/* Out: Modem Control Register */
+
+#define UART_LSR	5	/* In:  Line Status Register */
+#define UART_MSR	6	/* In:  Modem Status Register */
+
+#define UART_LCR_WLEN8  0x03	/* Wordlength: 8 bits */
			 ^^

please use tab

Quoted 30 lines
board/inka4x0/inkadiag.c new file +30 -0
+#define UART_LCR_DLAB	0x80	/* Divisor latch access bit */
+
+#define UART_LSR_THRE	0x20	/* Transmit-hold-register empty */
+#define UART_LSR_DR	0x01	/* Receiver data ready */
+
+#define UART_MCR_LOOP	0x10	/* Enable loopback test mode */
+#define UART_MCR_RTS	0x02	/* RTS complement */
+#define UART_MCR_DTR	0x01	/* DTR complement */
+
+#define UART_MSR_DCD	0x80	/* Data Carrier Detect */
+#define UART_MSR_DSR	0x20	/* Data Set Ready */
+#define UART_MSR_CTS	0x10	/* Clear to Send */
+#define PSC_OP1_RTS		0x01
+#define PSC_OP0_RTS		0x01
+
+static unsigned char serial_in(unsigned char num, int offset)
+{
+	return in_8(SERIAL_PORT_BASE + (num << 3) + offset);
+}
+
+static void serial_out(unsigned char num, int offset, unsigned char value)
+{
+	out_8(SERIAL_PORT_BASE + (num << 3) + offset, value);
+}
+
+TEST_NAME(serial, "inkadiag serial");
+TEST_USAGE(serial, "Usage: %s\n\
+       <num> <mode> <baudrate> <msg> # test uart num [0..11] in mode\
+ and baudrate with msg\n\
+");

board/inka4x0/inkadiag.c line 281

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+
+static int TEST_FUNCTION(serial) (cmd_tbl_t *cmdtp, int flag, int argc,
+				  char *argv[]) {
+	if (argc < 5)
+		PRINT_TEST_USAGE(serial);
+
+	argc--;
+	argv++;
+
+	unsigned int num = simple_strtol(argv[0], NULL, 0);

declaration must at the beginning of the function

board/inka4x0/inkadiag.c line 287

Quoted 6 lines
board/inka4x0/inkadiag.c new file +6 -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);

declaration must at the beginning of the function

board/inka4x0/inkadiag.c line 289

Quoted 2 lines
board/inka4x0/inkadiag.c new file +2 -0
+
+	int combrd = 0;

declaration must at the beginning of the function

Quoted 14 lines
board/inka4x0/inkadiag.c new file +14 -0
+
+	if (strcmp(argv[2], "115200") == 0)
+		combrd = 1;
+	else if (strcmp(argv[2], "57600") == 0)
+		combrd = 2;
+	else if (strcmp(argv[2], "38400") == 0)
+		combrd = 3;
+	else if (strcmp(argv[2], "19200") == 0)
+		combrd = 6;
+	else if (strcmp(argv[2], "9600") == 0)
+		combrd = 12;
+	else if (strcmp(argv[2], "2400") == 0)
+		combrd = 48;
+	else if (strcmp(argv[2], "1200") == 0)

board/inka4x0/inkadiag.c line 313

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+		combrd = 96;
+	else if (strcmp(argv[2], "300") == 0)
+		combrd = 384;
+	else
+		printf("Invalid baudrate: %s", argv[2]);
+
+	printf("Testing uart %d.\n\n", num);
+
+	if ((num >= 0) && (num <= 7)) {
+		if (mode & 1)

please add {}

Quoted 23 lines
board/inka4x0/inkadiag.c new file +23 -0
+			/* 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);

board/inka4x0/inkadiag.c line 346

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+			udelay(10);
+			/* check data set ready and carrier detect */
+			if ((serial_in(num, UART_MSR) &
+			     (UART_MSR_DSR | UART_MSR_DCD))
+			    != (UART_MSR_DSR | UART_MSR_DCD))
+				return -1;
+		}
+
+		/* write each message-character, read it back, and display it */
+		int i, len = strlen(argv[3]);

declaration must at the beginning of the function

board/inka4x0/inkadiag.c line 348

Quoted 2 lines
board/inka4x0/inkadiag.c new file +2 -0
+		for (i = 0; i < len; ++i) {
+			int j = 0;

please add a blank line

Quoted 11 lines
board/inka4x0/inkadiag.c new file +11 -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)

board/inka4x0/inkadiag.c line 369

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+					break;
+				udelay(10);
+			}
+			unsigned char data = serial_in(num, UART_RX);
+			printf("%c", data);
+		}
+		printf("\n\n");
+		serial_out(num, UART_MCR, 0x00);
+	} else {
+		int address, irq;

please add a blank line

Quoted 20 lines
board/inka4x0/inkadiag.c new file +20 -0
+		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;

board/inka4x0/inkadiag.c line 399

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+		ser_init(psc, simple_strtol(argv[2], NULL, 0));
+		if (mode & 2) {
+			/* set request to send */
+			out_8(&psc->op0, PSC_OP0_RTS);
+			udelay(10);
+			/* check clear to send */
+			if ((in_8(&psc->ip) & PSC_IPCR_CTS) == 0)
+				return -1;
+		}
+		int i, len = strlen(argv[3]);

declaration must at the beginning of the function

Quoted 16 lines
board/inka4x0/inkadiag.c new file +16 -0
+		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;

board/inka4x0/inkadiag.c line 425

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+	const u32 width = 64;
+
+	gpt->cir = (prescale << 16) | count;
+	gpt->pwmcr = width << 16;
+	gpt->emsr = 3;		/* Timer enabled for PWM */
+}
+
+static void buzzer_turn_off(void)
+{
+	struct mpc5xxx_gpt *gpt = (struct mpc5xxx_gpt *)(BUZZER_GPT);

please add a blank line

Quoted 8 lines
board/inka4x0/inkadiag.c new file +8 -0
+	gpt->emsr = 0;
+}
+
+TEST_NAME(buzzer, "inkadiag buzzer");
+TEST_USAGE(buzzer, "Usage: %s\n\
+       <period> <freq> # turn buzzer on for period ms with freq hz\n\
+");
+

board/inka4x0/inkadiag.c line 443

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+static int TEST_FUNCTION(buzzer) (cmd_tbl_t *cmdtp, int flag, int argc,
+				  char *argv[]) {
+
+	if (argc != 3)
+		PRINT_TEST_USAGE(buzzer);
+
+	argc--;
+	argv++;
+
+	unsigned int period = simple_strtol(argv[0], NULL, 0);

declaration must at the beginning of the function

board/inka4x0/inkadiag.c line 449

Quoted 6 lines
board/inka4x0/inkadiag.c new file +6 -0
+	if (!period)
+		printf("Zero period is senseless\n");
+	argc--;
+	argv++;
+
+	unsigned int freq = simple_strtol(argv[0], NULL, 0);

declaration must at the beginning of the function

board/inka4x0/inkadiag.c line 460

Quoted 11 lines
board/inka4x0/inkadiag.c new file +11 -0
+	/* avoid zero prescale in buzzer_turn_on() */
+	if (freq > gd->ipb_clk / 128)
+		printf("%dHz exceeds maximum (%dHz)\n", freq,
+		       gd->ipb_clk / 128);
+	else if (!freq)
+		printf("Zero frequency is senseless\n");
+	else
+		buzzer_turn_on(freq);
+
+	clear_ctrlc();
+	int prev = disable_ctrlc(0);

declaration must at the beginning of the function

Quoted 5 lines
board/inka4x0/inkadiag.c new file +5 -0
+
+	printf("Buzzing for %d ms. Type ^C to abort!\n\n", period);
+
+	int i = 0;
+

board/inka4x0/inkadiag.c line 475

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+	while (!ctrlc() && (i++ < CONFIG_SYS_HZ))
+		udelay(period);
+
+	clear_ctrlc();
+	disable_ctrlc(prev);
+
+	buzzer_turn_off();
+
+	return 0;
+}

Best Regards,
J.

#12 lore raw

PATCH 1/1 Hardware diagnosis for inka4x0 boards.

In reply to ap at denx.de

Dear ap at denx.de,

Quoted 6 lines · In message <[email protected]> you wrote:

RnJvbTogQW5kcmVhcyBQZmVmZmVybGUgPGFwQGRlbnguZGU+CgpUaGlzIHBhdGNoIGFkZHMgZGlh Z25vc2lzIGZ1bmN0aW9ucyBmb3IgdGhlIGJ1enplciwgVUFSVHMgYW5kIGRpZ2l0YWwKSU9zIG9u IGlua2E0eDAgaGFyZHdhcmUuCsKgIMKgClNpZ25lZC1vZmYtYnk6IEFuZHJlYXMgUGZlZmZlcmxl IDxhcEBkZW54LmRlPgotLS0KIGJvYXJkL2lua2E0eDAvTWFrZWZpbGUgICB8ICAgIDIgKy0KIGJv YXJkL2lua2E0eDAvaW5rYWRpYWcuYyB8ICA1MTUgKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrKysrKwogMiBmaWxlcyBjaGFuZ2VkLCA1MTYgaW5zZXJ0aW9ucygrKSwg

Quoted 8 lines

MSBkZWxldGlvbnMoLSkKIGNyZWF0ZSBtb2RlIDEwMDY0NCBib2FyZC9pbmthNHgwL2lua2FkaWFn LmMKCmRpZmYgLS1naXQgYS9ib2FyZC9pbmthNHgwL01ha2VmaWxlIGIvYm9hcmQvaW5rYTR4MC9N YWtlZmlsZQppbmRleCA0NDJlMmQwLi4yMjY0ZGFlIDEwMDY0NAotLS0gYS9ib2FyZC9pbmthNHgw L01ha2VmaWxlCisrKyBiL2JvYXJkL2lua2E0eDAvTWFrZWZpbGUKQEAgLTI1LDcgKzI1LDcgQEAg aW5jbHVkZSAkKFRPUERJUikvY29uZmlnLm1rCiAKIExJQgk9ICQob2JqKWxpYiQoQk9BUkQpLmEK IAotQ09CSlMJOj0gJChCT0FSRCkubworQ09CSlMJOj0gJChCT0FSRCkubyAgaW5rYWRpYWcubwog CiBTUkNTCTo9ICQoU09CSlM6Lm89LlMpICQoQ09CSlM6Lm89LmMpCiBPQkpTCTo9ICQoYWRkcHJl Zml4ICQob2JqKSwkKENPQkpTKSkKZGlmZiAtLWdpdCBhL2JvYXJkL2lua2E0eDAvaW5rYWRpYWcu

...

Please do not post base64 encoded messages!

Any further such postings will be ignored.

Quoted 1 line

This patch adds diagnosis functions for the buzzer, UARTs and digital

Such hardware test code should be implemented as part of the POST system, i. e. please create a post/board/inka4x0/ directory for it, and make it part of the POST.

Quoted 5 lines
board/inka4x0/inkadiag.c new file +0 -0
new file mode 100644
index 0000000..6194b58

...

board/inka4x0/inkadiag.c line 70

Quoted 7 lines
board/inka4x0/inkadiag.c new file +7 -0
+#define LED_HIGH(NUM)\
+out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\
+	  in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) | 0x10)
+
+#define LED_LOW(NUM)\
+out_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE,\
+	  in_be32((unsigned *)MPC5XXX_GPT##NUM##_ENABLE) & ~0x10)

Please use "do { ...} while (0)" wrappers for these macros.

Quoted 9 lines
board/inka4x0/inkadiag.c new file +9 -0
+static int io_helper(int argc, char *argv[])
+{
+	unsigned int state = inka_digin_get_input();
+	if (strcmp(argv[1], "drawer1") == 0) {
+		printf("exit code: 0x%X\n",
+		    (state & DIGIN_DRAWER_SW1) >> (ffs(DIGIN_DRAWER_SW1) - 1));
+	} else if (strcmp(argv[1], "drawer2") == 0) {
+		printf("exit code: 0x%X\n",
+		    (state & DIGIN_DRAWER_SW2) >> (ffs(DIGIN_DRAWER_SW2) - 1));

board/inka4x0/inkadiag.c line 141

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+	} else if (strcmp(argv[1], "other") == 0) {
+		printf("exit code: 0x%X\n",
+		      ((state & DIGIN_KEYB_MASK) >> (ffs(DIGIN_KEYB_MASK) - 1))
+		      | ((state & DIGIN_TOUCHSCR_MASK) >>
+		      (ffs(DIGIN_TOUCHSCR_MASK) - 2)));
+	} else {
+		printf("Invalid argument: %s\n", argv[1]);
+		return -1;
+	}
+	return 0;

Please use the existing command parser code here and below.

board/inka4x0/inkadiag.c line 172

Quoted 4 lines
board/inka4x0/inkadiag.c new file +4 -0
+DECLARE_GLOBAL_DATA_PTR;
+
+static int ser_init(volatile struct mpc5xxx_psc *psc, int baudrate)
+{

Maybe there should be some comments what these functions are doing?

board/inka4x0/inkadiag.c line 215

Quoted 7 lines
board/inka4x0/inkadiag.c new file +7 -0
+static void ser_putc(volatile struct mpc5xxx_psc *psc, const char c)
+{
+	/* Wait for last character to go. */
+	int i = 0;
+	while (!(psc->psc_status & PSC_SR_TXEMP) && (i++ < CONFIG_SYS_HZ))
+		udelay(10);
+	psc->psc_buffer_8 = c;

What exactly has CONFIG_SYS_HZ to do with that? I cannot understand this sort of magic.

board/inka4x0/inkadiag.c line 224

Quoted 6 lines
board/inka4x0/inkadiag.c new file +6 -0
+static int ser_getc(volatile struct mpc5xxx_psc *psc)
+{
+	/* Wait for a character to arrive. */
+	int i = 0;
+	while (!(psc->psc_status & PSC_SR_RXRDY) && (i++ < CONFIG_SYS_HZ))
+		udelay(10);

Ditto here.

board/inka4x0/inkadiag.c line 236

Quoted 11 lines
board/inka4x0/inkadiag.c new file +11 -0
+	return psc->psc_buffer_8;
+}
+
+#define SERIAL_PORT_BASE	(u_char *)0x80000000
+
+#define UART_RX		0	/* In:  Receive buffer (DLAB=0) */
+#define UART_TX		0	/* Out: Transmit buffer (DLAB=0) */
+#define UART_DLL	0	/* Out: Divisor Latch Low (DLAB=1) */
+
+#define UART_LCR	3	/* Out: Line Control Register */
+#define UART_MCR	4	/* Out: Modem Control Register */

...

Lots of #defines right in the middle of a C file? That's uygly. Please move these in a separate header file (or at least at the begin of the C file).

board/inka4x0/inkadiag.c line 265

Quoted 9 lines
board/inka4x0/inkadiag.c new file +9 -0
+static unsigned char serial_in(unsigned char num, int offset)
+{
+	return in_8(SERIAL_PORT_BASE + (num << 3) + offset);
+}
+
+static void serial_out(unsigned char num, int offset, unsigned char value)
+{
+	out_8(SERIAL_PORT_BASE + (num << 3) + offset, value);
+}

The amount of comments in this file sucessfully avoids all risks of fatiguing the reader...

Quoted 8 lines
board/inka4x0/inkadiag.c new file +8 -0
+	if (strcmp(argv[2], "115200") == 0)
+		combrd = 1;
+	else if (strcmp(argv[2], "57600") == 0)
+		combrd = 2;
+	else if (strcmp(argv[2], "38400") == 0)
+		combrd = 3;
+	else if (strcmp(argv[2], "19200") == 0)
+		combrd = 6;

board/inka4x0/inkadiag.c line 308

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+	else if (strcmp(argv[2], "9600") == 0)
+		combrd = 12;
+	else if (strcmp(argv[2], "2400") == 0)
+		combrd = 48;
+	else if (strcmp(argv[2], "1200") == 0)
+		combrd = 96;
+	else if (strcmp(argv[2], "300") == 0)
+		combrd = 384;
+	else
+		printf("Invalid baudrate: %s", argv[2]);

Instead of calling strcmp() again and again, you could convert the number just once and use a switch. Or table lookup. See existing code.

board/inka4x0/inkadiag.c line 316

Quoted 4 lines
board/inka4x0/inkadiag.c new file +4 -0
+		if (mode & 1)
+			/* turn on 'loopback' mode */
+			serial_out(num, UART_MCR, UART_MCR_LOOP);
+		else {

Multiline statements require braces.

board/inka4x0/inkadiag.c line 318

Quoted 2 lines
board/inka4x0/inkadiag.c new file +2 -0
+			/* establish the UART's operational parameters */
+			/* set DLAB=1 */

Multiline comment style is

			/*
			 * establish the UART's operational parameters
			 * set DLAB=1
			 */

board/inka4x0/inkadiag.c line 389

Quoted 2 lines
board/inka4x0/inkadiag.c new file +2 -0
+		volatile struct mpc5xxx_psc *psc =
+		    (struct mpc5xxx_psc *)address;

Indentation not by TAB.

board/inka4x0/inkadiag.c line 454

Quoted 4 lines
board/inka4x0/inkadiag.c new file +4 -0
+	if (freq > gd->ipb_clk / 128)
+		printf("%dHz exceeds maximum (%dHz)\n", freq,
+		       gd->ipb_clk / 128);
+	else if (!freq)

Multiline statements require braces.

board/inka4x0/inkadiag.c line 497

Quoted 5 lines
board/inka4x0/inkadiag.c new file +5 -0
+static int TEST_FUNCTION(inkadiag) (cmd_tbl_t *cmdtp, int flag, int argc,
+				    char *argv[]) {
+	switch (argc) {
+	case 1:
+		PRINT_TEST_USAGE(inkadiag);

"break;" missing?

Quoted 8 lines
board/inka4x0/inkadiag.c new file +8 -0
+	default:
+		argc--;
+		argv++;
+		CHECK_TEST(io);
+		CHECK_TEST(serial);
+		CHECK_TEST(buzzer);
+	}
+	PRINT_TEST_USAGE(inkadiag);

board/inka4x0/inkadiag.c line 515

Quoted 10 lines
board/inka4x0/inkadiag.c new file +10 -0
+}
+
+U_BOOT_CMD(inkadiag, 6, 1, TEST_FUNCTION(inkadiag),
+	   "inkadiag- inka diagnosis\n",
+	   "[inkadiag what ...]\n"
+	   "    - perform a diagnosis on inka hardware\n"
+	   "'inkadiag' performs hardware tests.\n\n"
+	   "Without arguments, inkadiag prints a list of available tests.\n\n"
+	   "To get detailed help information for specific tests you can type\n"
+	   "'inkadiag what' with a test name 'what' as argument.\n");

Please no prose here - be terse, please.

Viele Gr??e,

Wolfgang Denk

Signature · 5 lines
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
"Anyone attempting to generate random numbers by deterministic  means
is, of course, living in a state of sin."          - John Von Neumann
#13 lore raw

PATCH 1/1 Hardware diagnosis for inka4x0 boards.

Hi Wolfgang,

Quoted 5 lines
Quoted 1 line

This patch adds diagnosis functions for the buzzer, UARTs and digital

Such hardware test code should be implemented as part of the POST system, i. e. please create a post/board/inka4x0/ directory for it, and make it part of the POST.

But these tests cannot be part of any Power On Self Test as some of them are interactive. They were designed with the serial port being scripted - thus yielding an automated setup as a whole, but still such scripting is needed.

So I still see these tests to belong into the board directory.

Cheers
Detlev

Signature · 7 lines
But solicitors and stockbrokers and bookmakers often lead comfortable and
happy lives,  and it is very difficult to see how the world is richer for
their existence.                                  -- G. H. Hardy
--
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