[U-Boot] serial_device member functions don't know their serial port

[U-Boot] serial_device member functions don't know their serial port

[Simon Glass]

Hi,

I am taking a look at serial ports in U-Boot and it seems that the various
functions in 'struct serial_device' do not include a pointer to the device
they are talking about. Therefore for devices with multiple ports there is
no way for these functions to know what to do.

The workaround in drivers/serial/serial.c (at least for NS16550) seems to be
to declare stub functions which have the port number built in:

/* Multi serial device functions */
#define DECLARE_ESERIAL_FUNCTIONS(port) \
    int  eserial##port##_init (void) {\
int clock_divisor; \
clock_divisor = calc_divisor(serial_ports[port-1]); \
NS16550_init(serial_ports[port-1], clock_divisor); \
return(0);}\
    void eserial##port##_setbrg (void) {\
serial_setbrg_dev(port);}\
    int  eserial##port##_getc (void) {\
return serial_getc_dev(port);}\
    int  eserial##port##_tstc (void) {\
return serial_tstc_dev(port);}\
    void eserial##port##_putc (const char c) {\
serial_putc_dev(port, c);}\
    void eserial##port##_puts (const char *s) {\
serial_puts_dev(port, s);}

...
DECLARE_ESERIAL_FUNCTIONS(1);
struct serial_device eserial1_device =
INIT_ESERIAL_STRUCTURE(1,"eserial0","EUART1");
DECLARE_ESERIAL_FUNCTIONS(2);
struct serial_device eserial2_device =
INIT_ESERIAL_STRUCTURE(2,"eserial1","EUART2");

Would it not be better to pass the port number (or better the serial_device
*) to each of the functions putc(), puts(), etc? What am I missing here?

Regards,
Simon

[U-Boot] serial_device member functions don't know their serial port

[Mike Frysinger]

On Thursday, May 12, 2011 20:35:39 Simon Glass wrote:
> I am taking a look at serial ports in U-Boot and it seems that the various
> functions in 'struct serial_device' do not include a pointer to the device
> they are talking about. Therefore for devices with multiple ports there is
> no way for these functions to know what to do.

yep

> The workaround in drivers/serial/serial.c (at least for NS16550) seems to
> be to declare stub functions which have the port number built in:

that tends to be how they all support multi instances.  it's how i recently 
implemented support for multiple Blackfin UARTs.

> Would it not be better to pass the port number (or better the serial_device
> *) to each of the functions putc(), puts(), etc? What am I missing here?

i'm assuming you mean only the func pointers and not the common api.  we 
wouldnt want general code to have to do this.

the device<->serial glue is awful.  you can only output to one serial device 
at a time due to the global serial_current pointer.  so if you set stdout to 
uart0 and then set stderr to uart1, both stdout and stderr will go to uart1.  
all silently and without warning of course.

i'd like to see the device<->serial glue thrown out, as well as the serial 
layer itself.  all the serial devices should simply be another stdio_dev.  
once we do that, we could update the stdio_dev api to pass a handle to its own 
stdio_dev->priv state to all of its func pointer.  this, ultimately, should 
satisfy your original query.

i'm guessing the serial_* funcs exist for historical/relocation purposes, but 
i'm not terribly familiar with either, so i dont know how viable it is to kill 
them off completely.  if we do want to keep them around, i think it'd be easy 
to provide common stubs on top of the stdio_dev api ...

a fairly large undertaking of code grind/shuffle, so i havent embarked on it, 
nor do i have plans to do so.  maybe you're more masochistic than i :).
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
Url : http://lists.denx.de/pipermail/u-boot/attachments/20110512/bbd6c820/attachment.pgp