diff mbox series

[v4,2/3] i2c: Retain info->of_node in i2c_new_device()

Message ID 20180325124903.2909-3-boris.brezillon@bootlin.com
State Accepted
Headers show
Series i2c: Prepare things for I3C | expand

Commit Message

Boris Brezillon March 25, 2018, 12:49 p.m. UTC 
Currently, of_i2c_register_devices() is responsible for retaining
info->of_node, but we're about to expose a function to parse I2C board
info without registering the I2C device.

We could possibly let this function retain ->of_node, but this approach
is prone to reference leak since people will have to remember to call
of_node_put() if something goes wrong between the OF node parsing and
the registration step.
Let's just retain the ->of_node in i2c_new_register() instead.

Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
---
Changes in v4:
- patch added in v4
---
 drivers/i2c/i2c-core-base.c | 6 ++++--
 drivers/i2c/i2c-core-of.c   | 3 +--
 2 files changed, 5 insertions(+), 4 deletions(-)

Comments

Wolfram Sang May 22, 2018, 11:41 a.m. UTC | #1 
On Sun, Mar 25, 2018 at 02:49:02PM +0200, Boris Brezillon wrote:
> Currently, of_i2c_register_devices() is responsible for retaining
> info->of_node, but we're about to expose a function to parse I2C board
> info without registering the I2C device.
> 
> We could possibly let this function retain ->of_node, but this approach
> is prone to reference leak since people will have to remember to call
> of_node_put() if something goes wrong between the OF node parsing and
> the registration step.
> Let's just retain the ->of_node in i2c_new_register() instead.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>

Applied to for-next, thanks!

What I also like is that the of_node_get/put-pair is in the same source
file now. Much easier to comprehend.
diff mbox series

Patch

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 0d710eae5422..5227a3ceb659 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -742,7 +742,7 @@  i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 	client->dev.parent = &client->adapter->dev;
 	client->dev.bus = &i2c_bus_type;
 	client->dev.type = &i2c_client_type;
-	client->dev.of_node = info->of_node;
+	client->dev.of_node = of_node_get(info->of_node);
 	client->dev.fwnode = info->fwnode;
 
 	i2c_dev_set_name(adap, client, info);
@@ -753,7 +753,7 @@  i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 			dev_err(&adap->dev,
 				"Failed to add properties to client %s: %d\n",
 				client->name, status);
-			goto out_err;
+			goto out_err_put_of_node;
 		}
 	}
 
@@ -769,6 +769,8 @@  i2c_new_device(struct i2c_adapter *adap, struct i2c_board_info const *info)
 out_free_props:
 	if (info->properties)
 		device_remove_properties(&client->dev);
+out_err_put_of_node:
+	of_node_put(info->of_node);
 out_err:
 	dev_err(&adap->dev,
 		"Failed to register i2c client %s at 0x%02x (%d)\n",
diff --git a/drivers/i2c/i2c-core-of.c b/drivers/i2c/i2c-core-of.c
index 15bd51eca37b..9fb38e99a6c6 100644
--- a/drivers/i2c/i2c-core-of.c
+++ b/drivers/i2c/i2c-core-of.c
@@ -55,7 +55,7 @@  static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 	}
 
 	info.addr = addr;
-	info.of_node = of_node_get(node);
+	info.of_node = node;
 
 	if (of_property_read_bool(node, "host-notify"))
 		info.flags |= I2C_CLIENT_HOST_NOTIFY;
@@ -66,7 +66,6 @@  static struct i2c_client *of_i2c_register_device(struct i2c_adapter *adap,
 	client = i2c_new_device(adap, &info);
 	if (!client) {
 		dev_err(&adap->dev, "of_i2c: Failure registering %pOF\n", node);
-		of_node_put(node);
 		return ERR_PTR(-EINVAL);
 	}
 	return client;