Skip to contentSkip to conversation

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

PATCH New Ulf Hansson 6 participants 18 messages Oct 30, 2014 – Nov 15, 2014

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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.

---

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(-)
include/linux/pm_domain.h +1 -1
index 73e938b..d44f071 100644
@@ -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);
 };
 
Signature · 1 line
1.9.1
#1 lore raw

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

Quoted 4 lines · Ulf Hansson

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

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

#2 lore raw

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

In reply to Ulf Hansson
Quoted 4 lines · Ulf Hansson

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

Quoted 28 lines

---

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(-)
include/linux/pm_domain.h +1 -1
index 73e938b..d44f071 100644
@@ -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);
 };
 
Signature · 2 lines
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
#3 lore raw

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

In reply to Ulf Hansson
Quoted 6 lines · Ulf Hansson

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

---

Quoted 8 lines

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?

Quoted 18 lines
---
 include/linux/pm_domain.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/pm_domain.h +1 -1
index 73e938b..d44f071 100644
@@ -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);
 };
 
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#4 lore raw

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

Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 11 lines
Quoted 9 lines

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

#5 lore raw

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

Quoted 17 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 7 lines
Quoted 7 lines

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.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

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.

Signature · 1 line
Dmitry
#6 lore raw

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

Quoted 23 lines · Dmitry Torokhov
Quoted 22 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 11 lines
Quoted 9 lines

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?

Quoted 7 lines
Quoted 3 lines

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

#7 lore raw

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

Quoted 29 lines · Geert Uytterhoeven
Quoted 27 lines · Dmitry Torokhov
Quoted 25 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 11 lines
Quoted 9 lines

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.

Quoted 8 lines
Quoted 3 lines

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.

Signature · 1 line
Dmitry
#8 lore raw

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

In reply to Dmitry Torokhov
Quoted 23 lines · Dmitry Torokhov
Quoted 22 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 11 lines
Quoted 9 lines

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?

Quoted 7 lines
Quoted 3 lines

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

Quoted 1 line

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

From: Ulf Hansson <[email protected]>
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 <[email protected]>
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/base/power/domain.c +2 -2
index 40bc2f4072cc..b520687046d4 100644
@@ -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);
 
include/linux/pm_domain.h +4 -2
index 73e938b7e937..b3ed7766a291 100644
@@ -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)
Signature · 1 line
2.1.0
#9 lore raw

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

Quoted 48 lines · Kevin Hilman
Quoted 23 lines · Dmitry Torokhov
Quoted 22 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 11 lines
Quoted 9 lines

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?

Quoted 7 lines
Quoted 3 lines

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 <[email protected]>
Date: Thu, 30 Oct 2014 13:02:49 +0100
Subject: [PATCH] PM / Domains: Change prototype for the ->attach_dev()
callback

Quoted 7 lines

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

Cc Dmitry Torokhov <[email protected]>
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
Quoted 10 lines
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/base/power/domain.c +0 -0
index 40bc2f4072cc..b520687046d4 100644

drivers/base/power/domain.c int __pm_genpd_add_device(struct generic_pm_domain *genpd, struct device *dev, · line 1440

Quoted 5 lines
drivers/base/power/domain.c +1 -1
@@ -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.

Signature · 1 line
Dmitry
#10 lore raw

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

In reply to Kevin Hilman
Quoted 29 lines · Kevin Hilman
Quoted 27 lines · Dmitry Torokhov
Quoted 25 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 11 lines
Quoted 9 lines

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.

Quoted 8 lines
Quoted 3 lines

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

----- >8 ------
From 353a62ffae2f9228142c8a2093108f9eda8dc6b4 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <[email protected]>
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 <[email protected]>
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/base/power/domain.c +2 -2
index 40bc2f4072cc..b520687046d4 100644
@@ -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);
 
include/linux/pm_domain.h +4 -2
index 73e938b7e937..b3ed7766a291 100644
@@ -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)
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#11 lore raw

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

Quoted 34 lines · Rafael J. Wysocki
Quoted 33 lines · Kevin Hilman
Quoted 30 lines · Dmitry Torokhov
Quoted 17 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 7 lines
Quoted 7 lines

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.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

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

Quoted 7 lines
Quoted 4 lines

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

Quoted 1 line

From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001

From: Ulf Hansson <[email protected]>
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.

