All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] Impedance matcher for Samsung boards
@ 2013-12-11 13:07 Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 1/8] registers: add write u8 type Piotr Wilczek
                   ` (9 more replies)
  0 siblings, 10 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset implements the impedance-matcher for Samsung Exynos boards
and does the following:
 - fix strncmp function;
 - ensure each DT blob is 4-byte aligned;
 - port 'atags_to_fdt' form Kernel;
 - add Exynos specific serial driver;
 - add support for Samsung boards, Trats and Trats2;

This patch set is based on:
https://github.com/zonque/pxa-impedance-matcher.git

Piotr Wilczek (8):
  registers: add write u8 type
  string: fix strncmp function
  device tree blob must be 4-bytes alingned
  Makefile: disable generating Thumb code
  add memory operations
  add support to supplement atags to device tree
  serial: add Exynos specific serial driver
  board: add Samsung specific board support

 Makefile                       |   12 +-
 append_dtbs.sh                 |   11 +
 atags.h                        |    2 +
 atags_to_fdt.c                 |  197 ++++++
 board-exynos.c                 |   77 +++
 configs/Makefile.config.exynos |   13 +
 dtbs.c                         |    4 +
 fdt.c                          |  224 ++++++
 fdt_ro.c                       |  576 ++++++++++++++++
 fdt_rw.c                       |  494 ++++++++++++++
 fdt_wip.c                      |  120 ++++
 libfdt.h                       | 1478 ++++++++++++++++++++++++++++++++++++++++
 libfdt_env.h                   |   13 +
 libfdt_internal.h              |   95 +++
 main.c                         |    4 +
 register.c                     |   10 +
 register.h                     |    3 +
 serial-exynos.c                |   84 +++
 setup.h                        |  187 +++++
 string.c                       |  119 +++-
 string.h                       |   10 +
 types.h                        |   32 +
 22 files changed, 3761 insertions(+), 4 deletions(-)
 create mode 100644 atags_to_fdt.c
 create mode 100644 board-exynos.c
 create mode 100644 configs/Makefile.config.exynos
 create mode 100644 fdt.c
 create mode 100644 fdt_ro.c
 create mode 100644 fdt_rw.c
 create mode 100644 fdt_wip.c
 create mode 100644 libfdt.h
 create mode 100644 libfdt_env.h
 create mode 100644 libfdt_internal.h
 create mode 100644 serial-exynos.c
 create mode 100644 setup.h

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 1/8] registers: add write u8 type
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
@ 2013-12-11 13:07 ` Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 2/8] string: fix strncmp function Piotr Wilczek
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 register.c |   10 ++++++++++
 register.h |    3 +++
 types.h    |    1 +
 3 files changed, 14 insertions(+)

diff --git a/register.c b/register.c
index dfbe145..af992ec 100644
--- a/register.c
+++ b/register.c
@@ -1,5 +1,15 @@
 #include "types.h"
 
+inline void writeb(u8 val, u32 addr)
+{
+	*(volatile u32 *)addr = val;
+}
+
+inline u8 readb(u32 addr)
+{
+	return *(volatile u32 *)addr;
+}
+
 inline void writel(u32 val, u32 addr)
 {
 	*(volatile u32 *) addr = val;
diff --git a/register.h b/register.h
index b7831eb..5a89802 100644
--- a/register.h
+++ b/register.h
@@ -3,6 +3,9 @@
 
 #include "types.h"
 
+void writeb(u8, u32);
+u8 readb(u32);
+
 void writel(u32, u32);
 u32 readl(u32);
 
diff --git a/types.h b/types.h
index 1e6c56a..636f95b 100644
--- a/types.h
+++ b/types.h
@@ -1,6 +1,7 @@
 #ifndef _TYPES_H
 #define _TYPES_H
 
+typedef unsigned char u8;
 typedef unsigned int u32;
 typedef unsigned long long u64;
 #define NULL	((void *) 0)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 2/8] string: fix strncmp function
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 1/8] registers: add write u8 type Piotr Wilczek
@ 2013-12-11 13:07 ` Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 3/8] device tree blob must be 4-bytes alingned Piotr Wilczek
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

Fix function to return the number of characters that differ in
the compared strings.

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 string.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/string.c b/string.c
index d13e22b..ced25c8 100644
--- a/string.c
+++ b/string.c
@@ -24,7 +24,7 @@ int strncmp(const char *stra, const char *strb, int len)
 	const char *b = strb;
 
 	while ((a - stra) < len)
-		diff += *(a++) - *(b++);
+		diff += !!(*(a++) - *(b++));
 
 	return diff;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 3/8] device tree blob must be 4-bytes alingned
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 1/8] registers: add write u8 type Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 2/8] string: fix strncmp function Piotr Wilczek
@ 2013-12-11 13:07 ` Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 4/8] Makefile: disable generating Thumb code Piotr Wilczek
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

Each appended device tree blob must be 4-bytes aligned.
This patch modifies append_dtbs script to append bytes to each device tree
file so the size is multiple of 4.
When finding dtb the addres of next dtb is also aligned to 4-bytes.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 append_dtbs.sh |   11 +++++++++++
 dtbs.c         |    4 ++++
 2 files changed, 15 insertions(+)

diff --git a/append_dtbs.sh b/append_dtbs.sh
index 82bb463..a8a7cda 100755
--- a/append_dtbs.sh
+++ b/append_dtbs.sh
@@ -4,5 +4,16 @@ OUT="$1"
 shift
 DTBS="$*"
 
+while [ -n "$*" ]
+do
+	SIZE=`stat $1 -c %s`
+	MOD=`expr $SIZE % 4`
+	if [ $MOD -ne 0 ]; then
+		PADDING=`expr 4 - $MOD`
+		dd if=/dev/zero count=$PADDING bs=1 >> $1 2> /dev/null
+	fi
+	shift
+done
+
 cat $DTBS >>$OUT
 dd if=/dev/zero of=$OUT oflag=append conv=notrunc bs=1 count=8 #sentinel
diff --git a/dtbs.c b/dtbs.c
index 812c2ef..26e3b83 100644
--- a/dtbs.c
+++ b/dtbs.c
@@ -30,6 +30,10 @@ void *find_dtb(void *dtbs, const char *compat)
 			return d;
 
 		d = (struct fdt_header *)((char *)d + be_to_cpu(d->totalsize));
+
+		/* align to 4-bytes */
+		d = (struct fdt_header *)((((unsigned int)d + 0x3) & ~0x3));
+
 	}
 
 	return NULL;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 4/8] Makefile: disable generating Thumb code
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                   ` (2 preceding siblings ...)
  2013-12-11 13:07 ` [PATCH 3/8] device tree blob must be 4-bytes alingned Piotr Wilczek
@ 2013-12-11 13:07 ` Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 5/8] add memory operations Piotr Wilczek
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 29575cc..bddec8b 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-CFLAGS+=-Wall -ffreestanding
+CFLAGS+=-Wall -ffreestanding -marm
 LDFLAGS=-static -nostdlib
 GCC=$(CROSS_COMPILE)gcc
 OBJCOPY=$(CROSS_COMPILE)objcopy
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 5/8] add memory operations
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                   ` (3 preceding siblings ...)
  2013-12-11 13:07 ` [PATCH 4/8] Makefile: disable generating Thumb code Piotr Wilczek
@ 2013-12-11 13:07 ` Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 7/8] serial: add Exynos specific serial driver Piotr Wilczek
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

This code is from Kernel 3.10.

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 string.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 string.h |   10 ++++++
 types.h  |    3 ++
 3 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/string.c b/string.c
index ced25c8..ec66b27 100644
--- a/string.c
+++ b/string.c
@@ -1,4 +1,3 @@
-#include "types.h"
 #include "string.h"
 
 int hexlut[1 + 'F' - '0'] = {
@@ -97,3 +96,119 @@ int getaddrs(void **k, void **d, const char *str)
 
 	return 0;
 }
+
+/**
+ * memchr - Find a character in an area of memory.
+ * @s: The memory area
+ * @c: The byte to search for
+ * @n: The size of the area.
+ *
+ * returns the address of the first occurrence of @c, or %NULL
+ * if @c is not found
+ */
+void *memchr(const void *s, int c, size_t n)
+{
+	const unsigned char *p = s;
+	while (n-- != 0) {
+		if ((unsigned char)c == *p++) {
+			return (void *)(p - 1);
+		}
+	}
+	return NULL;
+}
+
+/**
+ * strchr - Find the first occurrence of a character in a string
+ * @s: The string to be searched
+ * @c: The character to search for
+ */
+char *strchr(const char *s, int c)
+{
+	for (; *s != (char)c; ++s)
+		if (*s == '\0')
+			return NULL;
+	return (char *)s;
+}
+
+/**
+ * memset - Fill a region of memory with the given value
+ * @s: Pointer to the start of the area.
+ * @c: The byte to fill the area with
+ * @count: The size of the area.
+ *
+ * Do not use memset() to access IO space, use memset_io() instead.
+ */
+void *memset(void *s, int c, size_t count)
+{
+	char *xs = s;
+
+	while (count--)
+		*xs++ = c;
+	return s;
+}
+
+/**
+ * memcpy - Copy one area of memory to another
+ * @dest: Where to copy to
+ * @src: Where to copy from
+ * @count: The size of the area.
+ *
+ * You should not use this function to access IO space, use memcpy_toio()
+ * or memcpy_fromio() instead.
+ */
+void *memcpy(void *dest, const void *src, size_t count)
+{
+	char *tmp = dest;
+	const char *s = src;
+
+	while (count--)
+		*tmp++ = *s++;
+	return dest;
+}
+
+/**
+ * memmove - Copy one area of memory to another
+ * @dest: Where to copy to
+ * @src: Where to copy from
+ * @count: The size of the area.
+ *
+ * Unlike memcpy(), memmove() copes with overlapping areas.
+ */
+void *memmove(void *dest, const void *src, size_t count)
+{
+	char *tmp;
+	const char *s;
+
+	if (dest <= src) {
+		tmp = dest;
+		s = src;
+		while (count--)
+			*tmp++ = *s++;
+	} else {
+		tmp = dest;
+		tmp += count;
+		s = src;
+		s += count;
+		while (count--)
+			*--tmp = *--s;
+	}
+	return dest;
+}
+
+/**
+ * memcmp - Compare two areas of memory
+ * @cs: One area of memory
+ * @ct: Another area of memory
+ * @count: The size of the area.
+ */
+#undef memcmp
+int memcmp(const void *cs, const void *ct, size_t count)
+{
+	const unsigned char *su1, *su2;
+	int res = 0;
+
+	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
+		if ((res = *su1 - *su2) != 0)
+			break;
+	return res;
+}
diff --git a/string.h b/string.h
index 0b99785..41ddd3a 100644
--- a/string.h
+++ b/string.h
@@ -1,9 +1,19 @@
 #ifndef _STRING_H
 #define _STRING_H
 
