Linux-SPI Archive on lore.kernel.org
 help / color / Atom feed
From: Dmitry Torokhov <dmitry.torokhov@gmail.com>
To: Mark Brown <broonie@kernel.org>
Cc: linux-spi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH] spi: ensure timely release of driver-allocated resources
Date: Sun, 21 Mar 2021 18:43:32 -0700
Message-ID: <YFf2RD931nq3RudJ@google.com> (raw)

More and more drivers rely on devres to manage their resources, however
if bus' probe() and release() methods are not trivial and control some
of resources as well (for example enable or disable clocks, or attach
device to a power domain), we need to make sure that driver-allocated
resources are released immediately after driver's remove() method
returns, and not postponed until driver core gets around to releasing
resources. To fix that we open a new devres group before calling
driver's probe() and explicitly release it when we return from driver's
remove().

Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Note that this is not SPI-specific issue. I already send a similar
patch for I2C and will be sending more.

 drivers/spi/spi.c       | 25 +++++++++++++++++++++++--
 include/linux/spi/spi.h |  4 ++++
 2 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
index c19a09201358..7c369cebebbd 100644
--- a/drivers/spi/spi.c
+++ b/drivers/spi/spi.c
@@ -421,29 +421,50 @@ static int spi_probe(struct device *dev)
 	if (ret)
 		return ret;
 
+	spi->devres_group_id = devres_open_group(dev, NULL, GFP_KERNEL);
+	if (!spi->devres_group_id) {
+		ret = -ENOMEM;
+		goto err_detach_pm_domain;
+	}
+
 	if (sdrv->probe) {
 		ret = sdrv->probe(spi);
 		if (ret)
-			dev_pm_domain_detach(dev, true);
+			goto err_release_driver_resources;
 	}
 
+	/*
+	 * Note that we are not closing the devres group opened above so
+	 * even resources that were attached to the device after probe has
+	 * run are released when spi_remove() is executed. This is needed as
+	 * some drivers might allocate additional resources.
+	 */
+
+	return 0;
+
+err_release_driver_resources:
+	devres_release_group(dev, spi->devres_group_id);
+err_detach_pm_domain:
+	dev_pm_domain_detach(dev, true);
 	return ret;
 }
 
 static int spi_remove(struct device *dev)
 {
 	const struct spi_driver		*sdrv = to_spi_driver(dev->driver);
+	struct spi_device		*spi = to_spi_device(dev);
 
 	if (sdrv->remove) {
 		int ret;
 
-		ret = sdrv->remove(to_spi_device(dev));
+		ret = sdrv->remove(spi);
 		if (ret)
 			dev_warn(dev,
 				 "Failed to unbind driver (%pe), ignoring\n",
 				 ERR_PTR(ret));
 	}
 
+	devres_release_group(dev, spi->devres_group_id);
 	dev_pm_domain_detach(dev, true);
 
 	return 0;
diff --git a/include/linux/spi/spi.h b/include/linux/spi/spi.h
index aa09fdc8042d..969dd8ccc657 100644
--- a/include/linux/spi/spi.h
+++ b/include/linux/spi/spi.h
@@ -144,6 +144,8 @@ extern int spi_delay_exec(struct spi_delay *_delay, struct spi_transfer *xfer);
  *	not using a GPIO line)
  * @word_delay: delay to be inserted between consecutive
  *	words of a transfer
+ * @devres_group_id: id of the devres group that will be created for resources
+ *	acquired when probing this device.
  *
  * @statistics: statistics for the spi_device
  *
@@ -195,6 +197,8 @@ struct spi_device {
 	struct gpio_desc	*cs_gpiod;	/* chip select gpio desc */
 	struct spi_delay	word_delay; /* inter-word delay */
 
+	void			*devres_group_id;
+
 	/* the statistics */
 	struct spi_statistics	statistics;
 
-- 
2.31.0.rc2.261.g7f71774620-goog


-- 
Dmitry

             reply index

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-22  1:43 Dmitry Torokhov [this message]
2021-03-22 12:37 ` Mark Brown
2021-03-22 19:38   ` Dmitry Torokhov
2021-03-23 17:36     ` Mark Brown
2021-03-23 19:04       ` Dmitry Torokhov
2021-03-24 21:32         ` Mark Brown
2021-03-24 22:27           ` Dmitry Torokhov
2021-03-24 23:39             ` Mark Brown
2021-03-25  0:17               ` Dmitry Torokhov
2021-03-30 17:19                 ` 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=YFf2RD931nq3RudJ@google.com \
    --to=dmitry.torokhov@gmail.com \
    --cc=broonie@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --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