During shutdown, the driver should unregister the SPI controller and stop the hardware. Otherwise the dspi_transfer_one_message() could wait on completion infinitely.

Additionally, calling spi_unregister_controller() first in device shutdown reverse-matches the probe function, where SPI controller is registered at the end.

Fixes: dc234825997e ("spi: spi-fsl-dspi: Adding shutdown hook") Cc: stable@vger.kernel.org Reported-by: Vladimir Oltean olteanv@gmail.com Signed-off-by: Krzysztof Kozlowski krzk@kernel.org

---

kexec() not tested, only system shutdown.

Changes since v3: 1. New patch. --- drivers/spi/spi-fsl-dspi.c | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c index ec0fd0d366eb..ec7919d9c0d9 100644 --- a/drivers/spi/spi-fsl-dspi.c +++ b/drivers/spi/spi-fsl-dspi.c @@ -1452,20 +1452,7 @@ static int dspi_remove(struct platform_device *pdev)

static void dspi_shutdown(struct platform_device *pdev) { - struct spi_controller *ctlr = platform_get_drvdata(pdev); - struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr); - - /* Disable RX and TX */ - regmap_update_bits(dspi->regmap, SPI_MCR, - SPI_MCR_DIS_TXF | SPI_MCR_DIS_RXF, - SPI_MCR_DIS_TXF | SPI_MCR_DIS_RXF); - - /* Stop Running */ - regmap_update_bits(dspi->regmap, SPI_MCR, SPI_MCR_HALT, SPI_MCR_HALT); - - dspi_release_dma(dspi); - clk_disable_unprepare(dspi->clk); - spi_unregister_controller(dspi->ctlr); + dspi_remove(pdev); }

static struct platform_driver fsl_dspi_driver = {

If shared interrupt comes late, during probe error path or device remove (could be triggered with CONFIG_DEBUG_SHIRQ), the interrupt handler dspi_interrupt() will access registers with the clock being disabled. This leads to external abort on non-linefetch on Toradex Colibri VF50 module (with Vybrid VF5xx):

$ echo 4002d000.spi > /sys/devices/platform/soc/40000000.bus/4002d000.spi/driver/unbind

Unhandled fault: external abort on non-linefetch (0x1008) at 0x8887f02c Internal error: : 1008 [#1] ARM Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree) Backtrace: (regmap_mmio_read32le) (regmap_mmio_read) (_regmap_bus_reg_read) (_regmap_read) (regmap_read) (dspi_interrupt) (free_irq) (devm_irq_release) (release_nodes) (devres_release_all) (device_release_driver_internal)

The resource-managed framework should not be used for shared interrupt handling, because the interrupt handler might be called after releasing other resources and disabling clocks.

Similar bug could happen during suspend - the shared interrupt handler could be invoked after suspending the device. Each device sharing this interrupt line should disable the IRQ during suspend so handler will be invoked only in following cases: 1. None suspended, 2. All devices resumed.

Fixes: 349ad66c0ab0 ("spi:Add Freescale DSPI driver for Vybrid VF610 platform") Cc: stable@vger.kernel.org Signed-off-by: Krzysztof Kozlowski krzk@kernel.org

---

Changes since v3: 1. Use simpler if (dspi->irq).

Changes since v2: 1. Go back to v1 and use non-devm interface, 2. Fix also suspend/resume paths.

Changes since v1: 1. Disable the IRQ instead of using non-devm interface. --- drivers/spi/spi-fsl-dspi.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c index ec7919d9c0d9..e0b30e4b1b69 100644 --- a/drivers/spi/spi-fsl-dspi.c +++ b/drivers/spi/spi-fsl-dspi.c @@ -1109,6 +1109,8 @@ static int dspi_suspend(struct device *dev) struct spi_controller *ctlr = dev_get_drvdata(dev); struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr);

+ if (dspi->irq) + disable_irq(dspi->irq); spi_controller_suspend(ctlr); clk_disable_unprepare(dspi->clk);

@@ -1129,6 +1131,8 @@ static int dspi_resume(struct device *dev) if (ret) return ret; spi_controller_resume(ctlr); + if (dspi->irq) + enable_irq(dspi->irq);

return 0; } @@ -1385,8 +1389,8 @@ static int dspi_probe(struct platform_device *pdev) goto poll_mode; }

