On Wed, Nov 23, 2022 at 10:28:24AM +0200, Tony Lindgren wrote:
> We want to enable runtime PM for serial port device drivers in a generic
> way. To do this, we want to have the serial core layer manage the
> registered physical serial controller devices.
>
> The serial layer has a few challenges to deal with:
>
> 1. The serial port mapping to a physical serial port controller device
> is currently not easily available after the physical serial controller
> struct device gets registered uart_add_one_port() time
>
> 2. The serial port device drivers have their own driver data. So we cannot
> currently start making use of serial core generic data easily without
> changing all the serial port device drivers
>
> To find the serial ports for a controller based on struct device, let's
> add a new data structure for a serial_controller. On registering a port,
> we can use the drv->state array to find the associated serial port
> controller and initialize the serial core controller.
>
> As some serial port device drivers enable runtime PM in their probe before
> registering with the serial core layer, and some do not enable runtime PM
> at all currently, we need check the state in the serial core layer on
> uart_port_startup(). We need to also consider that a serial port device
> may have multiple ports.
>
> Initially we enable runtime PM for all the serial port controller devices.
> This allows us to add runtime PM calls and properly handle any errors
> without a need for serial layer specific runtime PM wrapper functions.
>
> After this patch no functional changes for the serial port device drivers
> are intended. We just enable runtime PM and keep the runtime PM usage
> count until all the serial controller ports are unregistered. For drivers
> implementing PM runtime, we just keep track of the runtime PM
> configuration.
>
> For the serial port drivers, the serial core layer has the following use
> cases to deal with:
>
> 1. If a serial port device driver does not implement runtime PM, the
> device state is set to active state, and the runtime PM usage count
> is kept until the last port for a device is unregistered
>
> 2. If a serial port device driver implements runtime PM, the runtime PM
> usage count is kept until the last port for the device is unregistered
>
> 3. If a serial port device driver implements runtime PM autosuspend,
> autosuspend is not prevented. This currently gets set only for the
> 8250_omap driver to keep runtime PM working for it
>
> For system suspend, things should be mostly detached from the runtime PM.
> The serial port device drivers may call pm_runtime_force_suspend() and
> pm_runtime_force_resume() as needed.
...
> +static struct serial_controller *to_controller(struct uart_port *port)
Perhaps to_serial_controller()? Or to_serial_ctrl() ?
> +{
> + if (!port->dev)
> + return NULL;
> +
> + return port->state->controller;
> +}
...
> +/*
> + * Starts runtime PM for the serial controller device if not already started
> + * by the serial port driver. Called from uart_add_one_port() with port_mutex
> + * held.
Perhaps we may utilize lockdep_assert() here?
> + */
Also maybe might_sleep() can be added where it's appropriate?
...
> +static int serial_core_register_port(struct uart_port *port,
> + struct uart_driver *drv)
> +{
> + struct serial_controller *controller;
> + bool allocated = false;
> + int ret;
> +
> + if (!port->dev)
> + return 0;
> +
> + controller = serial_core_find_controller(drv, port->dev);
> + if (!controller) {
> + controller = kzalloc(sizeof(*controller), GFP_KERNEL);
> + if (!controller)
> + return -ENOMEM;
> +
> + controller->drv = drv;
> + controller->dev = port->dev;
> + controller->supports_autosuspend = port->supports_autosuspend;
> + allocated = true;
Maybe split this to a serial_core_controller_alloc()?
> + }
> +
> + port->state->controller = controller;
> + WARN_ON(port->supports_autosuspend != controller->supports_autosuspend);
> +
> + ret = serial_core_pm_runtime_start(port);
> + if (ret < 0)
> + goto err_free;
> +
> + return 0;
> +
> +err_free:
> + port->state->controller = NULL;
> + if (allocated)
> + kfree(controller);
> +
> + return ret;
> +}
--
With Best Regards,
Andy Shevchenko