+#include "types.h"
+
 int strlen(const char *);
 int strncmp(const char *, const char *, int);
 void *gethexaddr(const char *, const char **);
 int getaddrs(void **, void **, const char *);
 
+void *memchr(const void *s, int c, size_t n);
+char *strchr(const char *s, int c);
+
+void *memset(void *s, int c, size_t count);
+void *memcpy(void *dest, const void *src, size_t count);
+void *memmove(void *dest, const void *src, size_t count);
+int memcmp(const void *cs, const void *ct, size_t count);
+
 #endif
diff --git a/types.h b/types.h
index 636f95b..b8b4a95 100644
--- a/types.h
+++ b/types.h
@@ -4,6 +4,9 @@
 typedef unsigned char u8;
 typedef unsigned int u32;
 typedef unsigned long long u64;
+
+typedef unsigned long size_t;
+
 #define NULL	((void *) 0)
 
 #endif /* _TYPES_H */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 7/8] serial: add Exynos specific serial driver
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                   ` (4 preceding siblings ...)
  2013-12-11 13:07 ` [PATCH 5/8] add memory operations Piotr Wilczek
@ 2013-12-11 13:07 ` Piotr Wilczek
  2013-12-11 13:07 ` [PATCH 8/8] board: add Samsung specific board support Piotr Wilczek
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 Makefile        |    1 +
 serial-exynos.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 serial-exynos.c

diff --git a/Makefile b/Makefile
index 6074106..78684b7 100644
--- a/Makefile
+++ b/Makefile
@@ -8,6 +8,7 @@ include Makefile.config
 
 CFLAGS+=-DUART_BASE=$(UART_BASE)
 CFLAGS+=-DNR_BANKS=$(NR_BANKS)
+CFLAGS+=-DUART_PORT=$(UART_PORT)
 
 BOARD_OBJ = board-$(MFG).o
 UART_OBJ = serial-$(UART).o
diff --git a/serial-exynos.c b/serial-exynos.c
new file mode 100644
index 0000000..3ba4fc9
--- /dev/null
+++ b/serial-exynos.c
@@ -0,0 +1,84 @@
+#include "types.h"
+#include "register.h"
+#include "serial.h"
+
+#define EXYNOS_BASE_UART	0x13800000
+#define TX_FIFO_FULL_MASK	(1 << 24)
+
+#define UART_BAUDRATE_115200	115200
+
+union br_rest {
+	unsigned short	slot;		/* udivslot */
+	unsigned char	value;		/* ufracval */
+};
+
+struct s5p_uart {
+	unsigned int	ulcon;
+	unsigned int	ucon;
+	unsigned int	ufcon;
+	unsigned int	umcon;
+	unsigned int	utrstat;
+	unsigned int	uerstat;
+	unsigned int	ufstat;
+	unsigned int	umstat;
+	unsigned char	utxh;
+	unsigned char	res1[3];
+	unsigned char	urxh;
+	unsigned char	res2[3];
+	unsigned int	ubrdiv;
+	union br_rest	rest;
+	unsigned char	res3[0xffd0];
+};
+
+static inline struct s5p_uart *s5p_get_base_uart(int dev_index)
+{
+	u32 offset = dev_index * sizeof(struct s5p_uart);
+	return (struct s5p_uart *)(EXYNOS_BASE_UART + offset);
+}
+
+static inline int s5p_uart_divslot(void)
+{
+	return 0;
+}
+
+static int serial_err_check(const int dev_index, int op)
+{
+	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	unsigned int mask;
+
+	/*
+	 * UERSTAT
+	 * Break Detect	[3]
+	 * Frame Err	[2] : receive operation
+	 * Parity Err	[1] : receive operation
+	 * Overrun Err	[0] : receive operation
+	 */
+	if (op)
+		mask = 0x8;
+	else
+		mask = 0xf;
+
+	return readl((unsigned int)&uart->uerstat) & mask;
+}
+
+/*
+ * Output a single byte to the serial port.
+ */
+void __putch(char c)
+{
+	const int dev_index = UART_PORT;
+
+	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+
+	/* wait for room in the tx FIFO */
+	while ((readl((unsigned int)&uart->ufstat) & TX_FIFO_FULL_MASK)) {
+		if (serial_err_check(dev_index, 1))
+			return;
+	}
+
+	writeb(c, (unsigned int)&uart->utxh);
+
+	/* If \n, also do \r */
+	if (c == '\n')
+		__putch('\r');
+}
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 8/8] board: add Samsung specific board support
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                   ` (5 preceding siblings ...)
  2013-12-11 13:07 ` [PATCH 7/8] serial: add Exynos specific serial driver Piotr Wilczek
@ 2013-12-11 13:07 ` Piotr Wilczek
  2013-12-11 13:16 ` [PATCH 0/8] Impedance matcher for Samsung boards Daniel Mack
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 13:07 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 board-exynos.c                 |   77 ++++++++++++++++++++++++++++++++++++++++
 configs/Makefile.config.exynos |   13 +++++++
 2 files changed, 90 insertions(+)
 create mode 100644 board-exynos.c
 create mode 100644 configs/Makefile.config.exynos

diff --git a/board-exynos.c b/board-exynos.c
new file mode 100644
index 0000000..2769a5f
--- /dev/null
+++ b/board-exynos.c
@@ -0,0 +1,77 @@
+#include "atags.h"
+#include "board.h"
+#include "dtbs.h"
+#include "print.h"
+#include "register.h"
+#include "types.h"
+
+extern u32 _binary_input_zImage_start;
+extern u32 _binary_dtbs_bin_start;
+
+struct board board;
+
+#define EXYNOS_PRO_ID		0x10000000
+#define EXYNOS_PRO_ID_SHIFT	12
+
+struct exynos_board {
+	u32		proid;
+	u32		system_rev;
+	const char	*compatible;
+};
+
+static struct exynos_board exboards[] = {
+	/* Controller */
+	{
+		.proid		= 0xe4412,
+		.system_rev	= 0,
+		.compatible	= "samsung,trats2",
+	},
+	{
+		.proid		= 0x43210,
+		.system_rev	= 0,
+		.compatible	= "samsung,trats",
+	},
+	{ 0, 0, NULL }	/* sentinel */
+};
+
+static void panic(void)
+{
+	while (1)
+		;
+}
+
+static u32 get_proid()
+{
+	return readl(EXYNOS_PRO_ID) >> EXYNOS_PRO_ID_SHIFT;
+}
+
+struct board *match_board(u32 machid, const struct tag *tags)
+{
+	struct exynos_board *exboard;
+	u32 proid;
+
+	proid = get_proid();
+
+	for (exboard = exboards; exboard->proid; exboard++) {
+		if (exboard->proid == proid)
+			break;
+	}
+
+	if (exboard->compatible == NULL) {
+		putstr("ERROR MATCHING BOARD!\n");
+		panic(); /* doesn't return */
+	}
+
+	board.kernel = &_binary_input_zImage_start;
+	board.compatible = exboard->compatible;
+	board.dtb = find_dtb(&_binary_dtbs_bin_start, exboard->compatible);
+
+	if (board.dtb == NULL) {
+		putstr("NO DTB BLOB FOUND FOR ");
+		putstr(exboard->compatible);
+		putstr("\n");
+		panic(); /* doesn't return */
+	}
+
+	return &board;
+}
diff --git a/configs/Makefile.config.exynos b/configs/Makefile.config.exynos
new file mode 100644
index 0000000..af5ae53
--- /dev/null
+++ b/configs/Makefile.config.exynos
@@ -0,0 +1,13 @@
+# config for Exynos boards
+
+CROSS_COMPILE=arm-linux-gnueabi-
+MFG=exynos
+UART=exynos
+UART_PORT=2
+BINFMT=elf32-littlearm
+LOADADDR=0x41008000
+UART_BASE=0x40100000
+APPEND_KERNEL=input/zImage
+APPEND_DTBS=input/exynos4210-trats.dtb input/exynos4412-trats2.dtb
+
+NR_BANKS=8
\ No newline at end of file
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH 0/8] Impedance matcher for Samsung boards
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                   ` (6 preceding siblings ...)
  2013-12-11 13:07 ` [PATCH 8/8] board: add Samsung specific board support Piotr Wilczek
@ 2013-12-11 13:16 ` Daniel Mack
  2013-12-11 14:03   ` Piotr Wilczek
       [not found] ` <1386767259-15693-7-git-send-email-p.wilczek@samsung.com>
  2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
  9 siblings, 1 reply; 35+ messages in thread
From: Daniel Mack @ 2013-12-11 13:16 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/11/2013 02:07 PM, Piotr Wilczek wrote:
> This patchset implements the impedance-matcher for Samsung Exynos boards
> and does the following:
>  - fix strncmp function;
>  - ensure each DT blob is 4-byte aligned;
>  - port 'atags_to_fdt' form Kernel;
>  - add Exynos specific serial driver;
>  - add support for Samsung boards, Trats and Trats2;
> 
> This patch set is based on:
> https://github.com/zonque/pxa-impedance-matcher.git

Thanks for the patches!

I'm considering merging most of them, but I have my problems with the
inclusion of the libfdt library.

Could you explain what you need this for, exactly? Why can't you just
fix the device tree blob that you include instead of modifying it at
runtime?


Daniel

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 0/8] Impedance matcher for Samsung boards
  2013-12-11 13:16 ` [PATCH 0/8] Impedance matcher for Samsung boards Daniel Mack
