[PATCH] spi: virtio: move teardown to .remove()

[PATCH] spi: virtio: move teardown to .remove()

[Francesco Valla]

The .remove() callback is not marked as optional for a virtio_driver,
and thus a missing one is a bug. Moreover, the virtio framework expects
a driver to reset the device it controls during teardown and explicitly
checks for this reading the device status and WARNing in case it is
still running. Any devm cleanup action gets however called after that
check, leading to the WARN_ON condition to always be true for
spi-virtio; thus, an empty remove() is not an option.

Move the cleanup actions to the .remove() callback of the driver and
adapt the surrounding code to the new behavior.

Fixes: f98cabe3f6cf ("SPI: Add virtio SPI driver")
Signed-off-by: Francesco Valla <francesco@valla.it>
---
 drivers/spi/spi-virtio.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/spi/spi-virtio.c b/drivers/spi/spi-virtio.c
index 26f9711c99b3..2eeb79ff5aa2 100644
--- a/drivers/spi/spi-virtio.c
+++ b/drivers/spi/spi-virtio.c
@@ -315,10 +315,8 @@ static int virtio_spi_find_vqs(struct virtio_spi_priv *priv)
 }
 
 /* Function must not be called before virtio_spi_find_vqs() has been run */
-static void virtio_spi_del_vq(void *data)
+static void virtio_spi_del_vq(struct virtio_device *vdev)
 {
-	struct virtio_device *vdev = data;
-
 	virtio_reset_device(vdev);
 	vdev->config->del_vqs(vdev);
 }
@@ -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);
@@ -415,6 +418,7 @@ static struct virtio_driver virtio_spi_driver = {
 	},
 	.id_table = virtio_spi_id_table,
 	.probe = virtio_spi_probe,
+	.remove = virtio_spi_remove,
 };
 module_virtio_driver(virtio_spi_driver);
 

---
base-commit: fa4ad798f6f681031775266044ba1b0cf3c670bd
change-id: 20261108-spi_virtio_remove-8c285a0d4725

Best regards,
--  
Francesco Valla <francesco@valla.it>

Re: [PATCH] spi: virtio: move teardown to .remove()

[sashiko-bot]

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 <francesco@valla.it>

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/20261108-spi_virtio_remove-v1-1-fda5f0996290@valla.it?part=1

Re: [PATCH] spi: virtio: move teardown to .remove()

[Mark Brown]

On Sun, 08 Nov 2026 18:41:17 +0100, Francesco Valla wrote:
> spi: virtio: move teardown to .remove()

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/spi.git for-7.4

Thanks!

[1/1] spi: virtio: move teardown to .remove()
      https://git.kernel.org/broonie/spi/c/9060f5ba658c

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark