On Wed, 2026-09-23 at 12:35 +0200, Greg Kroah-Hartman wrote: > On Sun, Sep 20, 2026 at 02:29:58PM +0000, Markus Probst wrote: > > These functions will be used to simply the serdev rust abstraction. It > > also contributes to the fixing of 2 race conditions in the serdev rust > > abstraction. > > > > Signed-off-by: Markus Probst > > --- > > drivers/tty/serdev/core.c | 50 ++++++++++++++++++++++++++++++++++++- > > drivers/tty/serdev/serdev-ttyport.c | 38 ++++++++++++++++++++++++++++ > > include/linux/serdev.h | 6 +++++ > > 3 files changed, 93 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/tty/serdev/core.c b/drivers/tty/serdev/core.c > > index 7500efcdfc21..7d24f16710cb 100644 > > --- a/drivers/tty/serdev/core.c > > +++ b/drivers/tty/serdev/core.c > > @@ -187,6 +187,51 @@ void serdev_device_close(struct serdev_device *serdev) > > } > > EXPORT_SYMBOL_GPL(serdev_device_close); > > > > +/** > > + * serdev_device_pause_rx() - pause data receive > > + * @serdev: serdev device > > + * > > + * Pause calls to receive_buf. > > + * > > + * The caller must guarantee that this does not run concurrently with > > + * `serdev_device_open` or `serdev_device_close`. > > + * > > + * Note that if a call to receive_buf is currently executed, the function will > > + * sleep until it has finished. > > + */ > > +void serdev_device_pause_rx(struct serdev_device *serdev) > > +{ > > + struct serdev_controller *ctrl = serdev->ctrl; > > + > > + if (!ctrl || !ctrl->ops->pause_rx) > > + return; > > + > > + ctrl->ops->pause_rx(ctrl); > > +} > > +EXPORT_SYMBOL_GPL(serdev_device_pause_rx); > > + > > +/** > > + * serdev_device_resume_rx() - resume data receive > > + * @serdev: serdev device > > + * > > + * Resume calls to receive_buf. > > + * > > + * The caller must guarantee that this does not run concurrently with > > + * `serdev_device_open` or `serdev_device_close`. > > + * > > + * This can be called even if not paused to ensure data receive is active. > > + */ > > +void serdev_device_resume_rx(struct serdev_device *serdev) > > +{ > > + struct serdev_controller *ctrl = serdev->ctrl; > > + > > + if (!ctrl || !ctrl->ops->resume_rx) > > + return; > > + > > + ctrl->ops->resume_rx(ctrl); > > +} > > +EXPORT_SYMBOL_GPL(serdev_device_resume_rx); > > + > > static void devm_serdev_device_close(void *serdev) > > { > > serdev_device_close(serdev); > > @@ -398,6 +443,7 @@ EXPORT_SYMBOL_GPL(serdev_device_break_ctl); > > static int serdev_drv_probe(struct device *dev) > > { > > const struct serdev_device_driver *sdrv = to_serdev_device_driver(dev->driver); > > + struct serdev_device *sdev = to_serdev_device(dev); > > int ret; > > > > ret = dev_pm_domain_attach(dev, PD_FLAG_ATTACH_POWER_ON | > > @@ -405,7 +451,9 @@ static int serdev_drv_probe(struct device *dev) > > if (ret) > > return ret; > > > > - return sdrv->probe(to_serdev_device(dev)); > > + serdev_device_resume_rx(sdev); > > + > > + return sdrv->probe(sdev); > > } > > > > static void serdev_drv_remove(struct device *dev) > > diff --git a/drivers/tty/serdev/serdev-ttyport.c b/drivers/tty/serdev/serdev-ttyport.c > > index bab1b143b8a6..e8aa89e733bd 100644 > > --- a/drivers/tty/serdev/serdev-ttyport.c > > +++ b/drivers/tty/serdev/serdev-ttyport.c > > @@ -7,8 +7,10 @@ > > #include > > #include > > #include > > +#include "../tty.h" > > > > #define SERPORT_ACTIVE 1 > > +#define SERPORT_PAUSE_RX 2 > > > > struct serport { > > struct tty_port *port; > > @@ -32,6 +34,14 @@ static size_t ttyport_receive_buf(struct tty_port *port, const u8 *cp, > > if (!test_bit(SERPORT_ACTIVE, &serport->flags)) > > return 0; > > > > + if (test_bit(SERPORT_PAUSE_RX, &serport->flags)) > > + return 0; > > + > > + /* > > + * Ensure writes by the driver are visible before allowing traffic to resume. > > + */ > > + smp_mb__after_atomic(); > > This scares me. Why not use a real lock?  > I can use locks to make it less "fragile". But I don't I think I need them. > WHat's the issue here, you > need this to be "flushed" before this call: > > > + > > ret = serdev_controller_receive_buf(ctrl, cp, count); > > here? > > And you just tested a bit, you didn't set a bit, so what are you trying > to ensure is written exactly? This should be an acquire load operation (paired with the release store operation in `ttyport_resume_rx`). It ensures that any writes before calling `ttyport_resume_rx` are visible in the `serdev_controller_receive_buf` invocation. For instance, in the Rust abstraction the following will be called in order in probe (with the following patches): - serdev_device_pause_rx - serdev_device_open - dev_set_drvdata - serdev_device_resume_rx This atomic lock effectively ensures in this example that the set device driver data is visible to `ttyport_receive_buf` before `SERPORT_PAUSE_RX` is unset in `ttyport_resume_rx`. > > > > > > dev_WARN_ONCE(&ctrl->dev, ret > count, > > @@ -156,6 +166,32 @@ static void ttyport_close(struct serdev_controller *ctrl) > > tty_release_struct(tty, serport->tty_idx); > > } > > > > +static void ttyport_pause_rx(struct serdev_controller *ctrl) > > +{ > > + struct serport *serport = serdev_controller_get_drvdata(ctrl); > > + struct tty_struct *tty = serport->tty; > > + > > + set_bit(SERPORT_PAUSE_RX, &serport->flags); > > + > > + if (test_bit(SERPORT_ACTIVE, &serport->flags)) > > What keeps this bit from being set right after you test it? The statement "The caller must guarantee that this does not run concurrently with `serdev_device_open` or `serdev_device_close`." in the kdoc of `serdev_device_pause_rx` does. > > > > > + tty_buffer_flush_work(tty->port); > > +} > > + > > +static void ttyport_resume_rx(struct serdev_controller *ctrl) > > +{ > > + struct serport *serport = serdev_controller_get_drvdata(ctrl); > > + struct tty_struct *tty = serport->tty; > > + > > + /* > > + * Ensure writes by the driver are visible before allowing traffic to resume. > > + */ > > + smp_mb__before_atomic(); > > + clear_bit(SERPORT_PAUSE_RX, &serport->flags); > > + > > + if (test_bit(SERPORT_ACTIVE, &serport->flags)) > > Same here. Are you sure you don't need locks? This feels wrong. Same as above. Thanks - Markus Probst > > thanks, > > greg k-h