- ret = devm_request_irq(&pdev->dev, dspi->irq, dspi_interrupt, - IRQF_SHARED, pdev->name, dspi); + ret = request_threaded_irq(dspi->irq, dspi_interrupt, NULL, + IRQF_SHARED, pdev->name, dspi); if (ret < 0) { dev_err(&pdev->dev, "Unable to attach DSPI interrupt\n"); goto out_clk_put; @@ -1400,7 +1404,7 @@ static int dspi_probe(struct platform_device *pdev) ret = dspi_request_dma(dspi, res->start); if (ret < 0) { dev_err(&pdev->dev, "can't get dma channels\n"); - goto out_clk_put; + goto out_free_irq; } }

@@ -1415,11 +1419,14 @@ static int dspi_probe(struct platform_device *pdev) ret = spi_register_controller(ctlr); if (ret != 0) { dev_err(&pdev->dev, "Problem registering DSPI ctlr\n"); - goto out_clk_put; + goto out_free_irq; }

return ret;

+out_free_irq: + if (dspi->irq) + free_irq(dspi->irq, dspi); out_clk_put: clk_disable_unprepare(dspi->clk); out_ctlr_put: @@ -1445,6 +1452,8 @@ static int dspi_remove(struct platform_device *pdev) regmap_update_bits(dspi->regmap, SPI_MCR, SPI_MCR_HALT, SPI_MCR_HALT);

dspi_release_dma(dspi); + if (dspi->irq) + free_irq(dspi->irq, dspi); clk_disable_unprepare(dspi->clk);

return 0;

On Mon, 22 Jun 2020 at 14:06, Krzysztof Kozlowski krzk@kernel.org wrote:

During device removal, the driver should unregister the SPI controller and stop the hardware. Otherwise the dspi_transfer_one_message() could wait on completion infinitely.

Additionally, calling spi_unregister_controller() first in device removal reverse-matches the probe function, where SPI controller is registered at the end.

Fixes: 05209f457069 ("spi: fsl-dspi: add missing clk_disable_unprepare() in dspi_remove()") Cc: stable@vger.kernel.org Reported-by: Vladimir Oltean olteanv@gmail.com Signed-off-by: Krzysztof Kozlowski krzk@kernel.org

---

Reviewed-by: Vladimir Oltean vladimir.oltean@nxp.com Tested-by: Vladimir Oltean vladimir.oltean@nxp.com

Changes since v3:

1. New patch.

---

drivers/spi/spi-fsl-dspi.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c index 58190c94561f..ec0fd0d366eb 100644 --- a/drivers/spi/spi-fsl-dspi.c +++ b/drivers/spi/spi-fsl-dspi.c @@ -1434,9 +1434,18 @@ static int dspi_remove(struct platform_device *pdev) struct fsl_dspi *dspi = spi_controller_get_devdata(ctlr);

    /* Disconnect from the SPI framework */

•   spi_unregister_controller(dspi->ctlr);
  

•   /* Disable RX and TX */
  

•   regmap_update_bits(dspi->regmap, SPI_MCR,
  

•                      SPI_MCR_DIS_TXF | SPI_MCR_DIS_RXF,
  

•                      SPI_MCR_DIS_TXF | SPI_MCR_DIS_RXF);
  

•   /* Stop Running */
  

•   regmap_update_bits(dspi->regmap, SPI_MCR, SPI_MCR_HALT, SPI_MCR_HALT);
  

•   dspi_release_dma(dspi);
    clk_disable_unprepare(dspi->clk);
  

•   spi_unregister_controller(dspi->ctlr);
  
    return 0;
  

}

2.17.1