2018-05-31 09:11:16

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 0/6] qtnfmac: enable more features

Hello Kalle and all,

Here is the next update for qtnfmac driver which enables more wireless
features. The major changes include the following items:
- cfg80211 power management callback
- net_device_ops set MAC addr callback
- advanced scan features: multiple SSIDs and randomized source MACs
- basic WoWLAN support: disconnect/magic/pattern triggers
-- not yet: PCIe backend suspend/resume, reporting wakeup reason

Regards,
Sergey

Sergei Maksimenko (1)
qtnfmac: implement cfg80211 power management callback

Sergey Matyukevich (1)
qtnfmac: implement basic WoWLAN support

Igor Mitsyanko (1)
qtnfmac: implement net_device_ops callback to set MAC address

Andrey Shevchenko (3)
qtnfmac: add support for PTA configuration
qtnfmac: enable multiple SSIDs scan support
qtnfmac: enable source MAC address randomization support


Makefile | 4 -
cfg80211.c | 107 ++++++++++++++++++++++++++++
commands.c | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
commands.h | 5 +
core.c | 26 ++++++
core.h | 3
qlink.h | 165 +++++++++++++++++++++++++++++++++++++++++++
vendor.c | 145 ++++++++++++++++++++++++++++++++++++++
vendor.h | 35 +++++++++
9 files changed, 711 insertions(+), 9 deletions(-)


2018-05-31 11:12:20

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/6] qtnfmac: enable more features

Sergey Matyukevich <[email protected]> writes:

> Hello Kalle,
>
>> > Here is the next update for qtnfmac driver which enables more wireless
>> > features. The major changes include the following items:
>> > - cfg80211 power management callback
>> > - net_device_ops set MAC addr callback
>> > - advanced scan features: multiple SSIDs and randomized source MACs
>> > - basic WoWLAN support: disconnect/magic/pattern triggers
>> > -- not yet: PCIe backend suspend/resume, reporting wakeup reason
>>
>> BTW, most likely the merge window will start on Sunday and I'm planning
>> to send the last w-d-next pull request to Dave today. So you just missed
>> the window to get these to 4.18 by a day.
>
> Ok. Thanks for letting us know. I assume that our previous 'fixes' patch
> series (which you applied today) will be included into your
> 4.18 pull request. Correct ?

Correct. Everything in w-d-next as of now will be included to the pull
request.

--
Kalle Valo

2018-05-31 09:11:21

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 3/6] qtnfmac: implement cfg80211 power management callback

From: Sergei Maksimenko <[email protected]>

Implement set_power_mgmt() callback that forwards power saving
settings to the device firmware.

Signed-off-by: Sergei Maksimenko <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 21 ++++++++++++++
drivers/net/wireless/quantenna/qtnfmac/commands.c | 34 +++++++++++++++++++++++
drivers/net/wireless/quantenna/qtnfmac/commands.h | 1 +
drivers/net/wireless/quantenna/qtnfmac/qlink.h | 26 +++++++++++++++++
4 files changed, 82 insertions(+)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 23366be9e394..15bc26c9933f 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -844,6 +844,22 @@ static int qtnf_set_mac_acl(struct wiphy *wiphy,
return ret;
}

+static int qtnf_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
+ bool enabled, int timeout)
+{
+ struct qtnf_vif *vif = qtnf_netdev_get_priv(dev);
+ int ret;
+
+ ret = qtnf_cmd_send_pm_set(vif, enabled ? QLINK_PM_AUTO_STANDBY :
+ QLINK_PM_OFF, timeout);
+ if (ret) {
+ pr_err("%s: failed to set PM mode ret=%d\n", dev->name, ret);
+ return ret;
+ }
+
+ return ret;
+}
+
static struct cfg80211_ops qtn_cfg80211_ops = {
.add_virtual_intf = qtnf_add_virtual_intf,
.change_virtual_intf = qtnf_change_virtual_intf,
@@ -870,6 +886,7 @@ static struct cfg80211_ops qtn_cfg80211_ops = {
.channel_switch = qtnf_channel_switch,
.start_radar_detection = qtnf_start_radar_detection,
.set_mac_acl = qtnf_set_mac_acl,
+ .set_power_mgmt = qtnf_set_power_mgmt,
};

static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy_in,
@@ -922,6 +939,9 @@ struct wiphy *qtnf_wiphy_allocate(struct qtnf_bus *bus)
if (bus->hw_info.hw_capab & QLINK_HW_CAPAB_DFS_OFFLOAD)
qtn_cfg80211_ops.start_radar_detection = NULL;

+ if (!(bus->hw_info.hw_capab & QLINK_HW_CAPAB_PWR_MGMT))
+ qtn_cfg80211_ops.set_power_mgmt = NULL;
+
wiphy = wiphy_new(&qtn_cfg80211_ops, sizeof(struct qtnf_wmac));
if (!wiphy)
return NULL;
@@ -995,6 +1015,7 @@ int qtnf_wiphy_register(struct qtnf_hw_info *hw_info, struct qtnf_wmac *mac)
WIPHY_FLAG_AP_PROBE_RESP_OFFLOAD |
WIPHY_FLAG_AP_UAPSD |
WIPHY_FLAG_HAS_CHANNEL_SWITCH;
+ wiphy->flags &= ~WIPHY_FLAG_PS_ON_BY_DEFAULT;

if (hw_info->hw_capab & QLINK_HW_CAPAB_DFS_OFFLOAD)
wiphy_ext_feature_set(wiphy, NL80211_EXT_FEATURE_DFS_OFFLOAD);
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 710806466bd9..a96d58f72c07 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -2799,3 +2799,37 @@ int qtnf_cmd_set_mac_acl(const struct qtnf_vif *vif,

return ret;
}
+
+int qtnf_cmd_send_pm_set(const struct qtnf_vif *vif, u8 pm_mode, int timeout)
+{
+ struct qtnf_bus *bus = vif->mac->bus;
+ struct sk_buff *cmd_skb;
+ u16 res_code = QLINK_CMD_RESULT_OK;
+ struct qlink_cmd_pm_set *cmd;
+ int ret = 0;
+
+ cmd_skb = qtnf_cmd_alloc_new_cmdskb(vif->mac->macid, vif->vifid,
+ QLINK_CMD_PM_SET, sizeof(*cmd));
+ if (!cmd_skb)
+ return -ENOMEM;
+
+ cmd = (struct qlink_cmd_pm_set *)cmd_skb->data;
+ cmd->pm_mode = pm_mode;
+ cmd->pm_standby_timer = cpu_to_le32(timeout);
+
+ qtnf_bus_lock(bus);
+
+ ret = qtnf_cmd_send(bus, cmd_skb, &res_code);
+
+ if (unlikely(ret))
+ goto out;
+
+ if (unlikely(res_code != QLINK_CMD_RESULT_OK)) {
+ pr_err("cmd exec failed: 0x%.4X\n", res_code);
+ ret = -EFAULT;
+ }
+
+out:
+ qtnf_bus_unlock(bus);
+ return ret;
+}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.h b/drivers/net/wireless/quantenna/qtnfmac/commands.h
index cf9274add26d..03a57e37a665 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.h
@@ -76,5 +76,6 @@ int qtnf_cmd_start_cac(const struct qtnf_vif *vif,
u32 cac_time_ms);
int qtnf_cmd_set_mac_acl(const struct qtnf_vif *vif,
const struct cfg80211_acl_data *params);
+int qtnf_cmd_send_pm_set(const struct qtnf_vif *vif, u8 pm_mode, int timeout);

#endif /* QLINK_COMMANDS_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 4a32967d0479..cbdebf0410df 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -77,6 +77,7 @@ enum qlink_hw_capab {
QLINK_HW_CAPAB_STA_INACT_TIMEOUT = BIT(1),
QLINK_HW_CAPAB_DFS_OFFLOAD = BIT(2),
QLINK_HW_CAPAB_SCAN_RANDOM_MAC_ADDR = BIT(3),
+ QLINK_HW_CAPAB_PWR_MGMT = BIT(4),
};

enum qlink_iface_type {
@@ -256,6 +257,7 @@ enum qlink_cmd_type {
QLINK_CMD_CHAN_STATS = 0x0054,
QLINK_CMD_CONNECT = 0x0060,
QLINK_CMD_DISCONNECT = 0x0061,
+ QLINK_CMD_PM_SET = 0x0062,
};

/**
@@ -668,6 +670,30 @@ struct qlink_acl_data {
struct qlink_mac_address mac_addrs[0];
} __packed;

+/**
+ * enum qlink_pm_mode - Power Management mode
+ *
+ * @QLINK_PM_OFF: normal mode, no power saving enabled
+ * @QLINK_PM_AUTO_STANDBY: enable auto power save mode
+ */
+enum qlink_pm_mode {
+ QLINK_PM_OFF = 0,
+ QLINK_PM_AUTO_STANDBY = 1,
+};
+
+/**
+ * struct qlink_cmd_pm_set - data for QLINK_CMD_PM_SET command
+ *
+ * @pm_standby timer: period of network inactivity in seconds before
+ * putting a radio in power save mode
+ * @pm_mode: power management mode
+ */
+struct qlink_cmd_pm_set {
+ struct qlink_cmd chdr;
+ __le32 pm_standby_timer;
+ u8 pm_mode;
+} __packed;
+
/* QLINK Command Responses messages related definitions
*/

--
2.11.0

2018-05-31 09:36:28

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/6] qtnfmac: enable more features

Sergey Matyukevich <[email protected]> writes:

> Here is the next update for qtnfmac driver which enables more wireless
> features. The major changes include the following items:
> - cfg80211 power management callback
> - net_device_ops set MAC addr callback
> - advanced scan features: multiple SSIDs and randomized source MACs
> - basic WoWLAN support: disconnect/magic/pattern triggers
> -- not yet: PCIe backend suspend/resume, reporting wakeup reason

