[v1] i2c: base: Drop unneeded checks for of_node

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(-)

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
>

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