linux-pm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
@ 2014-10-30 12:02 Ulf Hansson
  2014-10-30 12:17 ` Geert Uytterhoeven
                   ` (2 more replies)
  0 siblings, 3 replies; 18+ messages in thread
From: Ulf Hansson @ 2014-10-30 12:02 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown, Pavel Machek, linux-pm
  Cc: linux-arm-kernel, linux-samsung-soc, Geert Uytterhoeven,
	Kevin Hilman, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Dmitry Torokhov,
	Jack Dai, Jinkun Hong, Aaron Lu, Sylwester Nawrocki, Ulf Hansson

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

Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
---

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.

---
 include/linux/pm_domain.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 73e938b..d44f071 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -72,7 +72,7 @@ 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);
+	int (*attach_dev)(struct device *dev);
 	void (*detach_dev)(struct device *dev);
 };
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-10-30 12:02 [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback Ulf Hansson
@ 2014-10-30 12:17 ` Geert Uytterhoeven
  2014-10-30 12:28 ` Pavel Machek
  2014-10-30 20:04 ` Rafael J. Wysocki
  2 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2014-10-30 12:17 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rafael J. Wysocki, Len Brown, Pavel Machek, Linux PM list,
	linux-arm-kernel, linux-samsung-soc, Geert Uytterhoeven,
	Kevin Hilman, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Dmitry Torokhov,
	Jack Dai

On Thu, Oct 30, 2014 at 1:02 PM, Ulf Hansson <ulf.hansson@linaro.org> wrote:
> Convert the prototype to return and int. This is just an initial step,
> needed to support error handling.
>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-10-30 12:02 [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback Ulf Hansson
  2014-10-30 12:17 ` Geert Uytterhoeven
@ 2014-10-30 12:28 ` Pavel Machek
  2014-10-30 20:04 ` Rafael J. Wysocki
  2 siblings, 0 replies; 18+ messages in thread
From: Pavel Machek @ 2014-10-30 12:28 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Rafael J. Wysocki, Len Brown, linux-pm, linux-arm-kernel,
	linux-samsung-soc, Geert Uytterhoeven, Kevin Hilman, Alan Stern,
	Greg Kroah-Hartman, Tomasz Figa, Simon Horman, Magnus Damm,
	Ben Dooks, Kukjin Kim, Philipp Zabel, Mark Brown, Wolfram Sang,
	Russell King, Dmitry Torokhov, Jack Dai, Jinkun Hong, Aaron Lu,
	Sylwester Nawrocki

On Thu 2014-10-30 13:02:49, Ulf Hansson wrote:
> Convert the prototype to return and int. This is just an initial step,
> needed to support error handling.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Pavel Machek <pavel@ucw.cz>


> ---
> 
> 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.
> 
> ---
>  include/linux/pm_domain.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 73e938b..d44f071 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -72,7 +72,7 @@ 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);
> +	int (*attach_dev)(struct device *dev);
>  	void (*detach_dev)(struct device *dev);
>  };
>  

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-10-30 12:02 [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback Ulf Hansson
  2014-10-30 12:17 ` Geert Uytterhoeven
  2014-10-30 12:28 ` Pavel Machek
@ 2014-10-30 20:04 ` Rafael J. Wysocki
  2014-10-30 20:38   ` Kevin Hilman
  2 siblings, 1 reply; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-10-30 20:04 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Len Brown, Pavel Machek, linux-pm, linux-arm-kernel,
	linux-samsung-soc, Geert Uytterhoeven, Kevin Hilman, Alan Stern,
	Greg Kroah-Hartman, Tomasz Figa, Simon Horman, Magnus Damm,
	Ben Dooks, Kukjin Kim, Philipp Zabel, Mark Brown, Wolfram Sang,
	Russell King, Dmitry Torokhov, Jack Dai, Jinkun Hong, Aaron Lu,
	Sylwester Nawrocki

On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> Convert the prototype to return and int. This is just an initial step,
> needed to support error handling.
> 
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
> 
> 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?

> ---
>  include/linux/pm_domain.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 73e938b..d44f071 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -72,7 +72,7 @@ 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);
> +	int (*attach_dev)(struct device *dev);
>  	void (*detach_dev)(struct device *dev);
>  };
>  
> 

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-10-30 20:04 ` Rafael J. Wysocki
@ 2014-10-30 20:38   ` Kevin Hilman
  2014-11-05  1:33     ` Dmitry Torokhov
  0 siblings, 1 reply; 18+ messages in thread
From: Kevin Hilman @ 2014-10-30 20:38 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Ulf Hansson, Len Brown, Pavel Machek, linux-pm, linux-arm-kernel,
	linux-samsung-soc, Geert Uytterhoeven, Alan Stern,
	Greg Kroah-Hartman, Tomasz Figa, Simon Horman, Magnus Damm,
	Ben Dooks, Kukjin Kim, Philipp Zabel, Mark Brown, Wolfram Sang,
	Russell King, Dmitry Torokhov, Jack Dai, Jinkun Hong, Aaron Lu,
	Sylwester Nawrocki

"Rafael J. Wysocki" <rjw@rjwysocki.net> writes:

> On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
>> Convert the prototype to return and int. This is just an initial step,
>> needed to support error handling.
>> 
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>

Acked-by: Kevin Hilman <khilman@linaro.org>

>> 
>> 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.

Kevin

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-10-30 20:38   ` Kevin Hilman
@ 2014-11-05  1:33     ` Dmitry Torokhov
  2014-11-05  7:43       ` Geert Uytterhoeven
  2014-11-05 22:43       ` Kevin Hilman
  0 siblings, 2 replies; 18+ messages in thread
From: Dmitry Torokhov @ 2014-11-05  1:33 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Rafael J. Wysocki, Ulf Hansson, Len Brown, Pavel Machek,
	linux-pm, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai, Jinkun Hong,
	Aaron Lu, Sylwester Nawrocki

On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> 
> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> >> Convert the prototype to return and int. This is just an initial step,
> >> needed to support error handling.
> >> 
> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> 
> Acked-by: Kevin Hilman <khilman@linaro.org>
> 
> >> 
> >> 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).

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05  1:33     ` Dmitry Torokhov
@ 2014-11-05  7:43       ` Geert Uytterhoeven
  2014-11-05  7:54         ` Dmitry Torokhov
  2014-11-05 22:43       ` Kevin Hilman
  1 sibling, 1 reply; 18+ messages in thread
From: Geert Uytterhoeven @ 2014-11-05  7:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Kevin Hilman, Rafael J. Wysocki, Ulf Hansson, Len Brown,
	Pavel Machek, Linux PM list, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai, Jinkun

On Wed, Nov 5, 2014 at 2:33 AM, Dmitry Torokhov
<dmitry.torokhov@gmail.com> wrote:
> On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>>
>> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
>> >> Convert the prototype to return and int. This is just an initial step,
>> >> needed to support error handling.
>> >>
>> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> Acked-by: Kevin Hilman <khilman@linaro.org>
>>
>> >>
>> >> 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).

You can use dev->pm_domain, which is already set.

Note that this is no longer the case after Ulf's "[PATCH 3/4] PM / Domains:
Improve error handling while adding/removing devices"!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05  7:43       ` Geert Uytterhoeven
@ 2014-11-05  7:54         ` Dmitry Torokhov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Torokhov @ 2014-11-05  7:54 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Kevin Hilman, Rafael J. Wysocki, Ulf Hansson, Len Brown,
	Pavel Machek, Linux PM list, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai, Jinkun

On Wed, Nov 05, 2014 at 08:43:29AM +0100, Geert Uytterhoeven wrote:
> On Wed, Nov 5, 2014 at 2:33 AM, Dmitry Torokhov
> <dmitry.torokhov@gmail.com> wrote:
> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
> >> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> >>
> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> >> >> Convert the prototype to return and int. This is just an initial step,
> >> >> needed to support error handling.
> >> >>
> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >>
> >> Acked-by: Kevin Hilman <khilman@linaro.org>
> >>
> >> >>
> >> >> 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).
> 
> You can use dev->pm_domain, which is already set.
> 
> Note that this is no longer the case after Ulf's "[PATCH 3/4] PM / Domains:
> Improve error handling while adding/removing devices"!

Right, but I'd rather not poke in dev structure directly if I can help
it.

Thanks.

-- 
Dmitry

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05  1:33     ` Dmitry Torokhov
  2014-11-05  7:43       ` Geert Uytterhoeven
@ 2014-11-05 22:43       ` Kevin Hilman
  2014-11-05 22:49         ` Dmitry Torokhov
  2014-11-05 23:21         ` Rafael J. Wysocki
  1 sibling, 2 replies; 18+ messages in thread
