[U-Boot] serial ifdef mess

[Graeme Russ]

Hi Simon,

On Tue, Sep 20, 2011 at 2:28 PM, Simon Glass <sjg at chromium.org> wrote:
> 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:

[snip]

>>
>> 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?

Having a quick look via git web, no, SERIAL_MULTI does not look to add too
much

>
>> 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?

Nope - Some arches simply do not have the room for such a buffer. But for
boards that can implement a pre-console buffer, you can buffer until
environment is initialised and then output everything to the configured
serial port

> Serial init is the one peripheral still needed prior to reloc. At

Timers are also needed pre-reloc

> least this could be the default option, with something like
> CONFIG_EARLY_UART defined to revert to current behavior for debugging
> reasons.

Yes, I like this idea. Even when using the pre-console buffer, pre-
console output can still be sent to the 'early' UART so you get
real-time output on the serial port as U-Boot is booting. You can then
use your favourite serial terminal to look at the timing.

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

Agree - I think there is some pre-reloc output which could be moved to
post-reloc

> 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

The environment can specify which port is used for console and the baud
rate. So environement needs to be up before redirecting output through
SERIAL_MULTI

> 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...

That can be done via putc() in console.c - That is where I redirect to the
pre-console buffer prior to console init. It would be trivial to also add
an #ifdef CONFIG_EARLY_UART to redirect to early_uart_putc()

>> 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.

Yes, I hate them with a passion - I really dislike how all the serial
port permutations are hard-coded. The extern mess in /include/serial.h
really highlights the problem. And /drivers/serial/serial.c being so
16550 centric is so confusing when you also have /drivers/serial/ns16550.c
And then you have /common/serial.c...

So maybe we trash externs in /include/serial.h and move the implementation
of serial_initialize() out of /common/serial.c and into board/SoC files
by define a weak alias at SoC level so a board can still call it and
register additional serial ports on top. I think the initcall method I
put forward before can get around that more cleanly (YMMV)

And tidy up /drivers/serial/serial.c to not reference ns16550

> 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...

LOL - and mine is only just now dry from initcall :)


> 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

2