arm:samsung:serial: Extract common Samsung UART code

This commit brings removal of duplicated code for UART IP block embedded at Samsung SoCs.
New include/asm/samsung-common directory has been created to store common code for existing and future Samsung targets.

Moreover building of UART code now depends on more verbose CONFIG_S5P_SERIAL. Thereof all relevant boards configs have been adjusted.

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v3:
- Comply with SPDX license format

Changes for v2:
- Remove S3C64XX define from the code

---
 arch/arm/include/asm/arch-exynos/uart.h    |   44 -------------------
 arch/arm/include/asm/arch-s5pc1xx/uart.h   |   44 -------------------
 arch/arm/include/asm/samsung-common/uart.h |   64
++++++++++++++++++++++++++++
drivers/serial/Makefile                    |    2 +-
drivers/serial/serial_s5p.c                |   13 +-----
include/configs/exynos5250-dt.h            |    1 +
include/configs/origen.h                   |    1 +
include/configs/s5p_goni.h                 |    1 +
include/configs/s5pc210_universal.h        |    1 +
include/configs/smdkc100.h                 |    1 +
include/configs/smdkv310.h                 |    1 +
include/configs/trats.h                    |    1 + 12 files changed,

74 insertions(+), 100 deletions(-) delete mode 100644 arch/arm/include/asm/arch-exynos/uart.h delete mode 100644 arch/arm/include/asm/arch-s5pc1xx/uart.h create mode 100644 arch/arm/include/asm/samsung-common/uart.h

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-exynos/uart.h deleted file mode 100644 index 33d6ba3..0000000

arch/arm/include/asm/arch-exynos/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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 int s5p_uart_divslot(void)
-{
-	return 0;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-s5pc1xx/uart.h deleted file mode 100644 index 26db098..0000000

arch/arm/include/asm/arch-s5pc1xx/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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[0x3d0];
-};
-
-static inline int s5p_uart_divslot(void)
-{
-	return 1;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/samsung-common/uart.h new file mode 100644 index 0000000..ce92399

arch/arm/include/asm/samsung-common/uart.h new file +64 -0

@@ -0,0 +1,64 @@
+/*
+ * (C) Copyright 2009 Samsung Electronics
+ * Minkyu Kang <[email protected]>
+ * Heungjun Kim <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __ASM_ARCH_UART_H_
+#define __ASM_ARCH_UART_H_
+
+#ifndef __ASSEMBLY__
+/* baudrate rest value */
+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;
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)
+	unsigned char	res3[0x3d0];
+#else /* Exynos 4 and 5 */
+	unsigned char	res3[0xffd0];
+#endif
+};
+
+
+static inline int s5p_uart_divslot(void)
+{
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)
+	return 1;
+#else /* Exynos 4 and 5 */
+	return 0;
+#endif
+}
+
+#define RX_FIFO_COUNT_MASK      0xff
+#define RX_FIFO_FULL_MASK       (1 << 8)
+#define TX_FIFO_FULL_MASK       (1 << 24)
+
+/* Information about a serial port */
+struct fdt_serial {
+	u32 base_addr;  /* address of registers in physical memory */
+	u8 port_id;     /* uart port number */
+	u8 enabled;     /* 1 if enabled, 0 if disabled */
+};
+
+#endif	/* __ASSEMBLY__ */
+
+#endif

drivers/serial/Makefile +1 -1

index 697f2bb..e3ca49a 100644
@@ -19,7 +19,7 @@ COBJS-$(CONFIG_LPC32XX_HSUART) += lpc32xx_hsuart.o
 COBJS-$(CONFIG_MCFUART) += mcfuart.o
 COBJS-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o
 COBJS-$(CONFIG_SYS_NS16550) += ns16550.o
-COBJS-$(CONFIG_S5P) += serial_s5p.o
+COBJS-$(CONFIG_S5P_SERIAL) += serial_s5p.o
 COBJS-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
 COBJS-$(CONFIG_IXP_SERIAL) += serial_ixp.o

drivers/serial/serial_s5p.c +2 -11

index f98b422..be08925 100644
@@ -12,22 +12,13 @@
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
-#include <asm/arch/uart.h>
+#include <asm/samsung-common/uart.h>
 #include <asm/arch/clk.h>
 #include <serial.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK	0xff
