Linux-SPI Archive on lore.kernel.org
 help / color / Atom feed
From: Lukas Wunner <lukas@wunner.de>
To: Mark Brown <broonie@kernel.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
	Jarkko Nikula <jarkko.nikula@linux.intel.com>,
	linux-spi@vger.kernel.org, Baruch Siach <baruch@tkos.co.il>,
	Serge Semin <Sergey.Semin@baikalelectronics.ru>
Subject: [PATCH 1/3] spi: dw: Fix controller unregister order
Date: Mon, 25 May 2020 14:25:01 +0200
Message-ID: <3fff8cb8ae44a9893840d0688be15bb88c090a14.1590408496.git.lukas@wunner.de> (raw)
In-Reply-To: <cover.1590408496.git.lukas@wunner.de>

The Designware SPI driver uses devm_spi_register_controller() on bind.
As a consequence, on unbind, __device_release_driver() first invokes
dw_spi_remove_host() before unregistering the SPI controller via
devres_release_all().

This order is incorrect:  dw_spi_remove_host() shuts down the chip,
rendering the SPI bus inaccessible even though the SPI controller is
still registered.  When the SPI controller is subsequently unregistered,
it unbinds all its slave devices.  Because their drivers cannot access
the SPI bus, e.g. to quiesce interrupts, the slave devices may be left
in an improper state.

As a rule, devm_spi_register_controller() must not be used if the
->remove() hook performs teardown steps which shall be performed after
unregistering the controller and specifically after unbinding of slaves.

Fix by reverting to the non-devm variant of spi_register_controller().

An alternative approach would be to use device-managed functions for all
steps in dw_spi_remove_host(), e.g. by calling devm_add_action_or_reset()
on probe.  However that approach would add more LoC to the driver and
it wouldn't lend itself as well to backporting to stable.

Fixes: 04f421e7b0b1 ("spi: dw: use managed resources")
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Cc: stable@vger.kernel.org # v3.14+
Cc: Baruch Siach <baruch@tkos.co.il>
---
 drivers/spi/spi-dw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/spi/spi-dw.c b/drivers/spi/spi-dw.c
index 1edb8cdd11ee..9d6904d30104 100644
--- a/drivers/spi/spi-dw.c
+++ b/drivers/spi/spi-dw.c
@@ -520,7 +520,7 @@ int dw_spi_add_host(struct device *dev, struct dw_spi *dws)
 		}
 	}
 
-	ret = devm_spi_register_controller(dev, master);
+	ret = spi_register_controller(master);
 	if (ret) {
 		dev_err(&master->dev, "problem registering spi master\n");
 		goto err_dma_exit;
@@ -544,6 +544,8 @@ void dw_spi_remove_host(struct dw_spi *dws)
 {
 	dw_spi_debugfs_remove(dws);
 
+	spi_unregister_controller(dws->master);
+
 	if (dws->dma_ops && dws->dma_ops->dma_exit)
 		dws->dma_ops->dma_exit(dws);
 
-- 
2.25.0


  reply index

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-25 12:25 [PATCH 0/3] Intel SPI unbind fixes Lukas Wunner
2020-05-25 12:25 ` Lukas Wunner [this message]
2020-05-25 13:15   ` [PATCH 1/3] spi: dw: Fix controller unregister order Andy Shevchenko
2020-05-25 12:25 ` [PATCH 2/3] spi: pxa2xx: " Lukas Wunner
2020-05-25 13:21   ` Andy Shevchenko
2020-05-26  7:39     ` Lukas Wunner
2020-05-26  8:22       ` Andy Shevchenko
2020-05-27 12:09         ` Tsuchiya Yuto
2020-05-27 12:27           ` Andy Shevchenko
2020-05-27 13:14             ` Tsuchiya Yuto
2020-05-28  7:02               ` Tsuchiya Yuto
2020-05-28  8:41                 ` Andy Shevchenko
2020-05-28  9:31                   ` Lukas Wunner
2020-05-29 13:54                     ` Tsuchiya Yuto
2020-05-25 12:25 ` [PATCH 3/3] spi: pxa2xx: Fix runtime PM ref imbalance on probe error Lukas Wunner
2020-05-25 13:12   ` Andy Shevchenko
2020-05-26  8:46     ` Jarkko Nikula
2020-05-25 13:23 ` [PATCH 0/3] Intel SPI unbind fixes Andy Shevchenko
2020-05-26 16:46 ` Mark Brown

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=3fff8cb8ae44a9893840d0688be15bb88c090a14.1590408496.git.lukas@wunner.de \
    --to=lukas@wunner.de \
    --cc=Sergey.Semin@baikalelectronics.ru \
    --cc=andriy.shevchenko@linux.intel.com \
    --cc=baruch@tkos.co.il \
    --cc=broonie@kernel.org \
    --cc=jarkko.nikula@linux.intel.com \
    --cc=linux-spi@vger.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

Linux-SPI Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-spi/0 linux-spi/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-spi linux-spi/ https://lore.kernel.org/linux-spi \
		linux-spi@vger.kernel.org
	public-inbox-index linux-spi

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-spi


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git