[patch] regulator: core: remove some unneeded conditions

[patch] regulator: core: remove some unneeded conditions

[Dan Carpenter]

These always true or always false conditions cause static checker
warnings.

The check prior to checking "if (pin->enable_count <= 1) {" is
"if (pin->enable_count > 1) ", so we know that ->enable_count is <= 1
already and can delete the check.

The queue_delayed_work() function is type bool so the return value can
never be less than zero.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f245214..9606bb7 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1758,11 +1758,9 @@ static int regulator_ena_gpio_ctrl(struct regulator_dev *rdev, bool enable)
 		}
 
 		/* Disable GPIO if not used */
-		if (pin->enable_count <= 1) {
-			gpiod_set_value_cansleep(pin->gpiod,
-						 pin->ena_gpio_invert);
-			pin->enable_count = 0;
-		}
+		gpiod_set_value_cansleep(pin->gpiod,
+					 pin->ena_gpio_invert);
+		pin->enable_count = 0;
 	}
 
 	return 0;
@@ -2146,7 +2144,6 @@ static void regulator_disable_work(struct work_struct *work)
 int regulator_disable_deferred(struct regulator *regulator, int ms)
 {
 	struct regulator_dev *rdev = regulator->rdev;
-	int ret;
 
 	if (regulator->always_on)
 		return 0;
@@ -2158,13 +2155,10 @@ int regulator_disable_deferred(struct regulator *regulator, int ms)
 	rdev->deferred_disables++;
 	mutex_unlock(&rdev->mutex);
 
-	ret = queue_delayed_work(system_power_efficient_wq,
-				 &rdev->disable_work,
-				 msecs_to_jiffies(ms));
-	if (ret < 0)
-		return ret;
-	else
-		return 0;
+	queue_delayed_work(system_power_efficient_wq,
+			   &rdev->disable_work,
+			   msecs_to_jiffies(ms));
+	return 0;
 }
 EXPORT_SYMBOL_GPL(regulator_disable_deferred);
 

[patch] regulator: core: remove some unneeded conditions

[Dan Carpenter]

These always true or always false conditions cause static checker
warnings.

The check prior to checking "if (pin->enable_count <= 1) {" is
"if (pin->enable_count > 1) ", so we know that ->enable_count is <= 1
already and can delete the check.

The queue_delayed_work() function is type bool so the return value can
never be less than zero.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/regulator/core.c b/drivers/regulator/core.c
index f245214..9606bb7 100644
--- a/drivers/regulator/core.c
+++ b/drivers/regulator/core.c
@@ -1758,11 +1758,9 @@ static int regulator_ena_gpio_ctrl(struct regulator_dev *rdev, bool enable)
 		}
 
 		/* Disable GPIO if not used */
-		if (pin->enable_count <= 1) {
-			gpiod_set_value_cansleep(pin->gpiod,
-						 pin->ena_gpio_invert);
-			pin->enable_count = 0;
-		}
+		gpiod_set_value_cansleep(pin->gpiod,
+					 pin->ena_gpio_invert);
+		pin->enable_count = 0;
 	}
 
 	return 0;
@@ -2146,7 +2144,6 @@ static void regulator_disable_work(struct work_struct *work)
 int regulator_disable_deferred(struct regulator *regulator, int ms)
 {
 	struct regulator_dev *rdev = regulator->rdev;
-	int ret;
 
 	if (regulator->always_on)
 		return 0;
@@ -2158,13 +2155,10 @@ int regulator_disable_deferred(struct regulator *regulator, int ms)
 	rdev->deferred_disables++;
 	mutex_unlock(&rdev->mutex);
 
-	ret = queue_delayed_work(system_power_efficient_wq,
-				 &rdev->disable_work,
-				 msecs_to_jiffies(ms));
-	if (ret < 0)
-		return ret;
-	else
-		return 0;
+	queue_delayed_work(system_power_efficient_wq,
+			   &rdev->disable_work,
+			   msecs_to_jiffies(ms));
+	return 0;
 }
 EXPORT_SYMBOL_GPL(regulator_disable_deferred);
 

Re: [patch] regulator: core: remove some unneeded conditions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

On Thu, Feb 26, 2015 at 11:40:46PM +0300, Dan Carpenter wrote:
> These always true or always false conditions cause static checker
> warnings.
> 
> The check prior to checking "if (pin->enable_count <= 1) {" is
> "if (pin->enable_count > 1) ", so we know that ->enable_count is <= 1
> already and can delete the check.

> The queue_delayed_work() function is type bool so the return value can
> never be less than zero.

Please don't submit multiple changes in a single patch, it makes it much
harder to review things and means that problems with one part of the
change block other bits.  Especially the second bit here looks wrong as
is...

> @@ -2158,13 +2155,10 @@ int regulator_disable_deferred(struct regulator *regulator, int ms)
>  	rdev->deferred_disables++;
>  	mutex_unlock(&rdev->mutex);
>  
> -	ret = queue_delayed_work(system_power_efficient_wq,
> -				 &rdev->disable_work,
> -				 msecs_to_jiffies(ms));
> -	if (ret < 0)
> -		return ret;
> -	else
> -		return 0;
> +	queue_delayed_work(system_power_efficient_wq,
> +			   &rdev->disable_work,
> +			   msecs_to_jiffies(ms));
> +	return 0;

This doesn't look right - it looks like someone changed the return type
at some point and failed to update all the callers.  Presumbly the
return value means something (though a quick grep suggests the change is
correct as the value says if the work was already queued).  It does mean
that the changelog needs revision though, the point isn't that the
return value can't be less than zero it's that the function can't fail.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

Re: [patch] regulator: core: remove some unneeded conditions

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1566 bytes --]

On Thu, Feb 26, 2015 at 11:40:46PM +0300, Dan Carpenter wrote:
> These always true or always false conditions cause static checker
> warnings.
> 
> The check prior to checking "if (pin->enable_count <= 1) {" is
> "if (pin->enable_count > 1) ", so we know that ->enable_count is <= 1
> already and can delete the check.

> The queue_delayed_work() function is type bool so the return value can
> never be less than zero.

Please don't submit multiple changes in a single patch, it makes it much
harder to review things and means that problems with one part of the
change block other bits.  Especially the second bit here looks wrong as
is...

> @@ -2158,13 +2155,10 @@ int regulator_disable_deferred(struct regulator *regulator, int ms)
>  	rdev->deferred_disables++;
>  	mutex_unlock(&rdev->mutex);
>  
> -	ret = queue_delayed_work(system_power_efficient_wq,
> -				 &rdev->disable_work,
> -				 msecs_to_jiffies(ms));
> -	if (ret < 0)
> -		return ret;
> -	else
> -		return 0;
> +	queue_delayed_work(system_power_efficient_wq,
> +			   &rdev->disable_work,
> +			   msecs_to_jiffies(ms));
> +	return 0;

This doesn't look right - it looks like someone changed the return type
at some point and failed to update all the callers.  Presumbly the
return value means something (though a quick grep suggests the change is
correct as the value says if the work was already queued).  It does mean
that the changelog needs revision though, the point isn't that the
return value can't be less than zero it's that the function can't fail.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]