[Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Pierre Ossman]

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

Perhaps Mr Morton can have a look? This patch is probably -mm material 
anyhow.

Rgds
Pierre


[-- Attachment #2: [PATCH] [PNP][RFC] Suspend support for PNP bus. --]
[-- Type: message/rfc822, Size: 9280 bytes --]

From: Pierre Ossman <drzeus@drzeus.cx>
To: ambx1@neo.rr.com
Cc: Pierre Ossman <drzeus-list@drzeus.cx>
Cc: linux-kernel@vger.kernel.org
Subject: [PATCH] [PNP][RFC] Suspend support for PNP bus.
Date: Fri, 28 Oct 2005 09:39:01 +0200
Message-ID: <20051028073900.4148.83481.stgit@poseidon.drzeus.cx>

Add support for suspending devices connected to the PNP bus. New
callbacks are added for the drivers and the PNP hardware layer is
told to disable the device during the suspend.

Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
---

 drivers/pnp/driver.c  |   51 +++++++++++++++++++++++++++++++-
 drivers/pnp/manager.c |   78 +++++++++++++++++++++++++++++++++++++------------
 include/linux/pnp.h   |   10 +++++-
 3 files changed, 116 insertions(+), 23 deletions(-)

diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -146,10 +146,57 @@ static int pnp_bus_match(struct device *
 	return 1;
 }
 
+static int pnp_bus_suspend(struct device *dev, pm_message_t state)
+{
+	struct pnp_dev * pnp_dev = to_pnp_dev(dev);
+	struct pnp_driver * drv = pnp_dev->driver;
+	int error;
+
+	if (!drv)
+		return 0;
+
+	if (drv->suspend) {
+		error = drv->suspend(pnp_dev, state);
+		if (error)
+			return error;
+	}
+
+	if (!(drv->flags & PNP_DRIVER_RES_DO_NOT_CHANGE) &&
+	    pnp_can_disable(pnp_dev)) {
+	    	error = pnp_stop_dev(pnp_dev);
+	    	if (error)
+	    		return error;
+	}
+
+	return 0;
+}
+
+static int pnp_bus_resume(struct device *dev)
+{
+	struct pnp_dev * pnp_dev = to_pnp_dev(dev);
+	struct pnp_driver * drv = pnp_dev->driver;
+	int error;
+
+	if (!drv)
+		return 0;
+
+	if (!(drv->flags & PNP_DRIVER_RES_DO_NOT_CHANGE)) {
+		error = pnp_start_dev(pnp_dev);
+		if (error)
+			return error;
+	}
+
+	if (drv->resume)
+		return drv->resume(pnp_dev);
+
+	return 0;
+}
 
 struct bus_type pnp_bus_type = {
-	.name	= "pnp",
-	.match	= pnp_bus_match,
+	.name		= "pnp",
+	.match		= pnp_bus_match,
+	.suspend	= pnp_bus_suspend,
+	.resume		= pnp_bus_resume,
 };
 
 
diff --git a/drivers/pnp/manager.c b/drivers/pnp/manager.c
--- a/drivers/pnp/manager.c
+++ b/drivers/pnp/manager.c
@@ -468,6 +468,53 @@ int pnp_auto_config_dev(struct pnp_dev *
 }
 
 /**
+ * pnp_start_dev - low-level start of the PnP device
+ * @dev: pointer to the desired device
+ *
+ * assumes that resources have alread been allocated
+ */
+
+int pnp_start_dev(struct pnp_dev *dev)
+{
+	if (!pnp_can_write(dev)) {
+		pnp_info("Device %s does not supported activation.", dev->dev.bus_id);
+		return -EINVAL;
+	}
+
+	if (dev->protocol->set(dev, &dev->res)<0) {
+		pnp_err("Failed to activate device %s.", dev->dev.bus_id);
+		return -EIO;
+	}
+
+	pnp_info("Device %s activated.", dev->dev.bus_id);
+
+	return 0;
+}
+
+/**
+ * pnp_stop_dev - low-level disable of the PnP device
+ * @dev: pointer to the desired device
+ *
+ * does not free resources
+ */
+
+int pnp_stop_dev(struct pnp_dev *dev)
+{
+	if (!pnp_can_disable(dev)) {
+		pnp_info("Device %s does not supported disabling.", dev->dev.bus_id);
+		return -EINVAL;
+	}
+	if (dev->protocol->disable(dev)<0) {
+		pnp_err("Failed to disable device %s.", dev->dev.bus_id);
+		return -EIO;
+	}
+
+	pnp_info("Device %s disabled.", dev->dev.bus_id);
+
+	return 0;
+}
+
+/**
  * pnp_activate_dev - activates a PnP device for use
  * @dev: pointer to the desired device
  *
@@ -475,6 +522,8 @@ int pnp_auto_config_dev(struct pnp_dev *
  */
 int pnp_activate_dev(struct pnp_dev *dev)
 {
+	int error;
+
 	if (!dev)
 		return -EINVAL;
 	if (dev->active) {
@@ -485,18 +534,11 @@ int pnp_activate_dev(struct pnp_dev *dev
 	if (pnp_auto_config_dev(dev))
 		return -EBUSY;
 
-	if (!pnp_can_write(dev)) {
-		pnp_info("Device %s does not supported activation.", dev->dev.bus_id);
-		return -EINVAL;
-	}
-
-	if (dev->protocol->set(dev, &dev->res)<0) {
-		pnp_err("Failed to activate device %s.", dev->dev.bus_id);
-		return -EIO;
-	}
+	error = pnp_start_dev(dev);
+	if (error)
+		return error;
 
 	dev->active = 1;
-	pnp_info("Device %s activated.", dev->dev.bus_id);
 
 	return 1;
 }
@@ -509,23 +551,19 @@ int pnp_activate_dev(struct pnp_dev *dev
  */
 int pnp_disable_dev(struct pnp_dev *dev)
 {
+	int error;
+
         if (!dev)
                 return -EINVAL;
 	if (!dev->active) {
 		return 0; /* the device is already disabled */
 	}
 
-	if (!pnp_can_disable(dev)) {
-		pnp_info("Device %s does not supported disabling.", dev->dev.bus_id);
-		return -EINVAL;
-	}
-	if (dev->protocol->disable(dev)<0) {
-		pnp_err("Failed to disable device %s.", dev->dev.bus_id);
-		return -EIO;
-	}
+	error = pnp_stop_dev(dev);
+	if (error)
+		return error;
 
 	dev->active = 0;
-	pnp_info("Device %s disabled.", dev->dev.bus_id);
 
 	/* release the resources so that other devices can use them */
 	down(&pnp_res_mutex);
@@ -554,6 +592,8 @@ void pnp_resource_change(struct resource
 
 EXPORT_SYMBOL(pnp_manual_config_dev);
 EXPORT_SYMBOL(pnp_auto_config_dev);
+EXPORT_SYMBOL(pnp_start_dev);
+EXPORT_SYMBOL(pnp_stop_dev);
 EXPORT_SYMBOL(pnp_activate_dev);
 EXPORT_SYMBOL(pnp_disable_dev);
 EXPORT_SYMBOL(pnp_resource_change);
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -292,8 +292,10 @@ struct pnp_driver {
 	char * name;
 	const struct pnp_device_id *id_table;
 	unsigned int flags;
-	int  (*probe)  (struct pnp_dev *dev, const struct pnp_device_id *dev_id);
-	void (*remove) (struct pnp_dev *dev);
+	int  (*probe)   (struct pnp_dev *dev, const struct pnp_device_id *dev_id);
+	void (*remove)  (struct pnp_dev *dev);
+	int  (*suspend) (struct pnp_dev *dev, pm_message_t state);
+	int  (*resume)  (struct pnp_dev *dev);
 	struct device_driver driver;
 };
 
@@ -381,6 +383,8 @@ void pnp_init_resource_table(struct pnp_
 int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res, int mode);
 int pnp_auto_config_dev(struct pnp_dev *dev);
 int pnp_validate_config(struct pnp_dev *dev);
+int pnp_start_dev(struct pnp_dev *dev);
+int pnp_stop_dev(struct pnp_dev *dev);
 int pnp_activate_dev(struct pnp_dev *dev);
 int pnp_disable_dev(struct pnp_dev *dev);
 void pnp_resource_change(struct resource *resource, unsigned long start, unsigned long size);
@@ -425,6 +429,8 @@ static inline void pnp_init_resource_tab
 static inline int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res, int mode) { return -ENODEV; }
 static inline int pnp_auto_config_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_validate_config(struct pnp_dev *dev) { return -ENODEV; }
+static inline int pnp_start_dev(struct pnp_dev *dev) { return -ENODEV; }
+static inline int pnp_stop_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_activate_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_disable_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline void pnp_resource_change(struct resource *resource, unsigned long start, unsigned long size) { }

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Dmitry Torokhov]

On 11/4/05, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> +
> +int pnp_start_dev(struct pnp_dev *dev)
> +{
> +       if (!pnp_can_write(dev)) {
> +               pnp_info("Device %s does not supported activation.", dev->dev.bus_id);

"...does not support...", there is no "ed" at the end.

> +               return -EINVAL;
> +       }

Hmm, would'nt presence of such device stop suspend process? It will
cause pnp_bus_resume to fail too. Perhaps returning 0 in this case is
better.

> +
> +int pnp_stop_dev(struct pnp_dev *dev)
> +{
> +       if (!pnp_can_disable(dev)) {
> +               pnp_info("Device %s does not supported disabling.", dev->dev.bus_id);
> +               return -EINVAL;

Same here. No "ed" and -EINVAL will hurt.

--
Dmitry

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Pierre Ossman]

Dmitry Torokhov wrote:
> On 11/4/05, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>   
>> +
>> +int pnp_start_dev(struct pnp_dev *dev)
>> +{
>> +       if (!pnp_can_write(dev)) {
>> +               pnp_info("Device %s does not supported activation.", dev->dev.bus_id);
>>     
>
> "...does not support...", there is no "ed" at the end.
>
>   

That's just code that's been moved around. But I suppose a speling fix 
could be included in the same patch. :)

>> +               return -EINVAL;
>> +       }
>>     
>
> Hmm, would'nt presence of such device stop suspend process? It will
> cause pnp_bus_resume to fail too. Perhaps returning 0 in this case is
> better.
>
>   

The problem is that this code is also visited from pnp_activate_dev() & 
co where this return value is needed. For pnp_stop_dev() the same check 
(pnp_can_disable()) is performed in the suspend routine to avoid that 
particular problem. For resume my assumption was that a device that 
doesn't support activation will not have a driver attached to it. 
Perhaps this is wrong?

Rgds
Pierre

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Dmitry Torokhov]

On 11/4/05, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> Dmitry Torokhov wrote:
> >
> > Hmm, would'nt presence of such device stop suspend process? It will
> > cause pnp_bus_resume to fail too. Perhaps returning 0 in this case is
> > better.
> >
> >
>
> The problem is that this code is also visited from pnp_activate_dev() &
> co where this return value is needed. For pnp_stop_dev() the same check
> (pnp_can_disable()) is performed in the suspend routine to avoid that
> particular problem. For resume my assumption was that a device that
> doesn't support activation will not have a driver attached to it.
> Perhaps this is wrong?
>

i8042 registers drivers for keyboard and AUX ports to gather
information whether the ports are present and what IRQ and IO ports
shoudl be used to access them. And I have seen a few boxes that do not
alloe [de]activate these devices.

--
Dmitry

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Pierre Ossman]

Dmitry Torokhov wrote:
> On 11/4/05, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>   
>> Dmitry Torokhov wrote:
>>     
>>> Hmm, would'nt presence of such device stop suspend process? It will
>>> cause pnp_bus_resume to fail too. Perhaps returning 0 in this case is
>>> better.
>>>
>>>
>>>       
>> The problem is that this code is also visited from pnp_activate_dev() &
>> co where this return value is needed. For pnp_stop_dev() the same check
>> (pnp_can_disable()) is performed in the suspend routine to avoid that
>> particular problem. For resume my assumption was that a device that
>> doesn't support activation will not have a driver attached to it.
>> Perhaps this is wrong?
>>
>>     
>
> i8042 registers drivers for keyboard and AUX ports to gather
> information whether the ports are present and what IRQ and IO ports
> shoudl be used to access them. And I have seen a few boxes that do not
> alloe [de]activate these devices.
>
>   

But the activation is performed by the PNP layer when the driver is 
matched with a driver. So the driver isn't really responsible for the 
activation. It can prevent activation through 
PNP_DRIVER_RES_DO_NOT_CHANGE though, but that flag is also checked in 
the suspend routines.

Rgds
Pierre

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Dmitry Torokhov]

On 11/4/05, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> Dmitry Torokhov wrote:
> > On 11/4/05, Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> >
> >> Dmitry Torokhov wrote:
> >>
> >>> Hmm, would'nt presence of such device stop suspend process? It will
> >>> cause pnp_bus_resume to fail too. Perhaps returning 0 in this case is
> >>> better.
> >>>
> >>>
> >>>
> >> The problem is that this code is also visited from pnp_activate_dev() &
> >> co where this return value is needed. For pnp_stop_dev() the same check
> >> (pnp_can_disable()) is performed in the suspend routine to avoid that
> >> particular problem. For resume my assumption was that a device that
> >> doesn't support activation will not have a driver attached to it.
> >> Perhaps this is wrong?
> >>
> >>
> >
> > i8042 registers drivers for keyboard and AUX ports to gather
> > information whether the ports are present and what IRQ and IO ports
> > shoudl be used to access them. And I have seen a few boxes that do not
> > alloe [de]activate these devices.
> >
> >
>
> But the activation is performed by the PNP layer when the driver is
> matched with a driver. So the driver isn't really responsible for the
> activation. It can prevent activation through
> PNP_DRIVER_RES_DO_NOT_CHANGE though, but that flag is also checked in
> the suspend routines.
>

You lost me... We have a scenario when a PNP driver is bound to a PNP
device that does not support deactivation. Looking at the proposed PNP
bus suspend code presence of such device will cause suspend process to
fail. Are you saying this is what you want?

--
Dmitry

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Dmitry Torokhov]

On 11/4/05, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> You lost me... We have a scenario when a PNP driver is bound to a PNP
> device that does not support deactivation. Looking at the proposed PNP
> bus suspend code presence of such device will cause suspend process to
> fail. Are you saying this is what you want?
>

Ugh, scratch whatever I wrote earlier. Such devices should be marked
with RES_DO_NOT_CHANEG so everything is fine.

Sorry about the noise.

--
Dmitry

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Andrew Morton]

Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
>
> On 11/4/05, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >
> > You lost me... We have a scenario when a PNP driver is bound to a PNP
> > device that does not support deactivation. Looking at the proposed PNP
> > bus suspend code presence of such device will cause suspend process to
> > fail. Are you saying this is what you want?
> >
> 
> Ugh, scratch whatever I wrote earlier. Such devices should be marked
> with RES_DO_NOT_CHANEG so everything is fine.
> 
> Sorry about the noise.

So...  You're OK with the patch as proposed?

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Dmitry Torokhov]