From: Kevin Hilman @ 2014-11-05 22:43 UTC (permalink / raw)
  To: Dmitry Torokhov
  Cc: Rafael J. Wysocki, Ulf Hansson, Len Brown, Pavel Machek,
	linux-pm, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai, Jinkun Hong,
	Aaron Lu, Sylwester Nawrocki

Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:

> On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>> 
>> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
>> >> Convert the prototype to return and int. This is just an initial step,
>> >> needed to support error handling.
>> >> 
>> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> 
>> Acked-by: Kevin Hilman <khilman@linaro.org>
>> 
>> >> 
>> >> 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 <ulf.hansson@linaro.org>
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 <ulf.hansson@linaro.org>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05 22:43       ` Kevin Hilman
@ 2014-11-05 22:49         ` Dmitry Torokhov
  2014-11-05 23:21         ` Rafael J. Wysocki
  1 sibling, 0 replies; 18+ messages in thread
From: Dmitry Torokhov @ 2014-11-05 22:49 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Rafael J. Wysocki, Ulf Hansson, Len Brown, Pavel Machek,
	linux-pm, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai, Jinkun Hong,
	Aaron Lu, Sylwester Nawrocki

On Wed, Nov 05, 2014 at 02:43:31PM -0800, Kevin Hilman wrote:
> Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
> 
> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
> >> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> >> 
> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> >> >> Convert the prototype to return and int. This is just an initial step,
> >> >> needed to support error handling.
> >> >> 
> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >> 
> >> Acked-by: Kevin Hilman <khilman@linaro.org>
> >> 
> >> >> 
> >> >> 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 <ulf.hansson@linaro.org>
> 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 <ulf.hansson@linaro.org>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>

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

> ---
>  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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05 23:21         ` Rafael J. Wysocki
@ 2014-11-05 23:11           ` Kevin Hilman
  2014-11-06  0:54             ` Rafael J. Wysocki
                               ` (3 more replies)
  0 siblings, 4 replies; 18+ messages in thread
From: Kevin Hilman @ 2014-11-05 23:11 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Dmitry Torokhov, Ulf Hansson, Len Brown, Pavel Machek, linux-pm,
	linux-arm-kernel, linux-samsung-soc, Geert Uytterhoeven,
	Alan Stern, Greg Kroah-Hartman, Tomasz Figa, Simon Horman,
	Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel, Mark Brown,
	Wolfram Sang, Russell King, Jack Dai, Jinkun Hong, Aaron Lu,
	Sylwester Nawrocki

"Rafael J. Wysocki" <rjw@rjwysocki.net> writes:

> On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
>> Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
>> 
>> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
>> >> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>> >> 
>> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
>> >> >> Convert the prototype to return and int. This is just an initial step,
>> >> >> needed to support error handling.
>> >> >> 
>> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> >> 
>> >> Acked-by: Kevin Hilman <khilman@linaro.org>
>> >> 
>> >> >> 
>> >> >> 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.
>

Right.  Here you go.

Kevin


----- >8 ------
>From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <ulf.hansson@linaro.org>
Date: Thu, 30 Oct 2014 13:02:49 +0100
Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
 callbacks

Convert the prototypes to return an int in order to support error
handling in these callbacks.

Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
inside the callbacks, and so that they match the existing
power_on/power_off callbacks which currently take the domain pointer.

Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
Signed-off-by: Kevin Hilman <khilman@linaro.org>
---
 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

^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05 22:43       ` Kevin Hilman
  2014-11-05 22:49         ` Dmitry Torokhov
@ 2014-11-05 23:21         ` Rafael J. Wysocki
  2014-11-05 23:11           ` Kevin Hilman
  1 sibling, 1 reply; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-11-05 23:21 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Dmitry Torokhov, Ulf Hansson, Len Brown, Pavel Machek, linux-pm,
	linux-arm-kernel, linux-samsung-soc, Geert Uytterhoeven,
	Alan Stern, Greg Kroah-Hartman, Tomasz Figa, Simon Horman,
	Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel, Mark Brown,
	Wolfram Sang, Russell King, Jack Dai, Jinkun Hong, Aaron Lu,
	Sylwester Nawrocki

On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
> Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
> 
> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
> >> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> >> 
> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> >> >> Convert the prototype to return and int. This is just an initial step,
> >> >> needed to support error handling.
> >> >> 
> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >> 
> >> Acked-by: Kevin Hilman <khilman@linaro.org>
> >> 
> >> >> 
> >> >> 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.

> ----- >8 ------
> From 353a62ffae2f9228142c8a2093108f9eda8dc6b4 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> 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 <ulf.hansson@linaro.org>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05 23:11           ` Kevin Hilman
@ 2014-11-06  0:54             ` Rafael J. Wysocki
  2014-11-14  8:17               ` Geert Uytterhoeven
  2014-11-06  7:36             ` Geert Uytterhoeven
                               ` (2 subsequent siblings)
  3 siblings, 1 reply; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-11-06  0:54 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Dmitry Torokhov, Ulf Hansson, Len Brown, Pavel Machek, linux-pm,
	linux-arm-kernel, linux-samsung-soc, Geert Uytterhoeven,
	Alan Stern, Greg Kroah-Hartman, Tomasz Figa, Simon Horman,
	Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel, Mark Brown,
	Wolfram Sang, Russell King, Jack Dai, Jinkun Hong, Aaron Lu,
	Sylwester Nawrocki

