if_spi: howto implement suspend/resume?

Dan Williams dcbw at redhat.com
Thu Apr 23 06:55:16 EDT 2009


On Thu, 2009-04-23 at 00:49 -0700, Colin McCabe wrote:
> There is a way to put the WLAN module itself into sleep mode by
> talking to the SPU.
> You have to know which SPU registers to poke, and how (check out
> IF_SPI_CIC_POWER_DOWN and IF_SPI_HICT_WAKE_UP).
> 
> The module can wake up when various events happen. This is not
> implemented yet for G-SPI, but it could be.

That's the Libertas' "deep sleep" mode, right?

> If your board just cuts the power to the WLAN module, then I think
> your only option for resume is to redo the initial chip configuration
> in if_spi_probe. Perhaps this could be moved into a separate function.
> Right now, Linux initialization (initialize semaphores, kzalloc, etc.)
> are combined with the hardware initialization (loading firmware,
> setting interrupt modes, etc.)-- but they don't have to be.
> 
> It takes a while for the firmware to load. Some WLAN modules give you
> the option of preloading the firmware into EEPROM. If your board
> allows you to set the appropriate configuration strapping resistor,
> this would allow you to avoid reloading the firmware over the slow SPI
> bus.

I think what we should do here is to refactor both the lbs_suspend and
lbs_resume function to take a "mode" parameter:

typedef enum {
	LBS_SUSPEND_MODE_ALWAYS_ON = 0x00,   /* OLPC-style */
	LBS_SUSPEND_MODE_UNPOWERED = 0x01,
	LBS_SUSPEND_MODE_DEEP_SLEEP = 0x02
} lbs_suspend_mode;

With UNPOWERED, lbs_suspend() might just call lbs_stop_card (with the
exception of removing the debugfs stuff), and the interface code would
be responsible for disposing of its own resources (killing URBs, etc).
On resume, the interface code would be responsible for re-initializing
the card & bus, reloading the firmware, and then calling lbs_start_card.

DEEP_SLEEP would do some of the same stuff as UNPOWERED but of course
would send the sleep command, stall the command queue, etc.

The platform/interface code would be responsible for picking the right
sleep mode and figuring out whether the card was powered or unpowered
during sleep.

Those are my thoughts, anyway :)

Dan


