[U-Boot] [PATCH v3 1/3] arm:samsung:serial Extract common UART code

Minkyu Kang mk7.kang at samsung.com
Wed Aug 28 12:11:33 CEST 2013


Dear Lukasz,

On 13/08/13 06:15, Lukasz Majewski wrote:
> 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 <l.majewski at majess.pl>
> 
> ---
> 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
> 
> diff --git a/arch/arm/include/asm/arch-exynos/uart.h b/arch/arm/include/asm/arch-exynos/uart.h
> deleted file mode 100644
> index 33d6ba3..0000000
> --- a/arch/arm/include/asm/arch-exynos/uart.h
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -/*
> - * (C) Copyright 2009 Samsung Electronics
> - * Minkyu Kang <mk7.kang at samsung.com>
> - * Heungjun Kim <riverful.kim at samsung.com>
> - *
> - * 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
> diff --git a/arch/arm/include/asm/arch-s5pc1xx/uart.h b/arch/arm/include/asm/arch-s5pc1xx/uart.h
> deleted file mode 100644
> index 26db098..0000000
> --- a/arch/arm/include/asm/arch-s5pc1xx/uart.h
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -/*
> - * (C) Copyright 2009 Samsung Electronics
> - * Minkyu Kang <mk7.kang at samsung.com>
> - * Heungjun Kim <riverful.kim at samsung.com>
> - *
> - * 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
> diff --git a/arch/arm/include/asm/samsung-common/uart.h b/arch/arm/include/asm/samsung-common/uart.h
> new file mode 100644
> index 0000000..ce92399
> --- /dev/null
> +++ b/arch/arm/include/asm/samsung-common/uart.h
> @@ -0,0 +1,64 @@
> +/*
> + * (C) Copyright 2009 Samsung Electronics
> + * Minkyu Kang <mk7.kang at samsung.com>
> + * Heungjun Kim <riverful.kim at samsung.com>
> + *
> + * 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)

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.

> +	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
> diff --git a/drivers/serial/Makefile b/drivers/serial/Makefile
> index 697f2bb..e3ca49a 100644
> --- a/drivers/serial/Makefile
> +++ b/drivers/serial/Makefile
> @@ -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
> diff --git a/drivers/serial/serial_s5p.c b/drivers/serial/serial_s5p.c
> index f98b422..be08925 100644
> --- a/drivers/serial/serial_s5p.c
> +++ b/drivers/serial/serial_s5p.c
> @@ -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)
>  {
> diff --git a/include/configs/exynos5250-dt.h b/include/configs/exynos5250-dt.h
> index 8f8f85f..a759d07 100644
> --- a/include/configs/exynos5250-dt.h
> +++ b/include/configs/exynos5250-dt.h
> @@ -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
> diff --git a/include/configs/origen.h b/include/configs/origen.h
> index da13574..a59419d 100644
> --- a/include/configs/origen.h
> +++ b/include/configs/origen.h
> @@ -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
> diff --git a/include/configs/s5p_goni.h b/include/configs/s5p_goni.h
> index d0fafd7..812b7f3 100644
> --- a/include/configs/s5p_goni.h
> +++ b/include/configs/s5p_goni.h
> @@ -42,6 +42,7 @@
>  /*
>   * select serial console configuration
>   */
> +#define CONFIG_S5P_SERIAL
>  #define CONFIG_SERIAL2			1	/* use SERIAL2 */
>  #define CONFIG_BAUDRATE			115200
>  
> diff --git a/include/configs/s5pc210_universal.h b/include/configs/s5pc210_universal.h
> index 97a4008..2270449 100644
> --- a/include/configs/s5pc210_universal.h
> +++ b/include/configs/s5pc210_universal.h
> @@ -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
>  
> diff --git a/include/configs/smdkc100.h b/include/configs/smdkc100.h
> index a572e62..4631dac 100644
> --- a/include/configs/smdkc100.h
> +++ b/include/configs/smdkc100.h
> @@ -47,6 +47,7 @@
>  /*
>   * select serial console configuration
>   */
> +#define CONFIG_S5P_SERIAL
>  #define CONFIG_SERIAL0			1	/* use SERIAL 0 on SMDKC100 */
>  
>  /* PWM */
> diff --git a/include/configs/smdkv310.h b/include/configs/smdkv310.h
> index 0496661..9e10bf1 100644
> --- a/include/configs/smdkv310.h
> +++ b/include/configs/smdkv310.h
> @@ -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
> diff --git a/include/configs/trats.h b/include/configs/trats.h
> index 9b6aac9..6b301c8 100644
> --- a/include/configs/trats.h
> +++ b/include/configs/trats.h
> @@ -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
>  
> 

Thanks,
Minkyu Kang.


More information about the U-Boot mailing list