On Wed, Sep 07, 2022 at 09:19:25AM +0200, 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). Sure, but you can't have that at the cost of something that is unreadable. Arnd's suggestions seems to improve things somewhat in this respect, though. > > [ And note that you're "saving" something like 20 lines per driver: > > It's not about saving, it's about deduplicating and unifying. It was you who brought the diff stat that up and I put this in parentheses for a reason. > > 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. Yeah, that was even worse so no need to reiterate that. > 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/ If that works then that should improve readability as well. Johan
