[U-Boot] [PATCH 3/4 v2] s5pc1xx: support serial driver

[Wolfgang Denk]

Dear Minkyu Kang,

In message <4AA8AC42.50406 at samsung.com> you wrote:
> This patch includes the serial driver for s5pc1xx
> 
> Signed-off-by: Minkyu Kang <mk7.kang at samsung.com>
...
> +static inline struct s5pc1xx_uart *s5pc1xx_get_base_uart(int dev_index)
> +{
> +	u32 offset = dev_index * 0x400;
> +
> +	if (cpu_is_s5pc100())
> +		return (struct s5pc1xx_uart *)(S5PC100_PA_UART + offset);
> +	else
> +		return (struct s5pc1xx_uart *)(S5PC110_PA_UART + offset);

Please do not hard-wire the size of the struct s5pc1xx_uart here; use
sizeof() instead of the magic constant 0x400.

> +#if defined(CONFIG_SERIAL_MULTI)
> +static inline void
> +serial_setbrg_dev(unsigned int dev_index)
> +{
> +	_serial_setbrg(dev_index);
> +}
> +#else
> +void serial_setbrg(void)
> +{
> +	_serial_setbrg(UART_NR);
> +}
> +#endif

Why do we need these basicly empty wrapper functions?

> +#if !defined(CONFIG_SERIAL_MULTI)
> +int serial_init(void)
> +{
> +	return serial_init_dev(UART_NR);
> +}
> +#endif
...
> +#if defined(CONFIG_SERIAL_MULTI)
> +static inline int serial_getc_dev(unsigned int dev_index)
> +{
> +	return _serial_getc(dev_index);
> +}
> +#else
> +int serial_getc(void)
> +{
> +	return _serial_getc(UART_NR);
> +}
> +#endif
...
> +#if defined(CONFIG_SERIAL_MULTI)
> +static inline void serial_putc_dev(unsigned int dev_index, const char c)
> +{
> +	_serial_putc(c, dev_index);
> +}
> +#else
> +void serial_putc(const char c)
> +{
> +	_serial_putc(c, UART_NR);
> +}
> +#endif
...
> +#if defined(CONFIG_SERIAL_MULTI)
> +static inline int serial_tstc_dev(unsigned int dev_index)
> +{
> +	return _serial_tstc(dev_index);
> +}
> +#else
> +int serial_tstc(void)
> +{
> +	return _serial_tstc(UART_NR);
> +}
> +#endif
...
> +#if defined(CONFIG_SERIAL_MULTI)
> +static inline void serial_puts_dev(int dev_index, const char *s)
> +{
> +	_serial_puts(s, dev_index);
> +}
> +#else
> +void serial_puts(const char *s)
> +{
> +	_serial_puts(s, UART_NR);
> +}
> +#endif

Do we really need all this?


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Don't tell me how hard you work.  Tell me how much you get done.
- James J. Ling