On Wednesday, November 05, 2014 03:11:23 PM Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> 
> > On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
> >> Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
> >> 
> >> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
> >> >> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> >> >> 
> >> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> >> >> >> Convert the prototype to return and int. This is just an initial step,
> >> >> >> needed to support error handling.
> >> >> >> 
> >> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >> >> 
> >> >> Acked-by: Kevin Hilman <khilman@linaro.org>
> >> >> 
> >> >> >> 
> >> >> >> 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.
> >
> 
> Right.  Here you go.

I've replaced the Ulf's original with this one, thanks!

> ----- >8 ------
> From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Thu, 30 Oct 2014 13:02:49 +0100
> Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
>  callbacks
> 
> Convert the prototypes to return an int in order to support error
> handling in these callbacks.
> 
> Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
> inside the callbacks, and so that they match the existing
> power_on/power_off callbacks which currently take the domain pointer.
> 
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> [khilman: added domain as parameter to callbacks, as suggested by Dmitry]
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05 23:11           ` Kevin Hilman
  2014-11-06  0:54             ` Rafael J. Wysocki
@ 2014-11-06  7:36             ` Geert Uytterhoeven
  2014-11-06  9:55             ` Ulf Hansson
  2014-11-14 23:48             ` Rafael J. Wysocki
  3 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2014-11-06  7:36 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Rafael J. Wysocki, Dmitry Torokhov, Ulf Hansson, Len Brown,
	Pavel Machek, Linux PM list, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai

On Thu, Nov 6, 2014 at 12:11 AM, Kevin Hilman <khilman@kernel.org> wrote:
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Thu, 30 Oct 2014 13:02:49 +0100
> Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
>  callbacks
>
> Convert the prototypes to return an int in order to support error
> handling in these callbacks.
>
> Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
> inside the callbacks, and so that they match the existing
> power_on/power_off callbacks which currently take the domain pointer.
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> [khilman: added domain as parameter to callbacks, as suggested by Dmitry]

OK, so back to the signature of the original version...

> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>

Acked-by: Geert Uytterhoeven <geert+renesas@glider.be>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05 23:11           ` Kevin Hilman
  2014-11-06  0:54             ` Rafael J. Wysocki
  2014-11-06  7:36             ` Geert Uytterhoeven
