Messages in this thread | | | Date | Fri, 2 Sep 2022 12:24:10 +0200 | Subject | Re: [PATCH v2 1/3] tty: serial: introduce transmit helper generators | From | Jiri Slaby <> |
| |
On 02. 09. 22, 12:22, Ilpo Järvinen wrote: > On Thu, 1 Sep 2022, Jiri Slaby wrote: > >> Many serial drivers do the same thing: >> * send x_char if set >> * keep sending from the xmit circular buffer until either >> - the loop reaches the end of the xmit buffer >> - TX is stopped >> - HW fifo is full >> * check for pending characters and: >> - wake up tty writers to fill for more data into xmit buffer >> - stop TX if there is nothing in the xmit buffer >> >> The only differences are: >> * how to write the character to the HW fifo >> * the check of the end condition: >> - is the HW fifo full? >> - is limit of the written characters reached? >> >> So unify the above into two helper generators: >> * DEFINE_UART_PORT_TX_HELPER_LIMITED() -- it performs the above taking >> the written characters limit into account, and >> * DEFINE_UART_PORT_TX_HELPER() -- the same as above, except it only >> checks the HW readiness, not the characters limit. >> >> The HW specific operations (as stated as "differences" above) are passed >> as arguments to the macros. They are: >> * tx_ready() -- returns true if HW can accept more data. >> * put_char() -- write a character to the device. >> * tx_done() -- when the write loop is done, perform arbitrary action >> before potential invocation of ops->stop_tx() happens. >> >> Note that the above macros are generators. This means the code is >> generated in place and the above 3 arguments are "inlined". I.e. no >> added penalty by generating call instructions for every single >> character. Nor any indirect calls. (As in previous versions of this >> patchset.) >> >> Signed-off-by: Jiri Slaby <jslaby@suse.cz> >> --- >> >> Notes: >> [v2] instead of a function (uart_port_tx_limit()) in serial_core, >> generate these in-place using macros. Thus eliminating "call" >> penalty. >> >> Documentation/driver-api/serial/driver.rst | 3 + >> include/linux/serial_core.h | 86 ++++++++++++++++++++++ >> 2 files changed, 89 insertions(+) >> >> diff --git a/Documentation/driver-api/serial/driver.rst b/Documentation/driver-api/serial/driver.rst >> index 23c6b956cd90..25775bf1fcc6 100644 >> --- a/Documentation/driver-api/serial/driver.rst >> +++ b/Documentation/driver-api/serial/driver.rst >> @@ -78,6 +78,9 @@ Other functions >> uart_get_lsr_info uart_handle_dcd_change uart_handle_cts_change >> uart_try_toggle_sysrq uart_get_console >> >> +.. kernel-doc:: include/linux/serial_core.h >> + :identifiers: DEFINE_UART_PORT_TX_HELPER_LIMITED DEFINE_UART_PORT_TX_HELPER >> + >> Other notes >> ----------- >> >> diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h >> index 6e4f4765d209..715778160ae1 100644 >> --- a/include/linux/serial_core.h >> +++ b/include/linux/serial_core.h >> @@ -646,6 +646,92 @@ struct uart_driver { >> >> void uart_write_wakeup(struct uart_port *port); >> >> +#define __DEFINE_UART_PORT_TX_HELPER(name, port, ch, tx_ready, put_char, \ >> + tx_done, for_test, for_post, ...) \ >> +unsigned int name(struct uart_port *port __VA_OPT__(,) __VA_ARGS__) \ >> +{ \ >> + struct circ_buf *xmit = &port->state->xmit; \ >> + unsigned int pending; \ >> + u8 ch; \ >> + \ >> + for (; (for_test) && (tx_ready); (for_post), port->icount.tx++) { \ > >> + * The functions in parameters shall be designed as follows: >> + * * **tx_ready(port):** the function shall return true if the HW can accept >> + * more data to be sent. This function can be %NULL, which means the HW is >> + * always ready. > > So if tx_ready can be NULL, how does that for() loop above work??
Sorry, the docs is wrong (corresponded to v1). I fixed it locally already:
https://git.kernel.org/pub/scm/linux/kernel/git/jirislaby/linux.git/commit/?h=devel&id=989c488c69faf2987648e09358c79d75b4a3b5c7
thanks, -- js suse labs
| |