Commit 3eecce52 authored by Johannes Berg's avatar Johannes Berg Committed by John W. Linville

iwlwifi: unify scan start checks

Rather than duplicating all the checks and even
in case of errors accepting the scan request
from mac80211, we can push the checks to the
caller and in all error cases reject the scan
request right away (rather than accepting and
then saying it was aborted).
Signed-off-by: default avatarJohannes Berg <johannes.berg@intel.com>
Signed-off-by: default avatarStanislaw Gruszka <sgruszka@redhat.com>
Acked-by: default avatarWey-Yi W Guy <wey-yi.w.guy@intel.com>
Signed-off-by: default avatarJohn W. Linville <linville@tuxdriver.com>
parent b5be3efc
...@@ -295,7 +295,7 @@ extern const struct iwl_channel_info *iwl3945_get_channel_info( ...@@ -295,7 +295,7 @@ extern const struct iwl_channel_info *iwl3945_get_channel_info(
extern int iwl3945_rs_next_rate(struct iwl_priv *priv, int rate); extern int iwl3945_rs_next_rate(struct iwl_priv *priv, int rate);
/* scanning */ /* scanning */
void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif); int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
/* Requires full declaration of iwl_priv before including */ /* Requires full declaration of iwl_priv before including */
#include "iwl-io.h" #include "iwl-io.h"
......
...@@ -1150,7 +1150,7 @@ static int iwl_get_channels_for_scan(struct iwl_priv *priv, ...@@ -1150,7 +1150,7 @@ static int iwl_get_channels_for_scan(struct iwl_priv *priv,
return added; return added;
} }
void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
{ {
struct iwl_host_cmd cmd = { struct iwl_host_cmd cmd = {
.id = REPLY_SCAN_CMD, .id = REPLY_SCAN_CMD,
...@@ -1170,57 +1170,20 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) ...@@ -1170,57 +1170,20 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
int chan_mod; int chan_mod;
u8 active_chains; u8 active_chains;
u8 scan_tx_antennas = priv->hw_params.valid_tx_ant; u8 scan_tx_antennas = priv->hw_params.valid_tx_ant;
int ret;
lockdep_assert_held(&priv->mutex);
if (vif) if (vif)
ctx = iwl_rxon_ctx_from_vif(vif); ctx = iwl_rxon_ctx_from_vif(vif);
cancel_delayed_work(&priv->scan_check);
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
}
/* Make sure the scan wasn't canceled before this queued work
* was given the chance to run... */
if (!test_bit(STATUS_SCANNING, &priv->status))
goto done;
/* This should never be called or scheduled if there is currently
* a scan active in the hardware. */
if (test_bit(STATUS_SCAN_HW, &priv->status)) {
IWL_DEBUG_INFO(priv, "Multiple concurrent scan requests in parallel. "
"Ignoring second request.\n");
goto done;
}
if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
IWL_DEBUG_SCAN(priv, "Aborting scan due to device shutdown\n");
goto done;
}
if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
IWL_DEBUG_HC(priv, "Scan request while abort pending. Queuing.\n");
goto done;
}
if (iwl_is_rfkill(priv)) {
IWL_DEBUG_HC(priv, "Aborting scan due to RF Kill activation\n");
goto done;
}
if (!test_bit(STATUS_READY, &priv->status)) {
IWL_DEBUG_HC(priv, "Scan request while uninitialized. Queuing.\n");
goto done;
}
if (!priv->scan_cmd) { if (!priv->scan_cmd) {
priv->scan_cmd = kmalloc(sizeof(struct iwl_scan_cmd) + priv->scan_cmd = kmalloc(sizeof(struct iwl_scan_cmd) +
IWL_MAX_SCAN_SIZE, GFP_KERNEL); IWL_MAX_SCAN_SIZE, GFP_KERNEL);
if (!priv->scan_cmd) { if (!priv->scan_cmd) {
IWL_DEBUG_SCAN(priv, IWL_DEBUG_SCAN(priv,
"fail to allocate memory for scan\n"); "fail to allocate memory for scan\n");
goto done; return -ENOMEM;
} }
} }
scan = priv->scan_cmd; scan = priv->scan_cmd;
...@@ -1327,8 +1290,8 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) ...@@ -1327,8 +1290,8 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
IWL_GOOD_CRC_TH_NEVER; IWL_GOOD_CRC_TH_NEVER;
break; break;
default: default:
IWL_WARN(priv, "Invalid scan band count\n"); IWL_WARN(priv, "Invalid scan band\n");
goto done; return -EIO;
} }
band = priv->scan_band; band = priv->scan_band;
...@@ -1408,7 +1371,7 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) ...@@ -1408,7 +1371,7 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
} }
if (scan->channel_count == 0) { if (scan->channel_count == 0) {
IWL_DEBUG_SCAN(priv, "channel count %d\n", scan->channel_count); IWL_DEBUG_SCAN(priv, "channel count %d\n", scan->channel_count);
goto done; return -EIO;
} }
cmd.len += le16_to_cpu(scan->tx_cmd.len) + cmd.len += le16_to_cpu(scan->tx_cmd.len) +
...@@ -1416,30 +1379,21 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) ...@@ -1416,30 +1379,21 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
cmd.data = scan; cmd.data = scan;
scan->len = cpu_to_le16(cmd.len); scan->len = cpu_to_le16(cmd.len);
set_bit(STATUS_SCAN_HW, &priv->status); if (priv->cfg->ops->hcmd->set_pan_params) {
ret = priv->cfg->ops->hcmd->set_pan_params(priv);
if (priv->cfg->ops->hcmd->set_pan_params && if (ret)
priv->cfg->ops->hcmd->set_pan_params(priv)) return ret;
goto done; }
if (iwl_send_cmd_sync(priv, &cmd)) set_bit(STATUS_SCAN_HW, &priv->status);
goto done; ret = iwl_send_cmd_sync(priv, &cmd);
if (ret) {
clear_bit(STATUS_SCAN_HW, &priv->status);
if (priv->cfg->ops->hcmd->set_pan_params)
priv->cfg->ops->hcmd->set_pan_params(priv);
}
queue_delayed_work(priv->workqueue, &priv->scan_check, return ret;
IWL_SCAN_CHECK_WATCHDOG);
return;
done:
/* Cannot perform scan. Make sure we clear scanning
* bits from status so next scan request can be performed.
* If we don't clear scanning status bit here all next scan
* will fail
*/
clear_bit(STATUS_SCAN_HW, &priv->status);
clear_bit(STATUS_SCANNING, &priv->status);
/* inform mac80211 scan aborted */
queue_work(priv->workqueue, &priv->scan_completed);
} }
int iwlagn_manage_ibss_station(struct iwl_priv *priv, int iwlagn_manage_ibss_station(struct iwl_priv *priv,
......
...@@ -219,7 +219,7 @@ void iwl_reply_statistics(struct iwl_priv *priv, ...@@ -219,7 +219,7 @@ void iwl_reply_statistics(struct iwl_priv *priv,
struct iwl_rx_mem_buffer *rxb); struct iwl_rx_mem_buffer *rxb);
/* scan */ /* scan */
void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif); int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
/* station mgmt */ /* station mgmt */
int iwlagn_manage_ibss_station(struct iwl_priv *priv, int iwlagn_manage_ibss_station(struct iwl_priv *priv,
......
...@@ -111,7 +111,7 @@ struct iwl_hcmd_utils_ops { ...@@ -111,7 +111,7 @@ struct iwl_hcmd_utils_ops {
__le16 fc, __le32 *tx_flags); __le16 fc, __le32 *tx_flags);
int (*calc_rssi)(struct iwl_priv *priv, int (*calc_rssi)(struct iwl_priv *priv,
struct iwl_rx_phy_res *rx_resp); struct iwl_rx_phy_res *rx_resp);
void (*request_scan)(struct iwl_priv *priv, struct ieee80211_vif *vif); int (*request_scan)(struct iwl_priv *priv, struct ieee80211_vif *vif);
}; };
struct iwl_apm_ops { struct iwl_apm_ops {
......
...@@ -324,19 +324,68 @@ void iwl_init_scan_params(struct iwl_priv *priv) ...@@ -324,19 +324,68 @@ void iwl_init_scan_params(struct iwl_priv *priv)
} }
EXPORT_SYMBOL(iwl_init_scan_params); EXPORT_SYMBOL(iwl_init_scan_params);
static int iwl_scan_initiate(struct iwl_priv *priv, struct ieee80211_vif *vif) static int __must_check iwl_scan_initiate(struct iwl_priv *priv,
struct ieee80211_vif *vif,
bool internal,
enum nl80211_band band)
{ {
int ret;
lockdep_assert_held(&priv->mutex); lockdep_assert_held(&priv->mutex);
IWL_DEBUG_INFO(priv, "Starting scan...\n"); if (WARN_ON(!priv->cfg->ops->utils->request_scan))
return -EOPNOTSUPP;
cancel_delayed_work(&priv->scan_check);
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
return -EIO;
}
if (test_bit(STATUS_SCAN_HW, &priv->status)) {
IWL_DEBUG_INFO(priv,
"Multiple concurrent scan requests in parallel.\n");
return -EBUSY;
}
if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
IWL_DEBUG_SCAN(priv, "Aborting scan due to device shutdown\n");
return -EIO;
}
if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
IWL_DEBUG_HC(priv, "Scan request while abort pending.\n");
return -EBUSY;
}
if (iwl_is_rfkill(priv)) {
IWL_DEBUG_HC(priv, "Aborting scan due to RF Kill activation\n");
return -EIO;
}
if (!test_bit(STATUS_READY, &priv->status)) {
IWL_DEBUG_HC(priv, "Scan request while uninitialized.\n");
return -EBUSY;
}
IWL_DEBUG_INFO(priv, "Starting %sscan...\n",
internal ? "internal short " : "");
set_bit(STATUS_SCANNING, &priv->status); set_bit(STATUS_SCANNING, &priv->status);
priv->is_internal_short_scan = false; priv->is_internal_short_scan = internal;
priv->scan_start = jiffies; priv->scan_start = jiffies;
priv->scan_band = band;
if (WARN_ON(!priv->cfg->ops->utils->request_scan)) ret = priv->cfg->ops->utils->request_scan(priv, vif);
return -EOPNOTSUPP; if (ret) {
clear_bit(STATUS_SCANNING, &priv->status);
priv->is_internal_short_scan = false;
return ret;
}
priv->cfg->ops->utils->request_scan(priv, vif); queue_delayed_work(priv->workqueue, &priv->scan_check,
IWL_SCAN_CHECK_WATCHDOG);
return 0; return 0;
} }
...@@ -355,12 +404,6 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw, ...@@ -355,12 +404,6 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw,
mutex_lock(&priv->mutex); mutex_lock(&priv->mutex);
if (!iwl_is_ready_rf(priv)) {
ret = -EIO;
IWL_DEBUG_MAC80211(priv, "leave - not ready or exit pending\n");
goto out_unlock;
}
if (test_bit(STATUS_SCANNING, &priv->status) && if (test_bit(STATUS_SCANNING, &priv->status) &&
!priv->is_internal_short_scan) { !priv->is_internal_short_scan) {
IWL_DEBUG_SCAN(priv, "Scan already in progress.\n"); IWL_DEBUG_SCAN(priv, "Scan already in progress.\n");
...@@ -368,14 +411,7 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw, ...@@ -368,14 +411,7 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw,
goto out_unlock; goto out_unlock;
} }
if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
IWL_DEBUG_SCAN(priv, "Scan request while abort pending\n");
ret = -EAGAIN;
goto out_unlock;
}
/* mac80211 will only ask for one band at a time */ /* mac80211 will only ask for one band at a time */
priv->scan_band = req->channels[0]->band;
priv->scan_request = req; priv->scan_request = req;
priv->scan_vif = vif; priv->scan_vif = vif;
...@@ -386,7 +422,8 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw, ...@@ -386,7 +422,8 @@ int iwl_mac_hw_scan(struct ieee80211_hw *hw,
if (priv->is_internal_short_scan) if (priv->is_internal_short_scan)
ret = 0; ret = 0;
else else
ret = iwl_scan_initiate(priv, vif); ret = iwl_scan_initiate(priv, vif, false,
req->channels[0]->band);
IWL_DEBUG_MAC80211(priv, "leave\n"); IWL_DEBUG_MAC80211(priv, "leave\n");
...@@ -418,31 +455,13 @@ static void iwl_bg_start_internal_scan(struct work_struct *work) ...@@ -418,31 +455,13 @@ static void iwl_bg_start_internal_scan(struct work_struct *work)
goto unlock; goto unlock;
} }
if (!iwl_is_ready_rf(priv)) {
IWL_DEBUG_SCAN(priv, "not ready or exit pending\n");
goto unlock;
}
if (test_bit(STATUS_SCANNING, &priv->status)) { if (test_bit(STATUS_SCANNING, &priv->status)) {
IWL_DEBUG_SCAN(priv, "Scan already in progress.\n"); IWL_DEBUG_SCAN(priv, "Scan already in progress.\n");
goto unlock; goto unlock;
} }
if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) { if (iwl_scan_initiate(priv, NULL, true, priv->band))
IWL_DEBUG_SCAN(priv, "Scan request while abort pending\n"); IWL_DEBUG_SCAN(priv, "failed to start internal short scan\n");
goto unlock;
}
priv->scan_band = priv->band;
IWL_DEBUG_SCAN(priv, "Start internal short scan...\n");
set_bit(STATUS_SCANNING, &priv->status);
priv->is_internal_short_scan = true;
if (WARN_ON(!priv->cfg->ops->utils->request_scan))
goto unlock;
priv->cfg->ops->utils->request_scan(priv, NULL);
unlock: unlock:
mutex_unlock(&priv->mutex); mutex_unlock(&priv->mutex);
} }
...@@ -536,7 +555,6 @@ static void iwl_bg_scan_completed(struct work_struct *work) ...@@ -536,7 +555,6 @@ static void iwl_bg_scan_completed(struct work_struct *work)
struct iwl_priv *priv = struct iwl_priv *priv =
container_of(work, struct iwl_priv, scan_completed); container_of(work, struct iwl_priv, scan_completed);
bool internal = false; bool internal = false;
bool scan_completed = false;
struct iwl_rxon_context *ctx; struct iwl_rxon_context *ctx;
IWL_DEBUG_SCAN(priv, "SCAN complete scan\n"); IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
...@@ -549,16 +567,27 @@ static void iwl_bg_scan_completed(struct work_struct *work) ...@@ -549,16 +567,27 @@ static void iwl_bg_scan_completed(struct work_struct *work)
IWL_DEBUG_SCAN(priv, "internal short scan completed\n"); IWL_DEBUG_SCAN(priv, "internal short scan completed\n");
internal = true; internal = true;
} else if (priv->scan_request) { } else if (priv->scan_request) {
scan_completed = true;
priv->scan_request = NULL; priv->scan_request = NULL;
priv->scan_vif = NULL; priv->scan_vif = NULL;
ieee80211_scan_completed(priv->hw, false);
} }
if (test_bit(STATUS_EXIT_PENDING, &priv->status)) if (test_bit(STATUS_EXIT_PENDING, &priv->status))
goto out; goto out;
if (internal && priv->scan_request) if (internal && priv->scan_request) {
iwl_scan_initiate(priv, priv->scan_vif); int err = iwl_scan_initiate(priv, priv->scan_vif, false,
priv->scan_request->channels[0]->band);
if (err) {
IWL_DEBUG_SCAN(priv,
"failed to initiate pending scan: %d\n", err);
priv->scan_request = NULL;
priv->scan_vif = NULL;
ieee80211_scan_completed(priv->hw, true);
} else
goto out;
}
/* Since setting the TXPOWER may have been deferred while /* Since setting the TXPOWER may have been deferred while
* performing the scan, fire one off */ * performing the scan, fire one off */
...@@ -571,19 +600,11 @@ static void iwl_bg_scan_completed(struct work_struct *work) ...@@ -571,19 +600,11 @@ static void iwl_bg_scan_completed(struct work_struct *work)
for_each_context(priv, ctx) for_each_context(priv, ctx)
iwlcore_commit_rxon(priv, ctx); iwlcore_commit_rxon(priv, ctx);
out:
if (priv->cfg->ops->hcmd->set_pan_params) if (priv->cfg->ops->hcmd->set_pan_params)
priv->cfg->ops->hcmd->set_pan_params(priv); priv->cfg->ops->hcmd->set_pan_params(priv);
out:
mutex_unlock(&priv->mutex); mutex_unlock(&priv->mutex);
/*
* Do not hold mutex here since this will cause mac80211 to call
* into driver again into functions that will attempt to take
* mutex.
*/
if (scan_completed)
ieee80211_scan_completed(priv->hw, false);
} }
void iwl_setup_scan_deferred_work(struct iwl_priv *priv) void iwl_setup_scan_deferred_work(struct iwl_priv *priv)
......
...@@ -2816,7 +2816,7 @@ static void iwl3945_rfkill_poll(struct work_struct *data) ...@@ -2816,7 +2816,7 @@ static void iwl3945_rfkill_poll(struct work_struct *data)
} }
void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
{ {
struct iwl_host_cmd cmd = { struct iwl_host_cmd cmd = {
.id = REPLY_SCAN_CMD, .id = REPLY_SCAN_CMD,
...@@ -2827,55 +2827,16 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) ...@@ -2827,55 +2827,16 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
u8 n_probes = 0; u8 n_probes = 0;
enum ieee80211_band band; enum ieee80211_band band;
bool is_active = false; bool is_active = false;
int ret;
cancel_delayed_work(&priv->scan_check); lockdep_assert_held(&priv->mutex);
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
}
/* Make sure the scan wasn't canceled before this queued work
* was given the chance to run... */
if (!test_bit(STATUS_SCANNING, &priv->status))
goto done;
/* This should never be called or scheduled if there is currently
* a scan active in the hardware. */
if (test_bit(STATUS_SCAN_HW, &priv->status)) {
IWL_DEBUG_INFO(priv, "Multiple concurrent scan requests "
"Ignoring second request.\n");
goto done;
}
if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
IWL_DEBUG_SCAN(priv, "Aborting scan due to device shutdown\n");
goto done;
}
if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
IWL_DEBUG_HC(priv,
"Scan request while abort pending. Queuing.\n");
goto done;
}
if (iwl_is_rfkill(priv)) {
IWL_DEBUG_HC(priv, "Aborting scan due to RF Kill activation\n");
goto done;
}
if (!test_bit(STATUS_READY, &priv->status)) {
IWL_DEBUG_HC(priv,
"Scan request while uninitialized. Queuing.\n");
goto done;
}
if (!priv->scan_cmd) { if (!priv->scan_cmd) {
priv->scan_cmd = kmalloc(sizeof(struct iwl3945_scan_cmd) + priv->scan_cmd = kmalloc(sizeof(struct iwl3945_scan_cmd) +
IWL_MAX_SCAN_SIZE, GFP_KERNEL); IWL_MAX_SCAN_SIZE, GFP_KERNEL);
if (!priv->scan_cmd) { if (!priv->scan_cmd) {
IWL_DEBUG_SCAN(priv, "Fail to allocate scan memory\n"); IWL_DEBUG_SCAN(priv, "Fail to allocate scan memory\n");
goto done; return -ENOMEM;
} }
} }
scan = priv->scan_cmd; scan = priv->scan_cmd;
...@@ -2970,7 +2931,7 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) ...@@ -2970,7 +2931,7 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
break; break;
default: default:
IWL_WARN(priv, "Invalid scan band\n"); IWL_WARN(priv, "Invalid scan band\n");
goto done; return -EIO;
} }
if (!priv->is_internal_short_scan) { if (!priv->is_internal_short_scan) {
...@@ -3005,7 +2966,7 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) ...@@ -3005,7 +2966,7 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
if (scan->channel_count == 0) { if (scan->channel_count == 0) {
IWL_DEBUG_SCAN(priv, "channel count %d\n", scan->channel_count); IWL_DEBUG_SCAN(priv, "channel count %d\n", scan->channel_count);
goto done; return -EIO;
} }
cmd.len += le16_to_cpu(scan->tx_cmd.len) + cmd.len += le16_to_cpu(scan->tx_cmd.len) +
...@@ -3014,25 +2975,10 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif) ...@@ -3014,25 +2975,10 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
scan->len = cpu_to_le16(cmd.len); scan->len = cpu_to_le16(cmd.len);
set_bit(STATUS_SCAN_HW, &priv->status); set_bit(STATUS_SCAN_HW, &priv->status);
if (iwl_send_cmd_sync(priv, &cmd)) ret = iwl_send_cmd_sync(priv, &cmd);
goto done; if (ret)
clear_bit(STATUS_SCAN_HW, &priv->status);
queue_delayed_work(priv->workqueue, &priv->scan_check, return ret;
IWL_SCAN_CHECK_WATCHDOG);
return;
done:
/* can not perform scan make sure we clear scanning
* bits from status so next scan request can be performed.
* if we dont clear scanning status bit here all next scan
* will fail
*/
clear_bit(STATUS_SCAN_HW, &priv->status);
clear_bit(STATUS_SCANNING, &priv->status);
/* inform mac80211 scan aborted */
queue_work(priv->workqueue, &priv->scan_completed);
} }
static void iwl3945_bg_restart(struct work_struct *data) static void iwl3945_bg_restart(struct work_struct *data)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment