[U-Boot] serial ifdef mess

[Simon Glass]

Hi Graeme & Mike,

On Mon, Sep 19, 2011 at 6:07 PM, Graeme Russ <graeme.russ at gmail.com> wrote:
> Hi Mike,
>
> On Tue, Sep 20, 2011 at 10:52 AM, Mike Frysinger <vapier at gentoo.org> wrote:
>> On Monday, September 19, 2011 20:41:20 Graeme Russ wrote:
>>> Hi Mike,
>>>
>>> On Tue, Sep 20, 2011 at 6:57 AM, Mike Frysinger <vapier at gentoo.org> wrote:
>>> > On Sunday, September 18, 2011 09:08:35 Graeme Russ wrote:
>>> >> I mean, it irks me to no end that /common/serial.c, /drivers/serial.c
>>> >> and /include/serial.h are such an ugly mess of #ifdef's - I'm working
>>> >> on a new SoC at the moment, and it just plain weird that I have to
>>> >> touch these :(
>>> >
>>> > well, there's two things there.  the init mess which could get fixed in
>>> > two diff ways: part of your larger init cleanup, or turn it into board
>>> > callbacks like most of the other frameworks we have atm.
>>>
>>> I don't think the serial mess is related to the init sequence at all (but
>>> I could be wrong)
>>
>> the only way to register a new serial device is to add a call to it in
>> common/serial.c:serial_initialize().  and the only thing that func does is
>> call all the various register funcs which are simply init calls.
>
> Ah, I see - I think I see an architectural thunk! about to happen ;)
>
>>> > the second thing is the CONFIG_SERIAL_MULTI hell.  that mess i'd like to
>>> > gut with a dull blade at some point.
>>>
>>> Or a sledgehammer!
>>>
>>> The big question I suppose is where we are at with the _MULTI interfaces.
>>> From what I can gather, these should now be the only ones in use and we
>>> should start to apply pressure on board maintainers (i.e. break their
>>> boards) to fix any depricated usage. I think the same philosophy should
>>> be applied to the various boards with 'flash.c' which should all be
>>> using CFI by now.
>>
>> i dont have a problem with non-multi instances since it produces thinner code
>>
>> i dont think there is anyone driving the serial core atm though ... i dont
>> recall seeing any patches there other than new device drivers since ive been
>> watching the list ...
>
> Hmmm, so let me ponder (keep in mind I am not familiar with the serial code
> and do not have it in front of me)...
>
> I can see two implementations that a board could decide to use
>
> a) Use a particular serial driver directly - perfect if you have only one
>   serial port (or don't care about the others)

Do we really want this? Is the code overhead of SERIAL_MULTI so bad
that people insist on not defining it? If so, can we reduce that code
overhead? It doesn't seem like it should be large. Perhaps non-MULTI
could be implemented by macros and inline functions in the header
file?

> b) Use the SERIAL_MULTI 'management layer' and 'register' each relavent
>   serial port on the board. The board will need to define a (probably
>   hard-coded) a default to handle I/O until the environment can be read
>   and the hardware initialised to actually make the serial ports
>   operational.

Prior to relocation there is a single console UART. I wonder whether
it would be acceptable to buffer all output until after relocation?
Serial init is the one peripheral still needed prior to reloc. At
least this could be the default option, with something like
CONFIG_EARLY_UART defined to revert to current behavior for debugging
reasons.

Slightly more radical: just move the U-Boot banner, etc. into
board_init_r. What could possible go wrong?

?

But to truly deal with your point, I don't think the environment tells
the serial ports much. They get their registers address and settings
from other places (normally hard-coded in the driver, perhaps in
future through an fdt). It should be possible for any serial driver to
output things before the env is loaded. To implement an early UART we
simply need the serial layer to pass serial calls straight onto the
selected driver without going through the mux or anything else that
needs state. That bit already works...

>
> So in theory, we should be able to register an arbitrary number of serial
> ports, each with potentially different hardware and therefore different
> drivers. The board (or SoC) init function should be able to simply call
> serial_register() for each serial port with a name and info into how to
> talk to the hardware (hardware type, base address etc). The SERIAL_MULTI
> framework should then simply manage the list of serial devices and
> redirect I/O based on environment settings

The main bugbear for me is that there is no handle passed to the
serial functions. All of the serial devices should have an extra
parameter at the start which is perhaps struct seral_device *, and
that structure should have a ->priv member, pointing to a
driver-specific structure which we can
 use to store things like the port number / register address. So in
other words make it like most other driver layers in U-Boot.

This would clean up the eserial macros for one thing.

I don't think this is a huge job. What is a huge job is managing the
resulting centithread, laundry costs for the asbestos suit, falling
back to the status quo in stages, and Mike's mention of a dull gutting
blade...

Perhaps we can avoid that by finding out if there is a real consensus
on the list for a jolly good refactoring in the U-Boot serial code.
I'll start the count:

1

Regards,
Simon

>
> Regards,
>
> Graeme
> _______________________________________________
> U-Boot mailing list
> U-Boot at lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot
>