On Saturday 05 November 2005 02:15, Andrew Morton wrote:
> Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> >
> > On 11/4/05, Dmitry Torokhov <dmitry.torokhov@gmail.com> wrote:
> > >
> > > You lost me... We have a scenario when a PNP driver is bound to a PNP
> > > device that does not support deactivation. Looking at the proposed PNP
> > > bus suspend code presence of such device will cause suspend process to
> > > fail. Are you saying this is what you want?
> > >
> > 
> > Ugh, scratch whatever I wrote earlier. Such devices should be marked
> > with RES_DO_NOT_CHANEG so everything is fine.
> > 
> > Sorry about the noise.
> 
> So...  You're OK with the patch as proposed?
> 

Yes I am.

-- 
Dmitry

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Pierre Ossman]

Andrew Morton wrote:

>Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>  
>
>>Add support for suspending devices connected to the PNP bus. New
>>callbacks are added for the drivers and the PNP hardware layer is
>>told to disable the device during the suspend.
>>    
>>
>
>The ALSA guys have gone off and implemented their own version of this, and
>it's a bit different.   I'll need to drop this patch now.
>
>Please review http://www.zip.com.au/~akpm/linux/patches/stuff/git-alsa.patch, sort
>things out?
>  
>

That things is huge! Do the ALSA guys perhaps have a patch with just the
PnP bit in it?

