Re: [PATCH v3 0/4] tty: TX helpers

[Jiri Slaby]

On 06. 09. 22, 13:30, Johan Hovold wrote:
> On Tue, Sep 06, 2022 at 12:48:01PM +0200, Jiri Slaby wrote:
> > This series introduces DEFINE_UART_PORT_TX_HELPER +
> > DEFINE_UART_PORT_TX_HELPER_LIMITED TX helpers. See PATCH 2/4 for the
> > details. Comments welcome.
> >
> > Then it switches drivers to use them. First, to
> > DEFINE_UART_PORT_TX_HELPER() in 3/4 and then
> > DEFINE_UART_PORT_TX_HELPER_LIMITED() in 4/4.
> >
> > The diffstat of patches 3+4 is as follows:
> >   26 files changed, 191 insertions(+), 823 deletions(-)
> > which appears to be nice.
>
> Not really. This is horrid. Quality can't be measured in LoC (only).
>
> The resulting code is unreadable. And for no good reason.

IMO, it's much more readable than the original ~ 30 various (and buggy 
-- see Ilpo's fixes) copies of this code. Apart from that, it makes 
further rework much easier (I have switch to kfifo in my mind for example).

> [ And note that you're "saving" something like 20 lines per driver:

It's not about saving, it's about deduplicating and unifying.

> 	 12 files changed, 84 insertions(+), 349 deletions(-)
> ]
>
> NAK

I'd love to come up with something nicer. That would be a function in 
serial-core calling hooks like I had [1] for example. But provided all 
those CPU workarounds/thunks, it'd be quite expensive to call two 
functions per character.

Or creating a static inline (having ± the macro content) and the hooks 
as parameters and hope for optimizations to eliminate thunks (also 
suggested in the past [1]).

[1] https://lore.kernel.org/all/20220411105405.9519-1-jslaby@xxxxxxx/

thanks,
--
js
suse labs