Dear Jean-Christophe PLAGNIOL-VILLARD,
In message <[email protected]> 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 <[email protected]>
> ---
> 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: [email protected]
A witty saying proves nothing, but saying something pointless gets
people's attention.
_______________________________________________
U-Boot mailing list
[email protected]
http://lists.denx.de/mailman/listinfo/u-boot
