soc: samsung: Modify and enhance power domain driver

From: Pankaj Dubey <[email protected]>

This patch moves Exynos PMU driver implementation from "arm/mach-exynos" to "drivers/mfd". This driver is mainly used for setting misc bits of register from PMU IP of Exynos SoC which will be required to configure before Suspend/Resume. Currently all these settings are done in "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC support, there is a need of DT based implementation of PMU driver.

This driver uses already existing DT binding information.

CC Sangbeom Kim <[email protected]>

CC Samuel Ortiz <[email protected]>

CC Lee Jones <[email protected]>

Signed-off-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 arch/arm/mach-exynos/Kconfig                       |    1 +
 arch/arm/mach-exynos/Makefile                      |    2 +-
 drivers/mfd/Kconfig                                |    9 +++++++++
 drivers/mfd/Makefile                               |    1 +
 .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c  |    0
 5 files changed, 12 insertions(+), 1 deletion(-)
 rename arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%)

Subject: Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

Quoted 16 lines · Amit Daniel Kachhap

From: Pankaj Dubey <[email protected]>

This patch moves Exynos PMU driver implementation from "arm/mach-exynos" to "drivers/mfd". This driver is mainly used for setting misc bits of register from PMU IP of Exynos SoC which will be required to configure before Suspend/Resume. Currently all these settings are done in "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC support, there is a need of DT based implementation of PMU driver.

This driver uses already existing DT binding information.

CC Sangbeom Kim <[email protected]>

CC Samuel Ortiz <[email protected]>

CC Lee Jones <[email protected]>

Signed-off-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

Quoted 8 lines

---
 arch/arm/mach-exynos/Kconfig                       |    1 +
 arch/arm/mach-exynos/Makefile                      |    2 +-
 drivers/mfd/Kconfig                                |    9 +++++++++
 drivers/mfd/Makefile                               |    1 +
 .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c  |    0
 5 files changed, 12 insertions(+), 1 deletion(-)  rename

arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%)

I thought we spoke about this already [1]? What's changed?

[1] https://lkml.org/lkml/2014/4/29/193

--
Lee Jones

Linaro STMicroelectronics Landing Team Lead Linaro.org │ Open source software
for ARM SoCs Follow Linaro: Facebook | Twitter | Blog

On Monday, November 03, 2014 8:56 PM, Lee Jones wrote,

Quoted 29 lines

Subject: Re: [PATCH 02/12] drivers: mfd: Add support for Exynos PMU driver

Quoted 24 lines · Amit Daniel Kachhap

From: Pankaj Dubey <[email protected]>

This patch moves Exynos PMU driver implementation from "arm/mach-exynos" to "drivers/mfd". This driver is mainly used for setting misc bits of register from PMU IP of Exynos SoC which will be required to configure before Suspend/Resume. Currently all these settings are done in "arch/arm/mach-exynos/pmu.c" but moving ahead for ARM64 based SoC support, there is a need of DT based implementation of PMU driver.

This driver uses already existing DT binding information.

CC Sangbeom Kim <[email protected]>

CC Samuel Ortiz <[email protected]>

CC Lee Jones <[email protected]>

Signed-off-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 arch/arm/mach-exynos/Kconfig                       |    1 +
 arch/arm/mach-exynos/Makefile                      |    2 +-
 drivers/mfd/Kconfig                                |    9 +++++++++
 drivers/mfd/Makefile                               |    1 +
 .../mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c  |    0
 5 files changed, 12 insertions(+), 1 deletion(-)  rename

arch/arm/mach-exynos/pmu.c => drivers/mfd/exynos-pmu.c (100%)

Quoted 4 lines

I thought we spoke about this already [1]? What's changed?

[1] https://lkml.org/lkml/2014/4/29/193

Yes correct, we had discussion about this in above post. But at that time I was not having enough points to put forward for keeping Exynos PMU driver in MFD folder.

At that point there was only one idea behind keeping Exynos PMU that in future it will be reused for ARM64 based SoC for PM related functionalities.
But in this series we have implemented Exynos Power Domain for Samsung's ARM64 based SoCs.

As far as I can see this will in following ways -

1: Moving out PM domain specific code from mach-exynos and reuse of same code for ARM64 based SoC also. 2: Since PM domain relies on PMU registers and does not have its own DT binding, MFD client and MFD device is most suitable for making this kind of platform drivers.

2: Since PM domain relies on PMU registers and does not have its own DT binding, MFD client and MFD device
is most suitable for making this kind of platform drivers.

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

4 type of notifications are added,

GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,

			enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd name. The actual invoking of the notfiers will be done by the platform implementing power domain enable/disable low level handlers according to the above defined notification types. This approach will considerably reduce the number of call to notifiers as compared to calling always from core powerdomain subsystem.

Also the registered domain's will be managed inside a cache list and not part of the genpd structure. This will help in registration of notifiers from subsystems (such as clock) even when the PD subsystem is still not initialised. This list also filters out the unregistered pd's requesting notification.

Cc Rafael J. Wysocki <[email protected]>

Reviewed-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |   31 ++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)

drivers/base/power/domain.c +111 -1

index 40bc2f4..e05045e 100644
@@ -46,10 +46,19 @@
 	__retval;								\
 })
 
+struct cache_notify_domains {
+	char *name;
+	/* Node in the cache pm domain name list */
+	struct list_head cache_list_node;
+};
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
+static LIST_HEAD(domain_notify_list);
+static DEFINE_MUTEX(domain_notify_list_lock);
+static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
 
-static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
 	struct generic_pm_domain *genpd = NULL, *gpd;
 

@@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 	mutex_unlock(&gpd_list_lock);
 	return genpd;
 }
+EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
 
 struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {

@@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	mutex_unlock(&gpd_list_lock);
 }
 
+/**
+ * genpd_register_notifier - Register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+	int ret;
+	struct cache_notify_domains *entry;
+
+	if (!pd_name)
+		return -EINVAL;
+
+	/* Search if the pd is already registered */
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, pd_name))
+			break;
+	}
+	mutex_unlock(&domain_notify_list_lock);
+
+	if (entry) {
+		pr_err("%s: pd already exists\n", __func__);
+		return -EINVAL;
+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->name = pd_name;
+
+	mutex_lock(&domain_notify_list_lock);
+	list_add(&entry->cache_list_node, &domain_notify_list);
+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_register(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_unregister_notifier - Un-register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+	int ret;
+	struct cache_notify_domains *entry;
+
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, pd_name))
+			break;
+	}
+	if (!entry) {
+		mutex_unlock(&domain_notify_list_lock);
+		pr_err("%s: Invalid pd name\n", __func__);
+		return -EINVAL;
+	}
+	list_del(&entry->cache_list_node);
+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_unregister(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_invoke_transition_notifier - Calls the matching notification handler.
+ * @genpd: generic power domain structure.
+ * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
+ */
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state)
+{
+	struct cache_notify_domains *entry;
+
+	if (!genpd) {
+		pr_err("Invalid genpd parameter\n");
+		return;
+	}
+
+	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
+			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
+		pr_err("Invalid state parameter\n");
+		return;
+	}
+
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, genpd->name))
+			break;
+	}
+	mutex_unlock(&domain_notify_list_lock);
+	if (!entry) /* Simply ignore */
+		return;
+
+	blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
+				genpd);
+}
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.

include/linux/pm_domain.h +31 -0

index 73e938b..659997f 100644
@@ -25,6 +25,13 @@ enum gpd_status {
 	GPD_STATE_POWER_OFF,	/* PM domain is off */
 };
 
+enum gpd_on_off_state {
+	GPD_OFF_PRE = 0,	/* GPD state before power off */
+	GPD_OFF_POST,		/* GPD state after power off */
+	GPD_ON_PRE,		/* GPD state before power on */
+	GPD_ON_POST,		/* GPD state after power on */
+};
+
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
 	bool (*stop_ok)(struct device *dev);

@@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
+
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name);
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state);
 #else
 
 static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)

@@ -219,6 +232,24 @@ static inline int pm_genpd_name_poweron(const char *domain_name)
 {
 	return -ENOSYS;
 }
+static inline struct
+generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+{
+	return NULL;
+}
+static inline int
+genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+	return 0;
+}
+static inline int
+genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+	return 0;
+}
+static inline void
+genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state) { }
 #define simple_qos_governor NULL
 #define pm_domain_always_on_gov NULL
 #endif

CC Kevin, Ulf, Geert.

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

Quoted 220 lines

4 type of notifications are added,

GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,

                      enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd name. The actual invoking of the notfiers will be done by the platform implementing power domain enable/disable low level handlers according to the above defined notification types. This approach will considerably reduce the number of call to notifiers as compared to calling always from core powerdomain subsystem.

Also the registered domain's will be managed inside a cache list and not part of the genpd structure. This will help in registration of notifiers from subsystems (such as clock) even when the PD subsystem is still not initialised. This list also filters out the unregistered pd's requesting notification.

Cc Rafael J. Wysocki <[email protected]>

Reviewed-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |   31 ++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)

drivers/base/power/domain.c +111 -1

index 40bc2f4..e05045e 100644
@@ -46,10 +46,19 @@
      __retval;                                                               \
 })
 
+struct cache_notify_domains {
+     char *name;
+     /* Node in the cache pm domain name list */
+     struct list_head cache_list_node;
+};
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
+static LIST_HEAD(domain_notify_list);
+static DEFINE_MUTEX(domain_notify_list_lock);
+static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
 
-static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
      struct generic_pm_domain *genpd = NULL, *gpd;
 

@@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
      mutex_unlock(&gpd_list_lock);
      return genpd;
 }
+EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
 
 struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {

@@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
      mutex_unlock(&gpd_list_lock);
 }
 
+/**
+ * genpd_register_notifier - Register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+     int ret;
+     struct cache_notify_domains *entry;
+
+     if (!pd_name)
+             return -EINVAL;
+
+     /* Search if the pd is already registered */
+     mutex_lock(&domain_notify_list_lock);
+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, pd_name))
+                     break;
+     }
+     mutex_unlock(&domain_notify_list_lock);
+
+     if (entry) {
+             pr_err("%s: pd already exists\n", __func__);
+             return -EINVAL;
+     }
+
+     entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+     if (!entry)
+             return -ENOMEM;
+
+     entry->name = pd_name;
+
+     mutex_lock(&domain_notify_list_lock);
+     list_add(&entry->cache_list_node, &domain_notify_list);
+     mutex_unlock(&domain_notify_list_lock);
+     ret = blocking_notifier_chain_register(
+                             &genpd_transition_notifier_list, nb);
+     return ret;
+}
+
+/**
+ * genpd_unregister_notifier - Un-register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+     int ret;
+     struct cache_notify_domains *entry;
+
+     mutex_lock(&domain_notify_list_lock);
+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, pd_name))
+                     break;
+     }
+     if (!entry) {
+             mutex_unlock(&domain_notify_list_lock);
+             pr_err("%s: Invalid pd name\n", __func__);
+             return -EINVAL;
+     }
+     list_del(&entry->cache_list_node);
+     mutex_unlock(&domain_notify_list_lock);
+     ret = blocking_notifier_chain_unregister(
+                             &genpd_transition_notifier_list, nb);
+     return ret;
+}
+
+/**
+ * genpd_invoke_transition_notifier - Calls the matching notification handler.
+ * @genpd: generic power domain structure.
+ * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
+ */
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+                     enum gpd_on_off_state state)
+{
+     struct cache_notify_domains *entry;
+
+     if (!genpd) {
+             pr_err("Invalid genpd parameter\n");
+             return;
+     }
+
+     if (state != GPD_OFF_PRE || state != GPD_OFF_POST
+                     || state != GPD_ON_PRE || state != GPD_ON_POST) {
+             pr_err("Invalid state parameter\n");
+             return;
+     }
+
+     mutex_lock(&domain_notify_list_lock);
+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, genpd->name))
+                     break;
+     }
+     mutex_unlock(&domain_notify_list_lock);
+     if (!entry) /* Simply ignore */
+             return;
+
+     blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
+                             genpd);
+}
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.

include/linux/pm_domain.h +24 -0

index 73e938b..659997f 100644
@@ -25,6 +25,13 @@ enum gpd_status {
      GPD_STATE_POWER_OFF,    /* PM domain is off */
 };
 
+enum gpd_on_off_state {
+     GPD_OFF_PRE = 0,        /* GPD state before power off */
+     GPD_OFF_POST,           /* GPD state after power off */
+     GPD_ON_PRE,             /* GPD state before power on */
+     GPD_ON_POST,            /* GPD state after power on */
+};
+
 struct dev_power_governor {
      bool (*power_down_ok)(struct dev_pm_domain *domain);
      bool (*stop_ok)(struct device *dev);

@@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
+
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name);
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+                     enum gpd_on_off_state state);
 #else
 
 static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)

@@ -219,6 +232,24 @@ static inline int pm_genpd_name_poweron(const char *domain_name)
 {
      return -ENOSYS;
 }
+static inline struct
+generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+{
+     return NULL;
+}
+static inline int
+genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+     return 0;
+}
+static inline int

include/linux/pm_domain.h static inline int pm_genpd_name_poweron(const char *domain_name) · line 255

Quoted 10 lines

include/linux/pm_domain.h +7 -0

+genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+     return 0;
+}
+static inline void
+genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+                     enum gpd_on_off_state state) { }
 #define simple_qos_governor NULL
 #define pm_domain_always_on_gov NULL
 #endif

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 7 lines · Rafael J. Wysocki

CC Kevin, Ulf, Geert.

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

Hi Amit,

This mechanism seems to be needed - somehow.
A similar approach has been posted earlier, I am not sure that discussion was fully settled though.

http://marc.info/?l=linux-pm&m=136448756308203&w=2

Anyway, I plan to review this shortly.

Kind regards
Uffe

Quoted 231 lines

Quoted 220 lines

4 type of notifications are added,

GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,

                      enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd name. The actual invoking of the notfiers will be done by the platform implementing power domain enable/disable low level handlers according to the above defined notification types. This approach will considerably reduce the number of call to notifiers as compared to calling always from core powerdomain subsystem.

Also the registered domain's will be managed inside a cache list and not part of the genpd structure. This will help in registration of notifiers from subsystems (such as clock) even when the PD subsystem is still not initialised. This list also filters out the unregistered pd's requesting notification.

Cc Rafael J. Wysocki <[email protected]>

Reviewed-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |   31 ++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)

drivers/base/power/domain.c +111 -1

index 40bc2f4..e05045e 100644
@@ -46,10 +46,19 @@
      __retval;                                                               \
 })
 
+struct cache_notify_domains {
+     char *name;
+     /* Node in the cache pm domain name list */
+     struct list_head cache_list_node;
+};
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
+static LIST_HEAD(domain_notify_list);
+static DEFINE_MUTEX(domain_notify_list_lock);
+static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
 
