Skip to contentSkip to conversation

regulator: core: remove some unneeded conditions

PATCH In review Dan Carpenter 2 participants 2 messages Feb 26, 2015 – Mar 3, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 regulator: core: remove some unneeded conditions

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.

drivers/regulator/core.c +7 -13
index f245214..9606bb7 100644
@@ -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);
 
maintainer
#1 lore raw
Quoted 9 lines · 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.

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...

Quoted 5 lines
drivers/regulator/core.c +0 -1
@@ -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,

drivers/regulator/core.c int regulator_disable_deferred(struct regulator *regulator, int ms) · line 2161

Quoted 10 lines
drivers/regulator/core.c +4 -6
-				 &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.