@ 2014-11-06  9:55             ` Ulf Hansson
  2014-11-14 23:48             ` Rafael J. Wysocki
  3 siblings, 0 replies; 18+ messages in thread
From: Ulf Hansson @ 2014-11-06  9:55 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Rafael J. Wysocki, Dmitry Torokhov, Len Brown, Pavel Machek,
	linux-pm, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai, Jinkun Hong

On 6 November 2014 00:11, Kevin Hilman <khilman@kernel.org> wrote:
> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>
>> On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
>>> Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
>>>
>>> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
>>> >> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>>> >>
>>> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
>>> >> >> Convert the prototype to return and int. This is just an initial step,
>>> >> >> needed to support error handling.
>>> >> >>
>>> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>>> >>
>>> >> Acked-by: Kevin Hilman <khilman@linaro.org>
>>> >>
>>> >> >>
>>> >> >> 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.
>>
>
> Right.  Here you go.
>
> Kevin

Thanks for helping out Kevin!

Kind regards
Uffe

>
>
> ----- >8 ------
> From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Thu, 30 Oct 2014 13:02:49 +0100
> Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
>  callbacks
>
> Convert the prototypes to return an int in order to support error
> handling in these callbacks.
>
> Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
> inside the callbacks, and so that they match the existing
> power_on/power_off callbacks which currently take the domain pointer.
>
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> [khilman: added domain as parameter to callbacks, as suggested by Dmitry]
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>
> ---
>  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
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-06  0:54             ` Rafael J. Wysocki
@ 2014-11-14  8:17               ` Geert Uytterhoeven
  0 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2014-11-14  8:17 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Kevin Hilman, Dmitry Torokhov, Ulf Hansson, Len Brown,
	Pavel Machek, Linux PM list, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai, Ji

