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

> 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 */
--
To unsubscribe from this list: send the line "unsubscribe kernel-testers" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to