-static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
      struct generic_pm_domain *genpd = NULL, *gpd;
 

@@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
      mutex_unlock(&gpd_list_lock);
      return genpd;
 }
+EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
 
 struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {

@@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
      mutex_unlock(&gpd_list_lock);
 }
 
+/**
+ * genpd_register_notifier - Register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+     int ret;
+     struct cache_notify_domains *entry;
+
+     if (!pd_name)
+             return -EINVAL;
+
+     /* Search if the pd is already registered */
+     mutex_lock(&domain_notify_list_lock);
+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, pd_name))
+                     break;
+     }
+     mutex_unlock(&domain_notify_list_lock);
+
+     if (entry) {
+             pr_err("%s: pd already exists\n", __func__);
+             return -EINVAL;
+     }
+
+     entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+     if (!entry)
+             return -ENOMEM;
+
+     entry->name = pd_name;
+
+     mutex_lock(&domain_notify_list_lock);
+     list_add(&entry->cache_list_node, &domain_notify_list);
+     mutex_unlock(&domain_notify_list_lock);
+     ret = blocking_notifier_chain_register(
+                             &genpd_transition_notifier_list, nb);
+     return ret;
+}
+
+/**
+ * genpd_unregister_notifier - Un-register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+     int ret;
+     struct cache_notify_domains *entry;
+
+     mutex_lock(&domain_notify_list_lock);
+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, pd_name))
+                     break;
+     }
+     if (!entry) {
+             mutex_unlock(&domain_notify_list_lock);
+             pr_err("%s: Invalid pd name\n", __func__);
+             return -EINVAL;
+     }
+     list_del(&entry->cache_list_node);
+     mutex_unlock(&domain_notify_list_lock);
+     ret = blocking_notifier_chain_unregister(
+                             &genpd_transition_notifier_list, nb);
+     return ret;
+}
+
+/**
+ * genpd_invoke_transition_notifier - Calls the matching notification handler.
+ * @genpd: generic power domain structure.
+ * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
+ */
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+                     enum gpd_on_off_state state)
+{
+     struct cache_notify_domains *entry;
+
+     if (!genpd) {
+             pr_err("Invalid genpd parameter\n");
+             return;
+     }
+
+     if (state != GPD_OFF_PRE || state != GPD_OFF_POST
+                     || state != GPD_ON_PRE || state != GPD_ON_POST) {
+             pr_err("Invalid state parameter\n");
+             return;
+     }
+
+     mutex_lock(&domain_notify_list_lock);
+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, genpd->name))
+                     break;
+     }
+     mutex_unlock(&domain_notify_list_lock);
+     if (!entry) /* Simply ignore */
+             return;
+
+     blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
+                             genpd);
+}
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.

include/linux/pm_domain.h +24 -0

index 73e938b..659997f 100644
@@ -25,6 +25,13 @@ enum gpd_status {
      GPD_STATE_POWER_OFF,    /* PM domain is off */
 };
 
+enum gpd_on_off_state {
+     GPD_OFF_PRE = 0,        /* GPD state before power off */
+     GPD_OFF_POST,           /* GPD state after power off */
+     GPD_ON_PRE,             /* GPD state before power on */
+     GPD_ON_POST,            /* GPD state after power on */
+};
+
 struct dev_power_governor {
      bool (*power_down_ok)(struct dev_pm_domain *domain);
      bool (*stop_ok)(struct device *dev);

@@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);
 
 extern struct dev_power_governor simple_qos_governor;
 extern struct dev_power_governor pm_domain_always_on_gov;
+
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name);
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+                     enum gpd_on_off_state state);
 #else
 
 static inline struct generic_pm_domain_data *dev_gpd_data(struct device *dev)

@@ -219,6 +232,24 @@ static inline int pm_genpd_name_poweron(const char *domain_name)
 {
      return -ENOSYS;
 }
+static inline struct
+generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+{
+     return NULL;
+}
+static inline int
+genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+     return 0;
+}
+static inline int

include/linux/pm_domain.h static inline int pm_genpd_name_poweron(const char *domain_name) · line 255

Quoted 10 lines

include/linux/pm_domain.h +7 -0

+genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+     return 0;
+}
+static inline void
+genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+                     enum gpd_on_off_state state) { }
 #define simple_qos_governor NULL
 #define pm_domain_always_on_gov NULL
 #endif

Quoted 7 lines

--
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

CC Kevin, Ulf, Geert.

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

+Mike Turquette

Hi Amit,

Quoted 7 lines · Rafael J. Wysocki

CC Kevin, Ulf, Geert.

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

The runtime PM framework already provides callbacks that are useful for context save/restore for devices. Could you please describe in more detail which registers in which kind of devices need to be saved/restored, and why they cannot be saved/restored using existing mechanisms.

Personally, I'm uncomfortable with notifiers like this because it suggests that underlying frameworks are not doing the right thing, or are not being used. (I also don't like the implementation here where a single global notifier list is maintained by the core, but the notifiers are actually triggered by SoC specific code.)

IIUC, the usage in this series seems to be that certain clock related registers need to be saved/restored across a power domain transition.

Wouldn't an alternative solution be to add a feature to the clock driver such that the state of each clock is saved when the clock is disabled,
and restored when the clock is enabled? That would allow any clock
context to survive any power domain transtion also, correct?

I have some issues with the implementaion as well, but I think we need to first sort out the real need for this before going into those details.

Kevin

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 7 lines · Kevin Hilman

Quoted 5 lines · Rafael J. Wysocki

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

Quoted 5 lines

The runtime PM framework already provides callbacks that are useful for context save/restore for devices. Could you please describe in more detail which registers in which kind of devices need to be saved/restored, and why they cannot be saved/restored using existing mechanisms.

Basically the requirement is mandated by exynos7 manual. It tells that before turning off the power domain, some clock registers need to saved and should be restored just after turning the power domain. These clock registers are not necessarily gate clocks but could be mux clocks etc. The driver may not have all information of these clocks also. I suppose these are Soc specific changes but drivers should work across Socs. This behavior is almost similar to suspend/resume case where a whole list of clock registers are saved/restored.

Even earlier post by Sylwester (https://lkml.org/lkml/2014/8/5/182)
also points to the need of this feature.

Quoted 6 lines

Personally, I'm uncomfortable with notifiers like this because it suggests that underlying frameworks are not doing the right thing, or are not being used. (I also don't like the implementation here where a single global notifier list is maintained by the core, but the notifiers are actually triggered by SoC specific code.)

Yes right the global notifier block can be moved to per genpd structure. Also SoC trigger can be moved to core files.

Quoted 4 lines

IIUC, the usage in this series seems to be that certain clock related registers need to be saved/restored across a power domain transition.

Quoted 4 lines

Wouldn't an alternative solution be to add a feature to the clock driver such that the state of each clock is saved when the clock is disabled,
and restored when the clock is enabled? That would allow any clock
context to survive any power domain transtion also, correct?

I also thought about same. But the trigger point for this would be driver calling clk disable/enable and not the power domain. so this will lead to lot of save/restore for each power domain child.

Quoted 14 lines · amit daniel kachhap

Quoted 7 lines · Kevin Hilman

Quoted 5 lines · Rafael J. Wysocki

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

Quoted 5 lines

The runtime PM framework already provides callbacks that are useful for context save/restore for devices. Could you please describe in more detail which registers in which kind of devices need to be saved/restored, and why they cannot be saved/restored using existing mechanisms.

Quoted 8 lines

