| Message ID | 1312280832-27675-1-git-send-email-anarsoul@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On 2 August 2011 11:27, Vasily Khoruzhick <anarsoul@gmail.com> wrote: > Could be usefull if it's not possible to keep power on the card > when host goes into suspend. If it's not possible to keep power during suspend, then I don't see why you need to explicitly turn it off on the way down. If power can be maintained, this is going to confuse userspace. If the network interface is up while you go into suspend, this code would kick in and power down the device. Then, during resume, it would power it up again. During this whole time, the network interface has remained UP, so userspace isn't aware that anything has happened. But, since the hardware has been powered off and on again, it has lost all state (association, encryption keys, etc). The way we handle this in libertas_sdio is that if we don't want the card powered during suspend, we ask the MMC layer to remove the device. This causes the remove function to be run, and the interface disappears. During resume, it gets probed again from scratch, which then prompts userspace into deciding which network to connect to, reprogramming keys, etc. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 02 August 2011 13:54:07 Daniel Drake wrote: > On 2 August 2011 11:27, Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > Could be usefull if it's not possible to keep power on the card > > when host goes into suspend. > > If it's not possible to keep power during suspend, then I don't see > why you need to explicitly turn it off on the way down. Because no one puts interface down before suspend? I just re-tested - if interface is up - kernel does not put it down before suspend, so I got a lot of timeouts and errors on dmesg after resume. Regards Vasily -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2 August 2011 21:09, Vasily Khoruzhick <anarsoul@gmail.com> wrote: > Because no one puts interface down before suspend? > I just re-tested - if interface is up - kernel does not put it down before > suspend, so I got a lot of timeouts and errors on dmesg after resume. I can't see how this would have been influenced by my work. Your resume handler should already be fixing up the hardware state so that it can be operated (regardless of any power saving that might come into effect), or otherwise your suspend handler should cause the device to be removed so that it can be reprobed completely during resume. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 02 August 2011 23:32:27 Daniel Drake wrote: > On 2 August 2011 21:09, Vasily Khoruzhick <anarsoul@gmail.com> wrote: > > Because no one puts interface down before suspend? > > I just re-tested - if interface is up - kernel does not put it down > > before suspend, so I got a lot of timeouts and errors on dmesg after > > resume. > > I can't see how this would have been influenced by my work. Now generic libertas driver controls power, not if_spi. So it should be able to turn off card before going suspend. Messing with power in if_* and generic part does not look like a good idea to me. > Your resume handler should already be fixing up the hardware state so > that it can be operated (regardless of any power saving that might > come into effect), or otherwise your suspend handler should cause the > device to be removed so that it can be reprobed completely during > resume. My resume handler calls lbs_suspend to detach device, and it expects that lbs_suspend will disable card power. > Daniel Regards Vasily -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2 August 2011 22:12, Vasily Khoruzhick <anarsoul@gmail.com> wrote: > Now generic libertas driver controls power, not if_spi. So it should be able > to turn off card before going suspend. Messing with power in if_* and generic > part does not look like a good idea to me. But there's no way your approach will bring good results. Userspace won't see any interface state change, but the entire hardware and firmware will be fully reset. Things will get very confused. A lot of the stuff done in stop_iface is not relevant for the suspend case. e.g. If commands were queued before suspend, they should execute after resume. stop_iface is designed to run when the userspace-visible interfaces are brought down - by running it while they are still up, you will confuse things. You are battling with a general suspend/resume problem which should be handled by suspend/resume handlers alone. libertas already has plenty of experience in this area which you can build upon. Daniel -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/wireless/libertas/dev.h b/drivers/net/wireless/libertas/dev.h index f526633..563a8b0 100644 --- a/drivers/net/wireless/libertas/dev.h +++ b/drivers/net/wireless/libertas/dev.h @@ -86,9 +86,10 @@ struct lbs_private { /* Hardware access */ void *card; bool iface_running; + bool suspend_iface_status; u8 fw_ready; u8 surpriseremoved; - u8 setup_fw_on_resume; + u8 disable_on_suspend; int (*hw_host_to_card) (struct lbs_private *priv, u8 type, u8 *payload, u16 nb); void (*reset_card) (struct lbs_private *priv); int (*power_save) (struct lbs_private *priv); diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c index 208e4d9..5522202 100644 --- a/drivers/net/wireless/libertas/main.c +++ b/drivers/net/wireless/libertas/main.c @@ -644,7 +644,7 @@ done: int lbs_suspend(struct lbs_private *priv) { - int ret; + int ret = 0; lbs_deb_enter(LBS_DEB_FW); @@ -658,7 +658,13 @@ int lbs_suspend(struct lbs_private *priv) priv->deep_sleep_required = 1; } - ret = lbs_set_host_sleep(priv, 1); + if (priv->disable_on_suspend) { + /* Disable card */ + priv->suspend_iface_status = priv->iface_running; + if (priv->iface_running) + lbs_stop_iface(priv); + } else + ret = lbs_set_host_sleep(priv, 1); netif_device_detach(priv->dev); if (priv->mesh_dev) @@ -671,11 +677,16 @@ EXPORT_SYMBOL_GPL(lbs_suspend); int lbs_resume(struct lbs_private *priv) { - int ret; + int ret = 0; lbs_deb_enter(LBS_DEB_FW); - ret = lbs_set_host_sleep(priv, 0); + if (priv->disable_on_suspend) { + /* Enable card */ + if (priv->suspend_iface_status) + lbs_start_iface(priv); + } else + ret = lbs_set_host_sleep(priv, 0); netif_device_attach(priv->dev); if (priv->mesh_dev) @@ -689,9 +700,6 @@ int lbs_resume(struct lbs_private *priv) "deep sleep activation failed: %d\n", ret); } - if (priv->setup_fw_on_resume) - ret = lbs_setup_firmware(priv); - lbs_deb_leave_args(LBS_DEB_FW, "ret %d", ret); return ret; }
Could be usefull if it's not possible to keep power on the card when host goes into suspend. Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com> --- drivers/net/wireless/libertas/dev.h | 3 ++- drivers/net/wireless/libertas/main.c | 22 +++++++++++++++------- 2 files changed, 17 insertions(+), 8 deletions(-)