Messages in this thread | | | Date | Fri, 9 Sep 2022 12:53:04 +0200 | Subject | Re: [PATCH v3 0/4] tty: TX helpers | From | Jiri Slaby <> |
| |
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.
thanks, -- js suse labs
| |