Re: RFC: Clarifying UART API


Paul Sokolovsky
 

Hello Daniel,

On Mon, 8 May 2017 14:32:56 +0100
Daniel Thompson <daniel.thompson@...> wrote:

[]

Interpretation 1:
"ready" means "ready to accept more chars to transmit", "empty"
means "transmission complete".

Interpretation 2:
"empty" means "there's empty room in TX FIFO (so more chars can be
written to it)", "ready" means "transmission complete".
Perhaps we should note that interpretation 1 is the "right" one in
the sense that this is what the current implementations of the API
currently approximate to.
Yes, I kinda implied that, and that's why I sent this RFC ahead of full
analysis of each and every existing drivers/serial/ case, but just
confirming on my side your previous suggestion to use uart_ns16550.c
driver as the reference.

I also view interpretation #2 is only possible when the reasoner
neglects the FIFO since when there is a FIFO we have to redefine the
normal meaning of empty[1] to mean not-full.
I don't agree here. It's very easy to get to that interpretation by
reading almost any hardware datasheet, even 16550 itself have things
called "Transmitter Holding Register Empty" and "Transmitter Empty",
the first meaning it can slurp more, while latter meaning it's done.
And yep, FIFO is always optional, so reasoning should start without it,
and then FIFO should be consistently integrated into it.

[]

I'm proposing to make the following changes to deconfuse it:

1. Given that there's a (sole) uart_irq_rx_ready() with the meaning
"there's a char ready to be received", it's natural to leave
uart_irq_tx_ready() as it is, and explicitly describe as "TX block
is ready to accept a char for a transmission".
Agree. Perhaps phrase as "accept at least one char for a
transmission".
Sure, there would be a patch review where the exact formulation cab be
polished.


Note a couple of existing drivers check their interrupt mask and only
admit to being ready if their interrupt is enabled. These drivers are
anomalous and should be fixed to meet the above anyway.
I kind of agree that it would be almost unrealistic to enforce behavior
like "if irq_rx_disable() was called, then irq_rx_ready() should always
return false". Then, any driver which so far does that in software can
be made few bytes shorter and few cycles faster by removing the ANDing
stuff.

[]

Similarly I agree that uart_irq_tx_complete() is a useful addition to
the API.

Sorry to make you read to far just to find out that this RFC gets a
+1 from me...
I definitely appreciate detailed multi-faceted discussion of the matter,
though with all discussion we already had in previous mails, in
tickets, on IRC, I wonder if sometimes we lose track that the top-level
goal of this effort is improving Zephyr's console subsystem, not just
speculative research in UART "zoology" ;-).

Thanks!


Daniel.


--
Best Regards,
Paul

Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://www.facebook.com/pages/Linaro
http://twitter.com/#!/linaroorg - http://www.linaro.org/linaro-blog

Join devel@lists.zephyrproject.org to automatically receive all group messages.