Rgds
Pierre

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Andrew Morton]

Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>
> Add support for suspending devices connected to the PNP bus. New
> callbacks are added for the drivers and the PNP hardware layer is
> told to disable the device during the suspend.

The ALSA guys have gone off and implemented their own version of this, and
it's a bit different.   I'll need to drop this patch now.

Please review http://www.zip.com.au/~akpm/linux/patches/stuff/git-alsa.patch, sort
things out?

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Pierre Ossman]

Andrew Morton wrote:

>Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>  
>
>>Andrew Morton wrote:
>>
>>    
>>
>>>Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>>> 
>>>
>>>      
>>>
>>>>Add support for suspending devices connected to the PNP bus. New
>>>>callbacks are added for the drivers and the PNP hardware layer is
>>>>told to disable the device during the suspend.
>>>>   
>>>>
>>>>        
>>>>
>>>The ALSA guys have gone off and implemented their own version of this, and
>>>it's a bit different.   I'll need to drop this patch now.
>>>
>>>Please review http://www.zip.com.au/~akpm/linux/patches/stuff/git-alsa.patch, sort
>>>things out?
>>> 
>>>
>>>      
>>>
>>That things is huge! Do the ALSA guys perhaps have a patch with just the
>>PnP bit in it?
>>
>>    
>>
>
>http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/perex/alsa-current.git;a=commitdiff;h=5353d906effe648dd20899fe61ecb6982ad93cdd
>
>  
>