@ 2013-12-11 14:03   ` Piotr Wilczek
  2013-12-11 14:10     ` Daniel Mack
  0 siblings, 1 reply; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-11 14:03 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

> -----Original Message-----
> From: Daniel Mack [mailto:zonque at gmail.com]
> Sent: Wednesday, December 11, 2013 2:17 PM
> To: Piotr Wilczek; devicetree-discuss at lists.ozlabs.org; linux-arm-
> kernel at lists.infradead.org
> Cc: Jason Cooper; Nicolas Pitre
> Subject: Re: [PATCH 0/8] Impedance matcher for Samsung boards
> 
> On 12/11/2013 02:07 PM, Piotr Wilczek wrote:
> > This patchset implements the impedance-matcher for Samsung Exynos
> > boards and does the following:
> >  - fix strncmp function;
> >  - ensure each DT blob is 4-byte aligned;
> >  - port 'atags_to_fdt' form Kernel;
> >  - add Exynos specific serial driver;
> >  - add support for Samsung boards, Trats and Trats2;
> >
> > This patch set is based on:
> > https://github.com/zonque/pxa-impedance-matcher.git
> 
> Thanks for the patches!
> 
> I'm considering merging most of them, but I have my problems with the
> inclusion of the libfdt library.
> 
> Could you explain what you need this for, exactly? Why can't you just
> fix the device tree blob that you include instead of modifying it at
> runtime?
When bootloader needs to pass some information in ATAGS and CONFIG_ARM_APPENDED_DTB is turned off in kernel, then it has to be added to the device tree blob at runtime.

> 
> 
> Daniel

Best regards,
Piotr Wilczek

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 0/8] Impedance matcher for Samsung boards
  2013-12-11 14:03   ` Piotr Wilczek
@ 2013-12-11 14:10     ` Daniel Mack
  2013-12-11 14:28       ` Tomasz Figa
  0 siblings, 1 reply; 35+ messages in thread
From: Daniel Mack @ 2013-12-11 14:10 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/11/2013 03:03 PM, Piotr Wilczek wrote:
> Hi,
> 
>> -----Original Message----- From: Daniel Mack
>> [mailto:zonque at gmail.com] Sent: Wednesday, December 11, 2013 2:17
>> PM To: Piotr Wilczek; devicetree-discuss at lists.ozlabs.org;
>> linux-arm- kernel at lists.infradead.org Cc: Jason Cooper; Nicolas
>> Pitre Subject: Re: [PATCH 0/8] Impedance matcher for Samsung
>> boards
>> 
>> On 12/11/2013 02:07 PM, Piotr Wilczek wrote:
>>> This patchset implements the impedance-matcher for Samsung
>>> Exynos boards and does the following: - fix strncmp function; -
>>> ensure each DT blob is 4-byte aligned; - port 'atags_to_fdt' form
>>> Kernel; - add Exynos specific serial driver; - add support for
>>> Samsung boards, Trats and Trats2;
>>> 
>>> This patch set is based on: 
>>> https://github.com/zonque/pxa-impedance-matcher.git
>> 
>> Thanks for the patches!
>> 
>> I'm considering merging most of them, but I have my problems with
>> the inclusion of the libfdt library.
>> 
>> Could you explain what you need this for, exactly? Why can't you
>> just fix the device tree blob that you include instead of modifying
>> it at runtime?
> When bootloader needs to pass some information in ATAGS and
> CONFIG_ARM_APPENDED_DTB is turned off in kernel, then it has to be
> added to the device tree blob at runtime.

The question is: can't you just provide a dtb for each of your machines,
and distinguish them via their machine ID? Or do you really have
machines that come up with the same machine ID but differ in details
only stored in ATAGs?

After all, adding libfdt makes the code grow by more than factor 4.



Daniel

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 0/8] Impedance matcher for Samsung boards
  2013-12-11 14:10     ` Daniel Mack
@ 2013-12-11 14:28       ` Tomasz Figa
  2013-12-11 15:19         ` Daniel Mack
  0 siblings, 1 reply; 35+ messages in thread
From: Tomasz Figa @ 2013-12-11 14:28 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Daniel,

On Wednesday 11 of December 2013 15:10:39 Daniel Mack wrote:
> On 12/11/2013 03:03 PM, Piotr Wilczek wrote:
> > Hi,
> > 
> >> -----Original Message----- From: Daniel Mack
> >> [mailto:zonque at gmail.com] Sent: Wednesday, December 11, 2013 2:17
> >> PM To: Piotr Wilczek; devicetree-discuss at lists.ozlabs.org;
> >> linux-arm- kernel at lists.infradead.org Cc: Jason Cooper; Nicolas
> >> Pitre Subject: Re: [PATCH 0/8] Impedance matcher for Samsung
> >> boards
> >> 
> >> On 12/11/2013 02:07 PM, Piotr Wilczek wrote:
> >>> This patchset implements the impedance-matcher for Samsung
> >>> Exynos boards and does the following: - fix strncmp function; -
> >>> ensure each DT blob is 4-byte aligned; - port 'atags_to_fdt' form
> >>> Kernel; - add Exynos specific serial driver; - add support for
> >>> Samsung boards, Trats and Trats2;
> >>> 
> >>> This patch set is based on: 
> >>> https://github.com/zonque/pxa-impedance-matcher.git
> >> 
> >> Thanks for the patches!
> >> 
> >> I'm considering merging most of them, but I have my problems with
> >> the inclusion of the libfdt library.
> >> 
> >> Could you explain what you need this for, exactly? Why can't you
> >> just fix the device tree blob that you include instead of modifying
> >> it at runtime?
> > When bootloader needs to pass some information in ATAGS and
> > CONFIG_ARM_APPENDED_DTB is turned off in kernel, then it has to be
> > added to the device tree blob at runtime.
> 
> The question is: can't you just provide a dtb for each of your machines,
> and distinguish them via their machine ID? Or do you really have
> machines that come up with the same machine ID but differ in details
> only stored in ATAGs?
> 
> After all, adding libfdt makes the code grow by more than factor 4.

There are some parameters that bootloaders normally pass with ATAGS
and can't be hardcoded in DT, such as initrd location in memory, serial
number, board revision, command line and in some cases even memory,
as on some boards it might vary between units.

Since impedance matches can't forward the ATAGs to the kernel, when the
kernel doesn't boot in appended DTB mode, there is a need to inject these
data into DT in the matches itself, based on ATAGs it gets from the
firmware.

Best regards,
Tomasz

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 0/8] Impedance matcher for Samsung boards
  2013-12-11 14:28       ` Tomasz Figa
@ 2013-12-11 15:19         ` Daniel Mack
  2013-12-11 15:48           ` Nicolas Pitre
  2013-12-11 19:06           ` Jason Cooper
  0 siblings, 2 replies; 35+ messages in thread
From: Daniel Mack @ 2013-12-11 15:19 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/11/2013 03:28 PM, Tomasz Figa wrote:
> On Wednesday 11 of December 2013 15:10:39 Daniel Mack wrote:
>> On 12/11/2013 03:03 PM, Piotr Wilczek wrote:
>>> Hi,
>>>
>>>> -----Original Message----- From: Daniel Mack
>>>> [mailto:zonque at gmail.com] Sent: Wednesday, December 11, 2013 2:17
>>>> PM To: Piotr Wilczek; devicetree-discuss at lists.ozlabs.org;
>>>> linux-arm- kernel at lists.infradead.org Cc: Jason Cooper; Nicolas
>>>> Pitre Subject: Re: [PATCH 0/8] Impedance matcher for Samsung
>>>> boards
>>>>
>>>> On 12/11/2013 02:07 PM, Piotr Wilczek wrote:
>>>>> This patchset implements the impedance-matcher for Samsung
>>>>> Exynos boards and does the following: - fix strncmp function; -
>>>>> ensure each DT blob is 4-byte aligned; - port 'atags_to_fdt' form
>>>>> Kernel; - add Exynos specific serial driver; - add support for
>>>>> Samsung boards, Trats and Trats2;
>>>>>
>>>>> This patch set is based on: 
>>>>> https://github.com/zonque/pxa-impedance-matcher.git
>>>>
>>>> Thanks for the patches!
>>>>
>>>> I'm considering merging most of them, but I have my problems with
>>>> the inclusion of the libfdt library.
>>>>
>>>> Could you explain what you need this for, exactly? Why can't you
>>>> just fix the device tree blob that you include instead of modifying
>>>> it at runtime?
>>> When bootloader needs to pass some information in ATAGS and
>>> CONFIG_ARM_APPENDED_DTB is turned off in kernel, then it has to be
>>> added to the device tree blob at runtime.
>>
>> The question is: can't you just provide a dtb for each of your machines,
>> and distinguish them via their machine ID? Or do you really have
>> machines that come up with the same machine ID but differ in details
>> only stored in ATAGs?
>>
>> After all, adding libfdt makes the code grow by more than factor 4.
> 
> There are some parameters that bootloaders normally pass with ATAGS
> and can't be hardcoded in DT, such as initrd location in memory, serial
> number, board revision, command line and in some cases even memory,
> as on some boards it might vary between units.

Ok, that was my question :) Sure, if you have units that differ in
hardware details, but are indistinguishable by their machine ID or board
revision number, there's not much that we can do but modify the FDT at
runtime. Serial numbers are also something that can't be solved in a
nicer way.

What bugs me, though, is that it blows up the code base so massively.

So I'm about to merge this. Jason, and final thoughts?