Basically the requirement is mandated by exynos7 manual. It tells that before turning off the power domain, some clock registers need to saved and should be restored just after turning the power domain. These clock registers are not necessarily gate clocks but could be mux clocks etc. The driver may not have all information of these clocks also. I suppose these are Soc specific changes but drivers should work across Socs. This behavior is almost similar to suspend/resume case where a whole list of clock registers are saved/restored.

Indeed, the somehow complicated power domain power on/off sequences are SoC specific. They involve not only groups of clocks (usually gate, mux clock registers of all devices in a power domain) but also SoC-specific PMU (Power Management Unit) registers.
I assume it would be inappropriate to push such details to device drivers. Moreover, a device driver could not be even loaded.

Since the clocks' state is already maintained by clk driver we came up with an idea of having generic calls from power domain driver back to the clock controller driver.

It might not to be the prettiest solution, nevertheless I couldn't come up with a better one which would satisfy all the requirements. The power domain should only be provided for use with all the clk/PMU sequences handling in place.

Clocks need to also be touched before a power domain switch on or off, so attaching the clock controller to some power domain wouldn't help, unless we modify/add to existing power domain related callbacks for devices. Another issue is the clock controller device would need to be attached to multiple power domains, and for each power domain the power on/off sequence is usually slightly different.

There are also hierarchical power domains where each: the master and the sub-domain need they own sequence and device usually is attached only to a sub-domain.

Quoted 20 lines

Even earlier post by Sylwester (https://lkml.org/lkml/2014/8/5/182)
also points to the need of this feature.

Quoted 6 lines

Personally, I'm uncomfortable with notifiers like this because it suggests that underlying frameworks are not doing the right thing, or are not being used. (I also don't like the implementation here where a single global notifier list is maintained by the core, but the notifiers are actually triggered by SoC specific code.)

Yes right the global notifier block can be moved to per genpd structure. Also SoC trigger can be moved to core files.

Quoted 8 lines

IIUC, the usage in this series seems to be that certain clock related registers need to be saved/restored across a power domain transition.

Wouldn't an alternative solution be to add a feature to the clock driver such that the state of each clock is saved when the clock is disabled,
and restored when the clock is enabled? That would allow any clock
context to survive any power domain transtion also, correct?

Quoted 3 lines

I also thought about same. But the trigger point for this would be driver calling clk disable/enable and not the power domain. so this will lead to lot of save/restore for each power domain child.

Even though we would have saved/restored at that points still the power domain driver would need to enforce some specific clock/PMU registers state before/after a power domain state transition. And this is what I found difficult with the existing APIs.

Quoted 31 lines · Sylwester Nawrocki

Quoted 14 lines · amit daniel kachhap

Quoted 7 lines · Kevin Hilman

Quoted 5 lines · Rafael J. Wysocki

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

Quoted 5 lines

The runtime PM framework already provides callbacks that are useful for context save/restore for devices. Could you please describe in more detail which registers in which kind of devices need to be saved/restored, and why they cannot be saved/restored using existing mechanisms.

Quoted 8 lines

Basically the requirement is mandated by exynos7 manual. It tells that before turning off the power domain, some clock registers need to saved and should be restored just after turning the power domain. These clock registers are not necessarily gate clocks but could be mux clocks etc. The driver may not have all information of these clocks also. I suppose these are Soc specific changes but drivers should work across Socs. This behavior is almost similar to suspend/resume case where a whole list of clock registers are saved/restored.

Indeed, the somehow complicated power domain power on/off sequences are SoC specific. They involve not only groups of clocks (usually gate, mux clock registers of all devices in a power domain) but also SoC-specific PMU (Power Management Unit) registers.
I assume it would be inappropriate to push such details to device drivers. Moreover, a device driver could not be even loaded.

Quoted 3 lines

Since the clocks' state is already maintained by clk driver we came up with an idea of having generic calls from power domain driver back to the clock controller driver.

For the clock tree, it still seems to me that this is better handled in the SoC clock driver. For example, when a power domain is about to be gated, all the devices in that domain are runtime suspended, and presumably all of their gate clocks are disabled. Now, doesn't the clock driver know the clock tree parent-child hierarchy and shouldn't it be capable of saving the state of parent clocks (like mux clocks) etc?

Stated diffrently, it still seems to me like we're pushing functionality in PM core notifiers that should be the responsibility of subsystem drivers.

Quoted 36 lines

It might not to be the prettiest solution, nevertheless I couldn't come up with a better one which would satisfy all the requirements. The power domain should only be provided for use with all the clk/PMU sequences handling in place.

Clocks need to also be touched before a power domain switch on or off, so attaching the clock controller to some power domain wouldn't help, unless we modify/add to existing power domain related callbacks for devices. Another issue is the clock controller device would need to be attached to multiple power domains, and for each power domain the power on/off sequence is usually slightly different.

There are also hierarchical power domains where each: the master and the sub-domain need they own sequence and device usually is attached only to a sub-domain.

Quoted 20 lines

Even earlier post by Sylwester (https://lkml.org/lkml/2014/8/5/182)
also points to the need of this feature.

Quoted 6 lines

Personally, I'm uncomfortable with notifiers like this because it suggests that underlying frameworks are not doing the right thing, or are not being used. (I also don't like the implementation here where a single global notifier list is maintained by the core, but the notifiers are actually triggered by SoC specific code.)

Yes right the global notifier block can be moved to per genpd structure. Also SoC trigger can be moved to core files.

Quoted 8 lines

IIUC, the usage in this series seems to be that certain clock related registers need to be saved/restored across a power domain transition.

Wouldn't an alternative solution be to add a feature to the clock driver such that the state of each clock is saved when the clock is disabled,
and restored when the clock is enabled? That would allow any clock
context to survive any power domain transtion also, correct?

Quoted 8 lines

Quoted 3 lines

I also thought about same. But the trigger point for this would be driver calling clk disable/enable and not the power domain. so this will lead to lot of save/restore for each power domain child.

Even though we would have saved/restored at that points still the power domain driver would need to enforce some specific clock/PMU registers state before/after a power domain state transition. And this is what I found difficult with the existing APIs.

This is what I'm not understanding.

Why can't the power domain driver's power_on/power_off callback just call the PMU APIs and/or the clk_enable/_disable calls it needs?

Kevin

--
To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo at vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

Quoted 4 lines · Sylwester Nawrocki

Quoted 3 lines · amit daniel kachhap

Quoted 2 lines · Kevin Hilman

Quoted 1 line · Rafael J. Wysocki

On Monday, November 03, 2014 09:23:01 AM Amit Daniel Kachhap wrote:

Quoted 10 lines

Indeed, the somehow complicated power domain power on/off sequences are SoC specific. They involve not only groups of clocks (usually gate, mux clock registers of all devices in a power domain) but also SoC-specific PMU (Power Management Unit) registers.
I assume it would be inappropriate to push such details to device drivers. Moreover, a device driver could not be even loaded.

Since the clocks' state is already maintained by clk driver we came up with an idea of having generic calls from power domain driver back to the clock controller driver.

For the clock tree, it still seems to me that this is better handled in the SoC clock driver. For example, when a power domain is about to be gated, all the devices in that domain are runtime suspended, and presumably all of their gate clocks are disabled. Now, doesn't the clock driver know the clock tree parent-child hierarchy and shouldn't it be capable of saving the state of parent clocks (like mux clocks) etc?

Stated diffrently, it still seems to me like we're pushing functionality in PM core notifiers that should be the responsibility of subsystem drivers.

