stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Krzysztof Kozlowski <krzk@kernel.org>
To: Mark Brown <broonie@kernel.org>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org,
	Pengutronix Kernel Team <kernel@pengutronix.de>,
	Marc Kleine-Budde <mkl@pengutronix.de>,
	Wolfram Sang <wsa@kernel.org>
Cc: Krzysztof Kozlowski <krzk@kernel.org>, stable@vger.kernel.org
Subject: [PATCH 1/2] spi: spi-fsl-dspi: Fix external abort on interrupt in exit paths
Date: Sun, 14 Jun 2020 12:55:53 +0200	[thread overview]
Message-ID: <1592132154-20175-1-git-send-email-krzk@kernel.org> (raw)

If 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
    CPU: 0 PID: 136 Comm: sh Not tainted 5.7.0-next-20200610-00009-g5c913fa0f9c5-dirty #74
    Hardware name: Freescale Vybrid VF5xx/VF6xx (Device Tree)
      (regmap_mmio_read32le) from [<8061885c>] (regmap_mmio_read+0x48/0x68)
      (regmap_mmio_read) from [<8060e3b8>] (_regmap_bus_reg_read+0x24/0x28)
      (_regmap_bus_reg_read) from [<80611c50>] (_regmap_read+0x70/0x1c0)
      (_regmap_read) from [<80611dec>] (regmap_read+0x4c/0x6c)
      (regmap_read) from [<80678ca0>] (dspi_interrupt+0x3c/0xa8)
      (dspi_interrupt) from [<8017acec>] (free_irq+0x26c/0x3cc)
      (free_irq) from [<8017dcec>] (devm_irq_release+0x1c/0x20)
      (devm_irq_release) from [<805f98ec>] (release_nodes+0x1e4/0x298)
      (release_nodes) from [<805f9ac8>] (devres_release_all+0x40/0x60)
      (devres_release_all) from [<805f5134>] (device_release_driver_internal+0x108/0x1ac)
      (device_release_driver_internal) from [<805f521c>] (device_driver_detach+0x20/0x24)

The resource-managed framework should not be used for interrupt handling,
because the resource will be released too late - after disabling clocks.
The interrupt handler is not prepared for such case.

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

---

This is an follow up of my other patch for I2C IMX driver [1]. Let's fix the
issues consistently.

[1] https://lore.kernel.org/lkml/1592130544-19759-2-git-send-email-krzk@kernel.org/T/#u
---
 drivers/spi/spi-fsl-dspi.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/spi/spi-fsl-dspi.c b/drivers/spi/spi-fsl-dspi.c
index 58190c94561f..57e7a626ba00 100644
--- a/drivers/spi/spi-fsl-dspi.c
+++ b/drivers/spi/spi-fsl-dspi.c
@@ -1385,8 +1385,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 +1400,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 +1415,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 > 0)
+		free_irq(dspi->irq, dspi);
 out_clk_put:
 	clk_disable_unprepare(dspi->clk);
 out_ctlr_put:
@@ -1435,6 +1438,8 @@ static int dspi_remove(struct platform_device *pdev)
 
 	/* Disconnect from the SPI framework */
 	dspi_release_dma(dspi);
+	if (dspi->irq > 0)
+		free_irq(dspi->irq, dspi);
 	clk_disable_unprepare(dspi->clk);
 	spi_unregister_controller(dspi->ctlr);
 
-- 
2.7.4


             reply	other threads:[~2020-06-14 10:56 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-14 10:55 Krzysztof Kozlowski [this message]
2020-06-14 10:55 ` [PATCH 2/2] spi: spi-fsl-dspi: Initialize completion before possible interrupt Krzysztof Kozlowski
2020-06-14 11:14   ` Vladimir Oltean
2020-06-14 11:18     ` Krzysztof Kozlowski
2020-06-14 13:39       ` Vladimir Oltean
2020-06-14 13:43         ` Vladimir Oltean
2020-06-14 15:12           ` Krzysztof Kozlowski
2020-06-14 15:34             ` Vladimir Oltean
2020-06-15  7:08               ` Krzysztof Kozlowski
2020-06-15  9:26                 ` Vladimir Oltean
2020-06-15  9:30                   ` Krzysztof Kozlowski
2020-06-14 11:18     ` Wolfram Sang
2020-06-14 15:48 ` [PATCH 1/2] spi: spi-fsl-dspi: Fix external abort on interrupt in exit paths Vladimir Oltean
2020-06-15  7:15   ` Krzysztof Kozlowski
2020-06-15  7:51     ` Krzysztof Kozlowski

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1592132154-20175-1-git-send-email-krzk@kernel.org \
    --to=krzk@kernel.org \
    --cc=broonie@kernel.org \
    --cc=kernel@pengutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-spi@vger.kernel.org \
    --cc=mkl@pengutronix.de \
    --cc=stable@vger.kernel.org \
    --cc=vladimir.oltean@nxp.com \
    --cc=wsa@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).