[PATCH] PM / Domains: Change prototype for the ->attach_dev() callback

Subsystems: driver core, kobjects, debugfs and sysfs, generic pm domains, hibernation (aka software suspend, aka swsusp), power management core, suspend to ram, the rest

STALE4288d

3 messages, 3 authors, 2014-11-05 · open the first message on its own page

[PATCH] PM / Domains: Change prototype for the ->attach_dev() callback

From: khilman@kernel.org (Kevin Hilman)
Date: 2014-11-05 22:43:31

Dmitry Torokhov [off-list ref] writes:

On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:

quoted

"Rafael J. Wysocki" [off-list ref] writes:

quoted

On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:

quoted

Convert the prototype to return and int. This is just an initial step,
needed to support error handling.

Signed-off-by: Ulf Hansson <redacted>

Acked-by: Kevin Hilman <redacted>

quoted

This patch is intended as fix for 3.18 rc[n]. Why?

There are other SOC specific patches around that adds genpd support and which
implements the ->attach_dev() callback. To prevent having an "atomic" patch
during the next release cycle, let's change the prototype now instead.

Further patches will add the actual error handling in genpd and these can then
be reviewed and tested thoroughly.

So we have no users of ->attach_dev at the moment, right?

Not in mainline, but there are a couple getting ready to hit -next, so
we wanted to fix this before they arrive so that adding the error
handling will be easier.

BTW, while we are at it, can we also pass the domain itself to
attach_dev() and detach_dev()? If anything it helps with debugging (you
can print domain name from the callbacks).

Agreed, and it makes it match the other callbacks (power_off, power_on)
which currently take struct generic_pm_domain *domain.  

Updated version of $SUBJECT patch below.

Kevin


----- >8 ------

From 353a62ffae2f9228142c8a2093108f9eda8dc6b4 Mon Sep 17 00:00:00 2001

From: Ulf Hansson <redacted>
Date: Thu, 30 Oct 2014 13:02:49 +0100
Subject: [PATCH] PM / Domains: Change prototype for the ->attach_dev()
 callback

Convert the prototype to return and int. This is just an initial step,
needed to support error handling.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
Signed-off-by: Ulf Hansson <redacted>
Signed-off-by: Kevin Hilman <redacted>
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4072cc..b520687046d4 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c

@@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	spin_unlock_irq(&dev->power.lock);
 
 	if (genpd->attach_dev)
-		genpd->attach_dev(dev);
+		genpd->attach_dev(genpd, dev);
 
 	mutex_lock(&gpd_data->lock);
 	gpd_data->base.dev = dev;

@@ -1499,7 +1499,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 	genpd->max_off_time_changed = true;
 
 	if (genpd->detach_dev)
-		genpd->detach_dev(dev);
+		genpd->detach_dev(genpd, dev);
 
 	spin_lock_irq(&dev->power.lock);

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 73e938b7e937..b3ed7766a291 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h

@@ -72,8 +72,10 @@ struct generic_pm_domain {
 	bool max_off_time_changed;
 	bool cached_power_down_ok;
 	struct gpd_cpuidle_data *cpuidle_data;
-	void (*attach_dev)(struct device *dev);
-	void (*detach_dev)(struct device *dev);
+	int (*attach_dev)(struct generic_pm_domain *domain,
+			  struct device *dev);
+	void (*detach_dev)(struct generic_pm_domain *domain,
+			   struct device *dev);
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)

-- 
2.1.0

[PATCH] PM / Domains: Change prototype for the ->attach_dev() callback

From: dmitry.torokhov@gmail.com (Dmitry Torokhov)
Date: 2014-11-05 22:49:20

On Wed, Nov 05, 2014 at 02:43:31PM -0800, Kevin Hilman wrote:

Dmitry Torokhov [off-list ref] writes:

quoted

On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:

quoted

"Rafael J. Wysocki" [off-list ref] writes:

quoted

On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:

quoted

Convert the prototype to return and int. This is just an initial step,
needed to support error handling.

Signed-off-by: Ulf Hansson <redacted>

Acked-by: Kevin Hilman <redacted>

quoted

This patch is intended as fix for 3.18 rc[n]. Why?

There are other SOC specific patches around that adds genpd support and which
implements the ->attach_dev() callback. To prevent having an "atomic" patch
during the next release cycle, let's change the prototype now instead.

Further patches will add the actual error handling in genpd and these can then
be reviewed and tested thoroughly.

So we have no users of ->attach_dev at the moment, right?

Not in mainline, but there are a couple getting ready to hit -next, so
we wanted to fix this before they arrive so that adding the error
handling will be easier.

