drivers: power_domain: gpio: Allow enabled at boot

[rtalbott-tmo]

Allow for gpio power domain to be enabled at boot time.

[JordanYates]

If regulators can now be powered on at boot the assumption here that a device is unpowered if it is on a power domain itself is no longer valid

[rtalbott-tmo]

Agreed, I'll update the logic.

[JordanYates]

My preference is for something closer to "If set the regulator is automatically powered on at boot time". Minor suggestion.

[rtalbott-tmo]

sure, updated

[JordanYates]

This is not correct because the purpose of the original check is to determine if this device is unpowered, where as what you added is checking against the desired state of this device.
More correctly, the original check should be replaced with pm_device_is_powered

[rtalbott-tmo]

Trying to understand the logic here..
If the device is powered, then we call suspend and put the GPIO inactive,
but - if it's not currently powered and set to power on at boot (cfg->boot) then make the GPIO active,
else - the device is off so disconnect the GPIO.

if (pm_device_is_powered(dev)) {
	pm_device_init_suspended(dev);
	rc = gpio_pin_configure_dt(&cfg->enable, GPIO_OUTPUT_INACTIVE);
} else if (cfg->boot_on) {
	rc = gpio_pin_configure_dt(&cfg->enable, GPIO_OUTPUT_ACTIVE);
} else {
	pm_device_init_off(dev);
	rc = gpio_pin_configure_dt(&cfg->enable, GPIO_DISCONNECTED);
}

Does this make sense? Or is there something I'm missing?

[JordanYates]

The original call (pm_device_on_power_domain) is used to determine if the GPIO regulator is itself powered by another GPIO regulator. Additionally, the previous assumption was that power domains are always powered off at boot.

Taken together, the original reasoning was that if this current device is on a power domain, it is not currently powered, so attempting to enable it via a GPIO is not possible. The new chain should be:

if (!pm_device_is_powered) {
        /* Device is not powered (we cannot respect boot_on) */
  	pm_device_init_off(dev);
	rc = gpio_pin_configure_dt(&cfg->enable, GPIO_DISCONNECTED);
} else if (cfg->boot_on) {
	rc = gpio_pin_configure_dt(&cfg->enable, GPIO_OUTPUT_ACTIVE);
} else {
	pm_device_init_suspended(dev);
	rc = gpio_pin_configure_dt(&cfg->enable, GPIO_OUTPUT_INACTIVE);
}

[rtalbott-tmo]

Thanks for the explanation. I have a much better understanding now.
I have updated accordingly.

[JordanYates]

Thinking about this a bit further, I don't believe the feature should be solved this way, as the expected state of drivers after booting should be consistent. Adding a device specific flag to make this driver consistent is a hack that will need to be removed later anyway. In principle, the expected states are:

• No pm_device_runtime, has power applied: ACTIVE
• No pm_device_runtime, does not have power: OFF
• pm_device_runtime, has power applied: SUSPENDED
• pm_device_runtime, does not have power: OFF

I am in the process of upstreaming initialisation work that does this, which I think is a more appropriate solution.

[rtalbott-tmo]

Sounds good. I'm looking forward to seeing the PR.

[JordanYates]

Alternative that I believe does what you want: #60411