[U-Boot] [RFC PATCH 1/3] serial: join stdio_dev and serial_device

[Wolfgang Denk]

Dear Jean-Christophe PLAGNIOL-VILLARD,

In message <1251034019-10787-1-git-send-email-plagnioj at jcrosoft.com> you wrote:
> we use for the serail multi api the struct stdio_dev also which will reduce
> and simplify the code and prepare the join of all serial APIs.
> 
> Signed-off-by: Jean-Christophe PLAGNIOL-VILLARD <plagnioj at jcrosoft.com>
> ---
>  board/esd/pmc440/pmc440.c        |    2 +-
>  board/lwmon/lwmon.c              |    2 +-
>  board/omap3/zoom2/zoom2_serial.h |   18 +++---
>  board/trizepsiv/conxs.c          |    8 ++--
>  common/serial.c                  |   89 +++++++++++++++----------------
>  common/stdio.c                   |    3 -
>  cpu/mpc5xxx/serial.c             |   36 ++++++------
>  cpu/mpc8xx/serial.c              |   36 ++++++------
>  cpu/ppc4xx/4xx_uart.c            |   38 +++++++-------
>  drivers/serial/serial.c          |   24 ++++----
>  drivers/serial/serial_pxa.c      |   54 +++++++++---------
>  drivers/serial/serial_s3c24x0.c  |   22 ++++----
>  include/serial.h                 |  109 ++++++++++++++++---------------------
>  include/stdio_dev.h              |    9 +++-
>  14 files changed, 218 insertions(+), 232 deletions(-)
>  rewrite include/serial.h (65%)
> 
> diff --git a/board/esd/pmc440/pmc440.c b/board/esd/pmc440/pmc440.c
> index 9ffb08e..de01e93 100644
> --- a/board/esd/pmc440/pmc440.c
> +++ b/board/esd/pmc440/pmc440.c
> @@ -53,7 +53,7 @@ int is_monarch(void);
>  int bootstrap_eeprom_read(unsigned dev_addr, unsigned offset,
>  			  uchar *buffer, unsigned cnt);
>  
> -struct serial_device *default_serial_console(void)
> +struct stdio_dev *default_serial_console(void)


I don't think the renaming of "serial" into "stdio" is a good idea.

"stdio" is a more specific term, which can only be applied to the
standard input/output channes (sdin, stdout and stderr); in other
words, this is restricted to the serial ports used as console
interface.

However, "serial" is a more general term - in addition to the stdio
devices this may be used for example for additional serial ports
controlling other things.


...
> -int serial_register (struct serial_device *dev)
> +int serial_register (struct stdio_dev *dev)
>  {
> -	dev->init += gd->reloc_off;
> +	dev->flags = DEV_FLAGS_OUTPUT | DEV_FLAGS_INPUT | DEV_FLAGS_SERIAL;

Please see comment below about DEV_FLAGS_SERIAL.

> -void serial_stdio_init (void)
> +static struct stdio_dev* serial_get_by_name(char* name)
>  {
> -	struct stdio_dev dev;
> -	struct serial_device *s = serial_devices;
> +	struct stdio_dev *dev = stdio_get_by_name(name);

Here it becomes clear that your renaming of serial into stdio is not
consistent; to me it makes no sense.

Why is it serial_get_by_name() [note: "serial"], when it actually
returns a "struct stdio_dev*"? 

You call stdio_get_by_name() internally? 

That does not amke sense. How can you get a device from a wider class
(serial devices), when you consider only a smaller selection (stio
devices)?

Hmmm... What is a "stdio device" by the way?

When we use a USB keyboard and assign it as stdin, it is a stdio
device, right? when we assing a LCD outpout as stdout and stderr, it
is a stdio device, right?

> --- a/include/stdio_dev.h
> +++ b/include/stdio_dev.h
> @@ -32,14 +32,19 @@
>  
>  #define DEV_FLAGS_INPUT	 0x00000001	/* Device can be used as input	console */
>  #define DEV_FLAGS_OUTPUT 0x00000002	/* Device can be used as output console */
> +#define DEV_FLAGS_SERIAL 0x00000003	/* Device is a serial device		*/

No.

This makes no sense at all. DEV_FLAGS_* are bit names.
DEV_FLAGS_SERIAL migfht be 4, but in no case it makes sense to use 3
here, which is "DEV_FLAGS_INPUT | DEV_FLAGS_OUTPUT".

>  /* Device information */
>  struct stdio_dev {
>  	int	flags;			/* Device flags: input/output/system	*/
>  	int	ext;			/* Supported extensions			*/
> -	char	name[16];		/* Device name				*/
> +	char	name[NAMESIZE];		/* Device name				*/
> +	char	ctlr[CTLRSIZE];

Comment? What's ctlr[CTLRSIZE] supposed to mean?


NAK.

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
A witty saying proves nothing, but saying  something  pointless  gets
people's attention.