[PATCH v3 3/3] power: regulator: common: fix compilation issue

[Quentin Schulz]

Hi Julien,

On 1/15/26 3:30 PM, Julien Stephan wrote:
[...]
> @@ -65,47 +67,47 @@ int regulator_common_set_enable(const struct udevice *dev,
>   	debug("%s: dev='%s', enable=%d, delay=%d, has_gpio=%d\n", __func__,
>   	      dev->name, enable, plat->startup_delay_us,
>   	      dm_gpio_is_valid(&plat->gpio));
> +
>   	/* Enable GPIO is optional */
> -	if (!dm_gpio_is_valid(&plat->gpio)) {
> -		if (!enable)
> -			return -ENOSYS;
> -		return 0;
> -	}
> +	if (CONFIG_IS_ENABLED(DM_GPIO) && dm_gpio_is_valid(&plat->gpio)) {
> +		/* If previously enabled, increase count */
> +		if (enable && plat->enable_count > 0) {
> +			plat->enable_count++;
> +			return -EALREADY;
> +		}
>   
> -	/* If previously enabled, increase count */
> -	if (enable && plat->enable_count > 0) {
> -		plat->enable_count++;
> -		return -EALREADY;
> -	}
> +		if (!enable) {
> +			if (plat->enable_count > 1) {
> +				/* If enabled multiple times, decrease count */
> +				plat->enable_count--;
> +				return -EBUSY;
> +			} else if (!plat->enable_count) {
> +				/* If already disabled, do nothing */
> +				return -EALREADY;
> +			}
> +		}
>   
> -	if (!enable) {
> -		if (plat->enable_count > 1) {
> -			/* If enabled multiple times, decrease count */
> -			plat->enable_count--;
> -			return -EBUSY;
> -		} else if (!plat->enable_count) {
> -			/* If already disabled, do nothing */
> -			return -EALREADY;
> +		ret = dm_gpio_set_value(&plat->gpio, enable);
> +		if (ret) {
> +			pr_err("Can't set regulator : %s gpio to: %d\n", dev->name,
> +			       enable);
> +			return ret;
>   		}
> -	}
>   
> -	ret = dm_gpio_set_value(&plat->gpio, enable);
> -	if (ret) {
> -		pr_err("Can't set regulator : %s gpio to: %d\n", dev->name,
> -		      enable);
> -		return ret;
> -	}
> +		if (enable && plat->startup_delay_us)
> +			udelay(plat->startup_delay_us);
>   
> -	if (enable && plat->startup_delay_us)
> -		udelay(plat->startup_delay_us);
> +		if (!enable && plat->off_on_delay_us)
> +			udelay(plat->off_on_delay_us);
>   
> -	if (!enable && plat->off_on_delay_us)
> -		udelay(plat->off_on_delay_us);
> +		if (enable)
> +			plat->enable_count++;
> +		else
> +			plat->enable_count--;
>   
> -	if (enable)
> -		plat->enable_count++;
> -	else
> -		plat->enable_count--;
> +	} else {
> +		ret = enable ? 0 : -ENOSYS;
> +	}
>   
> -	return 0;
> +	return ret;

Phew, big diff. Wondering if:

@@ -66,7 +68,7 @@ int regulator_common_set_enable(const struct udevice *dev,
  	      dev->name, enable, plat->startup_delay_us,
  	      dm_gpio_is_valid(&plat->gpio));
  	/* Enable GPIO is optional */
-	if (!dm_gpio_is_valid(&plat->gpio)) {
+	if (!CONFIG_IS_ENABLED(DM_GPIO) || !dm_gpio_is_valid(&plat->gpio)) {
  		if (!enable)
  			return -ENOSYS;
  		return 0;


is enough? The compiler is hopefully smart enough to figure out that an 
always-true if-block which ends with a return means everything after 
this if-block doesn't need to be compiled in.

The current diff looks ok to me, though unnecessarily big I think. In 
any case,

Reviewed-by: Quentin Schulz <quentin.schulz at cherry.de>

Thanks!
Quentin