BTW, most likely the merge window will start on Sunday and I'm planning
to send the last w-d-next pull request to Dave today. So you just missed
the window to get these to 4.18 by a day.

--
Kalle Valo

2018-05-31 09:11:19

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 2/6] qtnfmac: enable source MAC address randomization support

From: Andrey Shevchenko <[email protected]>

Enable support for source MAC address randomization of probe request
frames. Pass addr/mask randomization parameters to firmware.

Signed-off-by: Andrey Shevchenko <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 3 +++
drivers/net/wireless/quantenna/qtnfmac/commands.c | 25 +++++++++++++++++++++++
drivers/net/wireless/quantenna/qtnfmac/qlink.h | 20 ++++++++++++++++++
3 files changed, 48 insertions(+)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 220e2b710208..23366be9e394 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -1014,6 +1014,9 @@ int qtnf_wiphy_register(struct qtnf_hw_info *hw_info, struct qtnf_wmac *mac)
if (hw_info->hw_capab & QLINK_HW_CAPAB_STA_INACT_TIMEOUT)
wiphy->features |= NL80211_FEATURE_INACTIVITY_TIMER;

+ if (hw_info->hw_capab & QLINK_HW_CAPAB_SCAN_RANDOM_MAC_ADDR)
+ wiphy->features |= NL80211_FEATURE_SCAN_RANDOM_MAC_ADDR;
+
if (hw_info->hw_capab & QLINK_HW_CAPAB_REG_UPDATE) {
wiphy->regulatory_flags |= REGULATORY_STRICT_REG |
REGULATORY_CUSTOM_REG;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 5eb143667539..710806466bd9 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -2234,6 +2234,22 @@ static void qtnf_cmd_channel_tlv_add(struct sk_buff *cmd_skb,
qchan->chan.flags = cpu_to_le32(flags);
}

+static void qtnf_cmd_randmac_tlv_add(struct sk_buff *cmd_skb,
+ const u8 *mac_addr,
+ const u8 *mac_addr_mask)
+{
+ struct qlink_random_mac_addr *randmac;
+ struct qlink_tlv_hdr *hdr =
+ skb_put(cmd_skb, sizeof(*hdr) + sizeof(*randmac));
+
+ hdr->type = cpu_to_le16(QTN_TLV_ID_RANDOM_MAC_ADDR);
+ hdr->len = cpu_to_le16(sizeof(*randmac));
+ randmac = (struct qlink_random_mac_addr *)hdr->val;
+
+ memcpy(randmac->mac_addr, mac_addr, ETH_ALEN);
+ memcpy(randmac->mac_addr_mask, mac_addr_mask, ETH_ALEN);
+}
+
int qtnf_cmd_send_scan(struct qtnf_wmac *mac)
{
struct sk_buff *cmd_skb;
@@ -2291,6 +2307,15 @@ int qtnf_cmd_send_scan(struct qtnf_wmac *mac)
}
}

+ if (scan_req->flags & NL80211_SCAN_FLAG_RANDOM_ADDR) {
+ pr_debug("MAC%u: scan with random addr=%pM, mask=%pM\n",
+ mac->macid,
+ scan_req->mac_addr, scan_req->mac_addr_mask);
+
+ qtnf_cmd_randmac_tlv_add(cmd_skb, scan_req->mac_addr,
+ scan_req->mac_addr_mask);
+ }
+
ret = qtnf_cmd_send(mac->bus, cmd_skb, &res_code);

