2011-08-01 15:54:09

by Daniel Drake

[permalink] [raw]
Subject: [PATCH 1/2] libertas: disable functionality when interface is down

Modify the driver so that it does not function when the interface is
down, in preparation for runtime power management.

No commands can be run while the interface is down, so the ndo_dev_stop
routine now directly does all necessary work (including asking the device
to disconnect from the network and disabling multicast functionality)
directly.

power_save and power_restore hooks are added meaning that card drivers
can take steps to turn the device off when the interface is down.

The MAC address can now only be changed when all interfaces are down;
the new address will be programmed when an interface gets brought up.
This matches mac80211 behaviour.

Also, some small cleanups/simplifications were made in the surrounding
device handling logic.

Signed-off-by: Daniel Drake <[email protected]>
---
drivers/net/wireless/libertas/cfg.c | 39 +++++---
drivers/net/wireless/libertas/cfg.h | 1 +
drivers/net/wireless/libertas/cmd.c | 6 +-
drivers/net/wireless/libertas/decl.h | 4 +
drivers/net/wireless/libertas/dev.h | 16 +++-
drivers/net/wireless/libertas/if_usb.c | 2 +-
drivers/net/wireless/libertas/main.c | 168 ++++++++++++++++++++-----------
drivers/net/wireless/libertas/mesh.c | 9 ++-
8 files changed, 163 insertions(+), 82 deletions(-)

diff --git a/drivers/net/wireless/libertas/cfg.c b/drivers/net/wireless/libertas/cfg.c
index 63009c7..85b3169 100644
--- a/drivers/net/wireless/libertas/cfg.c
+++ b/drivers/net/wireless/libertas/cfg.c
@@ -712,7 +712,7 @@ static void lbs_scan_worker(struct work_struct *work)

if (priv->scan_channel < priv->scan_req->n_channels) {
cancel_delayed_work(&priv->scan_work);
- if (!priv->stopping)
+ if (netif_running(priv->dev))
queue_delayed_work(priv->work_thread, &priv->scan_work,
msecs_to_jiffies(300));
}
@@ -1409,31 +1409,23 @@ static int lbs_cfg_connect(struct wiphy *wiphy, struct net_device *dev,
return ret;
}

