On Fri, Mar 21, 2025 at 10:23:07AM -0500, Han Xu wrote:
> On 25/03/21 08:40PM, Kevin Hao wrote:
> > Since the host is already disabled inside the remove(), any pending IO
> > from child devices can easily corrupt the kernel.
...
> > 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.
This is a bug in the driver, it needs to be able to continue supporting
child devices until it is unregistered. This means that either the
unregistration needs to be manual or any disabling also needs to be done
via devm.
>
> >
> > 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
> >
Attachment:
signature.asc
Description: PGP signature