-#define RX_FIFO_FULL_MASK	(1 << 8)
-#define TX_FIFO_FULL_MASK	(1 << 24)
-
-/* Information about a serial port */
-struct fdt_serial {
-	u32 base_addr;  /* address of registers in physical memory */
-	u8 port_id;     /* uart port number */
-	u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
+struct fdt_serial config __attribute__ ((section(".data")));
 
 static inline struct s5p_uart *s5p_get_base_uart(int dev_index)
 {

(quoted diff) +0 -0

b/include/configs/exynos5250-dt.h index 8f8f85f..a759d07 100644

include/configs/exynos5250-dt.h +1 -0

@@ -69,6 +69,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (4
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_BAUDRATE			115200
 #define EXYNOS5_DEFAULT_UART_OFFSET	0x010000
 #define CONFIG_SILENT_CONSOLE

include/configs/origen.h +1 -0

index da13574..a59419d 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use SERIAL

2 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x020000

include/configs/s5p_goni.h +1 -0

index d0fafd7..812b7f3 100644
@@ -42,6 +42,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use

SERIAL2 */ #define CONFIG_BAUDRATE 115200

(quoted diff) +0 -0

b/include/configs/s5pc210_universal.h index 97a4008..2270449 100644

include/configs/s5pc210_universal.h +1 -0

@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2		1	/* use SERIAL 2 */
 #define CONFIG_BAUDRATE		115200
 

include/configs/smdkc100.h +1 -0

index a572e62..4631dac 100644
@@ -47,6 +47,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL0			1	/* use SERIAL

0 on SMDKC100 */
/* PWM */

include/configs/smdkv310.h +1 -0

index 0496661..9e10bf1 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL1			1	/* use SERIAL

1 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x010000

include/configs/trats.h +1 -0

index 9b6aac9..6b301c8 100644
@@ -53,6 +53,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (16
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			/* use SERIAL 2 */
 #define CONFIG_BAUDRATE			115200
 

This commit brings removal of duplicated code for UART IP block embedded at Samsung SoCs.
New include/asm/samsung-common directory has been created to store common code for existing and future Samsung targets.

Moreover building of UART code now depends on more verbose CONFIG_S5P_SERIAL. Thereof all relevant boards configs have been adjusted.

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v3:
- Comply with SPDX license format

Changes for v2:
- Remove S3C64XX define from the code

---
 arch/arm/include/asm/arch-exynos/uart.h    |   44 -------------------
 arch/arm/include/asm/arch-s5pc1xx/uart.h   |   44 -------------------
 arch/arm/include/asm/samsung-common/uart.h |   64 ++++++++++++++++++++++++++++
 drivers/serial/Makefile                    |    2 +-
 drivers/serial/serial_s5p.c                |   13 +-----
 include/configs/exynos5250-dt.h            |    1 +
 include/configs/origen.h                   |    1 +
 include/configs/s5p_goni.h                 |    1 +
 include/configs/s5pc210_universal.h        |    1 +
 include/configs/smdkc100.h                 |    1 +
 include/configs/smdkv310.h                 |    1 +
 include/configs/trats.h                    |    1 +
 12 files changed, 74 insertions(+), 100 deletions(-)
 delete mode 100644 arch/arm/include/asm/arch-exynos/uart.h
 delete mode 100644 arch/arm/include/asm/arch-s5pc1xx/uart.h
 create mode 100644 arch/arm/include/asm/samsung-common/uart.h

arch/arm/include/asm/arch-exynos/uart.h deleted +0 -44

deleted file mode 100644
index 33d6ba3..0000000
@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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 int s5p_uart_divslot(void)
-{
-	return 0;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

arch/arm/include/asm/arch-s5pc1xx/uart.h deleted +0 -44

deleted file mode 100644
index 26db098..0000000
@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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[0x3d0];
-};
-
-static inline int s5p_uart_divslot(void)
-{
-	return 1;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

arch/arm/include/asm/samsung-common/uart.h new file +24 -0

new file mode 100644
index 0000000..ce92399
@@ -0,0 +1,64 @@
+/*
+ * (C) Copyright 2009 Samsung Electronics
+ * Minkyu Kang <[email protected]>
+ * Heungjun Kim <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __ASM_ARCH_UART_H_
+#define __ASM_ARCH_UART_H_
+
+#ifndef __ASSEMBLY__
+/* baudrate rest value */
+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;

arch/arm/include/asm/samsung-common/uart.h new file +30 -0

+	unsigned char	res3[0x3d0];
+#else /* Exynos 4 and 5 */
+	unsigned char	res3[0xffd0];
+#endif
+};
+
+
+static inline int s5p_uart_divslot(void)
+{
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)
+	return 1;
+#else /* Exynos 4 and 5 */
+	return 0;
+#endif
+}
+
+#define RX_FIFO_COUNT_MASK      0xff
+#define RX_FIFO_FULL_MASK       (1 << 8)
+#define TX_FIFO_FULL_MASK       (1 << 24)
+
+/* Information about a serial port */
+struct fdt_serial {
+	u32 base_addr;  /* address of registers in physical memory */
+	u8 port_id;     /* uart port number */
+	u8 enabled;     /* 1 if enabled, 0 if disabled */
+};
+
+#endif	/* __ASSEMBLY__ */
+
+#endif

drivers/serial/Makefile +1 -1

index 697f2bb..e3ca49a 100644
@@ -19,7 +19,7 @@ COBJS-$(CONFIG_LPC32XX_HSUART) += lpc32xx_hsuart.o
 COBJS-$(CONFIG_MCFUART) += mcfuart.o
 COBJS-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o
 COBJS-$(CONFIG_SYS_NS16550) += ns16550.o
-COBJS-$(CONFIG_S5P) += serial_s5p.o
+COBJS-$(CONFIG_S5P_SERIAL) += serial_s5p.o
 COBJS-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
 COBJS-$(CONFIG_IXP_SERIAL) += serial_ixp.o

drivers/serial/serial_s5p.c +2 -11

index f98b422..be08925 100644
@@ -12,22 +12,13 @@
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
-#include <asm/arch/uart.h>
+#include <asm/samsung-common/uart.h>
 #include <asm/arch/clk.h>
 #include <serial.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK	0xff
-#define RX_FIFO_FULL_MASK	(1 << 8)
-#define TX_FIFO_FULL_MASK	(1 << 24)
-
-/* Information about a serial port */
-struct fdt_serial {
-	u32 base_addr;  /* address of registers in physical memory */
-	u8 port_id;     /* uart port number */
-	u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
+struct fdt_serial config __attribute__ ((section(".data")));
 
 static inline struct s5p_uart *s5p_get_base_uart(int dev_index)
 {

include/configs/exynos5250-dt.h +1 -0

index 8f8f85f..a759d07 100644
@@ -69,6 +69,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (4 << 20))
 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_BAUDRATE			115200
 #define EXYNOS5_DEFAULT_UART_OFFSET	0x010000
 #define CONFIG_SILENT_CONSOLE

include/configs/origen.h +1 -0

index da13574..a59419d 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1 << 20))
 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use SERIAL 2 */
 #define CONFIG_BAUDRATE			115200
 #define EXYNOS4_DEFAULT_UART_OFFSET	0x020000

include/configs/s5p_goni.h +1 -0

index d0fafd7..812b7f3 100644
@@ -42,6 +42,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use SERIAL2 */
 #define CONFIG_BAUDRATE			115200
 

include/configs/s5pc210_universal.h +1 -0

index 97a4008..2270449 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1 << 20))
 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2		1	/* use SERIAL 2 */
 #define CONFIG_BAUDRATE		115200
 

include/configs/smdkc100.h +1 -0

index a572e62..4631dac 100644
@@ -47,6 +47,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL0			1	/* use SERIAL 0 on SMDKC100 */
 
 /* PWM */

include/configs/smdkv310.h +1 -0

index 0496661..9e10bf1 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1 << 20))
 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL1			1	/* use SERIAL 1 */
 #define CONFIG_BAUDRATE			115200
 #define EXYNOS4_DEFAULT_UART_OFFSET	0x010000

include/configs/trats.h +0 -0

index 9b6aac9..6b301c8 100644

Dear Lukasz,

Quoted 168 lines · Lukasz Majewski

This commit brings removal of duplicated code for UART IP block embedded at Samsung SoCs.
New include/asm/samsung-common directory has been created to store common code for existing and future Samsung targets.

Moreover building of UART code now depends on more verbose CONFIG_S5P_SERIAL. Thereof all relevant boards configs have been adjusted.

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v3:
- Comply with SPDX license format

Changes for v2:
- Remove S3C64XX define from the code

---
 arch/arm/include/asm/arch-exynos/uart.h    |   44
------------------- arch/arm/include/asm/arch-s5pc1xx/uart.h   |
44 ------------------- arch/arm/include/asm/samsung-common/uart.h
|   64 ++++++++++++++++++++++++++++
drivers/serial/Makefile                    |    2 +-
drivers/serial/serial_s5p.c                |   13 +-----
include/configs/exynos5250-dt.h            |    1 +
include/configs/origen.h                   |    1 +
include/configs/s5p_goni.h                 |    1 +
include/configs/s5pc210_universal.h        |    1 +
include/configs/smdkc100.h                 |    1 +
include/configs/smdkv310.h                 |    1 +
include/configs/trats.h                    |    1 + 12 files

changed, 74 insertions(+), 100 deletions(-) delete mode 100644 arch/arm/include/asm/arch-exynos/uart.h delete mode 100644 arch/arm/include/asm/arch-s5pc1xx/uart.h create mode 100644 arch/arm/include/asm/samsung-common/uart.h

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-exynos/uart.h deleted file mode 100644 index 33d6ba3..0000000

arch/arm/include/asm/arch-exynos/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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 int s5p_uart_divslot(void)
-{
-	return 0;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-s5pc1xx/uart.h deleted file mode 100644 index 26db098..0000000

arch/arm/include/asm/arch-s5pc1xx/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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[0x3d0];
-};
-
-static inline int s5p_uart_divslot(void)
-{
-	return 1;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/samsung-common/uart.h new file mode 100644 index 0000000..ce92399

arch/arm/include/asm/samsung-common/uart.h new file +26 -0

@@ -0,0 +1,64 @@
+/*
+ * (C) Copyright 2009 Samsung Electronics
+ * Minkyu Kang <[email protected]>
+ * Heungjun Kim <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __ASM_ARCH_UART_H_
+#define __ASM_ARCH_UART_H_
+
+#ifndef __ASSEMBLY__
+/* baudrate rest value */
+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;

arch/arm/include/asm/samsung-common/uart.h line 34

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h new file +8 -0

+	unsigned int	umstat;
+	unsigned char	utxh;
+	unsigned char	res1[3];
+	unsigned char	urxh;
+	unsigned char	res2[3];
+	unsigned int	ubrdiv;
+	union br_rest	rest;
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)