-static int lbs_cfg_disconnect(struct wiphy *wiphy, struct net_device *dev,
- u16 reason_code)
+int lbs_disconnect(struct lbs_private *priv, u16 reason)
{
- struct lbs_private *priv = wiphy_priv(wiphy);
struct cmd_ds_802_11_deauthenticate cmd;
-
- if (dev == priv->mesh_dev)
- return -EOPNOTSUPP;
-
- lbs_deb_enter_args(LBS_DEB_CFG80211, "reason_code %d", reason_code);
-
- /* store for lbs_cfg_ret_disconnect() */
- priv->disassoc_reason = reason_code;
+ int ret;

memset(&cmd, 0, sizeof(cmd));
cmd.hdr.size = cpu_to_le16(sizeof(cmd));
/* Mildly ugly to use a locally store my own BSSID ... */
memcpy(cmd.macaddr, &priv->assoc_bss, ETH_ALEN);
- cmd.reasoncode = cpu_to_le16(reason_code);
+ cmd.reasoncode = cpu_to_le16(reason);

- if (lbs_cmd_with_response(priv, CMD_802_11_DEAUTHENTICATE, &cmd))
- return -EFAULT;
+ ret = lbs_cmd_with_response(priv, CMD_802_11_DEAUTHENTICATE, &cmd);
+ if (ret)
+ return ret;

cfg80211_disconnected(priv->dev,
- priv->disassoc_reason,
+ reason,
NULL, 0,
GFP_KERNEL);
priv->connect_status = LBS_DISCONNECTED;
@@ -1441,6 +1433,21 @@ static int lbs_cfg_disconnect(struct wiphy *wiphy, struct net_device *dev,
return 0;
}

+static int lbs_cfg_disconnect(struct wiphy *wiphy, struct net_device *dev,
+ u16 reason_code)
+{
+ struct lbs_private *priv = wiphy_priv(wiphy);
+
+ if (dev == priv->mesh_dev)
+ return -EOPNOTSUPP;
+
+ lbs_deb_enter_args(LBS_DEB_CFG80211, "reason_code %d", reason_code);
+
+ /* store for lbs_cfg_ret_disconnect() */
+ priv->disassoc_reason = reason_code;
+
+ return lbs_disconnect(priv, reason_code);
+}

static int lbs_cfg_set_default_key(struct wiphy *wiphy,
struct net_device *netdev,
diff --git a/drivers/net/wireless/libertas/cfg.h b/drivers/net/wireless/libertas/cfg.h
index 4f46bb7..a02ee15 100644
--- a/drivers/net/wireless/libertas/cfg.h
+++ b/drivers/net/wireless/libertas/cfg.h
@@ -17,5 +17,6 @@ void lbs_send_disconnect_notification(struct lbs_private *priv);
void lbs_send_mic_failureevent(struct lbs_private *priv, u32 event);

void lbs_scan_deinit(struct lbs_private *priv);
+int lbs_disconnect(struct lbs_private *priv, u16 reason);

#endif
diff --git a/drivers/net/wireless/libertas/cmd.c b/drivers/net/wireless/libertas/cmd.c
index dbd24a4..e08ab1d 100644
--- a/drivers/net/wireless/libertas/cmd.c
+++ b/drivers/net/wireless/libertas/cmd.c
@@ -1088,7 +1088,7 @@ void __lbs_complete_command(struct lbs_private *priv, struct cmd_ctrl_node *cmd,
if (!cmd->callback || cmd->callback == lbs_cmd_async_callback)
__lbs_cleanup_and_insert_cmd(priv, cmd);
priv->cur_cmd = NULL;
- wake_up_interruptible(&priv->waitq);
+ wake_up(&priv->waitq);
}

void lbs_complete_command(struct lbs_private *priv, struct cmd_ctrl_node *cmd,
@@ -1627,7 +1627,7 @@ struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv,
lbs_deb_host("PREP_CMD: cmdnode is NULL\n");

/* Wake up main thread to execute next command */
- wake_up_interruptible(&priv->waitq);
+ wake_up(&priv->waitq);
cmdnode = ERR_PTR(-ENOBUFS);
goto done;
}
@@ -1647,7 +1647,7 @@ struct cmd_ctrl_node *__lbs_cmd_async(struct lbs_private *priv,

cmdnode->cmdwaitqwoken = 0;
lbs_queue_cmd(priv, cmdnode);
- wake_up_interruptible(&priv->waitq);
+ wake_up(&priv->waitq);

done:
lbs_deb_leave_args(LBS_DEB_HOST, "ret %p", cmdnode);
diff --git a/drivers/net/wireless/libertas/decl.h b/drivers/net/wireless/libertas/decl.h
index da0b05b..9304e6f 100644
--- a/drivers/net/wireless/libertas/decl.h
+++ b/drivers/net/wireless/libertas/decl.h
@@ -43,10 +43,14 @@ int lbs_start_card(struct lbs_private *priv);
void lbs_stop_card(struct lbs_private *priv);
void lbs_host_to_card_done(struct lbs_private *priv);

+int lbs_start_iface(struct lbs_private *priv);
+int lbs_stop_iface(struct lbs_private *priv);
+
int lbs_rtap_supported(struct lbs_private *priv);

int lbs_set_mac_address(struct net_device *dev, void *addr);
void lbs_set_multicast_list(struct net_device *dev);
+void lbs_update_mcast(struct lbs_private *priv);

int lbs_suspend(struct lbs_private *priv);
int lbs_resume(struct lbs_private *priv);
diff --git a/drivers/net/wireless/libertas/dev.h b/drivers/net/wireless/libertas/dev.h
index 133ff1c..8148389 100644
--- a/drivers/net/wireless/libertas/dev.h
+++ b/drivers/net/wireless/libertas/dev.h
@@ -46,7 +46,6 @@ struct lbs_private {
/* CFG80211 */
struct wireless_dev *wdev;
bool wiphy_registered;
- bool stopping;
struct cfg80211_scan_request *scan_req;
u8 assoc_bss[ETH_ALEN];
u8 disassoc_reason;
@@ -96,11 +95,14 @@ struct lbs_private {

/* Hardware access */
void *card;
+ bool iface_running;
u8 fw_ready;
u8 surpriseremoved;
u8 setup_fw_on_resume;
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);
+ int (*power_restore) (struct lbs_private *priv);
int (*enter_deep_sleep) (struct lbs_private *priv);
int (*exit_deep_sleep) (struct lbs_private *priv);
int (*reset_deep_sleep_wakeup) (struct lbs_private *priv);
@@ -182,4 +184,16 @@ struct lbs_private {

extern struct cmd_confirm_sleep confirm_sleep;

+/* Check if there is an interface active. */
+static inline int lbs_iface_active(struct lbs_private *priv)
+{
+ int r;
+
+ r = netif_running(priv->dev);
+ if (priv->mesh_dev);
+ r |= netif_running(priv->dev);
+
+ return r;
+}
+
#endif
diff --git a/drivers/net/wireless/libertas/if_usb.c b/drivers/net/wireless/libertas/if_usb.c
index e368b29..0775f45 100644
--- a/drivers/net/wireless/libertas/if_usb.c
+++ b/drivers/net/wireless/libertas/if_usb.c
@@ -956,7 +956,7 @@ static int if_usb_prog_firmware(struct if_usb_card *cardp,
priv->dnld_sent = DNLD_RES_RECEIVED;
spin_unlock_irqrestore(&priv->driver_lock, flags);

- wake_up_interruptible(&priv->waitq);
+ wake_up(&priv->waitq);

return ret;
}
diff --git a/drivers/net/wireless/libertas/main.c b/drivers/net/wireless/libertas/main.c
index ee28ae5..d62d1fb 100644
--- a/drivers/net/wireless/libertas/main.c
+++ b/drivers/net/wireless/libertas/main.c
@@ -99,6 +99,37 @@ u8 lbs_data_rate_to_fw_index(u32 rate)
return 0;
}

+int lbs_start_iface(struct lbs_private *priv)
+{
+ struct cmd_ds_802_11_mac_address cmd;
+ int ret;
+
+ if (priv->power_restore) {
+ ret = priv->power_restore(priv);
+ if (ret)
+ return ret;
+ }
+
+ cmd.hdr.size = cpu_to_le16(sizeof(cmd));
+ cmd.action = cpu_to_le16(CMD_ACT_SET);
+ memcpy(cmd.macadd, priv->current_addr, ETH_ALEN);
+
+ ret = lbs_cmd_with_response(priv, CMD_802_11_MAC_ADDRESS, &cmd);
+ if (ret) {
+ lbs_deb_net("set MAC address failed\n");
+ goto err;
+ }
+
+ lbs_update_channel(priv);
+
+ priv->iface_running = true;
+ return 0;
+
+err:
+ if (priv->power_save)
+ priv->power_save(priv);
+ return ret;
+}

/**
* lbs_dev_open - open the ethX interface
@@ -112,23 +143,64 @@ static int lbs_dev_open(struct net_device *dev)
int ret = 0;

lbs_deb_enter(LBS_DEB_NET);
+ if (!priv->iface_running) {
+ ret = lbs_start_iface(priv);
+ if (ret)
+ goto out;
+ }

spin_lock_irq(&priv->driver_lock);
- priv->stopping = false;

- if (priv->connect_status == LBS_CONNECTED)
- netif_carrier_on(dev);
- else
- netif_carrier_off(dev);
+ netif_carrier_off(dev);

if (!priv->tx_pending_len)
netif_wake_queue(dev);

spin_unlock_irq(&priv->driver_lock);
+
+out:
lbs_deb_leave_args(LBS_DEB_NET, "ret %d", ret);
return ret;
}

+static bool lbs_command_queue_empty(struct lbs_private *priv)
+{
+ unsigned long flags;
+ bool ret;
+ spin_lock_irqsave(&priv->driver_lock, flags);
+ ret = priv->cur_cmd == NULL && list_empty(&priv->cmdpendingq);
+ spin_unlock_irqrestore(&priv->driver_lock, flags);
+ return ret;
+}
+
+int lbs_stop_iface(struct lbs_private *priv)
+{
+ unsigned long flags;
+ int ret = 0;
+
+ lbs_deb_enter(LBS_DEB_MAIN);
+
+ spin_lock_irqsave(&priv->driver_lock, flags);
+ priv->iface_running = false;
+ kfree_skb(priv->currenttxskb);
+ priv->currenttxskb = NULL;
+ priv->tx_pending_len = 0;
+ spin_unlock_irqrestore(&priv->driver_lock, flags);
+
+ cancel_work_sync(&priv->mcast_work);
+
+ /* Disable command processing, and wait for all commands to complete */
+ lbs_deb_main("waiting for commands to complete\n");
+ wait_event(priv->waitq, lbs_command_queue_empty(priv));
+ lbs_deb_main("all commands completed\n");
+
+ if (priv->power_save)
+ ret = priv->power_save(priv);
+
+ lbs_deb_leave(LBS_DEB_MAIN);
+ return ret;
+}
+
/**
* lbs_eth_stop - close the ethX interface
*
@@ -141,18 +213,25 @@ static int lbs_eth_stop(struct net_device *dev)

lbs_deb_enter(LBS_DEB_NET);

+ if (priv->connect_status == LBS_CONNECTED)
+ lbs_disconnect(priv, WLAN_REASON_DEAUTH_LEAVING);
+
spin_lock_irq(&priv->driver_lock);
- priv->stopping = true;
netif_stop_queue(dev);
spin_unlock_irq(&priv->driver_lock);

- schedule_work(&priv->mcast_work);
+ lbs_update_mcast(priv);
cancel_delayed_work_sync(&priv->scan_work);
if (priv->scan_req) {
cfg80211_scan_done(priv->scan_req, false);
priv->scan_req = NULL;
}

+ netif_carrier_off(priv->dev);
+
+ if (!lbs_iface_active(priv))
+ lbs_stop_iface(priv);
+
lbs_deb_leave(LBS_DEB_NET);
return 0;
}
@@ -170,7 +249,7 @@ void lbs_host_to_card_done(struct lbs_private *priv)
/* Wake main thread if commands are pending */
if (!priv->cur_cmd || priv->tx_pending_len > 0) {
if (!priv->wakeup_dev_required)
- wake_up_interruptible(&priv->waitq);
+ wake_up(&priv->waitq);
}

spin_unlock_irqrestore(&priv->driver_lock, flags);
@@ -183,29 +262,24 @@ int lbs_set_mac_address(struct net_device *dev, void *addr)
int ret = 0;
struct lbs_private *priv = dev->ml_priv;
struct sockaddr *phwaddr = addr;
- struct cmd_ds_802_11_mac_address cmd;

lbs_deb_enter(LBS_DEB_NET);

+ /*
+ * Can only set MAC address when all interfaces are down, to be written
+ * to the hardware when one of them is brought up.
+ */
+ if (lbs_iface_active(priv))
+ return -EBUSY;
+
/* In case it was called from the mesh device */
dev = priv->dev;

- cmd.hdr.size = cpu_to_le16(sizeof(cmd));
- cmd.action = cpu_to_le16(CMD_ACT_SET);
- memcpy(cmd.macadd, phwaddr->sa_data, ETH_ALEN);
-
- ret = lbs_cmd_with_response(priv, CMD_802_11_MAC_ADDRESS, &cmd);
- if (ret) {
- lbs_deb_net("set MAC address failed\n");
- goto done;
- }
-
memcpy(priv->current_addr, phwaddr->sa_data, ETH_ALEN);
memcpy(dev->dev_addr, phwaddr->sa_data, ETH_ALEN);
if (priv->mesh_dev)
memcpy(priv->mesh_dev->dev_addr, phwaddr->sa_data, ETH_ALEN);

-done:
lbs_deb_leave_args(LBS_DEB_NET, "ret %d", ret);
return ret;
}
@@ -259,18 +333,18 @@ static int lbs_add_mcast_addrs(struct cmd_ds_mac_multicast_adr *cmd,
return i;
}

-static void lbs_set_mcast_worker(struct work_struct *work)
+void lbs_update_mcast(struct lbs_private *priv)
{
- struct lbs_private *priv = container_of(work, struct lbs_private, mcast_work);
struct cmd_ds_mac_multicast_adr mcast_cmd;
- int dev_flags;
+ int dev_flags = 0;
int nr_addrs;
int old_mac_control = priv->mac_control;

lbs_deb_enter(LBS_DEB_NET);

- dev_flags = priv->dev->flags;
- if (priv->mesh_dev)
+ if (netif_running(priv->dev))
+ dev_flags |= priv->dev->flags;
+ if (priv->mesh_dev && netif_running(priv->mesh_dev))
dev_flags |= priv->mesh_dev->flags;

if (dev_flags & IFF_PROMISC) {
@@ -316,6 +390,12 @@ static void lbs_set_mcast_worker(struct work_struct *work)
lbs_deb_leave(LBS_DEB_NET);
}

+static void lbs_set_mcast_worker(struct work_struct *work)
+{
+ struct lbs_private *priv = container_of(work, struct lbs_private, mcast_work);
+ lbs_update_mcast(priv);
+}
+
void lbs_set_multicast_list(struct net_device *dev)
{
struct lbs_private *priv = dev->ml_priv;
@@ -648,7 +728,7 @@ static void lbs_cmd_timeout_handler(unsigned long data)
if (priv->dnld_sent == DNLD_CMD_SENT)
priv->dnld_sent = DNLD_RES_RECEIVED;

- wake_up_interruptible(&priv->waitq);
+ wake_up(&priv->waitq);
out:
spin_unlock_irqrestore(&priv->driver_lock, flags);
lbs_deb_leave(LBS_DEB_CMD);
@@ -890,10 +970,6 @@ void lbs_remove_card(struct lbs_private *priv)
lbs_remove_mesh(priv);
lbs_scan_deinit(priv);

- dev = priv->dev;
-
- cancel_work_sync(&priv->mcast_work);
-
/* worker thread destruction blocks on the in-flight command which
* should have been cleared already in lbs_stop_card().
*/
@@ -964,8 +1040,6 @@ int lbs_start_card(struct lbs_private *priv)
if (lbs_mesh_activated(priv))
lbs_start_mesh(priv);

- lbs_update_channel(priv);
-
lbs_debugfs_init_one(priv, dev);

netdev_info(dev, "Marvell WLAN 802.11 adapter\n");
@@ -982,8 +1056,6 @@ EXPORT_SYMBOL_GPL(lbs_start_card);
void lbs_stop_card(struct lbs_private *priv)
{
struct net_device *dev;
- struct cmd_ctrl_node *cmdnode;
- unsigned long flags;

lbs_deb_enter(LBS_DEB_MAIN);

@@ -996,30 +1068,6 @@ void lbs_stop_card(struct lbs_private *priv)

lbs_debugfs_remove_one(priv);
lbs_deinit_mesh(priv);
-
- /* Delete the timeout of the currently processing command */
- del_timer_sync(&priv->command_timer);
- del_timer_sync(&priv->auto_deepsleep_timer);
-
- /* Flush pending command nodes */
- spin_lock_irqsave(&priv->driver_lock, flags);
- lbs_deb_main("clearing pending commands\n");
- list_for_each_entry(cmdnode, &priv->cmdpendingq, list) {
- cmdnode->result = -ENOENT;
- cmdnode->cmdwaitqwoken = 1;
- wake_up(&cmdnode->cmdwait_q);
- }
-
- /* Flush the command the card is currently processing */
- if (priv->cur_cmd) {
- lbs_deb_main("clearing current command\n");
- priv->cur_cmd->result = -ENOENT;
- priv->cur_cmd->cmdwaitqwoken = 1;
- wake_up(&priv->cur_cmd->cmdwait_q);
- }
- lbs_deb_main("done clearing commands\n");
- spin_unlock_irqrestore(&priv->driver_lock, flags);
-
unregister_netdev(dev);

out:
@@ -1040,7 +1088,7 @@ void lbs_queue_event(struct lbs_private *priv, u32 event)

kfifo_in(&priv->event_fifo, (unsigned char *) &event, sizeof(u32));

- wake_up_interruptible(&priv->waitq);
+ wake_up(&priv->waitq);

spin_unlock_irqrestore(&priv->driver_lock, flags);
lbs_deb_leave(LBS_DEB_THREAD);
@@ -1058,7 +1106,7 @@ void lbs_notify_command_response(struct lbs_private *priv, u8 resp_idx)
BUG_ON(resp_idx > 1);
priv->resp_idx = resp_idx;

- wake_up_interruptible(&priv->waitq);
+ wake_up(&priv->waitq);

lbs_deb_leave(LBS_DEB_THREAD);
}
diff --git a/drivers/net/wireless/libertas/mesh.c b/drivers/net/wireless/libertas/mesh.c
index 2a635d2..138699b 100644
--- a/drivers/net/wireless/libertas/mesh.c
+++ b/drivers/net/wireless/libertas/mesh.c
@@ -924,7 +924,9 @@ static int lbs_mesh_stop(struct net_device *dev)

spin_unlock_irq(&priv->driver_lock);

- schedule_work(&priv->mcast_work);
+ lbs_update_mcast(priv);
+ if (!lbs_iface_active(priv))
+ lbs_stop_iface(priv);

lbs_deb_leave(LBS_DEB_MESH);
return 0;
@@ -942,6 +944,11 @@ static int lbs_mesh_dev_open(struct net_device *dev)
int ret = 0;

lbs_deb_enter(LBS_DEB_NET);
+ if (!priv->iface_running) {
+ ret = lbs_start_iface(priv);
+ if (ret)
+ goto out;
+ }

spin_lock_irq(&priv->driver_lock);

--
1.7.6



2011-08-02 20:09:40

by Vasily Khoruzhick

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: add ability to power off card on suspend

On Tuesday 02 August 2011 13:54:07 Daniel Drake wrote:
> On 2 August 2011 11:27, Vasily Khoruzhick <[email protected]> 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

2011-08-02 09:02:41

by Vasily Khoruzhick

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: disable functionality when interface is down

On Monday 01 August 2011 18:43:13 Daniel Drake wrote:
> Modify the driver so that it does not function when the interface is
> down, in preparation for runtime power management.
>
> No commands can be run while the interface is down, so the ndo_dev_stop
> routine now directly does all necessary work (including asking the device
> to disconnect from the network and disabling multicast functionality)
> directly.
>
> power_save and power_restore hooks are added meaning that card drivers
> can take steps to turn the device off when the interface is down.
>
> The MAC address can now only be changed when all interfaces are down;
> the new address will be programmed when an interface gets brought up.
> This matches mac80211 behaviour.
>
> Also, some small cleanups/simplifications were made in the surrounding
> device handling logic.

Ok, one more question: what about suspend? Maybe it worth to add flag to
disable card before going suspend and re-enable it during resume?

> Signed-off-by: Daniel Drake <[email protected]>
> ---
> drivers/net/wireless/libertas/cfg.c | 39 +++++---
> drivers/net/wireless/libertas/cfg.h | 1 +
> drivers/net/wireless/libertas/cmd.c | 6 +-
> drivers/net/wireless/libertas/decl.h | 4 +
> drivers/net/wireless/libertas/dev.h | 16 +++-
> drivers/net/wireless/libertas/if_usb.c | 2 +-
> drivers/net/wireless/libertas/main.c | 168
> ++++++++++++++++++++----------- drivers/net/wireless/libertas/mesh.c |
> 9 ++-
> 8 files changed, 163 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/net/wireless/libertas/cfg.c
> b/drivers/net/wireless/libertas/cfg.c index 63009c7..85b3169 100644
> --- a/drivers/net/wireless/libertas/cfg.c
> +++ b/drivers/net/wireless/libertas/cfg.c
> @@ -712,7 +712,7 @@ static void lbs_scan_worker(struct work_struct *work)
>
> if (priv->scan_channel < priv->scan_req->n_channels) {
> cancel_delayed_work(&priv->scan_work);
> - if (!priv->stopping)
> + if (netif_running(priv->dev))
> queue_delayed_work(priv->work_thread, &priv->scan_work,
> msecs_to_jiffies(300));
> }
> @@ -1409,31 +1409,23 @@ static int lbs_cfg_connect(struct wiphy *wiphy,
> struct net_device *dev, return ret;
> }
>
> -static int lbs_cfg_disconnect(struct wiphy *wiphy, struct net_device *dev,
> - u16 reason_code)
> +int lbs_disconnect(struct lbs_private *priv, u16 reason)
> {
> - struct lbs_private *priv = wiphy_priv(wiphy);
> struct cmd_ds_802_11_deauthenticate cmd;
> -
> - if (dev == priv->mesh_dev)
> - return -EOPNOTSUPP;
> -
> - lbs_deb_enter_args(LBS_DEB_CFG80211, "reason_code %d", reason_code);
> -
> - /* store for lbs_cfg_ret_disconnect() */
> - priv->disassoc_reason = reason_code;
> + int ret;
>
> memset(&cmd, 0, sizeof(cmd));
> cmd.hdr.size = cpu_to_le16(sizeof(cmd));
> /* Mildly ugly to use a locally store my own BSSID ... */
> memcpy(cmd.macaddr, &priv->assoc_bss, ETH_ALEN);
> - cmd.reasoncode = cpu_to_le16(reason_code);
> + cmd.reasoncode = cpu_to_le16(reason);
>
> - if (lbs_cmd_with_response(priv, CMD_802_11_DEAUTHENTICATE, &cmd))
> - return -EFAULT;
> + ret = lbs_cmd_with_response(priv, CMD_802_11_DEAUTHENTICATE, &cmd);
> + if (ret)
> + return ret;
>
> cfg80211_disconnected(priv->dev,
> - priv->disassoc_reason,
> + reason,
> NULL, 0,
> GFP_KERNEL);
> priv->connect_status = LBS_DISCONNECTED;
> @@ -1441,6 +1433,21 @@ static int lbs_cfg_disconnect(struct wiphy *wiphy,
> struct net_device *dev, return 0;
> }
>
> +static int lbs_cfg_disconnect(struct wiphy *wiphy, struct net_device *dev,
> + u16 reason_code)
> +{
> + struct lbs_private *priv = wiphy_priv(wiphy);
> +
> + if (dev == priv->mesh_dev)
> + return -EOPNOTSUPP;
> +
> + lbs_deb_enter_args(LBS_DEB_CFG80211, "reason_code %d", reason_code);
> +
> + /* store for lbs_cfg_ret_disconnect() */
> + priv->disassoc_reason = reason_code;
> +
> + return lbs_disconnect(priv, reason_code);
> +}
>
> static int lbs_cfg_set_default_key(struct wiphy *wiphy,
> struct net_device *netdev,
> diff --git a/drivers/net/wireless/libertas/cfg.h
> b/drivers/net/wireless/libertas/cfg.h index 4f46bb7..a02ee15 100644
> --- a/drivers/net/wireless/libertas/cfg.h
> +++ b/drivers/net/wireless/libertas/cfg.h
> @@ -17,5 +17,6 @@ void lbs_send_disconnect_notification(struct lbs_private
> *priv); void lbs_send_mic_failureevent(struct lbs_private *priv, u32
> event);
>
> void lbs_scan_deinit(struct lbs_private *priv);
> +int lbs_disconnect(struct lbs_private *priv, u16 reason);
>
> #endif
> diff --git a/drivers/net/wireless/libertas/cmd.c
> b/drivers/net/wireless/libertas/cmd.c index dbd24a4..e08ab1d 100644
> --- a/drivers/net/wireless/libertas/cmd.c
> +++ b/drivers/net/wireless/libertas/cmd.c
> @@ -1088,7 +1088,7 @@ void __lbs_complete_command(struct lbs_private *priv,
> struct cmd_ctrl_node *cmd, if (!cmd->callback || cmd->callback ==
> lbs_cmd_async_callback)
> __lbs_cleanup_and_insert_cmd(priv, cmd);
> priv->cur_cmd = NULL;
> - wake_up_interruptible(&priv->waitq);
> + wake_up(&priv->waitq);
> }
>
> void lbs_complete_command(struct lbs_private *priv, struct cmd_ctrl_node
> *cmd, @@ -1627,7 +1627,7 @@ struct cmd_ctrl_node *__lbs_cmd_async(struct
> lbs_private *priv, lbs_deb_host("PREP_CMD: cmdnode is NULL\n");
>
> /* Wake up main thread to execute next command */
> - wake_up_interruptible(&priv->waitq);
> + wake_up(&priv->waitq);
> cmdnode = ERR_PTR(-ENOBUFS);
> goto done;
> }
> @@ -1647,7 +1647,7 @@ struct cmd_ctrl_node *__lbs_cmd_async(struct
> lbs_private *priv,
>
> cmdnode->cmdwaitqwoken = 0;
> lbs_queue_cmd(priv, cmdnode);
> - wake_up_interruptible(&priv->waitq);
> + wake_up(&priv->waitq);
>
> done:
> lbs_deb_leave_args(LBS_DEB_HOST, "ret %p", cmdnode);
> diff --git a/drivers/net/wireless/libertas/decl.h
> b/drivers/net/wireless/libertas/decl.h index da0b05b..9304e6f 100644
> --- a/drivers/net/wireless/libertas/decl.h
> +++ b/drivers/net/wireless/libertas/decl.h
> @@ -43,10 +43,14 @@ int lbs_start_card(struct lbs_private *priv);
> void lbs_stop_card(struct lbs_private *priv);
> void lbs_host_to_card_done(struct lbs_private *priv);
>
> +int lbs_start_iface(struct lbs_private *priv);
> +int lbs_stop_iface(struct lbs_private *priv);
> +
> int lbs_rtap_supported(struct lbs_private *priv);
>
> int lbs_set_mac_address(struct net_device *dev, void *addr);
> void lbs_set_multicast_list(struct net_device *dev);
> +void lbs_update_mcast(struct lbs_private *priv);
>
> int lbs_suspend(struct lbs_private *priv);
> int lbs_resume(struct lbs_private *priv);
> diff --git a/drivers/net/wireless/libertas/dev.h
> b/drivers/net/wireless/libertas/dev.h index 133ff1c..8148389 100644
> --- a/drivers/net/wireless/libertas/dev.h
> +++ b/drivers/net/wireless/libertas/dev.h
> @@ -46,7 +46,6 @@ struct lbs_private {
> /* CFG80211 */
> struct wireless_dev *wdev;
> bool wiphy_registered;
> - bool stopping;
> struct cfg80211_scan_request *scan_req;
> u8 assoc_bss[ETH_ALEN];
> u8 disassoc_reason;
> @@ -96,11 +95,14 @@ struct lbs_private {
>
> /* Hardware access */
> void *card;
> + bool iface_running;
> u8 fw_ready;
> u8 surpriseremoved;
> u8 setup_fw_on_resume;
> 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);
> + int (*power_restore) (struct lbs_private *priv);
> int (*enter_deep_sleep) (struct lbs_private *priv);
> int (*exit_deep_sleep) (struct lbs_private *priv);
> int (*reset_deep_sleep_wakeup) (struct lbs_private *priv);
> @@ -182,4 +184,16 @@ struct lbs_private {
>
> extern struct cmd_confirm_sleep confirm_sleep;
>
> +/* Check if there is an interface active. */
> +static inline int lbs_iface_active(struct lbs_private *priv)
> +{
> + int r;
> +
> + r = netif_running(priv->dev);
> + if (priv->mesh_dev);
> + r |= netif_running(priv->dev);
> +
> + return r;
> +}
> +
> #endif
> diff --git a/drivers/net/wireless/libertas/if_usb.c
> b/drivers/net/wireless/libertas/if_usb.c index e368b29..0775f45 100644
> --- a/drivers/net/wireless/libertas/if_usb.c
> +++ b/drivers/net/wireless/libertas/if_usb.c
> @@ -956,7 +956,7 @@ static int if_usb_prog_firmware(struct if_usb_card
> *cardp, priv->dnld_sent = DNLD_RES_RECEIVED;
> spin_unlock_irqrestore(&priv->driver_lock, flags);
>
> - wake_up_interruptible(&priv->waitq);
> + wake_up(&priv->waitq);
>
> return ret;
> }
> diff --git a/drivers/net/wireless/libertas/main.c
> b/drivers/net/wireless/libertas/main.c index ee28ae5..d62d1fb 100644
> --- a/drivers/net/wireless/libertas/main.c
> +++ b/drivers/net/wireless/libertas/main.c
> @@ -99,6 +99,37 @@ u8 lbs_data_rate_to_fw_index(u32 rate)
> return 0;
> }
>
> +int lbs_start_iface(struct lbs_private *priv)
> +{
> + struct cmd_ds_802_11_mac_address cmd;
> + int ret;
> +
> + if (priv->power_restore) {
> + ret = priv->power_restore(priv);
> + if (ret)
> + return ret;
> + }
> +
> + cmd.hdr.size = cpu_to_le16(sizeof(cmd));
> + cmd.action = cpu_to_le16(CMD_ACT_SET);
> + memcpy(cmd.macadd, priv->current_addr, ETH_ALEN);
> +
> + ret = lbs_cmd_with_response(priv, CMD_802_11_MAC_ADDRESS, &cmd);
> + if (ret) {
> + lbs_deb_net("set MAC address failed\n");
> + goto err;
> + }
> +
> + lbs_update_channel(priv);
> +
> + priv->iface_running = true;
> + return 0;
> +
> +err:
> + if (priv->power_save)
> + priv->power_save(priv);
> + return ret;
> +}
>
> /**
> * lbs_dev_open - open the ethX interface
> @@ -112,23 +143,64 @@ static int lbs_dev_open(struct net_device *dev)
> int ret = 0;
>
> lbs_deb_enter(LBS_DEB_NET);
> + if (!priv->iface_running) {
> + ret = lbs_start_iface(priv);
> + if (ret)
> + goto out;
> + }
>
> spin_lock_irq(&priv->driver_lock);
> - priv->stopping = false;
>
> - if (priv->connect_status == LBS_CONNECTED)
> - netif_carrier_on(dev);
> - else
> - netif_carrier_off(dev);
> + netif_carrier_off(dev);
>
> if (!priv->tx_pending_len)
> netif_wake_queue(dev);
>
> spin_unlock_irq(&priv->driver_lock);
> +
> +out:
> lbs_deb_leave_args(LBS_DEB_NET, "ret %d", ret);
> return ret;
> }
>
> +static bool lbs_command_queue_empty(struct lbs_private *priv)
> +{
> + unsigned long flags;
> + bool ret;
> + spin_lock_irqsave(&priv->driver_lock, flags);
> + ret = priv->cur_cmd == NULL && list_empty(&priv->cmdpendingq);
> + spin_unlock_irqrestore(&priv->driver_lock, flags);
> + return ret;
> +}
> +
> +int lbs_stop_iface(struct lbs_private *priv)
> +{
> + unsigned long flags;
> + int ret = 0;
> +
> + lbs_deb_enter(LBS_DEB_MAIN);
> +
> + spin_lock_irqsave(&priv->driver_lock, flags);
> + priv->iface_running = false;
> + kfree_skb(priv->currenttxskb);
> + priv->currenttxskb = NULL;
> + priv->tx_pending_len = 0;
> + spin_unlock_irqrestore(&priv->driver_lock, flags);
> +
> + cancel_work_sync(&priv->mcast_work);
> +
> + /* Disable command processing, and wait for all commands to complete */
> + lbs_deb_main("waiting for commands to complete\n");
> + wait_event(priv->waitq, lbs_command_queue_empty(priv));
> + lbs_deb_main("all commands completed\n");
> +
> + if (priv->power_save)
> + ret = priv->power_save(priv);
> +
> + lbs_deb_leave(LBS_DEB_MAIN);
> + return ret;
> +}
> +
> /**
> * lbs_eth_stop - close the ethX interface
> *
> @@ -141,18 +213,25 @@ static int lbs_eth_stop(struct net_device *dev)
>
> lbs_deb_enter(LBS_DEB_NET);
>
> + if (priv->connect_status == LBS_CONNECTED)
> + lbs_disconnect(priv, WLAN_REASON_DEAUTH_LEAVING);
> +
> spin_lock_irq(&priv->driver_lock);
> - priv->stopping = true;
> netif_stop_queue(dev);
> spin_unlock_irq(&priv->driver_lock);
>
> - schedule_work(&priv->mcast_work);
> + lbs_update_mcast(priv);
> cancel_delayed_work_sync(&priv->scan_work);
> if (priv->scan_req) {
> cfg80211_scan_done(priv->scan_req, false);
> priv->scan_req = NULL;
> }
>
> + netif_carrier_off(priv->dev);
> +
> + if (!lbs_iface_active(priv))
> + lbs_stop_iface(priv);
> +
> lbs_deb_leave(LBS_DEB_NET);
> return 0;
> }
> @@ -170,7 +249,7 @@ void lbs_host_to_card_done(struct lbs_private *priv)
> /* Wake main thread if commands are pending */
> if (!priv->cur_cmd || priv->tx_pending_len > 0) {
> if (!priv->wakeup_dev_required)
> - wake_up_interruptible(&priv->waitq);
> + wake_up(&priv->waitq);
> }
>
> spin_unlock_irqrestore(&priv->driver_lock, flags);
> @@ -183,29 +262,24 @@ int lbs_set_mac_address(struct net_device *dev, void
> *addr) int ret = 0;
> struct lbs_private *priv = dev->ml_priv;
> struct sockaddr *phwaddr = addr;
> - struct cmd_ds_802_11_mac_address cmd;
>
> lbs_deb_enter(LBS_DEB_NET);
>
> + /*
> + * Can only set MAC address when all interfaces are down, to be written
> + * to the hardware when one of them is brought up.
> + */
> + if (lbs_iface_active(priv))
> + return -EBUSY;
> +
> /* In case it was called from the mesh device */
> dev = priv->dev;
>
> - cmd.hdr.size = cpu_to_le16(sizeof(cmd));
> - cmd.action = cpu_to_le16(CMD_ACT_SET);
> - memcpy(cmd.macadd, phwaddr->sa_data, ETH_ALEN);
> -
> - ret = lbs_cmd_with_response(priv, CMD_802_11_MAC_ADDRESS, &cmd);
> - if (ret) {
> - lbs_deb_net("set MAC address failed\n");
> - goto done;
> - }
> -
> memcpy(priv->current_addr, phwaddr->sa_data, ETH_ALEN);
> memcpy(dev->dev_addr, phwaddr->sa_data, ETH_ALEN);
> if (priv->mesh_dev)
> memcpy(priv->mesh_dev->dev_addr, phwaddr->sa_data, ETH_ALEN);
>
> -done:
> lbs_deb_leave_args(LBS_DEB_NET, "ret %d", ret);
> return ret;
> }
> @@ -259,18 +333,18 @@ static int lbs_add_mcast_addrs(struct
> cmd_ds_mac_multicast_adr *cmd, return i;
> }
>
> -static void lbs_set_mcast_worker(struct work_struct *work)
> +void lbs_update_mcast(struct lbs_private *priv)
> {
> - struct lbs_private *priv = container_of(work, struct lbs_private,
> mcast_work); struct cmd_ds_mac_multicast_adr mcast_cmd;
> - int dev_flags;
> + int dev_flags = 0;
> int nr_addrs;
> int old_mac_control = priv->mac_control;
>
> lbs_deb_enter(LBS_DEB_NET);
>
> - dev_flags = priv->dev->flags;
> - if (priv->mesh_dev)
> + if (netif_running(priv->dev))
> + dev_flags |= priv->dev->flags;
> + if (priv->mesh_dev && netif_running(priv->mesh_dev))
> dev_flags |= priv->mesh_dev->flags;
>
> if (dev_flags & IFF_PROMISC) {
> @@ -316,6 +390,12 @@ static void lbs_set_mcast_worker(struct work_struct
> *work) lbs_deb_leave(LBS_DEB_NET);
> }
>
> +static void lbs_set_mcast_worker(struct work_struct *work)
> +{
> + struct lbs_private *priv = container_of(work, struct lbs_private,
> mcast_work); + lbs_update_mcast(priv);
> +}
> +
> void lbs_set_multicast_list(struct net_device *dev)
> {
> struct lbs_private *priv = dev->ml_priv;
> @@ -648,7 +728,7 @@ static void lbs_cmd_timeout_handler(unsigned long data)
> if (priv->dnld_sent == DNLD_CMD_SENT)
> priv->dnld_sent = DNLD_RES_RECEIVED;
>
> - wake_up_interruptible(&priv->waitq);
> + wake_up(&priv->waitq);
> out:
> spin_unlock_irqrestore(&priv->driver_lock, flags);
> lbs_deb_leave(LBS_DEB_CMD);
> @@ -890,10 +970,6 @@ void lbs_remove_card(struct lbs_private *priv)
> lbs_remove_mesh(priv);
> lbs_scan_deinit(priv);
>
> - dev = priv->dev;
> -
> - cancel_work_sync(&priv->mcast_work);
> -
> /* worker thread destruction blocks on the in-flight command which
> * should have been cleared already in lbs_stop_card().
> */
> @@ -964,8 +1040,6 @@ int lbs_start_card(struct lbs_private *priv)
> if (lbs_mesh_activated(priv))
> lbs_start_mesh(priv);
>
> - lbs_update_channel(priv);
> -
> lbs_debugfs_init_one(priv, dev);
>
> netdev_info(dev, "Marvell WLAN 802.11 adapter\n");
> @@ -982,8 +1056,6 @@ EXPORT_SYMBOL_GPL(lbs_start_card);
> void lbs_stop_card(struct lbs_private *priv)
> {
> struct net_device *dev;
> - struct cmd_ctrl_node *cmdnode;
> - unsigned long flags;
>
> lbs_deb_enter(LBS_DEB_MAIN);
>
> @@ -996,30 +1068,6 @@ void lbs_stop_card(struct lbs_private *priv)
>
> lbs_debugfs_remove_one(priv);
> lbs_deinit_mesh(priv);
> -
> - /* Delete the timeout of the currently processing command */
> - del_timer_sync(&priv->command_timer);
> - del_timer_sync(&priv->auto_deepsleep_timer);
> -
> - /* Flush pending command nodes */
> - spin_lock_irqsave(&priv->driver_lock, flags);
> - lbs_deb_main("clearing pending commands\n");
> - list_for_each_entry(cmdnode, &priv->cmdpendingq, list) {
> - cmdnode->result = -ENOENT;
> - cmdnode->cmdwaitqwoken = 1;
> - wake_up(&cmdnode->cmdwait_q);
> - }
> -
> - /* Flush the command the card is currently processing */
> - if (priv->cur_cmd) {
> - lbs_deb_main("clearing current command\n");
> - priv->cur_cmd->result = -ENOENT;
> - priv->cur_cmd->cmdwaitqwoken = 1;
> - wake_up(&priv->cur_cmd->cmdwait_q);
> - }
> - lbs_deb_main("done clearing commands\n");
> - spin_unlock_irqrestore(&priv->driver_lock, flags);
> -
> unregister_netdev(dev);
>
> out:
> @@ -1040,7 +1088,7 @@ void lbs_queue_event(struct lbs_private *priv, u32
> event)
>
> kfifo_in(&priv->event_fifo, (unsigned char *) &event, sizeof(u32));
>
> - wake_up_interruptible(&priv->waitq);
> + wake_up(&priv->waitq);
>
> spin_unlock_irqrestore(&priv->driver_lock, flags);
> lbs_deb_leave(LBS_DEB_THREAD);
> @@ -1058,7 +1106,7 @@ void lbs_notify_command_response(struct lbs_private
> *priv, u8 resp_idx) BUG_ON(resp_idx > 1);
> priv->resp_idx = resp_idx;
>
> - wake_up_interruptible(&priv->waitq);
> + wake_up(&priv->waitq);
>
> lbs_deb_leave(LBS_DEB_THREAD);
> }
> diff --git a/drivers/net/wireless/libertas/mesh.c
> b/drivers/net/wireless/libertas/mesh.c index 2a635d2..138699b 100644
> --- a/drivers/net/wireless/libertas/mesh.c
> +++ b/drivers/net/wireless/libertas/mesh.c
> @@ -924,7 +924,9 @@ static int lbs_mesh_stop(struct net_device *dev)
>
> spin_unlock_irq(&priv->driver_lock);
>
> - schedule_work(&priv->mcast_work);
> + lbs_update_mcast(priv);
> + if (!lbs_iface_active(priv))
> + lbs_stop_iface(priv);
>
> lbs_deb_leave(LBS_DEB_MESH);
> return 0;
> @@ -942,6 +944,11 @@ static int lbs_mesh_dev_open(struct net_device *dev)
> int ret = 0;
>
> lbs_deb_enter(LBS_DEB_NET);
> + if (!priv->iface_running) {
> + ret = lbs_start_iface(priv);
> + if (ret)
> + goto out;
> + }
>
> spin_lock_irq(&priv->driver_lock);

2011-08-02 22:32:23

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: add ability to power off card on suspend

On 2 August 2011 22:12, Vasily Khoruzhick <[email protected]> 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

2011-08-02 10:12:55

by Vasily Khoruzhick

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: disable functionality when interface is down

On Tuesday 02 August 2011 12:02:03 Vasily Khoruzhick wrote:
> On Monday 01 August 2011 18:43:13 Daniel Drake wrote:
> > Modify the driver so that it does not function when the interface is
> > down, in preparation for runtime power management.
> >
> > No commands can be run while the interface is down, so the ndo_dev_stop
> > routine now directly does all necessary work (including asking the device
> > to disconnect from the network and disabling multicast functionality)
> > directly.
> >
> > power_save and power_restore hooks are added meaning that card drivers
> > can take steps to turn the device off when the interface is down.
> >
> > The MAC address can now only be changed when all interfaces are down;
> > the new address will be programmed when an interface gets brought up.
> > This matches mac80211 behaviour.
> >
> > Also, some small cleanups/simplifications were made in the surrounding
> > device handling logic.
>
> Ok, one more question: what about suspend? Maybe it worth to add flag to
> disable card before going suspend and re-enable it during resume?

Implemented it. The only remaining question is messing with power_{on,off}
functions in if_sdio, but I'm OK with it. Will send my patches in few minutes.

Regards
Vasily

2011-08-02 20:32:29

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: add ability to power off card on suspend

On 2 August 2011 21:09, Vasily Khoruzhick <[email protected]> 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

2011-08-02 10:28:06

by Vasily Khoruzhick

[permalink] [raw]
Subject: [PATCH 1/2] libertas: add ability to power off card on suspend

Could be usefull if it's not possible to keep power on the card
when host goes into suspend.

Signed-off-by: Vasily Khoruzhick <[email protected]>
---
drivers/net/wireless/libertas/dev.h | 3 ++-
drivers/net/wireless/libertas/main.c | 22 +++++++++++++++-------
2 files changed, 17 insertions(+), 8 deletions(-)

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;
}
--
1.7.5.rc3


2011-08-01 19:13:44

by Vasily Khoruzhick

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: disable functionality when interface is down

On Monday 01 August 2011 20:50:03 Daniel Drake wrote:
> On 1 August 2011 18:38, Vasily Khoruzhick <[email protected]> wrote:
> >> power_save and power_restore hooks are added meaning that card drivers
> >> can take steps to turn the device off when the interface is down.
> >
> > You mean completely off? And firmware needs to be reloaded after power
> > up?
>
> Yep, exactly like that. Same as various mac80211-based drivers.
>
> The 2nd patch in this series implements it for libertas_sdio, the
> other libertas interfaces have their behaviour as before.

Cool, I hope I'll get some time tomorrow to test your changes with
libertas_spi.

Regards
Vasily

2011-08-01 17:50:04

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: disable functionality when interface is down

On 1 August 2011 18:38, Vasily Khoruzhick <[email protected]> wrote:
>> power_save and power_restore hooks are added meaning that card drivers
>> can take steps to turn the device off when the interface is down.
>
> You mean completely off? And firmware needs to be reloaded after power up?

Yep, exactly like that. Same as various mac80211-based drivers.

The 2nd patch in this series implements it for libertas_sdio, the
other libertas interfaces have their behaviour as before.

Daniel

2011-08-02 10:28:14

by Vasily Khoruzhick

[permalink] [raw]
Subject: [PATCH 2/2] libertas: implement if_spi runtime power management

The SPI card is now fully powered down when the network
interface is brought down.

Signed-off-by: Vasily Khoruzhick <[email protected]>
---
drivers/net/wireless/libertas/if_spi.c | 80 +++++++++++++++++--------------
1 files changed, 44 insertions(+), 36 deletions(-)

diff --git a/drivers/net/wireless/libertas/if_spi.c b/drivers/net/wireless/libertas/if_spi.c
index 9a1468c..c07db7d 100644
--- a/drivers/net/wireless/libertas/if_spi.c
+++ b/drivers/net/wireless/libertas/if_spi.c
@@ -1113,21 +1113,39 @@ static void if_spi_resume_worker(struct work_struct *work)
card = container_of(work, struct if_spi_card, resume_work);

if (card->suspended) {
- if (card->pdata->setup)
- card->pdata->setup(card->spi);
-
- /* Init card ... */
- if_spi_init_card(card);
-
- enable_irq(card->spi->irq);
-
- /* And resume it ... */
lbs_resume(card->priv);

card->suspended = 0;
}
}

+static int if_spi_power_save(struct lbs_private *priv)
+{
+ struct if_spi_card *card = priv->card;
+
+ flush_workqueue(card->workqueue);
+ disable_irq(card->spi->irq);
+ card->pdata->teardown(card->spi);
+ priv->fw_ready = 0;
+
+ return 0;
+}
+
+static int if_spi_power_restore(struct lbs_private *priv)
+{
+ struct if_spi_card *card = priv->card;
+ int ret;
+
+ card->pdata->setup(card->spi);
+ ret = if_spi_init_card(card);
+ if (ret)
+ return ret;
+ enable_irq(card->spi->irq);
+ priv->fw_ready = 1;
+
+ return 0;
+}
+
static int __devinit if_spi_probe(struct spi_device *spi)
{
struct if_spi_card *card;
@@ -1142,17 +1160,11 @@ static int __devinit if_spi_probe(struct spi_device *spi)
goto out;
}

- if (pdata->setup) {
- err = pdata->setup(spi);
- if (err)
- goto out;
- }
-
/* Allocate card structure to represent this specific device */
card = kzalloc(sizeof(struct if_spi_card), GFP_KERNEL);
if (!card) {
err = -ENOMEM;
- goto teardown;
+ goto out;
}
spi_set_drvdata(spi, card);
card->pdata = pdata;
@@ -1163,13 +1175,6 @@ static int __devinit if_spi_probe(struct spi_device *spi)
INIT_LIST_HEAD(&card->data_packet_list);
spin_lock_init(&card->buffer_lock);

- /* Initialize the SPI Interface Unit */
-
- /* Firmware load */
- err = if_spi_init_card(card);
- if (err)
- goto free_card;
-
/*
* Register our card with libertas.
* This will call alloc_etherdev.
@@ -1180,13 +1185,16 @@ static int __devinit if_spi_probe(struct spi_device *spi)
goto free_card;
}
card->priv = priv;
- priv->setup_fw_on_resume = 1;
priv->card = card;
priv->hw_host_to_card = if_spi_host_to_card;
+ if (pdata->setup && pdata->teardown) {
+ priv->power_save = if_spi_power_save;
+ priv->power_restore = if_spi_power_restore;
+ }
priv->enter_deep_sleep = NULL;
priv->exit_deep_sleep = NULL;
priv->reset_deep_sleep_wakeup = NULL;
- priv->fw_ready = 1;
+ priv->disable_on_suspend = 1;

/* Initialize interrupt handling stuff. */
card->workqueue = create_workqueue("libertas_spi");
@@ -1200,6 +1208,13 @@ static int __devinit if_spi_probe(struct spi_device *spi)
goto terminate_workqueue;
}

+ /* Disable IRQ, hw is not ready yet */
+ disable_irq(spi->irq);
+
+ err = if_spi_power_restore(priv);
+ if (err)
+ goto release_irq;
+
/*
* Start the card.
* This will call register_netdev, and we'll start
@@ -1207,13 +1222,16 @@ static int __devinit if_spi_probe(struct spi_device *spi)
*/
err = lbs_start_card(priv);
if (err)
- goto release_irq;
+ goto teardown;
+ if_spi_power_save(priv);

lbs_deb_spi("Finished initializing WLAN module.\n");

/* successful exit */
goto out;

+teardown:
+ if_spi_power_save(priv);
release_irq:
free_irq(spi->irq, card);
terminate_workqueue:
@@ -1222,9 +1240,6 @@ terminate_workqueue:
lbs_remove_card(priv); /* will call free_netdev */
free_card:
free_if_spi_card(card);
-teardown:
- if (pdata->teardown)
- pdata->teardown(spi);
out:
lbs_deb_leave_args(LBS_DEB_SPI, "err %d\n", err);
return err;
@@ -1247,8 +1262,6 @@ static int __devexit libertas_spi_remove(struct spi_device *spi)
cancel_work_sync(&card->packet_work);
flush_workqueue(card->workqueue);
destroy_workqueue(card->workqueue);
- if (card->pdata->teardown)
- card->pdata->teardown(spi);
free_if_spi_card(card);
lbs_deb_leave(LBS_DEB_SPI);
return 0;
@@ -1261,11 +1274,6 @@ static int if_spi_suspend(struct device *dev)

if (!card->suspended) {
lbs_suspend(card->priv);
- flush_workqueue(card->workqueue);
- disable_irq(spi->irq);
-
- if (card->pdata->teardown)
- card->pdata->teardown(spi);
card->suspended = 1;
}

--
1.7.5.rc3


2011-08-01 19:38:49

by Vasily Khoruzhick

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: disable functionality when interface is down

On Monday 01 August 2011 22:19:12 Daniel Drake wrote:
> On 1 August 2011 20:13, Vasily Khoruzhick <[email protected]> wrote:
> > Cool, I hope I'll get some time tomorrow to test your changes with
> > libertas_spi.
>
> Just to clarify, you won't see any power saving effects until you
> implement power_save and power_restore on the SPI level. See patch 2/2
> for how I did it for SDIO (it builds upon quite a bit of hard work
> implementing runtime power management in the MMC layer).

Yep, I understand. I sent pretty similar patch with same functionality some
month ago, but it was not clean enough, and I had no time to clean it up. So I
have almost all things ready, just need some tuning :)

Regards
Vasily

2011-08-02 10:54:08

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: add ability to power off card on suspend

On 2 August 2011 11:27, Vasily Khoruzhick <[email protected]> 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

2011-08-02 21:12:52

by Vasily Khoruzhick

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: add ability to power off card on suspend

On Tuesday 02 August 2011 23:32:27 Daniel Drake wrote:
> On 2 August 2011 21:09, Vasily Khoruzhick <[email protected]> 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

2011-08-01 17:39:12

by Vasily Khoruzhick

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: disable functionality when interface is down

On Monday 01 August 2011 18:43:13 Daniel Drake wrote:

Hi, Daniel

> Modify the driver so that it does not function when the interface is
> down, in preparation for runtime power management.
>
> No commands can be run while the interface is down, so the ndo_dev_stop
> routine now directly does all necessary work (including asking the device
> to disconnect from the network and disabling multicast functionality)
> directly.
>
> power_save and power_restore hooks are added meaning that card drivers
> can take steps to turn the device off when the interface is down.

You mean completely off? And firmware needs to be reloaded after power up?

> The MAC address can now only be changed when all interfaces are down;
> the new address will be programmed when an interface gets brought up.
> This matches mac80211 behaviour.
>
> Also, some small cleanups/simplifications were made in the surrounding
> device handling logic.

Regards
Vasily

2011-08-01 19:19:14

by Daniel Drake

[permalink] [raw]
Subject: Re: [PATCH 1/2] libertas: disable functionality when interface is down

On 1 August 2011 20:13, Vasily Khoruzhick <[email protected]> wrote:
> Cool, I hope I'll get some time tomorrow to test your changes with
> libertas_spi.

Just to clarify, you won't see any power saving effects until you
implement power_save and power_restore on the SPI level. See patch 2/2
for how I did it for SDIO (it builds upon quite a bit of hard work
implementing runtime power management in the MMC layer).

Testing is appreciated anyway, because the driver shape changed in
general with this patch: e.g. it used to let you scan while the
interface is down, and now it doesn't. Would be good to have
additional confirmation that nothing has broken :)

Thanks,
Daniel