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

From: Greg Kroah-Hartman
Date: Fri Sep 09 2022 - 07:06:21 EST


On Fri, Sep 09, 2022 at 12:53:04PM +0200, Jiri Slaby wrote:
> On 07. 09. 22, 16:56, Arnd Bergmann wrote:
> > On Wed, Sep 7, 2022, at 3:52 PM, Russell King (Oracle) wrote:
> > > On Wed, Sep 07, 2022 at 02:36:37PM +0200, Greg Kroah-Hartman wrote:
> > >
> > > Of course, it would have been nicer to see the definition of this
> > > macro, because then we can understand what the "ch" argument is to
> > > this macro, and how that relates to the macro argument that is
> > > shown in the example as a writel().
> >
> > I pulled out the 'ch' variable from the macro to avoid having
> > the macro define local variables that are then passed to the
> > inner expressions.
>
> Note that I had "port" and "ch" as a part of the macro parameters in [v2],
> but it didn't help the situation much.
> > > Maybe a more complete example would help clear up the confusion?
> > > Arnd?
> >
> > Here is a patch on top of the series that would implement the
> > uart_port_tx_helper_limited() and uart_port_tx_helper()
> > macros that can be used directly from drivers in place of defining
> > local functions, with the (alphabetically) first two drivers
> > converted to that.
>
> If there are no objections, I will push the patches this directorin. I like
> this more than [v2] or [v3] (the helper macros). Actually, I mentioned this
> wait_event() style in [v1], but I perhaps simplified the concept too much to
> completely eliminate the need of a wrapper function. And that made it too
> complicated/too hard to understand.
>
> Except I'd drop the "_helper" part from the name. Originally (in [v1]), I
> had uart_port_tx() and uart_port_tx_limited() functions. In [v2+v3], I added
> _helper to avoid confusion as we were generating a helpers using the macros.
> Yes, technically, uart_port_tx() is still a helper, but I think it's
> superfluous to have it in the name now.

No objection from me, thanks for doing this work!

greg k-h