OK. I understood your patch's concept and patch looks good. But, we have not been allowed ifdef at our SoCs. So, I can't accept your patch easily.

How you think?

Do we really need to combine headers?
I agree that they are almost duplicate codes.
But I thinks it's not a redundant.

Quoted 154 lines

arch/arm/include/asm/samsung-common/uart.h new file +23 -0

+	unsigned char	res3[0x3d0];
+#else /* Exynos 4 and 5 */
+	unsigned char	res3[0xffd0];
+#endif
+};
+
+
+static inline int s5p_uart_divslot(void)
+{
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)
+	return 1;
+#else /* Exynos 4 and 5 */
+	return 0;
+#endif
+}
+
+#define RX_FIFO_COUNT_MASK      0xff
+#define RX_FIFO_FULL_MASK       (1 << 8)
+#define TX_FIFO_FULL_MASK       (1 << 24)
+
+/* Information about a serial port */
+struct fdt_serial {
+	u32 base_addr;  /* address of registers in physical memory

*/

arch/arm/include/asm/samsung-common/uart.h new file +7 -0

+	u8 port_id;     /* uart port number */
+	u8 enabled;     /* 1 if enabled, 0 if disabled */
+};
+
+#endif	/* __ASSEMBLY__ */
+
+#endif

drivers/serial/Makefile +1 -1

index 697f2bb..e3ca49a 100644
@@ -19,7 +19,7 @@ COBJS-$(CONFIG_LPC32XX_HSUART) += lpc32xx_hsuart.o
 COBJS-$(CONFIG_MCFUART) += mcfuart.o
 COBJS-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o
 COBJS-$(CONFIG_SYS_NS16550) += ns16550.o
-COBJS-$(CONFIG_S5P) += serial_s5p.o
+COBJS-$(CONFIG_S5P_SERIAL) += serial_s5p.o
 COBJS-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
 COBJS-$(CONFIG_IXP_SERIAL) += serial_ixp.o

(quoted diff) +0 -0

b/drivers/serial/serial_s5p.c index f98b422..be08925 100644

drivers/serial/serial_s5p.c +2 -11

@@ -12,22 +12,13 @@
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
-#include <asm/arch/uart.h>
+#include <asm/samsung-common/uart.h>
 #include <asm/arch/clk.h>
 #include <serial.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK	0xff
