mbox series

[RFC,v2,0/2] serial: imx: Switch to nbcon console

Message ID cover.1712303358.git.esben@geanix.com
Headers show
Series serial: imx: Switch to nbcon console | expand

Message

Esben Haabendal April 5, 2024, 7:56 a.m. UTC 
This is a first attempt at porting another serial console driver to nbcon.

The first patch is a preparatory patch, thought to be suitable for mainline
inclusion now, while the second patch is for rt only for now.

The second patch does have quite a copy-paste feeling to it.

The imx_uart_console_write_atomic() and imx_uart_console_write_thread()
includes copies of code from the old imx_uart_console_write() function.
This is similar to current status of nbcon support in 8250 driver.

The midle part of imx_uart_console_write_thread() is identical to the
middle part of serial8250_console_write_thread() in
drivers/tty/serial/8250/8250_port.c, except for the arguments to
uart_console_write() function.

There is clearly potential for writing some common code to avoid both cases
of copy-paste code.

I am unsure about what the plan is for keeping the legacy console code in
the drivers. Are we going to drop that before submitting the code to
mainline?

v2:
- Switch to tight loop (no udelay()) in atomic context.
- Increase timeout to 1 second.
- Add note in commit message about (no) error handling on timeout.


Esben Haabendal (2):
  serial: imx: Introduce timeout when waiting on transmitter empty
  serial: imx: Switch to nbcon console

 drivers/tty/serial/imx.c | 161 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 157 insertions(+), 4 deletions(-)

Comments

Marc Kleine-Budde April 5, 2024, 8:43 a.m. UTC | #1 
On 05.04.2024 09:56:36, Esben Haabendal wrote:
> This is a first attempt at porting another serial console driver to nbcon.
> 
> The first patch is a preparatory patch, thought to be suitable for mainline
> inclusion now, while the second patch is for rt only for now.
> 
> The second patch does have quite a copy-paste feeling to it.
> 
> The imx_uart_console_write_atomic() and imx_uart_console_write_thread()
> includes copies of code from the old imx_uart_console_write() function.
> This is similar to current status of nbcon support in 8250 driver.
> 
> The midle part of imx_uart_console_write_thread() is identical to the
> middle part of serial8250_console_write_thread() in
> drivers/tty/serial/8250/8250_port.c, except for the arguments to
> uart_console_write() function.
> 
> There is clearly potential for writing some common code to avoid both cases
> of copy-paste code.
> 
> I am unsure about what the plan is for keeping the legacy console code in
> the drivers. Are we going to drop that before submitting the code to
> mainline?

Some additional thoughts:
- Code duplication should be avoided.
- You can remove the first "ifdef" if you mark the functions as __maybe_unused.
- Use get_maintainer.pl to address all relevant parties and discuss if
  you should keep the legacy console. You should do this next, as the
  above gets irrelevant if the legacy console can be removed.

regards,
Marc