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

[Markus Probst]

On Sat, 2026-10-17 at 09:55 +0200, Greg Kroah-Hartman wrote:
> On Fri, Oct 16, 2026 at 06:00:32PM +0000, Markus Probst wrote:
> > Besides more predictable behaviour, this allows for several hardened
> > behaviour changes:
> > 
> > Return -EALREADY in `serdev_device_open` if the device is already open
> > instead of causing undefined behaviour.
> > 
> > Allow calling `serdev_device_close`, even if the device is already
> > closed instead of causing a null pointer dereference.
> > 
> > If the device is left open by the driver after remove, close it and warn
> > instead of leaving it in a invalid state.
> > 
> > Signed-off-by: Markus Probst <markus.probst@xxxxxxxxx>
> > ---
> >  drivers/tty/serdev/core.c           | 10 +++++---
> >  drivers/tty/serdev/serdev-ttyport.c | 46 +++++++++++++++++++++++++++++++------
> >  include/linux/serdev.h              |  2 +-
> >  3 files changed, 47 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c
> > index 7500efcdfc21..77e8e1d4d2a6 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);
> 
> Wait, how can close() ever fail?  That should be something that just
> can't happen right?
It can't fail and its not a fail. It returns true if the device was
closed, false if already closed. Currently the driver is responsible
for closing the device, so this should always be false.

The WARN_ON was intended if the driver did not close the device and
close it.

Thats the "If the device is left open by the driver after remove, close
it and warn instead of leaving it in a invalid state." part of the
commit msg.

But I see that a panic (panic-on-warn) is definitely not appropriate
here.

Thanks
- Markus Probst

> 
> thanks,
> 
> greg k-h

Attachment: signature.asc
Description: This is a digitally signed message part