-#define RX_FIFO_FULL_MASK	(1 << 8)
-#define TX_FIFO_FULL_MASK	(1 << 24)
-
-/* Information about a serial port */
-struct fdt_serial {
-	u32 base_addr;  /* address of registers in physical memory
 */
-	u8 port_id;     /* uart port number */
-	u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
+struct fdt_serial config __attribute__ ((section(".data")));
 
 static inline struct s5p_uart *s5p_get_base_uart(int dev_index)

{

(quoted diff) +0 -0

b/include/configs/exynos5250-dt.h index 8f8f85f..a759d07 100644

include/configs/exynos5250-dt.h +1 -0

@@ -69,6 +69,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (4
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_BAUDRATE			115200
 #define EXYNOS5_DEFAULT_UART_OFFSET	0x010000
 #define CONFIG_SILENT_CONSOLE

include/configs/origen.h +1 -0

index da13574..a59419d 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use

SERIAL 2 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x020000

include/configs/s5p_goni.h +1 -0

index d0fafd7..812b7f3 100644
@@ -42,6 +42,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use

SERIAL2 */ #define CONFIG_BAUDRATE 115200

(quoted diff) +0 -0

b/include/configs/s5pc210_universal.h index 97a4008..2270449 100644

include/configs/s5pc210_universal.h +1 -0

@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2		1	/* use SERIAL 2 */
 #define CONFIG_BAUDRATE		115200
 

include/configs/smdkc100.h +1 -0

index a572e62..4631dac 100644
@@ -47,6 +47,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL0			1	/* use

SERIAL 0 on SMDKC100 */
/* PWM */

include/configs/smdkv310.h +1 -0

index 0496661..9e10bf1 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL1			1	/* use

SERIAL 1 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x010000

include/configs/trats.h +0 -0

index 9b6aac9..6b301c8 100644
@@ -53,6 +53,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE +
 (16 << 20)) 
 /* select serial console configuration */

include/configs/trats.h line 59

Quoted 5 lines

include/configs/trats.h +1 -0

+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			/* use SERIAL 2 */
 #define CONFIG_BAUDRATE			115200
 

Thanks,
Minkyu Kang.

Hi Minkyu

Quoted 171 lines

Dear Lukasz,

Quoted 168 lines · Lukasz Majewski

This commit brings removal of duplicated code for UART IP block embedded at Samsung SoCs.
New include/asm/samsung-common directory has been created to store common code for existing and future Samsung targets.

Moreover building of UART code now depends on more verbose CONFIG_S5P_SERIAL. Thereof all relevant boards configs have been adjusted.

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v3:
- Comply with SPDX license format

Changes for v2:
- Remove S3C64XX define from the code

---
 arch/arm/include/asm/arch-exynos/uart.h    |   44
------------------- arch/arm/include/asm/arch-s5pc1xx/uart.h   |
44 ------------------- arch/arm/include/asm/samsung-common/uart.h
|   64 ++++++++++++++++++++++++++++
drivers/serial/Makefile                    |    2 +-
drivers/serial/serial_s5p.c                |   13 +-----
include/configs/exynos5250-dt.h            |    1 +
include/configs/origen.h                   |    1 +
include/configs/s5p_goni.h                 |    1 +
include/configs/s5pc210_universal.h        |    1 +
include/configs/smdkc100.h                 |    1 +
include/configs/smdkv310.h                 |    1 +
include/configs/trats.h                    |    1 + 12 files

changed, 74 insertions(+), 100 deletions(-) delete mode 100644 arch/arm/include/asm/arch-exynos/uart.h delete mode 100644 arch/arm/include/asm/arch-s5pc1xx/uart.h create mode 100644 arch/arm/include/asm/samsung-common/uart.h

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-exynos/uart.h deleted file mode 100644 index 33d6ba3..0000000

arch/arm/include/asm/arch-exynos/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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 int s5p_uart_divslot(void)
-{
-	return 0;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-s5pc1xx/uart.h deleted file mode 100644 index 26db098..0000000

arch/arm/include/asm/arch-s5pc1xx/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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[0x3d0];
-};
-
-static inline int s5p_uart_divslot(void)
-{
-	return 1;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/samsung-common/uart.h new file mode 100644 index 0000000..ce92399

arch/arm/include/asm/samsung-common/uart.h new file +26 -0

@@ -0,0 +1,64 @@
+/*
+ * (C) Copyright 2009 Samsung Electronics
+ * Minkyu Kang <[email protected]>
+ * Heungjun Kim <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __ASM_ARCH_UART_H_
+#define __ASM_ARCH_UART_H_
+
+#ifndef __ASSEMBLY__
+/* baudrate rest value */
+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;

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h line 34

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h new file +8 -0

+	unsigned int	umstat;
+	unsigned char	utxh;
+	unsigned char	res1[3];
+	unsigned char	urxh;
+	unsigned char	res2[3];
+	unsigned int	ubrdiv;
+	union br_rest	rest;
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)

     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^[1]

Quoted 4 lines

OK. I understood your patch's concept and patch looks good. But, we have not been allowed ifdef at our SoCs. So, I can't accept your patch easily.

The problem is that several Samsung SoCs reuse the same IP blocks.

For example S3C6410 (which I want to add to u-boot), S5PC100, S5PC110, Exynos4 and Exynos5 are using the same timer, pwm, sromc, uart, udc, sdhci, and many others IP blocks.

As I've proposed in this patch series - several IP blocks *.c files shall go into samsung-common directory at arch/arm/cpu/samsung-common. This arm soc version independent code can be reused at s3c6410 (arm1176) CPU.

Another issue - headers. The register map for relevant IP blocks is the same at s3c6410 and exynos5250. The only difference is the offset at the end of the structure.

Quoted 2 lines

How you think?

We can remove [1] if we define the struct s5p_uart without the "unsigned char res3[xxx];"in the end.

It is correct, since we are using the "IP block offset defines" anyway to index correct IP block instance (like uart0/1/2...).

Quoted 4 lines

Do we really need to combine headers?
I agree that they are almost duplicate codes.
But I thinks it's not a redundant.

Another issue is the artificial distinction between s5pc1xx and exynos families of the processors.

Both are armv7, both are cortex (A8 and A9).I think that arch/arm/cpu/armv7/s5pc1xx code shall be moved to arch/arm/cpu/armv7/exynos

Also arch/arm/include/asm/arch-{s5pc1xx|exynos} code can be put to samsung-common directory (as also proposed in those patch series).

Frankly, we shall mimic the tegra or imx tree. As fair as I've noticed at those SoCs the common code has been put to various *-common directories.

I've added other Samsung boards maintainers to Cc.

Quoted 155 lines

Quoted 155 lines

arch/arm/include/asm/samsung-common/uart.h new file +23 -0

+	unsigned char	res3[0x3d0];
+#else /* Exynos 4 and 5 */
+	unsigned char	res3[0xffd0];
+#endif
+};
+
+
+static inline int s5p_uart_divslot(void)
+{
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)
+	return 1;
+#else /* Exynos 4 and 5 */
+	return 0;
+#endif
+}
+
+#define RX_FIFO_COUNT_MASK      0xff
+#define RX_FIFO_FULL_MASK       (1 << 8)
+#define TX_FIFO_FULL_MASK       (1 << 24)
+
+/* Information about a serial port */
+struct fdt_serial {
+	u32 base_addr;  /* address of registers in physical

memory */

arch/arm/include/asm/samsung-common/uart.h new file +7 -0

+	u8 port_id;     /* uart port number */
+	u8 enabled;     /* 1 if enabled, 0 if disabled */
+};
+
+#endif	/* __ASSEMBLY__ */
+
+#endif

drivers/serial/Makefile +0 -0

index 697f2bb..e3ca49a 100644
@@ -19,7 +19,7 @@ COBJS-$(CONFIG_LPC32XX_HSUART) +=

lpc32xx_hsuart.o COBJS-$(CONFIG_MCFUART) += mcfuart.o
COBJS-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o COBJS-$(CONFIG_SYS_NS16550) += ns16550.o

drivers/serial/Makefile +1 -1

-COBJS-$(CONFIG_S5P) += serial_s5p.o
+COBJS-$(CONFIG_S5P_SERIAL) += serial_s5p.o
 COBJS-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
 COBJS-$(CONFIG_IXP_SERIAL) += serial_ixp.o

(quoted diff) +0 -0

b/drivers/serial/serial_s5p.c index f98b422..be08925 100644

drivers/serial/serial_s5p.c +1 -8

@@ -12,22 +12,13 @@
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
-#include <asm/arch/uart.h>
+#include <asm/samsung-common/uart.h>
 #include <asm/arch/clk.h>
 #include <serial.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK	0xff
-#define RX_FIFO_FULL_MASK	(1 << 8)
-#define TX_FIFO_FULL_MASK	(1 << 24)
-
-/* Information about a serial port */
-struct fdt_serial {
-	u32 base_addr;  /* address of registers in physical

memory */

drivers/serial/serial_s5p.c +1 -3

-	u8 port_id;     /* uart port number */
-	u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
+struct fdt_serial config __attribute__ ((section(".data")));
 
 static inline struct s5p_uart *s5p_get_base_uart(int dev_index)
 {

(quoted diff) +0 -0

b/include/configs/exynos5250-dt.h index 8f8f85f..a759d07 100644

include/configs/exynos5250-dt.h +1 -0

@@ -69,6 +69,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE +
 (4 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_BAUDRATE			115200
 #define EXYNOS5_DEFAULT_UART_OFFSET	0x010000
 #define CONFIG_SILENT_CONSOLE

include/configs/origen.h +1 -0

index da13574..a59419d 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE +
 (1 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use

SERIAL 2 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x020000

(quoted diff) +0 -0

b/include/configs/s5p_goni.h index d0fafd7..812b7f3 100644

include/configs/s5p_goni.h +1 -0

@@ -42,6 +42,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use

SERIAL2 */ #define CONFIG_BAUDRATE 115200

(quoted diff) +0 -0

b/include/configs/s5pc210_universal.h index 97a4008..2270449 100644 --- a/include/configs/s5pc210_universal.h +++ b/include/configs/s5pc210_universal.h

include/configs/s5p_goni.h +1 -0

@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE +
 (1 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2		1	/* use SERIAL 2 */
 #define CONFIG_BAUDRATE		115200
 

(quoted diff) +0 -0

b/include/configs/smdkc100.h index a572e62..4631dac 100644

include/configs/smdkc100.h +1 -0

@@ -47,6 +47,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL0			1	/* use

SERIAL 0 on SMDKC100 */
/* PWM */

(quoted diff) +0 -0

b/include/configs/smdkv310.h index 0496661..9e10bf1 100644

include/configs/smdkv310.h +1 -0

@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE +
 (1 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL1			1	/* use

SERIAL 1 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x010000

include/configs/trats.h +0 -0

index 9b6aac9..6b301c8 100644
@@ -53,6 +53,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE +
 (16 << 20)) 
 /* select serial console configuration */

Quoted 8 lines

include/configs/trats.h line 59

Quoted 5 lines

include/configs/trats.h +1 -0

+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			/* use SERIAL 2 */
 #define CONFIG_BAUDRATE			115200
 

Thanks,
Minkyu Kang.

Regards,

Lukasz Majewski

Hi Minkyu

Quoted 171 lines

Dear Lukasz,

Quoted 168 lines · Lukasz Majewski

This commit brings removal of duplicated code for UART IP block embedded at Samsung SoCs.
New include/asm/samsung-common directory has been created to store common code for existing and future Samsung targets.

Moreover building of UART code now depends on more verbose CONFIG_S5P_SERIAL. Thereof all relevant boards configs have been adjusted.

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v3:
- Comply with SPDX license format

Changes for v2:
- Remove S3C64XX define from the code

---
 arch/arm/include/asm/arch-exynos/uart.h    |   44
------------------- arch/arm/include/asm/arch-s5pc1xx/uart.h   |
44 ------------------- arch/arm/include/asm/samsung-common/uart.h
|   64 ++++++++++++++++++++++++++++
drivers/serial/Makefile                    |    2 +-
drivers/serial/serial_s5p.c                |   13 +-----
include/configs/exynos5250-dt.h            |    1 +
include/configs/origen.h                   |    1 +
include/configs/s5p_goni.h                 |    1 +
include/configs/s5pc210_universal.h        |    1 +
include/configs/smdkc100.h                 |    1 +
include/configs/smdkv310.h                 |    1 +
include/configs/trats.h                    |    1 + 12 files

changed, 74 insertions(+), 100 deletions(-) delete mode 100644 arch/arm/include/asm/arch-exynos/uart.h delete mode 100644 arch/arm/include/asm/arch-s5pc1xx/uart.h create mode 100644 arch/arm/include/asm/samsung-common/uart.h

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-exynos/uart.h deleted file mode 100644 index 33d6ba3..0000000

arch/arm/include/asm/arch-exynos/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:        GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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 int s5p_uart_divslot(void)
-{
-   return 0;
-}
-
-#endif     /* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-s5pc1xx/uart.h deleted file mode 100644 index 26db098..0000000

arch/arm/include/asm/arch-s5pc1xx/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:        GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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[0x3d0];
-};
-
-static inline int s5p_uart_divslot(void)
-{
-   return 1;
-}
-
-#endif     /* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/samsung-common/uart.h new file mode 100644 index 0000000..ce92399

arch/arm/include/asm/samsung-common/uart.h new file +26 -0

@@ -0,0 +1,64 @@
+/*
+ * (C) Copyright 2009 Samsung Electronics
+ * Minkyu Kang <[email protected]>
+ * Heungjun Kim <[email protected]>
+ *
+ * SPDX-License-Identifier:        GPL-2.0+
+ */
+
+#ifndef __ASM_ARCH_UART_H_
+#define __ASM_ARCH_UART_H_
+
+#ifndef __ASSEMBLY__
+/* baudrate rest value */
+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;

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h line 34

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h new file +8 -0

+   unsigned int    umstat;
+   unsigned char   utxh;
+   unsigned char   res1[3];
+   unsigned char   urxh;
+   unsigned char   res2[3];
+   unsigned int    ubrdiv;
+   union br_rest   rest;
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)

     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^[1]

Quoted 4 lines

OK. I understood your patch's concept and patch looks good. But, we have not been allowed ifdef at our SoCs. So, I can't accept your patch easily.

The problem is that several Samsung SoCs reuse the same IP blocks.

For example S3C6410 (which I want to add to u-boot), S5PC100, S5PC110, Exynos4 and Exynos5 are using the same timer, pwm, sromc, uart, udc, sdhci, and many others IP blocks.

As I've proposed in this patch series - several IP blocks *.c files shall go into samsung-common directory at arch/arm/cpu/samsung-common. This arm soc version independent code can be reused at s3c6410 (arm1176) CPU.

Another issue - headers. The register map for relevant IP blocks is the same at s3c6410 and exynos5250. The only difference is the offset at the end of the structure.

Quoted 1 line

Quoted 1 line

How you think?

We can remove [1] if we define the struct s5p_uart without the "unsigned char res3[xxx];"in the end.

It is correct, since we are using the "IP block offset defines" anyway to index correct IP block instance (like uart0/1/2...).

Quoted 4 lines

Do we really need to combine headers?
I agree that they are almost duplicate codes.
But I thinks it's not a redundant.

Another issue is the artificial distinction between s5pc1xx and exynos families of the processors.

Both are armv7, both are cortex (A8 and A9).I think that arch/arm/cpu/armv7/s5pc1xx code shall be moved to arch/arm/cpu/armv7/exynos

Also arch/arm/include/asm/arch-{s5pc1xx|exynos} code can be put to samsung-common directory (as also proposed in those patch series).

Frankly, we shall mimic the tegra or imx tree. As fair as I've noticed at those SoCs the common code has been put to various *-common directories.

I've added other Samsung boards maintainers to Cc.

Quoted 10 lines

arch/arm/include/asm/samsung-common/uart.h line 44

Quoted 10 lines

arch/arm/include/asm/samsung-common/uart.h new file +10 -0

+   unsigned char   res3[0x3d0];
+#else /* Exynos 4 and 5 */
+   unsigned char   res3[0xffd0];
+#endif
+};
+
+
+static inline int s5p_uart_divslot(void)
+{
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)

Quoted 144 lines

Quoted 144 lines

arch/arm/include/asm/samsung-common/uart.h new file +13 -0

+   return 1;
+#else /* Exynos 4 and 5 */
+   return 0;
+#endif
+}
+
+#define RX_FIFO_COUNT_MASK      0xff
+#define RX_FIFO_FULL_MASK       (1 << 8)
+#define TX_FIFO_FULL_MASK       (1 << 24)
+
+/* Information about a serial port */
+struct fdt_serial {
+   u32 base_addr;  /* address of registers in physical memory

*/

arch/arm/include/asm/samsung-common/uart.h new file +7 -0

+   u8 port_id;     /* uart port number */
+   u8 enabled;     /* 1 if enabled, 0 if disabled */
+};
+
+#endif     /* __ASSEMBLY__ */
+
+#endif

drivers/serial/Makefile +1 -1

index 697f2bb..e3ca49a 100644
@@ -19,7 +19,7 @@ COBJS-$(CONFIG_LPC32XX_HSUART) += lpc32xx_hsuart.o
 COBJS-$(CONFIG_MCFUART) += mcfuart.o
 COBJS-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o
 COBJS-$(CONFIG_SYS_NS16550) += ns16550.o
-COBJS-$(CONFIG_S5P) += serial_s5p.o
+COBJS-$(CONFIG_S5P_SERIAL) += serial_s5p.o
 COBJS-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
 COBJS-$(CONFIG_IXP_SERIAL) += serial_ixp.o

(quoted diff) +0 -0

b/drivers/serial/serial_s5p.c index f98b422..be08925 100644

drivers/serial/serial_s5p.c +2 -11

@@ -12,22 +12,13 @@
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
-#include <asm/arch/uart.h>
+#include <asm/samsung-common/uart.h>
 #include <asm/arch/clk.h>
 #include <serial.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK 0xff
-#define RX_FIFO_FULL_MASK  (1 << 8)
-#define TX_FIFO_FULL_MASK  (1 << 24)
-
-/* Information about a serial port */
-struct fdt_serial {
-   u32 base_addr;  /* address of registers in physical memory
 */
-   u8 port_id;     /* uart port number */
-   u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
+struct fdt_serial config __attribute__ ((section(".data")));
 
 static inline struct s5p_uart *s5p_get_base_uart(int dev_index)

{

(quoted diff) +0 -0

b/include/configs/exynos5250-dt.h index 8f8f85f..a759d07 100644

include/configs/exynos5250-dt.h +1 -0

@@ -69,6 +69,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE + (4
 << 20))
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_BAUDRATE                    115200
 #define EXYNOS5_DEFAULT_UART_OFFSET        0x010000
 #define CONFIG_SILENT_CONSOLE

include/configs/origen.h +1 -0

index da13574..a59419d 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE + (1
 << 20))
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2                     1       /* use

SERIAL 2 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x020000

include/configs/s5p_goni.h +1 -0

index d0fafd7..812b7f3 100644
@@ -42,6 +42,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2                     1       /* use

SERIAL2 */ #define CONFIG_BAUDRATE 115200

(quoted diff) +0 -0

b/include/configs/s5pc210_universal.h index 97a4008..2270449 100644

include/configs/s5pc210_universal.h +1 -0

@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE + (1
 << 20))
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2             1       /* use SERIAL 2 */
 #define CONFIG_BAUDRATE            115200
 

include/configs/smdkc100.h +1 -0

index a572e62..4631dac 100644
@@ -47,6 +47,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL0                     1       /* use

SERIAL 0 on SMDKC100 */
/* PWM */

include/configs/smdkv310.h +1 -0

index 0496661..9e10bf1 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE + (1
 << 20))
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL1                     1       /* use

SERIAL 1 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x010000

include/configs/trats.h +0 -0

index 9b6aac9..6b301c8 100644
@@ -53,6 +53,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE +
 (16 << 20))
 /* select serial console configuration */

Quoted 8 lines

include/configs/trats.h line 59

Quoted 5 lines

include/configs/trats.h +1 -0

+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2                     /* use SERIAL 2 */
 #define CONFIG_BAUDRATE                    115200
 

Thanks,
Minkyu Kang.

Regards,

Lukasz Majewski

_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Hi Minkyu

Quoted 171 lines

Dear Lukasz,

Quoted 168 lines · Lukasz Majewski

This commit brings removal of duplicated code for UART IP block embedded at Samsung SoCs.
New include/asm/samsung-common directory has been created to store common code for existing and future Samsung targets.

Moreover building of UART code now depends on more verbose CONFIG_S5P_SERIAL. Thereof all relevant boards configs have been adjusted.

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v3:
- Comply with SPDX license format

Changes for v2:
- Remove S3C64XX define from the code

---
 arch/arm/include/asm/arch-exynos/uart.h    |   44
------------------- arch/arm/include/asm/arch-s5pc1xx/uart.h   |
44 ------------------- arch/arm/include/asm/samsung-common/uart.h
|   64 ++++++++++++++++++++++++++++
drivers/serial/Makefile                    |    2 +-
drivers/serial/serial_s5p.c                |   13 +-----
include/configs/exynos5250-dt.h            |    1 +
include/configs/origen.h                   |    1 +
include/configs/s5p_goni.h                 |    1 +
include/configs/s5pc210_universal.h        |    1 +
include/configs/smdkc100.h                 |    1 +
include/configs/smdkv310.h                 |    1 +
include/configs/trats.h                    |    1 + 12 files

changed, 74 insertions(+), 100 deletions(-) delete mode 100644 arch/arm/include/asm/arch-exynos/uart.h delete mode 100644 arch/arm/include/asm/arch-s5pc1xx/uart.h create mode 100644 arch/arm/include/asm/samsung-common/uart.h

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-exynos/uart.h deleted file mode 100644 index 33d6ba3..0000000

arch/arm/include/asm/arch-exynos/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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 int s5p_uart_divslot(void)
-{
-	return 0;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-s5pc1xx/uart.h deleted file mode 100644 index 26db098..0000000

arch/arm/include/asm/arch-s5pc1xx/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:	GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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[0x3d0];
-};
-
-static inline int s5p_uart_divslot(void)
-{
-	return 1;
-}
-
-#endif	/* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/samsung-common/uart.h new file mode 100644 index 0000000..ce92399

arch/arm/include/asm/samsung-common/uart.h new file +26 -0

@@ -0,0 +1,64 @@
+/*
+ * (C) Copyright 2009 Samsung Electronics
+ * Minkyu Kang <[email protected]>
+ * Heungjun Kim <[email protected]>
+ *
+ * SPDX-License-Identifier:	GPL-2.0+
+ */
+
+#ifndef __ASM_ARCH_UART_H_
+#define __ASM_ARCH_UART_H_
+
+#ifndef __ASSEMBLY__
+/* baudrate rest value */
+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;

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h line 34

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h new file +8 -0

+	unsigned int	umstat;
+	unsigned char	utxh;
+	unsigned char	res1[3];
+	unsigned char	urxh;
+	unsigned char	res2[3];
+	unsigned int	ubrdiv;
+	union br_rest	rest;
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)

     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^[1]

