On 25/03/21 08:40PM, Kevin Hao wrote:
>
> Currently, the SPI host is registered using a managed API, which
> automatically unregisters it when the device is detached from its driver.
> However, this unregistration occurs after the driver's remove() callback.
>
> Since the host is already disabled inside the remove(), any pending IO
> from child devices can easily corrupt the kernel.
>
> For example, the following steps on an imx8mq-evk board can trigger a
> kernel panic:
> while true; do cat /dev/mtd0 >/dev/null; done &
> echo 30bb0000.spi >/sys/bus/platform/drivers/fsl-quadspi/unbind
>
> To fix this, explicitly call spi_unregister_controller() in the
> remove() callback. This ensures that all child devices are properly
> removed before the host is disabled.
If you explicitly remove the child devices, such as
cd /sys/bus/spi/drivers/spi-nor
echo spi0.0 > unbind
then unbind the fsl-quadspi driver, the kernel panic does not occur.
Not sure if it should be the responsibility of the fsl-quadspi driver to handle
this, IMO it is a common issue with all SPI drivers.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 8fcb830a00f0 ("spi: spi-fsl-qspi: use devm_spi_register_controller")
> Signed-off-by: Kevin Hao <haokexin@xxxxxxxxx>
> ---
> drivers/spi/spi-fsl-qspi.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/spi/spi-fsl-qspi.c b/drivers/spi/spi-fsl-qspi.c
> index efd87f44c63a5b12b76538aa459ca8eb203b9dcd..4767d2085510c2f231476ba75e46f83271c4c645 100644
> --- a/drivers/spi/spi-fsl-qspi.c
> +++ b/drivers/spi/spi-fsl-qspi.c
> @@ -272,6 +272,7 @@ struct fsl_qspi {
> struct device *dev;
> int selected;
> u32 memmap_phy;
> + struct spi_controller *host;
> };
>
> static inline int needs_swap_endian(struct fsl_qspi *q)
> @@ -862,6 +863,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>
> q = spi_controller_get_devdata(ctlr);
> q->dev = dev;
> + q->host = ctlr;
> q->devtype_data = of_device_get_match_data(dev);
> if (!q->devtype_data) {
> ret = -ENODEV;
> @@ -934,7 +936,7 @@ static int fsl_qspi_probe(struct platform_device *pdev)
>
> ctlr->dev.of_node = np;
>
> - ret = devm_spi_register_controller(dev, ctlr);
> + ret = spi_register_controller(ctlr);
> if (ret)
> goto err_destroy_mutex;
>
> @@ -957,6 +959,8 @@ static void fsl_qspi_remove(struct platform_device *pdev)
> {
> struct fsl_qspi *q = platform_get_drvdata(pdev);
>
> + spi_unregister_controller(q->host);
> +
> /* disable the hardware */
> qspi_writel(q, QUADSPI_MCR_MDIS_MASK, q->iobase + QUADSPI_MCR);
> qspi_writel(q, 0x0, q->iobase + QUADSPI_RSER);
>
> --
> 2.48.1
>
