On Tue, Dec 20, 2011 at 11:38:22AM +0530, Yogesh Ashok Powar wrote:
> In case of firmware crash, reload the firmware and reconfigure it
> by triggering ieee80211_hw_restart; mac80211 utility function.
>
> Signed-off-by: Nishant Sarmukadam <nishants@xxxxxxxxxxx>
> Signed-off-by: Yogesh Ashok Powar <yogeshp@xxxxxxxxxxx>
> @@ -262,6 +262,11 @@ struct mwl8k_priv {
> */
> struct ieee80211_tx_queue_params wmm_params[MWL8K_TX_WMM_QUEUES];
>
> + /* To do the task of reloading the firmware */
> + struct work_struct fw_reload;
> +
> + atomic_t hw_reload_state;
Why is this an atomic_t?
> +/* FW reload states */
> +enum {
> + FW_RELOAD_INIT = 0,
> + FW_RELOAD_TEST,
> + FW_RELOAD_ALL_BLOCK,
> + FW_RELOAD_FINAL,
> +};
> +
> /* Request fw image */
> static int mwl8k_request_fw(struct mwl8k_priv *priv,
> const char *fname, const struct firmware **fw,
> @@ -1516,6 +1529,9 @@ static int mwl8k_tx_wait_empty(struct ieee80211_hw *hw)
>
> oldcount = priv->pending_tx_pkts;
>
> + if (!atomic_read(&priv->hw_reload_state))
> + atomic_set(&priv->hw_reload_state, FW_RELOAD_TEST);
Explicit comparison against FW_RELOAD_INIT ?
> @@ -1827,6 +1850,16 @@ mwl8k_txq_xmit(struct ieee80211_hw *hw, int index, struct sk_buff *skb)
> bool mgmtframe = false;
> struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)skb->data;
>
> + if (atomic_read(&priv->hw_reload_state) > FW_RELOAD_TEST) {
> + /*
> + * If fw reload is going on there is no point
> + * in sending the packets down to the firmware.
> + * Free the packets
> + */
> + dev_kfree_skb(skb);
> + return;
Why don't you stop the queues when commencing firmware reload?
> @@ -2102,6 +2135,13 @@ static int mwl8k_post_cmd(struct ieee80211_hw *hw, struct mwl8k_cmd_pkt *cmd)
> unsigned long timeout = 0;
> u8 buf[32];
>
> + if (atomic_read(&priv->hw_reload_state) == FW_RELOAD_ALL_BLOCK) {
> + wiphy_err(hw->wiphy, "Not executing command %s since "
> + "firmware reload is in progress\n",
> + mwl8k_cmd_name(cmd->code, buf, sizeof(buf)));
> + return -EBUSY;
> + }
Why don't you just hold the firmware lock while reloading the firmware?
> @@ -4510,8 +4552,10 @@ static void mwl8k_remove_interface(struct ieee80211_hw *hw,
>
> mwl8k_cmd_set_mac_addr(hw, vif, "\x00\x00\x00\x00\x00\x00");
>
> - priv->macids_used &= ~(1 << mwl8k_vif->macid);
> - list_del(&mwl8k_vif->list);
> + if (priv->macids_used) {
> + priv->macids_used &= ~(1 << mwl8k_vif->macid);
> + list_del(&mwl8k_vif->list);
> + }
If mwl8k_remove_interface() is called, ->macids_used is surely nonzero?
What does this change accomplish?
> +static void mwl8k_reload_fw_work(struct work_struct *work)
Redundant space
> +{
> + struct mwl8k_priv *priv =
> + container_of(work, struct mwl8k_priv, fw_reload);
Extra tab
> + /* If some command is waiting for a response, clear it */
> + if (priv->hostcmd_wait != NULL)
> + complete(priv->hostcmd_wait);
And set it to NULL?
> + if (mwl8k_reload_firmware(hw, di->fw_image_ap))
> + wiphy_err(hw->wiphy, "Firmware reloading failed\n");
> + else
> + wiphy_err(hw->wiphy, "Firmware restarted successfully\n");
Why are you always reloading the Access Point firmware image? The
STA firmware image never crashes?
> +
> + return;
> }
Redundant statement.
> static int mwl8k_init_firmware(struct ieee80211_hw *hw, char *fw_image,
> @@ -5268,6 +5335,8 @@ static int mwl8k_init_firmware(struct ieee80211_hw *hw, char *fw_image,
> {
> struct mwl8k_priv *priv = hw->priv;
> int rc;
> +#define MAX_RESATRT_ATTEMEPT_COUNT 5
> + static int restart_count;
eeeeew. no, no, no.
> static int mwl8k_reload_firmware(struct ieee80211_hw *hw, char *fw_image)
> {
> int i, rc = 0;
> struct mwl8k_priv *priv = hw->priv;
> + struct mwl8k_vif *mwl8k_vif, *tmp_vif;
>
> mwl8k_stop(hw);
> mwl8k_rxq_deinit(hw, 0);
>
> + list_for_each_entry_safe(mwl8k_vif, tmp_vif, &priv->vif_list, list) {
> + priv->macids_used &= ~(1 << mwl8k_vif->macid);
> + list_del(&mwl8k_vif->list);
> + }
This seems like a dirty hack to cover up for something else.
> @@ -5624,7 +5729,6 @@ static int __devinit mwl8k_probe(struct pci_dev *pdev,
> priv->pdev = pdev;
> priv->device_info = &mwl8k_info_tbl[id->driver_data];
>
> -
> priv->sram = pci_iomap(pdev, 0, 0x10000);
> if (priv->sram == NULL) {
> wiphy_err(hw->wiphy, "Cannot map device SRAM\n");
Unrelated whitespace change.
Isn't cozybit involved in this anymore?
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