Quoted 4 lines

OK. I understood your patch's concept and patch looks good. But, we have not been allowed ifdef at our SoCs. So, I can't accept your patch easily.

The problem is that several Samsung SoCs reuse the same IP blocks.

For example S3C6410 (which I want to add to u-boot), S5PC100, S5PC110, Exynos4 and Exynos5 are using the same timer, pwm, sromc, uart, udc, sdhci, and many others IP blocks.

As I've proposed in this patch series - several IP blocks *.c files shall go into samsung-common directory at arch/arm/cpu/samsung-common. This arm soc version independent code can be reused at s3c6410 (arm1176) CPU.

Another issue - headers. The register map for relevant IP blocks is the same at s3c6410 and exynos5250. The only difference is the offset at the end of the structure.

Quoted 1 line

Quoted 1 line

How you think?

We can remove [1] if we define the struct s5p_uart without the "unsigned char res3[xxx];"in the end.

It is correct, since we are using the "IP block offset defines" anyway to index correct IP block instance (like uart0/1/2...).

Quoted 4 lines

Do we really need to combine headers?
I agree that they are almost duplicate codes.
But I thinks it's not a redundant.

Another issue is the artificial distinction between s5pc1xx and exynos families of the processors.

Both are armv7, both are cortex (A8 and A9).I think that arch/arm/cpu/armv7/s5pc1xx code shall be moved to arch/arm/cpu/armv7/exynos

Also arch/arm/include/asm/arch-{s5pc1xx|exynos} code can be put to samsung-common directory (as also proposed in those patch series).

Frankly, we shall mimic the tegra or imx tree. As fair as I've noticed at those SoCs the common code has been put to various *-common directories.

I've added other Samsung boards maintainers to Cc.

Quoted 154 lines

Quoted 154 lines

arch/arm/include/asm/samsung-common/uart.h new file +23 -0

+	unsigned char	res3[0x3d0];
+#else /* Exynos 4 and 5 */
+	unsigned char	res3[0xffd0];
+#endif
+};
+
+
+static inline int s5p_uart_divslot(void)
+{
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)
+	return 1;
+#else /* Exynos 4 and 5 */
+	return 0;
+#endif
+}
+
+#define RX_FIFO_COUNT_MASK      0xff
+#define RX_FIFO_FULL_MASK       (1 << 8)
+#define TX_FIFO_FULL_MASK       (1 << 24)
+
+/* Information about a serial port */
+struct fdt_serial {
+	u32 base_addr;  /* address of registers in physical memory

*/

arch/arm/include/asm/samsung-common/uart.h new file +7 -0

+	u8 port_id;     /* uart port number */
+	u8 enabled;     /* 1 if enabled, 0 if disabled */
+};
+
+#endif	/* __ASSEMBLY__ */
+
+#endif

drivers/serial/Makefile +1 -1