BTW, while we are at it, can we also pass the domain itself to
attach_dev() and detach_dev()? If anything it helps with debugging (you
can print domain name from the callbacks).

Agreed, and it makes it match the other callbacks (power_off, power_on)
which currently take struct generic_pm_domain *domain.  

Updated version of $SUBJECT patch below.

Kevin


----- >8 ------
From 353a62ffae2f9228142c8a2093108f9eda8dc6b4 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <redacted>
Date: Thu, 30 Oct 2014 13:02:49 +0100
Subject: [PATCH] PM / Domains: Change prototype for the ->attach_dev()
 callback

Convert the prototype to return and int. This is just an initial step,
needed to support error handling.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
Signed-off-by: Ulf Hansson <redacted>
Signed-off-by: Kevin Hilman <redacted>

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>

quoted hunk

---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4072cc..b520687046d4 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c

@@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	spin_unlock_irq(&dev->power.lock);
 
 	if (genpd->attach_dev)
-		genpd->attach_dev(dev);
+		genpd->attach_dev(genpd, dev);

I guess as a followup we need to propagate the error returned by
genpd->attach_dev() here.

Thanks.

-- 
Dmitry

[PATCH] PM / Domains: Change prototype for the ->attach_dev() callback

From: Rafael J. Wysocki <hidden>
Date: 2014-11-05 23:21:01

On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:

Dmitry Torokhov [off-list ref] writes:

quoted

On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:

quoted

"Rafael J. Wysocki" [off-list ref] writes:

quoted

On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:

quoted

Convert the prototype to return and int. This is just an initial step,
needed to support error handling.

Signed-off-by: Ulf Hansson <redacted>

Acked-by: Kevin Hilman <redacted>

quoted

This patch is intended as fix for 3.18 rc[n]. Why?

There are other SOC specific patches around that adds genpd support and which
implements the ->attach_dev() callback. To prevent having an "atomic" patch
during the next release cycle, let's change the prototype now instead.

Further patches will add the actual error handling in genpd and these can then
be reviewed and tested thoroughly.

So we have no users of ->attach_dev at the moment, right?

Not in mainline, but there are a couple getting ready to hit -next, so
we wanted to fix this before they arrive so that adding the error
handling will be easier.

BTW, while we are at it, can we also pass the domain itself to
attach_dev() and detach_dev()? If anything it helps with debugging (you
can print domain name from the callbacks).

Agreed, and it makes it match the other callbacks (power_off, power_on)
which currently take struct generic_pm_domain *domain.  

Updated version of $SUBJECT patch below.

The subject and changelog need to be updated too IMO.

quoted hunk

----- >8 ------
From 353a62ffae2f9228142c8a2093108f9eda8dc6b4 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <redacted>
Date: Thu, 30 Oct 2014 13:02:49 +0100
Subject: [PATCH] PM / Domains: Change prototype for the ->attach_dev()
 callback

Convert the prototype to return and int. This is just an initial step,
needed to support error handling.

Cc: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
Signed-off-by: Ulf Hansson <redacted>
Signed-off-by: Kevin Hilman <redacted>
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 40bc2f4072cc..b520687046d4 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c

@@ -1437,7 +1437,7 @@ int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev,
 	spin_unlock_irq(&dev->power.lock);
 
 	if (genpd->attach_dev)
-		genpd->attach_dev(dev);
+		genpd->attach_dev(genpd, dev);
 
 	mutex_lock(&gpd_data->lock);
 	gpd_data->base.dev = dev;

@@ -1499,7 +1499,7 @@ int pm_genpd_remove_device(struct generic_pm_domain *genpd,
 	genpd->max_off_time_changed = true;
 
 	if (genpd->detach_dev)
-		genpd->detach_dev(dev);
+		genpd->detach_dev(genpd, dev);
 
 	spin_lock_irq(&dev->power.lock);

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 73e938b7e937..b3ed7766a291 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h

@@ -72,8 +72,10 @@ struct generic_pm_domain {
 	bool max_off_time_changed;
 	bool cached_power_down_ok;
 	struct gpd_cpuidle_data *cpuidle_data;
-	void (*attach_dev)(struct device *dev);
-	void (*detach_dev)(struct device *dev);
+	int (*attach_dev)(struct generic_pm_domain *domain,
+			  struct device *dev);
+	void (*detach_dev)(struct generic_pm_domain *domain,
+			   struct device *dev);
 };
 
 static inline struct generic_pm_domain *pd_to_genpd(struct dev_pm_domain *pd)

-- 
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help