Quoted 17 lines

Quoted 17 lines

Quoted 5 lines

Personally, I'm uncomfortable with notifiers like this because it suggests that underlying frameworks are not doing the right thing, or are not being used. (I also don't like the implementation here where a single global notifier list is maintained by the core, but the notifiers are actually triggered by SoC specific code.)

Yes right the global notifier block can be moved to per genpd structure. Also SoC trigger can be moved to core files.

Quoted 8 lines

IIUC, the usage in this series seems to be that certain clock related registers need to be saved/restored across a power domain transition.

Wouldn't an alternative solution be to add a feature to the clock driver such that the state of each clock is saved when the clock is disabled,
and restored when the clock is enabled? That would allow any clock
context to survive any power domain transtion also, correct?

Quoted 8 lines

Quoted 3 lines

I also thought about same. But the trigger point for this would be driver calling clk disable/enable and not the power domain. so this will lead to lot of save/restore for each power domain child.

Even though we would have saved/restored at that points still the power domain driver would need to enforce some specific clock/PMU registers state before/after a power domain state transition. And this is what I found difficult with the existing APIs.

This is what I'm not understanding.

Why can't the power domain driver's power_on/power_off callback just call the PMU APIs and/or the clk_enable/_disable calls it needs?

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

4 type of notifications are added,

GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,

			enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd name. The actual invoking of the notfiers will be done by the platform implementing power domain enable/disable low level handlers according to the above defined notification types. This approach will considerably reduce the number of call to notifiers as compared to calling always from core powerdomain subsystem.

Also the registered domain's will be managed inside a cache list and not part of the genpd structure. This will help in registration of notifiers from subsystems (such as clock) even when the PD subsystem is still not initialised. This list also filters out the unregistered pd's requesting notification.

Cc Rafael J. Wysocki <[email protected]>

Reviewed-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |   31 ++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)

drivers/base/power/domain.c +25 -1

index 40bc2f4..e05045e 100644
@@ -46,10 +46,19 @@
 	__retval;								\
 })
 
+struct cache_notify_domains {
+	char *name;
+	/* Node in the cache pm domain name list */
+	struct list_head cache_list_node;
+};
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
+static LIST_HEAD(domain_notify_list);
+static DEFINE_MUTEX(domain_notify_list_lock);
+static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
 
-static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
 	struct generic_pm_domain *genpd = NULL, *gpd;
 

@@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 	mutex_unlock(&gpd_list_lock);
 	return genpd;
 }
+EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
 
 struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {

@@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	mutex_unlock(&gpd_list_lock);
 }
 
+/**
+ * genpd_register_notifier - Register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+	int ret;
+	struct cache_notify_domains *entry;
+
+	if (!pd_name)
+		return -EINVAL;
+
+	/* Search if the pd is already registered */

drivers/base/power/domain.c +27 -0

+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->name = pd_name;
+
+	mutex_lock(&domain_notify_list_lock);
+	list_add(&entry->cache_list_node, &domain_notify_list);
+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_register(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_unregister_notifier - Un-register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+	int ret;
+	struct cache_notify_domains *entry;
+
+	mutex_lock(&domain_notify_list_lock);

drivers/base/power/domain.c +24 -0

+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_unregister(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_invoke_transition_notifier - Calls the matching notification handler.
+ * @genpd: generic power domain structure.
+ * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
+ */
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state)
+{
+	struct cache_notify_domains *entry;
+
+	if (!genpd) {
+		pr_err("Invalid genpd parameter\n");
+		return;
+	}
+
+	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
+			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
+		pr_err("Invalid state parameter\n");

drivers/base/power/domain.c +1 -0

+}
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.

include/linux/pm_domain.h +0 -0

index 73e938b..659997f 100644

Hi,

Cc Ulf, Kevin, Geert.

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

Other specific use cases I could add here is gating selected clocks to ensure proper signal propagation for devices attached to a power domain, e.g. ungating selected clocks before the power domain on and restoring them to previous state after the power domain was switched on.

Quoted 19 lines

4 type of notifications are added,

GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,

			enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd name. The actual invoking of the notfiers will be done by the platform implementing power domain enable/disable low level handlers according to the above defined notification types. This approach will considerably reduce the number of call to notifiers as compared to calling always from core powerdomain subsystem.

Quoted 4 lines

Also the registered domain's will be managed inside a cache list and not part of the genpd structure. This will help in registration of notifiers from subsystems (such as clock) even when the PD subsystem is still not initialised. This list also filters out the unregistered pd's requesting notification.

This patch is somewhat similar my patch adding power domain state change notifications [1]. I have already a reworked version of that patch, with the notifier list moved to struct generic_pm_domain and using genpd->lock instead of dev->power.lock. Anyway, while I'd leave the decision about the location from where the notifier chains are supposed to be called to the subsystem's maintainers preference, I'm rather reluctant to having one global notifiers list for all possible power domains and all the notification clients. The possibly long list needs to be traversed at each notifier call and it seems in your implementation any registered user of the notification gets notifications for all possible power domains.

[1] https://lkml.org/lkml/2014/8/5/182

Quoted 59 lines

Cc Rafael J. Wysocki <[email protected]>

Reviewed-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |   31 ++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)

drivers/base/power/domain.c +25 -1

index 40bc2f4..e05045e 100644
@@ -46,10 +46,19 @@
 	__retval;								\
 })
 
+struct cache_notify_domains {
+	char *name;
+	/* Node in the cache pm domain name list */
+	struct list_head cache_list_node;
+};
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
+static LIST_HEAD(domain_notify_list);
+static DEFINE_MUTEX(domain_notify_list_lock);
+static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
 
-static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
 	struct generic_pm_domain *genpd = NULL, *gpd;
 

@@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 	mutex_unlock(&gpd_list_lock);
 	return genpd;
 }
+EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
 
 struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {

@@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
 	mutex_unlock(&gpd_list_lock);
 }
 
+/**
+ * genpd_register_notifier - Register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+	int ret;
+	struct cache_notify_domains *entry;
+
+	if (!pd_name)
+		return -EINVAL;
+
+	/* Search if the pd is already registered */

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 1944

Quoted 10 lines

drivers/base/power/domain.c +10 -0

+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, pd_name))
+			break;
+	}
+	mutex_unlock(&domain_notify_list_lock);
+
+	if (entry) {
+		pr_err("%s: pd already exists\n", __func__);
+		return -EINVAL;

I suspect this code doesn't work as expected. 'entry' will be NULL only in case of an empty domain_notify_list list. Have you tested multiple calls to genpd_register_notifier() ? It looks like only the first call would work.

Quoted 27 lines

drivers/base/power/domain.c +27 -0

+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->name = pd_name;
+
+	mutex_lock(&domain_notify_list_lock);
+	list_add(&entry->cache_list_node, &domain_notify_list);
+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_register(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_unregister_notifier - Un-register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+	int ret;
+	struct cache_notify_domains *entry;
+
+	mutex_lock(&domain_notify_list_lock);

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 1981

Quoted 10 lines

drivers/base/power/domain.c +10 -0

+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, pd_name))
+			break;
+	}
+	if (!entry) {
+		mutex_unlock(&domain_notify_list_lock);
+		pr_err("%s: Invalid pd name\n", __func__);
+		return -EINVAL;
+	}
+	list_del(&entry->cache_list_node);

'entry' will not be NULL even when the requested notification entry was not found above. In such case it looks like last entry in the domain_notify_list list will be removed, not something we would expect.

Quoted 24 lines

drivers/base/power/domain.c +24 -0