index 697f2bb..e3ca49a 100644
@@ -19,7 +19,7 @@ COBJS-$(CONFIG_LPC32XX_HSUART) += lpc32xx_hsuart.o
 COBJS-$(CONFIG_MCFUART) += mcfuart.o
 COBJS-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o
 COBJS-$(CONFIG_SYS_NS16550) += ns16550.o
-COBJS-$(CONFIG_S5P) += serial_s5p.o
+COBJS-$(CONFIG_S5P_SERIAL) += serial_s5p.o
 COBJS-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
 COBJS-$(CONFIG_IXP_SERIAL) += serial_ixp.o

(quoted diff) +0 -0

b/drivers/serial/serial_s5p.c index f98b422..be08925 100644

drivers/serial/serial_s5p.c +2 -11

@@ -12,22 +12,13 @@
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
-#include <asm/arch/uart.h>
+#include <asm/samsung-common/uart.h>
 #include <asm/arch/clk.h>
 #include <serial.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK	0xff
-#define RX_FIFO_FULL_MASK	(1 << 8)
-#define TX_FIFO_FULL_MASK	(1 << 24)
-
-/* Information about a serial port */
-struct fdt_serial {
-	u32 base_addr;  /* address of registers in physical memory
 */
-	u8 port_id;     /* uart port number */
-	u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
+struct fdt_serial config __attribute__ ((section(".data")));
 
 static inline struct s5p_uart *s5p_get_base_uart(int dev_index)

{

(quoted diff) +0 -0

b/include/configs/exynos5250-dt.h index 8f8f85f..a759d07 100644

include/configs/exynos5250-dt.h +1 -0

@@ -69,6 +69,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (4
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_BAUDRATE			115200
 #define EXYNOS5_DEFAULT_UART_OFFSET	0x010000
 #define CONFIG_SILENT_CONSOLE

include/configs/origen.h +1 -0

index da13574..a59419d 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use

SERIAL 2 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x020000

include/configs/s5p_goni.h +1 -0

index d0fafd7..812b7f3 100644
@@ -42,6 +42,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			1	/* use

SERIAL2 */ #define CONFIG_BAUDRATE 115200

(quoted diff) +0 -0

b/include/configs/s5pc210_universal.h index 97a4008..2270449 100644

include/configs/s5pc210_universal.h +1 -0

@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2		1	/* use SERIAL 2 */
 #define CONFIG_BAUDRATE		115200
 

include/configs/smdkc100.h +1 -0

index a572e62..4631dac 100644
@@ -47,6 +47,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL0			1	/* use

SERIAL 0 on SMDKC100 */
/* PWM */

include/configs/smdkv310.h +1 -0

index 0496661..9e10bf1 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE + (1
 << 20)) 
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL1			1	/* use

SERIAL 1 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x010000

include/configs/trats.h +0 -0

index 9b6aac9..6b301c8 100644
@@ -53,6 +53,7 @@
 #define CONFIG_SYS_MALLOC_LEN		(CONFIG_ENV_SIZE +
 (16 << 20)) 
 /* select serial console configuration */

Quoted 8 lines

include/configs/trats.h line 59

Quoted 5 lines

include/configs/trats.h +1 -0

+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2			/* use SERIAL 2 */
 #define CONFIG_BAUDRATE			115200
 

Thanks,
Minkyu Kang.

Regards,

Lukasz Majewski

Dear Lukasz,

Quoted 204 lines · Lukasz Majewski

Hi Minkyu

Quoted 171 lines

Dear Lukasz,

Quoted 168 lines · Lukasz Majewski

This commit brings removal of duplicated code for UART IP block embedded at Samsung SoCs.
New include/asm/samsung-common directory has been created to store common code for existing and future Samsung targets.

Moreover building of UART code now depends on more verbose CONFIG_S5P_SERIAL. Thereof all relevant boards configs have been adjusted.

Signed-off-by Lukasz Majewski <[email protected]>

---
Changes for v3:
- Comply with SPDX license format

Changes for v2:
- Remove S3C64XX define from the code

---
 arch/arm/include/asm/arch-exynos/uart.h    |   44
------------------- arch/arm/include/asm/arch-s5pc1xx/uart.h   |
44 ------------------- arch/arm/include/asm/samsung-common/uart.h
|   64 ++++++++++++++++++++++++++++
drivers/serial/Makefile                    |    2 +-
drivers/serial/serial_s5p.c                |   13 +-----
include/configs/exynos5250-dt.h            |    1 +
include/configs/origen.h                   |    1 +
include/configs/s5p_goni.h                 |    1 +
include/configs/s5pc210_universal.h        |    1 +
include/configs/smdkc100.h                 |    1 +
include/configs/smdkv310.h                 |    1 +
include/configs/trats.h                    |    1 + 12 files

changed, 74 insertions(+), 100 deletions(-) delete mode 100644 arch/arm/include/asm/arch-exynos/uart.h delete mode 100644 arch/arm/include/asm/arch-s5pc1xx/uart.h create mode 100644 arch/arm/include/asm/samsung-common/uart.h

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-exynos/uart.h deleted file mode 100644 index 33d6ba3..0000000

arch/arm/include/asm/arch-exynos/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:        GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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 int s5p_uart_divslot(void)
-{
-   return 0;
-}
-
-#endif     /* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/arch-s5pc1xx/uart.h deleted file mode 100644 index 26db098..0000000

arch/arm/include/asm/arch-s5pc1xx/uart.h deleted +0 -44

@@ -1,44 +0,0 @@
-/*
- * (C) Copyright 2009 Samsung Electronics
- * Minkyu Kang <[email protected]>
- * Heungjun Kim <[email protected]>
- *
- * SPDX-License-Identifier:        GPL-2.0+
- */
-
-#ifndef __ASM_ARCH_UART_H_
-#define __ASM_ARCH_UART_H_
-
-#ifndef __ASSEMBLY__
-/* baudrate rest value */
-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[0x3d0];
-};
-
-static inline int s5p_uart_divslot(void)
-{
-   return 1;
-}
-
-#endif     /* __ASSEMBLY__ */
-
-#endif

(quoted diff) +0 -0

b/arch/arm/include/asm/samsung-common/uart.h new file mode 100644 index 0000000..ce92399

arch/arm/include/asm/samsung-common/uart.h new file +26 -0

@@ -0,0 +1,64 @@
+/*
+ * (C) Copyright 2009 Samsung Electronics
+ * Minkyu Kang <[email protected]>
+ * Heungjun Kim <[email protected]>
+ *
+ * SPDX-License-Identifier:        GPL-2.0+
+ */
+
+#ifndef __ASM_ARCH_UART_H_
+#define __ASM_ARCH_UART_H_
+
+#ifndef __ASSEMBLY__
+/* baudrate rest value */
+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;

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h line 34

Quoted 8 lines

arch/arm/include/asm/samsung-common/uart.h new file +8 -0

+   unsigned int    umstat;
+   unsigned char   utxh;
+   unsigned char   res1[3];
+   unsigned char   urxh;
+   unsigned char   res2[3];
+   unsigned int    ubrdiv;
+   union br_rest   rest;
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)

     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^[1]

Quoted 4 lines

OK. I understood your patch's concept and patch looks good. But, we have not been allowed ifdef at our SoCs. So, I can't accept your patch easily.

The problem is that several Samsung SoCs reuse the same IP blocks.

For example S3C6410 (which I want to add to u-boot), S5PC100, S5PC110, Exynos4 and Exynos5 are using the same timer, pwm, sromc, uart, udc, sdhci, and many others IP blocks.