Hi Rafael,

On Thu, Nov 6, 2014 at 1:54 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
> On Wednesday, November 05, 2014 03:11:23 PM Kevin Hilman wrote:
>> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>> > On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
>> >> Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
>> >> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
>> >> >> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
>> >> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
>> >> >> >> Convert the prototype to return and int. This is just an initial step,
>> >> >> >> needed to support error handling.
>> >> >> >>
>> >> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> >> >>
>> >> >> Acked-by: Kevin Hilman <khilman@linaro.org>
>> >> >>
>> >> >> >> 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.
>> >
>>
>> Right.  Here you go.
>
> I've replaced the Ulf's original with this one, thanks!

When do you plan to send a pull request for this to Linus?

There are PM domain drivers blocked on entering -next due to this
dependency. As they have to pass through arm-soc first, and the arm-soc
merge window will close soon, time is running out.

Alternatively, you could provide an immutable branch containing this fix
to unblock dependents.

Thanks for your understanding.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-05 23:11           ` Kevin Hilman
                               ` (2 preceding siblings ...)
  2014-11-06  9:55             ` Ulf Hansson
@ 2014-11-14 23:48             ` Rafael J. Wysocki
  2014-11-15 11:32               ` Geert Uytterhoeven
  3 siblings, 1 reply; 18+ messages in thread
From: Rafael J. Wysocki @ 2014-11-14 23:48 UTC (permalink / raw)
  To: Kevin Hilman
  Cc: Dmitry Torokhov, Ulf Hansson, Len Brown, Pavel Machek, linux-pm,
	linux-arm-kernel, linux-samsung-soc, Geert Uytterhoeven,
	Alan Stern, Greg Kroah-Hartman, Tomasz Figa, Simon Horman,
	Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel, Mark Brown,
	Wolfram Sang, Russell King, Jack Dai, Jinkun Hong, Aaron Lu,
	Sylwester Nawrocki

On Wednesday, November 05, 2014 03:11:23 PM Kevin Hilman wrote:
> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> 
> > On Wednesday, November 05, 2014 02:43:31 PM Kevin Hilman wrote:
> >> Dmitry Torokhov <dmitry.torokhov@gmail.com> writes:
> >> 
> >> > On Thu, Oct 30, 2014 at 01:38:30PM -0700, Kevin Hilman wrote:
> >> >> "Rafael J. Wysocki" <rjw@rjwysocki.net> writes:
> >> >> 
> >> >> > On Thursday, October 30, 2014 01:02:49 PM Ulf Hansson wrote:
> >> >> >> Convert the prototype to return and int. This is just an initial step,
> >> >> >> needed to support error handling.
> >> >> >> 
> >> >> >> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> >> >> 
> >> >> Acked-by: Kevin Hilman <khilman@linaro.org>
> >> >> 
> >> >> >> 
> >> >> >> 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.
> >
> 
> Right.  Here you go.
> 
> Kevin
> 
> 
> ----- >8 ------
> From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001
> From: Ulf Hansson <ulf.hansson@linaro.org>
> Date: Thu, 30 Oct 2014 13:02:49 +0100
> Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
>  callbacks
> 
> Convert the prototypes to return an int in order to support error
> handling in these callbacks.
> 
> Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
> inside the callbacks, and so that they match the existing
> power_on/power_off callbacks which currently take the domain pointer.
> 
> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> [khilman: added domain as parameter to callbacks, as suggested by Dmitry]
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> Signed-off-by: Kevin Hilman <khilman@linaro.org>

That's in the Linus' tree now.

> ---
>  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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback
  2014-11-14 23:48             ` Rafael J. Wysocki
