* Re: [PATCH v3 1/3] Documentation: dt: net: add mt76 wireless device binding
From: Rafał Miłecki @ 2016-12-28 13:28 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: Kalle Valo, Arnd Bergmann, Felix Fietkau,
linux-wireless@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <CAFBinCBzz0Jvk_jcWAJ1jEz17r-NYEE87xLUACTybRSHGE7uGA@mail.gmail.com>
On 28 December 2016 at 11:43, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
> On Wed, Dec 28, 2016 at 11:08 AM, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.c=
om> wrote:
>> On 3 October 2016 at 15:29, Kalle Valo <kvalo@codeaurora.org> wrote:
>>> Arnd Bergmann <arnd@arndb.de> writes:
>>>
>>>> On Friday 30 September 2016, Felix Fietkau wrote:
>>>>> >> >> >> + device_type =3D "pci";
>>>>> >> >> >> + mediatek,mtd-eeprom =3D <&factory 0x8000>;
>>>>> >> >> >> + mediatek,2ghz =3D <0>;
>>>>> >> >
>>>>> >> > It's not clear what the possible values for the 2ghz property ar=
e,
>>>>> >> > can you be more verbose in the description? How is <0> different
>>>>> >> > from no property?
>>>>> >> 0 means disabled, no property means unchanged (compared to EEPROM)=
.
>>>>> >
>>>>> > Maybe have a boolean property instead then to say "mediatek,2ghz-di=
sabled" ?
>>>>> >
>>>>> > If zero is the only possible value, there is no need to put a numbe=
r in there.
>>>>> 1 is also possible, which will force-enable the capability.
>>>>
>>>> Ok, then both those values should be documented in the binding.
>>>
>>> Related to this, Martin sent patches which add generic bindings for
>>> enabling 2 Ghz and 5 Ghz bands.
>>>
>>> [RFC,1/3] Documentation: dt-bindings: add IEEE 802.11 binding documenta=
tion
>>> https://patchwork.kernel.org/patch/9359833/
>>>
>>> [RFC,2/3] of: add IEEE 802.11 device configuration support code
>>> https://patchwork.kernel.org/patch/9359837/
>>
>> I would prefer something more generic. Many tri-band routers split 5
>> GHz band into 2 sets of channels and they have separated radios for
>> them.
>>
>> E.g. Netgear R8000 has phy0 which should be used for higher part of 5
>> GHz band (channels 149+) and phy2 which should be used for lower part
>> of 5 GHz band (channels from 36 to 48 or 64).
>>
>> What do you think about some more flexible properties like:
>> ieee80211-min-center-freq
>> ieee80211-max-center-freq
> what would happen if only one of these properties was given or would
> we forbid that (because the .dts should always describe the hardware,
> and if we describe a lower bound then we should also describe the
> upper bound)?
I didn't think about requiring both properties to be set, but if this
is more correct from DT point of view, we can always require that.
Let's see if we get DT guys opinion.
> the benefits of your solution are:
> - this would allow *enabling* bands as well (my proposal allows this
> as well, but the .dts is indeed a bit hard to read - unlike your
> solution which looks nice to me)
> - we could handle this within generic cfg80211/mac80211 code instead
> of "duplicating" it per driver
i'm happy to hear that :)
> should we describe the center freq in Hz or MHz (cfg80211's
> ieee80211_channel uses the latter)?
Is there any case that may require HZ accuracy? I was thinking about using =
MHz.
^ permalink raw reply
* Re: [PATCH v3 1/3] Documentation: dt: net: add mt76 wireless device binding
From: Rafał Miłecki @ 2016-12-28 13:51 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: Kalle Valo, Arnd Bergmann, Felix Fietkau,
linux-wireless@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <CACna6rzKc8kAnc2_Ca8pXtuu9Rw2mjqfV8VNumoF_E7GdvJx-g@mail.gmail.com>
On 28 December 2016 at 14:28, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wr=
ote:
> On 28 December 2016 at 11:43, Martin Blumenstingl
> <martin.blumenstingl@googlemail.com> wrote:
>> should we describe the center freq in Hz or MHz (cfg80211's
>> ieee80211_channel uses the latter)?
>
> Is there any case that may require HZ accuracy? I was thinking about usin=
g MHz.
Regulatory code uses KHz, so we may better do the same.
--=20
Rafa=C5=82
^ permalink raw reply
* [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2016-12-28 15:59 UTC (permalink / raw)
To: Kalle Valo, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arnd Bergmann, devicetree,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
This new file should be used for properties handled at higher level and
so usable with all drivers.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
.../devicetree/bindings/net/wireless/ieee80211.txt | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
new file mode 100644
index 0000000..c762769
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
@@ -0,0 +1,16 @@
+Common IEEE 802.11 properties
+
+This provides documentation of common properties that are handled by a proper
+net layer and don't require extra driver code.
+
+Optional properties:
+ - ieee80211-min-center-freq : minimal supported frequency in KHz
+ - ieee80211-max-center-freq : maximal supported frequency in KHz
+
+Example:
+
+pcie@0,0 {
+ reg = <0x0000 0 0 0 0>;
+ ieee80211-min-center-freq = <2437000>;
+ ieee80211-max-center-freq = <2457000>;
+};
--
2.10.1
^ permalink raw reply related
* [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties
From: Rafał Miłecki @ 2016-12-28 15:59 UTC (permalink / raw)
To: Kalle Valo, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arnd Bergmann, devicetree,
Rafał Miłecki
In-Reply-To: <20161228155955.25518-1-zajec5@gmail.com>
From: Rafał Miłecki <rafal@milecki.pl>
They allow specifying hardware limitations of supported channels. This
may be useful for specifying single band devices or devices that support
only some part of the whole band.
E.g. some tri-band routers have separated radios for lower and higher
part of 5 GHz band.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
1 file changed, 34 insertions(+)
diff --git a/net/wireless/reg.c b/net/wireless/reg.c
index 5dbac37..35ba5c7 100644
--- a/net/wireless/reg.c
+++ b/net/wireless/reg.c
@@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
}
EXPORT_SYMBOL(reg_initiator_name);
+static bool reg_center_freq_of_valid(struct wiphy *wiphy,
+ struct ieee80211_channel *chan)
+{
+ struct device_node *np = wiphy_dev(wiphy)->of_node;
+ u32 val;
+
+ if (!np)
+ return true;
+
+ if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
+ chan->center_freq < KHZ_TO_MHZ(val))
+ return false;
+
+ if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
+ chan->center_freq > KHZ_TO_MHZ(val))
+ return false;
+
+ return true;
+}
+
static uint32_t reg_rule_to_chan_bw_flags(const struct ieee80211_regdomain *regd,
const struct ieee80211_reg_rule *reg_rule,
const struct ieee80211_channel *chan)
@@ -1209,6 +1229,13 @@ static void handle_channel(struct wiphy *wiphy,
return;
}
+ if (!reg_center_freq_of_valid(wiphy, chan)) {
+ pr_debug("Disabling freq %d MHz as it's out of OF limits\n",
+ chan->center_freq);
+ chan->flags |= IEEE80211_CHAN_DISABLED;
+ return;
+ }
+
regd = reg_get_regdomain(wiphy);
power_rule = ®_rule->power_rule;
@@ -1741,6 +1768,13 @@ static void handle_channel_custom(struct wiphy *wiphy,
return;
}
+ if (!reg_center_freq_of_valid(wiphy, chan)) {
+ pr_debug("Disabling freq %d MHz as it's out of OF limits\n",
+ chan->center_freq);
+ chan->flags |= IEEE80211_CHAN_DISABLED;
+ return;
+ }
+
power_rule = ®_rule->power_rule;
bw_flags = reg_rule_to_chan_bw_flags(regd, reg_rule, chan);
--
2.10.1
^ permalink raw reply related
* BCM43602 -- Bluetooth while WiFi on 2.4GHz networks
From: Greg Oliver @ 2016-12-28 16:59 UTC (permalink / raw)
To: linux-wireless
I have been fighting this laptop (MacBookPro11,5) for a year now with
linux (mainly on Thunderbolt and power issues), but this is one of the
last ones that remains.
The system contains a BCM 43602 (rebranded by Apple of course):
04:00.0 Network controller [0280]: Broadcom Limited BCM43602 802.11ac
Wireless LAN SoC [14e4:43ba] (rev 01)
Subsystem: Apple Inc. Device [106b:0152]
Flags: bus master, fast devsel, latency 0, IRQ 49
Memory at b0800000 (64-bit, non-prefetchable) [size=32K]
Memory at b0400000 (64-bit, non-prefetchable) [size=4M]
Capabilities: [48] Power Management version 3
Capabilities: [58] MSI: Enable+ Count=1/16 Maskable- 64bit+
Capabilities: [68] Vendor Specific Information: Len=44 <?>
Capabilities: [ac] Express Endpoint, MSI 00
Capabilities: [100] Advanced Error Reporting
Capabilities: [13c] Device Serial Number cd-97-9b-ff-ff-0b-a0-99
Capabilities: [150] Power Budgeting <?>
Capabilities: [160] Virtual Channel
Capabilities: [1b0] Latency Tolerance Reporting
Capabilities: [220] #15
Capabilities: [240] L1 PM Substates
Kernel driver in use: brcmfmac
Kernel modules: brcmfmac
Modules options in use:
alternative_fw_path ::
debug :: 0
roamoff :: 1
Linux macbook 4.8.14-301.fc25.x86_64 #1 SMP Sun Dec 25 12:49:05 CST
2016 x86_64 x86_64 x86_64 GNU/Linux
Basically when wifi is on a 2.4G frequency, bluetooth (my keyboard and
mouse) are erratic (all over the place) - mouse stutters and keyboard
loses / repeats keys constantly. In the past I have used atheros and
intel nics with their bt coexistent modes, but I do not see that
option for broadcom modules.
Is there a trick to getting them to play nice? At home, it is not an
issue as I use 5GHz bands, but many times at customers they only have
2.4 on their "vendor supported" APs for me to use, so my mouse is a
no-go (and I truly hate trackpads).
Thanks for any help, and I can provide any info needed. This seems to
be a common issue, but I do not see where anyone has fixed it or
posted to linux-wireless as of yet.
Thanks
-Greg
^ permalink raw reply
* [PATCH 3/3] NFC: pn533: change order operations in dev registation
From: Andrey Rusalin @ 2016-12-28 17:10 UTC (permalink / raw)
To: lauro.venancio, aloisio.almeida, sameo, michael.thalmeier,
linux-wireless, linux-nfc
Cc: Andrey Rusalin
In-Reply-To: <1482945059-12249-1-git-send-email-arusalin@dev.rtsoft.ru>
Sometimes during probing and registration of pn533_i2c
NULL pointer dereference happens.
Reproduced in cycle of inserting and removing pn533_i2c
and pn533 modules.
Backtrace:
[<8004205c>] (__queue_work) from [<80042324>] (queue_work_on+0x50/0x5c)
r10:acdc7c80 r9:8006b330 r8:ac0dfb40 r7:ac50c600 r6:00000004 r5:acbbee40 r4:600f0113
[<800422d4>] (queue_work_on) from [<7f7d5b6c>] (pn533_recv_frame+0x158/0x1fc [pn533])
r7:ffffff87 r6:00000000 r5:acbbee40 r4:acbbee00
[<7f7d5a14>] (pn533_recv_frame [pn533]) from [<7f7df4b8>] (pn533_i2c_irq_thread_fn+0x184/0x)
r6:acb2a000 r5:00000000 r4:acdc7b90
[<7f7df334>] (pn533_i2c_irq_thread_fn [pn533_i2c]) from [<8006b354>] (irq_thread_fn+0x24/0x)
r7:00000000 r6:accde000 r5:ac0dfb40 r4:acdc7c80
...
Seems there is some race condition due registration of
irq handler until all data stuctures that could be needed
are ready. So I re-ordered some ops. After this, problem has gone.
Changes in USB part was not tested, but it should not break
anything.
Signed-off-by: Andrey Rusalin <arusalin@dev.rtsoft.ru>
---
drivers/nfc/pn533/i2c.c | 28 ++++++++++++++++++----------
drivers/nfc/pn533/pn533.c | 42 +++++++++++++++++++++++++-----------------
drivers/nfc/pn533/pn533.h | 1 +
drivers/nfc/pn533/usb.c | 4 ++++
4 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index e2848e4..c1302de 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -206,14 +206,6 @@ static int pn533_i2c_probe(struct i2c_client *client,
phy->i2c_dev = client;
i2c_set_clientdata(client, phy);
- r = request_threaded_irq(client->irq, NULL, pn533_i2c_irq_thread_fn,
- IRQF_TRIGGER_FALLING |
- IRQF_SHARED | IRQF_ONESHOT,
- PN533_I2C_DRIVER_NAME, phy);
-
- if (r < 0)
- nfc_err(&client->dev, "Unable to register IRQ handler\n");
-
priv = pn533_register_device(PN533_DEVICE_PN532,
PN533_NO_TYPE_B_PROTOCOLS,
PN533_PROTO_REQ_ACK_RESP,
@@ -223,16 +215,32 @@ static int pn533_i2c_probe(struct i2c_client *client,
if (IS_ERR(priv)) {
r = PTR_ERR(priv);
- goto err_register;
+ return r;
}
phy->priv = priv;
+ r = request_threaded_irq(client->irq, NULL, pn533_i2c_irq_thread_fn,
+ IRQF_TRIGGER_FALLING |
+ IRQF_SHARED | IRQF_ONESHOT,
+ PN533_I2C_DRIVER_NAME, phy);
+ if (r < 0) {
+ nfc_err(&client->dev, "Unable to register IRQ handler\n");
+ goto irq_rqst_err;
+ }
+
+ r = pn533_finalize_setup(priv);
+ if (r)
+ goto fn_setup_err;
+
return 0;
-err_register:
+fn_setup_err:
free_irq(client->irq, phy);
+irq_rqst_err:
+ pn533_unregister_device(phy->priv);
+
return r;
}
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index c67150f..bbae44b 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -2570,6 +2570,31 @@ static int pn533_setup(struct pn533 *dev)
return 0;
}
+int pn533_finalize_setup(struct pn533 *dev)
+{
+
+ struct pn533_fw_version fw_ver;
+ int rc;
+
+ memset(&fw_ver, 0, sizeof(fw_ver));
+
+ rc = pn533_get_firmware_version(dev, &fw_ver);
+ if (rc) {
+ nfc_err(dev->dev, "Unable to get FW version\n");
+ return rc;
+ }
+
+ nfc_info(dev->dev, "NXP PN5%02X firmware ver %d.%d now attached\n",
+ fw_ver.ic, fw_ver.ver, fw_ver.rev);
+
+ rc = pn533_setup(dev);
+ if (rc)
+ return rc;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pn533_finalize_setup);
+
struct pn533 *pn533_register_device(u32 device_type,
u32 protocols,
enum pn533_protocol_type protocol_type,
@@ -2579,7 +2604,6 @@ struct pn533 *pn533_register_device(u32 device_type,
struct device *dev,
struct device *parent)
{
- struct pn533_fw_version fw_ver;
struct pn533 *priv;
int rc = -ENOMEM;
@@ -2622,15 +2646,6 @@ struct pn533 *pn533_register_device(u32 device_type,
INIT_LIST_HEAD(&priv->cmd_queue);
- memset(&fw_ver, 0, sizeof(fw_ver));
- rc = pn533_get_firmware_version(priv, &fw_ver);
- if (rc < 0)
- goto destroy_wq;
-
- nfc_info(dev, "NXP PN5%02X firmware ver %d.%d now attached\n",
- fw_ver.ic, fw_ver.ver, fw_ver.rev);
-
-
priv->nfc_dev = nfc_allocate_device(&pn533_nfc_ops, protocols,
priv->ops->tx_header_len +
PN533_CMD_DATAEXCH_HEAD_LEN,
@@ -2647,15 +2662,8 @@ struct pn533 *pn533_register_device(u32 device_type,
if (rc)
goto free_nfc_dev;
- rc = pn533_setup(priv);
- if (rc)
- goto unregister_nfc_dev;
-
return priv;
-unregister_nfc_dev:
- nfc_unregister_device(priv->nfc_dev);
-
free_nfc_dev:
nfc_free_device(priv->nfc_dev);
diff --git a/drivers/nfc/pn533/pn533.h b/drivers/nfc/pn533/pn533.h
index e26d634..12ea04e 100644
--- a/drivers/nfc/pn533/pn533.h
+++ b/drivers/nfc/pn533/pn533.h
@@ -231,6 +231,7 @@ struct pn533 *pn533_register_device(u32 device_type,
struct device *dev,
struct device *parent);
+int pn533_finalize_setup(struct pn533 *dev);
void pn533_unregister_device(struct pn533 *priv);
void pn533_recv_frame(struct pn533 *dev, struct sk_buff *skb, int status);
diff --git a/drivers/nfc/pn533/usb.c b/drivers/nfc/pn533/usb.c
index bcf3f54..909806f 100644
--- a/drivers/nfc/pn533/usb.c
+++ b/drivers/nfc/pn533/usb.c
@@ -543,6 +543,10 @@ static int pn533_usb_probe(struct usb_interface *interface,
phy->priv = priv;
+ rc = pn533_finalize_setup(priv);
+ if (rc)
+ goto error;
+
usb_set_intfdata(interface, phy);
return 0;
--
2.7.4
^ permalink raw reply related
* [PATCH 0/3] NFC: pn533: fixes for i2c driver
From: Andrey Rusalin @ 2016-12-28 17:10 UTC (permalink / raw)
To: lauro.venancio, aloisio.almeida, sameo, michael.thalmeier,
linux-wireless, linux-nfc
Cc: Andrey Rusalin
Each of these patches fix some oops that I met during
tests of the driver with itead pn532 nfc module.
First and third patches related to order of initialization driver,
where interrupt handler was registered before work queues
were ready to handle it.
Also iqr was freed already after work queues were deinitialized.
Second patch originally sent by Michael Thalmeier.
I reworked a little bit to make it more readable.
Andrey Rusalin (3):
NFC: pn533: change order of free_irq and dev unregistration
NFC: pn533: improve cmd queue handling
NFC: pn533: change order operations in dev registation
drivers/nfc/pn533/i2c.c | 32 +++++++++++-------
drivers/nfc/pn533/pn533.c | 82 ++++++++++++++++++++++++++++++-----------------
drivers/nfc/pn533/pn533.h | 1 +
drivers/nfc/pn533/usb.c | 4 +++
4 files changed, 77 insertions(+), 42 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/3] NFC: pn533: change order of free_irq and dev unregistration
From: Andrey Rusalin @ 2016-12-28 17:10 UTC (permalink / raw)
To: lauro.venancio, aloisio.almeida, sameo, michael.thalmeier,
linux-wireless, linux-nfc
Cc: Andrey Rusalin
In-Reply-To: <1482945059-12249-1-git-send-email-arusalin@dev.rtsoft.ru>
Change order of free_irq and dev unregistration.
It fixes situation when device already unregistered and
an interrupt happens and nobody can handle it.
Signed-off-by: Andrey Rusalin <arusalin@dev.rtsoft.ru>
---
drivers/nfc/pn533/i2c.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/nfc/pn533/i2c.c b/drivers/nfc/pn533/i2c.c
index 5de5a49..e2848e4 100644
--- a/drivers/nfc/pn533/i2c.c
+++ b/drivers/nfc/pn533/i2c.c
@@ -242,10 +242,10 @@ static int pn533_i2c_remove(struct i2c_client *client)
dev_dbg(&client->dev, "%s\n", __func__);
- pn533_unregister_device(phy->priv);
-
free_irq(client->irq, phy);
+ pn533_unregister_device(phy->priv);
+
return 0;
}
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] NFC: pn533: improve cmd queue handling
From: Andrey Rusalin @ 2016-12-28 17:10 UTC (permalink / raw)
To: lauro.venancio, aloisio.almeida, sameo, michael.thalmeier,
linux-wireless, linux-nfc
Cc: Andrey Rusalin
In-Reply-To: <1482945059-12249-1-git-send-email-arusalin@dev.rtsoft.ru>
Make sure cmd is set before a frame is passed to the transport layer for
sending. In addition pn533_send_async_complete checks if cmd is set before
accessing its members.
Signed-off-by: Michael Thalmeier <michael.thalmeier@hale.at>
Rework a little bit changes in pn532_send_async_complete.
Signed-off-by: Andrey Rusalin <arusalin@dev.rtsoft.ru>
---
drivers/nfc/pn533/pn533.c | 40 +++++++++++++++++++++++++++-------------
1 file changed, 27 insertions(+), 13 deletions(-)
diff --git a/drivers/nfc/pn533/pn533.c b/drivers/nfc/pn533/pn533.c
index 2f817b3..c67150f 100644
--- a/drivers/nfc/pn533/pn533.c
+++ b/drivers/nfc/pn533/pn533.c
@@ -383,14 +383,18 @@ static void pn533_build_cmd_frame(struct pn533 *dev, u8 cmd_code,
static int pn533_send_async_complete(struct pn533 *dev)
{
struct pn533_cmd *cmd = dev->cmd;
- int status = cmd->status;
+ struct sk_buff *resp;
+ int status, rc = 0;
- struct sk_buff *req = cmd->req;
- struct sk_buff *resp = cmd->resp;
+ if (!cmd) {
+ dev_dbg(dev->dev, "%s: cmd not set\n", __func__);
+ goto done;
+ }
- int rc;
+ dev_kfree_skb(cmd->req);
- dev_kfree_skb(req);
+ status = cmd->status;
+ resp = cmd->resp;
if (status < 0) {
rc = cmd->complete_cb(dev, cmd->complete_cb_context,
@@ -399,8 +403,14 @@ static int pn533_send_async_complete(struct pn533 *dev)
goto done;
}
- skb_pull(resp, dev->ops->rx_header_len);
- skb_trim(resp, resp->len - dev->ops->rx_tail_len);
+ /* when no response is set we got interrupted */
+ if (!resp)
+ resp = ERR_PTR(-EINTR);
+
+ if (!IS_ERR(resp)) {
+ skb_pull(resp, dev->ops->rx_header_len);
+ skb_trim(resp, resp->len - dev->ops->rx_tail_len);
+ }
rc = cmd->complete_cb(dev, cmd->complete_cb_context, resp);
@@ -434,12 +444,14 @@ static int __pn533_send_async(struct pn533 *dev, u8 cmd_code,
mutex_lock(&dev->cmd_lock);
if (!dev->cmd_pending) {
+ dev->cmd = cmd;
rc = dev->phy_ops->send_frame(dev, req);
- if (rc)
+ if (rc) {
+ dev->cmd = NULL;
goto error;
+ }
dev->cmd_pending = 1;
- dev->cmd = cmd;
goto unlock;
}
@@ -511,11 +523,12 @@ static int pn533_send_cmd_direct_async(struct pn533 *dev, u8 cmd_code,
pn533_build_cmd_frame(dev, cmd_code, req);
+ dev->cmd = cmd;
rc = dev->phy_ops->send_frame(dev, req);
- if (rc < 0)
+ if (rc < 0) {
+ dev->cmd = NULL;
kfree(cmd);
- else
- dev->cmd = cmd;
+ }
return rc;
}
@@ -550,14 +563,15 @@ static void pn533_wq_cmd(struct work_struct *work)
mutex_unlock(&dev->cmd_lock);
+ dev->cmd = cmd;
rc = dev->phy_ops->send_frame(dev, cmd->req);
if (rc < 0) {
+ dev->cmd = NULL;
dev_kfree_skb(cmd->req);
kfree(cmd);
return;
}
- dev->cmd = cmd;
}
struct pn533_sync_cmd_response {
--
2.7.4
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Arend van Spriel @ 2016-12-28 20:05 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arnd Bergmann, devicetree,
Rafał Miłecki
In-Reply-To: <20161228155955.25518-1-zajec5@gmail.com>
On 28-12-16 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This new file should be used for properties handled at higher level and
> so usable with all drivers.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> new file mode 100644
> index 0000000..c762769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> @@ -0,0 +1,16 @@
> +Common IEEE 802.11 properties
> +
> +This provides documentation of common properties that are handled by a proper
> +net layer and don't require extra driver code.
Please do not make any assumptions on how DT properties are handled nor
by what. Just state that these properties apply to all wireless devices
and are applicable to device specific bindings.
> +Optional properties:
> + - ieee80211-min-center-freq : minimal supported frequency in KHz
> + - ieee80211-max-center-freq : maximal supported frequency in KHz
> +
> +Example:
> +
> +pcie@0,0 {
> + reg = <0x0000 0 0 0 0>;
> + ieee80211-min-center-freq = <2437000>;
> + ieee80211-max-center-freq = <2457000>;
> +};
Is this the proper level to define it. I was expecting a child node of
the pci(e) controller. Maybe I am misreading the example.
Regards,
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2016-12-28 20:32 UTC (permalink / raw)
To: Arend van Spriel
Cc: Kalle Valo, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <f387d897-a1e9-c932-8317-41246be245b0@broadcom.com>
On 28 December 2016 at 21:05, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> This new file should be used for properties handled at higher level and
>> so usable with all drivers.
>>
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 +++++++++=
+++++++
>> 1 file changed, 16 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee8=
0211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.tx=
t b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> new file mode 100644
>> index 0000000..c762769
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> @@ -0,0 +1,16 @@
>> +Common IEEE 802.11 properties
>> +
>> +This provides documentation of common properties that are handled by a =
proper
>> +net layer and don't require extra driver code.
>
> Please do not make any assumptions on how DT properties are handled nor
> by what. Just state that these properties apply to all wireless devices
> and are applicable to device specific bindings.
OK, I'll try to improve this description.
>> +Optional properties:
>> + - ieee80211-min-center-freq : minimal supported frequency in KHz
>> + - ieee80211-max-center-freq : maximal supported frequency in KHz
>> +
>> +Example:
>> +
>> +pcie@0,0 {
>> + reg =3D <0x0000 0 0 0 0>;
>> + ieee80211-min-center-freq =3D <2437000>;
>> + ieee80211-max-center-freq =3D <2457000>;
>> +};
>
> Is this the proper level to define it. I was expecting a child node of
> the pci(e) controller. Maybe I am misreading the example.
This is device node, not a controller node (and yes, it's complete
node). You just need to add such a node inside the controller one.
It doesn't seem to be clearly documented, but you can see it in examples in=
:
Documentation/devicetree/bindings/pci/mvebu-pci.txt
Documentation/devicetree/bindings/pci/nvidia,tegra20-pcie.txt
Documentation/devicetree/bindings/pci/pci-rcar-gen2.txt
The assignment is done in
pci_scan_device -> pci_set_of_node -> of_pci_find_child_device
(so this isn't controller specific thing).
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Martin Blumenstingl @ 2016-12-28 20:39 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Arend van Spriel, Kalle Valo, linux-wireless@vger.kernel.org,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <CACna6rwhC=28fCHDzXok8Ka08g3yhcD2VNgQrfUZUCQRGqksOg@mail.gmail.com>
On Wed, Dec 28, 2016 at 9:32 PM, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com>=
wrote:
> On 28 December 2016 at 21:05, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>
>>> This new file should be used for properties handled at higher level and
>>> so usable with all drivers.
>>>
>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>> ---
>>> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 ++++++++=
++++++++
>>> 1 file changed, 16 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee=
80211.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.t=
xt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>> new file mode 100644
>>> index 0000000..c762769
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>> @@ -0,0 +1,16 @@
>>> +Common IEEE 802.11 properties
>>> +
>>> +This provides documentation of common properties that are handled by a=
proper
>>> +net layer and don't require extra driver code.
>>
>> Please do not make any assumptions on how DT properties are handled nor
>> by what. Just state that these properties apply to all wireless devices
>> and are applicable to device specific bindings.
>
> OK, I'll try to improve this description.
>
>
>>> +Optional properties:
>>> + - ieee80211-min-center-freq : minimal supported frequency in KHz
>>> + - ieee80211-max-center-freq : maximal supported frequency in KHz
>>> +
>>> +Example:
>>> +
>>> +pcie@0,0 {
>>> + reg =3D <0x0000 0 0 0 0>;
>>> + ieee80211-min-center-freq =3D <2437000>;
>>> + ieee80211-max-center-freq =3D <2457000>;
>>> +};
>>
>> Is this the proper level to define it. I was expecting a child node of
>> the pci(e) controller. Maybe I am misreading the example.
>
> This is device node, not a controller node (and yes, it's complete
> node). You just need to add such a node inside the controller one.
you should name the node wifi@0,0 instead. I revised my ath9k OF
binding documentation due to similar concerns (and after seeing the
result I must admit that they were right). you can have a look at the
result here: [0]
apart from that: thanks for the patch, I will try this as soon as possible!
Regards,
Martin
[0] https://github.com/torvalds/linux/blob/master/Documentation/devicetree/=
bindings/net/wireless/qca%2Cath9k.txt
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2016-12-28 20:43 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: Arend van Spriel, Kalle Valo, linux-wireless@vger.kernel.org,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <CAFBinCBNXdM-xVH9SaPZdFr3X0=k+py9aZ6Qj4ng=v1L-EvS7A@mail.gmail.com>
On 28 December 2016 at 21:39, Martin Blumenstingl
<martin.blumenstingl@googlemail.com> wrote:
> On Wed, Dec 28, 2016 at 9:32 PM, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.co=
m> wrote:
>> On 28 December 2016 at 21:05, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>
>>>> This new file should be used for properties handled at higher level an=
d
>>>> so usable with all drivers.
>>>>
>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>> ---
>>>> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 +++++++=
+++++++++
>>>> 1 file changed, 16 insertions(+)
>>>> create mode 100644 Documentation/devicetree/bindings/net/wireless/iee=
e80211.txt
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.=
txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>>> new file mode 100644
>>>> index 0000000..c762769
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>>>> @@ -0,0 +1,16 @@
>>>> +Common IEEE 802.11 properties
>>>> +
>>>> +This provides documentation of common properties that are handled by =
a proper
>>>> +net layer and don't require extra driver code.
>>>
>>> Please do not make any assumptions on how DT properties are handled nor
>>> by what. Just state that these properties apply to all wireless devices
>>> and are applicable to device specific bindings.
>>
>> OK, I'll try to improve this description.
>>
>>
>>>> +Optional properties:
>>>> + - ieee80211-min-center-freq : minimal supported frequency in KHz
>>>> + - ieee80211-max-center-freq : maximal supported frequency in KHz
>>>> +
>>>> +Example:
>>>> +
>>>> +pcie@0,0 {
>>>> + reg =3D <0x0000 0 0 0 0>;
>>>> + ieee80211-min-center-freq =3D <2437000>;
>>>> + ieee80211-max-center-freq =3D <2457000>;
>>>> +};
>>>
>>> Is this the proper level to define it. I was expecting a child node of
>>> the pci(e) controller. Maybe I am misreading the example.
>>
>> This is device node, not a controller node (and yes, it's complete
>> node). You just need to add such a node inside the controller one.
> you should name the node wifi@0,0 instead. I revised my ath9k OF
> binding documentation due to similar concerns (and after seeing the
> result I must admit that they were right). you can have a look at the
> result here: [0]
Thanks for your comment, I'm far from considering myself DT expert, so
I often need help with such things, I'll change this in V2.
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties
From: Arend van Spriel @ 2016-12-28 21:07 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo, linux-wireless
Cc: Martin Blumenstingl, Felix Fietkau, Arnd Bergmann, devicetree,
Rafał Miłecki
In-Reply-To: <20161228155955.25518-2-zajec5@gmail.com>
On 28-12-16 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> They allow specifying hardware limitations of supported channels. This
> may be useful for specifying single band devices or devices that support
> only some part of the whole band.
> E.g. some tri-band routers have separated radios for lower and higher
> part of 5 GHz band.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
> 1 file changed, 34 insertions(+)
>
> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
> index 5dbac37..35ba5c7 100644
> --- a/net/wireless/reg.c
> +++ b/net/wireless/reg.c
> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
> }
> EXPORT_SYMBOL(reg_initiator_name);
>
> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
> + struct ieee80211_channel *chan)
> +{
> + struct device_node *np = wiphy_dev(wiphy)->of_node;
> + u32 val;
> +
> + if (!np)
> + return true;
> +
> + if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
> + chan->center_freq < KHZ_TO_MHZ(val))
> + return false;
> +
> + if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
> + chan->center_freq > KHZ_TO_MHZ(val))
> + return false;
I suspect these functions rely on CONFIG_OF. So might not build for
!CONFIG_OF.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties
From: Rafał Miłecki @ 2016-12-28 21:30 UTC (permalink / raw)
To: Arend van Spriel
Cc: Kalle Valo, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <CACna6rwKLr-makRauYQf51330p96QrSNEhtNqu927yHT_Xm7Wg@mail.gmail.com>
On 28 December 2016 at 22:28, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com> wr=
ote:
> On 28 December 2016 at 22:07, Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
>> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>
>>> They allow specifying hardware limitations of supported channels. This
>>> may be useful for specifying single band devices or devices that suppor=
t
>>> only some part of the whole band.
>>> E.g. some tri-band routers have separated radios for lower and higher
>>> part of 5 GHz band.
>>>
>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>> ---
>>> net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>> 1 file changed, 34 insertions(+)
>>>
>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>> index 5dbac37..35ba5c7 100644
>>> --- a/net/wireless/reg.c
>>> +++ b/net/wireless/reg.c
>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_=
initiator initiator)
>>> }
>>> EXPORT_SYMBOL(reg_initiator_name);
>>>
>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>> + struct ieee80211_channel *chan)
>>> +{
>>> + struct device_node *np =3D wiphy_dev(wiphy)->of_node;
>>> + u32 val;
>>> +
>>> + if (!np)
>>> + return true;
>>> +
>>> + if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) =
&&
>>> + chan->center_freq < KHZ_TO_MHZ(val))
>>> + return false;
>>> +
>>> + if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) =
&&
>>> + chan->center_freq > KHZ_TO_MHZ(val))
>>> + return false;
>>
>> I suspect these functions rely on CONFIG_OF. So might not build for
>> !CONFIG_OF.
>
> I compiled it with
> # CONFIG_OF is not set
>
> Can you test it and provide a non-working config if you see a
> compilation error, please?
include/linux/of.h provides a lot of dummy static inline functions if
CONFIG_OF is not used (they also allow compiler to drop most of the
code).
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Felix Fietkau @ 2016-12-28 21:35 UTC (permalink / raw)
To: Rafał Miłecki, Kalle Valo, linux-wireless
Cc: Martin Blumenstingl, Arnd Bergmann, devicetree,
Rafał Miłecki
In-Reply-To: <20161228155955.25518-1-zajec5@gmail.com>
On 2016-12-28 16:59, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> This new file should be used for properties handled at higher level and
> so usable with all drivers.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.txt b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> new file mode 100644
> index 0000000..c762769
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
> @@ -0,0 +1,16 @@
> +Common IEEE 802.11 properties
> +
> +This provides documentation of common properties that are handled by a proper
> +net layer and don't require extra driver code.
> +
> +Optional properties:
> + - ieee80211-min-center-freq : minimal supported frequency in KHz
> + - ieee80211-max-center-freq : maximal supported frequency in KHz
> +
> +Example:
> +
> +pcie@0,0 {
> + reg = <0x0000 0 0 0 0>;
> + ieee80211-min-center-freq = <2437000>;
> + ieee80211-max-center-freq = <2457000>;
I'm not sure that's the best way to deal with enabling/disabling bands.
If we get more bands in the future, there might be unsupported ones in
the middle, which min/max won't cover.
- Felix
^ permalink raw reply
* Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties
From: Rafał Miłecki @ 2016-12-28 21:28 UTC (permalink / raw)
To: Arend van Spriel
Cc: Kalle Valo, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <491a5af2-449d-4b2a-c4ed-af0e89b2ca78@broadcom.com>
On 28 December 2016 at 22:07, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> They allow specifying hardware limitations of supported channels. This
>> may be useful for specifying single band devices or devices that support
>> only some part of the whole band.
>> E.g. some tri-band routers have separated radios for lower and higher
>> part of 5 GHz band.
>>
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>> 1 file changed, 34 insertions(+)
>>
>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>> index 5dbac37..35ba5c7 100644
>> --- a/net/wireless/reg.c
>> +++ b/net/wireless/reg.c
>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_i=
nitiator initiator)
>> }
>> EXPORT_SYMBOL(reg_initiator_name);
>>
>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>> + struct ieee80211_channel *chan)
>> +{
>> + struct device_node *np =3D wiphy_dev(wiphy)->of_node;
>> + u32 val;
>> +
>> + if (!np)
>> + return true;
>> +
>> + if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &=
&
>> + chan->center_freq < KHZ_TO_MHZ(val))
>> + return false;
>> +
>> + if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &=
&
>> + chan->center_freq > KHZ_TO_MHZ(val))
>> + return false;
>
> I suspect these functions rely on CONFIG_OF. So might not build for
> !CONFIG_OF.
I compiled it with
# CONFIG_OF is not set
Can you test it and provide a non-working config if you see a
compilation error, please?
--=20
Rafa=C5=82
^ permalink raw reply
* [PATCH] rtlwifi: Fix alignment issues
From: Larry Finger @ 2016-12-28 21:40 UTC (permalink / raw)
To: kvalo; +Cc: linux-wireless, Ping-Ke Shih, Larry Finger, Stable
From: Ping-Ke Shih <pkshih@realtek.com>
The addresses of Wlan NIC registers are natural alignment, but some
drivers have bugs. These are evident on platforms that need natural
alignment to access registers. This change contains the following:
1. Function _rtl8821ae_dbi_read() is used to read one byte from DBI,
thus it should use rtl_read_byte().
2. Register 0x4C7 of 8192ee is single byte.
Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
Signed-off-by: Larry Finger <Larry.Finger@lwfinger.net>
Cc: Stable <stable@vger.kernel.org>
---
Kalle,
Please send this upstream when convenient. These fixes will improve operations
on architectures that are fussy about alignment.
Larry
---
drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c
index 99827d2..54ea173 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/hw.c
@@ -1006,7 +1006,7 @@ static void _rtl92ee_hw_configure(struct ieee80211_hw *hw)
rtl_write_word(rtlpriv, REG_SIFS_TRX, 0x100a);
/* Note Data sheet don't define */
- rtl_write_word(rtlpriv, 0x4C7, 0x80);
+ rtl_write_byte(rtlpriv, 0x4C7, 0x80);
rtl_write_byte(rtlpriv, REG_RX_PKT_LIMIT, 0x20);
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
index 0b26bd0..e374320 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/hw.c
@@ -1124,7 +1124,7 @@ static u8 _rtl8821ae_dbi_read(struct rtl_priv *rtlpriv, u16 addr)
}
if (0 == tmp) {
read_addr = REG_DBI_RDATA + addr % 4;
- ret = rtl_read_word(rtlpriv, read_addr);
+ ret = rtl_read_byte(rtlpriv, read_addr);
}
return ret;
}
--
1.7.9.5
^ permalink raw reply related
* Re: [PATCH 1/2] dt-bindings: document common IEEE 802.11 frequency properties
From: Rafał Miłecki @ 2016-12-28 22:22 UTC (permalink / raw)
To: Felix Fietkau
Cc: Kalle Valo, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <ddd1bb11-eb4d-7a53-c3ea-5ff5e59ae100@nbd.name>
On 28 December 2016 at 22:35, Felix Fietkau <nbd@nbd.name> wrote:
> On 2016-12-28 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>
>> This new file should be used for properties handled at higher level and
>> so usable with all drivers.
>>
>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>> ---
>> .../devicetree/bindings/net/wireless/ieee80211.txt | 16 +++++++++=
+++++++
>> 1 file changed, 16 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/wireless/ieee8=
0211.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/ieee80211.tx=
t b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> new file mode 100644
>> index 0000000..c762769
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/ieee80211.txt
>> @@ -0,0 +1,16 @@
>> +Common IEEE 802.11 properties
>> +
>> +This provides documentation of common properties that are handled by a =
proper
>> +net layer and don't require extra driver code.
>> +
>> +Optional properties:
>> + - ieee80211-min-center-freq : minimal supported frequency in KHz
>> + - ieee80211-max-center-freq : maximal supported frequency in KHz
>> +
>> +Example:
>> +
>> +pcie@0,0 {
>> + reg =3D <0x0000 0 0 0 0>;
>> + ieee80211-min-center-freq =3D <2437000>;
>> + ieee80211-max-center-freq =3D <2457000>;
> I'm not sure that's the best way to deal with enabling/disabling bands.
> If we get more bands in the future, there might be unsupported ones in
> the middle, which min/max won't cover.
Maybe we could try specifying list of ranges? E.g.
ieee80211-center-frequencies =3D <2412000 2422000
2442000 2452>;
or
ieee80211-center-frequencies =3D <2412000 2422000>,
<2442000 2452>;
for device supporting channels 1, 2, 3, 7, 8, 9?
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties
From: Arend van Spriel @ 2016-12-29 8:57 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Kalle Valo, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <CACna6rzJV-UdydKrXTquEzrkCfNXXKsHBrsZGjTJ8F=BSRyUjA@mail.gmail.com>
On 28-12-16 22:30, Rafał Miłecki wrote:
> On 28 December 2016 at 22:28, Rafał Miłecki <zajec5@gmail.com> wrote:
>> On 28 December 2016 at 22:07, Arend van Spriel
>> <arend.vanspriel@broadcom.com> wrote:
>>> On 28-12-16 16:59, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> They allow specifying hardware limitations of supported channels. This
>>>> may be useful for specifying single band devices or devices that support
>>>> only some part of the whole band.
>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>> part of 5 GHz band.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>> net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>> 1 file changed, 34 insertions(+)
>>>>
>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>> index 5dbac37..35ba5c7 100644
>>>> --- a/net/wireless/reg.c
>>>> +++ b/net/wireless/reg.c
>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_reg_initiator initiator)
>>>> }
>>>> EXPORT_SYMBOL(reg_initiator_name);
>>>>
>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>> + struct ieee80211_channel *chan)
>>>> +{
>>>> + struct device_node *np = wiphy_dev(wiphy)->of_node;
>>>> + u32 val;
>>>> +
>>>> + if (!np)
>>>> + return true;
>>>> +
>>>> + if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val) &&
>>>> + chan->center_freq < KHZ_TO_MHZ(val))
>>>> + return false;
>>>> +
>>>> + if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val) &&
>>>> + chan->center_freq > KHZ_TO_MHZ(val))
>>>> + return false;
>>>
>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>> !CONFIG_OF.
>>
>> I compiled it with
>> # CONFIG_OF is not set
>>
>> Can you test it and provide a non-working config if you see a
>> compilation error, please?
>
> include/linux/of.h provides a lot of dummy static inline functions if
> CONFIG_OF is not used (they also allow compiler to drop most of the
> code).
of_propeirty_read_u32 is static inline in of.h calling
of_property_read_u32_array, which has a dummy variant in of.h returning
-ENOSYS so -38. Pretty sure that is not what you want. At least it does
not allow the compiler to drop any code so probably better to do:
if (!IS_ENABLED(CONFIG_OF) || !np)
return true;
So with this patch you change the channel to DISABLED. I am not very
familiar with reg.c so do you know if this is done before or after
calling regulatory notifier in the driver. brcmfmac will enable channels
querying the device upon regulatory notifier call, which may undo the
behavior introduced by your patch.
Regards,
Arend
^ permalink raw reply
* Re: [PATCH 2/2] cfg80211: reg: support ieee80211-(min|max)-center-freq DT properties
From: Rafał Miłecki @ 2016-12-29 9:43 UTC (permalink / raw)
To: Arend van Spriel
Cc: Kalle Valo, linux-wireless@vger.kernel.org, Martin Blumenstingl,
Felix Fietkau, Arnd Bergmann, devicetree@vger.kernel.org,
Rafał Miłecki
In-Reply-To: <46007537-835c-90db-a44f-c45c8e2ecaed@broadcom.com>
On 29 December 2016 at 09:57, Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
> On 28-12-16 22:30, Rafa=C5=82 Mi=C5=82ecki wrote:
>> On 28 December 2016 at 22:28, Rafa=C5=82 Mi=C5=82ecki <zajec5@gmail.com>=
wrote:
>>> On 28 December 2016 at 22:07, Arend van Spriel
>>> <arend.vanspriel@broadcom.com> wrote:
>>>> On 28-12-16 16:59, Rafa=C5=82 Mi=C5=82ecki wrote:
>>>>> From: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>>
>>>>> They allow specifying hardware limitations of supported channels. Thi=
s
>>>>> may be useful for specifying single band devices or devices that supp=
ort
>>>>> only some part of the whole band.
>>>>> E.g. some tri-band routers have separated radios for lower and higher
>>>>> part of 5 GHz band.
>>>>>
>>>>> Signed-off-by: Rafa=C5=82 Mi=C5=82ecki <rafal@milecki.pl>
>>>>> ---
>>>>> net/wireless/reg.c | 34 ++++++++++++++++++++++++++++++++++
>>>>> 1 file changed, 34 insertions(+)
>>>>>
>>>>> diff --git a/net/wireless/reg.c b/net/wireless/reg.c
>>>>> index 5dbac37..35ba5c7 100644
>>>>> --- a/net/wireless/reg.c
>>>>> +++ b/net/wireless/reg.c
>>>>> @@ -1123,6 +1123,26 @@ const char *reg_initiator_name(enum nl80211_re=
g_initiator initiator)
>>>>> }
>>>>> EXPORT_SYMBOL(reg_initiator_name);
>>>>>
>>>>> +static bool reg_center_freq_of_valid(struct wiphy *wiphy,
>>>>> + struct ieee80211_channel *chan)
>>>>> +{
>>>>> + struct device_node *np =3D wiphy_dev(wiphy)->of_node;
>>>>> + u32 val;
>>>>> +
>>>>> + if (!np)
>>>>> + return true;
>>>>> +
>>>>> + if (!of_property_read_u32(np, "ieee80211-min-center-freq", &val=
) &&
>>>>> + chan->center_freq < KHZ_TO_MHZ(val))
>>>>> + return false;
>>>>> +
>>>>> + if (!of_property_read_u32(np, "ieee80211-max-center-freq", &val=
) &&
>>>>> + chan->center_freq > KHZ_TO_MHZ(val))
>>>>> + return false;
>>>>
>>>> I suspect these functions rely on CONFIG_OF. So might not build for
>>>> !CONFIG_OF.
>>>
>>> I compiled it with
>>> # CONFIG_OF is not set
>>>
>>> Can you test it and provide a non-working config if you see a
>>> compilation error, please?
>>
>> include/linux/of.h provides a lot of dummy static inline functions if
>> CONFIG_OF is not used (they also allow compiler to drop most of the
>> code).
>
> of_propeirty_read_u32 is static inline in of.h calling
> of_property_read_u32_array, which has a dummy variant in of.h returning
> -ENOSYS so -38. Pretty sure that is not what you want. At least it does
> not allow the compiler to drop any code so probably better to do:
>
> if (!IS_ENABLED(CONFIG_OF) || !np)
> return true;
Please verify that using a compiler. If there is a problem I'll be
happy to work on it, but I need a proof it exists.
If compilers sees a:
if (!-ENOSYS && chan->center_freq < KHZ_TO_MHZ(val))
condition, it's pretty clear it can be dropped. With both conditional
blocks dropped function always returns "true" and... can be dropped.
Let me see if I can convince you with the following test:
$ grep -m 1 CONFIG_OF .config
# CONFIG_OF is not set
$ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
0
$ grep -m 1 CONFIG_OF .config
CONFIG_OF=3Dy
$ objdump --syms net/wireless/reg.o | grep -c reg_center_freq_of_valid
1
> So with this patch you change the channel to DISABLED. I am not very
> familiar with reg.c so do you know if this is done before or after
> calling regulatory notifier in the driver. brcmfmac will enable channels
> querying the device upon regulatory notifier call, which may undo the
> behavior introduced by your patch.
I'm not regulatory export, so I hope someone will review this patch.
So far I can say it works for me after trying it on SR400ac with
BCM43602.
ieee80211-min-center-freq =3D <2437000>;
[ 11.986941] cfg80211: Disabling freq 2412 MHz as it's out of OF limits
[ 12.000466] cfg80211: Disabling freq 2417 MHz as it's out of OF limits
[ 12.013984] cfg80211: Disabling freq 2422 MHz as it's out of OF limits
[ 12.027497] cfg80211: Disabling freq 2427 MHz as it's out of OF limits
[ 12.041012] cfg80211: Disabling freq 2432 MHz as it's out of OF limits
root@lede:/# iw phy phy0 channels
Band 1:
* 2412 MHz [1] (disabled)
* 2417 MHz [2] (disabled)
* 2422 MHz [3] (disabled)
* 2427 MHz [4] (disabled)
* 2432 MHz [5] (disabled)
* 2437 MHz [6]
Maximum TX power: 20.0 dBm
Channel widths: 20MHz HT40- HT40+
* 2442 MHz [7]
Maximum TX power: 20.0 dBm
Channel widths: 20MHz HT40- HT40+
* 2447 MHz [8]
Maximum TX power: 20.0 dBm
Channel widths: 20MHz HT40-
* 2452 MHz [9]
Maximum TX power: 20.0 dBm
Channel widths: 20MHz HT40-
* 2457 MHz [10]
Maximum TX power: 20.0 dBm
Channel widths: 20MHz HT40-
* 2462 MHz [11]
Maximum TX power: 20.0 dBm
Channel widths: 20MHz HT40-
* 2467 MHz [12] (disabled)
* 2472 MHz [13] (disabled)
* 2484 MHz [14] (disabled)
--=20
Rafa=C5=82
^ permalink raw reply
* Re: [1/5] ath10k: fix IRAM banks number for QCA9377
From: Kalle Valo @ 2016-12-29 13:18 UTC (permalink / raw)
To: Bartosz Markowski; +Cc: ath10k, linux-wireless, Bartosz Markowski
In-Reply-To: <1481130454-27244-1-git-send-email-bartosz.markowski@tieto.com>
Bartosz Markowski <bartosz.markowski@tieto.com> wrote:
> QCA9377 firmware shall alloc 4 IRAM banks
>
> Signed-off-by: Bartosz Markowski <bartosz.markowski@tieto.com>
5 patches applied to ath-next branch of ath.git, thanks.
77cf13ad5731 ath10k: fix IRAM banks number for QCA9377
b08b5b53a1ed ath10k: override CE5 config for QCA9377
35bac90abf5e ath10k: decrease num of peers support
7cfe0455ee12 ath10k: set CTS protection VDEV param only if VDEV is up
861769017404 ath10k: add debug trace to rts/cts set function
--
https://patchwork.kernel.org/patch/9464915/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [1/2] ath10k: add accounting for the extended peer statistics
From: Kalle Valo @ 2016-12-29 14:11 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, ath10k
In-Reply-To: <992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunkeey@googlemail.com>
Christian Lamparter <chunkeey@googlemail.com> wrote:
> The 10.4 firmware adds extended peer information to the
> firmware's statistics payload. This additional info is
> stored as a separate data field and the elements are
> stored in their own "peers_extd" list.
>
> These elements can pile up in the same way as the peer
> information elements. This is because the
> ath10k_wmi_10_4_op_pull_fw_stats() function tries to
> pull the same amount (num_peer_stats) for every statistic
> data unit.
>
> Fixes: 4a49ae94a448faa ("ath10k: fix 10.4 extended peer stats update")
> Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
My understanding is that I should skip this patch 1. Please let me know if
I misunderstood. But I'm still plannning to apply patch 2.
Patch set to Changes Requested.
--
https://patchwork.kernel.org/patch/9461631/
Documentation about submitting wireless patches and checking status
from patchwork:
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply
* Re: [PATCH] ath10k: merge extended peer info data with existing peers info
From: Mohammed Shafi Shajakhan @ 2016-12-29 14:35 UTC (permalink / raw)
To: Christian Lamparter; +Cc: linux-wireless, ath10k, Kalle Valo, michal.kazior
In-Reply-To: <1563376.k5ulprKP3Z@debian64>
Hi Christian,
On Thu, Dec 22, 2016 at 06:58:41PM +0100, Christian Lamparter wrote:
> Hello Shafi,
>
> On Thursday, December 22, 2016 9:18:01 PM CET Mohammed Shafi Shajakhan wrote:
> > > On Monday, December 19, 2016 10:19:57 PM CET Mohammed Shafi Shajakhan wrote:
> > > > On Sat, Dec 17, 2016 at 06:46:34PM +0100, Christian Lamparter wrote:
> > > > > The 10.4 firmware adds extended peer information to the
> > > > > firmware's statistics payload. This additional info is
> > > > > stored as a separate data field. During review of
> > > > > "ath10k: add accounting for the extended peer statistics" [0]
> > > > >
> > > > > Mohammed Shafi Shajakhan commented that the extended peer statistics
> > > > > lists are of little use:"... there is not much use in appending
> > > > > the extended peer stats (which gets periodically updated) to the
> > > > > linked list '&ar->debug.fw_stats.peers_extd)' and should we get
> > > > > rid of the below (and the required cleanup as well)
> > > > >
> > > > > list_splice_tail_init(&stats.peers_extd,
> > > > > &ar->debug.fw_stats.peers_extd);
> > > > >
> > > > > since rx_duration is getting updated periodically to the per sta
> > > > > information."
> > > > >
> > > > > This patch replaces the extended peers list with a lookup and
> > > > > puts the retrieved data (rx_duration) into the existing
> > > > > ath10k_fw_stats_peer entry that was created earlier.
> > > >
> > > > [shafi] Its good to maintain the extended stats variable
> > > > and retain the two different functions to update rx_duration.
> > > >
> > > > a) extended peer stats supported - mainly for 10.4
> > > > b) extender peer stats not supported - for 10.2
> > > Well, I have to ask why you want to retain the two different
> > > functions to update the same arsta->rx_duration? I think a
> > > little bit of code that helps to explain what's on your mind
> > > (or how you would do it) would help immensely in this case.
> > > Since I have the feeling that this is the problem here.
> > > So please explain it in C(lang).
> >
> > [shafi] I see you prefer to stuff the 'rx_duration' from
> > the extended stats to the existing global 'ath10k_peer_stats'
> > The idea of extended stats is, ideally its not going to stop
> > for 'rx_duration' . Extended stats is maintained as a seperate
> > list / entry for addition of new fields as well
> I'm guessing you are trying to say here:
>
> replace:
>
> dst->rx_duration = __le32_to_cpu(src->rx_duration);
>
> with
>
> dst->rx_duration += __le32_to_cpu(src->rx_duration);
>
> in ath10k_wmi_10_4_op_pull_fw_stats()?
[shafi] oh no sorry, I am trying to say more members related
to stats shall be added in extended stats structure . The extended
stats structure is specifically introduced for adding more stats related
variables.
>
> Is this correct? If so then can you tell me why ath10k_wmi_10_4_op_pull_fw_stats()
> is using for (i = 0; i < num_peer_stats; i++) to iterate over the extended peer
> stats instead of looking up the number of extended peer items. Because this does
> imply that there are the "same" (and every peer stat has a matching extended
> peer stat)... (This will be important for the comment below - so ***).
> Of course, if this isn't true then this is a bug and should be fixed because
> otherwise the ath10k_wmi_10_4_op_pull_fw_stats functions might return -EPROTO
> at some point which causes a "failed to pull fw stats: -71" and unprocessed/lost
> stats.
[shafi] sorry i am not sure I got you, have you come across this error, please
let me know ?
> > >
> > > > > [0] <https://lkml.kernel.org/r/992a4e2676037a06f482cdbe2d3d39e287530be5.1480974623.git.chunkeey@googlemail.com>
> > > > > Cc: Mohammed Shafi Shajakhan <mohammed@codeaurora.org>
> > > > > Signed-off-by: Christian Lamparter <chunkeey@googlemail.com>
> > > > > ---
> > > > > drivers/net/wireless/ath/ath10k/core.h | 2 --
> > > > > drivers/net/wireless/ath/ath10k/debug.c | 17 --------------
> > > > > drivers/net/wireless/ath/ath10k/debugfs_sta.c | 32 ++-----------------------
> > > > > drivers/net/wireless/ath/ath10k/wmi.c | 34 ++++++++++++++++++++-------
> > > > > 4 files changed, 28 insertions(+), 57 deletions(-)
> > > > >
> > > > > diff --git a/drivers/net/wireless/ath/ath10k/core.h b/drivers/net/wireless/ath/ath10k/core.h
> > > > > index 09ff8b8a6441..3fffbbb18c25 100644
> > > > > --- a/drivers/net/wireless/ath/ath10k/core.h
> > > > > +++ b/drivers/net/wireless/ath/ath10k/core.h
> > > > > @@ -268,11 +268,9 @@ struct ath10k_fw_stats_pdev {
> > > > > };
> > > > >
> > > > > struct ath10k_fw_stats {
> > > > > - bool extended;
> > > > > struct list_head pdevs;
> > > > > struct list_head vdevs;
> > > > > struct list_head peers;
> > > > > - struct list_head peers_extd;
> > > > > };
> > > > >
> > > > > #define ATH10K_TPC_TABLE_TYPE_FLAG 1
> > > > > diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
> > > > > index 82a4c67f3672..89f7fde77cdf 100644
> > > > > --- a/drivers/net/wireless/ath/ath10k/debug.c
> > > > > +++ b/drivers/net/wireless/ath/ath10k/debug.c
> > > > > @@ -315,25 +315,13 @@ static void ath10k_fw_stats_peers_free(struct list_head *head)
> > > > > }
> > > > > }
> > > > >
> > > > > -static void ath10k_fw_extd_stats_peers_free(struct list_head *head)
> > > > > -{
> > > > > - struct ath10k_fw_extd_stats_peer *i, *tmp;
> > > > > -
> > > > > - list_for_each_entry_safe(i, tmp, head, list) {
> > > > > - list_del(&i->list);
> > > > > - kfree(i);
> > > > > - }
> > > > > -}
> > > > > -
> > > > > static void ath10k_debug_fw_stats_reset(struct ath10k *ar)
> > > > > {
> > > > > spin_lock_bh(&ar->data_lock);
> > > > > ar->debug.fw_stats_done = false;
> > > > > - ar->debug.fw_stats.extended = false;
> > > >
> > > > [shafi] this looks fine, but not removing the 'extended' variable
> > > > from ath10k_fw_stats structure, I see the design for 'rx_duration'
> > > > arguably some what convoluted !
> > > I removed extended because it is now a write-only variable.
> > > So I figured, there's no point in keeping it around? I don't have
> > > access to the firmware interface documentation, so I don't know
> > > if there's a reason why it would be good to have it later.
> > > So please tell me, what information do we gain from it?
[shafi] sorry this is purely ath10k specific nothing to map with the firmware
> >
> > [shafi] while parsing the stats from 'wmi' we clearly mark there whether
> > the extended stats is available (or) not and if its there parse it and
> > deal with it seperately
> No, there's no difference between stats or extended stats (10.2.4 vs 10.4):
> both ath10k_sta_update_extd_stats_rx_duration() [0]
> and ath10k_sta_update_stats_rx_duration() [1] are adding the
> ath10k_fw_stats_peer(_extd)->rx_duration to ath10k_sta->rx_duration.
>
> With the merge of the peer stats and extended peer stats, this also
> removed the only usage of stats->extended. And hence it becomes a
> write-only variable. So there's no point in keeping it around ATM (as
> all other extended peer stats entries aren't used).
[shafi] ok, I see the extended stats structure is introduced for 10.4 and i was
thinking its good to have two seperate API's for updating the rx_duration ..
>
> > > > *We get periodic events from firmware updating 'ath10k_debug_fw_stats_process'
> > > > *Fetch rx_duration from 'ath10k_wmi_pull_fw_stats(ar, skb, &stats)'
> > > > {certainly 'stats' object is for this particular update only, and freed
> > > > up later)
> > > > *Update immediately using 'ath10k_sta_update_rx_duration'
> > > >
> > > > 'ath10k_wmi_pull_fw_stats' has a slightly different implementation
> > > > for 10.2 and 10.4 (the later supporting extended peer stats)
> > >
> > > I see that 10.2.4's ath10k_wmi_10_2_4_op_pull_fw_stats()
> > > passes the rx_duration as part of the wmi_10_2_4_ext_peer_stats
> > > element which is basically a carbon-copy of wmi_10_2_4_peer_stats
> > > (but with one extra __le32 for the rx_duration at the end.)
> > > This rx_duration is then used to set the rx_duration field in the
> > > generated ath10k_fw_stats_peer struct.
[shafi] ok
> > >
> > > 10.4's ath10k_wmi_10_4_op_pull_fw_stats() has a "fixed" wmi_10_4_peer_stats
> > > element and uses an separate "fixed" wmi_10_4_peer_extd_stats element for
> > > the communicating the rx_duration to the driver.
[shafi] ok
> > >
> > > Thing is, both function have the same signature. They produce the same
> > > struct ath10k_fw_stats_peer for the given data in the sk_buff input. So
> > > why does 10.4 need to have it's peer_extd infrastructure, when it can use
> > > the existing rx_duration field in the *universal* ath10k_fw_stats_peer?
> >
> > [shafi] agreed we need to fix that, it would have been easier if 10.2.4
> > and 10.4 had the same definitions.
> Ok, I don't know the internals of the firmware to know what's the difference
> between 10.2.4 and 10.4's rx_duration (how both firmwares define them
> differently in this context) ? From what I can tell, it's just that
> the entry has moved from the peer stats to extended peer stats.
> Of course, this is based on the logic that both 10.2.4 and 10.4 rx_durations
> end up being added to arsta->rx_duration in the same way. There's no scaling
> or a comment that would indicate a difference.
[shafi] ok
>
> > >
> > > What's with the extra layers / HAL here? Because it looks like it's merged
> > > back together in the same manner into the same arsta->rx_duration?
> > > [ath10k_sta_update_stats_rx_duration() vs.
> > > ath10k_sta_update_extd_stats_rx_duration() - they are almost carbon copies too]
[shafi] my concern was for updating 'rx_duration' we just iterate over the list
and update the same.
> > >
> > > > > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c b/drivers/net/wireless/ath/ath10k/wmi.c
> > > > > index c893314a191f..c7ec7b9e9b55 100644
> > > > > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > > > > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > > > > @@ -3044,23 +3044,41 @@ static int ath10k_wmi_10_4_op_pull_fw_stats(struct ath10k *ar,
> > > > > if ((stats_id & WMI_10_4_STAT_PEER_EXTD) == 0)
> > > > > return 0;
> > > > >
> > > > > - stats->extended = true;
> > > > > -
> > > > > for (i = 0; i < num_peer_stats; i++) {
> > > > > const struct wmi_10_4_peer_extd_stats *src;
> > > > > - struct ath10k_fw_extd_stats_peer *dst;
> > > > > + struct ath10k_fw_stats_peer *dst;
> > > > >
> > > > > src = (void *)skb->data;
> > > > > if (!skb_pull(skb, sizeof(*src)))
> > > > > return -EPROTO;
> > > > >
> > > > > - dst = kzalloc(sizeof(*dst), GFP_ATOMIC);
> > > > > - if (!dst)
> > > > > - continue;
> > > > > + /* Because the stat data may exceed htc-wmi buffer
> > > > > + * limit the firmware might split the stats data
> > > > > + * and delivers it in multiple update events.
> > > > > + * if we can't find the entry in the current event
> > > > > + * payload, we have to look in main list as well.
> > > > > + */
> >
> > [shafi] thanks ! sorry i might have missed this bug, did you happen
> > to see this condition being hit ?
> This was copied from ath10k_debug_fw_stats_process() [2]. I've added Michal
> Kazior to the discussion since his patch "ath10k: fix fw stats processing"
> added this in debug.c. I think he knows the firmware internals well enough
> to tell if this is a problem or not. As it stands now, it is and will be
> in the future.
[shafi] sure.
>
> > > > > + list_for_each_entry(dst, &stats->peers, list) {
> > > > > + if (ether_addr_equal(dst->peer_macaddr,
> > > > > + src->peer_macaddr.addr))
> > > > > + goto found;
> > > > > + }
> > > > > +
> >
> > [shafi] Again, we can simply cache the macaddress and rx_duration
> > and deal with it later, rather than doing the whole lookup here ?
> > Will it be an overhead for large number of clients ?
> Well, show me how you would do it more efficiently otherwise? I don't
> see how you can cache the macaddress and rx_duration since you are
> basically forced to process all the peer stats first and later all
> the extended peer stats. They don't mix.
[shafi] hmmm, ok.
>
> As for how expensive it is: From what I can tell, the 10.4 firmware
> sends the stat events every few seconds. So they are not part of any
> rx or tx hot-paths. The list_for_each within the for
> (i = 0; i < num_peers;i++) is actually in the O(1) class as far
> as both loops go. This might sound funny at first, but the number of
> extended peer list is limited by TARGET_10_4_MAX_PEER_EXT_STATS to 16.
> And thanks to (***) the limit of num_peers is also 16. Furthermore
> we can add a if (ath10k_peer_stats_enabled(ar)) guard to skip it
> entirely if the stats are disabled.
[shafi] ok.
>
>
> > > > > +#ifdef CONFIG_ATH10K_DEBUGFS
> > > > > + list_for_each_entry(dst, &ar->debug.fw_stats.peers, list) {
> > > > > + if (ether_addr_equal(dst->peer_macaddr,
> > > > > + src->peer_macaddr.addr))
> > > > > + goto found;
> > > > > + }
> > > > > +#endif
> >
> > [shafi] again, this could be handled seperately.
> This is more expensive. As fw_stats.peers can contain more entries than 16.
> However, this might be unnecessary if both peers and extended peers stats
> entries in the wmi event are always for the same stations.
[shafi] ok
>
> Note: There's an alternative way too. Instead of writing rx_duration into
> ath10k_fw_stats, we could skip the middle man write it directly into
> arsta->rx_duration. This would also mean that we can get rid of
> ath10k_sta_update_extd_stats_rx_duration(), ath10k_sta_update_stats_rx_duration()
> and ath10k_sta_update_rx_duration(). This has the benifit that we can
> remove even more.
>
> > > > > +
> > > > > + ath10k_dbg(ar, ATH10K_DBG_WMI,
> > > > > + "Orphaned extended stats entry for station %pM.\n",
> > > > > + src->peer_macaddr.addr);
> > > > > + continue;
> > > > >
> > > > > - ether_addr_copy(dst->peer_macaddr, src->peer_macaddr.addr);
> > > > > +found:
> > > > > dst->rx_duration = __le32_to_cpu(src->rx_duration);
> > > > > - list_add_tail(&dst->list, &stats->peers_extd);
> > > > > }
> > > > >
> > > > > return 0;
> > > >
> > > > [shafi] Yes i am bit concerned about this change making 10.4 to update
> > > > over the existing peer_stats structure, the idea is to maintain uniformity
> > > > between the structures shared between ath10k and its corresponding to avoid
> > > > confusion/ bugs in the future. Kindly let me know your thoughts, feel free
> > > > to correct me if any of my analysis is incorrect. thank you !
> > > Isn't the point of the ath10k_wmi_10_*_op_pull_fw_stats() functions to make
> > > a "universal" statistic (in your case a unified ath10k_fw_stats_peer structure)
> > > that other functions can use, without caring about if the FW was 10.4
> > > or 10.2.4?
> > >
> > > There's no indication that the rx_duration field in wmi_10_2_4_ext_peer_stats
> > > conveys any different information than the rx_duration in
> > > wmi_10_4_peer_extd_stats. So, what's going on here? Can't you just make
> > > wmi_10_4_peer_extd_stats's rx_duration use the existing field in
> > > ath10k_fw_stats_peer? And if not: why exactly?
> > >
> > [shafi] I will soon test your change and keep you posted. We will also
> > get Kalle's take/view on this.
> Ok. That said, I added him to the CC from the beginning and so far
> he silently agreed.
[shafi] sure, I guess i quickly tested this change and I found things
are fine, it would be good if you can share your test results as well please
Yes we can discuss with Kalle and Michal ! If you guys think this is a more
optimized version of updating the peer stats, I am fine with that as well.
There is no doubt you guys know about open source better than me :-)
>
> Regards,
> Christian
>
> [0] <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath10k/debugfs_sta.c#L21>
> [1] <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath10k/debugfs_sta.c#L40>
> [2] <http://lxr.free-electrons.com/source/drivers/net/wireless/ath/ath10k/debug.c#L360>
>
>
^ permalink raw reply
* gpio outputs that disable wireless to PCI Express Mini Card and RFKILL
From: Tim Harvey @ 2016-12-29 15:07 UTC (permalink / raw)
To: linux-wireless
Greetings,
The PCI Express Mini Card (aka Mini PCIe) spec calls out pin 20 as
W_DISABLE# to allow a PCIe host to disable the add-in card's radio
operation and we have run this to a GPIO on our boards for some time.
The M.2 specs also provide such signals. Is there any support in the
Linux RFKILL subsystem to define this?
Looking over the RFKILL subsystem I see support for wireless drivers
to register with rfkill to support on/off/state hooks and support for
gpio based rfkill 'input' switches but I haven't seen anything that
deals with GPIO 'outputs' to add-in card slots. Perhaps support for
this hasn't been deemed necessary because instead software
controllable methods are always used to control rfkill states for the
wireless devices on add-in cards?
Best regards,
Tim
Tim Harvey - Principal Software Engineer
Gateworks Corporation - http://www.gateworks.com/
3026 S. Higuera St. San Luis Obispo CA 93401
805-781-2000
^ 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