[PATCH 4/4]An ACPI callback for pci_set_power_state

[PATCH 4/4]An ACPI callback for pci_set_power_state

[Li Shaohua]

[-- Attachment #1: Type: text/plain, Size: 4109 bytes --]

Hi,
This is an ACPI callback for pci_set_power_state. Besides setting PCI
config space, changing device's power state sometimes requires to power
on device's power source and to invoke other firmware methods.

Thanks,
Shaohua

SIgned-off-by: Li Shaohua<shaohua.li-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---

 2.5-root/drivers/acpi/bus.c     |    8 +++++++-
 2.5-root/drivers/pci/pci-acpi.c |   11 +++++++++++
 2.5-root/drivers/pci/pci.c      |   11 +++++++++--
 2.5-root/drivers/pci/pci.h      |    1 +
 4 files changed, 28 insertions(+), 3 deletions(-)

diff -puN drivers/acpi/bus.c~acpi-pci-set-power-state-callback drivers/acpi/bus.c
--- 2.5/drivers/acpi/bus.c~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.464923888 +0800
+++ 2.5-root/drivers/acpi/bus.c	2005-01-05 09:58:06.473922520 +0800
@@ -212,6 +212,12 @@ acpi_bus_set_power (
 		ACPI_DEBUG_PRINT((ACPI_DB_WARN, "Device is not power manageable\n"));
 		return_VALUE(-ENODEV);
 	}
+	/*
+	 * Get device's current power state if it's unknown
+	 * This means device power state isn't initialized or previous setting failed
+	 */
+	if (device->power.state == ACPI_STATE_UNKNOWN)
+		acpi_bus_get_power(device->handle, &device->power.state);
 	if (state == device->power.state) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at D%d\n", state));
 		return_VALUE(0);
@@ -231,7 +237,7 @@ acpi_bus_set_power (
 	 * On transitions to a high-powered state we first apply power (via
 	 * power resources) then evalute _PSx.  Conversly for transitions to
 	 * a lower-powered state.
-	 */ 
+	 */
 	if (state < device->power.state) {
 		if (device->power.flags.power_resources) {
 			result = acpi_power_transition(device, state);
diff -puN drivers/pci/pci-acpi.c~acpi-pci-set-power-state-callback drivers/pci/pci-acpi.c
--- 2.5/drivers/pci/pci-acpi.c~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.466923584 +0800
+++ 2.5-root/drivers/pci/pci-acpi.c	2005-01-05 09:58:06.472922672 +0800
@@ -226,6 +226,16 @@ static int acpi_get_suspend_state(struct
 	return -ENODEV;
 }
 
+static int acpi_set_power_state(struct pci_dev *dev, int state)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+	if (!handle)
+		return -ENODEV;
+	return acpi_bus_set_power(handle, state);
+}
+
+
 /* ACPI bus type */
 int pci_acpi_bind_device(struct device *dev, acpi_handle *handle)
 {
@@ -269,6 +279,7 @@ static int __init pci_acpi_init(void)
 	if (ret)
 		return 0;
 	platform_pci_get_suspend_state = acpi_get_suspend_state;
+	platform_pci_set_power_state = acpi_set_power_state;
 	return 0;
 }
 arch_initcall(pci_acpi_init);
diff -puN drivers/pci/pci.c~acpi-pci-set-power-state-callback drivers/pci/pci.c
--- 2.5/drivers/pci/pci.c~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.467923432 +0800
+++ 2.5-root/drivers/pci/pci.c	2005-01-05 09:58:06.473922520 +0800
@@ -240,7 +240,7 @@ pci_find_parent_resource(const struct pc
  * -EIO if device does not support PCI PM.
  * 0 if we can successfully change the power state.
  */
-
+int (*platform_pci_set_power_state)(struct pci_dev *, int) = NULL;
 int
 pci_set_power_state(struct pci_dev *dev, int state)
 {
@@ -294,8 +294,15 @@ pci_set_power_state(struct pci_dev *dev,
 		msleep(10);
 	else if(state == 2 || dev->current_state == 2)
 		udelay(200);
-	dev->current_state = state;
 
+	/*
+	 * Give firmware a chance to be called, such as ACPI _PRx, _PSx
+	 * Firmware method after natice method ?
+	 */
+	if (platform_pci_set_power_state)
+		platform_pci_set_power_state(dev, state);
+
+	dev->current_state = state;
 	return 0;
 }
 
diff -puN drivers/pci/pci.h~acpi-pci-set-power-state-callback drivers/pci/pci.h
--- 2.5/drivers/pci/pci.h~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.469923128 +0800
+++ 2.5-root/drivers/pci/pci.h	2005-01-05 09:58:06.473922520 +0800
@@ -13,6 +13,7 @@ extern int pci_bus_alloc_resource(struct
 				  void *alignf_data);
 /* Firmware callbacks */
 extern int (*platform_pci_get_suspend_state)(struct device *dev, u32 state);
+extern int (*platform_pci_set_power_state)(struct pci_dev *dev, int state);
 
 /* PCI /proc functions */
 #ifdef CONFIG_PROC_FS
_


[-- Attachment #2: p00004_acpi-pci-set-power-state-callback.patch --]
[-- Type: text/x-patch, Size: 3846 bytes --]


An ACPI callback for PCI set power state

---

 2.5-root/drivers/acpi/bus.c     |    8 +++++++-
 2.5-root/drivers/pci/pci-acpi.c |   11 +++++++++++
 2.5-root/drivers/pci/pci.c      |   11 +++++++++--
 2.5-root/drivers/pci/pci.h      |    1 +
 4 files changed, 28 insertions(+), 3 deletions(-)

diff -puN drivers/acpi/bus.c~acpi-pci-set-power-state-callback drivers/acpi/bus.c
--- 2.5/drivers/acpi/bus.c~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.464923888 +0800
+++ 2.5-root/drivers/acpi/bus.c	2005-01-05 09:58:06.473922520 +0800
@@ -212,6 +212,12 @@ acpi_bus_set_power (
 		ACPI_DEBUG_PRINT((ACPI_DB_WARN, "Device is not power manageable\n"));
 		return_VALUE(-ENODEV);
 	}
+	/*
+	 * Get device's current power state if it's unknown
+	 * This means device power state isn't initialized or previous setting failed
+	 */
+	if (device->power.state == ACPI_STATE_UNKNOWN)
+		acpi_bus_get_power(device->handle, &device->power.state);
 	if (state == device->power.state) {
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO, "Device is already at D%d\n", state));
 		return_VALUE(0);
@@ -231,7 +237,7 @@ acpi_bus_set_power (
 	 * On transitions to a high-powered state we first apply power (via
 	 * power resources) then evalute _PSx.  Conversly for transitions to
 	 * a lower-powered state.
-	 */ 
+	 */
 	if (state < device->power.state) {
 		if (device->power.flags.power_resources) {
 			result = acpi_power_transition(device, state);
diff -puN drivers/pci/pci-acpi.c~acpi-pci-set-power-state-callback drivers/pci/pci-acpi.c
--- 2.5/drivers/pci/pci-acpi.c~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.466923584 +0800
+++ 2.5-root/drivers/pci/pci-acpi.c	2005-01-05 09:58:06.472922672 +0800
@@ -226,6 +226,16 @@ static int acpi_get_suspend_state(struct
 	return -ENODEV;
 }
 
+static int acpi_set_power_state(struct pci_dev *dev, int state)
+{
+	acpi_handle handle = DEVICE_ACPI_HANDLE(&dev->dev);
+
+	if (!handle)
+		return -ENODEV;
+	return acpi_bus_set_power(handle, state);
+}
+
+
 /* ACPI bus type */
 int pci_acpi_bind_device(struct device *dev, acpi_handle *handle)
 {
@@ -269,6 +279,7 @@ static int __init pci_acpi_init(void)
 	if (ret)
 		return 0;
 	platform_pci_get_suspend_state = acpi_get_suspend_state;
+	platform_pci_set_power_state = acpi_set_power_state;
 	return 0;
 }
 arch_initcall(pci_acpi_init);
diff -puN drivers/pci/pci.c~acpi-pci-set-power-state-callback drivers/pci/pci.c
--- 2.5/drivers/pci/pci.c~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.467923432 +0800
+++ 2.5-root/drivers/pci/pci.c	2005-01-05 09:58:06.473922520 +0800
@@ -240,7 +240,7 @@ pci_find_parent_resource(const struct pc
  * -EIO if device does not support PCI PM.
  * 0 if we can successfully change the power state.
  */
-
+int (*platform_pci_set_power_state)(struct pci_dev *, int) = NULL;
 int
 pci_set_power_state(struct pci_dev *dev, int state)
 {
@@ -294,8 +294,15 @@ pci_set_power_state(struct pci_dev *dev,
 		msleep(10);
 	else if(state == 2 || dev->current_state == 2)
 		udelay(200);
-	dev->current_state = state;
 
+	/*
+	 * Give firmware a chance to be called, such as ACPI _PRx, _PSx
+	 * Firmware method after natice method ?
+	 */
+	if (platform_pci_set_power_state)
+		platform_pci_set_power_state(dev, state);
+
+	dev->current_state = state;
 	return 0;
 }
 
diff -puN drivers/pci/pci.h~acpi-pci-set-power-state-callback drivers/pci/pci.h
--- 2.5/drivers/pci/pci.h~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.469923128 +0800
+++ 2.5-root/drivers/pci/pci.h	2005-01-05 09:58:06.473922520 +0800
@@ -13,6 +13,7 @@ extern int pci_bus_alloc_resource(struct
 				  void *alignf_data);
 /* Firmware callbacks */
 extern int (*platform_pci_get_suspend_state)(struct device *dev, u32 state);
+extern int (*platform_pci_set_power_state)(struct pci_dev *dev, int state);
 
 /* PCI /proc functions */
 #ifdef CONFIG_PROC_FS
_

Re: [PATCH 4/4]An ACPI callback for pci_set_power_state

[Pavel Machek]

Hi!

> This is an ACPI callback for pci_set_power_state. Besides setting PCI
> config space, changing device's power state sometimes requires to power
> on device's power source and to invoke other firmware methods.


> diff -puN drivers/pci/pci.h~acpi-pci-set-power-state-callback drivers/pci/pci.h
> --- 2.5/drivers/pci/pci.h~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.469923128 +0800
> +++ 2.5-root/drivers/pci/pci.h	2005-01-05 09:58:06.473922520 +0800
> @@ -13,6 +13,7 @@ extern int pci_bus_alloc_resource(struct
>  				  void *alignf_data);
>  /* Firmware callbacks */
>  extern int (*platform_pci_get_suspend_state)(struct device *dev, u32 state);
> +extern int (*platform_pci_set_power_state)(struct pci_dev *dev, int state);

What kind of state is passed here? Why is it u32 in one case and int
in the second one?

I'm about to introduce separate types for pci power states and system
power states; could you at least add comments which states are which?
Also few lines of documentation would be very usefull...

								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: Re: [PATCH 4/4]An ACPI callback for pci_set_power_state

[Li Shaohua]

On Wed, 2005-01-05 at 16:49, Pavel Machek wrote:
> Hi!
> 
> > This is an ACPI callback for pci_set_power_state. Besides setting PCI
> > config space, changing device's power state sometimes requires to power
> > on device's power source and to invoke other firmware methods.
> 
> 
> > diff -puN drivers/pci/pci.h~acpi-pci-set-power-state-callback drivers/pci/pci.h
> > --- 2.5/drivers/pci/pci.h~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.469923128 +0800
> > +++ 2.5-root/drivers/pci/pci.h	2005-01-05 09:58:06.473922520 +0800
> > @@ -13,6 +13,7 @@ extern int pci_bus_alloc_resource(struct
> >  				  void *alignf_data);
> >  /* Firmware callbacks */
> >  extern int (*platform_pci_get_suspend_state)(struct device *dev, u32 state);
> > +extern int (*platform_pci_set_power_state)(struct pci_dev *dev, int state);
> 
> What kind of state is passed here? Why is it u32 in one case and int
> in the second one?
This is the types of current pci routines (pci_set_power_state and pci_device_suspend). 
Both are PCI device states. I agree it's confused.

> I'm about to introduce separate types for pci power states and system
> power states; could you at least add comments which states are which?
> Also few lines of documentation would be very usefull...
I planed post the patches after your patches enter before, but it seems
have a long delay. What's your plan? I'm ok to merge the patches after
yours.

Thanks,
Shaohua



-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt

Re: Re: [PATCH 4/4]An ACPI callback for pci_set_power_state

[Pavel Machek]

Hi!

> > > This is an ACPI callback for pci_set_power_state. Besides setting PCI
> > > config space, changing device's power state sometimes requires to power
> > > on device's power source and to invoke other firmware methods.
> > 
> > 
> > > diff -puN drivers/pci/pci.h~acpi-pci-set-power-state-callback drivers/pci/pci.h
> > > --- 2.5/drivers/pci/pci.h~acpi-pci-set-power-state-callback	2005-01-05 09:58:06.469923128 +0800
> > > +++ 2.5-root/drivers/pci/pci.h	2005-01-05 09:58:06.473922520 +0800
> > > @@ -13,6 +13,7 @@ extern int pci_bus_alloc_resource(struct
> > >  				  void *alignf_data);
> > >  /* Firmware callbacks */
> > >  extern int (*platform_pci_get_suspend_state)(struct device *dev, u32 state);
> > > +extern int (*platform_pci_set_power_state)(struct pci_dev *dev, int state);
> > 
> > What kind of state is passed here? Why is it u32 in one case and int
> > in the second one?
> This is the types of current pci routines (pci_set_power_state and pci_device_suspend). 
> Both are PCI device states. I agree it's confused.

Ok, that should become pci_power_t in very near future, then.

> > I'm about to introduce separate types for pci power states and system
> > power states; could you at least add comments which states are which?
> > Also few lines of documentation would be very usefull...
> I planed post the patches after your patches enter before, but it seems
> have a long delay. What's your plan? I'm ok to merge the patches after
> yours.

pci_power_t patches were merged to Greg, so I'd say they are going to
linus "real soon now" :-).
								Pavel
-- 
People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!


-------------------------------------------------------
The SF.Net email is sponsored by: Beat the post-holiday blues
Get a FREE limited edition SourceForge.net t-shirt from ThinkGeek.
It's fun and FREE -- well, almost....http://www.thinkgeek.com/sfshirt