+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_unregister(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_invoke_transition_notifier - Calls the matching notification handler.
+ * @genpd: generic power domain structure.
+ * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
+ */
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state)
+{
+	struct cache_notify_domains *entry;
+
+	if (!genpd) {
+		pr_err("Invalid genpd parameter\n");
+		return;
+	}
+
+	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
+			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
+		pr_err("Invalid state parameter\n");

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 2015

Quoted 10 lines

drivers/base/power/domain.c +10 -0

+		return;
+	}
+
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, genpd->name))
+			break;
+	}
+	mutex_unlock(&domain_notify_list_lock);
+	if (!entry) /* Simply ignore */

Similar issue here, this condition will only be true when the notifications list is empty.

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 2019

Quoted 4 lines

drivers/base/power/domain.c +4 -0

+		return;
+
+	blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
+				genpd);

And finally regardless of to what power domain the notification user has registered it will get notification for each possible power domain in the system? Are the notification users supposed to test the 'genpd' argument to react on a specific power domain? If so, how? I guess not by nother strcmp() ?

Quoted 9 lines

drivers/base/power/domain.c +1 -0

+}
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.

include/linux/pm_domain.h +0 -0

index 73e938b..659997f 100644

include/linux/pm_domain.h enum gpd_status { · line 29

Quoted 5 lines

include/linux/pm_domain.h +2 -0

@@ -25,6 +25,13 @@ enum gpd_status {
 	GPD_STATE_POWER_OFF,	/* PM domain is off */
 };
 
+enum gpd_on_off_state {
+	GPD_OFF_PRE = 0,	/* GPD state before power off */

The assignment is not needed.

include/linux/pm_domain.h extern int pm_genpd_name_poweron(const char *domain_name);

Quoted 9 lines

include/linux/pm_domain.h +5 -0

+	GPD_OFF_POST,		/* GPD state after power off */
+	GPD_ON_PRE,		/* GPD state before power on */
+	GPD_ON_POST,		/* GPD state after power on */
+};
+
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
 	bool (*stop_ok)(struct device *dev);

@@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);

--
Regards,
Sylwester

To: [email protected]
Cc: Amit Daniel Kachhap; [email protected]; linux- [email protected]; [email protected]; [email protected];

J. Wysocki; Beata Michalska; [email protected]; Kevin Hilman; Ulf

Subject: Re: [PATCH 03/12] PM / Domains: Add notifier support for power

transitions

Now really filling in the CC list, apologies for duplicate post.

Quoted 28 lines · Sylwester Nawrocki

Hi,

Cc Ulf, Kevin, Geert.

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

Other specific use cases I could add here is gating selected clocks to ensure proper signal propagation for devices attached to a power domain, e.g. ungating selected clocks before the power domain on and restoring them to previous state after the power domain was switched on.

Quoted 12 lines

4 type of notifications are added,

GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain

Quoted 3 lines

Quoted 3 lines

			enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd

Quoted 9 lines

Quoted 9 lines

The actual invoking of the notfiers will be done by the platform implementing power domain enable/disable low level handlers according to the above defined notification types. This approach will considerably reduce the number of call to notifiers as compared to calling always from core powerdomain subsystem.

Also the registered domain's will be managed inside a cache list and not part of the genpd structure. This will help in registration of notifiers from subsystems (such as clock) even when the PD subsystem is

initialised.

Quoted 1 line

Quoted 1 line

This list also filters out the unregistered pd's requesting

Quoted 8 lines

This patch is somewhat similar my patch adding power domain state change notifications [1]. I have already a reworked version of that patch, with the notifier list moved to struct generic_pm_domain and using genpd->lock instead of dev->power.lock. Anyway, while I'd leave the decision about the location from where the notifier chains are supposed to be called to the subsystem's maintainers preference, I'm rather reluctant to having one global notifiers list for all possible

and all the notification clients.

Quoted 11 lines

The possibly long list needs to be traversed at each notifier call and it seems in your implementation any registered user of the notification gets notifications for all possible power domains.

[1] https://lkml.org/lkml/2014/8/5/182

Quoted 5 lines

Cc Rafael J. Wysocki <[email protected]>

Reviewed-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 drivers/base/power/domain.c |  112

++++++++++++++++++++++++++++++++++++++++++-

Quoted 3 lines

Quoted 3 lines

 include/linux/pm_domain.h   |   31 ++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)

Quoted 6 lines

drivers/base/power/domain.c line 46

Quoted 6 lines

(quoted diff) +0 -0

b/drivers/base/power/domain.c index 40bc2f4..e05045e 100644

drivers/base/power/domain.c +0 -0

@@ -46,10 +46,19 @@
 	__retval;

Quoted 21 lines

Quoted 21 lines

drivers/base/power/domain.c +11 -1

 })
 
+struct cache_notify_domains {
+	char *name;
+	/* Node in the cache pm domain name list */
+	struct list_head cache_list_node;
+};
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
+static LIST_HEAD(domain_notify_list); static
+DEFINE_MUTEX(domain_notify_list_lock);
+static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
 
-static struct generic_pm_domain *pm_genpd_lookup_name(const char
 *domain_name)
+struct generic_pm_domain *pm_genpd_lookup_name(const char
+*domain_name)
 {
 	struct generic_pm_domain *genpd = NULL, *gpd;

Quoted 1 line

drivers/base/power/domain.c static struct generic_pm_domain

Quoted 1 line

drivers/base/power/domain.c +0 -0

@@ -66,6 +75,7 @@ static struct generic_pm_domain

*pm_genpd_lookup_name(const char *domain_name)

Quoted 7 lines

Quoted 7 lines

drivers/base/power/domain.c +1 -1

 	mutex_unlock(&gpd_list_lock);
 	return genpd;
 }
+EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
 
 struct generic_pm_domain *dev_to_genpd(struct device *dev)  { @@
-1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain

Quoted 5 lines

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd,

Quoted 5 lines

drivers/base/power/domain.c +2 -0

 	mutex_unlock(&gpd_list_lock);
 }
 
+/**
+ * genpd_register_notifier - Register a PM domain for future

Quoted 20 lines

Quoted 20 lines

+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */

drivers/base/power/domain.c +17 -0

+int genpd_register_notifier(struct notifier_block *nb, char
+*pd_name) {
+	int ret;
+	struct cache_notify_domains *entry;
+
+	if (!pd_name)
+		return -EINVAL;
+
+	/* Search if the pd is already registered */
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, pd_name))
+			break;
+	}
+	mutex_unlock(&domain_notify_list_lock);
+
+	if (entry) {

Quoted 8 lines

drivers/base/power/domain.c

Quoted 2 lines

drivers/base/power/domain.c +2 -0

+		pr_err("%s: pd already exists\n", __func__);
+		return -EINVAL;

I suspect this code doesn't work as expected. 'entry' will be NULL only in case of an empty domain_notify_list list. Have you tested multiple calls to genpd_register_notifier() ? It looks like only the first call would work.

Quoted 10 lines

Quoted 10 lines

drivers/base/power/domain.c +10 -0

+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	entry->name = pd_name;
+
+	mutex_lock(&domain_notify_list_lock);
+	list_add(&entry->cache_list_node, &domain_notify_list);

Quoted 8 lines

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd,

Quoted 8 lines

drivers/base/power/domain.c +8 -0

+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_register(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_unregister_notifier - Un-register a PM domain for future

Quoted 24 lines

Quoted 9 lines

+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */

drivers/base/power/domain.c +6 -0

+int genpd_unregister_notifier(struct notifier_block *nb, char
+*pd_name) {
+	int ret;
+	struct cache_notify_domains *entry;
+
+	mutex_lock(&domain_notify_list_lock);

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 1981

Quoted 10 lines

drivers/base/power/domain.c +10 -0

+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, pd_name))
+			break;
+	}
+	if (!entry) {
+		mutex_unlock(&domain_notify_list_lock);
+		pr_err("%s: Invalid pd name\n", __func__);
+		return -EINVAL;
+	}
+	list_del(&entry->cache_list_node);