> Colin
> 
> 
> On Wed, Apr 22, 2009 at 4:01 PM, Dan Williams <dcbw at redhat.com> wrote:
>         On Sun, 2009-04-19 at 09:59 +0300, Mike Rapoport wrote:
>         >
>         > Dan Williams wrote:
>         > > On Fri, 2009-04-17 at 18:24 +0200, Uli Luckas wrote:
>         > >> Hi all,
>         > >> I am trying to implement suspend/resume support for
>         libertas_spi as suspending
>         > >> a device with the libertas_spi driver loaded now crashes
>         the kernel.
>         > >> As I have no firmware documentation, I naively followed
>         the if_usb driver.
>         > >> As expected this did not work. With the patch below the
>         kernel does not crash
>         > >> any more but the libertas chip is non-functional after
>         resume:
>         > >> Apr 17 18:14:51 [kernel] [    0.000931] libertas: I/O
>         error
>         >
>         > [ snip ]
>         >
>         > >> Apr 17 18:14:51 [kernel] [    0.495033] libertas:
>         DNLD_CMD: hw_host_to_card
>         > >> failed: -22
>         > >>
>         > >> Can anybody sched some ligt on how to approach suspend
>         functionality in
>         > >> if_spi?
>         > >
>         > > Does SPI suspend cut power to the libertas chip?  Does it
>         do
>         > > bus-specific low-power stuff?  The current suspend code
>         assumes that the
>         > > libertas chip does not require complete reinitialization
>         (ie, new
>         > > firmware load, etc), which of course implies that the
>         libertas chip is
>         > > still powered across suspend so that when the host wakes
>         up, a command
>         > > can simply be sent to the card that will wake it up.
>         >
>         > I think that suspend behavior is machine dependent. In the
>         hardware I have the
>         > power to the libertas chip is cut during suspend, but in
>         other platforms the
>         > chip may be powered on.
>         
>         
>         Then yeah, suspend/resume here needs to be reworked to re-init
>         the chip
>         on resume.  The OLPC use-case is "special", and it's certainly
>         not the
>         norm.
>         
>         Dan
>         
>         
>         > > That is an artifact of the OLPC use-case, which isn't
>         necessarily
>         > > applicable elsewhere.
>         > >
>         > > Dan
>         > >
>         > >> Thanks
>         > >> Uli
>         > >>
>         > >>
>         > >> Index: drivers/net/wireless/libertas/if_spi.c
>         > >>
>         ===================================================================
>         > >> --- drivers/net/wireless/libertas/if_spi.c (revision
>         11391)
>         > >> +++ drivers/net/wireless/libertas/if_spi.c (working copy)
>         > >> @@ -31,6 +31,7 @@
>         > >>  #include "decl.h"
>         > >>  #include "defs.h"
>         > >>  #include "dev.h"
>         > >> +#include "cmd.h"
>         > >>  #include "if_spi.h"
>         > >>
>         > >>  struct if_spi_packet {
>         > >> @@ -1139,6 +1140,10 @@
>         > >>    if (err)
>         > >>            goto release_irq;
>         > >>
>         > >> +  priv->wol_gpio = 2; /* Wake via GPIO2... */
>         > >> +  priv->wol_gap = 20; /* ... after 20ms    */
>         > >> +  lbs_host_sleep_cfg(priv, EHS_WAKE_ON_UNICAST_DATA);
>         > >> +
>         > >>    lbs_deb_spi("Finished initializing WLAN module.\n");
>         > >>
>         > >>    /* successful exit */
>         > >> @@ -1183,9 +1188,49 @@
>         > >>    return 0;
>         > >>  }
>         > >>
>         > >> +#ifdef CONFIG_PM
>         > >> +static int if_spi_suspend(struct spi_device *spi,
>         pm_message_t mesg)
>         > >> +{
>         > >> +  struct if_spi_card *card = spi_get_drvdata(spi);
>         > >> +  struct lbs_private *priv = card->priv;
>         > >> +  int ret;
>         > >> +
>         > >> +  lbs_deb_enter(LBS_DEB_USB);
>         > >> +
>         > >> +  ret = lbs_suspend(priv);
>         > >> +  if (ret)
>         > >> +          goto out;
>         > >> +
>         > >> + out:
>         > >> +  lbs_deb_leave(LBS_DEB_USB);
>         > >> +  return ret;
>         > >> +}
>         > >> +
>         > >> +static int if_spi_resume(struct spi_device *spi)
>         > >> +{
>         > >> +  struct if_spi_card *card = spi_get_drvdata(spi);
>         > >> +  struct lbs_private *priv = card->priv;
>         > >> +
>         > >> +  lbs_deb_enter(LBS_DEB_USB);
>         > >> +
>         > >> +  up(&card->spi_ready);
>         > >> +
>         > >> +  lbs_resume(priv);
>         > >> +
>         > >> +  lbs_deb_leave(LBS_DEB_USB);
>         > >> +  return 0;
>         > >> +}
>         > >> +#else
>         > >> +#define if_spi_suspend NULL
>         > >> +#define if_spi_resume NULL
>         > >> +#endif
>         > >> +
>         > >> +
>         > >>  static struct spi_driver libertas_spi_driver = {
>         > >>    .probe  = if_spi_probe,
>         > >>    .remove = __devexit_p(libertas_spi_remove),
>         > >> +  .suspend = if_spi_suspend,
>         > >> +  .resume = if_spi_resume,
>         > >>    .driver = {
>         > >>            .name   = "libertas_spi",
>         > >>            .bus    = &spi_bus_type,
>         > >>
>         > >> --
>         > >>
>         > >> ------- ROAD ...the handyPC Company - - -  ) ) )
>         > >>
>         > >> Uli Luckas
>         > >> Head of Software Development
>         > >>
>         > >> ROAD GmbH
>         > >> Bennigsenstr. 14 | 12159 Berlin | Germany
>         > >> fon: +49 (30) 230069 - 62 | fax: +49 (30) 230069 - 69
>         > >> url: www.road.de
>         > >>
>         > >> Amtsgericht Charlottenburg: HRB 96688 B
>         > >> Managing director: Hans-Peter Constien
>         > >>
>         > >>
>         > >> _______________________________________________
>         > >> libertas-dev mailing list
>         > >> libertas-dev at lists.infradead.org
>         > >> http://lists.infradead.org/mailman/listinfo/libertas-dev
>         > >
>         > >
>         > > _______________________________________________
>         > > libertas-dev mailing list
>         > > libertas-dev at lists.infradead.org
>         > > http://lists.infradead.org/mailman/listinfo/libertas-dev
>         > >
>         >
>         
>         
>         _______________________________________________
>         libertas-dev mailing list
>         libertas-dev at lists.infradead.org
>         http://lists.infradead.org/mailman/listinfo/libertas-dev
>         
> 




More information about the libertas-dev mailing list