Re: [PATCH v4 1/4] tty: serdev: Add mutex lock

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pm: async pm_runtime_put() in remove path cancelled by pm_runtime_disable()
--

commit c4731a25fb19c22b675757b565d963b9ad0190e8
Author: Markus Probst <markus.probst@posteo.de>

tty: serdev: Add mutex lock

Besides more predictable behaviour, this allows for several hardened
behaviour changes. If the device is left open by the driver after remove,
close it and warn instead of leaving it in an invalid state.

> diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> index 7500efcdfc218..77e8e1d4d2a66 100644
> --- a/drivers/tty/serdev/core.c
> +++ b/drivers/tty/serdev/core.c
> @@ -142,6 +142,11 @@ void serdev_device_remove(struct serdev_device *serdev)
>  	struct serdev_controller *ctrl = serdev->ctrl;
>  
>  	device_unregister(&serdev->dev);
> +
> +	/* Warn if driver did not close the serial device. */
> +	if (ctrl->ops->close && WARN_ON(ctrl->ops->close(ctrl)))
> +		pm_runtime_put(&ctrl->dev);

[Severity: Medium]
When this path is reached during controller teardown, will the pending
asynchronous suspend work be cancelled?

In serdev_controller_remove(), device_for_each_child() synchronously invokes
the removal, and then immediately calls pm_runtime_disable(&ctrl->dev):

serdev_controller_remove() {
	...
	device_for_each_child(&ctrl->dev, NULL, serdev_remove_device);
	pm_runtime_disable(&ctrl->dev);
	device_del(&ctrl->dev);
}

Since pm_runtime_disable() cancels pending asynchronous idle and suspend work,
calling pm_runtime_put() here instead of pm_runtime_put_sync() can prevent the
hardware from actually suspending, leaving the controller powered on indefinitely.

Could pm_runtime_put_sync() be used here instead to ensure the device is
properly suspended before pm_runtime_disable() is called?

> +
>  	ctrl->serdev = NULL;
>  }
>  EXPORT_SYMBOL_GPL(serdev_device_remove);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016-rust_serdev_probe_refactor-v4-0-46832e034757@posteo.de?part=1