That patch is a bit dumber than mine. It doesn't do anything but call
the driver supplied suspend/resume function, i.e. no PnP handling during
suspend. It does handle cards though, something my patch doesn't do.
Perhaps a combination of the two is acceptable to the ALSA crowd?

Rgds
Pierre

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Andrew Morton]

Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>
> Andrew Morton wrote:
> 
> >Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> >  
> >
> >>Add support for suspending devices connected to the PNP bus. New
> >>callbacks are added for the drivers and the PNP hardware layer is
> >>told to disable the device during the suspend.
> >>    
> >>
> >
> >The ALSA guys have gone off and implemented their own version of this, and
> >it's a bit different.   I'll need to drop this patch now.
> >
> >Please review http://www.zip.com.au/~akpm/linux/patches/stuff/git-alsa.patch, sort
> >things out?
> >  
> >
> 
> That things is huge! Do the ALSA guys perhaps have a patch with just the
> PnP bit in it?
> 

http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/perex/alsa-current.git;a=commitdiff;h=5353d906effe648dd20899fe61ecb6982ad93cdd

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Pierre Ossman]

Andrew Morton wrote:

>Send an update agaisnt next -mm if you like.  We can feed that in through
>the alsa tree too.  It's not really appropriate to be updating the pnp
>system via the alsa tree, but as long as it's all in one place, it works.
>
>  
>

