[U-Boot] [PATCH] include/ns16550.h: Unify structure declaration for registers

Detlev Zundel dzu at denx.de
Mon May 4 17:40:37 CEST 2009


Hi Shinya,

>>>> I see.  Actually I was looking a lot at the Linux driver but was hoping
>>>> that we could away without introducing serial_{in,out}...
>>> In my horrible opinion, the combinations of base addres + reg_shift
>>> + iotype (char, long, or whatever), are simpler, more configurable,
>>> more slid, easy to use, than what we used to have or what you
>>> consolidated this time.
>>
>> You lost me here.
>>
>> You truly consider
>>
>> static unsigned int serial_in(struct uart_8250_port *up, int offset)
> [snip]
>> }
>>
>> to be "simpler and more solid" readb(struct->field) (which is
>> effectively what we have in the current implementation)?  You consider
>> "more configurable" to be a good in its own?
>
> Yes.

Wow.  As a rhetorical question - where do you actually draw the line if
you consider configurable to be a good in its own?  Shouldn't we then
have configuration options for UARTs who are attached bit-reversed on
the databus also?  And an option for a bit-shift in the data itself?

>> If your answers to these questions are yes, then we have different ideas
>> of writing code.
>
> Please make sure we don't need full serial_{in,out} port from Linux
> as-is.  As suggested, the combinations of base addres + reg_shift +
> iotype, are rather reasonable to support various kind of hardwares.
>
> I mean we need something like this:
>
> static unsigned int serial_in(struct uart_8250_port *up, int offset)
> {
>        unsigned int tmp;
>        int ret;
>        offset = map_8250_in_reg(up, offset) << up->port.regshift;
>
>        switch (up->port.iotype) {
>        case UPIO_MEM:
>                ret = readb(up->port.membase + offset);
>                break;
>
>        case UPIO_MEM32:
>                ret = readl(up->port.membase + offset);
>                break;
>
>        default:
>                ret = inb(up->port.iobase + offset);
>                break;
>        }
>        return ret;
> }
>
> Its implementation must be differed in U-Boot code, of course.

[...]
> I admit the address decoder in my UART hardware is weird and needs
> special configuration,  but this is not just for my case, it's not
> unusual.
>
> There're various kind of hardwares in the world, and there're many
> U-Boot ports which can not be pushed to upstream for various reasons.
> We can easily ignore such boards of course, but it would be very nice
> for U-Boot if it could provide easy configurable drivers and could
> support as many hardwares as possible.

Currently it seems that all in-tree boards can be accomodated with the
construct that I suggested.  I am not at all sure that we want code
which is only used by out-of-tree ports.

Post the port and we can rediscuss new code.

Cheers
  Detlev

-- 
[Linux] USB consoles was a  bad hack written on a drunken dare.   I'm still
constantly amazed that the thing even works at all, let alone the fact that
people are actually using it :) 
                            -- Greg KH <20090420225358.GC28697 at kroah.com>
--
DENX Software Engineering GmbH,      MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich,  Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-40 Fax: (+49)-8142-66989-80 Email: dzu at denx.de


More information about the U-Boot mailing list