With PM runtime enabled for the serial port controllers, we can now flush pending TX for the port on runtime PM resume as suggested by Johan Hovold <johan@xxxxxxxxxx>. To flush the pending TX, let's set up each port as a proper device as suggested by Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>. We set up each port as a child device for the serial port controller device. We use platform device for this and pass the port information in platform_data. Let's just do mimimal changes needed for now, more port specific code can be then moved from serial_core.c to serial_port.c as needed. Suggested-by: Johan Hovold <johan@xxxxxxxxxx> Suggested-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> Signed-off-by: Tony Lindgren <tony@xxxxxxxxxxx> --- drivers/tty/serial/Makefile | 2 +- drivers/tty/serial/serial_core.c | 77 ++++++++++++++++++++- drivers/tty/serial/serial_port.c | 111 +++++++++++++++++++++++++++++++ drivers/tty/serial/serial_port.h | 16 +++++ include/linux/serial_core.h | 1 + 5 files changed, 204 insertions(+), 3 deletions(-) create mode 100644 drivers/tty/serial/serial_port.c create mode 100644 drivers/tty/serial/serial_port.h diff --git a/drivers/tty/serial/Makefile b/drivers/tty/serial/Makefile --- a/drivers/tty/serial/Makefile +++ b/drivers/tty/serial/Makefile @@ -3,7 +3,7 @@ # Makefile for the kernel serial device drivers. # -obj-$(CONFIG_SERIAL_CORE) += serial_core.o +obj-$(CONFIG_SERIAL_CORE) += serial_core.o serial_port.o obj-$(CONFIG_SERIAL_EARLYCON) += earlycon.o obj-$(CONFIG_SERIAL_EARLYCON_ARM_SEMIHOST) += earlycon-arm-semihost.o diff --git a/drivers/tty/serial/serial_core.c b/drivers/tty/serial/serial_core.c --- a/drivers/tty/serial/serial_core.c +++ b/drivers/tty/serial/serial_core.c @@ -16,6 +16,7 @@ #include <linux/console.h> #include <linux/gpio/consumer.h> #include <linux/of.h> +#include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/proc_fs.h> #include <linux/seq_file.h> @@ -31,6 +32,8 @@ #include <linux/irq.h> #include <linux/uaccess.h> +#include "serial_port.h" + /* * Serial port device specific data for serial core. * @@ -151,9 +154,31 @@ static void __uart_start(struct tty_struct *tty) { struct uart_state *state = tty->driver_data; struct uart_port *port = state->uart_port; + struct device *port_dev; + int err; + + if (!port || uart_tx_stopped(port)) + return; - if (port && !uart_tx_stopped(port)) + port_dev = port->state->port_dev; + + err = pm_runtime_get(port_dev); + if (err < 0) { + /* Something went wrong, attempt to start TX anyways */ + port->ops->start_tx(port); + pm_runtime_put_noidle(port_dev); + return; + } + + /* + * Start TX if enabled, and kick runtime PM. Otherwise we must + * wait for a retry. See also serial_port.c for runtime PM + * autosuspend timeout. + */ + if (pm_runtime_active(port_dev)) port->ops->start_tx(port); + pm_runtime_mark_last_busy(port_dev); + pm_runtime_put_autosuspend(port_dev); } static void uart_start(struct tty_struct *tty) @@ -288,6 +313,37 @@ serial_core_find_controller(struct uart_driver *drv, struct device *dev) return NULL; } +static int serial_core_add_port_device(struct uart_port *port) +{ + struct serial_port_platdata pd; + struct platform_device *pdev; + int ret; + + pdev = platform_device_alloc("serial-port", PLATFORM_DEVID_AUTO); + if (!pdev) + return -ENOMEM; + + pdev->dev.parent = port->dev; + pd.state = port->state; + + ret = platform_device_add_data(pdev, &pd, sizeof(pd)); + if (ret) + goto err_put; + + ret = platform_device_add(pdev); + if (ret) + goto err_put; + + port->state->port_dev = &pdev->dev; + + return 0; + +err_put: + platform_device_put(pdev); + + return ret; +} + /* * Initialize a serial port device and serial port controller as * needed. Called from uart_add_one_port() with port_mutex held. @@ -317,13 +373,21 @@ static int serial_core_register_port(struct uart_port *port, port->state->controller = controller; WARN_ON(port->supports_autosuspend != controller->supports_autosuspend); + ret = serial_core_add_port_device(port); + if (ret) + goto err_free; + ret = serial_core_pm_runtime_start(port); if (ret < 0) - goto err_free; + goto err_del; return 0; +err_del: + platform_device_del(to_platform_device(port->state->port_dev)); + err_free: + port->state->port_dev = NULL; port->state->controller = NULL; if (allocated) kfree(controller); @@ -339,11 +403,14 @@ static int serial_core_register_port(struct uart_port *port, static void serial_core_unregister_port(struct uart_port *port) { struct serial_controller *controller = to_controller(port); + struct device *dev = port->state->port_dev; /* Check for a registered controller, no struct device early on */ if (!controller) return; + platform_device_del(to_platform_device(dev)); + port->state->port_dev = NULL; port->state->controller = NULL; kref_put(&controller->ref, serial_core_pm_runtime_cleanup); } @@ -363,6 +430,10 @@ static int uart_port_startup(struct tty_struct *tty, struct uart_state *state, if (uport->type == PORT_UNKNOWN) return 1; + retval = serial_port_get(state); + if (retval) + return retval; + /* * Make sure the device is in D0 state. */ @@ -2030,6 +2101,8 @@ static void uart_port_shutdown(struct tty_port *port) /* Ensure that the IRQ handler isn't running on another CPU. */ synchronize_irq(uport->irq); } + + serial_port_put(state); } static int uart_carrier_raised(struct tty_port *port) diff --git a/drivers/tty/serial/serial_port.c b/drivers/tty/serial/serial_port.c new file mode 100644 --- /dev/null +++ b/drivers/tty/serial/serial_port.c @@ -0,0 +1,111 @@ +// SPDX-License-Identifier: GPL-2.0-or-later + +/* + * Serial port driver to provide port specific services for serial_core + */ + +#include <linux/module.h> +#include <linux/platform_device.h> +#include <linux/pm_runtime.h> +#include <linux/serial_core.h> + +#include "serial_port.h" + +#define SERIAL_PORT_AUTOSUSPEND_DELAY_MS 500 + +struct serial_port_data { + struct uart_state *state; +}; + +int serial_port_get(struct uart_state *state) +{ + if (!state) + return -ENODEV; + + /* Prevent uart_port from unloading */ + if (!state->port_dev || !state->port_dev->driver || + !try_module_get(state->port_dev->driver->owner)) + return -ENODEV; + + return 0; +} +EXPORT_SYMBOL(serial_port_get); + +void serial_port_put(struct uart_state *state) +{ + if (!state || !state->port_dev || !state->port_dev->driver) + return; + + module_put(state->port_dev->driver->owner); +} +EXPORT_SYMBOL(serial_port_put); + +/* Only considers TX for now. Caller must take care of locking */ +static int __serial_port_busy(struct uart_port *port) +{ + return (!uart_tx_stopped(port) && + uart_circ_chars_pending(&port->state->xmit)); +} + +static int serial_port_runtime_resume(struct device *dev) +{ + struct serial_port_data *ddata = dev_get_drvdata(dev); + struct uart_port *port = ddata->state->uart_port; + unsigned long flags; + + /* Flush any pending TX for the port */ + spin_lock_irqsave(&port->lock, flags); + if (__serial_port_busy(port)) + port->ops->start_tx(port); + spin_unlock_irqrestore(&port->lock, flags); + pm_runtime_mark_last_busy(dev); + + return 0; +} + +static const struct dev_pm_ops serial_port_pm = { + SET_RUNTIME_PM_OPS(NULL, serial_port_runtime_resume, NULL) +}; + +static int serial_port_probe(struct platform_device *pdev) +{ + struct serial_port_platdata *pd = dev_get_platdata(&pdev->dev); + struct serial_port_data *ddata; + + ddata = devm_kzalloc(&pdev->dev, sizeof(*ddata), GFP_KERNEL); + if (!ddata) + return -ENOMEM; + + ddata->state = pd->state; + platform_set_drvdata(pdev, ddata); + + pm_runtime_enable(&pdev->dev); + pm_runtime_set_autosuspend_delay(&pdev->dev, + SERIAL_PORT_AUTOSUSPEND_DELAY_MS); + pm_runtime_use_autosuspend(&pdev->dev); + + return 0; +} + +static int serial_port_remove(struct platform_device *pdev) +{ + pm_runtime_dont_use_autosuspend(&pdev->dev); + pm_runtime_disable(&pdev->dev); + + return 0; +} + +static struct platform_driver serial_port_driver = { + .driver = { + .name = "serial-port", + .pm = &serial_port_pm, + }, + .probe = serial_port_probe, + .remove = serial_port_remove, +}; + +module_platform_driver(serial_port_driver); + +MODULE_AUTHOR("Tony Lindgren <tony@xxxxxxxxxxx>"); +MODULE_DESCRIPTION("Serial controller port driver"); +MODULE_LICENSE("GPL"); diff --git a/drivers/tty/serial/serial_port.h b/drivers/tty/serial/serial_port.h new file mode 100644 --- /dev/null +++ b/drivers/tty/serial/serial_port.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ + +/** + * struct serial_port_platdata - Serial port platform data + * @state: serial port state + * + * Used by serial_core and serial_port only. Allocated on uart_add_one_port(), + * and freed on uart_remove_one_port(). Note that the life cycle for uart_state + * is different from serial_port_device. + */ +struct serial_port_platdata { + struct uart_state *state; +}; + +extern int serial_port_get(struct uart_state *state); +extern void serial_port_put(struct uart_state *state); diff --git a/include/linux/serial_core.h b/include/linux/serial_core.h --- a/include/linux/serial_core.h +++ b/include/linux/serial_core.h @@ -614,6 +614,7 @@ struct uart_state { struct tty_port port; struct serial_controller *controller; + struct device *port_dev; enum uart_pm_state pm_state; struct circ_buf xmit; -- 2.38.1