* [PATCH 0/3] iwlwifi driver updates 03/26/2009 @ 2009-03-26 17:14 Reinette Chatre 2009-03-26 17:14 ` [PATCH 1/3] iwlwifi: merge and better support of suspend/resume for iwlagn and iwl3945 Reinette Chatre 0 siblings, 1 reply; 19+ messages in thread From: Reinette Chatre @ 2009-03-26 17:14 UTC (permalink / raw) To: linville; +Cc: linux-wireless, ipw3945-devel, Reinette Chatre In this series we improve support for suspend/resume by making use of infrastructure already in place in mac80211. We fix the problem where device cannot come back up if rfkill was enabled during ifdown. The 3945 is a porting patch but through this also fixes a locking issue that was present in iwl3945 but not in iwlagn (see https://bugzilla.redhat.com/show_bug.cgi?id=492116). [PATCH 1/3] iwlwifi: merge and better support of suspend/resume for iwlagn and iwl3945 [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill [PATCH 3/3] iwl3945: use iwl_mac_conf_tx Thank you Reinette ^ permalink raw reply [flat|nested] 19+ messages in thread
* [PATCH 1/3] iwlwifi: merge and better support of suspend/resume for iwlagn and iwl3945 2009-03-26 17:14 [PATCH 0/3] iwlwifi driver updates 03/26/2009 Reinette Chatre @ 2009-03-26 17:14 ` Reinette Chatre 2009-03-26 17:14 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Reinette Chatre 0 siblings, 1 reply; 19+ messages in thread From: Reinette Chatre @ 2009-03-26 17:14 UTC (permalink / raw) To: linville; +Cc: linux-wireless, ipw3945-devel, Wey-Yi Guy, Reinette Chatre From: Wey-Yi Guy <wey-yi.w.guy@intel.com> With mac80211's help to call stop() and start() in mac80211 suspend/resume function, both iwlagn and iwl3945 no longer calling stop() and start(); remove un-necessary STATUS_IN_SUSPEND bit from both header files and functions, Move apm_ops.stop() function into pci_suspend() to ensure DMA is stopped before go into suspend mode. iwl3945 has the similar suspend/resume function as iwlagn, so move both functions to iwlcore to be shared by both drivers. Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> --- drivers/net/wireless/iwlwifi/iwl-3945.h | 1 - drivers/net/wireless/iwlwifi/iwl-agn.c | 54 ++--------------------- drivers/net/wireless/iwlwifi/iwl-core.c | 40 +++++++++++++++++ drivers/net/wireless/iwlwifi/iwl-core.h | 5 ++- drivers/net/wireless/iwlwifi/iwl-debugfs.c | 2 - drivers/net/wireless/iwlwifi/iwl3945-base.c | 63 +++----------------------- 6 files changed, 56 insertions(+), 109 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h b/drivers/net/wireless/iwlwifi/iwl-3945.h index ab7aaf6..29bc0d2 100644 --- a/drivers/net/wireless/iwlwifi/iwl-3945.h +++ b/drivers/net/wireless/iwlwifi/iwl-3945.h @@ -162,7 +162,6 @@ struct iwl3945_frame { #define STATUS_TEMPERATURE 8 #define STATUS_GEO_CONFIGURED 9 #define STATUS_EXIT_PENDING 10 -#define STATUS_IN_SUSPEND 11 #define STATUS_STATISTICS 12 #define STATUS_SCANNING 13 #define STATUS_SCAN_ABORTING 14 diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c index 663dc83..669b435 100644 --- a/drivers/net/wireless/iwlwifi/iwl-agn.c +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c @@ -1654,7 +1654,7 @@ static void __iwl_down(struct iwl_priv *priv) ieee80211_stop_queues(priv->hw); /* If we have not previously called iwl_init() then - * clear all bits but the RF Kill and SUSPEND bits and return */ + * clear all bits but the RF Kill bits and return */ if (!iwl_is_init(priv)) { priv->status = test_bit(STATUS_RF_KILL_HW, &priv->status) << STATUS_RF_KILL_HW | @@ -1662,23 +1662,19 @@ static void __iwl_down(struct iwl_priv *priv) STATUS_RF_KILL_SW | test_bit(STATUS_GEO_CONFIGURED, &priv->status) << STATUS_GEO_CONFIGURED | - test_bit(STATUS_IN_SUSPEND, &priv->status) << - STATUS_IN_SUSPEND | test_bit(STATUS_EXIT_PENDING, &priv->status) << STATUS_EXIT_PENDING; goto exit; } - /* ...otherwise clear out all the status bits but the RF Kill and - * SUSPEND bits and continue taking the NIC down. */ + /* ...otherwise clear out all the status bits but the RF Kill + * bits and continue taking the NIC down. */ priv->status &= test_bit(STATUS_RF_KILL_HW, &priv->status) << STATUS_RF_KILL_HW | test_bit(STATUS_RF_KILL_SW, &priv->status) << STATUS_RF_KILL_SW | test_bit(STATUS_GEO_CONFIGURED, &priv->status) << STATUS_GEO_CONFIGURED | - test_bit(STATUS_IN_SUSPEND, &priv->status) << - STATUS_IN_SUSPEND | test_bit(STATUS_FW_ERROR, &priv->status) << STATUS_FW_ERROR | test_bit(STATUS_EXIT_PENDING, &priv->status) << @@ -1703,7 +1699,7 @@ static void __iwl_down(struct iwl_priv *priv) udelay(5); /* FIXME: apm_ops.suspend(priv) */ - if (exit_pending || test_bit(STATUS_IN_SUSPEND, &priv->status)) + if (exit_pending) priv->cfg->ops->lib->apm_ops.stop(priv); else priv->cfg->ops->lib->apm_ops.reset(priv); @@ -2064,9 +2060,6 @@ static int iwl_mac_start(struct ieee80211_hw *hw) IWL_DEBUG_INFO(priv, "Start UP work done.\n"); - if (test_bit(STATUS_IN_SUSPEND, &priv->status)) - return 0; - /* Wait for START_ALIVE from Run Time ucode. Otherwise callbacks from * mac80211 will not be run successfully. */ ret = wait_event_interruptible_timeout(priv->wait_command_queue, @@ -3566,45 +3559,6 @@ static void __devexit iwl_pci_remove(struct pci_dev *pdev) ieee80211_free_hw(priv->hw); } -#ifdef CONFIG_PM - -static int iwl_pci_suspend(struct pci_dev *pdev, pm_message_t state) -{ - struct iwl_priv *priv = pci_get_drvdata(pdev); - - if (priv->is_open) { - set_bit(STATUS_IN_SUSPEND, &priv->status); - iwl_mac_stop(priv->hw); - priv->is_open = 1; - } - - pci_save_state(pdev); - pci_disable_device(pdev); - pci_set_power_state(pdev, PCI_D3hot); - - return 0; -} - -static int iwl_pci_resume(struct pci_dev *pdev) -{ - struct iwl_priv *priv = pci_get_drvdata(pdev); - int ret; - - pci_set_power_state(pdev, PCI_D0); - ret = pci_enable_device(pdev); - if (ret) - return ret; - pci_restore_state(pdev); - iwl_enable_interrupts(priv); - - if (priv->is_open) - iwl_mac_start(priv->hw); - - clear_bit(STATUS_IN_SUSPEND, &priv->status); - return 0; -} - -#endif /* CONFIG_PM */ /***************************************************************************** * diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c index 4b1298c..e1b5472 100644 --- a/drivers/net/wireless/iwlwifi/iwl-core.c +++ b/drivers/net/wireless/iwlwifi/iwl-core.c @@ -2115,3 +2115,43 @@ void iwl_rx_reply_error(struct iwl_priv *priv, } EXPORT_SYMBOL(iwl_rx_reply_error); +#ifdef CONFIG_PM + +int iwl_pci_suspend(struct pci_dev *pdev, pm_message_t state) +{ + struct iwl_priv *priv = pci_get_drvdata(pdev); + + /* + * This function is called when system goes into suspend state + * mac80211 will call iwl_mac_stop() from the mac80211 suspend function + * first but since iwl_mac_stop() has no knowledge of who the caller is, + * it will not call apm_ops.stop() to stop the DMA operation. + * Calling apm_ops.stop here to make sure we stop the DMA. + */ + priv->cfg->ops->lib->apm_ops.stop(priv); + + pci_save_state(pdev); + pci_disable_device(pdev); + pci_set_power_state(pdev, PCI_D3hot); + + return 0; +} +EXPORT_SYMBOL(iwl_pci_suspend); + +int iwl_pci_resume(struct pci_dev *pdev) +{ + struct iwl_priv *priv = pci_get_drvdata(pdev); + int ret; + + pci_set_power_state(pdev, PCI_D0); + ret = pci_enable_device(pdev); + if (ret) + return ret; + pci_restore_state(pdev); + iwl_enable_interrupts(priv); + + return 0; +} +EXPORT_SYMBOL(iwl_pci_resume); + +#endif /* CONFIG_PM */ diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h index a8eac8c..5d49045 100644 --- a/drivers/net/wireless/iwlwifi/iwl-core.h +++ b/drivers/net/wireless/iwlwifi/iwl-core.h @@ -432,6 +432,10 @@ static inline u16 iwl_pcie_link_ctl(struct iwl_priv *priv) pci_read_config_word(priv->pci_dev, pos + PCI_EXP_LNKCTL, &pci_lnk_ctl); return pci_lnk_ctl; } +#ifdef CONFIG_PM +int iwl_pci_suspend(struct pci_dev *pdev, pm_message_t state); +int iwl_pci_resume(struct pci_dev *pdev); +#endif /* CONFIG_PM */ /***************************************************** * Error Handling Debugging @@ -458,7 +462,6 @@ void iwlcore_free_geos(struct iwl_priv *priv); #define STATUS_TEMPERATURE 8 #define STATUS_GEO_CONFIGURED 9 #define STATUS_EXIT_PENDING 10 -#define STATUS_IN_SUSPEND 11 #define STATUS_STATISTICS 12 #define STATUS_SCANNING 13 #define STATUS_SCAN_ABORTING 14 diff --git a/drivers/net/wireless/iwlwifi/iwl-debugfs.c b/drivers/net/wireless/iwlwifi/iwl-debugfs.c index 64eb585..af1d121 100644 --- a/drivers/net/wireless/iwlwifi/iwl-debugfs.c +++ b/drivers/net/wireless/iwlwifi/iwl-debugfs.c @@ -456,8 +456,6 @@ static ssize_t iwl_dbgfs_status_read(struct file *file, test_bit(STATUS_GEO_CONFIGURED, &priv->status)); pos += scnprintf(buf + pos, bufsz - pos, "STATUS_EXIT_PENDING:\t %d\n", test_bit(STATUS_EXIT_PENDING, &priv->status)); - pos += scnprintf(buf + pos, bufsz - pos, "STATUS_IN_SUSPEND:\t %d\n", - test_bit(STATUS_IN_SUSPEND, &priv->status)); pos += scnprintf(buf + pos, bufsz - pos, "STATUS_STATISTICS:\t %d\n", test_bit(STATUS_STATISTICS, &priv->status)); pos += scnprintf(buf + pos, bufsz - pos, "STATUS_SCANNING:\t %d\n", diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c index 279d10c..33f5b00 100644 --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c @@ -2996,7 +2996,7 @@ static void __iwl3945_down(struct iwl_priv *priv) ieee80211_stop_queues(priv->hw); /* If we have not previously called iwl3945_init() then - * clear all bits but the RF Kill and SUSPEND bits and return */ + * clear all bits but the RF Kill bits and return */ if (!iwl_is_init(priv)) { priv->status = test_bit(STATUS_RF_KILL_HW, &priv->status) << STATUS_RF_KILL_HW | @@ -3004,23 +3004,19 @@ static void __iwl3945_down(struct iwl_priv *priv) STATUS_RF_KILL_SW | test_bit(STATUS_GEO_CONFIGURED, &priv->status) << STATUS_GEO_CONFIGURED | - test_bit(STATUS_IN_SUSPEND, &priv->status) << - STATUS_IN_SUSPEND | test_bit(STATUS_EXIT_PENDING, &priv->status) << STATUS_EXIT_PENDING; goto exit; } - /* ...otherwise clear out all the status bits but the RF Kill and - * SUSPEND bits and continue taking the NIC down. */ + /* ...otherwise clear out all the status bits but the RF Kill + * bits and continue taking the NIC down. */ priv->status &= test_bit(STATUS_RF_KILL_HW, &priv->status) << STATUS_RF_KILL_HW | test_bit(STATUS_RF_KILL_SW, &priv->status) << STATUS_RF_KILL_SW | test_bit(STATUS_GEO_CONFIGURED, &priv->status) << STATUS_GEO_CONFIGURED | - test_bit(STATUS_IN_SUSPEND, &priv->status) << - STATUS_IN_SUSPEND | test_bit(STATUS_FW_ERROR, &priv->status) << STATUS_FW_ERROR | test_bit(STATUS_EXIT_PENDING, &priv->status) << @@ -3044,7 +3040,7 @@ static void __iwl3945_down(struct iwl_priv *priv) udelay(5); - if (exit_pending || test_bit(STATUS_IN_SUSPEND, &priv->status)) + if (exit_pending) priv->cfg->ops->lib->apm_ops.stop(priv); else priv->cfg->ops->lib->apm_ops.reset(priv); @@ -3097,10 +3093,8 @@ static int __iwl3945_up(struct iwl_priv *priv) clear_bit(STATUS_RF_KILL_HW, &priv->status); else { set_bit(STATUS_RF_KILL_HW, &priv->status); - if (!test_bit(STATUS_IN_SUSPEND, &priv->status)) { - IWL_WARN(priv, "Radio disabled by HW RF Kill switch\n"); - return -ENODEV; - } + IWL_WARN(priv, "Radio disabled by HW RF Kill switch\n"); + return -ENODEV; } iwl_write32(priv, CSR_INT, 0xFFFFFFFF); @@ -3592,9 +3586,6 @@ static int iwl3945_mac_start(struct ieee80211_hw *hw) IWL_DEBUG_INFO(priv, "Start UP work.\n"); - if (test_bit(STATUS_IN_SUSPEND, &priv->status)) - return 0; - /* Wait for START_ALIVE from ucode. Otherwise callbacks from * mac80211 will not be run successfully. */ ret = wait_event_interruptible_timeout(priv->wait_command_queue, @@ -5231,44 +5222,6 @@ static void __devexit iwl3945_pci_remove(struct pci_dev *pdev) ieee80211_free_hw(priv->hw); } -#ifdef CONFIG_PM - -static int iwl3945_pci_suspend(struct pci_dev *pdev, pm_message_t state) -{ - struct iwl_priv *priv = pci_get_drvdata(pdev); - - if (priv->is_open) { - set_bit(STATUS_IN_SUSPEND, &priv->status); - iwl3945_mac_stop(priv->hw); - priv->is_open = 1; - } - - pci_save_state(pdev); - pci_disable_device(pdev); - pci_set_power_state(pdev, PCI_D3hot); - - return 0; -} - -static int iwl3945_pci_resume(struct pci_dev *pdev) -{ - struct iwl_priv *priv = pci_get_drvdata(pdev); - int ret; - - pci_set_power_state(pdev, PCI_D0); - ret = pci_enable_device(pdev); - if (ret) - return ret; - pci_restore_state(pdev); - - if (priv->is_open) - iwl3945_mac_start(priv->hw); - - clear_bit(STATUS_IN_SUSPEND, &priv->status); - return 0; -} - -#endif /* CONFIG_PM */ /***************************************************************************** * @@ -5282,8 +5235,8 @@ static struct pci_driver iwl3945_driver = { .probe = iwl3945_pci_probe, .remove = __devexit_p(iwl3945_pci_remove), #ifdef CONFIG_PM - .suspend = iwl3945_pci_suspend, - .resume = iwl3945_pci_resume, + .suspend = iwl_pci_suspend, + .resume = iwl_pci_resume, #endif }; -- 1.5.6.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 17:14 ` [PATCH 1/3] iwlwifi: merge and better support of suspend/resume for iwlagn and iwl3945 Reinette Chatre @ 2009-03-26 17:14 ` Reinette Chatre 2009-03-26 17:14 ` [PATCH 3/3] iwl3945: use iwl_mac_conf_tx Reinette Chatre ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Reinette Chatre @ 2009-03-26 17:14 UTC (permalink / raw) To: linville; +Cc: linux-wireless, ipw3945-devel, Wey-Yi Guy, Reinette Chatre From: Wey-Yi Guy <wey-yi.w.guy@intel.com> Remove STATUS_ALIVE checking when HW RF KILL disabled, the bit get clear in __iwl_down() function; the additional checking will fail and cause RF can not be turn back on. Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> --- drivers/net/wireless/iwlwifi/iwl-core.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c index e1b5472..7560d6f 100644 --- a/drivers/net/wireless/iwlwifi/iwl-core.c +++ b/drivers/net/wireless/iwlwifi/iwl-core.c @@ -2056,8 +2056,7 @@ void iwl_bg_rf_kill(struct work_struct *work) IWL_DEBUG_RF_KILL(priv, "HW and/or SW RF Kill no longer active, restarting " "device\n"); - if (!test_bit(STATUS_EXIT_PENDING, &priv->status) && - test_bit(STATUS_ALIVE, &priv->status)) + if (!test_bit(STATUS_EXIT_PENDING, &priv->status)) queue_work(priv->workqueue, &priv->restart); } else { /* make sure mac80211 stop sending Tx frame */ -- 1.5.6.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* [PATCH 3/3] iwl3945: use iwl_mac_conf_tx 2009-03-26 17:14 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Reinette Chatre @ 2009-03-26 17:14 ` Reinette Chatre 2009-03-26 17:50 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Helmut Schaa 2009-03-27 6:18 ` Helmut Schaa 2 siblings, 0 replies; 19+ messages in thread From: Reinette Chatre @ 2009-03-26 17:14 UTC (permalink / raw) To: linville; +Cc: linux-wireless, ipw3945-devel, Abhijeet Kolekar, Reinette Chatre From: Abhijeet Kolekar <abhijeet.kolekar@intel.com> 3945 now uses iwl_mac_conf_tx. Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> --- drivers/net/wireless/iwlwifi/iwl-agn.c | 43 ------------------------ drivers/net/wireless/iwlwifi/iwl-core.c | 43 ++++++++++++++++++++++++ drivers/net/wireless/iwlwifi/iwl-core.h | 2 + drivers/net/wireless/iwlwifi/iwl3945-base.c | 48 +-------------------------- 4 files changed, 46 insertions(+), 90 deletions(-) diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c index 669b435..51f6a01 100644 --- a/drivers/net/wireless/iwlwifi/iwl-agn.c +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c @@ -2628,49 +2628,6 @@ static int iwl_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd, return ret; } -static int iwl_mac_conf_tx(struct ieee80211_hw *hw, u16 queue, - const struct ieee80211_tx_queue_params *params) -{ - struct iwl_priv *priv = hw->priv; - unsigned long flags; - int q; - - IWL_DEBUG_MAC80211(priv, "enter\n"); - - if (!iwl_is_ready_rf(priv)) { - IWL_DEBUG_MAC80211(priv, "leave - RF not ready\n"); - return -EIO; - } - - if (queue >= AC_NUM) { - IWL_DEBUG_MAC80211(priv, "leave - queue >= AC_NUM %d\n", queue); - return 0; - } - - q = AC_NUM - 1 - queue; - - spin_lock_irqsave(&priv->lock, flags); - - priv->qos_data.def_qos_parm.ac[q].cw_min = cpu_to_le16(params->cw_min); - priv->qos_data.def_qos_parm.ac[q].cw_max = cpu_to_le16(params->cw_max); - priv->qos_data.def_qos_parm.ac[q].aifsn = params->aifs; - priv->qos_data.def_qos_parm.ac[q].edca_txop = - cpu_to_le16((params->txop * 32)); - - priv->qos_data.def_qos_parm.ac[q].reserved1 = 0; - priv->qos_data.qos_active = 1; - - if (priv->iw_mode == NL80211_IFTYPE_AP) - iwl_activate_qos(priv, 1); - else if (priv->assoc_id && iwl_is_associated(priv)) - iwl_activate_qos(priv, 0); - - spin_unlock_irqrestore(&priv->lock, flags); - - IWL_DEBUG_MAC80211(priv, "leave\n"); - return 0; -} - static int iwl_mac_ampdu_action(struct ieee80211_hw *hw, enum ieee80211_ampdu_mlme_action action, struct ieee80211_sta *sta, u16 tid, u16 *ssn) diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c index 7560d6f..02fa5c7 100644 --- a/drivers/net/wireless/iwlwifi/iwl-core.c +++ b/drivers/net/wireless/iwlwifi/iwl-core.c @@ -2114,6 +2114,49 @@ void iwl_rx_reply_error(struct iwl_priv *priv, } EXPORT_SYMBOL(iwl_rx_reply_error); +int iwl_mac_conf_tx(struct ieee80211_hw *hw, u16 queue, + const struct ieee80211_tx_queue_params *params) +{ + struct iwl_priv *priv = hw->priv; + unsigned long flags; + int q; + + IWL_DEBUG_MAC80211(priv, "enter\n"); + + if (!iwl_is_ready_rf(priv)) { + IWL_DEBUG_MAC80211(priv, "leave - RF not ready\n"); + return -EIO; + } + + if (queue >= AC_NUM) { + IWL_DEBUG_MAC80211(priv, "leave - queue >= AC_NUM %d\n", queue); + return 0; + } + + q = AC_NUM - 1 - queue; + + spin_lock_irqsave(&priv->lock, flags); + + priv->qos_data.def_qos_parm.ac[q].cw_min = cpu_to_le16(params->cw_min); + priv->qos_data.def_qos_parm.ac[q].cw_max = cpu_to_le16(params->cw_max); + priv->qos_data.def_qos_parm.ac[q].aifsn = params->aifs; + priv->qos_data.def_qos_parm.ac[q].edca_txop = + cpu_to_le16((params->txop * 32)); + + priv->qos_data.def_qos_parm.ac[q].reserved1 = 0; + priv->qos_data.qos_active = 1; + + if (priv->iw_mode == NL80211_IFTYPE_AP) + iwl_activate_qos(priv, 1); + else if (priv->assoc_id && iwl_is_associated(priv)) + iwl_activate_qos(priv, 0); + + spin_unlock_irqrestore(&priv->lock, flags); + + IWL_DEBUG_MAC80211(priv, "leave\n"); + return 0; +} +EXPORT_SYMBOL(iwl_mac_conf_tx); #ifdef CONFIG_PM int iwl_pci_suspend(struct pci_dev *pdev, pm_message_t state) diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h index 5d49045..d56edcd 100644 --- a/drivers/net/wireless/iwlwifi/iwl-core.h +++ b/drivers/net/wireless/iwlwifi/iwl-core.h @@ -225,6 +225,8 @@ struct ieee80211_hw *iwl_alloc_all(struct iwl_cfg *cfg, void iwl_hw_detect(struct iwl_priv *priv); void iwl_reset_qos(struct iwl_priv *priv); void iwl_activate_qos(struct iwl_priv *priv, u8 force); +int iwl_mac_conf_tx(struct ieee80211_hw *hw, u16 queue, + const struct ieee80211_tx_queue_params *params); void iwl_set_rxon_hwcrypto(struct iwl_priv *priv, int hw_decrypt); int iwl_check_rxon_cmd(struct iwl_priv *priv); int iwl_full_rxon_required(struct iwl_priv *priv); diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c index 33f5b00..c29189b 100644 --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c @@ -4100,52 +4100,6 @@ static int iwl3945_mac_set_key(struct ieee80211_hw *hw, enum set_key_cmd cmd, return ret; } -static int iwl3945_mac_conf_tx(struct ieee80211_hw *hw, u16 queue, - const struct ieee80211_tx_queue_params *params) -{ - struct iwl_priv *priv = hw->priv; - unsigned long flags; - int q; - - IWL_DEBUG_MAC80211(priv, "enter\n"); - - if (!iwl_is_ready_rf(priv)) { - IWL_DEBUG_MAC80211(priv, "leave - RF not ready\n"); - return -EIO; - } - - if (queue >= AC_NUM) { - IWL_DEBUG_MAC80211(priv, "leave - queue >= AC_NUM %d\n", queue); - return 0; - } - - q = AC_NUM - 1 - queue; - - spin_lock_irqsave(&priv->lock, flags); - - priv->qos_data.def_qos_parm.ac[q].cw_min = cpu_to_le16(params->cw_min); - priv->qos_data.def_qos_parm.ac[q].cw_max = cpu_to_le16(params->cw_max); - priv->qos_data.def_qos_parm.ac[q].aifsn = params->aifs; - priv->qos_data.def_qos_parm.ac[q].edca_txop = - cpu_to_le16((params->txop * 32)); - - priv->qos_data.def_qos_parm.ac[q].reserved1 = 0; - priv->qos_data.qos_active = 1; - - spin_unlock_irqrestore(&priv->lock, flags); - - mutex_lock(&priv->mutex); - if (priv->iw_mode == NL80211_IFTYPE_AP) - iwl_activate_qos(priv, 1); - else if (priv->assoc_id && iwl_is_associated(priv)) - iwl_activate_qos(priv, 0); - - mutex_unlock(&priv->mutex); - - IWL_DEBUG_MAC80211(priv, "leave\n"); - return 0; -} - static int iwl3945_mac_get_tx_stats(struct ieee80211_hw *hw, struct ieee80211_tx_queue_stats *stats) { @@ -4809,7 +4763,7 @@ static struct ieee80211_ops iwl3945_hw_ops = { .configure_filter = iwl_configure_filter, .set_key = iwl3945_mac_set_key, .get_tx_stats = iwl3945_mac_get_tx_stats, - .conf_tx = iwl3945_mac_conf_tx, + .conf_tx = iwl_mac_conf_tx, .reset_tsf = iwl3945_mac_reset_tsf, .bss_info_changed = iwl3945_bss_info_changed, .hw_scan = iwl_mac_hw_scan -- 1.5.6.3 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 17:14 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Reinette Chatre 2009-03-26 17:14 ` [PATCH 3/3] iwl3945: use iwl_mac_conf_tx Reinette Chatre @ 2009-03-26 17:50 ` Helmut Schaa 2009-03-26 18:22 ` reinette chatre 2009-03-27 6:18 ` Helmut Schaa 2 siblings, 1 reply; 19+ messages in thread From: Helmut Schaa @ 2009-03-26 17:50 UTC (permalink / raw) To: Reinette Chatre; +Cc: linville, linux-wireless, ipw3945-devel, Wey-Yi Guy Hi, Am Donnerstag, 26. M=E4rz 2009 schrieb Reinette Chatre: > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> >=20 > Remove STATUS_ALIVE checking when HW RF KILL disabled, the bit get > clear in __iwl_down() function; the additional checking will fail and > cause RF can not be turn back on. Are you sure this is needed? I'd argue we should only restart the adapt= er if it was alive when it got rf_killed. In case the adapter was rf_kille= d while the interface was down I don't think we want to restart the adapt= er immediately but first when the interface is taken up again. Helmut > Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com> > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com> > --- > drivers/net/wireless/iwlwifi/iwl-core.c | 3 +-- > 1 files changed, 1 insertions(+), 2 deletions(-) >=20 > diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wi= reless/iwlwifi/iwl-core.c > index e1b5472..7560d6f 100644 > --- a/drivers/net/wireless/iwlwifi/iwl-core.c > +++ b/drivers/net/wireless/iwlwifi/iwl-core.c > @@ -2056,8 +2056,7 @@ void iwl_bg_rf_kill(struct work_struct *work) > IWL_DEBUG_RF_KILL(priv, > "HW and/or SW RF Kill no longer active, restarting " > "device\n"); > - if (!test_bit(STATUS_EXIT_PENDING, &priv->status) && > - test_bit(STATUS_ALIVE, &priv->status)) > + if (!test_bit(STATUS_EXIT_PENDING, &priv->status)) > queue_work(priv->workqueue, &priv->restart); > } else { > /* make sure mac80211 stop sending Tx frame */ -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 17:50 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Helmut Schaa @ 2009-03-26 18:22 ` reinette chatre 2009-03-26 19:11 ` Helmut Schaa 0 siblings, 1 reply; 19+ messages in thread From: reinette chatre @ 2009-03-26 18:22 UTC (permalink / raw) To: Helmut Schaa Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W On Thu, 2009-03-26 at 10:50 -0700, Helmut Schaa wrote: > Hi, >=20 > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre: > > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> > >=20 > > Remove STATUS_ALIVE checking when HW RF KILL disabled, the bit get > > clear in __iwl_down() function; the additional checking will fail a= nd > > cause RF can not be turn back on. >=20 > Are you sure this is needed? I'd argue we should only restart the ada= pter > if it was alive when it got rf_killed. In case the adapter was rf_kil= led > while the interface was down I don't think we want to restart the ada= pter > immediately but first when the interface is taken up again. We also need to consider if a suspend/resume happens in the middle. Without the patch, if you enable rfkill, suspend, resume, disable rfkill, then your interface cannot be brought up. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 18:22 ` reinette chatre @ 2009-03-26 19:11 ` Helmut Schaa 2009-03-26 19:30 ` reinette chatre 0 siblings, 1 reply; 19+ messages in thread From: Helmut Schaa @ 2009-03-26 19:11 UTC (permalink / raw) To: reinette chatre Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > On Thu, 2009-03-26 at 10:50 -0700, Helmut Schaa wrote: > > Hi, > >=20 > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre: > > > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> > > >=20 > > > Remove STATUS_ALIVE checking when HW RF KILL disabled, the bit ge= t > > > clear in __iwl_down() function; the additional checking will fail= and > > > cause RF can not be turn back on. > >=20 > > Are you sure this is needed? I'd argue we should only restart the a= dapter > > if it was alive when it got rf_killed. In case the adapter was rf_k= illed > > while the interface was down I don't think we want to restart the a= dapter > > immediately but first when the interface is taken up again. >=20 > We also need to consider if a suspend/resume happens in the middle. > Without the patch, if you enable rfkill, suspend, resume, disable > rfkill, then your interface cannot be brought up. I guess you refer to the situation where the interface is up, right? Something like: - ifconfig wlan0 up - press killswitch (kill wireless) - suspend - resume - press killswitch (enable wireless) - here the interface should still be up As the interface is/was up, mac80211's resume handler should restart th= e adapter and thus we wouldn't need to restart the adapter in the rfkill-handler, or did I miss anything? Thanks, Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 19:11 ` Helmut Schaa @ 2009-03-26 19:30 ` reinette chatre 2009-03-26 19:49 ` Helmut Schaa 0 siblings, 1 reply; 19+ messages in thread From: reinette chatre @ 2009-03-26 19:30 UTC (permalink / raw) To: Helmut Schaa Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W On Thu, 2009-03-26 at 12:11 -0700, Helmut Schaa wrote: > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > On Thu, 2009-03-26 at 10:50 -0700, Helmut Schaa wrote: > > > Hi, > > >=20 > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre: > > > > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> > > > >=20 > > > > Remove STATUS_ALIVE checking when HW RF KILL disabled, the bit = get > > > > clear in __iwl_down() function; the additional checking will fa= il and > > > > cause RF can not be turn back on. > > >=20 > > > Are you sure this is needed? I'd argue we should only restart the= adapter > > > if it was alive when it got rf_killed. In case the adapter was rf= _killed > > > while the interface was down I don't think we want to restart the= adapter > > > immediately but first when the interface is taken up again. > >=20 > > We also need to consider if a suspend/resume happens in the middle. > > Without the patch, if you enable rfkill, suspend, resume, disable > > rfkill, then your interface cannot be brought up. >=20 > I guess you refer to the situation where the interface is up, right? > Something like: >=20 > - ifconfig wlan0 up > - press killswitch (kill wireless) > - suspend > - resume > - press killswitch (enable wireless) > - here the interface should still be up >=20 > As the interface is/was up, mac80211's resume handler should restart = the > adapter and thus we wouldn't need to restart the adapter in the > rfkill-handler, or did I miss anything? Yes, the resume handler will start the adapter (call "start"), but the actions done by it will exit early because of rfkill being enabled. The STATUS_ALIVE bit will thus not be set after this is completed. Later, when user disables rfkill, we want to restart the adapter to get all this corrected, but this call currently fails because of this check. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 19:30 ` reinette chatre @ 2009-03-26 19:49 ` Helmut Schaa 2009-03-26 20:37 ` reinette chatre 0 siblings, 1 reply; 19+ messages in thread From: Helmut Schaa @ 2009-03-26 19:49 UTC (permalink / raw) To: reinette chatre Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > On Thu, 2009-03-26 at 12:11 -0700, Helmut Schaa wrote: > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > > On Thu, 2009-03-26 at 10:50 -0700, Helmut Schaa wrote: > > > > Hi, > > > >=20 > > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre: > > > > > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> > > > > >=20 > > > > > Remove STATUS_ALIVE checking when HW RF KILL disabled, the bi= t get > > > > > clear in __iwl_down() function; the additional checking will = fail and > > > > > cause RF can not be turn back on. > > > >=20 > > > > Are you sure this is needed? I'd argue we should only restart t= he adapter > > > > if it was alive when it got rf_killed. In case the adapter was = rf_killed > > > > while the interface was down I don't think we want to restart t= he adapter > > > > immediately but first when the interface is taken up again. > > >=20 > > > We also need to consider if a suspend/resume happens in the middl= e. > > > Without the patch, if you enable rfkill, suspend, resume, disable > > > rfkill, then your interface cannot be brought up. > >=20 > > I guess you refer to the situation where the interface is up, right= ? > > Something like: > >=20 > > - ifconfig wlan0 up > > - press killswitch (kill wireless) > > - suspend > > - resume > > - press killswitch (enable wireless) > > - here the interface should still be up > >=20 > > As the interface is/was up, mac80211's resume handler should restar= t the > > adapter and thus we wouldn't need to restart the adapter in the > > rfkill-handler, or did I miss anything? >=20 > Yes, the resume handler will start the adapter (call "start"), but th= e > actions done by it will exit early because of rfkill being enabled. T= he > STATUS_ALIVE bit will thus not be set after this is completed. Later, > when user disables rfkill, we want to restart the adapter to get all > this corrected, but this call currently fails because of this check. Got it, thanks for the explanation. Nevertheless, removing the check will result in restarting the adapter = even if the interface is down. So, I agree that we have a problem here but I= do not agree with the solution ;) Maybe taking the interface up (not only pseudo-up, as done currently) s= hould be allowed even if wireless is killed? We already allow the interface t= o stay up when the adapter gets rfkilled. Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 19:49 ` Helmut Schaa @ 2009-03-26 20:37 ` reinette chatre 2009-03-26 20:58 ` Helmut Schaa 0 siblings, 1 reply; 19+ messages in thread From: reinette chatre @ 2009-03-26 20:37 UTC (permalink / raw) To: Helmut Schaa Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W On Thu, 2009-03-26 at 12:49 -0700, Helmut Schaa wrote: > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > On Thu, 2009-03-26 at 12:11 -0700, Helmut Schaa wrote: > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > > > On Thu, 2009-03-26 at 10:50 -0700, Helmut Schaa wrote: > > > > > Hi, > > > > >=20 > > > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre: > > > > > > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> > > > > > >=20 > > > > > > Remove STATUS_ALIVE checking when HW RF KILL disabled, the = bit get > > > > > > clear in __iwl_down() function; the additional checking wil= l fail and > > > > > > cause RF can not be turn back on. > > > > >=20 > > > > > Are you sure this is needed? I'd argue we should only restart= the adapter > > > > > if it was alive when it got rf_killed. In case the adapter wa= s rf_killed > > > > > while the interface was down I don't think we want to restart= the adapter > > > > > immediately but first when the interface is taken up again. > > > >=20 > > > > We also need to consider if a suspend/resume happens in the mid= dle. > > > > Without the patch, if you enable rfkill, suspend, resume, disab= le > > > > rfkill, then your interface cannot be brought up. > > >=20 > > > I guess you refer to the situation where the interface is up, rig= ht? > > > Something like: > > >=20 > > > - ifconfig wlan0 up > > > - press killswitch (kill wireless) > > > - suspend > > > - resume > > > - press killswitch (enable wireless) > > > - here the interface should still be up > > >=20 > > > As the interface is/was up, mac80211's resume handler should rest= art the > > > adapter and thus we wouldn't need to restart the adapter in the > > > rfkill-handler, or did I miss anything? > >=20 > > Yes, the resume handler will start the adapter (call "start"), but = the > > actions done by it will exit early because of rfkill being enabled.= The > > STATUS_ALIVE bit will thus not be set after this is completed. Late= r, > > when user disables rfkill, we want to restart the adapter to get al= l > > this corrected, but this call currently fails because of this check= =2E >=20 > Got it, thanks for the explanation. >=20 > Nevertheless, removing the check will result in restarting the adapte= r even > if the interface is down. So, I agree that we have a problem here but= I do > not agree with the solution ;) I agree that it is not efficient ... but it seems harmless. > Maybe taking the interface up (not only pseudo-up, as done currently)= should > be allowed even if wireless is killed? We already allow the interface= to > stay up when the adapter gets rfkilled. Knowing that rfkill is enabled enables us to save power by not bringing everything up. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 20:37 ` reinette chatre @ 2009-03-26 20:58 ` Helmut Schaa 2009-03-26 23:26 ` reinette chatre 2009-03-27 3:49 ` reinette chatre 0 siblings, 2 replies; 19+ messages in thread From: Helmut Schaa @ 2009-03-26 20:58 UTC (permalink / raw) To: reinette chatre Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > On Thu, 2009-03-26 at 12:49 -0700, Helmut Schaa wrote: > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > > On Thu, 2009-03-26 at 12:11 -0700, Helmut Schaa wrote: > > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > > > > On Thu, 2009-03-26 at 10:50 -0700, Helmut Schaa wrote: > > > > > > Hi, > > > > > >=20 > > > > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre: > > > > > > > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> > > > > > > >=20 > > > > > > > Remove STATUS_ALIVE checking when HW RF KILL disabled, th= e bit get > > > > > > > clear in __iwl_down() function; the additional checking w= ill fail and > > > > > > > cause RF can not be turn back on. > > > > > >=20 > > > > > > Are you sure this is needed? I'd argue we should only resta= rt the adapter > > > > > > if it was alive when it got rf_killed. In case the adapter = was rf_killed > > > > > > while the interface was down I don't think we want to resta= rt the adapter > > > > > > immediately but first when the interface is taken up again. > > > > >=20 > > > > > We also need to consider if a suspend/resume happens in the m= iddle. > > > > > Without the patch, if you enable rfkill, suspend, resume, dis= able > > > > > rfkill, then your interface cannot be brought up. > > > >=20 > > > > I guess you refer to the situation where the interface is up, r= ight? > > > > Something like: > > > >=20 > > > > - ifconfig wlan0 up > > > > - press killswitch (kill wireless) > > > > - suspend > > > > - resume > > > > - press killswitch (enable wireless) > > > > - here the interface should still be up > > > >=20 > > > > As the interface is/was up, mac80211's resume handler should re= start the > > > > adapter and thus we wouldn't need to restart the adapter in the > > > > rfkill-handler, or did I miss anything? > > >=20 > > > Yes, the resume handler will start the adapter (call "start"), bu= t the > > > actions done by it will exit early because of rfkill being enable= d. The > > > STATUS_ALIVE bit will thus not be set after this is completed. La= ter, > > > when user disables rfkill, we want to restart the adapter to get = all > > > this corrected, but this call currently fails because of this che= ck. > >=20 > > Got it, thanks for the explanation. > >=20 > > Nevertheless, removing the check will result in restarting the adap= ter even > > if the interface is down. So, I agree that we have a problem here b= ut I do > > not agree with the solution ;) >=20 > I agree that it is not efficient ... but it seems harmless. Assume the following situation: You only have one killswitch for both, wireless and bluetooth. The wire= less interface is down because it is unused and the user wants to use blueto= oth and enables it via the killswitch which also means that wireless gets unkilled. Now restarting the adapter needs more power then keeping the adapter down. In short: if the interface is down the user (space) does = not want to use the interface and probably wants to save power as well. > > Maybe taking the interface up (not only pseudo-up, as done currentl= y) should > > be allowed even if wireless is killed? We already allow the interfa= ce to > > stay up when the adapter gets rfkilled. >=20 > Knowing that rfkill is enabled enables us to save power by not bringi= ng > everything up. Yes, but user (space) knows that the wireless card consumes more power = if the interface is up. And if you use NM for example it will just take the in= terface down when the killswitch gets activated. Another solution would be to 1) not allow the interface to go up when it is rfkilled, and 2) take the interface down when it gets rfkilled However, I'm not sure if 2 is possible without help of mac80211 and if = it makes sense to change an interface state from within the driver. Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 20:58 ` Helmut Schaa @ 2009-03-26 23:26 ` reinette chatre 2009-03-27 6:19 ` Helmut Schaa 2009-03-27 6:25 ` Helmut Schaa 2009-03-27 3:49 ` reinette chatre 1 sibling, 2 replies; 19+ messages in thread From: reinette chatre @ 2009-03-26 23:26 UTC (permalink / raw) To: Helmut Schaa Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Hi Helmut, On Thu, 2009-03-26 at 13:58 -0700, Helmut Schaa wrote: > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > On Thu, 2009-03-26 at 12:49 -0700, Helmut Schaa wrote: > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > > > On Thu, 2009-03-26 at 12:11 -0700, Helmut Schaa wrote: > > > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > > > > > On Thu, 2009-03-26 at 10:50 -0700, Helmut Schaa wrote: > > > > > > > Hi, > > > > > > >=20 > > > > > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre= : > > > > > > > > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> > > > > > > > >=20 > > > > > > > > Remove STATUS_ALIVE checking when HW RF KILL disabled, = the bit get > > > > > > > > clear in __iwl_down() function; the additional checking= will fail and > > > > > > > > cause RF can not be turn back on. > > > > > > >=20 > > > > > > > Are you sure this is needed? I'd argue we should only res= tart the adapter > > > > > > > if it was alive when it got rf_killed. In case the adapte= r was rf_killed > > > > > > > while the interface was down I don't think we want to res= tart the adapter > > > > > > > immediately but first when the interface is taken up agai= n. > > > > > >=20 > > > > > > We also need to consider if a suspend/resume happens in the= middle. > > > > > > Without the patch, if you enable rfkill, suspend, resume, d= isable > > > > > > rfkill, then your interface cannot be brought up. > > > > >=20 > > > > > I guess you refer to the situation where the interface is up,= right? > > > > > Something like: > > > > >=20 > > > > > - ifconfig wlan0 up > > > > > - press killswitch (kill wireless) > > > > > - suspend > > > > > - resume > > > > > - press killswitch (enable wireless) > > > > > - here the interface should still be up > > > > >=20 > > > > > As the interface is/was up, mac80211's resume handler should = restart the > > > > > adapter and thus we wouldn't need to restart the adapter in t= he > > > > > rfkill-handler, or did I miss anything? > > > >=20 > > > > Yes, the resume handler will start the adapter (call "start"), = but the > > > > actions done by it will exit early because of rfkill being enab= led. The > > > > STATUS_ALIVE bit will thus not be set after this is completed. = Later, > > > > when user disables rfkill, we want to restart the adapter to ge= t all > > > > this corrected, but this call currently fails because of this c= heck. > > >=20 > > > Got it, thanks for the explanation. > > >=20 > > > Nevertheless, removing the check will result in restarting the ad= apter even > > > if the interface is down. So, I agree that we have a problem here= but I do > > > not agree with the solution ;) > >=20 > > I agree that it is not efficient ... but it seems harmless. >=20 > Assume the following situation: >=20 > You only have one killswitch for both, wireless and bluetooth. The wi= reless > interface is down because it is unused and the user wants to use blue= tooth > and enables it via the killswitch which also means that wireless gets > unkilled. Now restarting the adapter needs more power then keeping th= e > adapter down. In short: if the interface is down the user (space) doe= s not > want to use the interface and probably wants to save power as well. Your measurements are very fine grained :) Will this be measureable? Right now things do not work and this patch fixes it.=20 > > > Maybe taking the interface up (not only pseudo-up, as done curren= tly) should > > > be allowed even if wireless is killed? We already allow the inter= face to > > > stay up when the adapter gets rfkilled. > >=20 > > Knowing that rfkill is enabled enables us to save power by not brin= ging > > everything up. >=20 > Yes, but user (space) knows that the wireless card consumes more powe= r if the > interface is up. And if you use NM for example it will just take the = interface > down when the killswitch gets activated. > Another solution would be to > 1) not allow the interface to go up when it is rfkilled, and We currently do this. > 2) take the interface down when it gets rfkilled >=20 > However, I'm not sure if 2 is possible without help of mac80211 and i= f it > makes sense to change an interface state from within the driver. Like you say ... probably not the driver's place to do this. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 23:26 ` reinette chatre @ 2009-03-27 6:19 ` Helmut Schaa 2009-03-27 6:25 ` Helmut Schaa 1 sibling, 0 replies; 19+ messages in thread From: Helmut Schaa @ 2009-03-27 6:19 UTC (permalink / raw) To: reinette chatre Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Am Freitag, 27. M=C3=A4rz 2009 schrieb reinette chatre: > On Thu, 2009-03-26 at 13:58 -0700, Helmut Schaa wrote: > > 2) take the interface down when it gets rfkilled > >=20 > > However, I'm not sure if 2 is possible without help of mac80211 and= if it > > makes sense to change an interface state from within the driver. >=20 > Like you say ... probably not the driver's place to do this. Yeah, just ignore that idea ;) Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 23:26 ` reinette chatre 2009-03-27 6:19 ` Helmut Schaa @ 2009-03-27 6:25 ` Helmut Schaa 2009-03-27 15:45 ` reinette chatre 1 sibling, 1 reply; 19+ messages in thread From: Helmut Schaa @ 2009-03-27 6:25 UTC (permalink / raw) To: reinette chatre Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Hi, Am Freitag, 27. M=C3=A4rz 2009 schrieb reinette chatre: > On Thu, 2009-03-26 at 13:58 -0700, Helmut Schaa wrote: > > Assume the following situation: > >=20 > > You only have one killswitch for both, wireless and bluetooth. The = wireless > > interface is down because it is unused and the user wants to use bl= uetooth > > and enables it via the killswitch which also means that wireless ge= ts > > unkilled. Now restarting the adapter needs more power then keeping = the > > adapter down. In short: if the interface is down the user (space) d= oes not > > want to use the interface and probably wants to save power as well. >=20 > Your measurements are very fine grained :) Will this be measureable? Heh, you know the internals of the devive ;) but I guess waking up the adapter (and thus starting the firmware) even if the interface is unuse= d will surely need more power than not doing so. Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-27 6:25 ` Helmut Schaa @ 2009-03-27 15:45 ` reinette chatre 0 siblings, 0 replies; 19+ messages in thread From: reinette chatre @ 2009-03-27 15:45 UTC (permalink / raw) To: Helmut Schaa Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Hi Helmut, On Thu, 2009-03-26 at 23:25 -0700, Helmut Schaa wrote: > Hi, >=20 > Am Freitag, 27. M=C3=A4rz 2009 schrieb reinette chatre: > > On Thu, 2009-03-26 at 13:58 -0700, Helmut Schaa wrote: > > > Assume the following situation: > > >=20 > > > You only have one killswitch for both, wireless and bluetooth. Th= e wireless > > > interface is down because it is unused and the user wants to use = bluetooth > > > and enables it via the killswitch which also means that wireless = gets > > > unkilled. Now restarting the adapter needs more power then keepin= g the > > > adapter down. In short: if the interface is down the user (space)= does not > > > want to use the interface and probably wants to save power as wel= l. > >=20 > > Your measurements are very fine grained :) Will this be measureable= ? >=20 > Heh, you know the internals of the devive ;) but I guess waking up th= e > adapter (and thus starting the firmware) even if the interface is unu= sed > will surely need more power than not doing so. If interface is down then mac will not ask us to start the interface in the first place. If interface is up and mac asks us to start interface then the driver will do so. In this case, if rfkill is enabled, then th= e driver will not do much ... definitely not load firmware and thus start using up power. Only when the user changes rfkill state will we actuall= y get the firmware going. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 20:58 ` Helmut Schaa 2009-03-26 23:26 ` reinette chatre @ 2009-03-27 3:49 ` reinette chatre 1 sibling, 0 replies; 19+ messages in thread From: reinette chatre @ 2009-03-27 3:49 UTC (permalink / raw) To: Helmut Schaa Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Hi Helmut, On Thu, 2009-03-26 at 13:58 -0700, Helmut Schaa wrote: > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > On Thu, 2009-03-26 at 12:49 -0700, Helmut Schaa wrote: > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > > > On Thu, 2009-03-26 at 12:11 -0700, Helmut Schaa wrote: > > > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb reinette chatre: > > > > > > On Thu, 2009-03-26 at 10:50 -0700, Helmut Schaa wrote: > > > > > > > Hi, > > > > > > >=20 > > > > > > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre= : > > > > > > > > From: Wey-Yi Guy <wey-yi.w.guy@intel.com> > > > > > > > >=20 > > > > > > > > Remove STATUS_ALIVE checking when HW RF KILL disabled, = the bit get > > > > > > > > clear in __iwl_down() function; the additional checking= will fail and > > > > > > > > cause RF can not be turn back on. > > > > > > >=20 > > > > > > > Are you sure this is needed? I'd argue we should only res= tart the adapter > > > > > > > if it was alive when it got rf_killed. In case the adapte= r was rf_killed > > > > > > > while the interface was down I don't think we want to res= tart the adapter > > > > > > > immediately but first when the interface is taken up agai= n. > > > > > >=20 > > > > > > We also need to consider if a suspend/resume happens in the= middle. > > > > > > Without the patch, if you enable rfkill, suspend, resume, d= isable > > > > > > rfkill, then your interface cannot be brought up. > > > > >=20 > > > > > I guess you refer to the situation where the interface is up,= right? > > > > > Something like: > > > > >=20 > > > > > - ifconfig wlan0 up > > > > > - press killswitch (kill wireless) > > > > > - suspend > > > > > - resume > > > > > - press killswitch (enable wireless) > > > > > - here the interface should still be up > > > > >=20 > > > > > As the interface is/was up, mac80211's resume handler should = restart the > > > > > adapter and thus we wouldn't need to restart the adapter in t= he > > > > > rfkill-handler, or did I miss anything? > > > >=20 > > > > Yes, the resume handler will start the adapter (call "start"), = but the > > > > actions done by it will exit early because of rfkill being enab= led. The > > > > STATUS_ALIVE bit will thus not be set after this is completed. = Later, > > > > when user disables rfkill, we want to restart the adapter to ge= t all > > > > this corrected, but this call currently fails because of this c= heck. > > >=20 > > > Got it, thanks for the explanation. > > >=20 > > > Nevertheless, removing the check will result in restarting the ad= apter even > > > if the interface is down. So, I agree that we have a problem here= but I do > > > not agree with the solution ;) > >=20 > > I agree that it is not efficient ... but it seems harmless. >=20 > Assume the following situation: >=20 > You only have one killswitch for both, wireless and bluetooth. The wi= reless > interface is down because it is unused and the user wants to use blue= tooth > and enables it via the killswitch which also means that wireless gets > unkilled. Now restarting the adapter needs more power then keeping th= e > adapter down. In short: if the interface is down the user (space) doe= s not > want to use the interface and probably wants to save power as well. >=20 > > > Maybe taking the interface up (not only pseudo-up, as done curren= tly) should > > > be allowed even if wireless is killed? We already allow the inter= face to > > > stay up when the adapter gets rfkilled. > >=20 > > Knowing that rfkill is enabled enables us to save power by not brin= ging > > everything up. >=20 > Yes, but user (space) knows that the wireless card consumes more powe= r if the > interface is up. And if you use NM for example it will just take the = interface > down when the killswitch gets activated. >=20 > Another solution would be to > 1) not allow the interface to go up when it is rfkilled, and > 2) take the interface down when it gets rfkilled >=20 > However, I'm not sure if 2 is possible without help of mac80211 and i= f it > makes sense to change an interface state from within the driver. I was just thinking that some of this thread can be interpreted incorrectly. There is a problem with shared information here ... mac80211 is what brings the interface up and down, yet it is the driver that has the knowledge about rfkill state. There has been talk about having rfkill support in mac80211, but it is not there at this time. Until it is, the driver needs to take care that interface is not fully up (firmware running full throttle) if rfkill enabled, yet let mac80211 think that interface up was successful. Then, when rfkill is disabled w= e can complete interface bringup. Reinette=20 -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-26 17:14 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Reinette Chatre 2009-03-26 17:14 ` [PATCH 3/3] iwl3945: use iwl_mac_conf_tx Reinette Chatre 2009-03-26 17:50 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Helmut Schaa @ 2009-03-27 6:18 ` Helmut Schaa 2009-03-27 16:03 ` reinette chatre 2 siblings, 1 reply; 19+ messages in thread From: Helmut Schaa @ 2009-03-27 6:18 UTC (permalink / raw) To: Reinette Chatre; +Cc: linville, linux-wireless, ipw3945-devel, Wey-Yi Guy Hi again, Am Donnerstag, 26. M=E4rz 2009 schrieb Reinette Chatre: > Remove STATUS_ALIVE checking when HW RF KILL disabled, the bit get > clear in __iwl_down() function; the additional checking will fail and > cause RF can not be turn back on. Maybe just checking for is_open instead of STATUS_ALIVE will fix the is= sue too? I only had a quick look but it looks possible. AFAICS is_open indi= cates if the interface was taken up by mac80211. So if the interface is up bu= t STATUS_ALIVE is not set we want to restart the adapter after unkilling = but if the interface is still down (is_open=3D=3D0) we do not want to resta= rt the adapter. Furthermore, is_open is also set in case the interface was tak= en up but wireless was rfkilled. Helmut -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-27 6:18 ` Helmut Schaa @ 2009-03-27 16:03 ` reinette chatre 2009-03-27 16:30 ` reinette chatre 0 siblings, 1 reply; 19+ messages in thread From: reinette chatre @ 2009-03-27 16:03 UTC (permalink / raw) To: Helmut Schaa Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W On Thu, 2009-03-26 at 23:18 -0700, Helmut Schaa wrote: > Hi again, >=20 > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre: > > Remove STATUS_ALIVE checking when HW RF KILL disabled, the bit get > > clear in __iwl_down() function; the additional checking will fail a= nd > > cause RF can not be turn back on. >=20 > Maybe just checking for is_open instead of STATUS_ALIVE will fix the = issue > too? I only had a quick look but it looks possible. AFAICS is_open in= dicates > if the interface was taken up by mac80211. So if the interface is up = but > STATUS_ALIVE is not set we want to restart the adapter after unkillin= g but > if the interface is still down (is_open=3D=3D0) we do not want to res= tart the > adapter. Furthermore, is_open is also set in case the interface was t= aken > up but wireless was rfkilled. Very good idea ... I'm looking into this. Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill 2009-03-27 16:03 ` reinette chatre @ 2009-03-27 16:30 ` reinette chatre 0 siblings, 0 replies; 19+ messages in thread From: reinette chatre @ 2009-03-27 16:30 UTC (permalink / raw) To: Helmut Schaa, linville@tuxdriver.com Cc: linux-wireless@vger.kernel.org, ipw3945-devel@lists.sourceforge.net, Guy, Wey-Yi W Hi John, On Fri, 2009-03-27 at 09:03 -0700, reinette chatre wrote: > On Thu, 2009-03-26 at 23:18 -0700, Helmut Schaa wrote: > > Hi again, > >=20 > > Am Donnerstag, 26. M=C3=A4rz 2009 schrieb Reinette Chatre: > > > Remove STATUS_ALIVE checking when HW RF KILL disabled, the bit ge= t > > > clear in __iwl_down() function; the additional checking will fail= and > > > cause RF can not be turn back on. > >=20 > > Maybe just checking for is_open instead of STATUS_ALIVE will fix th= e issue > > too? I only had a quick look but it looks possible. AFAICS is_open = indicates > > if the interface was taken up by mac80211. So if the interface is u= p but > > STATUS_ALIVE is not set we want to restart the adapter after unkill= ing but > > if the interface is still down (is_open=3D=3D0) we do not want to r= estart the > > adapter. Furthermore, is_open is also set in case the interface was= taken > > up but wireless was rfkilled. >=20 > Very good idea ... I'm looking into this. Could you please drop this patch? We will send a new fix for this problem using Helmut's suggestion. Thank you Reinette -- To unsubscribe from this list: send the line "unsubscribe linux-wireles= s" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2009-03-27 16:25 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-03-26 17:14 [PATCH 0/3] iwlwifi driver updates 03/26/2009 Reinette Chatre 2009-03-26 17:14 ` [PATCH 1/3] iwlwifi: merge and better support of suspend/resume for iwlagn and iwl3945 Reinette Chatre 2009-03-26 17:14 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Reinette Chatre 2009-03-26 17:14 ` [PATCH 3/3] iwl3945: use iwl_mac_conf_tx Reinette Chatre 2009-03-26 17:50 ` [PATCH 2/3] iwlwifi: remove STATUS_ALIVE checking from rf_kill Helmut Schaa 2009-03-26 18:22 ` reinette chatre 2009-03-26 19:11 ` Helmut Schaa 2009-03-26 19:30 ` reinette chatre 2009-03-26 19:49 ` Helmut Schaa 2009-03-26 20:37 ` reinette chatre 2009-03-26 20:58 ` Helmut Schaa 2009-03-26 23:26 ` reinette chatre 2009-03-27 6:19 ` Helmut Schaa 2009-03-27 6:25 ` Helmut Schaa 2009-03-27 15:45 ` reinette chatre 2009-03-27 3:49 ` reinette chatre 2009-03-27 6:18 ` Helmut Schaa 2009-03-27 16:03 ` reinette chatre 2009-03-27 16:30 ` reinette chatre
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).