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

[Minkyu Kang]

Dear Jean-Christophe

2009/9/11 Minkyu Kang <promsoft at gmail.com>:
> Dear Wolfgang,
>
> 2009/9/10 Wolfgang Denk <wd at denx.de>:
>> 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.
>
> agreed.
>
>>
>>> +#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?
>>
>
> I referenced the serial_s3c24x0.c and serial.c for multi serial api.
> so,..
> please let me know the correct way-
>
>>
>> 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
>> _______________________________________________
>> U-Boot mailing list
>> U-Boot at lists.denx.de
>> http://lists.denx.de/mailman/listinfo/u-boot
>>
>
>
>
> --
> from. prom.
> www.promsoft.net
>

Can you please comments for this patch?

Thanks
Minkyu Kang
-- 
from. prom.
www.promsoft.net