Messages in this thread | | | Date | Wed, 23 Nov 2022 20:37:03 +0200 | From | Andy Shevchenko <> | Subject | Re: [PATCH v3 2/2] serial: core: Add port port device to flush TX on runtime resume |
| |
On Wed, Nov 23, 2022 at 10:28:25AM +0200, Tony Lindgren wrote: > 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@kernel.org>.
I believe it's quite a duplication of email addresses (they are in Cc and will be on lore.kernel.org) and also below.
> To flush the pending TX, let's set up each port as a proper device as > suggested by Greg Kroah-Hartman <gregkh@linuxfoundation.org>. > > 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.
...
> +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;
Can we simply utilize platform_device_register_full()?
> +}
...
> +EXPORT_SYMBOL(serial_port_get);
Can we move these to namespace from day 1?
...
> + return (!uart_tx_stopped(port) && > + uart_circ_chars_pending(&port->state->xmit));
Outer parentheses are redundant.
...
> +static const struct dev_pm_ops serial_port_pm = { > + SET_RUNTIME_PM_OPS(NULL, serial_port_runtime_resume, NULL) > +};
Can't we use new macros / helpers which starts from DEFINE_...
...
> + pm_runtime_set_autosuspend_delay(&pdev->dev, > + SERIAL_PORT_AUTOSUSPEND_DELAY_MS);
With temporary
struct device *dev = &pdev->dev;
this in particular become one line.
...
> + .pm = &serial_port_pm,
As per above, use pm_ptr() ?
...
> +
Unneeded blank line.
> +module_platform_driver(serial_port_driver);
-- With Best Regards, Andy Shevchenko
| |