Re: Help needed, Re: [Bug #14334] pcmcia suspend regression from 2.6.31.1 to 2.6.31.2 - Dell Inspiron 600m

[Dominik Brodowski <linux@dominikbrodowski.net>]

Hey,

On Sun, Nov 01, 2009 at 09:36:10AM +0100, Rafael J. Wysocki wrote:
> Commit 0c570cdeb8fdfcb354a3e9cd81bfc6a09c19de0c
> (PM / yenta: Fix cardbus suspend/resume regression) caused resume to
> fail on systems with two CardBus bridges.  While the exact nature
> of the failure is not known at the moment, it can be worked around by
> splitting the yenta resume into an early part, executed during the
> early phase of resume, that will only resume the socket and power it
> up if there was a card in it during suspend, and a late part,
> executed during "regular" resume, that will carry out all of the
> remaining yenta resume operations.
> 
> Fixes http://bugzilla.kernel.org/show_bug.cgi?id=14334, which is a
> listed regression from 2.6.31.

The only issue I see is that we now return 0 unconditionally on the resume
callbacks. Otherwise, it's

Acked-by: Dominik Brodowski <linux@dominikbrodowski.net>

> Index: linux-2.6/drivers/pcmcia/yenta_socket.c
> ===================================================================
> --- linux-2.6.orig/drivers/pcmcia/yenta_socket.c
> +++ linux-2.6/drivers/pcmcia/yenta_socket.c
> @@ -1275,16 +1275,26 @@ static int yenta_dev_resume_noirq(struct
>  	if (socket->type && socket->type->restore_state)
>  		socket->type->restore_state(socket);
>  
> -	return pcmcia_socket_dev_resume(dev);
> +	pcmcia_socket_dev_early_resume(dev);
> +	return 0;
> +}
> +
> +static int yenta_dev_resume(struct device *dev)
> +{
> +	pcmcia_socket_dev_late_resume(dev);
> +	return 0;
>  }
>  
>  static struct dev_pm_ops yenta_pm_ops = {
>  	.suspend_noirq = yenta_dev_suspend_noirq,
>  	.resume_noirq = yenta_dev_resume_noirq,
> +	.resume = yenta_dev_resume,
>  	.freeze_noirq = yenta_dev_suspend_noirq,
>  	.thaw_noirq = yenta_dev_resume_noirq,
> +	.thaw = yenta_dev_resume,
>  	.poweroff_noirq = yenta_dev_suspend_noirq,
>  	.restore_noirq = yenta_dev_resume_noirq,
> +	.restore = yenta_dev_resume,
>  };
>  
>  #define YENTA_PM_OPS	(&yenta_pm_ops)
> Index: linux-2.6/drivers/pcmcia/cs.c
> ===================================================================
> --- linux-2.6.orig/drivers/pcmcia/cs.c
> +++ linux-2.6/drivers/pcmcia/cs.c
> @@ -98,10 +98,13 @@ EXPORT_SYMBOL(pcmcia_socket_list_rwsem);
>   * These functions check for the appropriate struct pcmcia_soket arrays,
>   * and pass them to the low-level functions pcmcia_{suspend,resume}_socket
>   */
> +static int socket_early_resume(struct pcmcia_socket *skt);
> +static int socket_late_resume(struct pcmcia_socket *skt);
>  static int socket_resume(struct pcmcia_socket *skt);
>  static int socket_suspend(struct pcmcia_socket *skt);
>  
> -int pcmcia_socket_dev_suspend(struct device *dev)
> +static void pcmcia_socket_dev_run(struct device *dev,
> +				  int (*cb)(struct pcmcia_socket *))
>  {
>  	struct pcmcia_socket *socket;
>  
> @@ -110,29 +113,34 @@ int pcmcia_socket_dev_suspend(struct dev
>  		if (socket->dev.parent != dev)
>  			continue;
>  		mutex_lock(&socket->skt_mutex);
> -		socket_suspend(socket);
> +		cb(socket);
>  		mutex_unlock(&socket->skt_mutex);
>  	}
>  	up_read(&pcmcia_socket_list_rwsem);
> +}
>  
> +int pcmcia_socket_dev_suspend(struct device *dev)
> +{
> +	pcmcia_socket_dev_run(dev, socket_suspend);
>  	return 0;
>  }
>  EXPORT_SYMBOL(pcmcia_socket_dev_suspend);
>  
> -int pcmcia_socket_dev_resume(struct device *dev)
> +void pcmcia_socket_dev_early_resume(struct device *dev)
>  {
> -	struct pcmcia_socket *socket;
> +	pcmcia_socket_dev_run(dev, socket_early_resume);
> +}
> +EXPORT_SYMBOL(pcmcia_socket_dev_early_resume);
>  
> -	down_read(&pcmcia_socket_list_rwsem);
> -	list_for_each_entry(socket, &pcmcia_socket_list, socket_list) {
> -		if (socket->dev.parent != dev)
> -			continue;
> -		mutex_lock(&socket->skt_mutex);
> -		socket_resume(socket);
> -		mutex_unlock(&socket->skt_mutex);
> -	}
> -	up_read(&pcmcia_socket_list_rwsem);
> +void pcmcia_socket_dev_late_resume(struct device *dev)
> +{
> +	pcmcia_socket_dev_run(dev, socket_late_resume);
> +}
> +EXPORT_SYMBOL(pcmcia_socket_dev_late_resume);
>  
> +int pcmcia_socket_dev_resume(struct device *dev)
> +{
> +	pcmcia_socket_dev_run(dev, socket_resume);
>  	return 0;
>  }
>  EXPORT_SYMBOL(pcmcia_socket_dev_resume);
> @@ -546,29 +554,34 @@ static int socket_suspend(struct pcmcia_
>  	return 0;
>  }
>  
> -/*
> - * Resume a socket.  If a card is present, verify its CIS against
> - * our cached copy.  If they are different, the card has been
> - * replaced, and we need to tell the drivers.
> - */
> -static int socket_resume(struct pcmcia_socket *skt)
> +static void socket_start_resume(struct pcmcia_socket *skt)
>  {
> -	int ret;
> -
> -	if (!(skt->state & SOCKET_SUSPEND))
> -		return -EBUSY;
> -
>  	skt->socket = dead_socket;
>  	skt->ops->init(skt);
>  	skt->ops->set_socket(skt, &skt->socket);
> +	if (skt->state & SOCKET_PRESENT)
> +		skt->resume_status = socket_setup(skt, resume_delay);
> +}
> +
> +static int socket_early_resume(struct pcmcia_socket *skt)
> +{
> +	if (skt->state & SOCKET_SUSPEND)
> +		socket_start_resume(skt);
> +
> +	return 0;
> +}
> +
> +static int socket_late_resume(struct pcmcia_socket *skt)
> +{
> +	if (!(skt->state & SOCKET_SUSPEND))
> +		return 0;
>  
>  	if (!(skt->state & SOCKET_PRESENT)) {
>  		skt->state &= ~SOCKET_SUSPEND;
>  		return socket_insert(skt);
>  	}
>  
> -	ret = socket_setup(skt, resume_delay);
> -	if (ret == 0) {
> +	if (skt->resume_status == 0) {
>  		/*
>  		 * FIXME: need a better check here for cardbus cards.
>  		 */
> @@ -596,6 +609,20 @@ static int socket_resume(struct pcmcia_s
>  	return 0;
>  }
>  
> +/*
> + * Resume a socket.  If a card is present, verify its CIS against
> + * our cached copy.  If they are different, the card has been
> + * replaced, and we need to tell the drivers.
> + */
> +static int socket_resume(struct pcmcia_socket *skt)
> +{
> +	if (!(skt->state & SOCKET_SUSPEND))
> +		return -EBUSY;
> +
> +	socket_start_resume(skt);
> +	return socket_late_resume(skt);
> +}
> +
>  static void socket_remove(struct pcmcia_socket *skt)
>  {
>  	dev_printk(KERN_NOTICE, &skt->dev,
> Index: linux-2.6/include/pcmcia/ss.h
> ===================================================================
> --- linux-2.6.orig/include/pcmcia/ss.h
> +++ linux-2.6/include/pcmcia/ss.h
> @@ -262,6 +262,8 @@ struct pcmcia_socket {
>  	struct device			dev;
>  	/* data internal to the socket driver */
>  	void				*driver_data;
> +	/* status of the card during resume from a system sleep state */
> +	int				resume_status;
>  };
>  
>  
> @@ -280,6 +282,8 @@ extern struct pccard_resource_ops pccard
>  
>  /* socket drivers are expected to use these callbacks in their .drv struct */
>  extern int pcmcia_socket_dev_suspend(struct device *dev);
> +extern void pcmcia_socket_dev_early_resume(struct device *dev);
> +extern void pcmcia_socket_dev_late_resume(struct device *dev);
>  extern int pcmcia_socket_dev_resume(struct device *dev);
>  
>  /* socket drivers use this callback in their IRQ handler */