Thanks,
Daniel

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 0/8] Impedance matcher for Samsung boards
  2013-12-11 15:19         ` Daniel Mack
@ 2013-12-11 15:48           ` Nicolas Pitre
  2013-12-11 19:06           ` Jason Cooper
  1 sibling, 0 replies; 35+ messages in thread
From: Nicolas Pitre @ 2013-12-11 15:48 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, 11 Dec 2013, Daniel Mack wrote:

> On 12/11/2013 03:28 PM, Tomasz Figa wrote:
> > There are some parameters that bootloaders normally pass with ATAGS
> > and can't be hardcoded in DT, such as initrd location in memory, serial
> > number, board revision, command line and in some cases even memory,
> > as on some boards it might vary between units.
> 
> Ok, that was my question :) Sure, if you have units that differ in
> hardware details, but are indistinguishable by their machine ID or board
> revision number, there's not much that we can do but modify the FDT at
> runtime. Serial numbers are also something that can't be solved in a
> nicer way.
> 
> What bugs me, though, is that it blows up the code base so massively.

You could investigate gcc's -ffunction-sections argument.  This won't 
help with the code size, but it does help for discarding unreferenced 
functions from the final lbinary.


Nicolas

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 6/8] add support to supplement atags to device tree
       [not found] ` <1386767259-15693-7-git-send-email-p.wilczek@samsung.com>
@ 2013-12-11 19:00   ` Jason Cooper
  2013-12-12 12:09     ` Piotr Wilczek
  0 siblings, 1 reply; 35+ messages in thread
From: Jason Cooper @ 2013-12-11 19:00 UTC (permalink / raw)
  To: linux-arm-kernel

Piotr,

First, thanks for the patch series!

On Wed, Dec 11, 2013 at 02:07:37PM +0100, Piotr Wilczek wrote:
> Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  Makefile          |    9 +-
>  atags.h           |    2 +
>  atags_to_fdt.c    |  197 +++++++
>  fdt.c             |  224 ++++++++
>  fdt_ro.c          |  576 +++++++++++++++++++++
>  fdt_rw.c          |  494 ++++++++++++++++++
>  fdt_wip.c         |  120 +++++
>  libfdt.h          | 1478 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  libfdt_env.h      |   13 +
>  libfdt_internal.h |   95 ++++
>  main.c            |    4 +
>  setup.h           |  187 +++++++
>  types.h           |   28 +
>  13 files changed, 3426 insertions(+), 1 deletion(-)
>  create mode 100644 atags_to_fdt.c
>  create mode 100644 fdt.c
>  create mode 100644 fdt_ro.c
>  create mode 100644 fdt_rw.c
>  create mode 100644 fdt_wip.c
>  create mode 100644 libfdt.h
>  create mode 100644 libfdt_env.h
>  create mode 100644 libfdt_internal.h
>  create mode 100644 setup.h

I was actually working on this, but you beat me to it :)

Could you copy the libfdt directory out of dtc and use it as is?
That'll make upgrading in the future easier. iow, place these files in
libfdt/.


thx,

Jason.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 0/8] Impedance matcher for Samsung boards
  2013-12-11 15:19         ` Daniel Mack
  2013-12-11 15:48           ` Nicolas Pitre
@ 2013-12-11 19:06           ` Jason Cooper
  1 sibling, 0 replies; 35+ messages in thread
From: Jason Cooper @ 2013-12-11 19:06 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 11, 2013 at 04:19:34PM +0100, Daniel Mack wrote:
> What bugs me, though, is that it blows up the code base so massively.

compile-time option?

> So I'm about to merge this. Jason, and final thoughts?

Just the location of the libfdt files.  But that's not a show stopper.

It would make the delineation cleaner and having a libfdt.a is easier to
make optional at compile-time if we want.

thx,

Jason.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 6/8] add support to supplement atags to device tree
  2013-12-11 19:00   ` [PATCH 6/8] add support to supplement atags to device tree Jason Cooper
@ 2013-12-12 12:09     ` Piotr Wilczek
  2013-12-12 12:36       ` Jason Cooper
  0 siblings, 1 reply; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-12 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

Jason,

> -----Original Message-----
> From: Jason Cooper [mailto:jason at lakedaemon.net]
> Sent: Wednesday, December 11, 2013 8:01 PM
> To: Piotr Wilczek
> Cc: devicetree-discuss at lists.ozlabs.org; linux-arm-
> kernel at lists.infradead.org; Daniel Mack; Nicolas Pitre; Kyungmin Park
> Subject: Re: [PATCH 6/8] add support to supplement atags to device tree
> 
> Piotr,
> 
> First, thanks for the patch series!
> 
> On Wed, Dec 11, 2013 at 02:07:37PM +0100, Piotr Wilczek wrote:
> > Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> >  Makefile          |    9 +-
> >  atags.h           |    2 +
> >  atags_to_fdt.c    |  197 +++++++
> >  fdt.c             |  224 ++++++++
> >  fdt_ro.c          |  576 +++++++++++++++++++++
> >  fdt_rw.c          |  494 ++++++++++++++++++
> >  fdt_wip.c         |  120 +++++
> >  libfdt.h          | 1478
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  libfdt_env.h      |   13 +
> >  libfdt_internal.h |   95 ++++
> >  main.c            |    4 +
> >  setup.h           |  187 +++++++
> >  types.h           |   28 +
> >  13 files changed, 3426 insertions(+), 1 deletion(-)  create mode
> > 100644 atags_to_fdt.c  create mode 100644 fdt.c  create mode 100644
> > fdt_ro.c  create mode 100644 fdt_rw.c  create mode 100644 fdt_wip.c
> > create mode 100644 libfdt.h  create mode 100644 libfdt_env.h  create
> > mode 100644 libfdt_internal.h  create mode 100644 setup.h
> 
> I was actually working on this, but you beat me to it :)
Sorry, I didn't know.

> 
> Could you copy the libfdt directory out of dtc and use it as is?
> That'll make upgrading in the future easier. iow, place these files in
> libfdt/.
> 
I will move it to the libfdt directory.

> 
> thx,
> 
> Jason.

Best regards,
Piotr

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH 6/8] add support to supplement atags to device tree
  2013-12-12 12:09     ` Piotr Wilczek
@ 2013-12-12 12:36       ` Jason Cooper
  0 siblings, 0 replies; 35+ messages in thread
From: Jason Cooper @ 2013-12-12 12:36 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Dec 12, 2013 at 01:09:59PM +0100, Piotr Wilczek wrote:
> Jason,
> 
> > -----Original Message-----
> > From: Jason Cooper [mailto:jason at lakedaemon.net]
> > Sent: Wednesday, December 11, 2013 8:01 PM
> > To: Piotr Wilczek
> > Cc: devicetree-discuss at lists.ozlabs.org; linux-arm-
> > kernel at lists.infradead.org; Daniel Mack; Nicolas Pitre; Kyungmin Park
> > Subject: Re: [PATCH 6/8] add support to supplement atags to device tree
> > 
> > Piotr,
> > 
> > First, thanks for the patch series!
> > 
> > On Wed, Dec 11, 2013 at 02:07:37PM +0100, Piotr Wilczek wrote:
> > > Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > ---
> > >  Makefile          |    9 +-
> > >  atags.h           |    2 +
> > >  atags_to_fdt.c    |  197 +++++++
> > >  fdt.c             |  224 ++++++++
> > >  fdt_ro.c          |  576 +++++++++++++++++++++
> > >  fdt_rw.c          |  494 ++++++++++++++++++
> > >  fdt_wip.c         |  120 +++++
> > >  libfdt.h          | 1478
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  libfdt_env.h      |   13 +
> > >  libfdt_internal.h |   95 ++++
> > >  main.c            |    4 +
> > >  setup.h           |  187 +++++++
> > >  types.h           |   28 +
> > >  13 files changed, 3426 insertions(+), 1 deletion(-)  create mode
> > > 100644 atags_to_fdt.c  create mode 100644 fdt.c  create mode 100644
> > > fdt_ro.c  create mode 100644 fdt_rw.c  create mode 100644 fdt_wip.c
> > > create mode 100644 libfdt.h  create mode 100644 libfdt_env.h  create
> > > mode 100644 libfdt_internal.h  create mode 100644 setup.h
> > 
> > I was actually working on this, but you beat me to it :)
> Sorry, I didn't know.

No need to apologize, I was just tinkering as I had free time.  And, I
didn't tell anybody, either.  ;-)

> > Could you copy the libfdt directory out of dtc and use it as is?
> > That'll make upgrading in the future easier. iow, place these files in
> > libfdt/.
> > 
> I will move it to the libfdt directory.

Great, thanks!

thx,

Jason.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 0/8] Impedance matcher for Samsung boards
  2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                   ` (8 preceding siblings ...)
       [not found] ` <1386767259-15693-7-git-send-email-p.wilczek@samsung.com>
@ 2013-12-18 12:09 ` Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 1/8] registers: add write u8 type Piotr Wilczek
                     ` (6 more replies)
  9 siblings, 7 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset implements the impedance-matcher for Samsung Exynos boards
and does the following:
 - fix strncmp function;
 - ensure each DT blob is 4-byte aligned;
 - port 'atags_to_fdt' form Kernel;
 - add Exynos specific serial driver;
 - add support for Samsung boards, Trats and Trats2;

This patch set is based on:
https://github.com/zonque/pxa-impedance-matcher.git

Changes for V2:
 - libfdt moved to libfdt directory

TODO:
Kernel uses some memory space right after uImage, overwriting the very first
appended device tree blob. As a workaround a fake, 32kB dtb is appended first.