'entry' will not be NULL even when the requested notification entry was not found above. In such case it looks like last entry in the domain_notify_list list will be removed, not something we would expect.

Quoted 8 lines

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd,

Quoted 8 lines

drivers/base/power/domain.c +8 -0

+	mutex_unlock(&domain_notify_list_lock);
+	ret = blocking_notifier_chain_unregister(
+				&genpd_transition_notifier_list, nb);
+	return ret;
+}
+
+/**
+ * genpd_invoke_transition_notifier - Calls the matching notification

Quoted 2 lines

Quoted 2 lines

+ * @genpd: generic power domain structure.
+ * @state: can be of type -

GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.

Quoted 27 lines

Quoted 14 lines

+ */

drivers/base/power/domain.c +13 -0

+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+			enum gpd_on_off_state state)
+{
+	struct cache_notify_domains *entry;
+
+	if (!genpd) {
+		pr_err("Invalid genpd parameter\n");
+		return;
+	}
+
+	if (state != GPD_OFF_PRE || state != GPD_OFF_POST
+			|| state != GPD_ON_PRE || state != GPD_ON_POST) {
+		pr_err("Invalid state parameter\n");

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 2015

Quoted 10 lines

drivers/base/power/domain.c +10 -0

+		return;
+	}
+
+	mutex_lock(&domain_notify_list_lock);
+	list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+		if (!strcmp(entry->name, genpd->name))
+			break;
+	}
+	mutex_unlock(&domain_notify_list_lock);
+	if (!entry) /* Simply ignore */

Similar issue here, this condition will only be true when the notifications list is empty.

Quoted 42 lines

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 2019

Quoted 5 lines

drivers/base/power/domain.c +4 -0

+		return;
+
+	blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
+				genpd);

And finally regardless of to what power domain the notification user has registered it will get notification for each possible power domain in the system? Are the notification users supposed to test the 'genpd' argument to react on a specific power domain? If so, how? I guess not by nother strcmp() ?

Quoted 9 lines

drivers/base/power/domain.c +1 -0

+}
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.

include/linux/pm_domain.h +0 -0

index 73e938b..659997f 100644

include/linux/pm_domain.h enum gpd_status { · line 29

Quoted 5 lines

include/linux/pm_domain.h +2 -0

@@ -25,6 +25,13 @@ enum gpd_status {
 	GPD_STATE_POWER_OFF,	/* PM domain is off */
 };
 
+enum gpd_on_off_state {
+	GPD_OFF_PRE = 0,	/* GPD state before power off */

The assignment is not needed.

Quoted 9 lines

include/linux/pm_domain.h +5 -0

+	GPD_OFF_POST,		/* GPD state after power off */
+	GPD_ON_PRE,		/* GPD state before power on */
+	GPD_ON_POST,		/* GPD state after power on */
+};
+
 struct dev_power_governor {
 	bool (*power_down_ok)(struct dev_pm_domain *domain);
 	bool (*stop_ok)(struct device *dev); @@ -148,6 +155,12 @@ extern

int pm_genpd_name_poweron(const char *domain_name);

--
Regards,
Sylwester

Hi,

Cc Ulf, Kevin, Geert.

Quoted 4 lines · Amit Daniel Kachhap

These power domain transition notifiers will assist in carrying out some activity associated with domain power on/off such as some registers which may lose its contents and need save/restore across domain power off/on.

Other specific use cases I could add here is gating selected clocks to ensure proper signal propagation for devices attached to a power domain, e.g. ungating selected clocks before the power domain on and restoring them to previous state after the power domain was switched on.

Quoted 20 lines

4 type of notifications are added,

GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,

                      enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd name. The actual invoking of the notfiers will be done by the platform implementing power domain enable/disable low level handlers according to the above defined notification types. This approach will considerably reduce the number of call to notifiers as compared to calling always from core powerdomain subsystem.

Quoted 4 lines

Also the registered domain's will be managed inside a cache list and not part of the genpd structure. This will help in registration of notifiers from subsystems (such as clock) even when the PD subsystem is still not initialised. This list also filters out the unregistered pd's requesting notification.

This patch is somewhat similar my patch adding power domain state change notifications [1]. I have already a reworked version of that patch, with the notifier list moved to struct generic_pm_domain and using genpd->lock instead

of dev->power.lock. Anyway, while I'd leave the decision about the location from where the notifier chains are supposed to be called to the subsystem's maintainers preference, I'm rather reluctant to having one global notifiers list for all possible power domains and all the notification clients. The possibly long list needs to be traversed at each notifier call and it seems in your implementation any registered user of the notification gets notifications for all possible power domains.

[1] https://lkml.org/lkml/2014/8/5/182

Quoted 67 lines

Cc Rafael J. Wysocki <[email protected]>

Reviewed-by Pankaj Dubey <[email protected]>

Signed-off-by Amit Daniel Kachhap <[email protected]>

---
 drivers/base/power/domain.c |  112 ++++++++++++++++++++++++++++++++++++++++++-
 include/linux/pm_domain.h   |   31 ++++++++++++
 2 files changed, 142 insertions(+), 1 deletion(-)

drivers/base/power/domain.c +32 -1

index 40bc2f4..e05045e 100644
@@ -46,10 +46,19 @@
      __retval;                                                               \
 })
 
+struct cache_notify_domains {
+     char *name;
+     /* Node in the cache pm domain name list */
+     struct list_head cache_list_node;
+};
+
 static LIST_HEAD(gpd_list);
 static DEFINE_MUTEX(gpd_list_lock);
+static LIST_HEAD(domain_notify_list);
+static DEFINE_MUTEX(domain_notify_list_lock);
+static BLOCKING_NOTIFIER_HEAD(genpd_transition_notifier_list);
 
-static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
+struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
 {
      struct generic_pm_domain *genpd = NULL, *gpd;
 

@@ -66,6 +75,7 @@ static struct generic_pm_domain *pm_genpd_lookup_name(const char *domain_name)
      mutex_unlock(&gpd_list_lock);
      return genpd;
 }
+EXPORT_SYMBOL_GPL(pm_genpd_lookup_name);
 
 struct generic_pm_domain *dev_to_genpd(struct device *dev)
 {

@@ -1908,6 +1918,106 @@ void pm_genpd_init(struct generic_pm_domain *genpd,
      mutex_unlock(&gpd_list_lock);
 }
 
+/**
+ * genpd_register_notifier - Register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_register_notifier(struct notifier_block *nb, char *pd_name)
+{
+     int ret;
+     struct cache_notify_domains *entry;
+
+     if (!pd_name)
+             return -EINVAL;
+
+     /* Search if the pd is already registered */
+     mutex_lock(&domain_notify_list_lock);
+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, pd_name))
+                     break;
+     }
+     mutex_unlock(&domain_notify_list_lock);
+

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 1944

Quoted 3 lines

drivers/base/power/domain.c +3 -0

