* [PATCH v2 0/2] iwlwifi: corner case fix and request module changes
From: Luis R. Rodriguez @ 2017-02-22 2:09 UTC (permalink / raw)
To: johannes.berg, luciano.coelho, emmanuel.grumbach
Cc: ming.lei, zajec5, linuxwifi, linux-wireless, linux-kernel,
Luis R. Rodriguez
In-Reply-To: <20170222001822.GE31264@wotan.suse.de>
This v2 addresses the preference to keep things simple on iwlwifi when
requesting modules and not implementing a verifier for loaing the opmode
module. We now know what a verifier looks like for both sync and async
approaches. The already established long standing practice of just doing
best effort to load suffices and keeps the driver cleaner.
There no change to the first patch. The second patch just embraces
request_module_nowait() instead of implementing a verifier for a sync
call. The remaining patches from the last series will be sent separately.
Luis R. Rodriguez (2):
iwlwifi: fix drv cleanup on opmode registration failure
iwlwifi: simplify requesting ops module
drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 28 ++++++++++------------------
1 file changed, 10 insertions(+), 18 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH v2 1/2] iwlwifi: fix drv cleanup on opmode registration failure
From: Luis R. Rodriguez @ 2017-02-22 2:09 UTC (permalink / raw)
To: johannes.berg, luciano.coelho, emmanuel.grumbach
Cc: ming.lei, zajec5, linuxwifi, linux-wireless, linux-kernel,
Luis R. Rodriguez
In-Reply-To: <20170222020939.28140-1-mcgrof@kernel.org>
The firmware async callback handles the device's opmode start
call, but optionally also allows opmode registration to take
care of its opmode start. If the firmware callback handles it
its error path in case of opmode start failure has a few pieces
of code missing from the opmode registration. The opmode
registration hanlder has no cleanup at all. Sync both error
paths.
This should in theory fix a detangled drv from the drv list should
either of the opmode modules loaded and handled registration for the
drv.
The path of having the opmode registration deal with the drv
opmode start is actually the more common path. The other path,
from the async callback is rathe rare (1/8 or so times for me) --
it happens when the the opmode driver's init routine completed
prior to the driver's async callback opmode start call.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index be466a074c1d..e198d6f5fcea 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1611,8 +1611,13 @@ int iwl_opmode_register(const char *name, const struct iwl_op_mode_ops *ops)
continue;
op->ops = ops;
/* TODO: need to handle exceptional case */
- list_for_each_entry(drv, &op->drv, list)
+ list_for_each_entry(drv, &op->drv, list) {
drv->op_mode = _iwl_op_mode_start(drv, op);
+ if (!drv->op_mode) {
+ complete(&drv->request_firmware_complete);
+ device_release_driver(drv->trans->dev);
+ }
+ }
mutex_unlock(&iwlwifi_opmode_table_mtx);
return 0;
--
2.11.0
^ permalink raw reply related
* [PATCH v2 2/2] iwlwifi: simplify requesting ops module
From: Luis R. Rodriguez @ 2017-02-22 2:09 UTC (permalink / raw)
To: johannes.berg, luciano.coelho, emmanuel.grumbach
Cc: ming.lei, zajec5, linuxwifi, linux-wireless, linux-kernel,
Luis R. Rodriguez
In-Reply-To: <20170222020939.28140-1-mcgrof@kernel.org>
The return value of request_module() being 0 does not mean that the
driver which was requested has loaded. To properly check that the driver
was loaded each driver can use internal mechanisms to vet the driver is
now present. The helper try_then_request_module() was added to help with
this, allowing drivers to specify their own validation as the first
argument for synchronous requests.
On iwlwifi the use case is a bit more complicated given that the value
we need to check for is protected with a mutex later used on the
module_init() of the module we are asking for. If we were to lock and
request_module() we'd deadlock. iwlwifi could use its own wrapper for
requesting its ops module synchronously so it can handle its special
locking requirements on its own but the given code has been in the
kernel for a long time and the preference is to keep its simplicity.
We can do two more things to simplify this even further:
a) just use the async request, request_module_nowait(), to acknowledge
we are using best effort -- we are not verifying the module is loaded
b) we can unconditionally call request_module_nowait() whether or not
we were built-in or not, if built-in this will return immediately
without failure.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 21 ++++-----------------
1 file changed, 4 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index e198d6f5fcea..e96095c1824a 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -1250,7 +1250,6 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
size_t trigger_tlv_sz[FW_DBG_TRIGGER_MAX];
u32 api_ver;
int i;
- bool load_module = false;
bool usniffer_images = false;
fw->ucode_capa.max_probe_length = IWL_DEFAULT_MAX_PROBE_LENGTH;
@@ -1454,8 +1453,6 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
mutex_unlock(&iwlwifi_opmode_table_mtx);
goto out_unbind;
}
- } else {
- load_module = true;
}
mutex_unlock(&iwlwifi_opmode_table_mtx);
@@ -1466,20 +1463,10 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
*/
complete(&drv->request_firmware_complete);
- /*
- * Load the module last so we don't block anything
- * else from proceeding if the module fails to load
- * or hangs loading.
- */
- if (load_module) {
- err = request_module("%s", op->name);
-#ifdef CONFIG_IWLWIFI_OPMODE_MODULAR
- if (err)
- IWL_ERR(drv,
- "failed to load module %s (error %d), is dynamic loading enabled?\n",
- op->name, err);
-#endif
- }
+ err = request_module_nowait("%s", op->name);
+ if (err)
+ goto out_unbind;
+
goto free;
try_again:
--
2.11.0
^ permalink raw reply related
* [PATCH v2 0/2] iwlwifi: share opmode start code
From: Luis R. Rodriguez @ 2017-02-22 2:10 UTC (permalink / raw)
To: johannes.berg, luciano.coelho, emmanuel.grumbach
Cc: ming.lei, zajec5, linuxwifi, linux-wireless, linux-kernel,
Luis R. Rodriguez
In-Reply-To: <20170222001822.GE31264@wotan.suse.de>
This v2 split off the opmode handling sharing code into its
own series, it however depends on the request_module_nowait()
change.
The sharing of the opmode handling makes it easier to share fixes
when dealing with opmode handling on devices. It should also hopefully
make the code easier to grok. Lastly, since we are moving things to
a workqueue naturally the module_init() for iwlmvm is not offloaded,
and so this should reduce the boot time by a bit.
As per the average of systemd-analyze on 5 boots using next-20170221
as base:
next-20170221:
Startup finished in 2.6142s (kernel) + 5.1916s (initrd) + 10.8968s (userspace) = 18.7036s
After these patches:
Startup finished in 2.5468s (kernel) + 4.9536s (initrd) + 10.798s (userspace) = 18.2994s
Luis R. Rodriguez (2):
iwlwifi: share opmode start work code
iwlwifi: convert final opmode work into a workqueue
drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 93 +++++++++++++++++++---------
1 file changed, 64 insertions(+), 29 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH v2 2/2] iwlwifi: convert final opmode work into a workqueue
From: Luis R. Rodriguez @ 2017-02-22 2:10 UTC (permalink / raw)
To: johannes.berg, luciano.coelho, emmanuel.grumbach
Cc: ming.lei, zajec5, linuxwifi, linux-wireless, linux-kernel,
Luis R. Rodriguez
In-Reply-To: <20170222021039.28193-1-mcgrof@kernel.org>
This lets us offload and share all the final opmode related work
necessary for either an opmode driver or new device. This has the most
impact for opmode drivers as this now offloads opmode start for each
device onto the workqueue.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index ea88b5cec869..1db372e9c039 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -143,6 +143,8 @@ static struct iwlwifi_opmode_table {
[DVM_OP_MODE] = { .name = "iwldvm", .ops = NULL },
[MVM_OP_MODE] = { .name = "iwlmvm", .ops = NULL },
};
+static void iwlwifi_opmode_dowork(struct work_struct *work);
+static DECLARE_WORK(iwlwifi_opmode_work, iwlwifi_opmode_dowork);
#define IWL_DEFAULT_SCAN_CHANNELS 40
@@ -1261,7 +1263,7 @@ static void iwlwifi_opmode_start(struct iwlwifi_opmode_table *op)
iwlwifi_opmode_start_drv(op, drv);
}
-static void iwlwifi_opmode_dowork(void)
+static void iwlwifi_opmode_dowork(struct work_struct *unused_work)
{
unsigned int i;
struct iwlwifi_opmode_table *op;
@@ -1497,7 +1499,7 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
if (err)
goto out_unbind;
- iwlwifi_opmode_dowork();
+ schedule_work(&iwlwifi_opmode_work);
goto free;
try_again:
@@ -1581,6 +1583,7 @@ struct iwl_drv *iwl_drv_start(struct iwl_trans *trans)
void iwl_drv_stop(struct iwl_drv *drv)
{
wait_for_completion(&drv->request_firmware_complete);
+ cancel_work_sync(&iwlwifi_opmode_work);
_iwl_op_mode_stop(drv);
@@ -1634,7 +1637,7 @@ int iwl_opmode_register(const char *name, const struct iwl_op_mode_ops *ops)
mutex_unlock(&iwlwifi_opmode_table_mtx);
if (!ret)
- iwlwifi_opmode_dowork();
+ schedule_work(&iwlwifi_opmode_work);
return ret;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v2 1/2] iwlwifi: share opmode start work code
From: Luis R. Rodriguez @ 2017-02-22 2:10 UTC (permalink / raw)
To: johannes.berg, luciano.coelho, emmanuel.grumbach
Cc: ming.lei, zajec5, linuxwifi, linux-wireless, linux-kernel,
Luis R. Rodriguez
In-Reply-To: <20170222021039.28193-1-mcgrof@kernel.org>
The firmware async callback and the opmode registration share
some functionality -- to start the drv's opmode. Move this work
into a helper which is shared. This should help us share fixes
should these diverging code paths change.
Signed-off-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
drivers/net/wireless/intel/iwlwifi/iwl-drv.c | 90 +++++++++++++++++++---------
1 file changed, 61 insertions(+), 29 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
index e96095c1824a..ea88b5cec869 100644
--- a/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
+++ b/drivers/net/wireless/intel/iwlwifi/iwl-drv.c
@@ -102,6 +102,7 @@ static struct dentry *iwl_dbgfs_root;
* @op_mode: the running op_mode
* @trans: transport layer
* @dev: for debug prints only
+ * @start_requested: start op has been requested and is pending on this device
* @fw_index: firmware revision to try loading
* @firmware_name: composite filename of ucode file to load
* @request_firmware_complete: the firmware has been obtained from user space
@@ -113,6 +114,7 @@ struct iwl_drv {
struct iwl_op_mode *op_mode;
struct iwl_trans *trans;
struct device *dev;
+ bool start_requested;
int fw_index; /* firmware we're trying to load */
char firmware_name[64]; /* name of firmware file to load */
@@ -1231,6 +1233,48 @@ static void _iwl_op_mode_stop(struct iwl_drv *drv)
}
}
+static void iwlwifi_opmode_start_drv(struct iwlwifi_opmode_table *op,
+ struct iwl_drv *drv)
+{
+ if (!drv->start_requested)
+ return;
+
+ drv->op_mode = _iwl_op_mode_start(drv, op);
+ drv->start_requested = false;
+
+ /*
+ * Complete the firmware request last so that
+ * a driver unbind (stop) doesn't run while we
+ * are doing the start() above.
+ */
+ complete(&drv->request_firmware_complete);
+
+ if (!drv->op_mode)
+ device_release_driver(drv->trans->dev);
+}
+
+static void iwlwifi_opmode_start(struct iwlwifi_opmode_table *op)
+{
+ struct iwl_drv *drv;
+
+ list_for_each_entry(drv, &op->drv, list)
+ iwlwifi_opmode_start_drv(op, drv);
+}
+
+static void iwlwifi_opmode_dowork(void)
+{
+ unsigned int i;
+ struct iwlwifi_opmode_table *op;
+
+ mutex_lock(&iwlwifi_opmode_table_mtx);
+ for (i = 0; i < ARRAY_SIZE(iwlwifi_opmode_table); i++) {
+ op = &iwlwifi_opmode_table[i];
+ if (op->ops)
+ iwlwifi_opmode_start(op);
+ }
+ mutex_unlock(&iwlwifi_opmode_table_mtx);
+}
+
/**
* iwl_req_fw_callback - callback when firmware was loaded
*
@@ -1443,30 +1487,17 @@ static void iwl_req_fw_callback(const struct firmware *ucode_raw, void *context)
IWL_INFO(drv, "loaded firmware version %s op_mode %s\n",
drv->fw.fw_version, op->name);
+ drv->start_requested = true;
/* add this device to the list of devices using this op_mode */
list_add_tail(&drv->list, &op->drv);
- if (op->ops) {
- drv->op_mode = _iwl_op_mode_start(drv, op);
-
- if (!drv->op_mode) {
- mutex_unlock(&iwlwifi_opmode_table_mtx);
- goto out_unbind;
- }
- }
mutex_unlock(&iwlwifi_opmode_table_mtx);
- /*
- * Complete the firmware request last so that
- * a driver unbind (stop) doesn't run while we
- * are doing the start() above.
- */
- complete(&drv->request_firmware_complete);
-
err = request_module_nowait("%s", op->name);
if (err)
goto out_unbind;
+ iwlwifi_opmode_dowork();
goto free;
try_again:
@@ -1588,8 +1619,8 @@ IWL_EXPORT_SYMBOL(iwlwifi_mod_params);
int iwl_opmode_register(const char *name, const struct iwl_op_mode_ops *ops)
{
int i;
- struct iwl_drv *drv;
struct iwlwifi_opmode_table *op;
+ int ret = -EIO;
mutex_lock(&iwlwifi_opmode_table_mtx);
for (i = 0; i < ARRAY_SIZE(iwlwifi_opmode_table); i++) {
@@ -1597,20 +1628,15 @@ int iwl_opmode_register(const char *name, const struct iwl_op_mode_ops *ops)
if (strcmp(op->name, name))
continue;
op->ops = ops;
- /* TODO: need to handle exceptional case */
- list_for_each_entry(drv, &op->drv, list) {
- drv->op_mode = _iwl_op_mode_start(drv, op);
- if (!drv->op_mode) {
- complete(&drv->request_firmware_complete);
- device_release_driver(drv->trans->dev);
- }
- }
-
- mutex_unlock(&iwlwifi_opmode_table_mtx);
- return 0;
+ ret = 0;
+ break;
}
mutex_unlock(&iwlwifi_opmode_table_mtx);
- return -EIO;
+
+ if (!ret)
+ iwlwifi_opmode_dowork();
+
+ return ret;
}
IWL_EXPORT_SYMBOL(iwl_opmode_register);
@@ -1626,8 +1652,14 @@ void iwl_opmode_deregister(const char *name)
iwlwifi_opmode_table[i].ops = NULL;
/* call the stop routine for all devices */
- list_for_each_entry(drv, &iwlwifi_opmode_table[i].drv, list)
+ list_for_each_entry(drv, &iwlwifi_opmode_table[i].drv, list) {
_iwl_op_mode_stop(drv);
+ /*
+ * So that if iwlmvm gets unloaded alone, but then
+ * loaded again we can kick the old registered devices
+ */
+ drv->start_requested = true;
+ }
mutex_unlock(&iwlwifi_opmode_table_mtx);
return;
--
2.11.0
^ permalink raw reply related
* [PATCH] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full
From: Tahia Khan @ 2017-02-22 4:55 UTC (permalink / raw)
To: outreachy-kernel, aditya.shankar, ganesh.krishna, gregkh,
linux-wireless, devel, linux-kernel
Fixes multiple camel case checks on struct tstrRSSI from checkpatch.pl:
Avoid CamelCase: <tstrRSSI>
Avoid CamelCase: <u8Full>
Avoid CamelCase: <u8Index>
Signed-off-by: Tahia Khan <tahia.khan@gmail.com>
---
drivers/staging/wilc1000/coreconfigurator.h | 8 ++++----
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 10 +++++-----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
index cff1698..c9cf23b 100644
--- a/drivers/staging/wilc1000/coreconfigurator.h
+++ b/drivers/staging/wilc1000/coreconfigurator.h
@@ -70,9 +70,9 @@ enum connect_status {
CONNECT_STS_FORCE_16_BIT = 0xFFFF
};
-struct tstrRSSI {
- u8 u8Full;
- u8 u8Index;
+struct tstr_RSSI {
+ u8 u8_full;
+ u8 u8_index;
s8 as8RSSI[NUM_RSSI];
};
@@ -93,7 +93,7 @@ struct network_info {
u8 *ies;
u16 ies_len;
void *join_params;
- struct tstrRSSI str_rssi;
+ struct tstr_RSSI str_rssi;
u64 tsf_hi;
};
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index f7ce47c..30285b9 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -205,8 +205,8 @@ static u32 get_rssi_avg(struct network_info *network_info)
{
u8 i;
int rssi_v = 0;
- u8 num_rssi = (network_info->str_rssi.u8Full) ?
- NUM_RSSI : (network_info->str_rssi.u8Index);
+ u8 num_rssi = (network_info->str_rssi.u8_full) ?
+ NUM_RSSI : (network_info->str_rssi.u8_index);
for (i = 0; i < num_rssi; i++)
rssi_v += network_info->str_rssi.as8RSSI[i];
@@ -346,13 +346,13 @@ static void add_network_to_shadow(struct network_info *pstrNetworkInfo,
} else {
ap_index = ap_found;
}
- rssi_index = last_scanned_shadow[ap_index].str_rssi.u8Index;
+ rssi_index = last_scanned_shadow[ap_index].str_rssi.u8_index;
last_scanned_shadow[ap_index].str_rssi.as8RSSI[rssi_index++] = pstrNetworkInfo->rssi;
if (rssi_index == NUM_RSSI) {
rssi_index = 0;
- last_scanned_shadow[ap_index].str_rssi.u8Full = 1;
+ last_scanned_shadow[ap_index].str_rssi.u8_full = 1;
}
- last_scanned_shadow[ap_index].str_rssi.u8Index = rssi_index;
+ last_scanned_shadow[ap_index].str_rssi.u8_index = rssi_index;
last_scanned_shadow[ap_index].rssi = pstrNetworkInfo->rssi;
last_scanned_shadow[ap_index].cap_info = pstrNetworkInfo->cap_info;
last_scanned_shadow[ap_index].ssid_len = pstrNetworkInfo->ssid_len;
--
2.7.4
^ permalink raw reply related
* Re: [Outreachy kernel] [PATCH] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full
From: Julia Lawall @ 2017-02-22 7:19 UTC (permalink / raw)
To: Tahia Khan
Cc: outreachy-kernel, aditya.shankar, ganesh.krishna, gregkh,
linux-wireless, devel, linux-kernel
In-Reply-To: <20170222045543.GA27847@coolbox>
On Tue, 21 Feb 2017, Tahia Khan wrote:
> Fixes multiple camel case checks on struct tstrRSSI from checkpatch.pl:
>
> Avoid CamelCase: <tstrRSSI>
> Avoid CamelCase: <u8Full>
> Avoid CamelCase: <u8Index>
>
> Signed-off-by: Tahia Khan <tahia.khan@gmail.com>
> ---
> drivers/staging/wilc1000/coreconfigurator.h | 8 ++++----
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 10 +++++-----
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
> index cff1698..c9cf23b 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.h
> +++ b/drivers/staging/wilc1000/coreconfigurator.h
> @@ -70,9 +70,9 @@ enum connect_status {
> CONNECT_STS_FORCE_16_BIT = 0xFFFF
> };
>
> -struct tstrRSSI {
> - u8 u8Full;
> - u8 u8Index;
> +struct tstr_RSSI {
> + u8 u8_full;
> + u8 u8_index;
The kernel doesn't use variable names that incorporate types. These could
just be full and index.
julia
> s8 as8RSSI[NUM_RSSI];
> };
>
> @@ -93,7 +93,7 @@ struct network_info {
> u8 *ies;
> u16 ies_len;
> void *join_params;
> - struct tstrRSSI str_rssi;
> + struct tstr_RSSI str_rssi;
> u64 tsf_hi;
> };
>
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index f7ce47c..30285b9 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -205,8 +205,8 @@ static u32 get_rssi_avg(struct network_info *network_info)
> {
> u8 i;
> int rssi_v = 0;
> - u8 num_rssi = (network_info->str_rssi.u8Full) ?
> - NUM_RSSI : (network_info->str_rssi.u8Index);
> + u8 num_rssi = (network_info->str_rssi.u8_full) ?
> + NUM_RSSI : (network_info->str_rssi.u8_index);
>
> for (i = 0; i < num_rssi; i++)
> rssi_v += network_info->str_rssi.as8RSSI[i];
> @@ -346,13 +346,13 @@ static void add_network_to_shadow(struct network_info *pstrNetworkInfo,
> } else {
> ap_index = ap_found;
> }
> - rssi_index = last_scanned_shadow[ap_index].str_rssi.u8Index;
> + rssi_index = last_scanned_shadow[ap_index].str_rssi.u8_index;
> last_scanned_shadow[ap_index].str_rssi.as8RSSI[rssi_index++] = pstrNetworkInfo->rssi;
> if (rssi_index == NUM_RSSI) {
> rssi_index = 0;
> - last_scanned_shadow[ap_index].str_rssi.u8Full = 1;
> + last_scanned_shadow[ap_index].str_rssi.u8_full = 1;
> }
> - last_scanned_shadow[ap_index].str_rssi.u8Index = rssi_index;
> + last_scanned_shadow[ap_index].str_rssi.u8_index = rssi_index;
> last_scanned_shadow[ap_index].rssi = pstrNetworkInfo->rssi;
> last_scanned_shadow[ap_index].cap_info = pstrNetworkInfo->cap_info;
> last_scanned_shadow[ap_index].ssid_len = pstrNetworkInfo->ssid_len;
> --
> 2.7.4
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20170222045543.GA27847%40coolbox.
> For more options, visit https://groups.google.com/d/optout.
>
^ permalink raw reply
* [PATCH] ath10k: Modify macros to fix style issues
From: Marcin Rokicki @ 2017-02-22 8:15 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
Both macros are used internally to convert incomming parameters
to strings in a switch case statement.
Current implementation gives following output from checkpatch.pl:
- ERROR: Macros with complex values should be enclosed in parentheses
- WARNING: Macros with flow control statements should be avoided
Fix them by modify local variable in the middle and just return at the end.
Btw add const to function that return string literals
Signed-off-by: Marcin Rokicki <marcin.rokicki@tieto.com>
---
drivers/net/wireless/ath/ath10k/wmi.h | 36 +++++++++++++++++++++++++----------
1 file changed, 26 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
index 427220c..0bf578f 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -312,9 +312,16 @@ enum wmi_10_4_service {
WMI_10_4_SERVICE_TX_MODE_DYNAMIC,
};
-static inline char *wmi_service_name(int service_id)
+#define SVCSTR(x) \
+{ \
+ case x: \
+ str = #x; \
+ break; \
+}
+
+static inline const char *wmi_service_name(int service_id)
{
-#define SVCSTR(x) case x: return #x
+ const char *str = NULL;
switch (service_id) {
SVCSTR(WMI_SERVICE_BEACON_OFFLOAD);
@@ -408,13 +415,13 @@ static inline char *wmi_service_name(int service_id)
SVCSTR(WMI_SERVICE_TX_MODE_PUSH_ONLY);
SVCSTR(WMI_SERVICE_TX_MODE_PUSH_PULL);
SVCSTR(WMI_SERVICE_TX_MODE_DYNAMIC);
- default:
- return NULL;
}
-#undef SVCSTR
+ return str;
}
+#undef SVCSTR
+
#define WMI_SERVICE_IS_ENABLED(wmi_svc_bmap, svc_id, len) \
((svc_id) < (len) && \
__le32_to_cpu((wmi_svc_bmap)[(svc_id) / (sizeof(u32))]) & \
@@ -6412,10 +6419,17 @@ enum wmi_wow_wakeup_event {
WOW_EVENT_MAX,
};
-#define C2S(x) case x: return #x
+#define C2S(x) \
+{ \
+ case x: \
+ str = #x; \
+ break; \
+}
static inline const char *wow_wakeup_event(enum wmi_wow_wakeup_event ev)
{
+ const char *str = NULL;
+
switch (ev) {
C2S(WOW_BMISS_EVENT);
C2S(WOW_BETTER_AP_EVENT);
@@ -6442,9 +6456,9 @@ static inline const char *wow_wakeup_event(enum wmi_wow_wakeup_event ev)
C2S(WOW_BEACON_EVENT);
C2S(WOW_CLIENT_KICKOUT_EVENT);
C2S(WOW_EVENT_MAX);
- default:
- return NULL;
}
+
+ return str;
}
enum wmi_wow_wake_reason {
@@ -6482,6 +6496,8 @@ enum wmi_wow_wake_reason {
static inline const char *wow_reason(enum wmi_wow_wake_reason reason)
{
+ const char *str = NULL;
+
switch (reason) {
C2S(WOW_REASON_UNSPECIFIED);
C2S(WOW_REASON_NLOD);
@@ -6513,9 +6529,9 @@ static inline const char *wow_reason(enum wmi_wow_wake_reason reason)
C2S(WOW_REASON_BEACON_RECV);
C2S(WOW_REASON_CLIENT_KICKOUT_EVENT);
C2S(WOW_REASON_DEBUG_TEST);
- default:
- return NULL;
}
+
+ return str;
}
#undef C2S
--
2.7.4
^ permalink raw reply related
* Re: [rtl8xxxu] rtl8192cu (0bda:8176) on linux-4.9.11 (arm) high sirq rate
From: Christian Hilberg @ 2017-02-22 9:54 UTC (permalink / raw)
To: linux-wireless
In-Reply-To: <1581189.s4o7FEpKoz@vbox>
[-- Attachment #1: Type: text/plain, Size: 2134 bytes --]
Hi again,
Am Montag, 20. Februar 2017, 14:51:54 CET schrieb Christian Hilberg:
> Hi all,
>
> currently I'm testing rtl8xxxu on an ARM device (i.mx28, armv5te) under
> linux-4.9.11 with a RealTek 8192cu (0bda:8176) wifi dongle. Since
> I had to set CONFIG_RTL8XXXU_UNTESTED, I'm prepared to see some
> rough edges here and there. :-)
The rtl8xxxu driver identifies the device as "8188CU", and so does lsusb.
> The driver actually works and I'm getting a stable connection on
> a WPA2 secured wifi network. As a firmware blob, the driver loads
> the "rtl8192cufw_TMSC.bin", which I extracted from [0].
>
> However, the sirg rate is rather high (between 20% to 50% in top),
> as soon as the device is getting upped, that is, as soon as IFF_UP
> gets written to the device via ioctl().
I've dug further into this.
It shows that if in the driver's rtl8xxxu_identify_chip() routine for the
RTL8188C variant I set priv->usb_interrupts = 0, then the sirq load vanishes
and the driver apparently still works:
---8<---
diff --git a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c b/drivers/
net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
index 82d949ede294..2daacfd059b8 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/rtl8xxxu_core.c
@@ -1687,7 +1687,7 @@ static int rtl8xxxu_identify_chip(struct rtl8xxxu_priv
*priv)
priv->rx_paths = 1;
priv->tx_paths = 1;
priv->rtl_chip = RTL8188C;
- priv->usb_interrupts = 1;
+ priv->usb_interrupts = 0;
priv->has_wifi = 1;
}
---8<---
Seems to me that rtl8xxxu_int_complete() does nothing with the URB data but to
re-submit it (just to satisfy hardware which wants its interrupts to be
handled maybe?).
I'm not at all clear whether the above change would be the Right Thing to do.
Jes, do you have any thoughts on this? Are there 8188CU variants which need
interrupts to be handled?
Regards,
Christian
> [0]
> http://ftp.de.debian.org/debian/pool/non-free/f/firmware-nonfree/firmware-r
> ealtek_20161130-2_all.deb
--
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply related
* Re: [PATCH] ath10k: Modify macros to fix style issues
From: Joe Perches @ 2017-02-22 11:34 UTC (permalink / raw)
To: Marcin Rokicki, ath10k; +Cc: linux-wireless
In-Reply-To: <1487751327-2917-1-git-send-email-marcin.rokicki@tieto.com>
On Wed, 2017-02-22 at 09:15 +0100, Marcin Rokicki wrote:
> Both macros are used internally to convert incomming parameters
> to strings in a switch case statement.
>
> Current implementation gives following output from checkpatch.pl:
> - ERROR: Macros with complex values should be enclosed in parentheses
> - WARNING: Macros with flow control statements should be avoided
>
> Fix them by modify local variable in the middle and just return at the end.
>
> Btw add const to function that return string literals
[]
> diff --git a/drivers/net/wireless/ath/ath10k/wmi.h b/drivers/net/wireless/ath/ath10k/wmi.h
[]
> @@ -312,9 +312,16 @@ enum wmi_10_4_service {
> WMI_10_4_SERVICE_TX_MODE_DYNAMIC,
> };
>
> -static inline char *wmi_service_name(int service_id)
> +#define SVCSTR(x) \
> +{ \
> + case x: \
> + str = #x; \
> + break; \
> +}
> +
> +static inline const char *wmi_service_name(int service_id)
> {
> -#define SVCSTR(x) case x: return #x
> + const char *str = NULL;
>
> switch (service_id) {
> SVCSTR(WMI_SERVICE_BEACON_OFFLOAD);
> @@ -408,13 +415,13 @@ static inline char *wmi_service_name(int service_id)
> SVCSTR(WMI_SERVICE_TX_MODE_PUSH_ONLY);
> SVCSTR(WMI_SERVICE_TX_MODE_PUSH_PULL);
> SVCSTR(WMI_SERVICE_TX_MODE_DYNAMIC);
> - default:
> - return NULL;
> }
>
> -#undef SVCSTR
> + return str;
> }
>
> +#undef SVCSTR
> +
> #define WMI_SERVICE_IS_ENABLED(wmi_svc_bmap, svc_id, len) \
> ((svc_id) < (len) && \
> __le32_to_cpu((wmi_svc_bmap)[(svc_id) / (sizeof(u32))]) & \
> @@ -6412,10 +6419,17 @@ enum wmi_wow_wakeup_event {
> WOW_EVENT_MAX,
> };
>
> -#define C2S(x) case x: return #x
> +#define C2S(x) \
> +{ \
> + case x: \
> + str = #x; \
> + break; \
> +}
>
> static inline const char *wow_wakeup_event(enum wmi_wow_wakeup_event ev)
> {
> + const char *str = NULL;
> +
> switch (ev) {
> C2S(WOW_BMISS_EVENT);
> C2S(WOW_BETTER_AP_EVENT);
> @@ -6442,9 +6456,9 @@ static inline const char *wow_wakeup_event(enum wmi_wow_wakeup_event ev)
> C2S(WOW_BEACON_EVENT);
> C2S(WOW_CLIENT_KICKOUT_EVENT);
> C2S(WOW_EVENT_MAX);
> - default:
> - return NULL;
> }
> +
> + return str;
> }
>
> enum wmi_wow_wake_reason {
> @@ -6482,6 +6496,8 @@ enum wmi_wow_wake_reason {
>
> static inline const char *wow_reason(enum wmi_wow_wake_reason reason)
> {
> + const char *str = NULL;
> +
> switch (reason) {
> C2S(WOW_REASON_UNSPECIFIED);
> C2S(WOW_REASON_NLOD);
> @@ -6513,9 +6529,9 @@ static inline const char *wow_reason(enum wmi_wow_wake_reason reason)
> C2S(WOW_REASON_BEACON_RECV);
> C2S(WOW_REASON_CLIENT_KICKOUT_EVENT);
> C2S(WOW_REASON_DEBUG_TEST);
> - default:
> - return NULL;
> }
> +
> + return str;
> }
>
> #undef C2S
Here is an alternate style used a few times in the kernel
Maybe it'd be nicer to change the macros to something like
#define CASE_STR(x) case x: return #x
and just return NULL after the switch/case block
Maybe make that a global macro and consolidate the various
uses to a single style
drivers/net/wireless/ath/ath9k/ath9k.h:#define case_rtn_string(val) case val: return #val
drivers/net/wireless/ath/ath10k/wmi.h:#define SVCSTR(x) case x: return #x
drivers/net/wireless/ath/ath10k/wmi.h:#define C2S(x) case x: return #x
drivers/net/wireless/intel/iwlegacy/common.h:#define IL_CMD(x) case x: return #x
drivers/net/wireless/intel/iwlwifi/iwl-io.c:#define IWL_CMD(x) case x: return #x
drivers/net/wireless/intel/iwlwifi/pcie/trans.c:#define IWL_CMD(x) case x: return #x
drivers/net/wireless/atmel/at76c50x-usb.c:#define MAKE_CMD_CASE(c) case (c): return #c
drivers/net/wireless/atmel/at76c50x-usb.c:#define MAKE_CMD_STATUS_CASE(c) case (c): return #c
drivers/net/ethernet/intel/fm10k/fm10k_pci.c:#define FM10K_ERR_MSG(type) case (type): error = #type; break
drivers/staging/lustre/lnet/selftest/selftest.h:#define STATE2STR(x) case x: return #x
include/linux/genl_magic_func.h: case op_num: return #op_name;
t_case_default.c:#define FOO(BAR) { case BAR: return #BAR; }
^ permalink raw reply
* Re: [PATCH] cfg80211: support 4-way handshake offloading for WPA/WPA2-PSK
From: Jithu Jance @ 2017-02-22 12:24 UTC (permalink / raw)
To: Arend Van Spriel
Cc: Johannes Berg, linux-wireless, Eliad Peller, Jouni Malinen
In-Reply-To: <3fa1e3ac-5722-c46d-acc1-62407f4eef07@broadcom.com>
Hi Jouni, Johannes et al.,
On Wed, Feb 22, 2017 at 1:27 AM, Arend Van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 21-2-2017 13:37, Johannes Berg wrote:
>>
>>> What I am also missing is how to deal with NL80211_CMD_CONNECTED
>>> event signalling. Should that be given after (un)successful
>>> completion of the 4-way handshake or should we have a separate event
>>> for that?
>>
>> That's a good point. I don't think a new event really works well, but
>> perhaps we should extend the NL80211_TIMEOUT_* values with this? It's
>> not strictly a timeout, I guess, but it's not a direct rejection of the
>> association either.
>>
>> Any thoughts?
>
> I discussed about the event mechanism with Jithu which is who asked me
> to look at their patches for this feature. His suggestion was to have a
> "port authorized" event after successful handshake as it will help in
> key mgmt offload cases. He gave the following example: Suppose you are
> doing a fresh 8021x followed by 4way handshake offload. Right now there
> is no event to indicate to the upper layer that the 4way handshake have
> been completed successfully. So the only thing to do in wpa_supplicant
> is to consider it completed and if not be hit with deauth.
In wpa_supplicant terms, I was looking for an event notification to move the
wpa_state from WPA_ASSOCIATED to WPA_COMPLETED. :)
Thanks,
--
Jithu Jance
^ permalink raw reply
* RE: [OpenWrt-Devel] ATH10K VLAN firmware issue
From: voncken @ 2017-02-22 13:55 UTC (permalink / raw)
To: 'Valo, Kalle'
Cc: 'OpenWrt Development List', 'linux-wireless',
ath10k
In-Reply-To: <87efyrd6lv.fsf@kamboji.qca.qualcomm.com>
Thanks for your answer,
How I can know when this issue will be fixed in ath10k firmware
mainline?
Do you know if Qualcomm publish a release note for this firmware?
> -----Message d'origine-----
> De : linux-wireless-owner@vger.kernel.org [mailto:linux-wireless-
> owner@vger.kernel.org] De la part de Valo, Kalle
> Envoyé : mardi 21 février 2017 14:19
> À : voncken
> Cc : 'OpenWrt Development List'; 'linux-wireless';
> ath10k@lists.infradead.org
> Objet : Re: [OpenWrt-Devel] ATH10K VLAN firmware issue
>
> voncken <cedric.voncken@acksys.fr> writes:
>
> > Do you know if the firmware team planned to fix the VLAN issue on
> > ath10k firmware?
>
> I reported it forward only this week.
>
> --
> Kalle Valo
^ permalink raw reply
* [PATCH] mac80211: don't handle filtered frames within a BA session
From: Felix Fietkau @ 2017-02-22 15:16 UTC (permalink / raw)
To: linux-wireless; +Cc: johannes
When running a BA session, the driver (or the hardware) already takes
care of retransmitting failed frames, since it has to keep the receiver
reorder window in sync.
Adding another layer of retransmit around that does not improve
anything. In fact, it can only lead to some strong reordering with huge
latency.
Cc: stable@vger.kernel.org
Signed-off-by: Felix Fietkau <nbd@nbd.name>
---
net/mac80211/status.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index b6a5394fa4c4..9d19d8698ac0 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -51,7 +51,8 @@ static void ieee80211_handle_filtered_frame(struct ieee80211_local *local,
struct ieee80211_hdr *hdr = (void *)skb->data;
int ac;
- if (info->flags & IEEE80211_TX_CTL_NO_PS_BUFFER) {
+ if (info->flags & (IEEE80211_TX_CTL_NO_PS_BUFFER |
+ IEEE80211_TX_CTL_AMPDU)) {
ieee80211_free_txskb(&local->hw, skb);
return;
}
--
2.11.0
^ permalink raw reply related
* [PATCH] ath10k: Disallow DFS simulation if DFS channel is not enabled
From: Mohammed Shafi Shajakhan @ 2017-02-22 15:33 UTC (permalink / raw)
To: ath10k; +Cc: mohammed, linux-wireless, Mohammed Shafi Shajakhan
From: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
If DFS is not enabled in hostapd (ieee80211h=0) DFS channels shall
not be available for use even though the hardware may have the capability
to support DFS. With this configuration (DFS disabled in hostapd) trying to
bring up ath10k device in DFS channel for AP mode fails and trying to
simulate DFS in ath10k debugfs results in a warning in cfg80211 complaining
invalid channel and this should be avoided in the driver itself rather than
false propogating RADAR detection to mac80211/cfg80211. Fix this by
checking for the first vif 'is_started' state(should work for client mode
as well) as all the vifs shall be configured for the same channel
sys/kernel/debug/ieee80211/phy1/ath10k# echo 1 > dfs_simulate_radar
WARNING: at net/wireless/chan.c:265 cfg80211_radar_event+0x24/0x60
Workqueue: phy0 ieee80211_dfs_radar_detected_work [mac80211]
[<c022f2d4>] (warn_slowpath_null) from
[<bf72dab8>] (cfg80211_radar_event+0x24/0x60 [cfg80211])
[<bf72dab8>] (cfg80211_radar_event [cfg80211]) from
[<bf7813e0>] (ieee80211_dfs_radar_detected_work+0x94/0xa0 [mac80211])
[<bf7813e0>] (ieee80211_dfs_radar_detected_work [mac80211]) from
[<c0242320>] (process_one_work+0x20c/0x32c)
WARNING: at net/wireless/nl80211.c:2488 nl80211_get_mpath+0x13c/0x4cc
Workqueue: phy0 ieee80211_dfs_radar_detected_work [mac80211]
[<c022f2d4>] (warn_slowpath_null) from
[<bf72dab8>] (cfg80211_radar_event+0x24/0x60 [cfg80211])
[<bf72dab8>] (cfg80211_radar_event [cfg80211]) from
[<bf7813e0>] (ieee80211_dfs_radar_detected_work+0x94/0xa0 [mac80211])
[<bf7813e0>] (ieee80211_dfs_radar_detected_work [mac80211]) from
[<c0242320>] (process_one_work+0x20c/0x32c)
Signed-off-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
---
[thanks Kalle, Jouni and Tamizh for the first level review]
drivers/net/wireless/ath/ath10k/debug.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index fb0ade3..ac9090b 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -1997,6 +1997,15 @@ static ssize_t ath10k_write_simulate_radar(struct file *file,
size_t count, loff_t *ppos)
{
struct ath10k *ar = file->private_data;
+ struct ath10k_vif *arvif;
+
+ /* Just check for for the first vif alone, as all the vifs will be
+ * sharing the same channel and if the channel is disabled, all the
+ * vifs will share the same 'is_started' state.
+ */
+ arvif = list_first_entry(&ar->arvifs, typeof(*arvif), list);
+ if (!arvif->is_started)
+ return -EINVAL;
ieee80211_radar_detected(ar->hw);
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] ath10k: Modify macros to fix style issues
From: Joe Perches @ 2017-02-22 16:19 UTC (permalink / raw)
To: Marcin Rokicki, ath10k; +Cc: linux-wireless
In-Reply-To: <CAN6SofZJjMmdkwqtD-amUTHzFJyQAYEKJUgOeHPKJEX7w0fD=g@mail.gmail.com>
(fyi Marcin, the reason this isn't getting on the list
is because your 3 tries have all included text and html)
On Wed, 2017-02-22 at 14:31 +0100, Marcin Rokicki wrote:
> >
> > Here is an alternate style used a few times in the kernel
> >
> > Maybe it'd be nicer to change the macros to something like
> >
> > #define CASE_STR(x) case x: return #x
> >
> > and just return NULL after the switch/case block
> >
> > Maybe make that a global macro and consolidate the various
> > uses to a single style
[]
> This alternate style used few times in the kernel cause that
> checkpatch.pl prints
> such messages:
> - ERROR: Macros with complex values should be enclosed in parentheses
> - WARNING: Macros with flow control statements should be avoided
>
> for "all" of your examples - except fm10k which is implemented (almost) in
> the same way like above patch
> but still prints:
> - ERROR: Macros with multiple statements should be enclosed in a do -
> while loop
Yes, checkpatch is and will always be imperfect.
It's just a bunch of regex tests.
Anyway, the point of my email was to highlight a
possible line count reduction and an opportunity
to standardize a style.
cheers, Joe
^ permalink raw reply
* [PATCH v2] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full
From: Tahia Khan @ 2017-02-22 17:14 UTC (permalink / raw)
To: outreachy-kernel, aditya.shankar, ganesh.krishna, gregkh,
linux-wireless, devel, linux-kernel
Fixes multiple camel case checks on struct tstrRSSI from checkpatch.pl:
Avoid CamelCase: <tstrRSSI>
Avoid CamelCase: <u8Full>
Avoid CamelCase: <u8Index>
Signed-off-by: Tahia Khan <tahia.khan@gmail.com>
---
drivers/staging/wilc1000/coreconfigurator.h | 8 ++++----
drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 10 +++++-----
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
index cff1698..5b65c4f 100644
--- a/drivers/staging/wilc1000/coreconfigurator.h
+++ b/drivers/staging/wilc1000/coreconfigurator.h
@@ -70,9 +70,9 @@ enum connect_status {
CONNECT_STS_FORCE_16_BIT = 0xFFFF
};
-struct tstrRSSI {
- u8 u8Full;
- u8 u8Index;
+struct tstr_RSSI {
+ u8 full;
+ u8 index;
s8 as8RSSI[NUM_RSSI];
};
@@ -93,7 +93,7 @@ struct network_info {
u8 *ies;
u16 ies_len;
void *join_params;
- struct tstrRSSI str_rssi;
+ struct tstr_RSSI str_rssi;
u64 tsf_hi;
};
diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
index f7ce47c..56f133e 100644
--- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
+++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
@@ -205,8 +205,8 @@ static u32 get_rssi_avg(struct network_info *network_info)
{
u8 i;
int rssi_v = 0;
- u8 num_rssi = (network_info->str_rssi.u8Full) ?
- NUM_RSSI : (network_info->str_rssi.u8Index);
+ u8 num_rssi = (network_info->str_rssi.full) ?
+ NUM_RSSI : (network_info->str_rssi.index);
for (i = 0; i < num_rssi; i++)
rssi_v += network_info->str_rssi.as8RSSI[i];
@@ -346,13 +346,13 @@ static void add_network_to_shadow(struct network_info *pstrNetworkInfo,
} else {
ap_index = ap_found;
}
- rssi_index = last_scanned_shadow[ap_index].str_rssi.u8Index;
+ rssi_index = last_scanned_shadow[ap_index].str_rssi.index;
last_scanned_shadow[ap_index].str_rssi.as8RSSI[rssi_index++] = pstrNetworkInfo->rssi;
if (rssi_index == NUM_RSSI) {
rssi_index = 0;
- last_scanned_shadow[ap_index].str_rssi.u8Full = 1;
+ last_scanned_shadow[ap_index].str_rssi.full = 1;
}
- last_scanned_shadow[ap_index].str_rssi.u8Index = rssi_index;
+ last_scanned_shadow[ap_index].str_rssi.index = rssi_index;
last_scanned_shadow[ap_index].rssi = pstrNetworkInfo->rssi;
last_scanned_shadow[ap_index].cap_info = pstrNetworkInfo->cap_info;
last_scanned_shadow[ap_index].ssid_len = pstrNetworkInfo->ssid_len;
--
2.7.4
^ permalink raw reply related
* Re: [PATCH v2] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full
From: Arend Van Spriel @ 2017-02-22 19:50 UTC (permalink / raw)
To: Tahia Khan, outreachy-kernel, aditya.shankar, ganesh.krishna,
gregkh, linux-wireless, devel, linux-kernel
In-Reply-To: <20170222171403.GA20626@coolbox>
On 22-2-2017 18:14, Tahia Khan wrote:
> Fixes multiple camel case checks on struct tstrRSSI from checkpatch.pl:
>
> Avoid CamelCase: <tstrRSSI>
> Avoid CamelCase: <u8Full>
> Avoid CamelCase: <u8Index>
Just a generic remark that may help you with other changes you will be
making in the linux kernel. Warnings from checkpatch.pl and other tools
are useful, but try to look further than just fixing a warning.
Understand what the code is doing is just as important.
> Signed-off-by: Tahia Khan <tahia.khan@gmail.com>
> ---
> drivers/staging/wilc1000/coreconfigurator.h | 8 ++++----
> drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 10 +++++-----
> 2 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
> index cff1698..5b65c4f 100644
> --- a/drivers/staging/wilc1000/coreconfigurator.h
> +++ b/drivers/staging/wilc1000/coreconfigurator.h
> @@ -70,9 +70,9 @@ enum connect_status {
> CONNECT_STS_FORCE_16_BIT = 0xFFFF
> };
>
> -struct tstrRSSI {
> - u8 u8Full;
> - u8 u8Index;
> +struct tstr_RSSI {
> + u8 full;
> + u8 index;
> s8 as8RSSI[NUM_RSSI];
So you have a struct here with three members and you choose to only
change the first two. What do you think about the third one just by
looking at it. And what about the name of the struct. What does 'tstr' mean?
> };
>
> @@ -93,7 +93,7 @@ struct network_info {
> u8 *ies;
> u16 ies_len;
> void *join_params;
> - struct tstrRSSI str_rssi;
> + struct tstr_RSSI str_rssi;
> u64 tsf_hi;
> };
>
> diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> index f7ce47c..56f133e 100644
> --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> @@ -205,8 +205,8 @@ static u32 get_rssi_avg(struct network_info *network_info)
> {
> u8 i;
> int rssi_v = 0;
> - u8 num_rssi = (network_info->str_rssi.u8Full) ?
> - NUM_RSSI : (network_info->str_rssi.u8Index);
> + u8 num_rssi = (network_info->str_rssi.full) ?
> + NUM_RSSI : (network_info->str_rssi.index);
so struct tstr_RSSI is really a rssi history buffer so maybe naming it
as such makes it clearer, ie. struct rssi_history_buffer.
> for (i = 0; i < num_rssi; i++)
> rssi_v += network_info->str_rssi.as8RSSI[i];
> @@ -346,13 +346,13 @@ static void add_network_to_shadow(struct network_info *pstrNetworkInfo,
> } else {
> ap_index = ap_found;
> }
> - rssi_index = last_scanned_shadow[ap_index].str_rssi.u8Index;
> + rssi_index = last_scanned_shadow[ap_index].str_rssi.index;
> last_scanned_shadow[ap_index].str_rssi.as8RSSI[rssi_index++] = pstrNetworkInfo->rssi;
> if (rssi_index == NUM_RSSI) {
> rssi_index = 0;
> - last_scanned_shadow[ap_index].str_rssi.u8Full = 1;
> + last_scanned_shadow[ap_index].str_rssi.full = 1;
So the 'full' member is actually a bool and you might type it as such.
Hope this helps.
Regards,
Arend
> }
> - last_scanned_shadow[ap_index].str_rssi.u8Index = rssi_index;
> + last_scanned_shadow[ap_index].str_rssi.index = rssi_index;
> last_scanned_shadow[ap_index].rssi = pstrNetworkInfo->rssi;
> last_scanned_shadow[ap_index].cap_info = pstrNetworkInfo->cap_info;
> last_scanned_shadow[ap_index].ssid_len = pstrNetworkInfo->ssid_len;
>
^ permalink raw reply
* Re: [PATCH v2] ath10k: Remove return statement from a void function
From: Arend Van Spriel @ 2017-02-22 19:54 UTC (permalink / raw)
To: Marcin Rokicki, ath10k; +Cc: linux-wireless
In-Reply-To: <1487672891-8584-1-git-send-email-marcin.rokicki@tieto.com>
On 21-2-2017 11:28, Marcin Rokicki wrote:
> The empty 'return;' statement in a void function should be
> used to return from somewhere else than the end.
>
> Signed-off-by: Marcin Rokicki <marcin.rokicki@tieto.com>
> ---
>
> Changes for v2
> -remove only return statement instead of empty err label
> which can be used in the future
>
> ---
> drivers/net/wireless/ath/ath10k/core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
> index 59729aa..a22d3c9 100644
> --- a/drivers/net/wireless/ath/ath10k/core.c
> +++ b/drivers/net/wireless/ath/ath10k/core.c
> @@ -2311,7 +2311,7 @@ static void ath10k_core_register_work(struct work_struct *work)
> /* TODO: It's probably a good idea to release device from the driver
> * but calling device_release_driver() here will cause a deadlock.
> */
> - return;
> + ;
Not exactly what I meant. Just drop the whole line include semicolon.
Regards,
Arend
> }
>
> int ath10k_core_register(struct ath10k *ar, u32 chip_id)
>
^ permalink raw reply
* Re: [PATCH] wireless-regdb: Update rules for Australia (AU) and add 60GHz rules
From: Seth Forshee @ 2017-02-22 21:30 UTC (permalink / raw)
To: Ryan Mounce; +Cc: wireless-regdb, linux-wireless
In-Reply-To: <20170218063515.16071-1-ryan@mounce.com.au>
On Sat, Feb 18, 2017 at 05:05:15PM +1030, Ryan Mounce wrote:
> Sourced from the latest legislation at
> https://www.legislation.gov.au/Details/F2016C0043
This link isn't working for me.
> The current rules exceed legal limits between 5250-5330MHz, and permit
> illegal operation in 5600-5650MHz (reserved for weather radar).
The changes also increases the limits for most other ranges, by quite a
bit in some cases. Seems that this should also be mentioned in the
changelog, and I'd like to be able to check those changes against the
documentation.
Thanks,
Seth
>
> Signed-off-by: Ryan Mounce <ryan@mounce.com.au>
> ---
> db.txt | 15 ++++++++++-----
> 1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/db.txt b/db.txt
> index 05108e0..b951e0c 100644
> --- a/db.txt
> +++ b/db.txt
> @@ -85,12 +85,17 @@ country AT: DFS-ETSI
> # 60 GHz band channels 1-4, ref: Etsi En 302 567
> (57000 - 66000 @ 2160), (40)
>
> +# Source:
> +# https://www.legislation.gov.au/Details/F2016C00432
> +# Both DFS-ETSI and DFS-FCC are acceptable per AS/NZS 4268 Appendix B
> country AU: DFS-ETSI
> - (2402 - 2482 @ 40), (20)
> - (5170 - 5250 @ 80), (17), AUTO-BW
> - (5250 - 5330 @ 80), (24), DFS, AUTO-BW
> - (5490 - 5710 @ 160), (24), DFS
> - (5735 - 5835 @ 80), (30)
> + (2402 - 2482 @ 40), (36)
> + (5170 - 5250 @ 80), (23), NO-OUTDOOR, AUTO-BW
> + (5250 - 5330 @ 80), (23), NO-OUTDOOR, AUTO-BW, DFS
> + (5490 - 5590 @ 80), (30), DFS
> + (5650 - 5730 @ 80), (30), DFS
> + (5735 - 5835 @ 80), (36)
> + (57000 - 66000 @ 2160), (43), NO-OUTDOOR
>
> country AW: DFS-ETSI
> (2402 - 2482 @ 40), (20)
> --
> 2.11.0
>
^ permalink raw reply
* Re: [PATCH] wireless-regdb: Update rules for Australia (AU) and add 60GHz rules
From: Ryan Mounce @ 2017-02-22 23:05 UTC (permalink / raw)
To: Seth Forshee; +Cc: wireless-regdb, linux-wireless
In-Reply-To: <20170222213057.GA33925@ubuntu-hedt>
On 23 February 2017 at 08:00, Seth Forshee <seth.forshee@canonical.com> wrote:
> On Sat, Feb 18, 2017 at 05:05:15PM +1030, Ryan Mounce wrote:
>> Sourced from the latest legislation at
>> https://www.legislation.gov.au/Details/F2016C0043
>
> This link isn't working for me.
Oops, I truncated that URL. Working link (also in the patch itself):
https://www.legislation.gov.au/Details/F2016C00432
>> The current rules exceed legal limits between 5250-5330MHz, and permit
>> illegal operation in 5600-5650MHz (reserved for weather radar).
>
> The changes also increases the limits for most other ranges, by quite a
> bit in some cases. Seems that this should also be mentioned in the
> changelog, and I'd like to be able to check those changes against the
> documentation.
All of the updated power limits are converted to dBm but otherwise
straight out of the document, so they shouldn't be a problem. I'll
update the changelog to note this.
The frequency ranges have been "rounded down" to conform with the
apparent convention of only including ranges used by standard 802.11
channels, rather than using the actual frequency ranges as has been
done for some countries (and most 60GHz rules). My preference is the
latter so that there is a clear correlation between the regdb and
relevant documentation, and so that the regdb may become useful beyond
802.11. With your blessing as maintainer I will also make this change
to the patch.
Regards,
Ryan Mounce
> Thanks,
> Seth
>
>>
>> Signed-off-by: Ryan Mounce <ryan@mounce.com.au>
>> ---
>> db.txt | 15 ++++++++++-----
>> 1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/db.txt b/db.txt
>> index 05108e0..b951e0c 100644
>> --- a/db.txt
>> +++ b/db.txt
>> @@ -85,12 +85,17 @@ country AT: DFS-ETSI
>> # 60 GHz band channels 1-4, ref: Etsi En 302 567
>> (57000 - 66000 @ 2160), (40)
>>
>> +# Source:
>> +# https://www.legislation.gov.au/Details/F2016C00432
>> +# Both DFS-ETSI and DFS-FCC are acceptable per AS/NZS 4268 Appendix B
>> country AU: DFS-ETSI
>> - (2402 - 2482 @ 40), (20)
>> - (5170 - 5250 @ 80), (17), AUTO-BW
>> - (5250 - 5330 @ 80), (24), DFS, AUTO-BW
>> - (5490 - 5710 @ 160), (24), DFS
>> - (5735 - 5835 @ 80), (30)
>> + (2402 - 2482 @ 40), (36)
>> + (5170 - 5250 @ 80), (23), NO-OUTDOOR, AUTO-BW
>> + (5250 - 5330 @ 80), (23), NO-OUTDOOR, AUTO-BW, DFS
>> + (5490 - 5590 @ 80), (30), DFS
>> + (5650 - 5730 @ 80), (30), DFS
>> + (5735 - 5835 @ 80), (36)
>> + (57000 - 66000 @ 2160), (43), NO-OUTDOOR
>>
>> country AW: DFS-ETSI
>> (2402 - 2482 @ 40), (20)
>> --
>> 2.11.0
>>
^ permalink raw reply
* [PATCH] rtlwifi: fix spelling mistake: "conuntry" -> "country"
From: Colin King @ 2017-02-22 23:35 UTC (permalink / raw)
To: Larry Finger, Chaoming Li, Kalle Valo, linux-wireless, netdev
Cc: linux-kernel
From: Colin Ian King <colin.king@canonical.com>
trivial fix to spelling mistake in RT_TRACE message
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/net/wireless/realtek/rtlwifi/regd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/regd.c b/drivers/net/wireless/realtek/rtlwifi/regd.c
index 558c31b..1bf3eb2 100644
--- a/drivers/net/wireless/realtek/rtlwifi/regd.c
+++ b/drivers/net/wireless/realtek/rtlwifi/regd.c
@@ -435,7 +435,7 @@ int rtl_regd_init(struct ieee80211_hw *hw,
channel_plan_to_country_code(rtlpriv->efuse.channel_plan);
RT_TRACE(rtlpriv, COMP_REGD, DBG_DMESG,
- "rtl: EEPROM regdomain: 0x%0x conuntry code: %d\n",
+ "rtl: EEPROM regdomain: 0x%0x country code: %d\n",
rtlpriv->efuse.channel_plan, rtlpriv->regd.country_code);
if (rtlpriv->regd.country_code >= COUNTRY_CODE_MAX) {
--
2.10.2
^ permalink raw reply related
* Re: [PATCH v2] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full
From: Tahia Khan @ 2017-02-23 3:54 UTC (permalink / raw)
To: arend.vanspriel, outreachy-kernel, aditya.shankar, ganesh.krishna,
gregkh, linux-wireless, devel, linux-kernel
In-Reply-To: <838d3e67-646d-b2d3-ef7d-5812675db6db@broadcom.com>
On Wed, Feb 22, 2017 at 08:50:31PM +0100, Arend Van Spriel wrote:
> On 22-2-2017 18:14, Tahia Khan wrote:
> > Fixes multiple camel case checks on struct tstrRSSI from checkpatch.pl:
> >
> > Avoid CamelCase: <tstrRSSI>
> > Avoid CamelCase: <u8Full>
> > Avoid CamelCase: <u8Index>
>
> Just a generic remark that may help you with other changes you will be
> making in the linux kernel. Warnings from checkpatch.pl and other tools
> are useful, but try to look further than just fixing a warning.
> Understand what the code is doing is just as important.
>
> > Signed-off-by: Tahia Khan <tahia.khan@gmail.com>
> > ---
> > drivers/staging/wilc1000/coreconfigurator.h | 8 ++++----
> > drivers/staging/wilc1000/wilc_wfi_cfgoperations.c | 10 +++++-----
> > 2 files changed, 9 insertions(+), 9 deletions(-)
> >
> > diff --git a/drivers/staging/wilc1000/coreconfigurator.h b/drivers/staging/wilc1000/coreconfigurator.h
> > index cff1698..5b65c4f 100644
> > --- a/drivers/staging/wilc1000/coreconfigurator.h
> > +++ b/drivers/staging/wilc1000/coreconfigurator.h
> > @@ -70,9 +70,9 @@ enum connect_status {
> > CONNECT_STS_FORCE_16_BIT = 0xFFFF
> > };
> >
> > -struct tstrRSSI {
> > - u8 u8Full;
> > - u8 u8Index;
> > +struct tstr_RSSI {
> > + u8 full;
> > + u8 index;
> > s8 as8RSSI[NUM_RSSI];
>
> So you have a struct here with three members and you choose to only
> change the first two. What do you think about the third one just by
> looking at it. And what about the name of the struct. What does 'tstr' mean?
>
> > };
> >
> > @@ -93,7 +93,7 @@ struct network_info {
> > u8 *ies;
> > u16 ies_len;
> > void *join_params;
> > - struct tstrRSSI str_rssi;
> > + struct tstr_RSSI str_rssi;
> > u64 tsf_hi;
> > };
> >
> > diff --git a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > index f7ce47c..56f133e 100644
> > --- a/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > +++ b/drivers/staging/wilc1000/wilc_wfi_cfgoperations.c
> > @@ -205,8 +205,8 @@ static u32 get_rssi_avg(struct network_info *network_info)
> > {
> > u8 i;
> > int rssi_v = 0;
> > - u8 num_rssi = (network_info->str_rssi.u8Full) ?
> > - NUM_RSSI : (network_info->str_rssi.u8Index);
> > + u8 num_rssi = (network_info->str_rssi.full) ?
> > + NUM_RSSI : (network_info->str_rssi.index);
>
> so struct tstr_RSSI is really a rssi history buffer so maybe naming it
> as such makes it clearer, ie. struct rssi_history_buffer.
>
> > for (i = 0; i < num_rssi; i++)
> > rssi_v += network_info->str_rssi.as8RSSI[i];
> > @@ -346,13 +346,13 @@ static void add_network_to_shadow(struct network_info *pstrNetworkInfo,
> > } else {
> > ap_index = ap_found;
> > }
> > - rssi_index = last_scanned_shadow[ap_index].str_rssi.u8Index;
> > + rssi_index = last_scanned_shadow[ap_index].str_rssi.index;
> > last_scanned_shadow[ap_index].str_rssi.as8RSSI[rssi_index++] = pstrNetworkInfo->rssi;
> > if (rssi_index == NUM_RSSI) {
> > rssi_index = 0;
> > - last_scanned_shadow[ap_index].str_rssi.u8Full = 1;
> > + last_scanned_shadow[ap_index].str_rssi.full = 1;
>
> So the 'full' member is actually a bool and you might type it as such.
>
> Hope this helps.
>
> Regards,
> Arend
>
> > }
> > - last_scanned_shadow[ap_index].str_rssi.u8Index = rssi_index;
> > + last_scanned_shadow[ap_index].str_rssi.index = rssi_index;
> > last_scanned_shadow[ap_index].rssi = pstrNetworkInfo->rssi;
> > last_scanned_shadow[ap_index].cap_info = pstrNetworkInfo->cap_info;
> > last_scanned_shadow[ap_index].ssid_len = pstrNetworkInfo->ssid_len;
> >
Thanks for the feedback Arend, I really appreciate it. I've decided to go with
these changes in my follow-up patch request:
- rename tstrRSSI to 'rssi_history_buffer' as Aren suggested since it makes the
purpose of the struct clear
- remove Hungarian notation from all tstrRSSI members' names
- change type of u8Full to bool since it's only ever 1 or 0
- change name of as8RSSI to 'samples' since this buffer is only ever used to
compute an average, and the "rssi" prefix is implied by the struct's name
- rename str_rssi to rssi_history in the network_info struct for clarity
Since my reasoning for these changes deviates from just "renaming to
avoid camel casing" (as in the original checkpatch.pl warning), would it still
make sense to submit all this in a single patch? I know my commit message
needs to change but I wonder if this is too much detail.
Tahia
^ permalink raw reply
* Re: [PATCH] wireless-regdb: Update rules for Australia (AU) and add 60GHz rules
From: Johannes Berg @ 2017-02-23 5:58 UTC (permalink / raw)
To: Ryan Mounce, Seth Forshee; +Cc: wireless-regdb, linux-wireless
In-Reply-To: <CAN+fvRb8R_3QyrnFFp0gd8PCcFXwbitNyh8K=ONqbmsGTrBsHQ@mail.gmail.com>
> The frequency ranges have been "rounded down" to conform with the
> apparent convention of only including ranges used by standard 802.11
> channels, rather than using the actual frequency ranges as has been
> done for some countries (and most 60GHz rules). My preference is the
> latter so that there is a clear correlation between the regdb and
> relevant documentation, and so that the regdb may become useful
> beyond 802.11. With your blessing as maintainer I will also make this
> change to the patch.
I think that's just an artifact of the original database source and
would also prefer the documented ranges to be used.
johannes
^ permalink raw reply
* Re: [PATCH v2] staging: wilc1000: renames struct tstrRSSI and its members u8Index, u8Full
From: Joe Perches @ 2017-02-23 6:24 UTC (permalink / raw)
To: Arend Van Spriel, Tahia Khan, outreachy-kernel, aditya.shankar,
ganesh.krishna, gregkh, linux-wireless, devel, linux-kernel
In-Reply-To: <838d3e67-646d-b2d3-ef7d-5812675db6db@broadcom.com>
On Wed, 2017-02-22 at 20:50 +0100, Arend Van Spriel wrote:
> On 22-2-2017 18:14, Tahia Khan wrote:
> > Fixes multiple camel case checks on struct tstrRSSI from checkpatch.pl:
[]
> Just a generic remark that may help you with other changes you will be
> making in the linux kernel. Warnings from checkpatch.pl and other tools
> are useful, but try to look further than just fixing a warning.
> Understand what the code is doing is just as important.
I'd assert understanding what the code is doing is
_more_ important. Style consistency simply helps
improve the speed of a new reader's understanding.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox