power-domain: Add refcounting

It is very surprising that such an uclass, specifically designed to
handle resources that may be shared by different devices, is not keeping
the count of the number of times a power domain has been
enabled/disabled to avoid shutting it down unexpectedly or disabling it
several times.

Doing this causes troubles on eg. i.MX8MP because disabling power
domains can be done in recursive loops were the same power domain
disabled up to 4 times in a row. PGCs seem to have tight FSM internal
timings to respect and it is easy to produce a race condition that puts
the power domains in an unstable state, leading to ADB400 errors and
later crashes in Linux.

CI tests using power domains are slightly updated to make sure the count
of on/off calls is even and the results match what we *now* expect.

As we do not want to break existing users while stile getting
interesting error codes, the implementation is split between:
- a low-level helper reporting error codes if the requested transition
  could not be operated,
- a higher-level helper ignoring the "non error" codes, like EALREADY and
  EBUSY.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>

diff --git a/drivers/firmware/scmi/sandbox-scmi_devices.c b/drivers/firmware/scmi/sandbox-scmi_devices.c
index 96c2922..9f253b0 100644 (file)
--- a/drivers/firmware/scmi/sandbox-scmi_devices.c
+++ b/drivers/firmware/scmi/sandbox-scmi_devices.c
@@ -163,4 +163,5 @@ U_BOOT_DRIVER(sandbox_scmi_devices) = {
        .priv_auto = sizeof(struct sandbox_scmi_device_priv),
        .remove = sandbox_scmi_devices_remove,
        .probe = sandbox_scmi_devices_probe,
+       .flags = DM_FLAG_DEFAULT_PD_CTRL_OFF,
 };

diff --git a/drivers/power/domain/power-domain-uclass.c b/drivers/power/domain/power-domain-uclass.c
index 938bd8c..a6e5f9e 100644 (file)
--- a/drivers/power/domain/power-domain-uclass.c
+++ b/drivers/power/domain/power-domain-uclass.c
@@ -12,6 +12,10 @@
 #include <power-domain-uclass.h>
 #include <dm/device-internal.h>
 
+struct power_domain_priv {
+       int on_count;
+};
+
 static inline struct power_domain_ops *power_domain_dev_ops(struct udevice *dev)
 {
        return (struct power_domain_ops *)dev->driver->ops;
@@ -107,22 +111,49 @@ int power_domain_free(struct power_domain *power_domain)
        return ops->rfree ? ops->rfree(power_domain) : 0;
 }
 
-int power_domain_on(struct power_domain *power_domain)
+int power_domain_on_lowlevel(struct power_domain *power_domain)
 {
+       struct power_domain_priv *priv = dev_get_uclass_priv(power_domain->dev);
        struct power_domain_ops *ops = power_domain_dev_ops(power_domain->dev);
+       int ret;
 
        debug("%s(power_domain=%p)\n", __func__, power_domain);
 
-       return ops->on ? ops->on(power_domain) : 0;
+       if (priv->on_count++ > 0)
+               return -EALREADY;
+
+       ret = ops->on ? ops->on(power_domain) : 0;
+       if (ret) {
+               priv->on_count--;
+               return ret;
+       }
+
+       return 0;
 }
 
-int power_domain_off(struct power_domain *power_domain)
+int power_domain_off_lowlevel(struct power_domain *power_domain)
 {
+       struct power_domain_priv *priv = dev_get_uclass_priv(power_domain->dev);
        struct power_domain_ops *ops = power_domain_dev_ops(power_domain->dev);
+       int ret;
 
        debug("%s(power_domain=%p)\n", __func__, power_domain);
 
-       return ops->off ? ops->off(power_domain) : 0;
+       if (priv->on_count <= 0) {
+               debug("Power domain %s already off.\n", power_domain->dev->name);
+               return -EALREADY;
+       }
+
+       if (priv->on_count-- > 1)
+               return -EBUSY;
+
+       ret = ops->off ? ops->off(power_domain) : 0;
+       if (ret) {
+               priv->on_count++;
+               return ret;
+       }
+
+       return 0;
 }
 
 #if CONFIG_IS_ENABLED(OF_REAL)
@@ -180,4 +211,5 @@ int dev_power_domain_off(struct udevice *dev)
 UCLASS_DRIVER(power_domain) = {
        .id             = UCLASS_POWER_DOMAIN,
        .name           = "power_domain",
+       .per_device_auto = sizeof(struct power_domain_priv),
 };

diff --git a/drivers/power/domain/sandbox-power-domain-test.c b/drivers/power/domain/sandbox-power-domain-test.c
index 08c15ef..5b53097 100644 (file)
--- a/drivers/power/domain/sandbox-power-domain-test.c
+++ b/drivers/power/domain/sandbox-power-domain-test.c
@@ -51,4 +51,5 @@ U_BOOT_DRIVER(sandbox_power_domain_test) = {
        .id = UCLASS_MISC,
        .of_match = sandbox_power_domain_test_ids,
        .priv_auto      = sizeof(struct sandbox_power_domain_test),
+       .flags = DM_FLAG_DEFAULT_PD_CTRL_OFF,
 };

diff --git a/include/power-domain.h b/include/power-domain.h
index 1852507..ad33dea 100644 (file)
--- a/include/power-domain.h
+++ b/include/power-domain.h
@@ -147,37 +147,81 @@ static inline int power_domain_free(struct power_domain *power_domain)
 #endif
 
 /**
- * power_domain_on - Enable power to a power domain.
+ * power_domain_on_lowlevel - Enable power to a power domain (with refcounting)
  *
  * @power_domain:      A power domain struct that was previously successfully
  *             requested by power_domain_get().
- * Return: 0 if OK, or a negative error code.
+ * Return: 0 if the transition has been performed correctly,
+ *         -EALREADY if the domain is already on,
+ *         a negative error code otherwise.
  */
 #if CONFIG_IS_ENABLED(POWER_DOMAIN)
-int power_domain_on(struct power_domain *power_domain);
+int power_domain_on_lowlevel(struct power_domain *power_domain);
 #else
-static inline int power_domain_on(struct power_domain *power_domain)
+static inline int power_domain_on_lowlevel(struct power_domain *power_domain)
 {
        return -ENOSYS;
 }
 #endif
 
 /**
- * power_domain_off - Disable power to a power domain.
+ * power_domain_on - Enable power to a power domain (ignores the actual state
+ *                   of the power domain)
  *
  * @power_domain:      A power domain struct that was previously successfully
  *             requested by power_domain_get().
- * Return: 0 if OK, or a negative error code.
+ * Return: a negative error code upon error during the transition, 0 otherwise.
+ */
+static inline int power_domain_on(struct power_domain *power_domain)
+{
+       int ret;
+
+       ret = power_domain_on_lowlevel(power_domain);
+       if (ret == -EALREADY)
+               ret = 0;
+
+       return ret;
+}
+
+/**
+ * power_domain_off_lowlevel - Disable power to a power domain (with refcounting)
+ *
+ * @power_domain:      A power domain struct that was previously successfully
+ *             requested by power_domain_get().
+ * Return: 0 if the transition has been performed correctly,
+ *         -EALREADY if the domain is already off,
+ *         -EBUSY if another device is keeping the domain on (but the refcounter
+ *         is decremented),
+ *         a negative error code otherwise.
  */
 #if CONFIG_IS_ENABLED(POWER_DOMAIN)
-int power_domain_off(struct power_domain *power_domain);
+int power_domain_off_lowlevel(struct power_domain *power_domain);
 #else
-static inline int power_domain_off(struct power_domain *power_domain)
+static inline int power_domain_off_lowlevel(struct power_domain *power_domain)
 {
        return -ENOSYS;
 }
 #endif
 
+/**
+ * power_domain_off - Disable power to a power domain (ignores the actual state
+ *                   of the power domain)
+ *
+ * @power_domain:      A power domain struct that was previously successfully
+ *             requested by power_domain_get().
+ * Return: a negative error code upon error during the transition, 0 otherwise.
+ */
+static inline int power_domain_off(struct power_domain *power_domain)
+{
+       int ret;
+
+       ret = power_domain_off_lowlevel(power_domain);
+       if (ret == -EALREADY || ret == -EBUSY)
+               ret = 0;
+
+       return ret;
+}
+
 /**
  * dev_power_domain_on - Enable power domains for a device .
  *

diff --git a/test/dm/power-domain.c b/test/dm/power-domain.c
index 896cf5b..8a95f6b 100644 (file)
--- a/test/dm/power-domain.c
+++ b/test/dm/power-domain.c
@@ -27,7 +27,7 @@ static int dm_test_power_domain(struct unit_test_state *uts)
 
        ut_assertok(uclass_get_device_by_name(UCLASS_MISC, "power-domain-test",
                                              &dev_test));
-       ut_asserteq(1, sandbox_power_domain_query(dev_power_domain,
+       ut_asserteq(0, sandbox_power_domain_query(dev_power_domain,
                                                  TEST_POWER_DOMAIN));
        ut_assertok(sandbox_power_domain_test_get(dev_test));