[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/base/power/domain.c +2 -2
index 40bc2f4072cc..b520687046d4 100644
@@ -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);
 
include/linux/pm_domain.h +4 -2
index 73e938b7e937..b3ed7766a291 100644
@@ -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)
Signature · 1 line
2.1.0
#12 lore raw

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

Quoted 39 lines · Kevin Hilman
Quoted 37 lines · Rafael J. Wysocki
Quoted 36 lines · Kevin Hilman
Quoted 23 lines · Dmitry Torokhov
Quoted 22 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 11 lines
Quoted 9 lines

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?

Quoted 7 lines
Quoted 3 lines

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.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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!

Quoted 63 lines

----- >8 ------
From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <[email protected]>
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.

[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/base/power/domain.c +2 -2
index 40bc2f4072cc..b520687046d4 100644
@@ -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);
 
include/linux/pm_domain.h +4 -2
index 73e938b7e937..b3ed7766a291 100644
@@ -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)
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#13 lore raw

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

Hi Rafael,

Quoted 36 lines · Rafael J. Wysocki
Quoted 35 lines · Kevin Hilman
Quoted 34 lines · Rafael J. Wysocki
Quoted 33 lines · Kevin Hilman
Quoted 21 lines · Dmitry Torokhov
Quoted 20 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 10 lines
Quoted 8 lines

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?

Quoted 7 lines
Quoted 3 lines

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.

Quoted 8 lines
Quoted 6 lines
Quoted 4 lines
Quoted 1 line

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

#14 lore raw

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

In reply to Kevin Hilman
Quoted 8 lines · Kevin Hilman

From: Ulf Hansson <[email protected]>
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.

Quoted 6 lines

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.

[khilman: added domain as parameter to callbacks, as suggested by Dmitry]

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

Quoted 2 lines

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at 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

#15 lore raw

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

In reply to Kevin Hilman
Quoted 39 lines · Kevin Hilman
Quoted 37 lines · Rafael J. Wysocki
Quoted 36 lines · Kevin Hilman
Quoted 23 lines · Dmitry Torokhov
Quoted 22 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 11 lines
Quoted 9 lines

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?

Quoted 7 lines
Quoted 3 lines

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.

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

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

Quoted 67 lines

----- >8 ------
From c18a6bf3121979c4102ba4f7edb3ac41e3921fc6 Mon Sep 17 00:00:00 2001
From: Ulf Hansson <[email protected]>
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.

[khilman: added domain as parameter to callbacks, as suggested by Dmitry]
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/base/power/domain.c +2 -2
index 40bc2f4072cc..b520687046d4 100644
@@ -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);
 
include/linux/pm_domain.h +4 -3
index 73e938b7e937..b3ed7766a291 100644
@@ -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

#16 lore raw

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

In reply to Kevin Hilman
Quoted 59 lines · Kevin Hilman
Quoted 34 lines · Rafael J. Wysocki
Quoted 33 lines · Kevin Hilman
Quoted 30 lines · Dmitry Torokhov
Quoted 17 lines · Kevin Hilman
Quoted 5 lines · Rafael J. Wysocki
Quoted 4 lines · Ulf Hansson

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

Quoted 7 lines
Quoted 7 lines

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.

Quoted 8 lines
Quoted 4 lines
Quoted 2 lines

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

Quoted 7 lines
Quoted 4 lines

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 <[email protected]>
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.

Quoted 8 lines

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.

[khilman: added domain as parameter to callbacks, as suggested by Dmitry]

That's in the Linus' tree now.

Quoted 45 lines
---
 drivers/base/power/domain.c | 4 ++--
 include/linux/pm_domain.h   | 6 ++++--
 2 files changed, 6 insertions(+), 4 deletions(-)
drivers/base/power/domain.c +2 -2
index 40bc2f4072cc..b520687046d4 100644
@@ -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);
 
include/linux/pm_domain.h +4 -2
index 73e938b7e937..b3ed7766a291 100644
@@ -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)
Signature · 2 lines
I speak only for myself.
Rafael J. Wysocki, Intel Open Source Technology Center.
#17 lore raw

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

Hi Rafael,

Quoted 10 lines · Rafael J. Wysocki
Quoted 10 lines

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.

Quoted 6 lines
Quoted 4 lines
[khilman: added domain as parameter to callbacks, as suggested by Dmitry]

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