Piotr Wilczek (8):
  registers: add write u8 type
  string: fix strncmp function
  device tree blob must be 4-bytes alingned
  Makefile: disable generating Thumb code
  add memory operations
  add support to supplement atags to device tree
  serial: add Exynos specific serial driver
  board: add Samsung specific board support

 Makefile                       |   16 +-
 append_dtbs.sh                 |   11 +
 atags.h                        |    2 +
 board-exynos.c                 |   77 ++
 configs/Makefile.config.exynos |   13 +
 dtbs.c                         |    4 +
 libfdt.h                       | 1478 ++++++++++++++++++++++++++++++++
 libfdt/Makefile                |   23 +
 libfdt/atags_to_fdt.c          |  199 +++++
 libfdt/fdt.c                   |  224 +++++
 libfdt/fdt_ro.c                |  576 +++++++++++++
 libfdt/fdt_rw.c                |  494 +++++++++++
 libfdt/fdt_wip.c               |  120 +++
 libfdt/libfdt.a                |  Bin 0 -> 32216 bytes
 libfdt/libfdt.h                | 1478 ++++++++++++++++++++++++++++++++
 libfdt/libfdt_env.h            |   13 +
 libfdt/libfdt_internal.h       |   95 +++
 libfdt/patch_libfdt.patch      | 1826 ++++++++++++++++++++++++++++++++++++++++
 libfdt/setup.h                 |  187 ++++
 main.c                         |    4 +
 register.c                     |   10 +
 register.h                     |    3 +
 serial-exynos.c                |   84 ++
 string.c                       |  119 ++-
 string.h                       |   10 +
 types.h                        |   32 +
 26 files changed, 7094 insertions(+), 4 deletions(-)
 create mode 100644 board-exynos.c
 create mode 100644 configs/Makefile.config.exynos
 create mode 100644 libfdt.h
 create mode 100644 libfdt/Makefile
 create mode 100644 libfdt/atags_to_fdt.c
 create mode 100644 libfdt/fdt.c
 create mode 100644 libfdt/fdt_ro.c
 create mode 100644 libfdt/fdt_rw.c
 create mode 100644 libfdt/fdt_wip.c
 create mode 100644 libfdt/libfdt.a
 create mode 100644 libfdt/libfdt.h
 create mode 100644 libfdt/libfdt_env.h
 create mode 100644 libfdt/libfdt_internal.h
 create mode 100644 libfdt/patch_libfdt.patch
 create mode 100644 libfdt/setup.h
 create mode 100644 serial-exynos.c

-- 
1.7.9.5

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 1/8] registers: add write u8 type
  2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
@ 2013-12-18 12:09   ` Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 2/8] string: fix strncmp function Piotr Wilczek
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes for V2:
 - none

 register.c |   10 ++++++++++
 register.h |    3 +++
 types.h    |    1 +
 3 files changed, 14 insertions(+)

diff --git a/register.c b/register.c
index dfbe145..af992ec 100644
--- a/register.c
+++ b/register.c
@@ -1,5 +1,15 @@
 #include "types.h"
 
+inline void writeb(u8 val, u32 addr)
+{
+	*(volatile u32 *)addr = val;
+}
+
+inline u8 readb(u32 addr)
+{
+	return *(volatile u32 *)addr;
+}
+
 inline void writel(u32 val, u32 addr)
 {
 	*(volatile u32 *) addr = val;
diff --git a/register.h b/register.h
index b7831eb..5a89802 100644
--- a/register.h
+++ b/register.h
@@ -3,6 +3,9 @@
 
 #include "types.h"
 
+void writeb(u8, u32);
+u8 readb(u32);
+
 void writel(u32, u32);
 u32 readl(u32);
 
diff --git a/types.h b/types.h
index 1e6c56a..636f95b 100644
--- a/types.h
+++ b/types.h
@@ -1,6 +1,7 @@
 #ifndef _TYPES_H
 #define _TYPES_H
 
+typedef unsigned char u8;
 typedef unsigned int u32;
 typedef unsigned long long u64;
 #define NULL	((void *) 0)
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 1/8] registers: add write u8 type Piotr Wilczek
@ 2013-12-18 12:09   ` Piotr Wilczek
  2013-12-18 12:15     ` Russell King - ARM Linux
  2013-12-18 12:09   ` [PATCH V2 3/8] device tree blob must be 4-bytes alingned Piotr Wilczek
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

Now function returns the number of characters that differ in the compared strings.

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes for V2:
 - none

 string.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/string.c b/string.c
index d13e22b..ced25c8 100644
--- a/string.c
+++ b/string.c
@@ -24,7 +24,7 @@ int strncmp(const char *stra, const char *strb, int len)
 	const char *b = strb;
 
 	while ((a - stra) < len)
-		diff += *(a++) - *(b++);
+		diff += !!(*(a++) - *(b++));
 
 	return diff;
 }
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH V2 3/8] device tree blob must be 4-bytes alingned
  2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 1/8] registers: add write u8 type Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 2/8] string: fix strncmp function Piotr Wilczek
@ 2013-12-18 12:09   ` Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 4/8] Makefile: disable generating Thumb code Piotr Wilczek
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

Each appended device tree blob must be 4-bytes aligned.
This patch modifies append_dtbs script to append bytes to each device tree
file so the size is multiple of 4.
When finding dtb the addres of next dtb is also aligned to 4-bytes.

Signed-off-by: Tomasz Figa <t.figa@samsung.com>
Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes for V2:
 - none

 append_dtbs.sh |   11 +++++++++++
 dtbs.c         |    4 ++++
 2 files changed, 15 insertions(+)

diff --git a/append_dtbs.sh b/append_dtbs.sh
index 82bb463..a8a7cda 100755
--- a/append_dtbs.sh
+++ b/append_dtbs.sh
@@ -4,5 +4,16 @@ OUT="$1"
 shift
 DTBS="$*"
 
+while [ -n "$*" ]
+do
+	SIZE=`stat $1 -c %s`
+	MOD=`expr $SIZE % 4`
+	if [ $MOD -ne 0 ]; then
+		PADDING=`expr 4 - $MOD`
+		dd if=/dev/zero count=$PADDING bs=1 >> $1 2> /dev/null
+	fi
+	shift
+done
+
 cat $DTBS >>$OUT
 dd if=/dev/zero of=$OUT oflag=append conv=notrunc bs=1 count=8 #sentinel
diff --git a/dtbs.c b/dtbs.c
index 812c2ef..26e3b83 100644
--- a/dtbs.c
+++ b/dtbs.c
@@ -30,6 +30,10 @@ void *find_dtb(void *dtbs, const char *compat)
 			return d;
 
 		d = (struct fdt_header *)((char *)d + be_to_cpu(d->totalsize));
+
+		/* align to 4-bytes */
+		d = (struct fdt_header *)((((unsigned int)d + 0x3) & ~0x3));
+
 	}
 
 	return NULL;
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH V2 4/8] Makefile: disable generating Thumb code
  2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                     ` (2 preceding siblings ...)
  2013-12-18 12:09   ` [PATCH V2 3/8] device tree blob must be 4-bytes alingned Piotr Wilczek
@ 2013-12-18 12:09   ` Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 5/8] add memory operations Piotr Wilczek
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes for V2:
 - none

 Makefile |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Makefile b/Makefile
index 29575cc..bddec8b 100644
--- a/Makefile
+++ b/Makefile
@@ -1,4 +1,4 @@
-CFLAGS+=-Wall -ffreestanding
+CFLAGS+=-Wall -ffreestanding -marm
 LDFLAGS=-static -nostdlib
 GCC=$(CROSS_COMPILE)gcc
 OBJCOPY=$(CROSS_COMPILE)objcopy
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH V2 5/8] add memory operations
  2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                     ` (3 preceding siblings ...)
  2013-12-18 12:09   ` [PATCH V2 4/8] Makefile: disable generating Thumb code Piotr Wilczek
