* [PATCH 2/5] ath10k: Remove unused 'num_vdev_stats' variable
From: Kirtika Ruchandani @ 2016-11-24 8:01 UTC (permalink / raw)
To: Kalle Valo
Cc: Arnd Bergmann, netdev, linux-wireless, Raja Mani, Michal Kazior
In-Reply-To: <cover.1479974100.git.kirtika@chromium.org>
Several functions for wmi stats parsing define and set num_vdev_stats
but do not use it. Compiling with W=1 gives the following warnings,
fix them.
drivers/net/wireless/ath/ath10k/wmi.c: In function ‘ath10k_wmi_main_op_pull_fw_stats’:
drivers/net/wireless/ath/ath10k/wmi.c:2680:22: warning: variable ‘num_vdev_stats’ set but not used [-Wunused-but-set-variable]
u32 num_pdev_stats, num_vdev_stats, num_peer_stats;
^
drivers/net/wireless/ath/ath10k/wmi.c: In function ‘ath10k_wmi_10x_op_pull_fw_stats’:
drivers/net/wireless/ath/ath10k/wmi.c:2735:22: warning: variable ‘num_vdev_stats’ set but not used [-Wunused-but-set-variable]
u32 num_pdev_stats, num_vdev_stats, num_peer_stats;
^
drivers/net/wireless/ath/ath10k/wmi.c: In function ‘ath10k_wmi_10_2_op_pull_fw_stats’:
drivers/net/wireless/ath/ath10k/wmi.c:2796:6: warning: variable ‘num_vdev_stats’ set but not used [-Wunused-but-set-variable]
u32 num_vdev_stats;
^
drivers/net/wireless/ath/ath10k/wmi.c: In function ‘ath10k_wmi_10_2_4_op_pull_fw_stats’:
drivers/net/wireless/ath/ath10k/wmi.c:2875:6: warning: variable ‘num_vdev_stats’ set but not used [-Wunused-but-set-variable]
u32 num_vdev_stats;
^
drivers/net/wireless/ath/ath10k/wmi.c: In function ‘ath10k_wmi_10_4_op_pull_fw_stats’:
drivers/net/wireless/ath/ath10k/wmi.c:2963:6: warning: variable ‘num_vdev_stats’ set but not used [-Wunused-but-set-variable]
u32 num_vdev_stats;
^
These are harmless warnings and are only being fixed to reduce the
noise with W=1 in the kernel.
Fixes: d15fb5200664 ("ath10k: split wmi stats parsing")
Fixes: 20de2229c634 ("ath10k: fix 10.2 fw stats parsing")
Cc: Michal Kazior <michal.kazior@tieto.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>
---
drivers/net/wireless/ath/ath10k/wmi.c | 12 ++----------
1 file changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 1f28187..10ec5a5 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2675,14 +2675,13 @@ static int ath10k_wmi_main_op_pull_fw_stats(struct ath10k *ar,
struct ath10k_fw_stats *stats)
{
const struct wmi_stats_event *ev = (void *)skb->data;
- u32 num_pdev_stats, num_vdev_stats, num_peer_stats;
+ u32 num_pdev_stats, num_peer_stats;
int i;
if (!skb_pull(skb, sizeof(*ev)))
return -EPROTO;
num_pdev_stats = __le32_to_cpu(ev->num_pdev_stats);
- num_vdev_stats = __le32_to_cpu(ev->num_vdev_stats);
num_peer_stats = __le32_to_cpu(ev->num_peer_stats);
for (i = 0; i < num_pdev_stats; i++) {
@@ -2730,14 +2729,13 @@ static int ath10k_wmi_10x_op_pull_fw_stats(struct ath10k *ar,
struct ath10k_fw_stats *stats)
{
const struct wmi_stats_event *ev = (void *)skb->data;
- u32 num_pdev_stats, num_vdev_stats, num_peer_stats;
+ u32 num_pdev_stats, num_peer_stats;
int i;
if (!skb_pull(skb, sizeof(*ev)))
return -EPROTO;
num_pdev_stats = __le32_to_cpu(ev->num_pdev_stats);
- num_vdev_stats = __le32_to_cpu(ev->num_vdev_stats);
num_peer_stats = __le32_to_cpu(ev->num_peer_stats);
for (i = 0; i < num_pdev_stats; i++) {
@@ -2791,7 +2789,6 @@ static int ath10k_wmi_10_2_op_pull_fw_stats(struct ath10k *ar,
const struct wmi_10_2_stats_event *ev = (void *)skb->data;
u32 num_pdev_stats;
u32 num_pdev_ext_stats;
- u32 num_vdev_stats;
u32 num_peer_stats;
int i;
@@ -2800,7 +2797,6 @@ static int ath10k_wmi_10_2_op_pull_fw_stats(struct ath10k *ar,
num_pdev_stats = __le32_to_cpu(ev->num_pdev_stats);
num_pdev_ext_stats = __le32_to_cpu(ev->num_pdev_ext_stats);
- num_vdev_stats = __le32_to_cpu(ev->num_vdev_stats);
num_peer_stats = __le32_to_cpu(ev->num_peer_stats);
for (i = 0; i < num_pdev_stats; i++) {
@@ -2870,7 +2866,6 @@ static int ath10k_wmi_10_2_4_op_pull_fw_stats(struct ath10k *ar,
const struct wmi_10_2_stats_event *ev = (void *)skb->data;
u32 num_pdev_stats;
u32 num_pdev_ext_stats;
- u32 num_vdev_stats;
u32 num_peer_stats;
int i;
@@ -2879,7 +2874,6 @@ static int ath10k_wmi_10_2_4_op_pull_fw_stats(struct ath10k *ar,
num_pdev_stats = __le32_to_cpu(ev->num_pdev_stats);
num_pdev_ext_stats = __le32_to_cpu(ev->num_pdev_ext_stats);
- num_vdev_stats = __le32_to_cpu(ev->num_vdev_stats);
num_peer_stats = __le32_to_cpu(ev->num_peer_stats);
for (i = 0; i < num_pdev_stats; i++) {
@@ -2958,7 +2952,6 @@ static int ath10k_wmi_10_4_op_pull_fw_stats(struct ath10k *ar,
const struct wmi_10_2_stats_event *ev = (void *)skb->data;
u32 num_pdev_stats;
u32 num_pdev_ext_stats;
- u32 num_vdev_stats;
u32 num_peer_stats;
u32 num_bcnflt_stats;
u32 stats_id;
@@ -2969,7 +2962,6 @@ static int ath10k_wmi_10_4_op_pull_fw_stats(struct ath10k *ar,
num_pdev_stats = __le32_to_cpu(ev->num_pdev_stats);
num_pdev_ext_stats = __le32_to_cpu(ev->num_pdev_ext_stats);
- num_vdev_stats = __le32_to_cpu(ev->num_vdev_stats);
num_peer_stats = __le32_to_cpu(ev->num_peer_stats);
num_bcnflt_stats = __le32_to_cpu(ev->num_bcnflt_stats);
stats_id = __le32_to_cpu(ev->stats_id);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 3/5] ath10k: Remove unused wmi_p2p_noa_descriptor 'noa' in wmi-tlv
From: Kirtika Ruchandani @ 2016-11-24 8:01 UTC (permalink / raw)
To: Kalle Valo
Cc: Arnd Bergmann, netdev, linux-wireless, Raja Mani, Michal Kazior
In-Reply-To: <cover.1479974100.git.kirtika@chromium.org>
Commit ca996ec56608 (ath10k: implement wmi-tlv backend)
introduced ath10k_wmi_tlv_op_gen_vdev_start() where
'struct wmi_p2p_noa_descriptor *noa' is defined and set but not used.
Compiling with W=1 gives the following warning, fix it.
drivers/net/wireless/ath/ath10k/wmi-tlv.c: In function ‘ath10k_wmi_tlv_op_gen_vdev_start’:
drivers/net/wireless/ath/ath10k/wmi-tlv.c:1647:33: warning: variable ‘noa’ set but not used [-Wunused-but-set-variable]
Fixes: ca996ec56608 ("ath10k: implement wmi-tlv backend")
Cc: Michal Kazior <michal.kazior@tieto.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>
---
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e64f593..0e4bd29 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1644,7 +1644,6 @@ ath10k_wmi_tlv_op_gen_vdev_start(struct ath10k *ar,
{
struct wmi_tlv_vdev_start_cmd *cmd;
struct wmi_channel *ch;
- struct wmi_p2p_noa_descriptor *noa;
struct wmi_tlv *tlv;
struct sk_buff *skb;
size_t len;
@@ -1702,7 +1701,6 @@ ath10k_wmi_tlv_op_gen_vdev_start(struct ath10k *ar,
tlv = ptr;
tlv->tag = __cpu_to_le16(WMI_TLV_TAG_ARRAY_STRUCT);
tlv->len = 0;
- noa = (void *)tlv->value;
/* Note: This is a nested TLV containing:
* [wmi_tlv][wmi_p2p_noa_descriptor][wmi_tlv]..
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 4/5] ath10k: Removed unused 'dev' in ath10k_ahb_clock_enable()
From: Kirtika Ruchandani @ 2016-11-24 8:02 UTC (permalink / raw)
To: Kalle Valo
Cc: Arnd Bergmann, netdev, linux-wireless, Raja Mani, Michal Kazior
In-Reply-To: <cover.1479974100.git.kirtika@chromium.org>
Commit 8beff219c528 introduced ath10k_ahb_clock_enable() which
defines and sets 'struct device* dev' but does not use it.
Compiling with W=1 gives the following warning, fix it.
drivers/net/wireless/ath/ath10k/ahb.c: In function ‘ath10k_ahb_clock_enable’:
drivers/net/wireless/ath/ath10k/ahb.c:133:17: warning: variable ‘dev’ set but not used [-Wunused-but-set-variable]
This is a harmless warning, and is only being fixed to reduce the
noise with W=1 in the kernel.
Fixes: 8beff219c528("ath10k: add clock ctrl related functions in ahb")
Cc: Raja Mani <rmani@qti.qualcomm.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>
---
drivers/net/wireless/ath/ath10k/ahb.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ahb.c b/drivers/net/wireless/ath/ath10k/ahb.c
index 766c63b..8078d64 100644
--- a/drivers/net/wireless/ath/ath10k/ahb.c
+++ b/drivers/net/wireless/ath/ath10k/ahb.c
@@ -130,11 +130,8 @@ static void ath10k_ahb_clock_deinit(struct ath10k *ar)
static int ath10k_ahb_clock_enable(struct ath10k *ar)
{
struct ath10k_ahb *ar_ahb = ath10k_ahb_priv(ar);
- struct device *dev;
int ret;
- dev = &ar_ahb->pdev->dev;
-
if (IS_ERR_OR_NULL(ar_ahb->cmd_clk) ||
IS_ERR_OR_NULL(ar_ahb->ref_clk) ||
IS_ERR_OR_NULL(ar_ahb->rtc_clk)) {
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 1/5] ath10k: Remove unused 'buf_len' variable
From: Kirtika Ruchandani @ 2016-11-24 8:01 UTC (permalink / raw)
To: Kalle Valo
Cc: Arnd Bergmann, netdev, linux-wireless, Raja Mani, Michal Kazior
In-Reply-To: <cover.1479974100.git.kirtika@chromium.org>
Commit 32653cf19554 removed the call to 'skb_trim(skb, buf_len)'
in ath10k_wmi_event_mgmt_rx(), leaving the buf_len variable set but
unused. Compiling with W=1 gives the following warning, fix it.
drivers/net/wireless/ath/ath10k/wmi.c: In function ‘ath10k_wmi_event_mgmt_rx’:
drivers/net/wireless/ath/ath10k/wmi.c:2280:6: warning: variable ‘buf_len’ set but not used [-Wunused-but-set-variable]
This is a harmless warning, and is only being fixed to reduce the
noise with W=1 in the kernel.
Fixes: 32653cf19554 ("ath10k: implement intermediate event args")
Cc: Michal Kazior <michal.kazior@tieto.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>
---
drivers/net/wireless/ath/ath10k/wmi.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
index 387c4ee..1f28187 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -2277,7 +2277,6 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
u32 phy_mode;
u32 snr;
u32 rate;
- u32 buf_len;
u16 fc;
int ret;
@@ -2289,7 +2288,6 @@ int ath10k_wmi_event_mgmt_rx(struct ath10k *ar, struct sk_buff *skb)
}
channel = __le32_to_cpu(arg.channel);
- buf_len = __le32_to_cpu(arg.buf_len);
rx_status = __le32_to_cpu(arg.status);
snr = __le32_to_cpu(arg.snr);
phy_mode = __le32_to_cpu(arg.phy_mode);
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 5/5] ath10k: Removed unused 'dev' in ath10k_ahb_resource_init
From: Kirtika Ruchandani @ 2016-11-24 8:02 UTC (permalink / raw)
To: Kalle Valo
Cc: Arnd Bergmann, netdev, linux-wireless, Raja Mani, Michal Kazior
In-Reply-To: <cover.1479974100.git.kirtika@chromium.org>
Commit 704dc4e36769 introduced ath10k_ahb_resource_init() which
defines and sets 'struct device* dev' but does not use it.
Compiling with W=1 gives the following warning, fix it.
drivers/net/wireless/ath/ath10k/ahb.c: In function ‘ath10k_ahb_resource_init’:
drivers/net/wireless/ath/ath10k/ahb.c:449:17: warning: variable ‘dev’ set but not used [-Wunused-but-set-variable]
This is a harmless warning, and is only being fixed to reduce the
noise with W=1 in the kernel.
Fixes: 704dc4e36769 ("ath10k: add resource init and deinit in ahb")
Cc: Raja Mani <rmani@qti.qualcomm.com>
Cc: Kalle Valo <kvalo@qca.qualcomm.com>
Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>
---
drivers/net/wireless/ath/ath10k/ahb.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ahb.c b/drivers/net/wireless/ath/ath10k/ahb.c
index 8078d64..19507fe 100644
--- a/drivers/net/wireless/ath/ath10k/ahb.c
+++ b/drivers/net/wireless/ath/ath10k/ahb.c
@@ -443,12 +443,10 @@ static int ath10k_ahb_resource_init(struct ath10k *ar)
{
struct ath10k_ahb *ar_ahb = ath10k_ahb_priv(ar);
struct platform_device *pdev;
- struct device *dev;
struct resource *res;
int ret;
pdev = ar_ahb->pdev;
- dev = &pdev->dev;
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
if (!res) {
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply related
* [PATCH 0/5] Fix -Wunused-but-set-variable in ath10k/
From: Kirtika Ruchandani @ 2016-11-24 8:01 UTC (permalink / raw)
To: Kalle Valo
Cc: Arnd Bergmann, netdev, linux-wireless, Raja Mani, Michal Kazior
This patchset is part of the effort led by Arnd Bergmann to clean up
warnings in the kernel. This and following patchsets will focus on
"-Wunused-but-set-variable" as it among the noisier ones. These were
found compiling with W=1.
Kirtika Ruchandani (5):
ath10k: Remove unused 'buf_len' variable
ath10k: Remove unused 'num_vdev_stats' variable
ath10k: Remove unused wmi_p2p_noa_descriptor 'noa' in wmi-tlv
ath10k: Removed unused 'dev' in ath10k_ahb_clock_enable()
ath10k: Removed unused 'dev' in ath10k_ahb_resource_init
drivers/net/wireless/ath/ath10k/ahb.c | 5 -----
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 --
drivers/net/wireless/ath/ath10k/wmi.c | 14 ++------------
3 files changed, 2 insertions(+), 19 deletions(-)
--
2.8.0.rc3.226.g39d4020
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-11-24 8:33 UTC (permalink / raw)
To: Pavel Machek
Cc: Michal Kazior, Kalle Valo, Ivaylo Dimitrov, Sebastian Reichel,
Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
linux-kernel
In-Reply-To: <20161124075104.GA26721@amd>
On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> Hi!
>
> > > "ifconfig hw ether XX" normally sets the address. I guess that's
> > > ioctl?
> >
> > This sets temporary address and it is ioctl. IIRC same as what ethtool
> > uses. (ifconfig is already deprecated).
> >
> > > And I guess we should use similar mechanism for permanent
> > > address.
> >
> > I'm not sure here... Above ioctl ↑↑↑ is for changing temporary mac
> > address. But here we do not want to change permanent mac address. We
> > want to tell kernel driver current permanent mac address which is
> > stored
>
> Well... I'd still use similar mechanism :-).
Thats problematic, because in time when wlan0 interface is registered
into system and visible in ifconfig output it already needs to have
permanent mac address assigned.
We should assign permanent mac address before wlan0 of wl1251 is
registered into system.
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply
* Re: [PATCH 3/5] ath10k: Remove unused wmi_p2p_noa_descriptor 'noa' in wmi-tlv
From: Michal Kazior @ 2016-11-24 9:50 UTC (permalink / raw)
To: Kirtika Ruchandani
Cc: Kalle Valo, Arnd Bergmann, Network Development, linux-wireless,
Raja Mani
In-Reply-To: <99d0ff42e57d5f62560e72d926b4d69d5d7c418b.1479974100.git.kirtika@chromium.org>
On 24 November 2016 at 09:01, Kirtika Ruchandani
<kirtika.ruchandani@gmail.com> wrote:
> Commit ca996ec56608 (ath10k: implement wmi-tlv backend)
> introduced ath10k_wmi_tlv_op_gen_vdev_start() where
> 'struct wmi_p2p_noa_descriptor *noa' is defined and set but not used.
> Compiling with W=3D1 gives the following warning, fix it.
> drivers/net/wireless/ath/ath10k/wmi-tlv.c: In function =E2=80=98ath10k_wm=
i_tlv_op_gen_vdev_start=E2=80=99:
> drivers/net/wireless/ath/ath10k/wmi-tlv.c:1647:33: warning: variable =E2=
=80=98noa=E2=80=99 set but not used [-Wunused-but-set-variable]
>
> Fixes: ca996ec56608 ("ath10k: implement wmi-tlv backend")
> Cc: Michal Kazior <michal.kazior@tieto.com>
> Cc: Kalle Valo <kvalo@qca.qualcomm.com>
> Signed-off-by: Kirtika Ruchandani <kirtika@chromium.org>
> ---
> drivers/net/wireless/ath/ath10k/wmi-tlv.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wire=
less/ath/ath10k/wmi-tlv.c
> index e64f593..0e4bd29 100644
> --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> @@ -1644,7 +1644,6 @@ ath10k_wmi_tlv_op_gen_vdev_start(struct ath10k *ar,
> {
> struct wmi_tlv_vdev_start_cmd *cmd;
> struct wmi_channel *ch;
> - struct wmi_p2p_noa_descriptor *noa;
> struct wmi_tlv *tlv;
> struct sk_buff *skb;
> size_t len;
> @@ -1702,7 +1701,6 @@ ath10k_wmi_tlv_op_gen_vdev_start(struct ath10k *ar,
> tlv =3D ptr;
> tlv->tag =3D __cpu_to_le16(WMI_TLV_TAG_ARRAY_STRUCT);
> tlv->len =3D 0;
> - noa =3D (void *)tlv->value;
>
> /* Note: This is a nested TLV containing:
> * [wmi_tlv][wmi_p2p_noa_descriptor][wmi_tlv]..
I would rather keep this one as it serves as documentation. Would
"(void) noa;" be enough satisfy the compiler?
Micha=C5=82
^ permalink raw reply
* Re: [1/2] rsi: New firware loading method for RSI 91X devices
From: Kalle Valo @ 2016-11-24 11:50 UTC (permalink / raw)
To: Prameela Rani Garnepudi
Cc: Prameela Rani Garnepudi, linux-wireless, johannes.berg, hofrat,
xypron.glpk
In-Reply-To: <188f9293-f6a8-bd2b-8c2a-79b620fa0787@redpinesignals.com>
Prameela Rani Garnepudi <prameela.garnepudi@redpinesignals.com> writes:
>
> On 11/09/2016 06:45 AM, Kalle Valo wrote:
>> Prameela Rani Garnepudi <prameela.j04cs@gmail.com> wrote:
>>> RSI deprecated the old firmware loading method and introduced
>>> new method using soft boot loader for 9113 chipsets.
>>> Current driver only supports 9113 device model hence firmware
>>> loading method has been changed.
>>>
>>> In the new method, complete RAM image and flash image are present
>>> in the flash. Two firmwares present in the device, Boot loader firmware
>>> and functional firmware. Boot loader firmware is fixed but functional
>>> firmware can be changed. Before loading the functional firmware, host
>>> issues commands to check whether existing firmware in the chip and the
>>> firmware file content to load are same or not. If not, host issues
>>> commands to load the RAM image and then boot loaded switches to the
>>> functioanl firmware.
>>>
>>> Signed-off-by: Prameela Rani Garnepudi <prameela.j04cs@gmail.com>
>> These two patches are quite big, difficult to review. Smaller changes
>> would help with that. Will review later.
>>
>> 2 patches set to Deferred.
>>
>> 9388629 [1/2] rsi: New firware loading method for RSI 91X devices
>> 9388627 [2/2] rsi: Device initialization sequence is changed
>>
> Hi,
>
> Can you please let me know when will you consider to review these two
> patches. Because these are the mandatory and important patches to go
> into the mail line kernel. If it gets too late can I send each one as
> patch set? Please let me know urgently.
I'm still catching up with everything after my travels, sorry for the
delay. I'm hoping to get to your patches in next few days and give
better answers.
>From the last review I noticed that you both moved functions and made
functionality changes in the same patch. These are two logically
different changes and should be in different patches to make the review
easier. That's why I put them to deferred in the first place.
--
Kalle Valo
^ permalink raw reply
* Re: [RFC] qtn: add FullMAC firmware for Quantenna QSR10G wifi device
From: Kalle Valo @ 2016-11-24 11:59 UTC (permalink / raw)
To: IgorMitsyanko
Cc: Johannes Berg, linux-wireless, btherthala, hwang, smaksimenko,
dlebed, Igor Mitsyanko, Kamlesh Rath, Sergey Matyukevich,
Avinash Patil, Ben Hutchings, Kyle McMartin
In-Reply-To: <a5fa00c7-f8db-708a-cd21-4d5b0fbdf49b@quantenna.com>
IgorMitsyanko <igor.mitsyanko.os@quantenna.com> writes:
> On 11/23/2016 06:25 PM, Kalle Valo wrote:
>> IgorMitsyanko <igor.mitsyanko.os@quantenna.com> writes:
>>
>>> To clarify with you and Kalle, as persons involved with
>>> linux-wireless: is my understanding correct that submitting firmware
>>> into linux-fimware repository is a prerequisite to accepting new
>>> driver into linux-wireless?
>> In my opinion the most important is that the device is usable with an
>> upstream driver so that anyone can start using the driver (if they have
>> the hardware).
>>
>>> There is an option to start Quantenna device from internal flash
>>> memory, no external binary files involved. If we will introduce this
>>> functionality and remove code handling external firmware for now
>>> (until firmware problem resolved), would that allow driver to be
>>> reviewed/accepted?
>> Do all the publically available hardware contain the firmware in
>> internal flash (flashed in the factory)? Or is this something which must
>> be installed separately for each board's internal flash by the user?
>
> Each board must have flash installed on it, preflashed in the factory
> with uboot and firmware binary, otherwise board won't boot
Will the preflashed firmware binary will have all the normal
functionality needed by this driver? I mean that you can start an AP
interface etc.
> (won't boot without uboot, firmware itself is not mandatory).
Are you expecting that there are devices on the field which have uboot
preflashed but not the firmware?
> Booting from flash is default behavior on boards that are currently on
> the market, but for developemnt purpuses it's not very convenient and
> harder to upgrade.
Sure, I understand.
>> BTW, the original mail with the firmware image didn't make it to the
>> list, I guess it was too big? It would be good if you could post the
>> license separately so that people can see it.
>>
>
> I resent the email without binary patch to linux-wireless.
Saw it now, thanks.
--
Kalle Valo
^ permalink raw reply
* RE: [PATCH v3 4/5] mwifiex: wait firmware dump complete during card remove process
From: Amitkumar Karwar @ 2016-11-24 12:14 UTC (permalink / raw)
To: Brian Norris
Cc: linux-wireless@vger.kernel.org, Cathy Luo, Nishant Sarmukadam,
rajatja@google.com, dmitry.torokhov@gmail.com, Xinming Hu
In-Reply-To: <20161121173602.GA147125@google.com>
Hi Brian,
> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Monday, November 21, 2016 11:06 PM
> To: Amitkumar Karwar
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Nishant Sarmukadam;
> rajatja@google.com; dmitry.torokhov@gmail.com; Xinming Hu
> Subject: Re: [PATCH v3 4/5] mwifiex: wait firmware dump complete during
> card remove process
>
> Hi,
>
> On Wed, Nov 16, 2016 at 06:39:08PM +0530, Amitkumar Karwar wrote:
> > From: Xinming Hu <huxm@marvell.com>
> >
> > Wait for firmware dump complete in card remove function.
> > For sdio interface, there are two diffenrent cases, card reset
> trigger
> > sdio_work and firmware dump trigger sdio_work.
> > Do code rearrangement for distinguish between these two cases.
>
> On second review of the SDIO card reset code (which I'll repeat is
> quite ugly), you seem to be making a bad distinction here. What if
> there is a firmware dump happening concurrently with your card-reset
> handling?
> You *do* want to synchronize with the firmware dump before completing the
> card reset, or else you might be freeing up internal card resources
> that are still in use. See below.
I ran some tests and observed that if same work function is scheduled by two threads, it won't have re-entrant calls. They will be executed one after another.
In SDIO work function, we have SDIO card reset call after completing firmware dump.
So firmware dump won't run concurrently with card-reset as per my understanding.
>
> >
> > Signed-off-by: Xinming Hu <huxm@marvell.com>
> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> > ---
> > v2: 1. Get rid of reset_triggered flag. Instead split the code and
> use
> > __mwifiex_sdio_remove() (Brian Norris/Dmitry Torokhov)
> > 2. "v1 4/5 mwifiex: firmware dump code rearrangement.." is
> dropped. So
> > rebased accordingly.
> > v3: same as [v2,5/5]. The improvement of 'moving pcie_work to card
> struct'
> > suggested by Brian is taken care in next patch.
> > ---
> > drivers/net/wireless/marvell/mwifiex/pcie.c | 6 +++++-
> > drivers/net/wireless/marvell/mwifiex/sdio.c | 15 ++++++++++++---
> > 2 files changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c
> > b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > index dd8f7aa..c8e69a4 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> > @@ -51,6 +51,9 @@ static int mwifiex_pcie_probe_of(struct device
> *dev)
> > return 0;
> > }
> >
> > +static void mwifiex_pcie_work(struct work_struct *work); static
> > +DECLARE_WORK(pcie_work, mwifiex_pcie_work);
> > +
> > static int
> > mwifiex_map_pci_memory(struct mwifiex_adapter *adapter, struct
> sk_buff *skb,
> > size_t size, int flags)
> > @@ -254,6 +257,8 @@ static void mwifiex_pcie_remove(struct pci_dev
> *pdev)
> > if (!adapter || !adapter->priv_num)
> > return;
> >
> > + cancel_work_sync(&pcie_work);
> > +
> > if (user_rmmod && !adapter->mfg_mode) {
> > mwifiex_deauthenticate_all(adapter);
> >
> > @@ -2722,7 +2727,6 @@ static void mwifiex_pcie_work(struct
> work_struct *work)
> > mwifiex_pcie_device_dump_work(save_adapter);
> > }
> >
> > -static DECLARE_WORK(pcie_work, mwifiex_pcie_work);
> > /* This function dumps FW information */ static void
> > mwifiex_pcie_device_dump(struct mwifiex_adapter *adapter) { diff
> > --git a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > index 16d1d30..78f2cc9 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/sdio.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/sdio.c
> > @@ -46,6 +46,9 @@
> > */
> > static u8 user_rmmod;
> >
> > +static void mwifiex_sdio_work(struct work_struct *work); static
> > +DECLARE_WORK(sdio_work, mwifiex_sdio_work);
> > +
> > static struct mwifiex_if_ops sdio_ops; static unsigned long
> > iface_work_flags;
> >
> > @@ -220,7 +223,7 @@ static int mwifiex_sdio_resume(struct device
> *dev)
> > * This function removes the interface and frees up the card
> structure.
> > */
> > static void
> > -mwifiex_sdio_remove(struct sdio_func *func)
> > +__mwifiex_sdio_remove(struct sdio_func *func)
> > {
> > struct sdio_mmc_card *card;
> > struct mwifiex_adapter *adapter;
> > @@ -249,6 +252,13 @@ static int mwifiex_sdio_resume(struct device
> *dev)
> > mwifiex_remove_card(adapter);
> > }
> >
> > +static void
> > +mwifiex_sdio_remove(struct sdio_func *func) {
> > + cancel_work_sync(&sdio_work);
> > + __mwifiex_sdio_remove(func);
> > +}
> > +
> > /*
> > * SDIO suspend.
> > *
> > @@ -2227,7 +2237,7 @@ static void mwifiex_recreate_adapter(struct
> sdio_mmc_card *card)
> > * discovered and initializes them from scratch.
> > */
> >
> > - mwifiex_sdio_remove(func);
> > + __mwifiex_sdio_remove(func);
>
> ^^ So here, you're trying to avoid syncing with the card-reset work
> event, except that function will free up all your resources (including
> the static save_adapter). Thus, you're explicitly allowing a use-after-
> free error here. That seems unwise.
Even if firmware dump is triggered after card reset is started, it will execute after card reset is completed as discussed above. Only problem I can see is with "save_adapter". We can put new_adapter pointer in "save_adapter" at the end of mwifiex_recreate_adapter() to solve the issue.
>
> Instead, you should actually retain the invariant that you're doing a
> full remove/reinitialize here, which includes doing the *same*
> cancel_work_sync() here in mwifiex_recreate_adapter() as you would in
> any other remove().
We are executing mwifiex_recreate_adapter() as a part of sdio_work(). Calling cancel_work_sync() here would cause deadlock. The API is supposed to waits until sdio_work() is finished.
>
> IOW, kill the __mwifiex_sdio_remove() and just call
> mwifiex_sdio_remove() as you were.
>
> That also means that you can do the same per-adapter cleanup in the
> following patch as you do for PCIe.
Currently as sdio_work recreates "card", the work structure can't be moved inside card structure.
Let me know your suggestions.
Regards,
Amitkumar
^ permalink raw reply
* Re: ath9k ARMv7 OOPS in v4.8.6, v4.2.8
From: Jason Cooper @ 2016-11-24 12:28 UTC (permalink / raw)
To: miaoqing
Cc: Russell King - ARM Linux, linux-wireless, Linux ARM Kernel,
ath9k-devel, ath9k-devel, Thomas Petazzoni, Gregory CLEMENT,
Kalle Valo, Andrew Lunn
In-Reply-To: <7aba95bdf0d562f3c9640d533c33e1ca@codeaurora.org>
On Thu, Nov 24, 2016 at 02:06:57PM +0800, miaoqing@codeaurora.org wrote:
>
> >>Okay, so i was 0, so running UP probably isn't going to help. r7 is
> >>also spec_priv->rfs_chan_spec_scan.
> >>
> >>So, I think the question is... how is this NULL - and has it always
> >>been NULL...
> >
> >The problem appears to be that ath_cmn_process_fft() isn't called that
> >often. When it is, it crashes in ath_cmn_is_fft_buf_full() because
> >spec_priv->rfs_chan_spec_scan is NULL when ATH9K_DEBUGFS=n. :-(
> >
> >I'm running with ATH9K_DEBUGFS=y now. If it goes a couple of days
> >without crashing, I'll gin up a patch.
> >
>
> A similar patch was applied to ath-next branch:
> https://patchwork.kernel.org/patch/9431163/.
Hmm. Ok, I'm giving it a spin on my board with SMP=y, ATH9K_DEBUGFS=n
(so the only change from known crashing is the patch) and we'll see how
it goes.
Honestly, though, I think the real problem is when kernels are built
without ATH9K_DEBUGFS. Did the reporter of the crash say if that was
enabled on his system or not?
I'm concerned that there may be other code lurking that secretly depends
on ATH9K_DEBUGFS being enabled.
thx,
Jason.
^ permalink raw reply
* Re: ath9k ARMv7 OOPS in v4.8.6, v4.2.8
From: Jason Cooper @ 2016-11-24 12:33 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: linux-wireless, Linux ARM Kernel, ath9k-devel, ath9k-devel,
Thomas Petazzoni, Gregory CLEMENT, Kalle Valo, Andrew Lunn
In-Reply-To: <20161123214053.GJ2799@io.lakedaemon.net>
All,
On Wed, Nov 23, 2016 at 09:40:53PM +0000, Jason Cooper wrote:
> I'm running with ATH9K_DEBUGFS=y now. If it goes a couple of days
> without crashing, I'll gin up a patch.
Well, it survived overnight, which it's never done before. :-) I'm
testing the relay_open() NULL patch now.
thx,
Jason.
^ permalink raw reply
* RE: [PATCH] mwifiex: pcie: implement timeout loop for FW programming doorbell
From: Amitkumar Karwar @ 2016-11-24 13:16 UTC (permalink / raw)
To: Brian Norris, Nishant Sarmukadam
Cc: linux-kernel@vger.kernel.org, Kalle Valo,
linux-wireless@vger.kernel.org, Cathy Luo, Dmitry Torokhov
In-Reply-To: <1479868785-16263-1-git-send-email-briannorris@chromium.org>
> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Wednesday, November 23, 2016 8:10 AM
> To: Amitkumar Karwar; Nishant Sarmukadam
> Cc: linux-kernel@vger.kernel.org; Kalle Valo; linux-
> wireless@vger.kernel.org; Cathy Luo; Dmitry Torokhov; Brian Norris
> Subject: [PATCH] mwifiex: pcie: implement timeout loop for FW
> programming doorbell
>
> Marvell Wifi PCIe modules don't always behave nicely for PCIe power
> management when their firmware hasn't been loaded, particularly after
> suspending the PCIe link one or more times. When this happens, we might
> end up spinning forever in this status-polling tight loop. Let's make
> this less tight by adding a timeout and by sleeping a bit in between
> reads, as we do with the other similar loops.
>
> This prevents us from hogging a CPU even in such pathological cases,
> and allows the FW initialization to just fail gracefully instead.
>
> I chose the same polling parameters as the earlier loop in this
> function, and empirically, I found that this loop never makes it more
> than about 12 cycles in a sane FW init sequence. I had no official
> information on the actual intended latency for this portion of the
> download.
>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> drivers/net/wireless/marvell/mwifiex/pcie.c | 16 +++++++++++++---
> 1 file changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/pcie.c
> b/drivers/net/wireless/marvell/mwifiex/pcie.c
> index 4b89f557d0b6..9f9ea1350591 100644
> --- a/drivers/net/wireless/marvell/mwifiex/pcie.c
> +++ b/drivers/net/wireless/marvell/mwifiex/pcie.c
> @@ -2050,7 +2050,7 @@ static int mwifiex_prog_fw_w_helper(struct
> mwifiex_adapter *adapter,
> }
>
> /* Wait for the command done interrupt */
> - do {
> + for (tries = 0; tries < MAX_POLL_TRIES; tries++) {
> if (mwifiex_read_reg(adapter, PCIE_CPU_INT_STATUS,
> &ireg_intr)) {
> mwifiex_dbg(adapter, ERROR,
> @@ -2062,8 +2062,18 @@ static int mwifiex_prog_fw_w_helper(struct
> mwifiex_adapter *adapter,
> ret = -1;
> goto done;
> }
> - } while ((ireg_intr & CPU_INTR_DOOR_BELL) ==
> - CPU_INTR_DOOR_BELL);
> + if (!(ireg_intr & CPU_INTR_DOOR_BELL))
> + break;
> + usleep_range(10, 20);
> + }
> + if (ireg_intr & CPU_INTR_DOOR_BELL) {
> + mwifiex_dbg(adapter, ERROR, "%s: Card failed to ACK
> download\n",
> + __func__);
> + mwifiex_unmap_pci_memory(adapter, skb,
> + PCI_DMA_TODEVICE);
> + ret = -1;
> + goto done;
> + }
>
> mwifiex_unmap_pci_memory(adapter, skb, PCI_DMA_TODEVICE);
>
Patch looks fine to me.
Acked-by: Amitkumar Karwar <akarwar@marvell.com>
Regards,
Amitkumar
^ permalink raw reply
* [PATCH v2] ath9k: Introduce airtime fairness scheduling between stations
From: Toke Høiland-Jørgensen @ 2016-11-24 13:54 UTC (permalink / raw)
To: make-wifi-fast, linux-wireless; +Cc: Toke Høiland-Jørgensen
In-Reply-To: <20160617090929.31606-1-toke@toke.dk>
This reworks the ath9k driver to schedule transmissions to connected
stations in a way that enforces airtime fairness between them. It
accomplishes this by measuring the time spent transmitting to or
receiving from a station at TX and RX completion, and accounting this to
a per-station, per-QoS level airtime deficit. Then, an FQ-CoDel based
deficit scheduler is employed at packet dequeue time, to control which
station gets the next transmission opportunity.
Airtime fairness can significantly improve the efficiency of the network
when station rates vary. The following throughput values are from a
simple three-station test scenario, where two stations operate at the
highest HT20 rate, and one station at the lowest, and the scheduler is
employed at the access point:
Before / After
Fast station 1: 19.17 / 25.09 Mbps
Fast station 2: 19.83 / 25.21 Mbps
Slow station: 2.58 / 1.77 Mbps
Total: 41.58 / 52.07 Mbps
The benefit of airtime fairness goes up the more stations are present.
In a 30-station test with one station artificially limited to 1 Mbps,
we have seen aggregate throughput go from 2.14 to 17.76 Mbps.
Signed-off-by: Toke Høiland-Jørgensen <toke@toke.dk>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 28 ++++-
drivers/net/wireless/ath/ath9k/channel.c | 26 ++++-
drivers/net/wireless/ath/ath9k/debug.c | 3 +
drivers/net/wireless/ath/ath9k/debug.h | 13 +++
drivers/net/wireless/ath/ath9k/debug_sta.c | 54 +++++++++
drivers/net/wireless/ath/ath9k/init.c | 2 +
drivers/net/wireless/ath/ath9k/main.c | 6 +-
drivers/net/wireless/ath/ath9k/recv.c | 65 +++++++++++
drivers/net/wireless/ath/ath9k/xmit.c | 177 +++++++++++++++++++++--------
9 files changed, 318 insertions(+), 56 deletions(-)
diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index 378d345..d16f430 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -112,6 +112,8 @@ int ath_descdma_setup(struct ath_softc *sc, struct ath_descdma *dd,
#define ATH_TXFIFO_DEPTH 8
#define ATH_TX_ERROR 0x01
+#define ATH_AIRTIME_QUANTUM 300 /* usec */
+
/* Stop tx traffic 1ms before the GO goes away */
#define ATH_P2P_PS_STOP_TIME 1000
@@ -247,6 +249,9 @@ struct ath_atx_tid {
bool has_queued;
};
+void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid);
+void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid);
+
struct ath_node {
struct ath_softc *sc;
struct ieee80211_sta *sta; /* station struct we're part of */
@@ -258,9 +263,12 @@ struct ath_node {
bool sleeping;
bool no_ps_filter;
+ s64 airtime_deficit[IEEE80211_NUM_ACS];
+ u32 airtime_rx_start;
#ifdef CONFIG_ATH9K_STATION_STATISTICS
struct ath_rx_rate_stats rx_rate_stats;
+ struct ath_airtime_stats airtime_stats;
#endif
u8 key_idx[4];
@@ -317,10 +325,16 @@ struct ath_rx {
/* Channel Context */
/*******************/
+struct ath_acq {
+ struct list_head acq_new;
+ struct list_head acq_old;
+ spinlock_t lock;
+};
+
struct ath_chanctx {
struct cfg80211_chan_def chandef;
struct list_head vifs;
- struct list_head acq[IEEE80211_NUM_ACS];
+ struct ath_acq acq[IEEE80211_NUM_ACS];
int hw_queue_base;
/* do not dereference, use for comparison only */
@@ -443,6 +457,9 @@ void ath_chanctx_init(struct ath_softc *sc);
void ath_chanctx_set_channel(struct ath_softc *sc, struct ath_chanctx *ctx,
struct cfg80211_chan_def *chandef);
+void ath_acq_lock(struct ath_softc *sc, struct ath_acq *acq);
+void ath_acq_unlock(struct ath_softc *sc, struct ath_acq *acq);
+
#ifdef CONFIG_ATH9K_CHANNEL_CONTEXT
static inline struct ath_chanctx *
@@ -575,6 +592,8 @@ void ath_txq_schedule_all(struct ath_softc *sc);
int ath_tx_init(struct ath_softc *sc, int nbufs);
int ath_txq_update(struct ath_softc *sc, int qnum,
struct ath9k_tx_queue_info *q);
+u32 ath_pkt_duration(struct ath_softc *sc, u8 rix, int pktlen,
+ int width, int half_gi, bool shortPreamble);
void ath_update_max_aggr_framelen(struct ath_softc *sc, int queue, int txop);
void ath_assign_seq(struct ath_common *common, struct sk_buff *skb);
int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
@@ -963,6 +982,11 @@ void ath_ant_comb_scan(struct ath_softc *sc, struct ath_rx_status *rs);
#define ATH9K_NUM_CHANCTX 2 /* supports 2 operating channels */
+#define AIRTIME_USE_TX BIT(0)
+#define AIRTIME_USE_RX BIT(1)
+#define AIRTIME_USE_NEW_QUEUES BIT(2)
+#define AIRTIME_ACTIVE(flags) (!!(flags & (AIRTIME_USE_TX|AIRTIME_USE_RX)))
+
struct ath_softc {
struct ieee80211_hw *hw;
struct device *dev;
@@ -1005,6 +1029,8 @@ struct ath_softc {
short nbcnvifs;
unsigned long ps_usecount;
+ u16 airtime_flags; /* AIRTIME_* */
+
struct ath_rx rx;
struct ath_tx tx;
struct ath_beacon beacon;
diff --git a/drivers/net/wireless/ath/ath9k/channel.c b/drivers/net/wireless/ath/ath9k/channel.c
index 929dd70..5d4d682 100644
--- a/drivers/net/wireless/ath/ath9k/channel.c
+++ b/drivers/net/wireless/ath/ath9k/channel.c
@@ -118,8 +118,11 @@ void ath_chanctx_init(struct ath_softc *sc)
INIT_LIST_HEAD(&ctx->vifs);
ctx->txpower = ATH_TXPOWER_MAX;
ctx->flush_timeout = HZ / 5; /* 200ms */
- for (j = 0; j < ARRAY_SIZE(ctx->acq); j++)
- INIT_LIST_HEAD(&ctx->acq[j]);
+ for (j = 0; j < ARRAY_SIZE(ctx->acq); j++) {
+ INIT_LIST_HEAD(&ctx->acq[j].acq_new);
+ INIT_LIST_HEAD(&ctx->acq[j].acq_old);
+ spin_lock_init(&ctx->acq[j].lock);
+ }
}
}
@@ -144,6 +147,18 @@ void ath_chanctx_set_channel(struct ath_softc *sc, struct ath_chanctx *ctx,
ath_set_channel(sc);
}
+void ath_acq_lock(struct ath_softc *sc, struct ath_acq *acq)
+ __acquires(&acq->lock)
+{
+ spin_lock_bh(&acq->lock);
+}
+
+void ath_acq_unlock(struct ath_softc *sc, struct ath_acq *acq)
+ __releases(&acq->lock)
+{
+ spin_unlock_bh(&acq->lock);
+}
+
#ifdef CONFIG_ATH9K_CHANNEL_CONTEXT
/*************/
@@ -1345,8 +1360,11 @@ void ath9k_offchannel_init(struct ath_softc *sc)
ctx->txpower = ATH_TXPOWER_MAX;
cfg80211_chandef_create(&ctx->chandef, chan, NL80211_CHAN_HT20);
- for (i = 0; i < ARRAY_SIZE(ctx->acq); i++)
- INIT_LIST_HEAD(&ctx->acq[i]);
+ for (i = 0; i < ARRAY_SIZE(ctx->acq); i++) {
+ INIT_LIST_HEAD(&ctx->acq[i].acq_new);
+ INIT_LIST_HEAD(&ctx->acq[i].acq_old);
+ spin_lock_init(&ctx->acq[i].lock);
+ }
sc->offchannel.chan.offchannel = true;
}
diff --git a/drivers/net/wireless/ath/ath9k/debug.c b/drivers/net/wireless/ath/ath9k/debug.c
index 89a94dd..43930c3 100644
--- a/drivers/net/wireless/ath/ath9k/debug.c
+++ b/drivers/net/wireless/ath/ath9k/debug.c
@@ -1399,5 +1399,8 @@ int ath9k_init_debug(struct ath_hw *ah)
debugfs_create_file("tpc", S_IRUSR | S_IWUSR,
sc->debug.debugfs_phy, sc, &fops_tpc);
+ debugfs_create_u16("airtime_flags", S_IRUSR | S_IWUSR,
+ sc->debug.debugfs_phy, &sc->airtime_flags);
+
return 0;
}
diff --git a/drivers/net/wireless/ath/ath9k/debug.h b/drivers/net/wireless/ath/ath9k/debug.h
index a078cdd..249f814 100644
--- a/drivers/net/wireless/ath/ath9k/debug.h
+++ b/drivers/net/wireless/ath/ath9k/debug.h
@@ -221,6 +221,11 @@ struct ath_rx_rate_stats {
} cck_stats[4];
};
+struct ath_airtime_stats {
+ u32 rx_airtime;
+ u32 tx_airtime;
+};
+
#define ANT_MAIN 0
#define ANT_ALT 1
@@ -314,12 +319,20 @@ ath9k_debug_sync_cause(struct ath_softc *sc, u32 sync_cause)
void ath_debug_rate_stats(struct ath_softc *sc,
struct ath_rx_status *rs,
struct sk_buff *skb);
+void ath_debug_airtime(struct ath_softc *sc,
+ struct ath_node *an,
+ u32 rx, u32 tx);
#else
static inline void ath_debug_rate_stats(struct ath_softc *sc,
struct ath_rx_status *rs,
struct sk_buff *skb)
{
}
+static inline void ath_debug_airtime(struct ath_softc *sc,
+ struct ath_node *an,
+ u32 rx, u32 tx)
+{
+}
#endif /* CONFIG_ATH9K_STATION_STATISTICS */
#endif /* DEBUG_H */
diff --git a/drivers/net/wireless/ath/ath9k/debug_sta.c b/drivers/net/wireless/ath/ath9k/debug_sta.c
index 2a3a3c4..524cbf13 100644
--- a/drivers/net/wireless/ath/ath9k/debug_sta.c
+++ b/drivers/net/wireless/ath/ath9k/debug_sta.c
@@ -242,6 +242,59 @@ static const struct file_operations fops_node_recv = {
.llseek = default_llseek,
};
+void ath_debug_airtime(struct ath_softc *sc,
+ struct ath_node *an,
+ u32 rx,
+ u32 tx)
+{
+ struct ath_airtime_stats *astats = &an->airtime_stats;
+
+ astats->rx_airtime += rx;
+ astats->tx_airtime += tx;
+}
+
+static ssize_t read_airtime(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ath_node *an = file->private_data;
+ struct ath_airtime_stats *astats;
+ static const char *qname[4] = {
+ "VO", "VI", "BE", "BK"
+ };
+ u32 len = 0, size = 256;
+ char *buf;
+ size_t retval;
+ int i;
+
+ buf = kzalloc(size, GFP_KERNEL);
+ if (buf == NULL)
+ return -ENOMEM;
+
+ astats = &an->airtime_stats;
+
+ len += scnprintf(buf + len, size - len, "RX: %u us\n", astats->rx_airtime);
+ len += scnprintf(buf + len, size - len, "TX: %u us\n", astats->tx_airtime);
+ len += scnprintf(buf + len, size - len, "Deficit: ");
+ for (i = 0; i < 4; i++)
+ len += scnprintf(buf+len, size - len, "%s: %lld us ", qname[i], an->airtime_deficit[i]);
+ if (len < size)
+ buf[len++] = '\n';
+
+ retval = simple_read_from_buffer(user_buf, count, ppos, buf, len);
+ kfree(buf);
+
+ return retval;
+}
+
+
+static const struct file_operations fops_airtime = {
+ .read = read_airtime,
+ .open = simple_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
+
void ath9k_sta_add_debugfs(struct ieee80211_hw *hw,
struct ieee80211_vif *vif,
struct ieee80211_sta *sta,
@@ -251,4 +304,5 @@ void ath9k_sta_add_debugfs(struct ieee80211_hw *hw,
debugfs_create_file("node_aggr", S_IRUGO, dir, an, &fops_node_aggr);
debugfs_create_file("node_recv", S_IRUGO, dir, an, &fops_node_recv);
+ debugfs_create_file("airtime", S_IRUGO, dir, an, &fops_airtime);
}
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index c0c8bf0..5aa665f 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -620,6 +620,8 @@ static int ath9k_init_softc(u16 devid, struct ath_softc *sc,
/* Will be cleared in ath9k_start() */
set_bit(ATH_OP_INVALID, &common->op_flags);
+ sc->airtime_flags = (AIRTIME_USE_TX | AIRTIME_USE_RX |
+ AIRTIME_USE_NEW_QUEUES);
sc->sc_ah = ah;
sc->dfs_detector = dfs_pattern_detector_init(common, NL80211_DFS_UNSET);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 59e3bd0..58f06ce 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -70,10 +70,10 @@ static bool ath9k_has_pending_frames(struct ath_softc *sc, struct ath_txq *txq,
goto out;
if (txq->mac80211_qnum >= 0) {
- struct list_head *list;
+ struct ath_acq *acq;
- list = &sc->cur_chan->acq[txq->mac80211_qnum];
- if (!list_empty(list))
+ acq = &sc->cur_chan->acq[txq->mac80211_qnum];
+ if (!list_empty(&acq->acq_new) || !list_empty(&acq->acq_old))
pending = true;
}
out:
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6697342..9f3880d 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -991,6 +991,70 @@ static void ath9k_apply_ampdu_details(struct ath_softc *sc,
}
}
+static void ath_rx_count_airtime(struct ath_softc *sc,
+ struct ath_rx_status *rs,
+ struct sk_buff *skb)
+{
+ struct ath_node *an;
+ struct ath_acq *acq;
+ struct ath_vif *avp;
+ struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
+ struct ath_hw *ah = sc->sc_ah;
+ struct ath_common *common = ath9k_hw_common(ah);
+ struct ieee80211_sta *sta;
+ struct ieee80211_rx_status *rxs;
+ const struct ieee80211_rate *rate;
+ bool is_sgi, is_40, is_sp;
+ int phy;
+ u16 len = rs->rs_datalen;
+ u32 airtime = 0;
+ u8 tidno, acno;
+
+ if (!ieee80211_is_data(hdr->frame_control))
+ return;
+
+ rcu_read_lock();
+
+ sta = ieee80211_find_sta_by_ifaddr(sc->hw, hdr->addr2, NULL);
+ if (!sta)
+ goto exit;
+ an = (struct ath_node *) sta->drv_priv;
+ avp = (struct ath_vif *) an->vif->drv_priv;
+ tidno = skb->priority & IEEE80211_QOS_CTL_TID_MASK;
+ acno = TID_TO_WME_AC(tidno);
+ acq = &avp->chanctx->acq[acno];
+
+ rxs = IEEE80211_SKB_RXCB(skb);
+
+ is_sgi = !!(rxs->flag & RX_FLAG_SHORT_GI);
+ is_40 = !!(rxs->flag & RX_FLAG_40MHZ);
+ is_sp = !!(rxs->flag & RX_FLAG_SHORTPRE);
+
+ if (!!(rxs->flag & RX_FLAG_HT)) {
+ /* MCS rates */
+
+ airtime += ath_pkt_duration(sc, rxs->rate_idx, len,
+ is_40, is_sgi, is_sp);
+ } else {
+
+ phy = IS_CCK_RATE(rs->rs_rate) ? WLAN_RC_PHY_CCK : WLAN_RC_PHY_OFDM;
+ rate = &common->sbands[rxs->band].bitrates[rxs->rate_idx];
+ airtime += ath9k_hw_computetxtime(ah, phy, rate->bitrate * 100,
+ len, rxs->rate_idx, is_sp);
+ }
+
+ if (!!(sc->airtime_flags & AIRTIME_USE_RX)) {
+ ath_acq_lock(sc, acq);
+ an->airtime_deficit[acno] -= airtime;
+ if (an->airtime_deficit[acno] <= 0)
+ __ath_tx_queue_tid(sc, ATH_AN_2_TID(an, tidno));
+ ath_acq_unlock(sc, acq);
+ }
+ ath_debug_airtime(sc, an, airtime, 0);
+exit:
+ rcu_read_unlock();
+}
+
int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
{
struct ath_rxbuf *bf;
@@ -1137,6 +1201,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
ath9k_antenna_check(sc, &rs);
ath9k_apply_ampdu_details(sc, &rs, rxs);
ath_debug_rate_stats(sc, &rs, skb);
+ ath_rx_count_airtime(sc, &rs, skb);
hdr = (struct ieee80211_hdr *)skb->data;
if (ieee80211_is_ack(hdr->frame_control))
diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 486afa9..7386e93 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -124,21 +124,44 @@ void ath_txq_unlock_complete(struct ath_softc *sc, struct ath_txq *txq)
ath_tx_status(hw, skb);
}
-static void ath_tx_queue_tid(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid)
+void __ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
{
- struct list_head *list;
struct ath_vif *avp = (struct ath_vif *) tid->an->vif->drv_priv;
struct ath_chanctx *ctx = avp->chanctx;
+ struct ath_acq *acq;
+ struct list_head *tid_list;
+ u8 acno = TID_TO_WME_AC(tid->tidno);
- if (!ctx)
+ if (!ctx || !list_empty(&tid->list))
return;
- list = &ctx->acq[TID_TO_WME_AC(tid->tidno)];
- if (list_empty(&tid->list))
- list_add_tail(&tid->list, list);
+
+ acq = &ctx->acq[acno];
+ if ((sc->airtime_flags & AIRTIME_USE_NEW_QUEUES) &&
+ tid->an->airtime_deficit[acno] > 0)
+ tid_list = &acq->acq_new;
+ else
+ tid_list = &acq->acq_old;
+
+ list_add_tail(&tid->list, tid_list);
}
+void ath_tx_queue_tid(struct ath_softc *sc, struct ath_atx_tid *tid)
+{
+ struct ath_vif *avp = (struct ath_vif *) tid->an->vif->drv_priv;
+ struct ath_chanctx *ctx = avp->chanctx;
+ struct ath_acq *acq;
+
+ if (!ctx || !list_empty(&tid->list))
+ return;
+
+ acq = &ctx->acq[TID_TO_WME_AC(tid->tidno)];
+ ath_acq_lock(sc, acq);
+ __ath_tx_queue_tid(sc, tid);
+ ath_acq_unlock(sc, acq);
+}
+
+
void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
{
struct ath_softc *sc = hw->priv;
@@ -153,7 +176,7 @@ void ath9k_wake_tx_queue(struct ieee80211_hw *hw, struct ieee80211_txq *queue)
ath_txq_lock(sc, txq);
tid->has_queued = true;
- ath_tx_queue_tid(sc, txq, tid);
+ ath_tx_queue_tid(sc, tid);
ath_txq_schedule(sc, txq);
ath_txq_unlock(sc, txq);
@@ -660,7 +683,7 @@ static void ath_tx_complete_aggr(struct ath_softc *sc, struct ath_txq *txq,
skb_queue_splice_tail(&bf_pending, &tid->retry_q);
if (!an->sleeping) {
- ath_tx_queue_tid(sc, txq, tid);
+ ath_tx_queue_tid(sc, tid);
if (ts->ts_status & (ATH9K_TXERR_FILT | ATH9K_TXERR_XRETRY))
tid->clear_ps_filter = true;
@@ -688,6 +711,53 @@ static bool bf_is_ampdu_not_probing(struct ath_buf *bf)
return bf_isampdu(bf) && !(info->flags & IEEE80211_TX_CTL_RATE_CTRL_PROBE);
}
+static void ath_tx_count_airtime(struct ath_softc *sc, struct ath_txq *txq,
+ struct ath_buf *bf, struct ath_tx_status *ts)
+{
+ struct ath_node *an;
+ struct ath_acq *acq = &sc->cur_chan->acq[txq->mac80211_qnum];
+ struct sk_buff *skb;
+ struct ieee80211_hdr *hdr;
+ struct ieee80211_hw *hw = sc->hw;
+ struct ieee80211_tx_rate rates[4];
+ struct ieee80211_sta *sta;
+ int i;
+ u32 airtime = 0;
+
+ skb = bf->bf_mpdu;
+ if(!skb)
+ return;
+
+ hdr = (struct ieee80211_hdr *)skb->data;
+ memcpy(rates, bf->rates, sizeof(rates));
+
+ rcu_read_lock();
+
+ sta = ieee80211_find_sta_by_ifaddr(hw, hdr->addr1, hdr->addr2);
+ if(!sta)
+ goto exit;
+
+
+ an = (struct ath_node *) sta->drv_priv;
+
+ airtime += ts->duration * (ts->ts_longretry + 1);
+
+ for(i=0; i < ts->ts_rateindex; i++)
+ airtime += ath9k_hw_get_duration(sc->sc_ah, bf->bf_desc, i) * rates[i].count;
+
+ if (!!(sc->airtime_flags & AIRTIME_USE_TX)) {
+ ath_acq_lock(sc, acq);
+ an->airtime_deficit[txq->mac80211_qnum] -= airtime;
+ if (an->airtime_deficit[txq->mac80211_qnum] <= 0)
+ __ath_tx_queue_tid(sc, ath_get_skb_tid(sc, an, skb));
+ ath_acq_unlock(sc, acq);
+ }
+ ath_debug_airtime(sc, an, 0, airtime);
+
+exit:
+ rcu_read_unlock();
+}
+
static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
struct ath_tx_status *ts, struct ath_buf *bf,
struct list_head *bf_head)
@@ -709,6 +779,7 @@ static void ath_tx_process_buffer(struct ath_softc *sc, struct ath_txq *txq,
ts->duration = ath9k_hw_get_duration(sc->sc_ah, bf->bf_desc,
ts->ts_rateindex);
+ ath_tx_count_airtime(sc, txq, bf, ts);
hdr = (struct ieee80211_hdr *) bf->bf_mpdu->data;
sta = ieee80211_find_sta_by_ifaddr(hw, hdr->addr1, hdr->addr2);
@@ -1068,8 +1139,8 @@ ath_tx_form_aggr(struct ath_softc *sc, struct ath_txq *txq,
* width - 0 for 20 MHz, 1 for 40 MHz
* half_gi - to use 4us v/s 3.6 us for symbol time
*/
-static u32 ath_pkt_duration(struct ath_softc *sc, u8 rix, int pktlen,
- int width, int half_gi, bool shortPreamble)
+u32 ath_pkt_duration(struct ath_softc *sc, u8 rix, int pktlen,
+ int width, int half_gi, bool shortPreamble)
{
u32 nbits, nsymbits, duration, nsymbols;
int streams;
@@ -1467,7 +1538,7 @@ ath_tx_form_burst(struct ath_softc *sc, struct ath_txq *txq,
}
static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
- struct ath_atx_tid *tid, bool *stop)
+ struct ath_atx_tid *tid)
{
struct ath_buf *bf;
struct ieee80211_tx_info *tx_info;
@@ -1489,7 +1560,6 @@ static bool ath_tx_sched_aggr(struct ath_softc *sc, struct ath_txq *txq,
if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
(!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH)) {
__skb_queue_tail(&tid->retry_q, bf->bf_mpdu);
- *stop = true;
return false;
}
@@ -1613,7 +1683,7 @@ void ath_tx_aggr_wakeup(struct ath_softc *sc, struct ath_node *an)
ath_txq_lock(sc, txq);
tid->clear_ps_filter = true;
if (ath_tid_has_buffered(tid)) {
- ath_tx_queue_tid(sc, txq, tid);
+ ath_tx_queue_tid(sc, tid);
ath_txq_schedule(sc, txq);
}
ath_txq_unlock_complete(sc, txq);
@@ -1912,9 +1982,10 @@ void ath_tx_cleanupq(struct ath_softc *sc, struct ath_txq *txq)
void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
{
struct ath_common *common = ath9k_hw_common(sc->sc_ah);
- struct ath_atx_tid *tid, *last_tid;
+ struct ath_atx_tid *tid;
struct list_head *tid_list;
- bool sent = false;
+ struct ath_acq *acq;
+ bool active = AIRTIME_ACTIVE(sc->airtime_flags);
if (txq->mac80211_qnum < 0)
return;
@@ -1923,48 +1994,55 @@ void ath_txq_schedule(struct ath_softc *sc, struct ath_txq *txq)
return;
spin_lock_bh(&sc->chan_lock);
- tid_list = &sc->cur_chan->acq[txq->mac80211_qnum];
-
- if (list_empty(tid_list)) {
- spin_unlock_bh(&sc->chan_lock);
- return;
- }
-
rcu_read_lock();
+ acq = &sc->cur_chan->acq[txq->mac80211_qnum];
- last_tid = list_entry(tid_list->prev, struct ath_atx_tid, list);
- while (!list_empty(tid_list)) {
- bool stop = false;
-
- if (sc->cur_chan->stopped)
- break;
-
- tid = list_first_entry(tid_list, struct ath_atx_tid, list);
- list_del_init(&tid->list);
+ if (sc->cur_chan->stopped)
+ goto out;
- if (ath_tx_sched_aggr(sc, txq, tid, &stop))
- sent = true;
+begin:
+ tid_list = &acq->acq_new;
+ if (list_empty(tid_list)) {
+ tid_list = &acq->acq_old;
+ if (list_empty(tid_list))
+ goto out;
+ }
+ tid = list_first_entry(tid_list, struct ath_atx_tid, list);
- /*
- * add tid to round-robin queue if more frames
- * are pending for the tid
- */
- if (ath_tid_has_buffered(tid))
- ath_tx_queue_tid(sc, txq, tid);
+ if (active && tid->an->airtime_deficit[txq->mac80211_qnum] <= 0) {
+ ath_acq_lock(sc, acq);
+ tid->an->airtime_deficit[txq->mac80211_qnum] += ATH_AIRTIME_QUANTUM;
+ list_move_tail(&tid->list, &acq->acq_old);
+ ath_acq_unlock(sc, acq);
+ goto begin;
+ }
- if (stop)
- break;
+ if (!ath_tid_has_buffered(tid)) {
+ ath_acq_lock(sc, acq);
+ if ((tid_list == &acq->acq_new) && !list_empty(&acq->acq_old))
+ list_move_tail(&tid->list, &acq->acq_old);
+ else {
+ list_del_init(&tid->list);
+ }
+ ath_acq_unlock(sc, acq);
+ goto begin;
+ }
- if (tid == last_tid) {
- if (!sent)
- break;
- sent = false;
- last_tid = list_entry(tid_list->prev,
- struct ath_atx_tid, list);
+ /*
+ * If we succeed in scheduling something, immediately restart to make
+ * sure we keep the HW busy.
+ */
+ if(ath_tx_sched_aggr(sc, txq, tid)) {
+ if (!active) {
+ ath_acq_lock(sc, acq);
+ list_move_tail(&tid->list, &acq->acq_old);
+ ath_acq_unlock(sc, acq);
}
+ goto begin;
}
+out:
rcu_read_unlock();
spin_unlock_bh(&sc->chan_lock);
}
@@ -2818,6 +2896,9 @@ void ath_tx_node_init(struct ath_softc *sc, struct ath_node *an)
struct ath_atx_tid *tid;
int tidno, acno;
+ for (acno = 0; acno < IEEE80211_NUM_ACS; acno++)
+ an->airtime_deficit[acno] = ATH_AIRTIME_QUANTUM;
+
for (tidno = 0; tidno < IEEE80211_NUM_TIDS; tidno++) {
tid = ath_node_to_tid(an, tidno);
tid->an = an;
base-commit: 705d7aa062203226c8df73f18622664e30bd8a61
--
2.10.2
^ permalink raw reply related
* [PATCH] ath5k: drop duplicate header vmalloc.h
From: Geliang Tang @ 2016-11-24 13:58 UTC (permalink / raw)
To: Jiri Slaby, Nick Kossifidis, Luis R. Rodriguez, Kalle Valo
Cc: Geliang Tang, linux-wireless, netdev, linux-kernel
Drop duplicate header vmalloc.h from ath5k/debug.c.
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
drivers/net/wireless/ath/ath5k/debug.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath5k/debug.c b/drivers/net/wireless/ath/ath5k/debug.c
index 4f8d9ed..d068df5 100644
--- a/drivers/net/wireless/ath/ath5k/debug.c
+++ b/drivers/net/wireless/ath/ath5k/debug.c
@@ -66,7 +66,6 @@
#include <linux/seq_file.h>
#include <linux/list.h>
-#include <linux/vmalloc.h>
#include "debug.h"
#include "ath5k.h"
#include "reg.h"
--
2.9.3
^ permalink raw reply related
* [PATCH] NFC: nfcmrvl: drop duplicate header gpio.h
From: Geliang Tang @ 2016-11-24 13:58 UTC (permalink / raw)
To: Lauro Ramos Venancio, Aloisio Almeida Jr, Samuel Ortiz
Cc: Geliang Tang, linux-wireless, linux-kernel
In-Reply-To: <15299de49216a2360976ca37ff774cae9d27d88b.1479991297.git.geliangtang@gmail.com>
Drop duplicate header gpio.h from nfcmrvl/spi.c.
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
---
drivers/nfc/nfcmrvl/spi.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/drivers/nfc/nfcmrvl/spi.c b/drivers/nfc/nfcmrvl/spi.c
index a7faa0b..e2881b15 100644
--- a/drivers/nfc/nfcmrvl/spi.c
+++ b/drivers/nfc/nfcmrvl/spi.c
@@ -26,7 +26,6 @@
#include <net/nfc/nci.h>
#include <net/nfc/nci_core.h>
#include <linux/spi/spi.h>
-#include <linux/gpio.h>
#include "nfcmrvl.h"
#define SPI_WAIT_HANDSHAKE 1
--
2.9.3
^ permalink raw reply related
* Re: many "changed bandwidth, new config is" messages in the log
From: Johannes Berg @ 2016-11-24 15:12 UTC (permalink / raw)
To: Michal Hocko; +Cc: LKML, linux-wireless
In-Reply-To: <20161124150723.GA4776@dhcp22.suse.cz>
On Thu, 2016-11-24 at 15:07 +0000, Michal Hocko wrote:
> I have only now managed to move to 4.9-rc5 (from 4.8) and started
> seeing quite a lot of following messages
> "
> [ 346.612211] wlan0: AP c0:4a:00:f1:48:f2 changed bandwidth, new
> config is 2472 MHz, width 1 (2472/0 MHz)
> [ 352.655929] wlan0: AP c0:4a:00:f1:48:f2 changed bandwidth, new
> config is 2472 MHz, width 2 (2462/0 MHz)
> "
I don't think these messages are new in any way. checking ... nope,
it's been around that way since 3.9 :-)
> It always seems to be changing width from 1 -> 2 and back
Makes sense, that's 20 MHz <-> 40 MHz.
Did you buy a new device that says it's 40 MHz incompatible or
something? Or perhaps one of your neighbors did ... Or something is
causing interference that makes the AP switch around.
> $ dmesg | grep "changed bandwidth" | wc -l
> 42
> in 13 minutes of uptime. I have noticed this came in via 30eb1dc2c430
> ("mac80211: properly track HT/VHT operation changes").
Right, but that went into 3.9 :-)
> Is this something to be worried about?
Not at all. I suppose we could make this a debug message though, it's
not super useful when it happens OK (sometimes it causes disconnections
when we can't support the new mode, which is more relevant).
johannes
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Sebastian Reichel @ 2016-11-24 15:13 UTC (permalink / raw)
To: Pali Rohár
Cc: Pavel Machek, Michal Kazior, Kalle Valo, Ivaylo Dimitrov,
Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
linux-kernel
In-Reply-To: <20161124083329.GB13735@pali>
[-- Attachment #1: Type: text/plain, Size: 1504 bytes --]
Hi,
On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote:
> On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> > Hi!
> >
> > > > "ifconfig hw ether XX" normally sets the address. I guess that's
> > > > ioctl?
> > >
> > > This sets temporary address and it is ioctl. IIRC same as what ethtool
> > > uses. (ifconfig is already deprecated).
> > >
> > > > And I guess we should use similar mechanism for permanent
> > > > address.
> > >
> > > I'm not sure here... Above ioctl ↑↑↑ is for changing temporary mac
> > > address. But here we do not want to change permanent mac address. We
> > > want to tell kernel driver current permanent mac address which is
> > > stored
> >
> > Well... I'd still use similar mechanism :-).
>
> Thats problematic, because in time when wlan0 interface is registered
> into system and visible in ifconfig output it already needs to have
> permanent mac address assigned.
>
> We should assign permanent mac address before wlan0 of wl1251 is
> registered into system.
You can just add the MAC address to the NVS data, which is also
required for the device initialization.
I wonder if those information could be put into DT. Iirc some
network devices get their MAC address from DT. Maybe we can add
all NVS info to DT? How much data is it?
Userspace application can add all those information to the DT
using a DT overlay. Also the u-boot could parse and add it at
some point in the future.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-11-24 15:20 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pavel Machek, Michal Kazior, Kalle Valo, Ivaylo Dimitrov,
Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
linux-kernel
In-Reply-To: <20161124151317.34yoza3dzuh46oa4@earth>
On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote:
> Hi,
>
> On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote:
> > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> > > Hi!
> > >
> > > > > "ifconfig hw ether XX" normally sets the address. I guess that's
> > > > > ioctl?
> > > >
> > > > This sets temporary address and it is ioctl. IIRC same as what ethtool
> > > > uses. (ifconfig is already deprecated).
> > > >
> > > > > And I guess we should use similar mechanism for permanent
> > > > > address.
> > > >
> > > > I'm not sure here... Above ioctl ↑↑↑ is for changing temporary mac
> > > > address. But here we do not want to change permanent mac address. We
> > > > want to tell kernel driver current permanent mac address which is
> > > > stored
> > >
> > > Well... I'd still use similar mechanism :-).
> >
> > Thats problematic, because in time when wlan0 interface is registered
> > into system and visible in ifconfig output it already needs to have
> > permanent mac address assigned.
> >
> > We should assign permanent mac address before wlan0 of wl1251 is
> > registered into system.
>
> You can just add the MAC address to the NVS data, which is also
> required for the device initialization.
NVS data file has fixed size, there is IIRC no place for it.
But one of my suggestion was to use another request_firmware for MAC
address. So this is similar to what you are proposing, as NVS data are
loaded by request_firmware too...
> I wonder if those information could be put into DT. Iirc some
> network devices get their MAC address from DT. Maybe we can add
> all NVS info to DT? How much data is it?
Proprietary, signed and closed bootloader NOLO does not support DT. So
for booting you need to append DTS file to kernel image.
U-Boot is optional and can be used as intermediate bootloader between
NOLO and kernel. But still it has problems with reading from nand, so
cannot read NVS data nor MAC address.
> Userspace application can add all those information to the DT
> using a DT overlay. Also the u-boot could parse and add it at
> some point in the future.
In case when wl1251 is statically linked into kernel image, it is loaded
and initialized before even userspace applications starts.
So no... adding NVS data or MAC address into DT or DT overlay is not a
solution.
--
Pali Rohár
pali.rohar@gmail.com
^ permalink raw reply
* Re: many "changed bandwidth, new config is" messages in the log
From: Michal Hocko @ 2016-11-24 15:25 UTC (permalink / raw)
To: Johannes Berg; +Cc: LKML, linux-wireless
In-Reply-To: <1480000334.4388.2.camel@sipsolutions.net>
On Thu 24-11-16 16:12:14, Johannes Berg wrote:
> On Thu, 2016-11-24 at 15:07 +0000, Michal Hocko wrote:
>
> > I have only now managed to move to 4.9-rc5 (from 4.8) and started
> > seeing quite a lot of following messages
> > "
> > [ 346.612211] wlan0: AP c0:4a:00:f1:48:f2 changed bandwidth, new
> > config is 2472 MHz, width 1 (2472/0 MHz)
> > [ 352.655929] wlan0: AP c0:4a:00:f1:48:f2 changed bandwidth, new
> > config is 2472 MHz, width 2 (2462/0 MHz)
> > "
>
> I don't think these messages are new in any way. checking ... nope,
> it's been around that way since 3.9 :-)
Right you are. I must have missed them before and git grep + git blame
fooled me.
> > It always seems to be changing width from 1 -> 2 and back
>
> Makes sense, that's 20 MHz <-> 40 MHz.
>
> Did you buy a new device that says it's 40 MHz incompatible or
> something?
I am using this device for years now. It is a cheap TP-Link home
wireless router. So hard to tell about above. I am far from an expert.
> Or perhaps one of your neighbors did ... Or something is
> causing interference that makes the AP switch around.
This might be possible. There are quite some devices broadcasting around
$ sudo iwlist wlan0 scan | grep "Channel:" | sort | uniq -c
6 Channel:1
6 Channel:11
1 Channel:112
2 Channel:13
2 Channel:3
7 Channel:6
1 Channel:9
my router is at channel 13 so there seems to be something else sitting
there as well.
> > $ dmesg | grep "changed bandwidth" | wc -l
> > 42
> > in 13 minutes of uptime. I have noticed this came in via 30eb1dc2c430
> > ("mac80211: properly track HT/VHT operation changes").
>
> Right, but that went into 3.9 :-)
>
> > Is this something to be worried about?
>
> Not at all. I suppose we could make this a debug message though, it's
> not super useful when it happens OK (sometimes it causes disconnections
> when we can't support the new mode, which is more relevant).
OK, I see. Then I would suggest lowering the loglevel ;)
Thanks!
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Ivaylo Dimitrov @ 2016-11-24 15:31 UTC (permalink / raw)
To: Pali Rohár, Sebastian Reichel
Cc: Pavel Machek, Michal Kazior, Kalle Valo, Aaro Koskinen,
Tony Lindgren, linux-wireless, Network Development, linux-kernel
In-Reply-To: <20161124152045.GK13735@pali>
Hi,
On 24.11.2016 17:20, Pali Rohár wrote:
>
> In case when wl1251 is statically linked into kernel image, it is loaded
> and initialized before even userspace applications starts.
>
Which leaves no option, but integrating libcal into kernel :).
Ivo
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Sebastian Reichel @ 2016-11-24 16:08 UTC (permalink / raw)
To: Pali Rohár
Cc: Pavel Machek, Michal Kazior, Kalle Valo, Ivaylo Dimitrov,
Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
linux-kernel
In-Reply-To: <20161124152045.GK13735@pali>
[-- Attachment #1: Type: text/plain, Size: 2902 bytes --]
Hi,
On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote:
> On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote:
> > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote:
> > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> > > > > > "ifconfig hw ether XX" normally sets the address. I guess that's
> > > > > > ioctl?
> > > > >
> > > > > This sets temporary address and it is ioctl. IIRC same as what ethtool
> > > > > uses. (ifconfig is already deprecated).
> > > > >
> > > > > > And I guess we should use similar mechanism for permanent
> > > > > > address.
> > > > >
> > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing temporary mac
> > > > > address. But here we do not want to change permanent mac address. We
> > > > > want to tell kernel driver current permanent mac address which is
> > > > > stored
> > > >
> > > > Well... I'd still use similar mechanism :-).
> > >
> > > Thats problematic, because in time when wlan0 interface is registered
> > > into system and visible in ifconfig output it already needs to have
> > > permanent mac address assigned.
> > >
> > > We should assign permanent mac address before wlan0 of wl1251 is
> > > registered into system.
> >
> > You can just add the MAC address to the NVS data, which is also
> > required for the device initialization.
>
> NVS data file has fixed size, there is IIRC no place for it.
>
> But one of my suggestion was to use another request_firmware for MAC
> address. So this is similar to what you are proposing, as NVS data are
> loaded by request_firmware too...
Just append it to NVS data and modify the size check accordingly?
> > I wonder if those information could be put into DT. Iirc some
> > network devices get their MAC address from DT. Maybe we can add
> > all NVS info to DT? How much data is it?
>
> Proprietary, signed and closed bootloader NOLO does not support DT. So
> for booting you need to append DTS file to kernel image.
Yeah, so NOLO without U-Boot will depend on userspace to fixup DT.
> U-Boot is optional and can be used as intermediate bootloader between
> NOLO and kernel. But still it has problems with reading from nand, so
> cannot read NVS data nor MAC address.
It may in the future?
> > Userspace application can add all those information to the DT
> > using a DT overlay. Also the u-boot could parse and add it at
> > some point in the future.
>
> In case when wl1251 is statically linked into kernel image, it is loaded
> and initialized before even userspace applications starts.
>
> So no... adding NVS data or MAC address into DT or DT overlay is not a
> solution.
Actually with data loaded from DT you *can* load data quite early in
the boot process, while your suggestions always require userspace.
So you argument against yourself?
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Pali Rohár @ 2016-11-24 16:49 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Pavel Machek, Michal Kazior, Kalle Valo, Ivaylo Dimitrov,
Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
linux-kernel
In-Reply-To: <20161124160830.kdpbonsz3l26uuo5@earth>
[-- Attachment #1: Type: Text/Plain, Size: 4604 bytes --]
On Thursday 24 November 2016 17:08:30 Sebastian Reichel wrote:
> Hi,
>
> On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote:
> > On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote:
> > > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote:
> > > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> > > > > > > "ifconfig hw ether XX" normally sets the address. I guess
> > > > > > > that's ioctl?
> > > > > >
> > > > > > This sets temporary address and it is ioctl. IIRC same as
> > > > > > what ethtool uses. (ifconfig is already deprecated).
> > > > > >
> > > > > > > And I guess we should use similar mechanism for permanent
> > > > > > > address.
> > > > > >
> > > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing
> > > > > > temporary mac address. But here we do not want to change
> > > > > > permanent mac address. We want to tell kernel driver
> > > > > > current permanent mac address which is stored
> > > > >
> > > > > Well... I'd still use similar mechanism :-).
> > > >
> > > > Thats problematic, because in time when wlan0 interface is
> > > > registered into system and visible in ifconfig output it
> > > > already needs to have permanent mac address assigned.
> > > >
> > > > We should assign permanent mac address before wlan0 of wl1251
> > > > is registered into system.
> > >
> > > You can just add the MAC address to the NVS data, which is also
> > > required for the device initialization.
> >
> > NVS data file has fixed size, there is IIRC no place for it.
> >
> > But one of my suggestion was to use another request_firmware for
> > MAC address. So this is similar to what you are proposing, as NVS
> > data are loaded by request_firmware too...
>
> Just append it to NVS data and modify the size check accordingly?
First that would mean to have request_firmware() function which will not
use direct VFS access, but instead use userspace helper.
NVS data file with some default values (not suitable for usage) is
already present in linux-firmware and available in /lib/firmware/...
Also I'm not sure if such thing is allowed by license:
https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.ti-connectivity
> > > I wonder if those information could be put into DT. Iirc some
> > > network devices get their MAC address from DT. Maybe we can add
> > > all NVS info to DT? How much data is it?
> >
> > Proprietary, signed and closed bootloader NOLO does not support DT.
> > So for booting you need to append DTS file to kernel image.
>
> Yeah, so NOLO without U-Boot will depend on userspace to fixup DT.
>
> > U-Boot is optional and can be used as intermediate bootloader
> > between NOLO and kernel. But still it has problems with reading
> > from nand, so cannot read NVS data nor MAC address.
>
> It may in the future?
I already tried that, but I failed. I was not able to access N900's nand
from u-boot. No idea where was problem...
And if somebody fix onenand in u-boot, then needs to reimplement Nokia's
proprietary parser of that partition where is stored NVS and mac address
&& make this support in upstream u-boot.
So... I doubt it will be in any future.
+ no men power
> > > Userspace application can add all those information to the DT
> > > using a DT overlay. Also the u-boot could parse and add it at
> > > some point in the future.
> >
> > In case when wl1251 is statically linked into kernel image, it is
> > loaded and initialized before even userspace applications starts.
> >
> > So no... adding NVS data or MAC address into DT or DT overlay is
> > not a solution.
>
> Actually with data loaded from DT you *can* load data quite early in
> the boot process, while your suggestions always require userspace.
> So you argument against yourself?
You cannot add DTS in uboot (no support). And if you modify DTS on
running kernel from userspace, then it is too late when wl1251 is
statically linked into kernel image.
wl1251 will not wait until some userspace modify DTS and add data...
But request_firmware() in fallback mode *can* wait for userspace so
wl1251 can postpone its operation until mdev/udev/whatever starts
listening for events and push needed firmware data to kernel.
So no, there is no argument against... request_firmware() in fallback
mode with userspace helper is by design blocking and waiting for
userspace. But waiting for some change in DST in kernel is just
nonsense.
--
Pali Rohár
pali.rohar@gmail.com
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: wl1251 & mac address & calibration data
From: Sebastian Reichel @ 2016-11-24 18:11 UTC (permalink / raw)
To: Pali Rohár
Cc: Pavel Machek, Michal Kazior, Kalle Valo, Ivaylo Dimitrov,
Aaro Koskinen, Tony Lindgren, linux-wireless, Network Development,
linux-kernel
In-Reply-To: <201611241749.33681@pali>
[-- Attachment #1: Type: text/plain, Size: 6231 bytes --]
Hi,
On Thu, Nov 24, 2016 at 05:49:33PM +0100, Pali Rohár wrote:
> On Thursday 24 November 2016 17:08:30 Sebastian Reichel wrote:
> > On Thu, Nov 24, 2016 at 04:20:45PM +0100, Pali Rohár wrote:
> > > On Thursday 24 November 2016 16:13:17 Sebastian Reichel wrote:
> > > > On Thu, Nov 24, 2016 at 09:33:29AM +0100, Pali Rohár wrote:
> > > > > On Thursday 24 November 2016 08:51:04 Pavel Machek wrote:
> > > > > > > > "ifconfig hw ether XX" normally sets the address. I guess
> > > > > > > > that's ioctl?
> > > > > > >
> > > > > > > This sets temporary address and it is ioctl. IIRC same as
> > > > > > > what ethtool uses. (ifconfig is already deprecated).
> > > > > > >
> > > > > > > > And I guess we should use similar mechanism for permanent
> > > > > > > > address.
> > > > > > >
> > > > > > > I'm not sure here... Above ioctl ↑↑↑ is for changing
> > > > > > > temporary mac address. But here we do not want to change
> > > > > > > permanent mac address. We want to tell kernel driver
> > > > > > > current permanent mac address which is stored
> > > > > >
> > > > > > Well... I'd still use similar mechanism :-).
> > > > >
> > > > > Thats problematic, because in time when wlan0 interface is
> > > > > registered into system and visible in ifconfig output it
> > > > > already needs to have permanent mac address assigned.
> > > > >
> > > > > We should assign permanent mac address before wlan0 of wl1251
> > > > > is registered into system.
> > > >
> > > > You can just add the MAC address to the NVS data, which is also
> > > > required for the device initialization.
> > >
> > > NVS data file has fixed size, there is IIRC no place for it.
> > >
> > > But one of my suggestion was to use another request_firmware for
> > > MAC address. So this is similar to what you are proposing, as NVS
> > > data are loaded by request_firmware too...
> >
> > Just append it to NVS data and modify the size check accordingly?
>
> First that would mean to have request_firmware() function which will not
> use direct VFS access, but instead use userspace helper.
Permanent MAC is device specific init data, NVS is device specific
init data => load together.
The userspace helper stuff is only needed to get the data from
the NAND calibration area on the fly.
> NVS data file with some default values (not suitable for usage) is
> already present in linux-firmware and available in /lib/firmware/...
You mentioned NVS data is fixed size, so this should be enough?
switch(loaded_size)
case IMAGE_SIZE + MAC_SIZE:
/* extract mac */
/* fallthrough */
case IMAGE_SIZE:
/* load NVS */
break;
default:
/* fail */
}
> Also I'm not sure if such thing is allowed by license:
> https://git.kernel.org/cgit/linux/kernel/git/firmware/linux-firmware.git/tree/LICENCE.ti-connectivity
concating data is not a modification, otherwise you can't
put the file in a filesystem.
> > > > I wonder if those information could be put into DT. Iirc some
> > > > network devices get their MAC address from DT. Maybe we can add
> > > > all NVS info to DT? How much data is it?
> > >
> > > Proprietary, signed and closed bootloader NOLO does not support DT.
> > > So for booting you need to append DTS file to kernel image.
> >
> > Yeah, so NOLO without U-Boot will depend on userspace to fixup DT.
> >
> > > U-Boot is optional and can be used as intermediate bootloader
> > > between NOLO and kernel. But still it has problems with reading
> > > from nand, so cannot read NVS data nor MAC address.
> >
> > It may in the future?
>
> I already tried that, but I failed. I was not able to access N900's nand
> from u-boot. No idea where was problem...
>
> And if somebody fix onenand in u-boot, then needs to reimplement Nokia's
> proprietary parser of that partition where is stored NVS and mac address
> && make this support in upstream u-boot.
Are we talking about this?
https://github.com/community-ssu/libcal/blob/master/cal.c
That's ~1k loc for read-only. Getting NAND working looks harder than
porting this to me.
> So... I doubt it will be in any future.
>
> + no men power
yeah :(
But with that reasoning you should just extract the NVS data
from NAND and put it in your rootfs.
> > > > Userspace application can add all those information to the DT
> > > > using a DT overlay. Also the u-boot could parse and add it at
> > > > some point in the future.
> > >
> > > In case when wl1251 is statically linked into kernel image, it is
> > > loaded and initialized before even userspace applications starts.
> > >
> > > So no... adding NVS data or MAC address into DT or DT overlay is
> > > not a solution.
> >
> > Actually with data loaded from DT you *can* load data quite early in
> > the boot process, while your suggestions always require userspace.
> > So you argument against yourself?
>
> You cannot add DTS in uboot (no support).
AFAIK that's supported:
http://www.denx.de/wiki/DULG/LinuxFDTBlob
"Note that U-Boot makes some automatic modifications to the blob
before passing it to the kernel - mainly adding and modifying
information that is learnt at run-time."
> And if you modify DTS on running kernel from userspace, then it is
> too late when wl1251 is statically linked into kernel image.
>
> wl1251 will not wait until some userspace modify DTS and add data...
if (nvs info missing)
return -EPROBE_DEFER;
> But request_firmware() in fallback mode *can* wait for userspace so
> wl1251 can postpone its operation until mdev/udev/whatever starts
> listening for events and push needed firmware data to kernel.
As you said one workaround is waiting. That's not a solution, that
only works with request_firmware().
> So no, there is no argument against... request_firmware() in fallback
> mode with userspace helper is by design blocking and waiting for
> userspace. But waiting for some change in DTS in kernel is just
> nonsense.
I would just mark the wlan device with status = "disabled" and
enable it in the overlay together with adding the NVS & MAC info.
-- Sebastian
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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