if (unlikely(ret))
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index f85deda703fb..4a32967d0479 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -69,11 +69,14 @@ struct qlink_msg_header {
* associated STAs due to inactivity. Inactivity timeout period is taken
* from QLINK_CMD_START_AP parameters.
* @QLINK_HW_CAPAB_DFS_OFFLOAD: device implements DFS offload functionality
+ * @QLINK_HW_CAPAB_SCAN_RANDOM_MAC_ADDR: device supports MAC Address
+ * Randomization in probe requests.
*/
enum qlink_hw_capab {
QLINK_HW_CAPAB_REG_UPDATE = BIT(0),
QLINK_HW_CAPAB_STA_INACT_TIMEOUT = BIT(1),
QLINK_HW_CAPAB_DFS_OFFLOAD = BIT(2),
+ QLINK_HW_CAPAB_SCAN_RANDOM_MAC_ADDR = BIT(3),
};

enum qlink_iface_type {
@@ -1089,6 +1092,7 @@ enum qlink_tlv_id {
QTN_TLV_ID_HW_ID = 0x0405,
QTN_TLV_ID_CALIBRATION_VER = 0x0406,
QTN_TLV_ID_UBOOT_VER = 0x0407,
+ QTN_TLV_ID_RANDOM_MAC_ADDR = 0x0408,
};

struct qlink_tlv_hdr {
@@ -1360,4 +1364,20 @@ struct qlink_sta_stats {
u8 rsvd[1];
};

+/**
+ * struct qlink_random_mac_addr - data for QTN_TLV_ID_RANDOM_MAC_ADDR TLV
+ *
+ * Specifies MAC address mask/value for generation random MAC address
+ * during scan.
+ *
+ * @mac_addr: MAC address used with randomisation
+ * @mac_addr_mask: MAC address mask used with randomisation, bits that
+ * are 0 in the mask should be randomised, bits that are 1 should
+ * be taken from the @mac_addr
+ */
+struct qlink_random_mac_addr {
+ u8 mac_addr[ETH_ALEN];
+ u8 mac_addr_mask[ETH_ALEN];
+} __packed;
+
#endif /* _QTN_QLINK_H_ */
--
2.11.0

2018-05-31 09:11:30

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 6/6] qtnfmac: implement basic WoWLAN support

This patch implements core WoWLAN support in qtnfmac driver, including
processing of WoWLAN features reported by firmware and implementation
of cfg80211 suspend/resume/wakeup callbacks. Currently the following
WoWLAN triggers are supported: disconnect, magic packet,
custom pattern packet.

Signed-off-by: Sergey Matyukevich <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 76 +++++++++++++++
drivers/net/wireless/quantenna/qtnfmac/commands.c | 112 ++++++++++++++++++++++
drivers/net/wireless/quantenna/qtnfmac/commands.h | 3 +
drivers/net/wireless/quantenna/qtnfmac/core.c | 1 +
drivers/net/wireless/quantenna/qtnfmac/core.h | 1 +
drivers/net/wireless/quantenna/qtnfmac/qlink.h | 56 +++++++++++
6 files changed, 249 insertions(+)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 956852748cb1..c0d5ca37168b 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -861,6 +861,72 @@ static int qtnf_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
return ret;
}

+#ifdef CONFIG_PM
+static int qtnf_suspend(struct wiphy *wiphy, struct cfg80211_wowlan *wowlan)
+{
+ struct qtnf_wmac *mac = wiphy_priv(wiphy);
+ struct qtnf_vif *vif;
+ int ret = 0;
+
+ vif = qtnf_mac_get_base_vif(mac);
+ if (!vif) {
+ pr_err("MAC%u: primary VIF is not configured\n", mac->macid);
+ ret = -EFAULT;
+ goto exit;
+ }
+
+ if (!wowlan) {
+ pr_debug("WoWLAN triggers are not enabled\n");
+ qtnf_virtual_intf_cleanup(vif->netdev);
+ goto exit;
+ }
+
+ qtnf_scan_done(vif->mac, true);
+
+ ret = qtnf_cmd_send_wowlan_set(vif, wowlan);
+ if (ret) {
+ pr_err("MAC%u: failed to set WoWLAN triggers\n",
+ mac->macid);
+ goto exit;
+ }
+
+exit:
+ return ret;
+}
+
+static int qtnf_resume(struct wiphy *wiphy)
+{
+ struct qtnf_wmac *mac = wiphy_priv(wiphy);
+ struct qtnf_vif *vif;
+ int ret = 0;
+
+ vif = qtnf_mac_get_base_vif(mac);
+ if (!vif) {
+ pr_err("MAC%u: primary VIF is not configured\n", mac->macid);
+ ret = -EFAULT;
+ goto exit;
+ }
+
+ ret = qtnf_cmd_send_wowlan_set(vif, NULL);
+ if (ret) {
+ pr_err("MAC%u: failed to reset WoWLAN triggers\n",
+ mac->macid);
+ goto exit;
+ }
+
+exit:
+ return ret;
+}
+
+static void qtnf_set_wakeup(struct wiphy *wiphy, bool enabled)
+{
+ struct qtnf_wmac *mac = wiphy_priv(wiphy);
+ struct qtnf_bus *bus = mac->bus;
+
+ device_set_wakeup_enable(bus->dev, enabled);
+}
+#endif
+
static struct cfg80211_ops qtn_cfg80211_ops = {
.add_virtual_intf = qtnf_add_virtual_intf,
.change_virtual_intf = qtnf_change_virtual_intf,
@@ -888,6 +954,11 @@ static struct cfg80211_ops qtn_cfg80211_ops = {
.start_radar_detection = qtnf_start_radar_detection,
.set_mac_acl = qtnf_set_mac_acl,
.set_power_mgmt = qtnf_set_power_mgmt,
+#ifdef CONFIG_PM
+ .suspend = qtnf_suspend,
+ .resume = qtnf_resume,
+ .set_wakeup = qtnf_set_wakeup,
+#endif
};

static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy_in,
@@ -1040,6 +1111,11 @@ int qtnf_wiphy_register(struct qtnf_hw_info *hw_info, struct qtnf_wmac *mac)
if (hw_info->hw_capab & QLINK_HW_CAPAB_SCAN_RANDOM_MAC_ADDR)
wiphy->features |= NL80211_FEATURE_SCAN_RANDOM_MAC_ADDR;

+#ifdef CONFIG_PM
+ if (macinfo->wowlan)
+ wiphy->wowlan = macinfo->wowlan;
+#endif
+
if (hw_info->hw_capab & QLINK_HW_CAPAB_REG_UPDATE) {
wiphy->regulatory_flags |= REGULATORY_STRICT_REG |
REGULATORY_CUSTOM_REG;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 45a2effa3eab..d9540edf78de 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -1139,6 +1139,37 @@ qtnf_cmd_resp_proc_hw_info(struct qtnf_bus *bus,
return 0;
}

+static void
+qtnf_parse_wowlan_info(struct qtnf_wmac *mac,
+ const struct qlink_wowlan_capab_data *wowlan)
+{
+ struct qtnf_mac_info *mac_info = &mac->macinfo;
+ const struct qlink_wowlan_support *data1;
+ struct wiphy_wowlan_support *supp;
+
+ supp = kzalloc(sizeof(*supp), GFP_KERNEL);
+ if (!supp)
+ return;
+
+ switch (le16_to_cpu(wowlan->version)) {
+ case 0x1:
+ data1 = (struct qlink_wowlan_support *)wowlan->data;
+
+ supp->flags = WIPHY_WOWLAN_MAGIC_PKT | WIPHY_WOWLAN_DISCONNECT;
+ supp->n_patterns = le32_to_cpu(data1->n_patterns);
+ supp->pattern_max_len = le32_to_cpu(data1->pattern_max_len);
+ supp->pattern_min_len = le32_to_cpu(data1->pattern_min_len);
+
+ mac_info->wowlan = supp;
+ break;
+ default:
+ pr_warn("MAC%u: unsupported WoWLAN version 0x%x\n",
+ mac->macid, le16_to_cpu(wowlan->version));
+ kfree(supp);
+ break;
+ }
+}
+
static int qtnf_parse_variable_mac_info(struct qtnf_wmac *mac,
const u8 *tlv_buf, size_t tlv_buf_size)
{
@@ -1148,6 +1179,7 @@ static int qtnf_parse_variable_mac_info(struct qtnf_wmac *mac,
const struct qlink_iface_comb_num *comb_num;
const struct qlink_iface_limit_record *rec;
const struct qlink_iface_limit *lim;
+ const struct qlink_wowlan_capab_data *wowlan;
u16 rec_len;
u16 tlv_type;
u16 tlv_value_len;
@@ -1256,7 +1288,31 @@ static int qtnf_parse_variable_mac_info(struct qtnf_wmac *mac,
ext_capa_mask = (u8 *)tlv->val;
ext_capa_mask_len = tlv_value_len;
break;
+ case QTN_TLV_ID_WOWLAN_CAPAB:
+ if (tlv_value_len < sizeof(*wowlan))
+ return -EINVAL;
+
+ wowlan = (void *)tlv->val;
+ if (!le16_to_cpu(wowlan->len)) {
+ pr_warn("MAC%u: skip empty WoWLAN data\n",
+ mac->macid);
+ break;
+ }
+
+ rec_len = sizeof(*wowlan) + le16_to_cpu(wowlan->len);
+ if (unlikely(tlv_value_len != rec_len)) {
+ pr_warn("MAC%u: WoWLAN data size mismatch\n",
+ mac->macid);
+ return -EINVAL;
+ }
+
+ kfree(mac->macinfo.wowlan);
+ mac->macinfo.wowlan = NULL;
+ qtnf_parse_wowlan_info(mac, wowlan);
+ break;
default:
+ pr_warn("MAC%u: unknown TLV type %u\n",
+ mac->macid, tlv_type);
break;
}

@@ -2882,3 +2938,59 @@ int qtnf_cmd_setget_pta_param(struct qtnf_wmac *mac,
consume_skb(resp_skb);
return ret;
}
+
+int qtnf_cmd_send_wowlan_set(const struct qtnf_vif *vif,
+ const struct cfg80211_wowlan *wowl)
+{
+ struct qtnf_bus *bus = vif->mac->bus;
+ struct sk_buff *cmd_skb;
+ u16 res_code = QLINK_CMD_RESULT_OK;
+ struct qlink_cmd_wowlan_set *cmd;
+ u32 triggers = 0;
+ int count = 0;
+ int ret = 0;
+
+ cmd_skb = qtnf_cmd_alloc_new_cmdskb(vif->mac->macid, vif->vifid,
+ QLINK_CMD_WOWLAN_SET, sizeof(*cmd));
+ if (!cmd_skb)
+ return -ENOMEM;
+
+ qtnf_bus_lock(bus);
+
+ cmd = (struct qlink_cmd_wowlan_set *)cmd_skb->data;
+
+ if (wowl) {
+ if (wowl->disconnect)
+ triggers |= QLINK_WOWLAN_TRIG_DISCONNECT;
+
+ if (wowl->magic_pkt)
+ triggers |= QLINK_WOWLAN_TRIG_MAGIC_PKT;
+
+ if (wowl->n_patterns && wowl->patterns) {
+ triggers |= QLINK_WOWLAN_TRIG_PATTERN_PKT;
+ while (count < wowl->n_patterns) {
+ qtnf_cmd_skb_put_tlv_arr(cmd_skb,
+ QTN_TLV_ID_WOWLAN_PATTERN,
+ wowl->patterns[count].pattern,
+ wowl->patterns[count].pattern_len);
+ count++;
+ }
+ }
+ }
+
+ cmd->triggers = cpu_to_le32(triggers);
+
+ ret = qtnf_cmd_send(bus, cmd_skb, &res_code);
+
+ if (unlikely(ret))
+ goto out;
+
+ if (unlikely(res_code != QLINK_CMD_RESULT_OK)) {
+ pr_err("cmd exec failed: 0x%.4X\n", res_code);
+ ret = -EFAULT;
+ }
+
+out:
+ qtnf_bus_unlock(bus);
+ return ret;
+}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.h b/drivers/net/wireless/quantenna/qtnfmac/commands.h
index 59e86db5e982..08cb98e8ba7f 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.h
@@ -79,4 +79,7 @@ int qtnf_cmd_set_mac_acl(const struct qtnf_vif *vif,
int qtnf_cmd_send_pm_set(const struct qtnf_vif *vif, u8 pm_mode, int timeout);
int qtnf_cmd_setget_pta_param(struct qtnf_wmac *mac,
int param_id, bool set_op, int *param_val);
+int qtnf_cmd_send_wowlan_set(const struct qtnf_vif *vif,
+ const struct cfg80211_wowlan *wowl);
+
#endif /* QLINK_COMMANDS_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c
index c318340e1bd5..19abbc4e23e0 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -495,6 +495,7 @@ static void qtnf_core_mac_detach(struct qtnf_bus *bus, unsigned int macid)
qtnf_mac_iface_comb_free(mac);
kfree(mac->macinfo.extended_capabilities);
kfree(mac->macinfo.extended_capabilities_mask);
+ kfree(mac->macinfo.wowlan);
wiphy_free(wiphy);
bus->mac[macid] = NULL;
}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index c4808f1ba8b0..a1e338a1f055 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -110,6 +110,7 @@ struct qtnf_mac_info {
u8 *extended_capabilities;
u8 *extended_capabilities_mask;
u8 extended_capabilities_len;
+ struct wiphy_wowlan_support *wowlan;
};

struct qtnf_chan_stats {
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 582d82ceaeef..8823ab12ac3a 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -258,6 +258,7 @@ enum qlink_cmd_type {
QLINK_CMD_CONNECT = 0x0060,
QLINK_CMD_DISCONNECT = 0x0061,
QLINK_CMD_PM_SET = 0x0062,
+ QLINK_CMD_WOWLAN_SET = 0x0063,
QLINK_CMD_PTA_PARAM = 0x0064,
};

@@ -741,6 +742,30 @@ struct qlink_cmd_pta_param {
__le32 pta_param_value;
} __packed;

+/**
+ * enum qlink_wowlan_trigger
+ *
+ * @QLINK_WOWLAN_TRIG_DISCONNECT: wakeup on disconnect
+ * @QLINK_WOWLAN_TRIG_MAGIC_PKT: wakeup on magic packet
+ * @QLINK_WOWLAN_TRIG_PATTERN_PKT: wakeup on user-defined packet
+ */
+enum qlink_wowlan_trigger {
+ QLINK_WOWLAN_TRIG_DISCONNECT = BIT(0),
+ QLINK_WOWLAN_TRIG_MAGIC_PKT = BIT(1),
+ QLINK_WOWLAN_TRIG_PATTERN_PKT = BIT(2),
+};
+
+/**
+ * struct qlink_cmd_wowlan_set - data for QLINK_CMD_WOWLAN_SET command
+ *
+ * @triggers: requested bitmask of WoWLAN triggers
+ */
+struct qlink_cmd_wowlan_set {
+ struct qlink_cmd chdr;
+ __le32 triggers;
+ u8 data[0];
+} __packed;
+
/* QLINK Command Responses messages related definitions
*/

@@ -1182,6 +1207,8 @@ enum qlink_tlv_id {
QTN_TLV_ID_UBOOT_VER = 0x0407,
QTN_TLV_ID_RANDOM_MAC_ADDR = 0x0408,
QTN_TLV_ID_MAX_SCAN_SSIDS = 0x0409,
+ QTN_TLV_ID_WOWLAN_CAPAB = 0x0410,
+ QTN_TLV_ID_WOWLAN_PATTERN = 0x0411,
};

struct qlink_tlv_hdr {
@@ -1469,4 +1496,33 @@ struct qlink_random_mac_addr {
u8 mac_addr_mask[ETH_ALEN];
} __packed;

+/**
+ * struct qlink_wowlan_capab_data - data for QTN_TLV_ID_WOWLAN_CAPAB TLV
+ *
+ * WoWLAN capabilities supported by cards.
+ *
+ * @version: version of WoWLAN data structure, to ensure backward
+ * compatibility for firmwares with limited WoWLAN support
+ * @len: Total length of WoWLAN data
+ * @data: supported WoWLAN features
+ */
+struct qlink_wowlan_capab_data {
+ __le16 version;
+ __le16 len;
+ u8 data[0];
+} __packed;
+
+/**
+ * struct qlink_wowlan_support - supported WoWLAN capabilities
+ *
+ * @n_patterns: number of supported wakeup patterns
+ * @pattern_max_len: maximum length of each pattern
+ * @pattern_min_len: minimum length of each pattern
+ */
+struct qlink_wowlan_support {
+ __le32 n_patterns;
+ __le32 pattern_max_len;
+ __le32 pattern_min_len;
+} __packed;
+
#endif /* _QTN_QLINK_H_ */
--
2.11.0

2018-05-31 10:35:18

by Sergey Matyukevich

[permalink] [raw]
Subject: Re: [PATCH 0/6] qtnfmac: enable more features

Hello Kalle,

> > Here is the next update for qtnfmac driver which enables more wireless
> > features. The major changes include the following items:
> > - cfg80211 power management callback
> > - net_device_ops set MAC addr callback
> > - advanced scan features: multiple SSIDs and randomized source MACs
> > - basic WoWLAN support: disconnect/magic/pattern triggers
> > -- not yet: PCIe backend suspend/resume, reporting wakeup reason
>
> BTW, most likely the merge window will start on Sunday and I'm planning
> to send the last w-d-next pull request to Dave today. So you just missed
> the window to get these to 4.18 by a day.

Ok. Thanks for letting us know. I assume that our previous 'fixes' patch
series (which you applied today) will be included into your
4.18 pull request. Correct ?

Regards,
Sergey

2018-05-31 09:11:24

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 4/6] qtnfmac: enable multiple SSIDs scan support

From: Andrey Shevchenko <[email protected]>

Enable support for multiple SSIDs scans. Get max number of supported
SSIDs from firmware and report to cfg80211 core.

Signed-off-by: Andrey Shevchenko <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 3 ++-
drivers/net/wireless/quantenna/qtnfmac/commands.c | 8 +++-----
drivers/net/wireless/quantenna/qtnfmac/core.h | 2 +-
drivers/net/wireless/quantenna/qtnfmac/qlink.h | 3 +++
4 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 15bc26c9933f..307ab5c59bfd 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -996,7 +996,8 @@ int qtnf_wiphy_register(struct qtnf_hw_info *hw_info, struct qtnf_wmac *mac)
wiphy->retry_long = macinfo->lretry_limit;
wiphy->coverage_class = macinfo->coverage_class;

- wiphy->max_scan_ssids = QTNF_MAX_SSID_LIST_LENGTH;
+ wiphy->max_scan_ssids =
+ (hw_info->max_scan_ssids) ? hw_info->max_scan_ssids : 1;
wiphy->max_scan_ie_len = QTNF_MAX_VSIE_LEN;
wiphy->mgmt_stypes = qtnf_mgmt_stypes;
wiphy->max_remain_on_channel_duration = 5000;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index a96d58f72c07..10836695c5f5 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -1092,6 +1092,9 @@ qtnf_cmd_resp_proc_hw_info(struct qtnf_bus *bus,
case QTN_TLV_ID_UBOOT_VER:
uboot_ver = (const void *)tlv->val;
break;
+ case QTN_TLV_ID_MAX_SCAN_SSIDS:
+ hwinfo->max_scan_ssids = *tlv->val;
+ break;
default:
break;
}
@@ -2260,11 +2263,6 @@ int qtnf_cmd_send_scan(struct qtnf_wmac *mac)
int count = 0;
int ret;

- if (scan_req->n_ssids > QTNF_MAX_SSID_LIST_LENGTH) {
- pr_err("MAC%u: too many SSIDs in scan request\n", mac->macid);
- return -EINVAL;
- }
-
cmd_skb = qtnf_cmd_alloc_new_cmdskb(mac->macid, QLINK_VIFID_RSVD,
QLINK_CMD_SCAN,
sizeof(struct qlink_cmd));
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index 214435448335..c4808f1ba8b0 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -40,7 +40,6 @@
#undef pr_fmt
#define pr_fmt(fmt) KBUILD_MODNAME ": %s: " fmt, __func__

-#define QTNF_MAX_SSID_LIST_LENGTH 2
#define QTNF_MAX_VSIE_LEN 255
#define QTNF_MAX_INTF 8
#define QTNF_MAX_EVENT_QUEUE_LEN 255
@@ -145,6 +144,7 @@ struct qtnf_hw_info {
u8 total_rx_chain;
char fw_version[ETHTOOL_FWVERS_LEN];
u32 hw_version;
+ u8 max_scan_ssids;
};

struct qtnf_vif *qtnf_mac_get_free_vif(struct qtnf_wmac *mac);
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index cbdebf0410df..8fbef67fbbb8 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -1091,6 +1091,8 @@ struct qlink_event_radar {
* @QTN_TLV_ID_STA_STATS: per-STA statistics as defined by
* &struct qlink_sta_stats. Valid values are marked as such in a bitmap
* carried by QTN_TLV_ID_STA_STATS_MAP.
+ * @QTN_TLV_ID_MAX_SCAN_SSIDS: maximum number of SSIDs the device can scan
+ * for in any given scan.
*/
enum qlink_tlv_id {
QTN_TLV_ID_FRAG_THRESH = 0x0201,
@@ -1119,6 +1121,7 @@ enum qlink_tlv_id {
QTN_TLV_ID_CALIBRATION_VER = 0x0406,
QTN_TLV_ID_UBOOT_VER = 0x0407,
QTN_TLV_ID_RANDOM_MAC_ADDR = 0x0408,
+ QTN_TLV_ID_MAX_SCAN_SSIDS = 0x0409,
};

struct qlink_tlv_hdr {
--
2.11.0

2018-05-31 09:11:18

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 1/6] qtnfmac: implement net_device_ops callback to set MAC address

From: Igor Mitsyanko <[email protected]>

Implement net_device_ops::ndo_set_mac_address callback to allow for
setting interface MAC address. Implementation is done through existing
CHANGE_INTF firmware command. All validation is to be done by firmware.

Signed-off-by: Igor Mitsyanko <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/core.c | 25 +++++++++++++++++++++++++
1 file changed, 25 insertions(+)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c
index a6a450984f9a..c318340e1bd5 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -179,6 +179,30 @@ static void qtnf_netdev_tx_timeout(struct net_device *ndev)
}
}

+static int qtnf_netdev_set_mac_address(struct net_device *ndev, void *addr)
+{
+ struct qtnf_vif *vif = qtnf_netdev_get_priv(ndev);
+ struct sockaddr *sa = addr;
+ int ret;
+ unsigned char old_addr[ETH_ALEN];
+
+ memcpy(old_addr, sa->sa_data, sizeof(old_addr));
+
+ ret = eth_mac_addr(ndev, sa);
+ if (ret)
+ return ret;
+
+ qtnf_scan_done(vif->mac, true);
+
+ ret = qtnf_cmd_send_change_intf_type(vif, vif->wdev.iftype,
+ sa->sa_data);
+
+ if (ret)
+ memcpy(ndev->dev_addr, old_addr, ETH_ALEN);
+
+ return ret;
+}
+
/* Network device ops handlers */
const struct net_device_ops qtnf_netdev_ops = {
.ndo_open = qtnf_netdev_open,
@@ -186,6 +210,7 @@ const struct net_device_ops qtnf_netdev_ops = {
.ndo_start_xmit = qtnf_netdev_hard_start_xmit,
.ndo_tx_timeout = qtnf_netdev_tx_timeout,
.ndo_get_stats64 = qtnf_netdev_get_stats64,
+ .ndo_set_mac_address = qtnf_netdev_set_mac_address,
};

static int qtnf_mac_init_single_band(struct wiphy *wiphy,
--
2.11.0

2018-05-31 09:11:25

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 5/6] qtnfmac: add support for PTA configuration

From: Andrey Shevchenko <[email protected]>

Implement support for PTA (Packet Traffic Arbitration) configuration.
The PTA mechanism is used to coordinate sharing of the medium between
WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.

This patch implements core infrastructure and vendor specific commands
to control PTA functionality in firmware.

Signed-off-by: Andrey Shevchenko <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/Makefile | 4 +-
drivers/net/wireless/quantenna/qtnfmac/cfg80211.c | 4 +
drivers/net/wireless/quantenna/qtnfmac/commands.c | 51 ++++++++
drivers/net/wireless/quantenna/qtnfmac/commands.h | 3 +-
drivers/net/wireless/quantenna/qtnfmac/qlink.h | 60 +++++++++
drivers/net/wireless/quantenna/qtnfmac/vendor.c | 145 ++++++++++++++++++++++
drivers/net/wireless/quantenna/qtnfmac/vendor.h | 35 ++++++
7 files changed, 299 insertions(+), 3 deletions(-)
create mode 100644 drivers/net/wireless/quantenna/qtnfmac/vendor.c
create mode 100644 drivers/net/wireless/quantenna/qtnfmac/vendor.h

diff --git a/drivers/net/wireless/quantenna/qtnfmac/Makefile b/drivers/net/wireless/quantenna/qtnfmac/Makefile
index 97f760a3d599..9e058617d694 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/Makefile
+++ b/drivers/net/wireless/quantenna/qtnfmac/Makefile
@@ -15,8 +15,8 @@ qtnfmac-objs += \
cfg80211.o \
event.o \
util.o \
- qlink_util.o
-
+ qlink_util.o \
+ vendor.o
#

obj-$(CONFIG_QTNFMAC_PEARL_PCIE) += qtnfmac_pearl_pcie.o
diff --git a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
index 307ab5c59bfd..956852748cb1 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/cfg80211.c
@@ -26,6 +26,7 @@
#include "core.h"
#include "util.h"
#include "bus.h"
+#include "vendor.h"

/* Supported rates to be advertised to the cfg80211 */
static struct ieee80211_rate qtnf_rates_2g[] = {
@@ -1052,6 +1053,9 @@ int qtnf_wiphy_register(struct qtnf_hw_info *hw_info, struct qtnf_wmac *mac)
sizeof(wiphy->fw_version));
wiphy->hw_version = hw_info->hw_version;

+ wiphy->vendor_commands = qtnf_vendor_cmds;
+ wiphy->n_vendor_commands = qtnf_vendor_get_cmds_number();
+
ret = wiphy_register(wiphy);
if (ret < 0)
goto out;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index 10836695c5f5..45a2effa3eab 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -22,6 +22,7 @@
#include "qlink_util.h"
#include "bus.h"
#include "commands.h"
+#include "vendor.h"

static int qtnf_cmd_check_reply_header(const struct qlink_resp *resp,
u16 cmd_id, u8 mac_id, u8 vif_id,
@@ -2831,3 +2832,53 @@ int qtnf_cmd_send_pm_set(const struct qtnf_vif *vif, u8 pm_mode, int timeout)
qtnf_bus_unlock(bus);
return ret;
}
+
+int qtnf_cmd_setget_pta_param(struct qtnf_wmac *mac,
+ int param_id, bool set_op, int *param_val)
+{
+ struct sk_buff *cmd_skb;
+ struct qlink_cmd_pta_param *cmd;
+ const struct qlink_resp_pta_param *resp;
+ struct sk_buff *resp_skb = NULL;
+ u16 res_code;
+ int ret;
+
+ if (!param_val)
+ return -EINVAL;
+
+ cmd_skb = qtnf_cmd_alloc_new_cmdskb(mac->macid, QLINK_VIFID_RSVD,
+ QLINK_CMD_PTA_PARAM,
+ sizeof(*cmd));
+ if (!cmd_skb)
+ return -ENOMEM;
+
+ cmd = (struct qlink_cmd_pta_param *)cmd_skb->data;
+ cmd->pta_param_id = param_id;
+ if (set_op) {
+ cmd->set_op = 1;
+ cmd->pta_param_value = cpu_to_le32(*param_val);
+ } else {
+ cmd->set_op = 0;
+ }
+
+ qtnf_bus_lock(mac->bus);
+ ret = qtnf_cmd_send_with_reply(mac->bus, cmd_skb, &resp_skb,
+ &res_code, sizeof(*resp), NULL);
+ qtnf_bus_unlock(mac->bus);
+
+ if (ret)
+ goto out;
+
+ if (res_code != QLINK_CMD_RESULT_OK) {
+ pr_err("cmd exec failed: 0x%x\n", res_code);
+ ret = -EFAULT;
+ goto out;
+ }
+
+ resp = (const struct qlink_resp_pta_param *)resp_skb->data;
+
+ *param_val = le32_to_cpu(resp->pta_param_value);
+out:
+ consume_skb(resp_skb);
+ return ret;
+}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.h b/drivers/net/wireless/quantenna/qtnfmac/commands.h
index 03a57e37a665..59e86db5e982 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.h
@@ -77,5 +77,6 @@ int qtnf_cmd_start_cac(const struct qtnf_vif *vif,
int qtnf_cmd_set_mac_acl(const struct qtnf_vif *vif,
const struct cfg80211_acl_data *params);
int qtnf_cmd_send_pm_set(const struct qtnf_vif *vif, u8 pm_mode, int timeout);
-
+int qtnf_cmd_setget_pta_param(struct qtnf_wmac *mac,
+ int param_id, bool set_op, int *param_val);
#endif /* QLINK_COMMANDS_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 8fbef67fbbb8..582d82ceaeef 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -258,6 +258,7 @@ enum qlink_cmd_type {
QLINK_CMD_CONNECT = 0x0060,
QLINK_CMD_DISCONNECT = 0x0061,
QLINK_CMD_PM_SET = 0x0062,
+ QLINK_CMD_PTA_PARAM = 0x0064,
};

/**
@@ -694,6 +695,52 @@ struct qlink_cmd_pm_set {
u8 pm_mode;
} __packed;

+/**
+ * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
+ *
+ * @QLINK_PTA_MODE_DISABLED: PTA is disabled
+ * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
+ */
+enum qlink_pta_mode {
+ QLINK_PTA_MODE_DISABLED = 0,
+ QLINK_PTA_MODE_2_WIRE = 2
+};
+
+/**
+ * enum qlink_pta_param - parameters to configure Packet Traffic Arbiter block
+ *
+ * @QLINK_PTA_PARAM_MODE: PTA operating mode. Supported parameter
+ * values are defined by &enum qlink_pta_mode
+ * @QLINK_PTA_PARAM_REQ_POL: request polarity, (0 or 1).
+ * @QLINK_PTA_PARAM_GNT_POL: grant polarity, (0 or 1).
+ * @QLINK_PTA_PARAM_REQ_TIMEOUT: request timeout, ms.
+ * @QLINK_PTA_PARAM_GNT_TIMEOUT: grant timeout, ms.
+ * @QLINK_PTA_PARAM_IFS_TIMEOUT: IFS timeout, ms.
+ */
+enum qlink_pta_param {
+ QLINK_PTA_PARAM_MODE = 0,
+ QLINK_PTA_PARAM_REQ_POL,
+ QLINK_PTA_PARAM_GNT_POL,
+ QLINK_PTA_PARAM_REQ_TIMEOUT,
+ QLINK_PTA_PARAM_GNT_TIMEOUT,
+ QLINK_PTA_PARAM_IFS_TIMEOUT
+};
+
+/**
+ * struct qlink_cmd_pta_param - data for QLINK_CMD_PTA_PARAM command
+ *
+ * @pta_param_id: identifies PTA parameter, one of &enum qlink_pta_param
+ * @set_op: set or get operation flag (0 - get, 1 - set)
+ * @pta_param_value: value of PTA parameter to set
+ */
+struct qlink_cmd_pta_param {
+ struct qlink_cmd chdr;
+ u8 pta_param_id;
+ u8 set_op;
+ u8 rsvd[2];
+ __le32 pta_param_value;
+} __packed;
+
/* QLINK Command Responses messages related definitions
*/

@@ -895,6 +942,19 @@ struct qlink_resp_channel_get {
struct qlink_chandef chan;
} __packed;

+/**
+ * struct qlink_resp_pta_param - response for QLINK_CMD_PTA_PARAM command
+ *
+ * @pta_param_id: identifies PTA parameter, one of &enum qlink_pta_param
+ * @pta_param_value: value of PTA parameter
+ */
+struct qlink_resp_pta_param {
+ struct qlink_resp rhdr;
+ u8 pta_param_id;
+ u8 rsvd[3];
+ __le32 pta_param_value;
+} __packed;
+
/* QLINK Events messages related definitions
*/

diff --git a/drivers/net/wireless/quantenna/qtnfmac/vendor.c b/drivers/net/wireless/quantenna/qtnfmac/vendor.c
new file mode 100644
index 000000000000..dbd290632e8b
--- /dev/null
+++ b/drivers/net/wireless/quantenna/qtnfmac/vendor.c
@@ -0,0 +1,145 @@
+/*
+ * Copyright (c) 2018 Quantenna Communications, Inc.
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+#include <linux/kernel.h>
+#include <net/cfg80211.h>
+#include <net/netlink.h>
+#include "qlink.h"
+#include "commands.h"
+#include "vendor.h"
+
+static int
+qtnf_vendor_parse_pta_param_id(const void *data, int len, int *param_id)
+{
+ struct nlattr *attr;
+
+ attr = nla_find((const struct nlattr *)data, len,
+ QTNF_VENDOR_ATTR_PTA_PARAM_ID);
+ if (!attr) {
+ pr_err("attribute does not exist\n");
+ return -ENOENT;
+ }
+
+ if (nla_len(attr) != sizeof(u8)) {
+ pr_err("invalid attribute type\n");
+ return -EINVAL;
+ }
+
+ *param_id = nla_get_u8(attr);
+
+ return 0;
+}
+
+static int
+qtnf_vendor_parse_pta_param_val(const void *data, int len, int *param_val)
+{
+ struct nlattr *attr;
+
+ attr = nla_find((const struct nlattr *)data, len,
+ QTNF_VENDOR_ATTR_PTA_PARAM_VAL);
+ if (!attr) {
+ pr_err("attribute does not exist\n");
+ return -EINVAL;
+ }
+
+ if (nla_len(attr) != sizeof(s32)) {
+ pr_err("invalid attribute type\n");
+ return -EINVAL;
+ }
+
+ *param_val = nla_get_s32(attr);
+
+ return 0;
+}
+
+static int
+qtnf_vendor_cmd_set_pta_param(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ const void *data, int data_len)
+{
+ struct qtnf_wmac *mac = wiphy_priv(wiphy);
+ int param_id;
+ int param_val;
+ int ret;
+
+ ret = qtnf_vendor_parse_pta_param_id(data, data_len, &param_id);
+ if (ret)
+ return ret;
+
+ ret = qtnf_vendor_parse_pta_param_val(data, data_len, &param_val);
+ if (ret)
+ return ret;
+
+ ret = qtnf_cmd_setget_pta_param(mac, param_id, true, &param_val);
+ return ret;
+}
+
+static int
+qtnf_vendor_cmd_get_pta_param(struct wiphy *wiphy,
+ struct wireless_dev *wdev,
+ const void *data, int data_len)
+{
+ struct qtnf_wmac *mac = wiphy_priv(wiphy);
+ struct sk_buff *reply;
+ int param_id;
+ int param_val;
+ int approx_size;
+ int ret;
+
+ ret = qtnf_vendor_parse_pta_param_id(data, data_len, &param_id);
+ if (ret)
+ return ret;
+
+ ret = qtnf_cmd_setget_pta_param(mac, param_id, false, &param_val);
+ if (ret)
+ return ret;
+
+ approx_size = nla_total_size(sizeof(s32));
+ reply = cfg80211_vendor_cmd_alloc_reply_skb(wiphy, approx_size);
+
+ if (!reply)
+ return -ENOMEM;
+
+ nla_put_u8(reply, QTNF_VENDOR_ATTR_PTA_PARAM_ID, param_id);
+ nla_put_s32(reply, QTNF_VENDOR_ATTR_PTA_PARAM_VAL, param_val);
+
+ ret = cfg80211_vendor_cmd_reply(reply);
+
+ return ret;
+}
+
+const struct wiphy_vendor_command qtnf_vendor_cmds[] = {
+ {
+ {
+ .vendor_id = QTNF_OUI,
+ .subcmd = QTNF_VENDOR_CMD_SET_PTA_PARAM
+ },
+ .flags = 0,
+ .doit = qtnf_vendor_cmd_set_pta_param
+ },
+ {
+ {
+ .vendor_id = QTNF_OUI,
+ .subcmd = QTNF_VENDOR_CMD_GET_PTA_PARAM
+ },
+ .flags = 0,
+ .doit = qtnf_vendor_cmd_get_pta_param
+ },
+};
+
+int qtnf_vendor_get_cmds_number(void)
+{
+ return ARRAY_SIZE(qtnf_vendor_cmds);
+}
diff --git a/drivers/net/wireless/quantenna/qtnfmac/vendor.h b/drivers/net/wireless/quantenna/qtnfmac/vendor.h
new file mode 100644
index 000000000000..d1bb662f8eb4
--- /dev/null
+++ b/drivers/net/wireless/quantenna/qtnfmac/vendor.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2018 Quantenna Communications, Inc.
+ * All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ */
+
+#ifndef QTNFMAC_VENDOR_H
+#define QTNFMAC_VENDOR_H
+
+#define QTNF_OUI 0x002686
+
+enum qtnf_vendor_cmd {
+ QTNF_VENDOR_CMD_SET_PTA_PARAM = 0x1,
+ QTNF_VENDOR_CMD_GET_PTA_PARAM = 0x2,
+};
+
+enum qtnf_vendor_attr_pta_param {
+ QTNF_VENDOR_ATTR_PTA_PARAM_ID = 1,
+ QTNF_VENDOR_ATTR_PTA_PARAM_VAL = 2
+};
+
+extern const struct wiphy_vendor_command qtnf_vendor_cmds[];
+int qtnf_vendor_get_cmds_number(void);
+
+#endif /* QTNFMAC_VENDOR_H */
--
2.11.0

2018-06-04 18:58:34

by Igor Mitsyanko

[permalink] [raw]
Subject: Re: [PATCH 3/6] qtnfmac: implement cfg80211 power management callback

On 06/04/2018 05:50 AM, Sergey Matyukevich wrote:
>> I think it's better be moved somewhere out of qtnf_wiphy_allocate() as OPS
>> are global for all MACs, while qtnf_wiphy_allocate() is called for each MAC
>> separately.
>
> Consider the case of splitting hardware capabilities into two groups:
> global and per-wmac. In this case we may need to register different
> subsets of cfg80211 operations for each wmac. In such a case, function
> qtnf_wiphy_allocate looks like a reasonable point where to merge both
> capability groups and customize cfg80211_ops structure for each wmac.
>


What I mean is that wiphy_new() will not dup cfg80211_ops that we pass,
it will just assign a pointer to whatever it gets.
qtn_cfg80211_ops is global right now, if we modify it, it will affect
all WMACs, not just the one that we're allocating.

Currently it's not a problem as we do not have any per-WMAC
capabilities. If we to have any, we would have to dup qtn_cfg80211_ops
in qtnf_wiphy_allocate().

Though I agree that we already have QLINK_HW_CAPAB_DFS_OFFLOAD
processing in qtnf_wiphy_allocate(), so it makes sense to do a cleanup
separately.

2018-06-01 01:11:49

by Igor Mitsyanko

[permalink] [raw]
Subject: Re: [PATCH 3/6] qtnfmac: implement cfg80211 power management callback

On 05/31/2018 02:10 AM, Sergey Matyukevich wrote:
> From: Sergei Maksimenko <[email protected]>
>
> Implement set_power_mgmt() callback that forwards power saving
> settings to the device firmware.
>
> Signed-off-by: Sergei Maksimenko <[email protected]>
> ---
> static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy_in,
> @@ -922,6 +939,9 @@ struct wiphy *qtnf_wiphy_allocate(struct qtnf_bus *bus)
> if (bus->hw_info.hw_capab & QLINK_HW_CAPAB_DFS_OFFLOAD)
> qtn_cfg80211_ops.start_radar_detection = NULL;
>
> + if (!(bus->hw_info.hw_capab & QLINK_HW_CAPAB_PWR_MGMT))
> + qtn_cfg80211_ops.set_power_mgmt = NULL;
> +

I think it's better be moved somewhere out of qtnf_wiphy_allocate() as
OPS are global for all MACs, while qtnf_wiphy_allocate() is called for
each MAC separately.

2018-06-01 01:11:48

by Igor Mitsyanko

[permalink] [raw]
Subject: Re: [PATCH 0/6] qtnfmac: enable more features

On 05/31/2018 02:10 AM, Sergey Matyukevich wrote:
> Hello Kalle and all,
>
> Here is the next update for qtnfmac driver which enables more wireless
> features. The major changes include the following items:
> - cfg80211 power management callback
> - net_device_ops set MAC addr callback
> - advanced scan features: multiple SSIDs and randomized source MACs
> - basic WoWLAN support: disconnect/magic/pattern triggers
> -- not yet: PCIe backend suspend/resume, reporting wakeup reason
>
> Regards,
> Sergey

All patches except for mine
Reviewed-by: Igor Mitsyanko <[email protected]>

2018-06-04 12:50:34

by Sergey Matyukevich

[permalink] [raw]
Subject: Re: [PATCH 3/6] qtnfmac: implement cfg80211 power management callback

Hello Igor,

> > Implement set_power_mgmt() callback that forwards power saving
> > settings to the device firmware.
> >
> > Signed-off-by: Sergei Maksimenko <[email protected]>
> > ---
> > static void qtnf_cfg80211_reg_notifier(struct wiphy *wiphy_in,
> > @@ -922,6 +939,9 @@ struct wiphy *qtnf_wiphy_allocate(struct qtnf_bus *bus)
> > if (bus->hw_info.hw_capab & QLINK_HW_CAPAB_DFS_OFFLOAD)
> > qtn_cfg80211_ops.start_radar_detection = NULL;
> > + if (!(bus->hw_info.hw_capab & QLINK_HW_CAPAB_PWR_MGMT))
> > + qtn_cfg80211_ops.set_power_mgmt = NULL;
> > +
>
> I think it's better be moved somewhere out of qtnf_wiphy_allocate() as OPS
> are global for all MACs, while qtnf_wiphy_allocate() is called for each MAC
> separately.

Consider the case of splitting hardware capabilities into two groups:
global and per-wmac. In this case we may need to register different
subsets of cfg80211 operations for each wmac. In such a case, function
qtnf_wiphy_allocate looks like a reasonable point where to merge both
capability groups and customize cfg80211_ops structure for each wmac.

Regards,
Sergey

2018-06-27 15:54:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [1/6] qtnfmac: implement net_device_ops callback to set MAC address

Sergey Matyukevich <[email protected]> wrote:

> From: Igor Mitsyanko <[email protected]>
>
> Implement net_device_ops::ndo_set_mac_address callback to allow for
> setting interface MAC address. Implementation is done through existing
> CHANGE_INTF firmware command. All validation is to be done by firmware.
>
> Signed-off-by: Igor Mitsyanko <[email protected]>

2 patches applied to wireless-drivers-next.git, thanks.

ed9f34bb9d33 qtnfmac: implement net_device_ops callback to set MAC address
6fbef9540af0 qtnfmac: enable source MAC address randomization support

--
https://patchwork.kernel.org/patch/10440557/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2018-07-30 15:48:54

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 6/6] qtnfmac: implement basic WoWLAN support

Sergey Matyukevich <[email protected]> wrote:

> This patch implements core WoWLAN support in qtnfmac driver, including
> processing of WoWLAN features reported by firmware and implementation
> of cfg80211 suspend/resume/wakeup callbacks. Currently the following
> WoWLAN triggers are supported: disconnect, magic packet,
> custom pattern packet.
>
> Signed-off-by: Sergey Matyukevich <[email protected]>

Doesn't apply anymore as I dropped patch 5

fatal: sha1 information is lacking or useless (drivers/net/wireless/quantenna/qtnfmac/cfg80211.c).
error: could not build fake ancestor
Applying: qtnfmac: implement basic WoWLAN support
Patch failed at 0001 qtnfmac: implement basic WoWLAN support
The copy of the patch that failed is found in: .git/rebase-apply/patch

Patch set to Changes Requested.

--
https://patchwork.kernel.org/patch/10440567/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2018-07-30 15:30:12

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/6] qtnfmac: implement cfg80211 power management callback

Sergey Matyukevich <[email protected]> writes:

> From: Sergei Maksimenko <[email protected]>
>
> Implement set_power_mgmt() callback that forwards power saving
> settings to the device firmware.
>
> Signed-off-by: Sergei Maksimenko <[email protected]>

[...]

> @@ -922,6 +939,9 @@ struct wiphy *qtnf_wiphy_allocate(struct qtnf_bus *bus)
> if (bus->hw_info.hw_capab & QLINK_HW_CAPAB_DFS_OFFLOAD)
> qtn_cfg80211_ops.start_radar_detection = NULL;
>
> + if (!(bus->hw_info.hw_capab & QLINK_HW_CAPAB_PWR_MGMT))
> + qtn_cfg80211_ops.set_power_mgmt = NULL;

Igor already commented about this but AFAICS this breaks if there are
more than one device on the host. Upstream drivers should not have any
limit for the number of devices they support.

--
Kalle Valo

2018-07-31 13:22:10

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: add support for PTA configuration

On 7/30/2018 4:06 PM, Kalle Valo wrote:
> Sergey Matyukevich <[email protected]> writes:
>
>> From: Andrey Shevchenko <[email protected]>
>>
>> Implement support for PTA (Packet Traffic Arbitration) configuration.
>> The PTA mechanism is used to coordinate sharing of the medium between
>> WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
>>
>> This patch implements core infrastructure and vendor specific commands
>> to control PTA functionality in firmware.
>
> And no description of the actual interface which would have helped with
> the review.
>
> Anyway, the vendor commands are pain and they just make me grumpy. The
> original idea was that upstream drivers should not support them at all,
> later we flexed the rules so that low level hardware specific interfaces
> might be ok, for example we added one in wil6210.
>
> If I would even consider applying a patch which adds a vendor command it
> needs a really good commit log with a proper description of the actual
> interface and good justifications why a generic nl80211 command won't
> work. I don't see anything even remotely close here.
>
> Sorry for being grumpy, I just hate these vendor commands. Especially
> when I see that a generic nl80211 command has not even be consired at
> all.

For what it is worth, looking at part of the patch:

+/**
+ * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
+ *
+ * @QLINK_PTA_MODE_DISABLED: PTA is disabled
+ * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
+ */
+enum qlink_pta_mode {
+ QLINK_PTA_MODE_DISABLED = 0,
+ QLINK_PTA_MODE_2_WIRE = 2
+};
+

it smells very much like low-level btcoex. The question is whether this
needs to be conveyed from user-space or should these be device
configuration, eg. like DT properties.

Regards,
Arend

2018-07-31 11:39:23

by Sergey Matyukevich

[permalink] [raw]
Subject: Re: [PATCH 6/6] qtnfmac: implement basic WoWLAN support


Hello Kalle,

> > This patch implements core WoWLAN support in qtnfmac driver, including
> > processing of WoWLAN features reported by firmware and implementation
> > of cfg80211 suspend/resume/wakeup callbacks. Currently the following
> > WoWLAN triggers are supported: disconnect, magic packet,
> > custom pattern packet.
> >
> > Signed-off-by: Sergey Matyukevich <[email protected]>
>
> Doesn't apply anymore as I dropped patch 5
>
> fatal: sha1 information is lacking or useless (drivers/net/wireless/quantenna/qtnfmac/cfg80211.c).
> error: could not build fake ancestor
> Applying: qtnfmac: implement basic WoWLAN support
> Patch failed at 0001 qtnfmac: implement basic WoWLAN support
> The copy of the patch that failed is found in: .git/rebase-apply/patch
>
> Patch set to Changes Requested.

Thanks for review. I will rebase the remaining patches on top of your
up-to-date tree and resubmit.

As for the patch with vendor specific commands, I will split it into
a separate discussion and make another attempt to convince you, adding
proper description. IIRC there is no generic functionality suitable
for this particular usecase.

Regards,
Sergey

2018-07-30 15:41:55

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: add support for PTA configuration

Sergey Matyukevich <[email protected]> writes:

> From: Andrey Shevchenko <[email protected]>
>
> Implement support for PTA (Packet Traffic Arbitration) configuration.
> The PTA mechanism is used to coordinate sharing of the medium between
> WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
>
> This patch implements core infrastructure and vendor specific commands
> to control PTA functionality in firmware.

And no description of the actual interface which would have helped with
the review.

Anyway, the vendor commands are pain and they just make me grumpy. The
original idea was that upstream drivers should not support them at all,
later we flexed the rules so that low level hardware specific interfaces
might be ok, for example we added one in wil6210.

If I would even consider applying a patch which adds a vendor command it
needs a really good commit log with a proper description of the actual
interface and good justifications why a generic nl80211 command won't
work. I don't see anything even remotely close here.

Sorry for being grumpy, I just hate these vendor commands. Especially
when I see that a generic nl80211 command has not even be consired at
all.

--
Kalle Valo

2018-07-30 15:48:07

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 3/6] qtnfmac: implement cfg80211 power management callback

Sergey Matyukevich <[email protected]> wrote:

> From: Sergei Maksimenko <[email protected]>
>
> Implement set_power_mgmt() callback that forwards power saving
> settings to the device firmware.
>
> Signed-off-by: Sergei Maksimenko <[email protected]>

2 patches applied to wireless-drivers-next.git, thanks.

4775ad06b56a qtnfmac: implement cfg80211 power management callback
8f1180e08ed4 qtnfmac: enable multiple SSIDs scan support

--
https://patchwork.kernel.org/patch/10440561/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2018-08-01 10:10:19

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: add support for PTA configuration

Arend van Spriel <[email protected]> writes:

> On 7/30/2018 4:06 PM, Kalle Valo wrote:
>> Sergey Matyukevich <[email protected]> writes:
>>
>>> From: Andrey Shevchenko <[email protected]>
>>>
>>> Implement support for PTA (Packet Traffic Arbitration) configuration.
>>> The PTA mechanism is used to coordinate sharing of the medium between
>>> WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
>>>
>>> This patch implements core infrastructure and vendor specific commands
>>> to control PTA functionality in firmware.
>>
>> And no description of the actual interface which would have helped with
>> the review.
>>
>> Anyway, the vendor commands are pain and they just make me grumpy. The
>> original idea was that upstream drivers should not support them at all,
>> later we flexed the rules so that low level hardware specific interfaces
>> might be ok, for example we added one in wil6210.
>>
>> If I would even consider applying a patch which adds a vendor command it
>> needs a really good commit log with a proper description of the actual
>> interface and good justifications why a generic nl80211 command won't
>> work. I don't see anything even remotely close here.
>>
>> Sorry for being grumpy, I just hate these vendor commands. Especially
>> when I see that a generic nl80211 command has not even be consired at
>> all.
>
> For what it is worth, looking at part of the patch:
>
> +/**
> + * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
> + *
> + * @QLINK_PTA_MODE_DISABLED: PTA is disabled
> + * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
> + */
> +enum qlink_pta_mode {
> + QLINK_PTA_MODE_DISABLED = 0,
> + QLINK_PTA_MODE_2_WIRE = 2
> +};
> +
>
> it smells very much like low-level btcoex. The question is whether
> this needs to be conveyed from user-space or should these be device
> configuration, eg. like DT properties.

That's a very good point.

--
Kalle Valo

2018-08-05 00:22:08

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: add support for PTA configuration

On 8/1/2018 10:23 AM, Sergey Matyukevich wrote:
>>>> Implement support for PTA (Packet Traffic Arbitration) configuration.
>>>> The PTA mechanism is used to coordinate sharing of the medium between
>>>> WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
>>>>
>>>> This patch implements core infrastructure and vendor specific commands
>>>> to control PTA functionality in firmware.
>>>
>>> And no description of the actual interface which would have helped with
>>> the review.
>>>
>>> Anyway, the vendor commands are pain and they just make me grumpy. The
>>> original idea was that upstream drivers should not support them at all,
>>> later we flexed the rules so that low level hardware specific interfaces
>>> might be ok, for example we added one in wil6210.
>>>
>>> If I would even consider applying a patch which adds a vendor command it
>>> needs a really good commit log with a proper description of the actual
>>> interface and good justifications why a generic nl80211 command won't
>>> work. I don't see anything even remotely close here.
>>>
>>> Sorry for being grumpy, I just hate these vendor commands. Especially
>>> when I see that a generic nl80211 command has not even be consired at
>>> all.
>>
>> For what it is worth, looking at part of the patch:
>>
>> +/**
>> + * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
>> + *
>> + * @QLINK_PTA_MODE_DISABLED: PTA is disabled
>> + * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
>> + */
>> +enum qlink_pta_mode {
>> + QLINK_PTA_MODE_DISABLED = 0,
>> + QLINK_PTA_MODE_2_WIRE = 2
>> +};
>> +
>>
>> it smells very much like low-level btcoex. The question is whether this
>> needs to be conveyed from user-space or should these be device
>> configuration, eg. like DT properties.
>
> Hello Arend,
>
> Yes, this is some kind of low-level BT coexistence mechanism, when WiFi and
> BT cards coordinate access to wireless media in 2.4G using gpio lines.
> Those lines connect WiFi and BT cards directly w/o host mediation.

Right.

> As for DT properties, the qustion still remains the same: how to propagate
> those settings to WiFi card. AFAIK there is no 'standard' interface for
> this kind of things. So using vendor commands looked like the only option.

So DT properties are available to the kernel so it is just between
device driver and wifi card. There is no involvement with user-space needed.

Regards,
Arend

2018-08-05 17:27:40

by Sergey Matyukevich

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: add support for PTA configuration

> > > > > Implement support for PTA (Packet Traffic Arbitration) configuration.
> > > > > The PTA mechanism is used to coordinate sharing of the medium between
> > > > > WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
> > > > >
> > > > > This patch implements core infrastructure and vendor specific commands
> > > > > to control PTA functionality in firmware.
> > > >
> > > > And no description of the actual interface which would have helped with
> > > > the review.
> > > >
> > > > Anyway, the vendor commands are pain and they just make me grumpy. The
> > > > original idea was that upstream drivers should not support them at all,
> > > > later we flexed the rules so that low level hardware specific interfaces
> > > > might be ok, for example we added one in wil6210.
> > > >
> > > > If I would even consider applying a patch which adds a vendor command it
> > > > needs a really good commit log with a proper description of the actual
> > > > interface and good justifications why a generic nl80211 command won't
> > > > work. I don't see anything even remotely close here.
> > > >
> > > > Sorry for being grumpy, I just hate these vendor commands. Especially
> > > > when I see that a generic nl80211 command has not even be consired at
> > > > all.
> > >
> > > For what it is worth, looking at part of the patch:
> > >
> > > +/**
> > > + * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
> > > + *
> > > + * @QLINK_PTA_MODE_DISABLED: PTA is disabled
> > > + * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
> > > + */
> > > +enum qlink_pta_mode {
> > > + QLINK_PTA_MODE_DISABLED = 0,
> > > + QLINK_PTA_MODE_2_WIRE = 2
> > > +};
> > > +
> > >
> > > it smells very much like low-level btcoex. The question is whether this
> > > needs to be conveyed from user-space or should these be device
> > > configuration, eg. like DT properties.
> >
> > Hello Arend,
> >
> > Yes, this is some kind of low-level BT coexistence mechanism, when WiFi and
> > BT cards coordinate access to wireless media in 2.4G using gpio lines.
> > Those lines connect WiFi and BT cards directly w/o host mediation.
>
> Right.
>
> > As for DT properties, the qustion still remains the same: how to propagate
> > those settings to WiFi card. AFAIK there is no 'standard' interface for
> > this kind of things. So using vendor commands looked like the only option.
>
> So DT properties are available to the kernel so it is just between
> device driver and wifi card. There is no involvement with user-space needed.

Ok, makes sense. But IIUC this approach with DT
does not cover PCIe/USB wireless cards.

Regards,
Sergey

2018-08-01 10:23:36

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 6/6] qtnfmac: implement basic WoWLAN support

Sergey Matyukevich <[email protected]> writes:

>> > This patch implements core WoWLAN support in qtnfmac driver, including
>> > processing of WoWLAN features reported by firmware and implementation
>> > of cfg80211 suspend/resume/wakeup callbacks. Currently the following
>> > WoWLAN triggers are supported: disconnect, magic packet,
>> > custom pattern packet.
>> >
>> > Signed-off-by: Sergey Matyukevich <[email protected]>
>>
>> Doesn't apply anymore as I dropped patch 5
>>
>> fatal: sha1 information is lacking or useless
>> (drivers/net/wireless/quantenna/qtnfmac/cfg80211.c).
>> error: could not build fake ancestor
>> Applying: qtnfmac: implement basic WoWLAN support
>> Patch failed at 0001 qtnfmac: implement basic WoWLAN support
>> The copy of the patch that failed is found in: .git/rebase-apply/patch
>>
>> Patch set to Changes Requested.
>
> Thanks for review. I will rebase the remaining patches on top of your
> up-to-date tree and resubmit.
>
> As for the patch with vendor specific commands, I will split it into
> a separate discussion and make another attempt to convince you, adding
> proper description.

Thanks, please do that. And always try to submit controversial patches
separately, not within a bigger patchset.

> IIRC there is no generic functionality suitable for this particular
> usecase.

The bigger problem here is that what should we do with the vendor
commands, the current way of things (=some low level hardware related
commands are accepted) does not work as it takes too much of my time.
People always try to take the easy way out so they provide minimal
documentation for vendors commands and don't even considering having a
generic nl80211 interface, which needs from me quite significant effort
to make the decision if the vendor interface is acceptable or not.

Personally I would just reject all vendor commands but on the otherhand
I somewhat understand why in some cases they might be good. Dunno.

--
Kalle Valo

2018-08-01 10:07:52

by Sergey Matyukevich

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: add support for PTA configuration

> > > Implement support for PTA (Packet Traffic Arbitration) configuration.
> > > The PTA mechanism is used to coordinate sharing of the medium between
> > > WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
> > >
> > > This patch implements core infrastructure and vendor specific commands
> > > to control PTA functionality in firmware.
> >
> > And no description of the actual interface which would have helped with
> > the review.
> >
> > Anyway, the vendor commands are pain and they just make me grumpy. The
> > original idea was that upstream drivers should not support them at all,
> > later we flexed the rules so that low level hardware specific interfaces
> > might be ok, for example we added one in wil6210.
> >
> > If I would even consider applying a patch which adds a vendor command it
> > needs a really good commit log with a proper description of the actual
> > interface and good justifications why a generic nl80211 command won't
> > work. I don't see anything even remotely close here.
> >
> > Sorry for being grumpy, I just hate these vendor commands. Especially
> > when I see that a generic nl80211 command has not even be consired at
> > all.
>
> For what it is worth, looking at part of the patch:
>
> +/**
> + * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
> + *
> + * @QLINK_PTA_MODE_DISABLED: PTA is disabled
> + * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
> + */
> +enum qlink_pta_mode {
> + QLINK_PTA_MODE_DISABLED = 0,
> + QLINK_PTA_MODE_2_WIRE = 2
> +};
> +
>
> it smells very much like low-level btcoex. The question is whether this
> needs to be conveyed from user-space or should these be device
> configuration, eg. like DT properties.

Hello Arend,

Yes, this is some kind of low-level BT coexistence mechanism, when WiFi and
BT cards coordinate access to wireless media in 2.4G using gpio lines.
Those lines connect WiFi and BT cards directly w/o host mediation.

As for DT properties, the qustion still remains the same: how to propagate
those settings to WiFi card. AFAIK there is no 'standard' interface for
this kind of things. So using vendor commands looked like the only option.

Regards,
Sergey

2018-08-05 23:40:11

by Arend van Spriel

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: add support for PTA configuration

On 8/5/2018 5:22 PM, Sergey Matyukevich wrote:
>>>>>> Implement support for PTA (Packet Traffic Arbitration) configuration.
>>>>>> The PTA mechanism is used to coordinate sharing of the medium between
>>>>>> WiFi and other 2.4 wireless networks, e.g. Bluetooth or ZigBee.
>>>>>>
>>>>>> This patch implements core infrastructure and vendor specific commands
>>>>>> to control PTA functionality in firmware.
>>>>>
>>>>> And no description of the actual interface which would have helped with
>>>>> the review.
>>>>>
>>>>> Anyway, the vendor commands are pain and they just make me grumpy. The
>>>>> original idea was that upstream drivers should not support them at all,
>>>>> later we flexed the rules so that low level hardware specific interfaces
>>>>> might be ok, for example we added one in wil6210.
>>>>>
>>>>> If I would even consider applying a patch which adds a vendor command it
>>>>> needs a really good commit log with a proper description of the actual
>>>>> interface and good justifications why a generic nl80211 command won't
>>>>> work. I don't see anything even remotely close here.
>>>>>
>>>>> Sorry for being grumpy, I just hate these vendor commands. Especially
>>>>> when I see that a generic nl80211 command has not even be consired at
>>>>> all.
>>>>
>>>> For what it is worth, looking at part of the patch:
>>>>
>>>> +/**
>>>> + * enum qlink_pta_mode - Packet Traffic Arbiter operating modes
>>>> + *
>>>> + * @QLINK_PTA_MODE_DISABLED: PTA is disabled
>>>> + * @QLINK_PTA_MODE_2_WIRE: enable PTA 2-wire mode
>>>> + */
>>>> +enum qlink_pta_mode {
>>>> + QLINK_PTA_MODE_DISABLED = 0,
>>>> + QLINK_PTA_MODE_2_WIRE = 2
>>>> +};
>>>> +
>>>>
>>>> it smells very much like low-level btcoex. The question is whether this
>>>> needs to be conveyed from user-space or should these be device
>>>> configuration, eg. like DT properties.
>>>
>>> Hello Arend,
>>>
>>> Yes, this is some kind of low-level BT coexistence mechanism, when WiFi and
>>> BT cards coordinate access to wireless media in 2.4G using gpio lines.
>>> Those lines connect WiFi and BT cards directly w/o host mediation.
>>
>> Right.
>>
>>> As for DT properties, the qustion still remains the same: how to propagate
>>> those settings to WiFi card. AFAIK there is no 'standard' interface for
>>> this kind of things. So using vendor commands looked like the only option.
>>
>> So DT properties are available to the kernel so it is just between
>> device driver and wifi card. There is no involvement with user-space needed.
>
> Ok, makes sense. But IIUC this approach with DT
> does not cover PCIe/USB wireless cards.

It can cover any device in the system regardless type of host interface.
For example in the ath10k bindings document [1] you see:

pci {
pcie@0 {
reg = <0 0 0 0 0>;
#interrupt-cells = <1>;
#size-cells = <2>;
#address-cells = <3>;
device_type = "pci";

ath10k@0,0 {
reg = <0 0 0 0 0>;
device_type = "pci";
qcom,ath10k-calibration-data = [ 01 02 03 ... ];
};
};
};

Regards,
Arend

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/tree/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt?h=linux-4.17.y