[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4e9b4471-a6f2-4b16-d830-67d253ae4e6a@linux.intel.com>
Date: Wed, 7 Sep 2022 13:16:23 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Jiri Slaby <jirislaby@...nel.org>, Johan Hovold <johan@...nel.org>
cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
linux-serial <linux-serial@...r.kernel.org>,
LKML <linux-kernel@...r.kernel.org>,
Tobias Klauser <tklauser@...tanz.ch>,
Richard Genoud <richard.genoud@...il.com>,
Nicolas Ferre <nicolas.ferre@...rochip.com>,
Alexandre Belloni <alexandre.belloni@...tlin.com>,
Claudiu Beznea <claudiu.beznea@...rochip.com>,
Vladimir Zapolskiy <vz@...ia.com>,
Liviu Dudau <liviu.dudau@....com>,
Sudeep Holla <sudeep.holla@....com>,
Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
Shawn Guo <shawnguo@...nel.org>,
Sascha Hauer <s.hauer@...gutronix.de>,
Pengutronix Kernel Team <kernel@...gutronix.de>,
Fabio Estevam <festevam@...il.com>,
NXP Linux Team <linux-imx@....com>,
Andreas Färber <afaerber@...e.de>,
Manivannan Sadhasivam <mani@...nel.org>,
Russell King <linux@...linux.org.uk>,
Florian Fainelli <f.fainelli@...il.com>,
bcm-kernel-feedback-list@...adcom.com,
Pali Rohár <pali@...nel.org>,
Kevin Cernekee <cernekee@...il.com>,
Palmer Dabbelt <palmer@...belt.com>,
Paul Walmsley <paul.walmsley@...ive.com>,
Orson Zhai <orsonzhai@...il.com>,
Baolin Wang <baolin.wang7@...il.com>,
Chunyan Zhang <zhang.lyra@...il.com>,
Patrice Chotard <patrice.chotard@...s.st.com>,
linux-riscv@...ts.infradead.org
Subject: Re: [PATCH v3 0/4] tty: TX helpers
On Wed, 7 Sep 2022, Jiri Slaby wrote:
> 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@suse.cz/
I second Jiri here.
Saving lines in drivers is not that important compared with all removing
all the variants of the same thing that have crept there over the years.
I suspect the main reason for the variants is that everybody just used
other drivers as examples and therefore we've a few "main" variant
branches depending on which of the drivers was used as an example for the
other. That is hardly a good enough reason to keep them different and as
long as each driver keeps its own function for this, it will eventually
lead to similar differentiation so e.g. a one-time band-aid similarization
would not help in the long run.
Also, I don't understand why you see it unreadable when the actual code is
out in the open in that macro. It's formatted much better than e.g.
read_poll_timeout() if you want an example of something that is hardly
readable ;-). I agree though there's a learning-curve, albeit small, that
it actually creates a function but that doesn't seem to me as big of an
obstacle you seem to think.
--
i.
Powered by blists - more mailing lists