@ 2014-11-15 11:32               ` Geert Uytterhoeven
  0 siblings, 0 replies; 18+ messages in thread
From: Geert Uytterhoeven @ 2014-11-15 11:32 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Kevin Hilman, Dmitry Torokhov, Ulf Hansson, Len Brown,
	Pavel Machek, Linux PM list, linux-arm-kernel, linux-samsung-soc,
	Geert Uytterhoeven, Alan Stern, Greg Kroah-Hartman, Tomasz Figa,
	Simon Horman, Magnus Damm, Ben Dooks, Kukjin Kim, Philipp Zabel,
	Mark Brown, Wolfram Sang, Russell King, Jack Dai, Ji

Hi Rafael,

On Sat, Nov 15, 2014 at 12:48 AM, Rafael J. Wysocki <rjw@rjwysocki.net> wrote:
>> Subject: [PATCH] PM / Domains: Change prototype for the attach and detach
>>  callbacks
>>
>> Convert the prototypes to return an int in order to support error
>> handling in these callbacks.
>>
>> Also, as suggested by Dmitry Torokhov, pass the domain pointer for use
>> inside the callbacks, and so that they match the existing
>> power_on/power_off callbacks which currently take the domain pointer.
>>
>> Acked-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
>> [khilman: added domain as parameter to callbacks, as suggested by Dmitry]
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Kevin Hilman <khilman@linaro.org>
>
> That's in the Linus' tree now.

Thank you very much.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2014-11-15 11:32 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-30 12:02 [PATCH] PM / Domains: Change prototype for the ->attach_dev() callback Ulf Hansson
2014-10-30 12:17 ` Geert Uytterhoeven
2014-10-30 12:28 ` Pavel Machek
2014-10-30 20:04 ` Rafael J. Wysocki
2014-10-30 20:38   ` Kevin Hilman
2014-11-05  1:33     ` Dmitry Torokhov
2014-11-05  7:43       ` Geert Uytterhoeven
2014-11-05  7:54         ` Dmitry Torokhov
2014-11-05 22:43       ` Kevin Hilman
2014-11-05 22:49         ` Dmitry Torokhov
2014-11-05 23:21         ` Rafael J. Wysocki
2014-11-05 23:11           ` Kevin Hilman
2014-11-06  0:54             ` Rafael J. Wysocki
2014-11-14  8:17               ` Geert Uytterhoeven
2014-11-06  7:36             ` Geert Uytterhoeven
2014-11-06  9:55             ` Ulf Hansson
2014-11-14 23:48             ` Rafael J. Wysocki
2014-11-15 11:32               ` Geert Uytterhoeven

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).