[1/3] PM / Domains: Allow overriding the ->xlate() callback

Commit Message

Thierry Reding March 14, 2017, 7:15 p.m. UTC

From: Thierry Reding <[e-mail]>

Allow generic power domain providers to override the ->xlate() callback
in case the default genpd_xlate_onecell() translation callback is not
good enough.

One potential use-case for this is to allow generic power domains to be
specified by an ID rather than an index.

Signed-off-by: Thierry Reding <[e-mail]>
---
 drivers/base/power/domain.c | 8 ++++----
 include/linux/pm_domain.h   | 4 ++++
 2 files changed, 8 insertions(+), 4 deletions(-)

Comments

Jon Hunter March 14, 2017, 8:34 p.m. UTC | #1

On 14/03/17 19:15, Thierry Reding wrote:
> From: Thierry Reding <[e-mail]>
> 
> Allow generic power domain providers to override the ->xlate() callback
> in case the default genpd_xlate_onecell() translation callback is not
> good enough.
> 
> One potential use-case for this is to allow generic power domains to be
> specified by an ID rather than an index.

Are you sure this is necessary, because I recall that the genpd core
does allow for holes in the domain array (which is what we have). See
commit 609bed67bd8f ('PM / Domains: Allow holes in genpd_data.domains
array').

Cheers
Jon

Thierry Reding March 15, 2017, 1:36 p.m. UTC | #2

On Tue, Mar 14, 2017 at 08:34:01PM +0000, Jon Hunter wrote:
> 
> On 14/03/17 19:15, Thierry Reding wrote:
> > From: Thierry Reding <[e-mail]>
> > 
> > Allow generic power domain providers to override the ->xlate() callback
> > in case the default genpd_xlate_onecell() translation callback is not
> > good enough.
> > 
> > One potential use-case for this is to allow generic power domains to be
> > specified by an ID rather than an index.
> 
> Are you sure this is necessary, because I recall that the genpd core
> does allow for holes in the domain array (which is what we have). See
> commit 609bed67bd8f ('PM / Domains: Allow holes in genpd_data.domains
> array').

I suppose we could use that, but it feels like the wrong solution to me.
Why would we want to deal with holes in the array when we can simply do
all the required work at probe time and add only the existing domains in
any particular SoC.

For example on Tegra186 we've got a maximum ID of 43, so we'd need to
allocate 44 pointers, but we only use 17 of those IDs. People usually
reply to that that newer SoCs come with enough memory to make this
waste irrelevant. I don't buy that. Why should we waste memory if it
can be easily avoided?

Thierry

Jon Hunter March 15, 2017, 1:43 p.m. UTC | #3

On 15/03/17 13:36, Thierry Reding wrote:
> * PGP Signed by an unknown key
> 
> On Tue, Mar 14, 2017 at 08:34:01PM +0000, Jon Hunter wrote:
>>
>> On 14/03/17 19:15, Thierry Reding wrote:
>>> From: Thierry Reding <[e-mail]>
>>>
>>> Allow generic power domain providers to override the ->xlate() callback
>>> in case the default genpd_xlate_onecell() translation callback is not
>>> good enough.
>>>
>>> One potential use-case for this is to allow generic power domains to be
>>> specified by an ID rather than an index.
>>
>> Are you sure this is necessary, because I recall that the genpd core
>> does allow for holes in the domain array (which is what we have). See
>> commit 609bed67bd8f ('PM / Domains: Allow holes in genpd_data.domains
>> array').
> 
> I suppose we could use that, but it feels like the wrong solution to me.
> Why would we want to deal with holes in the array when we can simply do
> all the required work at probe time and add only the existing domains in
> any particular SoC.
> 
> For example on Tegra186 we've got a maximum ID of 43, so we'd need to
> allocate 44 pointers, but we only use 17 of those IDs. People usually
> reply to that that newer SoCs come with enough memory to make this
> waste irrelevant. I don't buy that. Why should we waste memory if it
> can be easily avoided?

I was wondering how many holes we have. That seems a bit excessive and
so I would agree it is a waste.

Cheers
Jon

Patch

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index e697dec9d25b..c102557f6c7e 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -1570,9 +1570,6 @@  EXPORT_SYMBOL_GPL(pm_genpd_remove);
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF
 
-typedef struct generic_pm_domain *(*genpd_xlate_t)(struct of_phandle_args *args,
-						   void *data);
-
 /*
  * Device Tree based PM domain providers.
  *
@@ -1730,6 +1727,9 @@  int of_genpd_add_provider_onecell(struct device_node *np,
 
 	mutex_lock(&gpd_list_lock);
 
+	if (!data->xlate)
+		data->xlate = genpd_xlate_onecell;
+
 	for (i = 0; i < data->num_domains; i++) {
 		if (!data->domains[i])
 			continue;
@@ -1740,7 +1740,7 @@  int of_genpd_add_provider_onecell(struct device_node *np,
 		data->domains[i]->has_provider = true;
 	}
 
-	ret = genpd_add_provider(np, genpd_xlate_onecell, data);
+	ret = genpd_add_provider(np, data->xlate, data);
 	if (ret < 0)
 		goto error;
 
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 5339ed5bd6f9..5163f371a308 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -204,9 +204,13 @@  static inline void pm_genpd_syscore_poweron(struct device *dev) {}
 /* OF PM domain providers */
 struct of_device_id;
 
+typedef struct generic_pm_domain *(*genpd_xlate_t)(struct of_phandle_args *args,
+						   void *data);
+
 struct genpd_onecell_data {
 	struct generic_pm_domain **domains;
 	unsigned int num_domains;
+	genpd_xlate_t xlate;
 };
 
 #ifdef CONFIG_PM_GENERIC_DOMAINS_OF