[v1] i2c: base: Drop unneeded checks for of_node
diff mbox series

Message ID 20190312153005.63715-1-andriy.shevchenko@linux.intel.com
State Superseded
Headers show
Series
  • [v1] i2c: base: Drop unneeded checks for of_node
Related show

Commit Message

andriy.shevchenko@linux.intel.com March 12, 2019, 3:30 p.m. UTC
of_find_property() will return NULL if of_node is NULL,
thus of_irq_get_by_name() returns -EINVAL which we ignore,
so no need to double check.

of_node_put() is NULL-aware, no need to check there either.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/i2c/i2c-core-base.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

Comments

andriy.shevchenko@linux.intel.com June 19, 2019, 1:58 p.m. UTC | #1
On Tue, Mar 12, 2019 at 05:30:05PM +0200, Andy Shevchenko wrote:
> of_find_property() will return NULL if of_node is NULL,
> thus of_irq_get_by_name() returns -EINVAL which we ignore,
> so no need to double check.
> 
> of_node_put() is NULL-aware, no need to check there either.
> 

Wolfram, any comments on this?

> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/i2c/i2c-core-base.c | 15 ++++++---------
>  1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> index 28460f6a60cc..5118b59ca7ce 100644
> --- a/drivers/i2c/i2c-core-base.c
> +++ b/drivers/i2c/i2c-core-base.c
> @@ -354,13 +354,11 @@ static int i2c_device_probe(struct device *dev)
>  		return -ENODEV;
>  
>  	if (client->flags & I2C_CLIENT_WAKE) {
> -		int wakeirq = -ENOENT;
> +		int wakeirq;
>  
> -		if (dev->of_node) {
> -			wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
> -			if (wakeirq == -EPROBE_DEFER)
> -				return wakeirq;
> -		}
> +		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
> +		if (wakeirq == -EPROBE_DEFER)
> +			return wakeirq;
>  
>  		device_init_wakeup(&client->dev, true);
>  
> @@ -813,10 +811,9 @@ void i2c_unregister_device(struct i2c_client *client)
>  	if (!client)
>  		return;
>  
> -	if (client->dev.of_node) {
> +	if (client->dev.of_node)
>  		of_node_clear_flag(client->dev.of_node, OF_POPULATED);
> -		of_node_put(client->dev.of_node);
> -	}
> +	of_node_put(client->dev.of_node);
>  
>  	if (ACPI_COMPANION(&client->dev))
>  		acpi_device_clear_enumerated(ACPI_COMPANION(&client->dev));
> -- 
> 2.20.1
>
Wolfram Sang July 6, 2019, 3:02 p.m. UTC | #2
Hi Andy,

> @@ -354,13 +354,11 @@ static int i2c_device_probe(struct device *dev)
>  		return -ENODEV;
>  
>  	if (client->flags & I2C_CLIENT_WAKE) {
> -		int wakeirq = -ENOENT;
> +		int wakeirq;
>  
> -		if (dev->of_node) {
> -			wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
> -			if (wakeirq == -EPROBE_DEFER)
> -				return wakeirq;
> -		}
> +		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
> +		if (wakeirq == -EPROBE_DEFER)
> +			return wakeirq;
>  

I really like this chunk. It simplifies the code.

> @@ -813,10 +811,9 @@ void i2c_unregister_device(struct i2c_client *client)
>  	if (!client)
>  		return;
>  
> -	if (client->dev.of_node) {
> +	if (client->dev.of_node)
>  		of_node_clear_flag(client->dev.of_node, OF_POPULATED);
> -		of_node_put(client->dev.of_node);
> -	}
> +	of_node_put(client->dev.of_node);

This one, not so much. We do the check anyhow, so the grouping of
related calls in one block looks better readable to me than the chunk
above. Yes, we save the braces but it looks a bit messy to me.

Thanks,

   Wolfram

Patch
diff mbox series

diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 28460f6a60cc..5118b59ca7ce 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -354,13 +354,11 @@  static int i2c_device_probe(struct device *dev)
 		return -ENODEV;
 
 	if (client->flags & I2C_CLIENT_WAKE) {
-		int wakeirq = -ENOENT;
+		int wakeirq;
 
-		if (dev->of_node) {
-			wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
-			if (wakeirq == -EPROBE_DEFER)
-				return wakeirq;
-		}
+		wakeirq = of_irq_get_byname(dev->of_node, "wakeup");
+		if (wakeirq == -EPROBE_DEFER)
+			return wakeirq;
 
 		device_init_wakeup(&client->dev, true);
 
@@ -813,10 +811,9 @@  void i2c_unregister_device(struct i2c_client *client)
 	if (!client)
 		return;
 
-	if (client->dev.of_node) {
+	if (client->dev.of_node)
 		of_node_clear_flag(client->dev.of_node, OF_POPULATED);
-		of_node_put(client->dev.of_node);
-	}
+	of_node_put(client->dev.of_node);
 
 	if (ACPI_COMPANION(&client->dev))
 		acpi_device_clear_enumerated(ACPI_COMPANION(&client->dev));