Ok. I'll wait for your next compilation then.

Rgds
Pierre

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Andrew Morton]

Pierre Ossman <drzeus-list@drzeus.cx> wrote:
>
> >http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/perex/alsa-current.git;a=commitdiff;h=5353d906effe648dd20899fe61ecb6982ad93cdd
> >
> >  
> >
> 
> That patch is a bit dumber than mine. It doesn't do anything but call
> the driver supplied suspend/resume function, i.e. no PnP handling during
> suspend. It does handle cards though, something my patch doesn't do.
> Perhaps a combination of the two is acceptable to the ALSA crowd?

Send an update agaisnt next -mm if you like.  We can feed that in through
the alsa tree too.  It's not really appropriate to be updating the pnp
system via the alsa tree, but as long as it's all in one place, it works.

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Pierre Ossman]

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

Andrew Morton wrote:
> Send an update agaisnt next -mm if you like.  We can feed that in through
> the alsa tree too.  It's not really appropriate to be updating the pnp
> system via the alsa tree, but as long as it's all in one place, it works.
>
>   

Assuming the version you pointed out to me is the final one, then this 
patch will merge my changes into theirs.

Rgds
Pierre


[-- Attachment #2: pnp-suspend.patch --]
[-- Type: text/x-patch, Size: 6172 bytes --]

[PNP] Improved PnP suspend support.

Also use the PnP functions to start/stop the devices during the suspend so
that drivers will not have to duplicate this code.

Signed-off-by: Pierre Ossman <drzeus@drzeus.cx>
---

 drivers/pnp/driver.c  |   37 ++++++++++++++++++++---
 drivers/pnp/manager.c |   78 +++++++++++++++++++++++++++++++++++++------------
 include/linux/pnp.h   |    4 +++
 3 files changed, 95 insertions(+), 24 deletions(-)

diff --git a/drivers/pnp/driver.c b/drivers/pnp/driver.c
index ea2cb9a..15fb758 100644
--- a/drivers/pnp/driver.c
+++ b/drivers/pnp/driver.c
@@ -150,19 +150,46 @@ static int pnp_bus_suspend(struct device
 {
 	struct pnp_dev * pnp_dev = to_pnp_dev(dev);
 	struct pnp_driver * pnp_drv = pnp_dev->driver;
+	int error;
+
+	if (!pnp_drv)
+		return 0;
+
+	if (pnp_drv->suspend) {
+		error = pnp_drv->suspend(pnp_dev, state);
+		if (error)
+			return error;
+	}
+
+	if (!(pnp_drv->flags & PNP_DRIVER_RES_DO_NOT_CHANGE) &&
+	    pnp_can_disable(pnp_dev)) {
+	    	error = pnp_stop_dev(pnp_dev);
+	    	if (error)
+	    		return error;
+	}
 
-	if (pnp_drv && pnp_drv->suspend)
-		return pnp_drv->suspend(pnp_dev, state);
 	return 0;
 }
 
-static void pnp_bus_resume(struct device *dev)
+static int pnp_bus_resume(struct device *dev)
 {
 	struct pnp_dev * pnp_dev = to_pnp_dev(dev);
 	struct pnp_driver * pnp_drv = pnp_dev->driver;
+	int error;
 
-	if (pnp_drv && pnp_drv->resume)
-		pnp_drv->resume(pnp_dev);
+	if (!pnp_drv)
+		return 0;
+
+	if (!(pnp_drv->flags & PNP_DRIVER_RES_DO_NOT_CHANGE)) {
+		error = pnp_start_dev(pnp_dev);
+		if (error)
+			return error;
+	}
+
+	if (pnp_drv->resume)
+		return pnp_drv->resume(pnp_dev);
+
+	return 0;
 }
 
 struct bus_type pnp_bus_type = {
diff --git a/drivers/pnp/manager.c b/drivers/pnp/manager.c
index 2616686..c4256aa 100644
--- a/drivers/pnp/manager.c
+++ b/drivers/pnp/manager.c
@@ -470,6 +470,53 @@ int pnp_auto_config_dev(struct pnp_dev *
 }
 
 /**
+ * pnp_start_dev - low-level start of the PnP device
+ * @dev: pointer to the desired device
+ *
+ * assumes that resources have alread been allocated
+ */
+
+int pnp_start_dev(struct pnp_dev *dev)
+{
+	if (!pnp_can_write(dev)) {
+		pnp_info("Device %s does not supported activation.", dev->dev.bus_id);
+		return -EINVAL;
+	}
+
+	if (dev->protocol->set(dev, &dev->res)<0) {
+		pnp_err("Failed to activate device %s.", dev->dev.bus_id);
+		return -EIO;
+	}
+
+	pnp_info("Device %s activated.", dev->dev.bus_id);
+
+	return 0;
+}
+
+/**
+ * pnp_stop_dev - low-level disable of the PnP device
+ * @dev: pointer to the desired device
+ *
+ * does not free resources
+ */
+
+int pnp_stop_dev(struct pnp_dev *dev)
+{
+	if (!pnp_can_disable(dev)) {
+		pnp_info("Device %s does not supported disabling.", dev->dev.bus_id);
+		return -EINVAL;
+	}
+	if (dev->protocol->disable(dev)<0) {
+		pnp_err("Failed to disable device %s.", dev->dev.bus_id);
+		return -EIO;
+	}
+
+	pnp_info("Device %s disabled.", dev->dev.bus_id);
+
+	return 0;
+}
+
+/**
  * pnp_activate_dev - activates a PnP device for use
  * @dev: pointer to the desired device
  *
@@ -477,6 +524,8 @@ int pnp_auto_config_dev(struct pnp_dev *
  */
 int pnp_activate_dev(struct pnp_dev *dev)
 {
+	int error;
+
 	if (!dev)
 		return -EINVAL;
 	if (dev->active) {
@@ -487,18 +536,11 @@ int pnp_activate_dev(struct pnp_dev *dev
 	if (pnp_auto_config_dev(dev))
 		return -EBUSY;
 
-	if (!pnp_can_write(dev)) {
-		pnp_info("Device %s does not supported activation.", dev->dev.bus_id);
-		return -EINVAL;
-	}
-
-	if (dev->protocol->set(dev, &dev->res)<0) {
-		pnp_err("Failed to activate device %s.", dev->dev.bus_id);
-		return -EIO;
-	}
+	error = pnp_start_dev(dev);
+	if (error)
+		return error;
 
 	dev->active = 1;
-	pnp_info("Device %s activated.", dev->dev.bus_id);
 
 	return 1;
 }
@@ -511,23 +553,19 @@ int pnp_activate_dev(struct pnp_dev *dev
  */
 int pnp_disable_dev(struct pnp_dev *dev)
 {
+	int error;
+
         if (!dev)
                 return -EINVAL;
 	if (!dev->active) {
 		return 0; /* the device is already disabled */
 	}
 
-	if (!pnp_can_disable(dev)) {
-		pnp_info("Device %s does not supported disabling.", dev->dev.bus_id);
-		return -EINVAL;
-	}
-	if (dev->protocol->disable(dev)<0) {
-		pnp_err("Failed to disable device %s.", dev->dev.bus_id);
-		return -EIO;
-	}
+	error = pnp_stop_dev(dev);
+	if (error)
+		return error;
 
 	dev->active = 0;
-	pnp_info("Device %s disabled.", dev->dev.bus_id);
 
 	/* release the resources so that other devices can use them */
 	down(&pnp_res_mutex);
@@ -558,6 +596,8 @@ EXPORT_SYMBOL(pnp_manual_config_dev);
 #if 0
 EXPORT_SYMBOL(pnp_auto_config_dev);
 #endif
+EXPORT_SYMBOL(pnp_start_dev);
+EXPORT_SYMBOL(pnp_stop_dev);
 EXPORT_SYMBOL(pnp_activate_dev);
 EXPORT_SYMBOL(pnp_disable_dev);
 EXPORT_SYMBOL(pnp_resource_change);
diff --git a/include/linux/pnp.h b/include/linux/pnp.h
index 472319f..93b0959 100644
--- a/include/linux/pnp.h
+++ b/include/linux/pnp.h
@@ -385,6 +385,8 @@ void pnp_init_resource_table(struct pnp_
 int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res, int mode);
 int pnp_auto_config_dev(struct pnp_dev *dev);
 int pnp_validate_config(struct pnp_dev *dev);
+int pnp_start_dev(struct pnp_dev *dev);
+int pnp_stop_dev(struct pnp_dev *dev);
 int pnp_activate_dev(struct pnp_dev *dev);
 int pnp_disable_dev(struct pnp_dev *dev);
 void pnp_resource_change(struct resource *resource, unsigned long start, unsigned long size);
@@ -428,6 +430,8 @@ static inline void pnp_init_resource_tab
 static inline int pnp_manual_config_dev(struct pnp_dev *dev, struct pnp_resource_table *res, int mode) { return -ENODEV; }
 static inline int pnp_auto_config_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_validate_config(struct pnp_dev *dev) { return -ENODEV; }
+static inline int pnp_start_dev(struct pnp_dev *dev) { return -ENODEV; }
+static inline int pnp_stop_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_activate_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline int pnp_disable_dev(struct pnp_dev *dev) { return -ENODEV; }
 static inline void pnp_resource_change(struct resource *resource, unsigned long start, unsigned long size) { }

Re: [Fwd: [PATCH] [PNP][RFC] Suspend support for PNP bus.]

[Takashi Iwai]

At Tue, 29 Nov 2005 20:49:44 +0100,
Pierre Ossman wrote:
> 
> Andrew Morton wrote:
> 
> >Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> >  
> >
> >>Andrew Morton wrote:
> >>
> >>    
> >>
> >>>Pierre Ossman <drzeus-list@drzeus.cx> wrote:
> >>> 
> >>>
> >>>      
> >>>
> >>>>Add support for suspending devices connected to the PNP bus. New
> >>>>callbacks are added for the drivers and the PNP hardware layer is
> >>>>told to disable the device during the suspend.
> >>>>   
> >>>>
> >>>>        
> >>>>
> >>>The ALSA guys have gone off and implemented their own version of this, and
> >>>it's a bit different.   I'll need to drop this patch now.
> >>>
> >>>Please review http://www.zip.com.au/~akpm/linux/patches/stuff/git-alsa.patch, sort
> >>>things out?
> >>> 
> >>>
> >>>      
> >>>
> >>That things is huge! Do the ALSA guys perhaps have a patch with just the
> >>PnP bit in it?
> >>
> >>    
> >>
> >
> >http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/perex/alsa-current.git;a=commitdiff;h=5353d906effe648dd20899fe61ecb6982ad93cdd
> >
> >  
> >
> 
> That patch is a bit dumber than mine. It doesn't do anything but call
> the driver supplied suspend/resume function, i.e. no PnP handling during
> suspend. It does handle cards though, something my patch doesn't do.
> Perhaps a combination of the two is acceptable to the ALSA crowd?

Yep, my implementation is only for very basic things and may lack of
many stuff.  Any fix would be welcome.


Takashi