Re: [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.sch...@cherry.de>

Thanks!
Quentin