Thank you for your contribution! Sashiko AI review found no new issues in this patch, but noticed 1 related pre-existing issue it does not introduce: - [High] spi: virtio: virtqueue kick before DRIVER_OK in virtio_spi_restore() [1] --
commit 30a7d01f20f6ab641d59ca6ce3ddf0209bd0408f Author: Francesco Valla <[email protected]> spi: virtio: move teardown to .remove() This commit replaces devm-managed controller registration and virtqueue cleanup with an explicit .remove() callback so the virtio device is reset before the virtio core checks its status during teardown. > diff --git a/drivers/spi/spi-virtio.c b/drivers/spi/spi-virtio.c > index 26f9711c99b39..2eeb79ff5aa26 100644 > --- a/drivers/spi/spi-virtio.c > +++ b/drivers/spi/spi-virtio.c [ ... ] > @@ -347,21 +345,26 @@ static int virtio_spi_probe(struct virtio_device *vdev) > if (ret) > return dev_err_probe(&vdev->dev, ret, "Cannot setup > virtqueues\n"); > > - /* Register cleanup for virtqueues using devm */ > - ret = devm_add_action_or_reset(&vdev->dev, virtio_spi_del_vq, vdev); > - if (ret) > - return dev_err_probe(&vdev->dev, ret, "Cannot register > virtqueue cleanup\n"); > - > virtio_device_ready(vdev); > > - /* Use devm version to register controller */ > - ret = devm_spi_register_controller(&vdev->dev, ctrl); > - if (ret) > + ret = spi_register_controller(ctrl); > + if (ret) { > + virtio_spi_del_vq(vdev); > return dev_err_probe(&vdev->dev, ret, "Cannot register > controller\n"); > + } > > return 0; > } > > +static void virtio_spi_remove(struct virtio_device *vdev) > +{ > + struct spi_controller *ctrl = dev_get_drvdata(&vdev->dev); > + > + spi_unregister_controller(ctrl); > + > + virtio_spi_del_vq(vdev); > +} > + > static int virtio_spi_freeze(struct device *dev) > { > struct spi_controller *ctrl = dev_get_drvdata(dev); [Severity: High] This problem wasn't introduced by this patch, but can virtio_spi_restore() kick the virtqueue before VIRTIO_CONFIG_S_DRIVER_OK is set [1]? drivers/spi/spi-virtio.c:virtio_spi_restore() { ... ret = virtio_spi_find_vqs(vdev->priv); if (ret) { dev_err(dev, "problem starting vqueue (%d)\n", ret); return ret; } ret = spi_controller_resume(ctrl); ... } Unlike virtio_spi_probe(), which calls virtio_device_ready() before exposing the controller, virtio_spi_restore() calls spi_controller_resume() without marking the device ready first: CPU0 CPU1 (kworker) virtio_spi_restore() virtio_spi_find_vqs() spi_controller_resume() virtio_spi_transfer_one() virtqueue_kick() <-- before DRIVER_OK virtio_device_ready() Unblocking the SPI message pump wakes the kworker to process queued transfers and kick the virtqueue before the resume path returns to set DRIVER_OK, violating the virtio specification and risking dropped SPI transfers or a broken device state on resume. Calling virtio_device_ready(vdev) before spi_controller_resume(ctrl) would avoid this. [1] https://sashiko.dev/#/bug/linux-82cf02b8-4add-4c6f-b6da-ae25c86bd2bf -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