@ 2013-12-18 12:09   ` Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 7/8] serial: add Exynos specific serial driver Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 8/8] board: add Samsung specific board support Piotr Wilczek
  6 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

This code is from Kernel 3.10.

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes for V2:
 - none

 string.c |  117 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 string.h |   10 ++++++
 types.h  |    3 ++
 3 files changed, 129 insertions(+), 1 deletion(-)

diff --git a/string.c b/string.c
index ced25c8..ec66b27 100644
--- a/string.c
+++ b/string.c
@@ -1,4 +1,3 @@
-#include "types.h"
 #include "string.h"
 
 int hexlut[1 + 'F' - '0'] = {
@@ -97,3 +96,119 @@ int getaddrs(void **k, void **d, const char *str)
 
 	return 0;
 }
+
+/**
+ * memchr - Find a character in an area of memory.
+ * @s: The memory area
+ * @c: The byte to search for
+ * @n: The size of the area.
+ *
+ * returns the address of the first occurrence of @c, or %NULL
+ * if @c is not found
+ */
+void *memchr(const void *s, int c, size_t n)
+{
+	const unsigned char *p = s;
+	while (n-- != 0) {
+		if ((unsigned char)c == *p++) {
+			return (void *)(p - 1);
+		}
+	}
+	return NULL;
+}
+
+/**
+ * strchr - Find the first occurrence of a character in a string
+ * @s: The string to be searched
+ * @c: The character to search for
+ */
+char *strchr(const char *s, int c)
+{
+	for (; *s != (char)c; ++s)
+		if (*s == '\0')
+			return NULL;
+	return (char *)s;
+}
+
+/**
+ * memset - Fill a region of memory with the given value
+ * @s: Pointer to the start of the area.
+ * @c: The byte to fill the area with
+ * @count: The size of the area.
+ *
+ * Do not use memset() to access IO space, use memset_io() instead.
+ */
+void *memset(void *s, int c, size_t count)
+{
+	char *xs = s;
+
+	while (count--)
+		*xs++ = c;
+	return s;
+}
+
+/**
+ * memcpy - Copy one area of memory to another
+ * @dest: Where to copy to
+ * @src: Where to copy from
+ * @count: The size of the area.
+ *
+ * You should not use this function to access IO space, use memcpy_toio()
+ * or memcpy_fromio() instead.
+ */
+void *memcpy(void *dest, const void *src, size_t count)
+{
+	char *tmp = dest;
+	const char *s = src;
+
+	while (count--)
+		*tmp++ = *s++;
+	return dest;
+}
+
+/**
+ * memmove - Copy one area of memory to another
+ * @dest: Where to copy to
+ * @src: Where to copy from
+ * @count: The size of the area.
+ *
+ * Unlike memcpy(), memmove() copes with overlapping areas.
+ */
+void *memmove(void *dest, const void *src, size_t count)
+{
+	char *tmp;
+	const char *s;
+
+	if (dest <= src) {
+		tmp = dest;
+		s = src;
+		while (count--)
+			*tmp++ = *s++;
+	} else {
+		tmp = dest;
+		tmp += count;
+		s = src;
+		s += count;
+		while (count--)
+			*--tmp = *--s;
+	}
+	return dest;
+}
+
+/**
+ * memcmp - Compare two areas of memory
+ * @cs: One area of memory
+ * @ct: Another area of memory
+ * @count: The size of the area.
+ */
+#undef memcmp
+int memcmp(const void *cs, const void *ct, size_t count)
+{
+	const unsigned char *su1, *su2;
+	int res = 0;
+
+	for (su1 = cs, su2 = ct; 0 < count; ++su1, ++su2, count--)
+		if ((res = *su1 - *su2) != 0)
+			break;
+	return res;
+}
diff --git a/string.h b/string.h
index 0b99785..41ddd3a 100644
--- a/string.h
+++ b/string.h
@@ -1,9 +1,19 @@
 #ifndef _STRING_H
 #define _STRING_H
 
+#include "types.h"
+
 int strlen(const char *);
 int strncmp(const char *, const char *, int);
 void *gethexaddr(const char *, const char **);
 int getaddrs(void **, void **, const char *);
 
+void *memchr(const void *s, int c, size_t n);
+char *strchr(const char *s, int c);
+
+void *memset(void *s, int c, size_t count);
+void *memcpy(void *dest, const void *src, size_t count);
+void *memmove(void *dest, const void *src, size_t count);
+int memcmp(const void *cs, const void *ct, size_t count);
+
 #endif
diff --git a/types.h b/types.h
index 636f95b..b8b4a95 100644
--- a/types.h
+++ b/types.h
@@ -4,6 +4,9 @@
 typedef unsigned char u8;
 typedef unsigned int u32;
 typedef unsigned long long u64;
+
+typedef unsigned long size_t;
+
 #define NULL	((void *) 0)
 
 #endif /* _TYPES_H */
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH V2 7/8] serial: add Exynos specific serial driver
  2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                     ` (4 preceding siblings ...)
  2013-12-18 12:09   ` [PATCH V2 5/8] add memory operations Piotr Wilczek
@ 2013-12-18 12:09   ` Piotr Wilczek
  2013-12-18 12:09   ` [PATCH V2 8/8] board: add Samsung specific board support Piotr Wilczek
  6 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes for V2:
 - none

 Makefile        |    1 +
 serial-exynos.c |   84 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 85 insertions(+)
 create mode 100644 serial-exynos.c

diff --git a/Makefile b/Makefile
index be2dbe1..6ae08a2 100644
--- a/Makefile
+++ b/Makefile
@@ -8,6 +8,7 @@ include Makefile.config
 
 CFLAGS+=-DUART_BASE=$(UART_BASE)
 CFLAGS+=-DNR_BANKS=$(NR_BANKS)
+CFLAGS+=-DUART_PORT=$(UART_PORT)
 
 BOARD_OBJ = board-$(MFG).o
 UART_OBJ = serial-$(UART).o
diff --git a/serial-exynos.c b/serial-exynos.c
new file mode 100644
index 0000000..3ba4fc9
--- /dev/null
+++ b/serial-exynos.c
@@ -0,0 +1,84 @@
+#include "types.h"
+#include "register.h"
+#include "serial.h"
+
+#define EXYNOS_BASE_UART	0x13800000
+#define TX_FIFO_FULL_MASK	(1 << 24)
+
+#define UART_BAUDRATE_115200	115200
+
+union br_rest {
+	unsigned short	slot;		/* udivslot */
+	unsigned char	value;		/* ufracval */
+};
+
+struct s5p_uart {
+	unsigned int	ulcon;
+	unsigned int	ucon;
+	unsigned int	ufcon;
+	unsigned int	umcon;
+	unsigned int	utrstat;
+	unsigned int	uerstat;
+	unsigned int	ufstat;
+	unsigned int	umstat;
+	unsigned char	utxh;
+	unsigned char	res1[3];
+	unsigned char	urxh;
+	unsigned char	res2[3];
+	unsigned int	ubrdiv;
+	union br_rest	rest;
+	unsigned char	res3[0xffd0];
+};
+
+static inline struct s5p_uart *s5p_get_base_uart(int dev_index)
+{
+	u32 offset = dev_index * sizeof(struct s5p_uart);
+	return (struct s5p_uart *)(EXYNOS_BASE_UART + offset);
+}
+
+static inline int s5p_uart_divslot(void)
+{
+	return 0;
+}
+
+static int serial_err_check(const int dev_index, int op)
+{
+	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+	unsigned int mask;
+
+	/*
+	 * UERSTAT
+	 * Break Detect	[3]
+	 * Frame Err	[2] : receive operation
+	 * Parity Err	[1] : receive operation
+	 * Overrun Err	[0] : receive operation
+	 */
+	if (op)
+		mask = 0x8;
+	else
+		mask = 0xf;
+
+	return readl((unsigned int)&uart->uerstat) & mask;
+}
+
+/*
+ * Output a single byte to the serial port.
+ */
+void __putch(char c)
+{
+	const int dev_index = UART_PORT;
+
+	struct s5p_uart *const uart = s5p_get_base_uart(dev_index);
+
+	/* wait for room in the tx FIFO */
+	while ((readl((unsigned int)&uart->ufstat) & TX_FIFO_FULL_MASK)) {
+		if (serial_err_check(dev_index, 1))
+			return;
+	}
+
+	writeb(c, (unsigned int)&uart->utxh);
+
+	/* If \n, also do \r */
+	if (c == '\n')
+		__putch('\r');
+}
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH V2 8/8] board: add Samsung specific board support
  2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
                     ` (5 preceding siblings ...)
  2013-12-18 12:09   ` [PATCH V2 7/8] serial: add Exynos specific serial driver Piotr Wilczek
@ 2013-12-18 12:09   ` Piotr Wilczek
  6 siblings, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:09 UTC (permalink / raw)
  To: linux-arm-kernel

Support for Samsung's Trats  and Trats2 boards is added.

TODO:
Kernel uses some memory space right after uImage, overwriting the very first
appended device tree blob. As a workaround, a fake 32kB dtb is appended first.

Signed-off-by: Piotr Wilczek <p.wilczek@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
Changes for V2:
 - none

 board-exynos.c                 |   77 ++++++++++++++++++++++++++++++++++++++++
 configs/Makefile.config.exynos |   13 +++++++
 2 files changed, 90 insertions(+)
 create mode 100644 board-exynos.c
 create mode 100644 configs/Makefile.config.exynos

diff --git a/board-exynos.c b/board-exynos.c
new file mode 100644
index 0000000..2769a5f
--- /dev/null
+++ b/board-exynos.c
@@ -0,0 +1,77 @@
+#include "atags.h"
+#include "board.h"
+#include "dtbs.h"
+#include "print.h"
+#include "register.h"
+#include "types.h"
+
+extern u32 _binary_input_zImage_start;
+extern u32 _binary_dtbs_bin_start;
+
+struct board board;
+
+#define EXYNOS_PRO_ID		0x10000000
+#define EXYNOS_PRO_ID_SHIFT	12
+
+struct exynos_board {
+	u32		proid;
+	u32		system_rev;
+	const char	*compatible;
+};
+
+static struct exynos_board exboards[] = {
+	/* Controller */
+	{
+		.proid		= 0xe4412,
+		.system_rev	= 0,
+		.compatible	= "samsung,trats2",
+	},
+	{
+		.proid		= 0x43210,
+		.system_rev	= 0,
+		.compatible	= "samsung,trats",
+	},
+	{ 0, 0, NULL }	/* sentinel */
+};
+
+static void panic(void)
+{
+	while (1)
+		;
+}
+
+static u32 get_proid()
+{
+	return readl(EXYNOS_PRO_ID) >> EXYNOS_PRO_ID_SHIFT;
+}
+
+struct board *match_board(u32 machid, const struct tag *tags)
+{
+	struct exynos_board *exboard;
+	u32 proid;
+
+	proid = get_proid();
+
+	for (exboard = exboards; exboard->proid; exboard++) {
+		if (exboard->proid == proid)
+			break;
+	}
+
+	if (exboard->compatible == NULL) {
+		putstr("ERROR MATCHING BOARD!\n");
+		panic(); /* doesn't return */
+	}
+
+	board.kernel = &_binary_input_zImage_start;
+	board.compatible = exboard->compatible;
+	board.dtb = find_dtb(&_binary_dtbs_bin_start, exboard->compatible);
+
+	if (board.dtb == NULL) {
+		putstr("NO DTB BLOB FOUND FOR ");
+		putstr(exboard->compatible);
+		putstr("\n");
+		panic(); /* doesn't return */
+	}
+
+	return &board;
+}
diff --git a/configs/Makefile.config.exynos b/configs/Makefile.config.exynos
new file mode 100644
index 0000000..7933338
--- /dev/null
+++ b/configs/Makefile.config.exynos
@@ -0,0 +1,13 @@
+# config for Exynos boards
+
+CROSS_COMPILE=arm-linux-gnueabi-
+MFG=exynos
+UART=exynos
+UART_PORT=2
+BINFMT=elf32-littlearm
+LOADADDR=0x41008000
+UART_BASE=0x40100000
+APPEND_KERNEL=input/zImage
+APPEND_DTBS=input/padd32k.bin input/exynos4210-trats.dtb input/exynos4412-trats2.dtb
+LIBFDT = y
+NR_BANKS=8
\ No newline at end of file
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 12:09   ` [PATCH V2 2/8] string: fix strncmp function Piotr Wilczek
@ 2013-12-18 12:15     ` Russell King - ARM Linux
  2013-12-18 12:23       ` Daniel Mack
  2013-12-18 12:50       ` Piotr Wilczek
  0 siblings, 2 replies; 35+ messages in thread
From: Russell King - ARM Linux @ 2013-12-18 12:15 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 18, 2013 at 01:09:05PM +0100, Piotr Wilczek wrote:
> Now function returns the number of characters that differ in the
> compared strings.

I think this is a mistake.  You're defining a standard C function in
a non-standard way.  For strcmp() and strncmp(), the C standard
requires:

