diff mbox series

i2c: ensure timely release of driver-allocated resources

Message ID YFf1GFPephFxC0mC@google.com
State Accepted
Commit 5b5475826c5265cead7ce4ca6d34ec0c566c70aa
Headers show
Series i2c: ensure timely release of driver-allocated resources | expand

Commit Message

Dmitry Torokhov March 22, 2021, 1:38 a.m. UTC 
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().

Tested-by: Jeff LaBundy <jeff@labundy.com>
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Note that this problem is not I2C-specific and I will be sending patches
for other buses as well.

 drivers/i2c/i2c-core-base.c | 21 ++++++++++++++++++++-
 include/linux/i2c.h         |  3 +++
 2 files changed, 23 insertions(+), 1 deletion(-)

Comments

Wolfram Sang April 10, 2021, 8:01 p.m. UTC | #1 
On Sun, Mar 21, 2021 at 06:38:32PM -0700, Dmitry Torokhov wrote:
> 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().

> 

> Tested-by: Jeff LaBundy <jeff@labundy.com>

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


Applied to for-next, thanks!
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 63ebf722a424..b38ecd888b90 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -518,6 +518,13 @@  static int i2c_device_probe(struct device *dev)
 	if (status)
 		goto err_clear_wakeup_irq;
 
+	client->devres_group_id = devres_open_group(&client->dev, NULL,
+						    GFP_KERNEL);
+	if (!client->devres_group_id) {
+		status = -ENOMEM;
+		goto err_detach_pm_domain;
+	}
+
 	/*
 	 * When there are no more users of probe(),
 	 * rename probe_new to probe.
@@ -530,11 +537,21 @@  static int i2c_device_probe(struct device *dev)
 	else
 		status = -EINVAL;
 
+	/*
+	 * Note that we are not closing the devres group opened above so
+	 * even resources that were attached to the device after probe is
+	 * run are released when i2c_device_remove() is executed. This is
+	 * needed as some drivers would allocate additional resources,
+	 * for example when updating firmware.
+	 */
+
 	if (status)
-		goto err_detach_pm_domain;
+		goto err_release_driver_resources;
 
 	return 0;
 
+err_release_driver_resources:
+	devres_release_group(&client->dev, client->devres_group_id);
 err_detach_pm_domain:
 	dev_pm_domain_detach(&client->dev, true);
 err_clear_wakeup_irq:
@@ -563,6 +580,8 @@  static int i2c_device_remove(struct device *dev)
 			dev_warn(dev, "remove failed (%pe), will be ignored\n", ERR_PTR(status));
 	}
 
+	devres_release_group(&client->dev, client->devres_group_id);
+
 	dev_pm_domain_detach(&client->dev, true);
 
 	dev_pm_clear_wake_irq(&client->dev);
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 56622658b215..5d1f11c0deaa 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -306,6 +306,8 @@  struct i2c_driver {
  *	userspace_devices list
  * @slave_cb: Callback when I2C slave mode of an adapter is used. The adapter
  *	calls it to pass on slave events to the slave driver.
+ * @devres_group_id: id of the devres group that will be created for resources
+ *	acquired when probing this device.
  *
  * An i2c_client identifies a single device (i.e. chip) connected to an
  * i2c bus. The behaviour exposed to Linux is defined by the driver
@@ -334,6 +336,7 @@  struct i2c_client {
 #if IS_ENABLED(CONFIG_I2C_SLAVE)
 	i2c_slave_cb_t slave_cb;	/* callback for slave mode	*/
 #endif
+	void *devres_group_id;		/* ID of probe devres group	*/
 };
 #define to_i2c_client(d) container_of(d, struct i2c_client, dev)