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

[Mark Brown]

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: signature.asc
Description: Digital signature