RETURN VALUE
       The strcmp() and strncmp() functions return an integer less than, equal
       to, or greater than zero if s1 (or the first n bytes thereof) is found,
       respectively, to be less than, to match, or be greater than s2.

If you implement something different to that, don't call it strcmp() or
strncmp() but something else, because you're not providing the correct
functionality, and that will just confuse people and lead to bugs.

For instance, can you be sure that there aren't any uses of strncmp()
which already test for less-than-zero etc?

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 12:15     ` Russell King - ARM Linux
@ 2013-12-18 12:23       ` Daniel Mack
  2013-12-18 12:52         ` Russell King - ARM Linux
  2013-12-18 16:03         ` Jason Cooper
  2013-12-18 12:50       ` Piotr Wilczek
  1 sibling, 2 replies; 35+ messages in thread
From: Daniel Mack @ 2013-12-18 12:23 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/18/2013 01:15 PM, Russell King - ARM Linux wrote:
> On Wed, Dec 18, 2013 at 01:09:05PM +0100, Piotr Wilczek wrote:
>> Now function returns the number of characters that differ in the
>> compared strings.
> 
> I think this is a mistake.  You're defining a standard C function in
> a non-standard way.  For strcmp() and strncmp(), the C standard
> requires:
> 
> RETURN VALUE
>        The strcmp() and strncmp() functions return an integer less than, equal
>        to, or greater than zero if s1 (or the first n bytes thereof) is found,
>        respectively, to be less than, to match, or be greater than s2.
> 
> If you implement something different to that, don't call it strcmp() or
> strncmp() but something else, because you're not providing the correct
> functionality, and that will just confuse people and lead to bugs.

True.

> For instance, can you be sure that there aren't any uses of strncmp()
> which already test for less-than-zero etc?

In this case, yes. The code as it stands is very small, and users only
check for the return value of these functions to be 0.

But I wonder whether that patch is needed at all. At least the rest of
this series does not even seem to introduce new call sites of strcmp()
or strncmp() ...


Daniel

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 12:15     ` Russell King - ARM Linux
  2013-12-18 12:23       ` Daniel Mack
@ 2013-12-18 12:50       ` Piotr Wilczek
  1 sibling, 0 replies; 35+ messages in thread
From: Piotr Wilczek @ 2013-12-18 12:50 UTC (permalink / raw)
  To: linux-arm-kernel

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]
> Sent: Wednesday, December 18, 2013 1:16 PM
> To: Piotr Wilczek
> Cc: devicetree-discuss at lists.ozlabs.org; linux-arm-
> kernel at lists.infradead.org; Kyungmin Park; Nicolas Pitre; Jason Cooper;
> Daniel Mack
> Subject: Re: [PATCH V2 2/8] string: fix strncmp function
> 
> On Wed, Dec 18, 2013 at 01:09:05PM +0100, Piotr Wilczek wrote:
> > Now function returns the number of characters that differ in the
> > compared strings.
> 
> I think this is a mistake.  You're defining a standard C function in a
> non-standard way.  For strcmp() and strncmp(), the C standard
> requires:
> 
> RETURN VALUE
>        The strcmp() and strncmp() functions return an integer less
> than, equal
>        to, or greater than zero if s1 (or the first n bytes thereof) is
> found,
>        respectively, to be less than, to match, or be greater than s2.
> 
> If you implement something different to that, don't call it strcmp() or
> strncmp() but something else, because you're not providing the correct
> functionality, and that will just confuse people and lead to bugs.
Right, I will copy it from kernel.

> 
> For instance, can you be sure that there aren't any uses of strncmp()
> which already test for less-than-zero etc?
There aren't, but can be in future.

Thank you.
Piotr

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 12:23       ` Daniel Mack
@ 2013-12-18 12:52         ` Russell King - ARM Linux
  2013-12-18 16:01           ` Jason Cooper
  2013-12-18 16:03         ` Jason Cooper
  1 sibling, 1 reply; 35+ messages in thread
From: Russell King - ARM Linux @ 2013-12-18 12:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 18, 2013 at 01:23:26PM +0100, Daniel Mack wrote:
> On 12/18/2013 01:15 PM, Russell King - ARM Linux wrote:
> > For instance, can you be sure that there aren't any uses of strncmp()
> > which already test for less-than-zero etc?
> 
> In this case, yes. The code as it stands is very small, and users only
> check for the return value of these functions to be 0.
> 
> But I wonder whether that patch is needed at all. At least the rest of
> this series does not even seem to introduce new call sites of strcmp()
> or strncmp() ...

The implementation appears to be buggy as it stands - all it does is
sum up the differences in the whole string.

What if you're comparing two strings:

	ac
	ca

The first character has a difference of +2, the second has a difference
of -2.  Sum those together and you get zero.  Oh, the two strings are
identical!

Oh, and the other nitpick with the code is this:

	*(a++)

Really, are those parens really needed?  If you don't know the precendence
there, then you really shouldn't be programming in C, because this might
also be buggy:

	*(a++) - *(b++)

because if we declare that we don't know the precedence rules, it could be
that this is evaluated as *((a++) - (*(b++))) which would lead to errors!
Maybe some more parens should be added to make it clear!  Or maybe we
should just learn the precedence rules and realise that:

	*a++ - *b++

is correct and clear and there's no need for any stupid idiotic parens
here.

Yes, I loath unnecessary parens.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 12:52         ` Russell King - ARM Linux
@ 2013-12-18 16:01           ` Jason Cooper
  2013-12-18 18:50             ` Jason Cooper
  2013-12-18 18:56             ` Jason Cooper
  0 siblings, 2 replies; 35+ messages in thread
From: Jason Cooper @ 2013-12-18 16:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 18, 2013 at 12:52:03PM +0000, Russell King - ARM Linux wrote:
> On Wed, Dec 18, 2013 at 01:23:26PM +0100, Daniel Mack wrote:
> > On 12/18/2013 01:15 PM, Russell King - ARM Linux wrote:
> > > For instance, can you be sure that there aren't any uses of strncmp()
> > > which already test for less-than-zero etc?
> > 
> > In this case, yes. The code as it stands is very small, and users only
> > check for the return value of these functions to be 0.
> > 
> > But I wonder whether that patch is needed at all. At least the rest of
> > this series does not even seem to introduce new call sites of strcmp()
> > or strncmp() ...
> 
> The implementation appears to be buggy as it stands - all it does is
> sum up the differences in the whole string.
> 
> What if you're comparing two strings:
> 
> 	ac
> 	ca
> 
> The first character has a difference of +2, the second has a difference
> of -2.  Sum those together and you get zero.  Oh, the two strings are
> identical!

Yes, you are correct, the implementation isn't perfect.  I'll correct it
since I introduced it.

> Oh, and the other nitpick with the code is this:
> 
> 	*(a++)
> 
> Really, are those parens really needed?  If you don't know the precendence
> there, then you really shouldn't be programming in C, because this might
> also be buggy:
> 
> 	*(a++) - *(b++)
> 
> because if we declare that we don't know the precedence rules, it could be
> that this is evaluated as *((a++) - (*(b++))) which would lead to errors!
> Maybe some more parens should be added to make it clear!  Or maybe we
> should just learn the precedence rules and realise that:
> 
> 	*a++ - *b++
> 
> is correct and clear and there's no need for any stupid idiotic parens
> here.
> 
> Yes, I loath unnecessary parens.

Ahhh...  Good morning Russell! :)))

Yes, this is a bad habit I picked up, no shit, with gcc v2.96.  If you
recall, 2.96 was fucked up like a soup sandwich [1].  That was right
as I was starting my first job coding in C and I recall having to work
around many bugs, including segfaults when passing a double to printf().

The parentheses you caught above are another fallout from that.  iirc,
v2.96 *actually* did the wrong thing if we had

	*a++ = expr;

After a year or two of maintaining that code on Vermillion [2] (a Redhat
derivative the lead engineer liked), *(a++) was safe, and *a++ was
dangerous.

In Redhat's defense, I should've corrected the bad habit once I moved to
other distros and realized v2.96 was the problem.  But I didn't.  My
bad.  Patch on the way.

thx,

Jason.

[1] http://gcc.gnu.org/gcc-2.96.html
[2] "Vermillion is my customized linux distribution based on redhat."
    was found in $searchengine, but not at a site I want to link to.
    Apparently, the Internet forgot about this distro...

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 12:23       ` Daniel Mack
  2013-12-18 12:52         ` Russell King - ARM Linux
@ 2013-12-18 16:03         ` Jason Cooper
  2013-12-18 16:04           ` Daniel Mack
  1 sibling, 1 reply; 35+ messages in thread
From: Jason Cooper @ 2013-12-18 16:03 UTC (permalink / raw)
  To: linux-arm-kernel

Daniel,

On Wed, Dec 18, 2013 at 01:23:26PM +0100, Daniel Mack wrote:
> On 12/18/2013 01:15 PM, Russell King - ARM Linux wrote:
> > On Wed, Dec 18, 2013 at 01:09:05PM +0100, Piotr Wilczek wrote:
> >> Now function returns the number of characters that differ in the
> >> compared strings.
> > 
> > I think this is a mistake.  You're defining a standard C function in
> > a non-standard way.  For strcmp() and strncmp(), the C standard
> > requires:
> > 
> > RETURN VALUE
> >        The strcmp() and strncmp() functions return an integer less than, equal
> >        to, or greater than zero if s1 (or the first n bytes thereof) is found,
> >        respectively, to be less than, to match, or be greater than s2.
> > 
> > If you implement something different to that, don't call it strcmp() or
> > strncmp() but something else, because you're not providing the correct
> > functionality, and that will just confuse people and lead to bugs.
> 
> True.
> 
> > For instance, can you be sure that there aren't any uses of strncmp()
> > which already test for less-than-zero etc?
> 
> In this case, yes. The code as it stands is very small, and users only
> check for the return value of these functions to be 0.
> 
> But I wonder whether that patch is needed at all. At least the rest of
> this series does not even seem to introduce new call sites of strcmp()
> or strncmp() ...