As I've proposed in this patch series - several IP blocks *.c files shall go into samsung-common directory at arch/arm/cpu/samsung-common. This arm soc version independent code can be reused at s3c6410 (arm1176) CPU.

Another issue - headers. The register map for relevant IP blocks is the same at s3c6410 and exynos5250. The only difference is the offset at the end of the structure.

Quoted 1 line

Quoted 4 lines

Quoted 1 line

How you think?

We can remove [1] if we define the struct s5p_uart without the "unsigned char res3[xxx];"in the end.

It's OK.
but, how can you remove other ifdefs?

Quoted 10 lines

It is correct, since we are using the "IP block offset defines" anyway to index correct IP block instance (like uart0/1/2...).

Quoted 4 lines

Do we really need to combine headers?
I agree that they are almost duplicate codes.
But I thinks it's not a redundant.

Quoted 6 lines

Another issue is the artificial distinction between s5pc1xx and exynos families of the processors.

Both are armv7, both are cortex (A8 and A9).I think that arch/arm/cpu/armv7/s5pc1xx code shall be moved to arch/arm/cpu/armv7/exynos

hm.. then, some files (cpu, clock, gpio,,, ) will be complicated.

Quoted 174 lines

Also arch/arm/include/asm/arch-{s5pc1xx|exynos} code can be put to samsung-common directory (as also proposed in those patch series).

Frankly, we shall mimic the tegra or imx tree. As fair as I've noticed at those SoCs the common code has been put to various *-common directories.

I've added other Samsung boards maintainers to Cc.

Quoted 154 lines

Quoted 154 lines

arch/arm/include/asm/samsung-common/uart.h new file +23 -0

+   unsigned char   res3[0x3d0];
+#else /* Exynos 4 and 5 */
+   unsigned char   res3[0xffd0];
+#endif
+};
+
+
+static inline int s5p_uart_divslot(void)
+{
+#if defined(CONFIG_S5PC100) || defined(CONFIG_S5PC110)
+   return 1;
+#else /* Exynos 4 and 5 */
+   return 0;
+#endif
+}
+
+#define RX_FIFO_COUNT_MASK      0xff
+#define RX_FIFO_FULL_MASK       (1 << 8)
+#define TX_FIFO_FULL_MASK       (1 << 24)
+
+/* Information about a serial port */
+struct fdt_serial {
+   u32 base_addr;  /* address of registers in physical memory

*/

arch/arm/include/asm/samsung-common/uart.h new file +7 -0

+   u8 port_id;     /* uart port number */
+   u8 enabled;     /* 1 if enabled, 0 if disabled */
+};
+
+#endif     /* __ASSEMBLY__ */
+
+#endif

drivers/serial/Makefile +0 -0

index 697f2bb..e3ca49a 100644
@@ -19,7 +19,7 @@ COBJS-$(CONFIG_LPC32XX_HSUART) +=

lpc32xx_hsuart.o COBJS-$(CONFIG_MCFUART) += mcfuart.o
COBJS-$(CONFIG_OPENCORES_YANU) += opencores_yanu.o COBJS-$(CONFIG_SYS_NS16550) += ns16550.o

drivers/serial/Makefile +1 -1

-COBJS-$(CONFIG_S5P) += serial_s5p.o
+COBJS-$(CONFIG_S5P_SERIAL) += serial_s5p.o
 COBJS-$(CONFIG_SYS_NS16550_SERIAL) += serial_ns16550.o
 COBJS-$(CONFIG_IMX_SERIAL) += serial_imx.o
 COBJS-$(CONFIG_IXP_SERIAL) += serial_ixp.o

(quoted diff) +0 -0

b/drivers/serial/serial_s5p.c index f98b422..be08925 100644

drivers/serial/serial_s5p.c +2 -11

@@ -12,22 +12,13 @@
 #include <fdtdec.h>
 #include <linux/compiler.h>
 #include <asm/io.h>
-#include <asm/arch/uart.h>
+#include <asm/samsung-common/uart.h>
 #include <asm/arch/clk.h>
 #include <serial.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define RX_FIFO_COUNT_MASK 0xff
-#define RX_FIFO_FULL_MASK  (1 << 8)
-#define TX_FIFO_FULL_MASK  (1 << 24)
-
-/* Information about a serial port */
-struct fdt_serial {
-   u32 base_addr;  /* address of registers in physical memory
 */
-   u8 port_id;     /* uart port number */
-   u8 enabled;     /* 1 if enabled, 0 if disabled */
-} config __attribute__ ((section(".data")));
+struct fdt_serial config __attribute__ ((section(".data")));
 
 static inline struct s5p_uart *s5p_get_base_uart(int dev_index)

{

(quoted diff) +0 -0

b/include/configs/exynos5250-dt.h index 8f8f85f..a759d07 100644

include/configs/exynos5250-dt.h +1 -0

@@ -69,6 +69,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE + (4
 << 20))
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_BAUDRATE                    115200
 #define EXYNOS5_DEFAULT_UART_OFFSET        0x010000
 #define CONFIG_SILENT_CONSOLE

include/configs/origen.h +1 -0

index da13574..a59419d 100644
@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE + (1
 << 20))
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2                     1       /* use

SERIAL 2 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x020000

(quoted diff) +0 -0

b/include/configs/s5p_goni.h index d0fafd7..812b7f3 100644

include/configs/s5p_goni.h +1 -0

@@ -42,6 +42,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2                     1       /* use

SERIAL2 */ #define CONFIG_BAUDRATE 115200

(quoted diff) +0 -0

b/include/configs/s5pc210_universal.h index 97a4008..2270449 100644 --- a/include/configs/s5pc210_universal.h +++ b/include/configs/s5pc210_universal.h

include/configs/s5p_goni.h +1 -0

@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE + (1
 << 20))
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2             1       /* use SERIAL 2 */
 #define CONFIG_BAUDRATE            115200
 

(quoted diff) +0 -0

b/include/configs/smdkc100.h index a572e62..4631dac 100644

include/configs/smdkc100.h +1 -0

@@ -47,6 +47,7 @@
 /*
  * select serial console configuration
  */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL0                     1       /* use

SERIAL 0 on SMDKC100 */
/* PWM */

(quoted diff) +0 -0

b/include/configs/smdkv310.h index 0496661..9e10bf1 100644

include/configs/smdkv310.h +1 -0

@@ -48,6 +48,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE + (1
 << 20))
 /* select serial console configuration */
+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL1                     1       /* use

SERIAL 1 */ #define CONFIG_BAUDRATE 115200
#define EXYNOS4_DEFAULT_UART_OFFSET 0x010000

include/configs/trats.h +0 -0

index 9b6aac9..6b301c8 100644
@@ -53,6 +53,7 @@
 #define CONFIG_SYS_MALLOC_LEN              (CONFIG_ENV_SIZE +
 (16 << 20))
 /* select serial console configuration */

Quoted 8 lines

include/configs/trats.h line 59

Quoted 5 lines

include/configs/trats.h +1 -0

+#define CONFIG_S5P_SERIAL
 #define CONFIG_SERIAL2                     /* use SERIAL 2 */
 #define CONFIG_BAUDRATE                    115200
 

Thanks,
Minkyu Kang.

Quoted 4 lines

Regards,

Lukasz Majewski

Thanks,
Minkyu Kang.