+     if (entry) {
+             pr_err("%s: pd already exists\n", __func__);
+             return -EINVAL;

I suspect this code doesn't work as expected. 'entry' will be NULL only in case of an empty domain_notify_list list. Have you tested multiple calls to genpd_register_notifier() ? It looks like only the first call would work.

Quoted 28 lines

drivers/base/power/domain.c +27 -0

+     }
+
+     entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+     if (!entry)
+             return -ENOMEM;
+
+     entry->name = pd_name;
+
+     mutex_lock(&domain_notify_list_lock);
+     list_add(&entry->cache_list_node, &domain_notify_list);
+     mutex_unlock(&domain_notify_list_lock);
+     ret = blocking_notifier_chain_register(
+                             &genpd_transition_notifier_list, nb);
+     return ret;
+}
+
+/**
+ * genpd_unregister_notifier - Un-register a PM domain for future notification.
+ * @nb: notification block containing notification handle.
+ * @pd_name: PM domain name.
+ */
+int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name)
+{
+     int ret;
+     struct cache_notify_domains *entry;
+
+     mutex_lock(&domain_notify_list_lock);

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 1981

Quoted 10 lines

drivers/base/power/domain.c +10 -0

+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, pd_name))
+                     break;
+     }
+     if (!entry) {
+             mutex_unlock(&domain_notify_list_lock);
+             pr_err("%s: Invalid pd name\n", __func__);
+             return -EINVAL;
+     }
+     list_del(&entry->cache_list_node);

'entry' will not be NULL even when the requested notification entry was not found above. In such case it looks like last entry in the domain_notify_list list will be removed, not something we would expect.

Quoted 24 lines

drivers/base/power/domain.c +24 -0

+     mutex_unlock(&domain_notify_list_lock);
+     ret = blocking_notifier_chain_unregister(
+                             &genpd_transition_notifier_list, nb);
+     return ret;
+}
+
+/**
+ * genpd_invoke_transition_notifier - Calls the matching notification handler.
+ * @genpd: generic power domain structure.
+ * @state: can be of type - GPD_OFF_PRE/GPD_OFF_POST/GPD_ON_PRE/GPD_ON_POST.
+ */
+void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,
+                     enum gpd_on_off_state state)
+{
+     struct cache_notify_domains *entry;
+
+     if (!genpd) {
+             pr_err("Invalid genpd parameter\n");
+             return;
+     }
+
+     if (state != GPD_OFF_PRE || state != GPD_OFF_POST
+                     || state != GPD_ON_PRE || state != GPD_ON_POST) {
+             pr_err("Invalid state parameter\n");

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 2015

Quoted 10 lines

drivers/base/power/domain.c +10 -0

+             return;
+     }
+
+     mutex_lock(&domain_notify_list_lock);
+     list_for_each_entry(entry, &domain_notify_list, cache_list_node) {
+             if (!strcmp(entry->name, genpd->name))
+                     break;
+     }
+     mutex_unlock(&domain_notify_list_lock);
+     if (!entry) /* Simply ignore */

Similar issue here, this condition will only be true when the notifications list is empty.

drivers/base/power/domain.c void pm_genpd_init(struct generic_pm_domain *genpd, · line 2019

Quoted 4 lines

drivers/base/power/domain.c +4 -0

+             return;
+
+     blocking_notifier_call_chain(&genpd_transition_notifier_list, state,
+                             genpd);

And finally regardless of to what power domain the notification user has registered it will get notification for each possible power domain in the system? Are the notification users supposed to test the 'genpd' argument to react on a specific power domain? If so, how? I guess not by nother strcmp() ?

Quoted 12 lines

drivers/base/power/domain.c +1 -0

+}
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 /*
  * Device Tree based PM domain providers.

include/linux/pm_domain.h +0 -0

index 73e938b..659997f 100644
@@ -25,6 +25,13 @@ enum gpd_status {
      GPD_STATE_POWER_OFF,    /* PM domain is off */
 };
 

include/linux/pm_domain.h enum gpd_status { · line 29

Quoted 2 lines

include/linux/pm_domain.h +2 -0

+enum gpd_on_off_state {
+     GPD_OFF_PRE = 0,        /* GPD state before power off */

The assignment is not needed.

include/linux/pm_domain.h extern int pm_genpd_name_poweron(const char *domain_name);

Quoted 10 lines

include/linux/pm_domain.h +5 -0

+     GPD_OFF_POST,           /* GPD state after power off */
+     GPD_ON_PRE,             /* GPD state before power on */
+     GPD_ON_POST,            /* GPD state after power on */
+};
+
 struct dev_power_governor {
      bool (*power_down_ok)(struct dev_pm_domain *domain);
      bool (*stop_ok)(struct device *dev);

@@ -148,6 +155,12 @@ extern int pm_genpd_name_poweron(const char *domain_name);

--
Regards,
Sylwester

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel at lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Quoted 1 line · Sylwester Nawrocki

On 03/11/14 04:53, Amit Daniel Kachhap wrote:

Quoted 19 lines

Quoted 19 lines

4 type of notifications are added,

GPD_OFF_PRE     - GPD state before power off
GPD_OFF_POST    - GPD state after power off
GPD_ON_PRE      - GPD state before power off
GPD_ON_POST     - GPD state after power off

3 notfication API's are exported.
1) int genpd_register_notifier(struct notifier_block *nb, char *pd_name);
2) int genpd_unregister_notifier(struct notifier_block *nb, char *pd_name);
3) void genpd_invoke_transition_notifier(struct generic_pm_domain *genpd,

                      enum gpd_on_off_state state);

In this approach the notifiers are registered/unregistered with pd name. The actual invoking of the notfiers will be done by the platform implementing power domain enable/disable low level handlers according to the above defined notification types. This approach will considerably reduce the number of call to notifiers as compared to calling always from core powerdomain subsystem.

Quoted 8 lines

Quoted 4 lines

Also the registered domain's will be managed inside a cache list and not part of the genpd structure. This will help in registration of notifiers from subsystems (such as clock) even when the PD subsystem is still not initialised. This list also filters out the unregistered pd's requesting notification.

This patch is somewhat similar my patch adding power domain state change notifications [1]. I have already a reworked version of that patch, with the notifier list moved to struct generic_pm_domain and using genpd->lock instead

Yes this will be correct as others also suggested to make per genpd notifier block.

Quoted 9 lines

of dev->power.lock. Anyway, while I'd leave the decision about the location from where the notifier chains are supposed to be called to the subsystem's maintainers preference, I'm rather reluctant to having one global notifiers list for all possible power domains and all the notification clients. The possibly long list needs to be traversed at each notifier call and it seems in your implementation any registered user of the notification gets notifications for all possible power domains.

[1] https://lkml.org/lkml/2014/8/5/182

My fault, I somehow missed this link earlier. After going through this, I found it registers genpd from the platform driver, so the function signature is
int pm_genpd_register_notifier(struct device *dev, struct notifier_block *nb); I suggest to make the function signature to be like,
int pm_genpd_register_notifier(struct device_node *np, struct notifier_block *nb)

In this way this function should should be able to support both platform devices and non platform devices like clk.

The function may work like,

pdev = of_find_device_by_node(np);

if (pdev) {
         //  get genpd from device and go ahead with notfier registration.
         // blocking_notifier_chain_register(genpd->pd_notifier, nb)
} else
{
        // get pd_handle from np
        // get the pd_name from phandle and try registering this gen pd
        // if the genpd subsystem is not initialised then add this in

a temporary list and register the notifier later
}

Can you post your implementation with 1st part ? Later I can post the else part with my changes.