Go ahead and drop this one.  I'll do a patch to fix this and remove
those heinous parentheses.  :)

thx,

Jason.

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 16:03         ` Jason Cooper
@ 2013-12-18 16:04           ` Daniel Mack
  0 siblings, 0 replies; 35+ messages in thread
From: Daniel Mack @ 2013-12-18 16:04 UTC (permalink / raw)
  To: linux-arm-kernel

On 12/18/2013 05:03 PM, Jason Cooper wrote:
> Daniel,
> 
> On Wed, Dec 18, 2013 at 01:23:26PM +0100, Daniel Mack wrote:
>> On 12/18/2013 01:15 PM, Russell King - ARM Linux wrote:
>>> On Wed, Dec 18, 2013 at 01:09:05PM +0100, Piotr Wilczek wrote:
>>>> Now function returns the number of characters that differ in the
>>>> compared strings.
>>>
>>> I think this is a mistake.  You're defining a standard C function in
>>> a non-standard way.  For strcmp() and strncmp(), the C standard
>>> requires:
>>>
>>> RETURN VALUE
>>>        The strcmp() and strncmp() functions return an integer less than, equal
>>>        to, or greater than zero if s1 (or the first n bytes thereof) is found,
>>>        respectively, to be less than, to match, or be greater than s2.
>>>
>>> If you implement something different to that, don't call it strcmp() or
>>> strncmp() but something else, because you're not providing the correct
>>> functionality, and that will just confuse people and lead to bugs.
>>
>> True.
>>
>>> For instance, can you be sure that there aren't any uses of strncmp()
>>> which already test for less-than-zero etc?
>>
>> In this case, yes. The code as it stands is very small, and users only
>> check for the return value of these functions to be 0.
>>
>> But I wonder whether that patch is needed at all. At least the rest of
>> this series does not even seem to introduce new call sites of strcmp()
>> or strncmp() ...
> 
> Go ahead and drop this one.  I'll do a patch to fix this and remove
> those heinous parentheses.  :)

:)

Ok, I'll pick up the rest of this series then.

Thanks!
Daniel

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 16:01           ` Jason Cooper
@ 2013-12-18 18:50             ` Jason Cooper
  2013-12-18 18:56             ` Jason Cooper
  1 sibling, 0 replies; 35+ messages in thread
From: Jason Cooper @ 2013-12-18 18:50 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 18, 2013 at 11:01:15AM -0500, Jason Cooper wrote:
> On Wed, Dec 18, 2013 at 12:52:03PM +0000, Russell King - ARM Linux wrote:
> > On Wed, Dec 18, 2013 at 01:23:26PM +0100, Daniel Mack wrote:
> > > On 12/18/2013 01:15 PM, Russell King - ARM Linux wrote:
> > > > For instance, can you be sure that there aren't any uses of strncmp()
> > > > which already test for less-than-zero etc?
> > > 
> > > In this case, yes. The code as it stands is very small, and users only
> > > check for the return value of these functions to be 0.
> > > 
> > > But I wonder whether that patch is needed at all. At least the rest of
> > > this series does not even seem to introduce new call sites of strcmp()
> > > or strncmp() ...
> > 
> > The implementation appears to be buggy as it stands - all it does is
> > sum up the differences in the whole string.
> > 
> > What if you're comparing two strings:
> > 
> > 	ac
> > 	ca
> > 
> > The first character has a difference of +2, the second has a difference
> > of -2.  Sum those together and you get zero.  Oh, the two strings are
> > identical!
> 
> Yes, you are correct, the implementation isn't perfect.  I'll correct it
> since I introduced it.

I've pushed the following:

-------8<---------------------------------
commit c3da82f0caa50c0efadbd6f129a326dda3fe3dec
Author: Jason Cooper <jason@lakedaemon.net>
Date:   Wed Dec 18 18:24:43 2013 +0000

    string: use kernel version of strncmp()
    
    Signed-off-by: Jason Cooper <jason@lakedaemon.net>

diff --git a/string.c b/string.c
index 5105490143c8..629c281f3abd 100644
--- a/string.c
+++ b/string.c
@@ -18,14 +18,18 @@ int strlen(const char *str)
 
 int strncmp(const char *stra, const char *strb, int len)
 {
-	int diff=0;
-	const char *a = stra;
-	const char *b = strb;
-
-	while ((a - stra) < len)
-		diff += *a++ - *b++;
-
-	return diff;
+	unsigned char c1, c2;
+
+	while (len) {
+		c1 = *stra++;
+		c2 = *strb++;
+		if (c1 != c2)
+			return c1 < c2 ? -1 : 1;
+		if (!c1)
+			break;
+		len--;
+	}
+	return 0;
 }
 
 void *gethexaddr(const char *str, const char **end)

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH V2 2/8] string: fix strncmp function
  2013-12-18 16:01           ` Jason Cooper
  2013-12-18 18:50             ` Jason Cooper
@ 2013-12-18 18:56             ` Jason Cooper
  1 sibling, 0 replies; 35+ messages in thread
From: Jason Cooper @ 2013-12-18 18:56 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Dec 18, 2013 at 11:01:15AM -0500, Jason Cooper wrote:
> On Wed, Dec 18, 2013 at 12:52:03PM +0000, Russell King - ARM Linux wrote:
> > Oh, and the other nitpick with the code is this:
> > 
> > 	*(a++)
> > 
> > Really, are those parens really needed?  If you don't know the precendence
> > there, then you really shouldn't be programming in C, because this might
> > also be buggy:
> > 
> > 	*(a++) - *(b++)
> > 
> > because if we declare that we don't know the precedence rules, it could be
> > that this is evaluated as *((a++) - (*(b++))) which would lead to errors!
> > Maybe some more parens should be added to make it clear!  Or maybe we
> > should just learn the precedence rules and realise that:
> > 
> > 	*a++ - *b++
> > 
> > is correct and clear and there's no need for any stupid idiotic parens
> > here.
> > 
> > Yes, I loath unnecessary parens.

and I've also pushed the following:

---------------8<----------------------------
commit 6a5d02f396107b2a0f4337d11e63c12ecd2a49c1
Author: Jason Cooper <jason@lakedaemon.net>
Date:   Wed Dec 18 18:19:38 2013 +0000

    string: remove unneeded parentheses
    
    Signed-off-by: Jason Cooper <jason@lakedaemon.net>

diff --git a/string.c b/string.c
index 0bd71798c4e6..5105490143c8 100644
--- a/string.c
+++ b/string.c
@@ -10,7 +10,7 @@ int strlen(const char *str)
 {
 	const char *c = str;
 
-	while (*(c++))
+	while (*c++)
 		;
 
 	return c - str;
@@ -23,7 +23,7 @@ int strncmp(const char *stra, const char *strb, int len)
 	const char *b = strb;
 
 	while ((a - stra) < len)
-		diff += *(a++) - *(b++);
+		diff += *a++ - *b++;
 
 	return diff;
 }

^ permalink raw reply related	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2013-12-18 18:56 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-12-11 13:07 [PATCH 0/8] Impedance matcher for Samsung boards Piotr Wilczek
2013-12-11 13:07 ` [PATCH 1/8] registers: add write u8 type Piotr Wilczek
2013-12-11 13:07 ` [PATCH 2/8] string: fix strncmp function Piotr Wilczek
2013-12-11 13:07 ` [PATCH 3/8] device tree blob must be 4-bytes alingned Piotr Wilczek
2013-12-11 13:07 ` [PATCH 4/8] Makefile: disable generating Thumb code Piotr Wilczek
2013-12-11 13:07 ` [PATCH 5/8] add memory operations Piotr Wilczek
2013-12-11 13:07 ` [PATCH 7/8] serial: add Exynos specific serial driver Piotr Wilczek
2013-12-11 13:07 ` [PATCH 8/8] board: add Samsung specific board support Piotr Wilczek
2013-12-11 13:16 ` [PATCH 0/8] Impedance matcher for Samsung boards Daniel Mack
2013-12-11 14:03   ` Piotr Wilczek
2013-12-11 14:10     ` Daniel Mack
2013-12-11 14:28       ` Tomasz Figa
2013-12-11 15:19         ` Daniel Mack
2013-12-11 15:48           ` Nicolas Pitre
2013-12-11 19:06           ` Jason Cooper
     [not found] ` <1386767259-15693-7-git-send-email-p.wilczek@samsung.com>
2013-12-11 19:00   ` [PATCH 6/8] add support to supplement atags to device tree Jason Cooper
2013-12-12 12:09     ` Piotr Wilczek
2013-12-12 12:36       ` Jason Cooper
2013-12-18 12:09 ` [PATCH V2 0/8] Impedance matcher for Samsung boards Piotr Wilczek
2013-12-18 12:09   ` [PATCH V2 1/8] registers: add write u8 type Piotr Wilczek
2013-12-18 12:09   ` [PATCH V2 2/8] string: fix strncmp function Piotr Wilczek
2013-12-18 12:15     ` Russell King - ARM Linux
2013-12-18 12:23       ` Daniel Mack
2013-12-18 12:52         ` Russell King - ARM Linux
2013-12-18 16:01           ` Jason Cooper
2013-12-18 18:50             ` Jason Cooper
2013-12-18 18:56             ` Jason Cooper
2013-12-18 16:03         ` Jason Cooper
2013-12-18 16:04           ` Daniel Mack
2013-12-18 12:50       ` Piotr Wilczek
2013-12-18 12:09   ` [PATCH V2 3/8] device tree blob must be 4-bytes alingned Piotr Wilczek
2013-12-18 12:09   ` [PATCH V2 4/8] Makefile: disable generating Thumb code Piotr Wilczek
2013-12-18 12:09   ` [PATCH V2 5/8] add memory operations Piotr Wilczek
2013-12-18 12:09   ` [PATCH V2 7/8] serial: add Exynos specific serial driver Piotr Wilczek
2013-12-18 12:09   ` [PATCH V2 8/8] board: add Samsung specific board support Piotr Wilczek

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.