2018-02-05 15:05:32

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 0/6] qtnfmac: qsr10g pcie backend updates

Hello Kalle and all,

Here is a patch set with various fixes and enhacements for qsr10g
PCIe backend driver. The major changes include the following two
items:
- make rmmod/insmod work properly for qtnfmac_pcie driver
- provide configuration knobs to control NSM (networked standby mode)

NSM (networked standby mode) is not related to WiFi standard. This is
a term that has been introduced by European Commission in regulation
No. 1275/2008. Simplifying things a bit, that regulation specifies
ecodesing requirements for various operating modes. In that document
‘networked standby’ is defined as a condition in which the equipment
is able to resume a function by way of a remotely initiated trigger
from a network connection.

Regards,
Sergey

Sergei Maksimenko (3)
qtnfmac: implement asynchronous firmware loading
qtnfmac: enable reloading of qtnfmac kernel modules
qtnfmac: enable networked standby mode on device

Sergey Matyukevich (3)
qtnfmac: fix releasing Tx/Rx data buffers
qtnfmac: fix rmmod for firmware version mismatch
qtnfmac: fix rmmod for missing firmware


bus.h | 4
commands.c | 33 +++
commands.h | 1
core.c | 77 +++++++++
core.h | 2
pearl/pcie.c | 407 +++++++++++++++++++++++++-----------------------
pearl/pcie_ipc.h | 1
pearl/pcie_regs_pearl.h | 1
qlink.h | 30 +++
9 files changed, 363 insertions(+), 193 deletions(-)


2018-02-07 11:09:48

by Sergei Maksimenko

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: implement asynchronous firmware loading

Hi Arend,

Qsr10g devices support loading firmware not only from the host over PCIe bu=
t also from on-board flash.
Firmware start-up sequence and the following handshake are common for PCIe =
and flash boot modes. They take much more
time (about 10 seconds) than the firmware upload process.
To make flash mode startup also asynchronous we can't use the existing API =
such as request_firmware_nowait() as there is
no firmware to upload, and have to explicitly run the needed functions in a=
work queue.
In my opinion it is more neat to have one firmware load function invoked in=
a work queue for both modes than using
request_firmware_nowait() for one mode only.

Regards,
Sergei

On 06.02.2018 14:22, Arend van Spriel wrote:
>
> External Email
>
>
> On 2/5/2018 4:05 PM, Sergey Matyukevich wrote:
>> From: Sergei Maksimenko <[email protected]>
>>
>> In pci probe() function start firmware loading, protocol handshake
>> and driver core initialization, and not wait for completion.
>
> The moving of the debugfs stuff makes this a drag to review, but I get
> the gist. The thing is that the firmware api already provides an
> asynchronous api, ie. request_firmware_nowait(), so why not use that.
>
> Regards,
> Arend



This email, including its contents and any attachment(s), may contain confi=
dential information of Quantenna Communications, Inc. and is solely for the=
intended recipient(s). If you may have received this in error, please cont=
act the sender and permanently delete this email, its contents and any atta=
chment(s).

2018-02-05 15:05:37

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 1/6] qtnfmac: fix releasing Tx/Rx data buffers

Add missing PCI unmap for Tx buffers and release all buffers explicitly.
Managed release using devm_add_action is not suitable for qtnfmac Tx/Rx
data buffers. The reason is in ordering and dependencies: buffers
should be released after transmission is stopped but before PCI
device resources and DMA allocations are released.

Signed-off-by: Sergey Matyukevich <[email protected]>
---
.../net/wireless/quantenna/qtnfmac/pearl/pcie.c | 30 +++++++++++++---------
1 file changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index 6f6190964320..be5813aa1486 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -478,10 +478,11 @@ static int alloc_rx_buffers(struct qtnf_pcie_bus_priv *priv)
}

/* all rx/tx activity should have ceased before calling this function */
-static void free_xfer_buffers(void *data)
+static void qtnf_free_xfer_buffers(struct qtnf_pcie_bus_priv *priv)
{
- struct qtnf_pcie_bus_priv *priv = (struct qtnf_pcie_bus_priv *)data;
+ struct qtnf_tx_bd *txbd;
struct qtnf_rx_bd *rxbd;
+ struct sk_buff *skb;
dma_addr_t paddr;
int i;

@@ -489,19 +490,26 @@ static void free_xfer_buffers(void *data)
for (i = 0; i < priv->rx_bd_num; i++) {
if (priv->rx_skb && priv->rx_skb[i]) {
rxbd = &priv->rx_bd_vbase[i];
+ skb = priv->rx_skb[i];
paddr = QTN_HOST_ADDR(le32_to_cpu(rxbd->addr_h),
le32_to_cpu(rxbd->addr));
pci_unmap_single(priv->pdev, paddr, SKB_BUF_SIZE,
PCI_DMA_FROMDEVICE);
-
- dev_kfree_skb_any(priv->rx_skb[i]);
+ dev_kfree_skb_any(skb);
+ priv->rx_skb[i] = NULL;
}
}

/* free tx buffers */
for (i = 0; i < priv->tx_bd_num; i++) {
if (priv->tx_skb && priv->tx_skb[i]) {
- dev_kfree_skb_any(priv->tx_skb[i]);
+ txbd = &priv->tx_bd_vbase[i];
+ skb = priv->tx_skb[i];
+ paddr = QTN_HOST_ADDR(le32_to_cpu(txbd->addr_h),
+ le32_to_cpu(txbd->addr));
+ pci_unmap_single(priv->pdev, paddr, skb->len,
+ PCI_DMA_TODEVICE);
+ dev_kfree_skb_any(skb);
priv->tx_skb[i] = NULL;
}
}
@@ -1321,12 +1329,6 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto err_base;
}

- ret = devm_add_action(&pdev->dev, free_xfer_buffers, (void *)pcie_priv);
- if (ret) {
- pr_err("custom release callback init failed\n");
- goto err_base;
- }
-
ret = qtnf_pcie_init_xfer(pcie_priv);
if (ret) {
pr_err("PCIE xfer init failed\n");
@@ -1343,7 +1345,7 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
"qtnf_pcie_irq", (void *)bus);
if (ret) {
pr_err("failed to request pcie irq %d\n", pdev->irq);
- goto err_base;
+ goto err_xfer;
}

tasklet_init(&pcie_priv->reclaim_tq, qtnf_reclaim_tasklet_fn,
@@ -1387,6 +1389,9 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
err_bringup_fw:
netif_napi_del(&bus->mux_napi);

+err_xfer:
+ qtnf_free_xfer_buffers(pcie_priv);
+
err_base:
flush_workqueue(pcie_priv->workqueue);
destroy_workqueue(pcie_priv->workqueue);
@@ -1416,6 +1421,7 @@ static void qtnf_pcie_remove(struct pci_dev *pdev)
destroy_workqueue(priv->workqueue);
tasklet_kill(&priv->reclaim_tq);

+ qtnf_free_xfer_buffers(priv);
qtnf_debugfs_remove(bus);

qtnf_pcie_free_shm_ipc(priv);
--
2.11.0

2018-02-06 11:22:56

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: implement asynchronous firmware loading

On 2/5/2018 4:05 PM, Sergey Matyukevich wrote:
> From: Sergei Maksimenko <[email protected]>
>
> In pci probe() function start firmware loading, protocol handshake
> and driver core initialization, and not wait for completion.

The moving of the debugfs stuff makes this a drag to review, but I get
the gist. The thing is that the firmware api already provides an
asynchronous api, ie. request_firmware_nowait(), so why not use that.

Regards,
Arend

2018-02-27 14:38:08

by Sergey Matyukevich

[permalink] [raw]
Subject: Re: [PATCH 0/6] qtnfmac: qsr10g pcie backend updates

Hello Kalle,

> BTW, I don't see this patchset on patchwork.kernel.org[1], strangely I
> only see v2. I don't know if it's because of problems on patchwork or
> the submitter deleted the patches, but in case someone did delete these
> please do not that. I want to see the old versions from patchwork to
> check the old discussions etc.
>
> [1] https://patchwork.kernel.org/project/linux-wireless/list/?state=*&q=qtnfmac

I guess that was me. After the first review round we reworked the patch set.
So I posted v2 and marked v1 as 'Superseded' in Patchwork.

Regards,
Sergey

2018-02-05 15:05:40

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 2/6] qtnfmac: enable reloading of qtnfmac kernel modules

From: Sergei Maksimenko <[email protected]>

This patch enables rmmod/insmod for qtnfmac kernel modules:
- do not 'pin' pci device in order to disable it on module unload
- implement card reset procedure
- restore PCI bar addresses for restarted wireless card

Signed-off-by: Sergei Maksimenko <[email protected]>
Signed-off-by: Sergey Matyukevich <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 15 ++++++++++++++-
drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h | 1 +
.../wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h | 1 +
3 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index be5813aa1486..7aa222286d8e 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -162,6 +162,17 @@ static void qtnf_deassert_intx(struct qtnf_pcie_bus_priv *priv)
qtnf_non_posted_write(cfg, reg);
}

+static void qtnf_reset_card(struct qtnf_pcie_bus_priv *priv)
+{
+ const u32 data = QTN_PEARL_IPC_IRQ_WORD(QTN_PEARL_LHOST_EP_RESET);
+ void __iomem *reg = priv->sysctl_bar +
+ QTN_PEARL_SYSCTL_LHOST_IRQ_OFFSET;
+
+ qtnf_non_posted_write(data, reg);
+ msleep(QTN_EP_RESET_WAIT_MS);
+ pci_restore_state(priv->pdev);
+}
+
static void qtnf_ipc_gen_ep_int(void *arg)
{
const struct qtnf_pcie_bus_priv *priv = arg;
@@ -1308,7 +1319,6 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto err_base;
}

- pcim_pin_device(pdev);
pci_set_master(pdev);

ret = qtnf_pcie_init_irq(pcie_priv);
@@ -1323,6 +1333,8 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto err_base;
}

+ pci_save_state(pdev);
+
ret = qtnf_pcie_init_shm_ipc(pcie_priv);
if (ret < 0) {
pr_err("PCIE SHM IPC init failed\n");
@@ -1425,6 +1437,7 @@ static void qtnf_pcie_remove(struct pci_dev *pdev)
qtnf_debugfs_remove(bus);

qtnf_pcie_free_shm_ipc(priv);
+ qtnf_reset_card(priv);
}

#ifdef CONFIG_PM_SLEEP
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h
index c5a4e46d26ef..00bb21a1c47a 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_ipc.h
@@ -46,6 +46,7 @@
/* state transition timeouts */
#define QTN_FW_DL_TIMEOUT_MS 3000
#define QTN_FW_QLINK_TIMEOUT_MS 30000
+#define QTN_EP_RESET_WAIT_MS 1000

#define PCIE_HDP_INT_RX_BITS (0 \
| PCIE_HDP_INT_EP_TXDMA \
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h
index 5b48b425fa7f..0bfe285b6b48 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie_regs_pearl.h
@@ -351,5 +351,6 @@

#define QTN_PEARL_IPC_IRQ_WORD(irq) (BIT(irq) | BIT(irq + 16))
#define QTN_PEARL_LHOST_IPC_IRQ (6)
+#define QTN_PEARL_LHOST_EP_RESET (7)

#endif /* __PEARL_PCIE_H */
--
2.11.0

2018-02-08 09:06:18

by Sergey Matyukevich

[permalink] [raw]
Subject: Re: [PATCH 4/6] qtnfmac: fix rmmod for missing firmware

Hello Arend,

Thanks for review.

> > Check that firmware exists prior to starting firmware download.
>
> Why would you do that? It seems expensive given that you obtain the
> firmware and discard it immediately just to check it exists. Especially,
> given that such a call can take 60 seconds to complete depending on
> kernel config.
>
> Apart from that see minor comment below although I would seriously
> reconsider this patch altogether.

The idea behind this approach is simple: to quit early and to avoid starting
asynchronous card boot if no firmware file exists. However I didn't realize that
such a long delay may occur. What makes me worried is that the worst case
scenario may happen if firmware actually exists: we make two calls of
request_firmware, each of them taking long time.

I agree that it makes a lot of sense to reimplement this approach requesting
firmware only once. Will do for v2.

Regards,
Sergey

2018-02-05 15:06:07

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 6/6] qtnfmac: enable networked standby mode on device inactivity

From: Sergei Maksimenko <[email protected]>

Enable support of networked standby mode (NSM) on qsr10g devices.
Networked standby is a power saving mode when the device keeps
all existing network connections and returns to full power mode
on a network activity. When enabled, device enters standby mode
after 15 min of inactivity (no associated stations or no trattic).
This period can be changed by setting sysfs attribute standby_timeout
(0 disables NSM support). A module parameter auto_standby
(defaults to 1) controls enabling NSM support on module loading.

Signed-off-by: Sergei Maksimenko <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/bus.h | 1 +
drivers/net/wireless/quantenna/qtnfmac/commands.c | 33 ++++++++++
drivers/net/wireless/quantenna/qtnfmac/commands.h | 1 +
drivers/net/wireless/quantenna/qtnfmac/core.c | 77 +++++++++++++++++++++++
drivers/net/wireless/quantenna/qtnfmac/core.h | 2 +
drivers/net/wireless/quantenna/qtnfmac/qlink.h | 30 +++++++++
6 files changed, 144 insertions(+)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/bus.h b/drivers/net/wireless/quantenna/qtnfmac/bus.h
index 0a1604683bab..7a27ffc6c7a7 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/bus.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/bus.h
@@ -65,6 +65,7 @@ struct qtnf_bus {
struct work_struct event_work;
struct mutex bus_lock; /* lock during command/event processing */
struct dentry *dbg_dir;
+ u32 standby_timeout;
/* bus private data */
char bus_priv[0] __aligned(sizeof(void *));
};
diff --git a/drivers/net/wireless/quantenna/qtnfmac/commands.c b/drivers/net/wireless/quantenna/qtnfmac/commands.c
index deca0060eb27..1e730c9fa371 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.c
@@ -2753,3 +2753,36 @@ int qtnf_cmd_set_mac_acl(const struct qtnf_vif *vif,

return ret;
}
+
+int qtnf_cmd_send_pm_set(struct qtnf_bus *bus, u8 pm_mode, u32 timeout)
+{
+ 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(QLINK_MACID_RSVD, QLINK_VIFID_RSVD,
+ 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 69a7d56f7e58..a06e6a96c35d 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/commands.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/commands.h
@@ -81,5 +81,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(struct qtnf_bus *bus, u8 pm_mode, u32 timeout);

#endif /* QLINK_COMMANDS_H_ */
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c b/drivers/net/wireless/quantenna/qtnfmac/core.c
index cf26c15a84f8..10c4e3ea2404 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -26,6 +26,10 @@
#include "event.h"
#include "util.h"

+static bool auto_standby = true;
+module_param(auto_standby, bool, 0644);
+MODULE_PARM_DESC(auto_standby, "set to 0 to disable auto standby mode");
+
#define QTNF_DMP_MAX_LEN 48
#define QTNF_PRIMARY_VIF_IDX 0

@@ -552,6 +556,53 @@ static int qtnf_core_mac_attach(struct qtnf_bus *bus, unsigned int macid)
return ret;
}

+static ssize_t qtnf_pm_standby_timeout_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct qtnf_bus *bus = dev_get_drvdata(dev);
+
+ sprintf(buf, "%u\n", bus->standby_timeout);
+ return strlen(buf);
+}
+
+static ssize_t qtnf_pm_standby_timeout_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *buf, size_t count)
+{
+ struct qtnf_bus *bus = dev_get_drvdata(dev);
+ int timeout;
+
+ if (count < 1)
+ goto out;
+
+ if (kstrtoint(buf, 0, &timeout))
+ goto out;
+
+ if (timeout < 0)
+ timeout = 0;
+ else if (timeout > S32_MAX)
+ timeout = S32_MAX;
+
+ if (timeout == bus->standby_timeout)
+ goto out;
+
+ if (timeout) {
+ if (!qtnf_cmd_send_pm_set(bus, QLINK_PM_AUTO_STANDBY,
+ timeout))
+ bus->standby_timeout = timeout;
+ } else {
+ if (!qtnf_cmd_send_pm_set(bus, QLINK_PM_OFF, 0))
+ bus->standby_timeout = 0;
+ }
+
+out:
+ return (ssize_t)count;
+}
+
+static DEVICE_ATTR(standby_timeout, 0644, qtnf_pm_standby_timeout_show,
+ qtnf_pm_standby_timeout_store);
+
int qtnf_core_attach(struct qtnf_bus *bus)
{
unsigned int i;
@@ -608,6 +659,25 @@ int qtnf_core_attach(struct qtnf_bus *bus)
}
}

+ if (auto_standby) {
+ bus->standby_timeout = QTNF_DEFAULT_STANDBY_TIMER;
+ ret = qtnf_cmd_send_pm_set(bus, QLINK_PM_AUTO_STANDBY,
+ bus->standby_timeout);
+ if (ret)
+ bus->standby_timeout = 0;
+ } else {
+ bus->standby_timeout = 0;
+ ret = qtnf_cmd_send_pm_set(bus, QLINK_PM_OFF, 0);
+ }
+
+ if (ret) {
+ pr_err("failed to init PM auto standby: %d\n", ret);
+ /* Do not cancel init when PM mode not configured */
+ }
+
+ if (device_create_file(bus->dev, &dev_attr_standby_timeout))
+ pr_err("failed to init sysfs standby control file: %d\n", ret);
+
return 0;

error:
@@ -620,6 +690,13 @@ EXPORT_SYMBOL_GPL(qtnf_core_attach);
void qtnf_core_detach(struct qtnf_bus *bus)
{
unsigned int macid;
+ int ret;
+
+ device_remove_file(bus->dev, &dev_attr_standby_timeout);
+
+ ret = qtnf_cmd_send_pm_set(bus, QLINK_PM_OFF, 0);
+ if (ret)
+ pr_err("failed to deinit NSM: %d\n", ret);

qtnf_bus_data_rx_stop(bus);

diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.h b/drivers/net/wireless/quantenna/qtnfmac/core.h
index 3b884c80b6ab..9386f09e1fab 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.h
@@ -52,6 +52,8 @@
#define QTNF_DEF_WDOG_TIMEOUT 5
#define QTNF_TX_TIMEOUT_TRSHLD 100

+#define QTNF_DEFAULT_STANDBY_TIMER (15 * 60)
+
extern const struct net_device_ops qtnf_netdev_ops;

struct qtnf_bus;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/qlink.h b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
index 9bf3ae4d1b3b..ee3c65ca7148 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/qlink.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/qlink.h
@@ -251,6 +251,7 @@ enum qlink_cmd_type {
QLINK_CMD_CHAN_STATS = 0x0054,
QLINK_CMD_CONNECT = 0x0060,
QLINK_CMD_DISCONNECT = 0x0061,
+ QLINK_CMD_PM_SET = 0x0062,
};

/**
@@ -663,6 +664,35 @@ 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: Automatic Network Standby Mode - when there is no
+ * traffic for the certain period, device enters power saving mode without
+ * disconnecting peers. Device will wake up automatically on a new
+ * association or data frames to TX/RX. Standby mode is activated on each
+ * radio interface individually, based on traffic on it. When all the
+ * radios enter standby mode, device informs RC via MSI.
+ */
+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 standby 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-02-05 15:05:42

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 3/6] qtnfmac: fix rmmod for firmware version mismatch

Modify qtnf_pcie_probe error paths to fix rmmod of qtnfmac kernel
modules in the case when there is a version mismatch between
firmware and driver.

Signed-off-by: Sergey Matyukevich <[email protected]>
---
.../net/wireless/quantenna/qtnfmac/pearl/pcie.c | 44 ++++++++++------------
1 file changed, 20 insertions(+), 24 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index 7aa222286d8e..c0d1c5d94ef0 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -127,7 +127,7 @@ static inline void qtnf_dis_txdone_irq(struct qtnf_pcie_bus_priv *priv)
spin_unlock_irqrestore(&priv->irq_lock, flags);
}

-static int qtnf_pcie_init_irq(struct qtnf_pcie_bus_priv *priv)
+static void qtnf_pcie_init_irq(struct qtnf_pcie_bus_priv *priv)
{
struct pci_dev *pdev = priv->pdev;

@@ -148,8 +148,6 @@ static int qtnf_pcie_init_irq(struct qtnf_pcie_bus_priv *priv)
pr_warn("legacy PCIE interrupts enabled\n");
pci_intx(pdev, 1);
}
-
- return 0;
}

static void qtnf_deassert_intx(struct qtnf_pcie_bus_priv *priv)
@@ -1256,10 +1254,8 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)

bus = devm_kzalloc(&pdev->dev,
sizeof(*bus) + sizeof(*pcie_priv), GFP_KERNEL);
- if (!bus) {
- ret = -ENOMEM;
- goto err_init;
- }
+ if (!bus)
+ return -ENOMEM;

pcie_priv = get_bus_priv(bus);

@@ -1286,11 +1282,14 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
pcie_priv->tx_reclaim_done = 0;
pcie_priv->tx_reclaim_req = 0;

+ tasklet_init(&pcie_priv->reclaim_tq, qtnf_reclaim_tasklet_fn,
+ (unsigned long)pcie_priv);
+
pcie_priv->workqueue = create_singlethread_workqueue("QTNF_PEARL_PCIE");
if (!pcie_priv->workqueue) {
pr_err("failed to alloc bus workqueue\n");
ret = -ENODEV;
- goto err_priv;
+ goto err_init;
}

if (!pci_is_pcie(pdev)) {
@@ -1320,12 +1319,7 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
}

pci_set_master(pdev);
-
- ret = qtnf_pcie_init_irq(pcie_priv);
- if (ret < 0) {
- pr_err("irq init failed\n");
- goto err_base;
- }
+ qtnf_pcie_init_irq(pcie_priv);

ret = qtnf_pcie_init_memory(pcie_priv);
if (ret < 0) {
@@ -1344,7 +1338,7 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
ret = qtnf_pcie_init_xfer(pcie_priv);
if (ret) {
pr_err("PCIE xfer init failed\n");
- goto err_base;
+ goto err_ipc;
}

/* init default irq settings */
@@ -1360,33 +1354,31 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto err_xfer;
}

- tasklet_init(&pcie_priv->reclaim_tq, qtnf_reclaim_tasklet_fn,
- (unsigned long)pcie_priv);
init_dummy_netdev(&bus->mux_dev);
netif_napi_add(&bus->mux_dev, &bus->mux_napi,
qtnf_rx_poll, 10);

ret = qtnf_bringup_fw(bus);
if (ret < 0)
- goto err_bringup_fw;
+ goto err_fw;
else if (ret)
wait_for_completion(&bus->request_firmware_complete);

if (bus->fw_state != QTNF_FW_STATE_FW_DNLD_DONE) {
pr_err("failed to start FW\n");
- goto err_bringup_fw;
+ goto err_fw;
}

if (qtnf_poll_state(&pcie_priv->bda->bda_ep_state, QTN_EP_FW_QLINK_DONE,
QTN_FW_QLINK_TIMEOUT_MS)) {
pr_err("FW runtime failure\n");
- goto err_bringup_fw;
+ goto err_fw;
}

ret = qtnf_core_attach(bus);
if (ret) {
pr_err("failed to attach core\n");
- goto err_bringup_fw;
+ goto err_fw;
}

qtnf_debugfs_init(bus, DRV_NAME);
@@ -1398,20 +1390,24 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)

return 0;

-err_bringup_fw:
+err_fw:
+ qtnf_reset_card(pcie_priv);
netif_napi_del(&bus->mux_napi);

err_xfer:
qtnf_free_xfer_buffers(pcie_priv);

+err_ipc:
+ qtnf_pcie_free_shm_ipc(pcie_priv);
+
err_base:
flush_workqueue(pcie_priv->workqueue);
destroy_workqueue(pcie_priv->workqueue);

-err_priv:
+err_init:
+ tasklet_kill(&pcie_priv->reclaim_tq);
pci_set_drvdata(pdev, NULL);

-err_init:
return ret;
}

--
2.11.0

2018-02-08 09:51:15

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH 5/6] qtnfmac: implement asynchronous firmware loading

On 2/7/2018 12:09 PM, Sergei Maksimenko wrote:
> Hi Arend,
>
> Qsr10g devices support loading firmware not only from the host over PCIe
> but also from on-board flash.
> Firmware start-up sequence and the following handshake are common for
> PCIe and flash boot modes. They take much more
> time (about 10 seconds) than the firmware upload process.
> To make flash mode startup also asynchronous we can't use the existing
> API such as request_firmware_nowait() as there is
> no firmware to upload, and have to explicitly run the needed functions
> in a work queue.
> In my opinion it is more neat to have one firmware load function invoked
> in a work queue for both modes than using
> request_firmware_nowait() for one mode only.

I was taught "neat" is not a real argument ;-) I agree that if you need
the worker for async flashing it makes sense to use that for async
firmware loading as well. Missed that reading the patch itself.

Regards,
Arend

> Regards,
> Sergei
>
> On 06.02.2018 14:22, Arend van Spriel wrote:
>>
>> External Email
>>
>>
>> On 2/5/2018 4:05 PM, Sergey Matyukevich wrote:
>>> From: Sergei Maksimenko <[email protected]>
>>>
>>> In pci probe() function start firmware loading, protocol handshake
>>> and driver core initialization, and not wait for completion.
>>
>> The moving of the debugfs stuff makes this a drag to review, but I get
>> the gist. The thing is that the firmware api already provides an
>> asynchronous api, ie. request_firmware_nowait(), so why not use that.
>>
>> Regards,
>> Arend
>
>
>
> This email, including its contents and any attachment(s), may contain
> confidential information of Quantenna Communications, Inc. and is solely
> for the intended recipient(s). If you may have received this in error,
> please contact the sender and permanently delete this email, its
> contents and any attachment(s).

2018-02-06 11:18:48

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH 4/6] qtnfmac: fix rmmod for missing firmware

On 2/5/2018 4:05 PM, Sergey Matyukevich wrote:
> Check that firmware exists prior to starting firmware download.

Why would you do that? It seems expensive given that you obtain the
firmware and discard it immediately just to check it exists. Especially,
given that such a call can take 60 seconds to complete depending on
kernel config.

Apart from that see minor comment below although I would seriously
reconsider this patch altogether.

> Signed-off-by: Sergey Matyukevich <[email protected]>
> ---
> drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
> index c0d1c5d94ef0..86368e345276 100644
> --- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
> +++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
> @@ -1104,6 +1104,20 @@ static void qtnf_firmware_load(const struct firmware *fw, void *context)
> complete(&bus->request_firmware_complete);
> }
>
> +static int qtnf_fw_exists(struct qtnf_bus *bus)
> +{
> + struct qtnf_pcie_bus_priv *priv = (void *)get_bus_priv(bus);
> + struct pci_dev *pdev = priv->pdev;
> + const struct firmware *fw;

I think it is better to initialize fw to NULL here and ...

> + int ret;
> +
> + ret = request_firmware(&fw, bus->fwname, &pdev->dev);
> + if (!ret)

do 'if (fw)' here instead.

Regards,
Arend

> + release_firmware(fw);
> +
> + return !ret;
> +}
> +

2018-02-05 15:05:46

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 4/6] qtnfmac: fix rmmod for missing firmware

Check that firmware exists prior to starting firmware download.

Signed-off-by: Sergey Matyukevich <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index c0d1c5d94ef0..86368e345276 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -1104,6 +1104,20 @@ static void qtnf_firmware_load(const struct firmware *fw, void *context)
complete(&bus->request_firmware_complete);
}

+static int qtnf_fw_exists(struct qtnf_bus *bus)
+{
+ struct qtnf_pcie_bus_priv *priv = (void *)get_bus_priv(bus);
+ struct pci_dev *pdev = priv->pdev;
+ const struct firmware *fw;
+ int ret;
+
+ ret = request_firmware(&fw, bus->fwname, &pdev->dev);
+ if (!ret)
+ release_firmware(fw);
+
+ return !ret;
+}
+
static int qtnf_bringup_fw(struct qtnf_bus *bus)
{
struct qtnf_pcie_bus_priv *priv = (void *)get_bus_priv(bus);
@@ -1358,6 +1372,12 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
netif_napi_add(&bus->mux_dev, &bus->mux_napi,
qtnf_rx_poll, 10);

+ if (!flashboot && !qtnf_fw_exists(bus)) {
+ pr_err("failed to get firmware %s\n", bus->fwname);
+ ret = -ENOENT;
+ goto err_fw;
+ }
+
ret = qtnf_bringup_fw(bus);
if (ret < 0)
goto err_fw;
--
2.11.0

2018-02-27 14:28:43

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/6] qtnfmac: qsr10g pcie backend updates

Sergey Matyukevich <[email protected]> writes:

> Hello Kalle and all,
>
> Here is a patch set with various fixes and enhacements for qsr10g
> PCIe backend driver. The major changes include the following two
> items:
> - make rmmod/insmod work properly for qtnfmac_pcie driver
> - provide configuration knobs to control NSM (networked standby mode)
>
> NSM (networked standby mode) is not related to WiFi standard. This is
> a term that has been introduced by European Commission in regulation
> No. 1275/2008. Simplifying things a bit, that regulation specifies
> ecodesing requirements for various operating modes. In that document
> =E2=80=98networked standby=E2=80=99 is defined as a condition in which th=
e equipment
> is able to resume a function by way of a remotely initiated trigger
> from a network connection.
>
> Regards,
> Sergey
>
> Sergei Maksimenko (3)
> qtnfmac: implement asynchronous firmware loading
> qtnfmac: enable reloading of qtnfmac kernel modules
> qtnfmac: enable networked standby mode on device
>
> Sergey Matyukevich (3)
> qtnfmac: fix releasing Tx/Rx data buffers
> qtnfmac: fix rmmod for firmware version mismatch
> qtnfmac: fix rmmod for missing firmware

BTW, I don't see this patchset on patchwork.kernel.org[1], strangely I
only see v2. I don't know if it's because of problems on patchwork or
the submitter deleted the patches, but in case someone did delete these
please do not that. I want to see the old versions from patchwork to
check the old discussions etc.

[1] https://patchwork.kernel.org/project/linux-wireless/list/?state=3D*&q=
=3Dqtnfmac

--=20
Kalle Valo

2018-02-08 09:21:07

by Arend Van Spriel

[permalink] [raw]
Subject: Re: [PATCH 4/6] qtnfmac: fix rmmod for missing firmware

On 2/8/2018 10:06 AM, Sergey Matyukevich wrote:
> Hello Arend,
>
> Thanks for review.
>
>>> Check that firmware exists prior to starting firmware download.
>>
>> Why would you do that? It seems expensive given that you obtain the
>> firmware and discard it immediately just to check it exists. Especially,
>> given that such a call can take 60 seconds to complete depending on
>> kernel config.
>>
>> Apart from that see minor comment below although I would seriously
>> reconsider this patch altogether.
>
> The idea behind this approach is simple: to quit early and to avoid starting
> asynchronous card boot if no firmware file exists. However I didn't realize that
> such a long delay may occur. What makes me worried is that the worst case
> scenario may happen if firmware actually exists: we make two calls of
> request_firmware, each of them taking long time.

The delay I mentioned here is when using FW_LOADER_USER_HELPER_FALLBACK
option is selected and user-space is not handling the firmware request.
This may happen when the driver is built-in and the user-space
application is not yet started.

> I agree that it makes a lot of sense to reimplement this approach requesting
> firmware only once. Will do for v2.

Regards,
Arend

2018-02-05 15:05:57

by Sergey Matyukevich

[permalink] [raw]
Subject: [PATCH 5/6] qtnfmac: implement asynchronous firmware loading

From: Sergei Maksimenko <[email protected]>

In pci probe() function start firmware loading, protocol handshake
and driver core initialization, and not wait for completion.

Signed-off-by: Sergei Maksimenko <[email protected]>
---
drivers/net/wireless/quantenna/qtnfmac/bus.h | 3 +-
.../net/wireless/quantenna/qtnfmac/pearl/pcie.c | 326 ++++++++++-----------
2 files changed, 159 insertions(+), 170 deletions(-)

diff --git a/drivers/net/wireless/quantenna/qtnfmac/bus.h b/drivers/net/wireless/quantenna/qtnfmac/bus.h
index 56e5fed92a2a..0a1604683bab 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/bus.h
+++ b/drivers/net/wireless/quantenna/qtnfmac/bus.h
@@ -59,8 +59,9 @@ struct qtnf_bus {
char fwname[32];
struct napi_struct mux_napi;
struct net_device mux_dev;
- struct completion request_firmware_complete;
+ struct completion firmware_init_complete;
struct workqueue_struct *workqueue;
+ struct work_struct fw_work;
struct work_struct event_work;
struct mutex bus_lock; /* lock during command/event processing */
struct dentry *dbg_dir;
diff --git a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
index 86368e345276..3be5a79cbca0 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/pearl/pcie.c
@@ -954,6 +954,98 @@ static const struct qtnf_bus_ops qtnf_pcie_bus_ops = {
.data_rx_stop = qtnf_pcie_data_rx_stop,
};

+static int qtnf_dbg_mps_show(struct seq_file *s, void *data)
+{
+ struct qtnf_bus *bus = dev_get_drvdata(s->private);
+ struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
+
+ seq_printf(s, "%d\n", priv->mps);
+
+ return 0;
+}
+
+static int qtnf_dbg_msi_show(struct seq_file *s, void *data)
+{
+ struct qtnf_bus *bus = dev_get_drvdata(s->private);
+ struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
+
+ seq_printf(s, "%u\n", priv->msi_enabled);
+
+ return 0;
+}
+
+static int qtnf_dbg_irq_stats(struct seq_file *s, void *data)
+{
+ struct qtnf_bus *bus = dev_get_drvdata(s->private);
+ struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
+ u32 reg = readl(PCIE_HDP_INT_EN(priv->pcie_reg_base));
+ u32 status;
+
+ seq_printf(s, "pcie_irq_count(%u)\n", priv->pcie_irq_count);
+ seq_printf(s, "pcie_irq_tx_count(%u)\n", priv->pcie_irq_tx_count);
+ status = reg & PCIE_HDP_INT_TX_BITS;
+ seq_printf(s, "pcie_irq_tx_status(%s)\n",
+ (status == PCIE_HDP_INT_TX_BITS) ? "EN" : "DIS");
+ seq_printf(s, "pcie_irq_rx_count(%u)\n", priv->pcie_irq_rx_count);
+ status = reg & PCIE_HDP_INT_RX_BITS;
+ seq_printf(s, "pcie_irq_rx_status(%s)\n",
+ (status == PCIE_HDP_INT_RX_BITS) ? "EN" : "DIS");
+ seq_printf(s, "pcie_irq_uf_count(%u)\n", priv->pcie_irq_uf_count);
+ status = reg & PCIE_HDP_INT_HHBM_UF;
+ seq_printf(s, "pcie_irq_hhbm_uf_status(%s)\n",
+ (status == PCIE_HDP_INT_HHBM_UF) ? "EN" : "DIS");
+
+ return 0;
+}
+
+static int qtnf_dbg_hdp_stats(struct seq_file *s, void *data)
+{
+ struct qtnf_bus *bus = dev_get_drvdata(s->private);
+ struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
+
+ seq_printf(s, "tx_full_count(%u)\n", priv->tx_full_count);
+ seq_printf(s, "tx_done_count(%u)\n", priv->tx_done_count);
+ seq_printf(s, "tx_reclaim_done(%u)\n", priv->tx_reclaim_done);
+ seq_printf(s, "tx_reclaim_req(%u)\n", priv->tx_reclaim_req);
+
+ seq_printf(s, "tx_bd_r_index(%u)\n", priv->tx_bd_r_index);
+ seq_printf(s, "tx_bd_p_index(%u)\n",
+ readl(PCIE_HDP_RX0DMA_CNT(priv->pcie_reg_base))
+ & (priv->tx_bd_num - 1));
+ seq_printf(s, "tx_bd_w_index(%u)\n", priv->tx_bd_w_index);
+ seq_printf(s, "tx queue len(%u)\n",
+ CIRC_CNT(priv->tx_bd_w_index, priv->tx_bd_r_index,
+ priv->tx_bd_num));
+
+ seq_printf(s, "rx_bd_r_index(%u)\n", priv->rx_bd_r_index);
+ seq_printf(s, "rx_bd_p_index(%u)\n",
+ readl(PCIE_HDP_TX0DMA_CNT(priv->pcie_reg_base))
+ & (priv->rx_bd_num - 1));
+ seq_printf(s, "rx_bd_w_index(%u)\n", priv->rx_bd_w_index);
+ seq_printf(s, "rx alloc queue len(%u)\n",
+ CIRC_SPACE(priv->rx_bd_w_index, priv->rx_bd_r_index,
+ priv->rx_bd_num));
+
+ return 0;
+}
+
+static int qtnf_dbg_shm_stats(struct seq_file *s, void *data)
+{
+ struct qtnf_bus *bus = dev_get_drvdata(s->private);
+ struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
+
+ seq_printf(s, "shm_ipc_ep_in.tx_packet_count(%zu)\n",
+ priv->shm_ipc_ep_in.tx_packet_count);
+ seq_printf(s, "shm_ipc_ep_in.rx_packet_count(%zu)\n",
+ priv->shm_ipc_ep_in.rx_packet_count);
+ seq_printf(s, "shm_ipc_ep_out.tx_packet_count(%zu)\n",
+ priv->shm_ipc_ep_out.tx_timeout_count);
+ seq_printf(s, "shm_ipc_ep_out.rx_packet_count(%zu)\n",
+ priv->shm_ipc_ep_out.rx_packet_count);
+
+ return 0;
+}
+
static int qtnf_ep_fw_send(struct qtnf_pcie_bus_priv *priv, uint32_t size,
int blk, const u8 *pblk, const u8 *fw)
{
@@ -1069,41 +1161,6 @@ qtnf_ep_fw_load(struct qtnf_pcie_bus_priv *priv, const u8 *fw, u32 fw_size)
return 0;
}

-static void qtnf_firmware_load(const struct firmware *fw, void *context)
-{
- struct qtnf_pcie_bus_priv *priv = (void *)context;
- struct pci_dev *pdev = priv->pdev;
- struct qtnf_bus *bus = pci_get_drvdata(pdev);
- int ret;
-
- if (!fw) {
- pr_err("failed to get firmware %s\n", bus->fwname);
- goto fw_load_err;
- }
-
- ret = qtnf_ep_fw_load(priv, fw->data, fw->size);
- if (ret) {
- pr_err("FW upload error\n");
- goto fw_load_err;
- }
-
- if (qtnf_poll_state(&priv->bda->bda_ep_state, QTN_EP_FW_DONE,
- QTN_FW_DL_TIMEOUT_MS)) {
- pr_err("FW bringup timed out\n");
- goto fw_load_err;
- }
-
- bus->fw_state = QTNF_FW_STATE_FW_DNLD_DONE;
- pr_info("firmware is up and running\n");
-
-fw_load_err:
-
- if (fw)
- release_firmware(fw);
-
- complete(&bus->request_firmware_complete);
-}
-
static int qtnf_fw_exists(struct qtnf_bus *bus)
{
struct qtnf_pcie_bus_priv *priv = (void *)get_bus_priv(bus);
@@ -1118,10 +1175,12 @@ static int qtnf_fw_exists(struct qtnf_bus *bus)
return !ret;
}

-static int qtnf_bringup_fw(struct qtnf_bus *bus)
+static void qtnf_fw_work_handler(struct work_struct *work)
{
+ struct qtnf_bus *bus = container_of(work, struct qtnf_bus, fw_work);
struct qtnf_pcie_bus_priv *priv = (void *)get_bus_priv(bus);
struct pci_dev *pdev = priv->pdev;
+ const struct firmware *fw;
int ret;
u32 state = QTN_RC_FW_LOADRDY | QTN_RC_FW_QLINK;

@@ -1133,131 +1192,85 @@ static int qtnf_bringup_fw(struct qtnf_bus *bus)
if (qtnf_poll_state(&priv->bda->bda_ep_state, QTN_EP_FW_LOADRDY,
QTN_FW_DL_TIMEOUT_MS)) {
pr_err("card is not ready\n");
- return -ETIMEDOUT;
+ goto fw_load_fail;
}

qtnf_clear_state(&priv->bda->bda_ep_state, QTN_EP_FW_LOADRDY);

if (flashboot) {
- pr_info("Booting FW from flash\n");
-
- if (!qtnf_poll_state(&priv->bda->bda_ep_state, QTN_EP_FW_DONE,
- QTN_FW_DL_TIMEOUT_MS))
- bus->fw_state = QTNF_FW_STATE_FW_DNLD_DONE;
-
- return 0;
- }
-
- pr_info("starting firmware upload: %s\n", bus->fwname);
+ pr_info("booting firmware from flash\n");

- ret = request_firmware_nowait(THIS_MODULE, 1, bus->fwname, &pdev->dev,
- GFP_KERNEL, priv, qtnf_firmware_load);
- if (ret < 0)
- pr_err("request_firmware_nowait error %d\n", ret);
- else
- ret = 1;
-
- return ret;
-}
-
-static void qtnf_reclaim_tasklet_fn(unsigned long data)
-{
- struct qtnf_pcie_bus_priv *priv = (void *)data;
+ } else {
+ pr_info("starting firmware upload: %s\n", bus->fwname);

- qtnf_pcie_data_tx_reclaim(priv);
- qtnf_en_txdone_irq(priv);
-}
+ ret = request_firmware(&fw, bus->fwname, &pdev->dev);
+ if (ret < 0) {
+ pr_err("failed to get firmware %s\n", bus->fwname);
+ goto fw_load_fail;
+ }

-static int qtnf_dbg_mps_show(struct seq_file *s, void *data)
-{
- struct qtnf_bus *bus = dev_get_drvdata(s->private);
- struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
+ ret = qtnf_ep_fw_load(priv, fw->data, fw->size);
+ release_firmware(fw);
+ if (ret) {
+ pr_err("firmware upload error\n");
+ goto fw_load_fail;
+ }
+ }

- seq_printf(s, "%d\n", priv->mps);
+ if (qtnf_poll_state(&priv->bda->bda_ep_state, QTN_EP_FW_DONE,
+ QTN_FW_DL_TIMEOUT_MS)) {
+ pr_err("firmware bringup timed out\n");
+ goto fw_load_fail;
+ }

- return 0;
-}
+ bus->fw_state = QTNF_FW_STATE_FW_DNLD_DONE;
+ pr_info("firmware is up and running\n");

-static int qtnf_dbg_msi_show(struct seq_file *s, void *data)
-{
- struct qtnf_bus *bus = dev_get_drvdata(s->private);
- struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
+ if (qtnf_poll_state(&priv->bda->bda_ep_state,
+ QTN_EP_FW_QLINK_DONE, QTN_FW_QLINK_TIMEOUT_MS)) {
+ pr_err("firmware runtime failure\n");
+ goto fw_load_fail;
+ }

- seq_printf(s, "%u\n", priv->msi_enabled);
+ ret = qtnf_core_attach(bus);
+ if (ret) {
+ pr_err("failed to attach core\n");
+ goto fw_load_fail;
+ }

- return 0;
-}
+ qtnf_debugfs_init(bus, DRV_NAME);
+ qtnf_debugfs_add_entry(bus, "mps", qtnf_dbg_mps_show);
+ qtnf_debugfs_add_entry(bus, "msi_enabled", qtnf_dbg_msi_show);
+ qtnf_debugfs_add_entry(bus, "hdp_stats", qtnf_dbg_hdp_stats);
+ qtnf_debugfs_add_entry(bus, "irq_stats", qtnf_dbg_irq_stats);
+ qtnf_debugfs_add_entry(bus, "shm_stats", qtnf_dbg_shm_stats);

-static int qtnf_dbg_irq_stats(struct seq_file *s, void *data)
-{
- struct qtnf_bus *bus = dev_get_drvdata(s->private);
- struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
- u32 reg = readl(PCIE_HDP_INT_EN(priv->pcie_reg_base));
- u32 status;
+ goto fw_load_exit;

- seq_printf(s, "pcie_irq_count(%u)\n", priv->pcie_irq_count);
- seq_printf(s, "pcie_irq_tx_count(%u)\n", priv->pcie_irq_tx_count);
- status = reg & PCIE_HDP_INT_TX_BITS;
- seq_printf(s, "pcie_irq_tx_status(%s)\n",
- (status == PCIE_HDP_INT_TX_BITS) ? "EN" : "DIS");
- seq_printf(s, "pcie_irq_rx_count(%u)\n", priv->pcie_irq_rx_count);
- status = reg & PCIE_HDP_INT_RX_BITS;
- seq_printf(s, "pcie_irq_rx_status(%s)\n",
- (status == PCIE_HDP_INT_RX_BITS) ? "EN" : "DIS");
- seq_printf(s, "pcie_irq_uf_count(%u)\n", priv->pcie_irq_uf_count);
- status = reg & PCIE_HDP_INT_HHBM_UF;
- seq_printf(s, "pcie_irq_hhbm_uf_status(%s)\n",
- (status == PCIE_HDP_INT_HHBM_UF) ? "EN" : "DIS");
+fw_load_fail:
+ bus->fw_state = QTNF_FW_STATE_DEAD;

- return 0;
+fw_load_exit:
+ complete(&bus->firmware_init_complete);
+ put_device(&pdev->dev);
}

-static int qtnf_dbg_hdp_stats(struct seq_file *s, void *data)
+static void qtnf_bringup_fw_async(struct qtnf_bus *bus)
{
- struct qtnf_bus *bus = dev_get_drvdata(s->private);
- struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
-
- seq_printf(s, "tx_full_count(%u)\n", priv->tx_full_count);
- seq_printf(s, "tx_done_count(%u)\n", priv->tx_done_count);
- seq_printf(s, "tx_reclaim_done(%u)\n", priv->tx_reclaim_done);
- seq_printf(s, "tx_reclaim_req(%u)\n", priv->tx_reclaim_req);
-
- seq_printf(s, "tx_bd_r_index(%u)\n", priv->tx_bd_r_index);
- seq_printf(s, "tx_bd_p_index(%u)\n",
- readl(PCIE_HDP_RX0DMA_CNT(priv->pcie_reg_base))
- & (priv->tx_bd_num - 1));
- seq_printf(s, "tx_bd_w_index(%u)\n", priv->tx_bd_w_index);
- seq_printf(s, "tx queue len(%u)\n",
- CIRC_CNT(priv->tx_bd_w_index, priv->tx_bd_r_index,
- priv->tx_bd_num));
-
- seq_printf(s, "rx_bd_r_index(%u)\n", priv->rx_bd_r_index);
- seq_printf(s, "rx_bd_p_index(%u)\n",
- readl(PCIE_HDP_TX0DMA_CNT(priv->pcie_reg_base))
- & (priv->rx_bd_num - 1));
- seq_printf(s, "rx_bd_w_index(%u)\n", priv->rx_bd_w_index);
- seq_printf(s, "rx alloc queue len(%u)\n",
- CIRC_SPACE(priv->rx_bd_w_index, priv->rx_bd_r_index,
- priv->rx_bd_num));
+ struct qtnf_pcie_bus_priv *priv = (void *)get_bus_priv(bus);
+ struct pci_dev *pdev = priv->pdev;

- return 0;
+ get_device(&pdev->dev);
+ INIT_WORK(&bus->fw_work, qtnf_fw_work_handler);
+ schedule_work(&bus->fw_work);
}

-static int qtnf_dbg_shm_stats(struct seq_file *s, void *data)
+static void qtnf_reclaim_tasklet_fn(unsigned long data)
{
- struct qtnf_bus *bus = dev_get_drvdata(s->private);
- struct qtnf_pcie_bus_priv *priv = get_bus_priv(bus);
-
- seq_printf(s, "shm_ipc_ep_in.tx_packet_count(%zu)\n",
- priv->shm_ipc_ep_in.tx_packet_count);
- seq_printf(s, "shm_ipc_ep_in.rx_packet_count(%zu)\n",
- priv->shm_ipc_ep_in.rx_packet_count);
- seq_printf(s, "shm_ipc_ep_out.tx_packet_count(%zu)\n",
- priv->shm_ipc_ep_out.tx_timeout_count);
- seq_printf(s, "shm_ipc_ep_out.rx_packet_count(%zu)\n",
- priv->shm_ipc_ep_out.rx_packet_count);
+ struct qtnf_pcie_bus_priv *priv = (void *)data;

- return 0;
+ qtnf_pcie_data_tx_reclaim(priv);
+ qtnf_en_txdone_irq(priv);
}

static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
@@ -1280,7 +1293,7 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
pcie_priv->pdev = pdev;

strcpy(bus->fwname, QTN_PCI_PEARL_FW_NAME);
- init_completion(&bus->request_firmware_complete);
+ init_completion(&bus->firmware_init_complete);
mutex_init(&bus->bus_lock);
spin_lock_init(&pcie_priv->tx0_lock);
spin_lock_init(&pcie_priv->irq_lock);
@@ -1378,35 +1391,7 @@ static int qtnf_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
goto err_fw;
}

- ret = qtnf_bringup_fw(bus);
- if (ret < 0)
- goto err_fw;
- else if (ret)
- wait_for_completion(&bus->request_firmware_complete);
-
- if (bus->fw_state != QTNF_FW_STATE_FW_DNLD_DONE) {
- pr_err("failed to start FW\n");
- goto err_fw;
- }
-
- if (qtnf_poll_state(&pcie_priv->bda->bda_ep_state, QTN_EP_FW_QLINK_DONE,
- QTN_FW_QLINK_TIMEOUT_MS)) {
- pr_err("FW runtime failure\n");
- goto err_fw;
- }
-
- ret = qtnf_core_attach(bus);
- if (ret) {
- pr_err("failed to attach core\n");
- goto err_fw;
- }
-
- qtnf_debugfs_init(bus, DRV_NAME);
- qtnf_debugfs_add_entry(bus, "mps", qtnf_dbg_mps_show);
- qtnf_debugfs_add_entry(bus, "msi_enabled", qtnf_dbg_msi_show);
- qtnf_debugfs_add_entry(bus, "hdp_stats", qtnf_dbg_hdp_stats);
- qtnf_debugfs_add_entry(bus, "irq_stats", qtnf_dbg_irq_stats);
- qtnf_debugfs_add_entry(bus, "shm_stats", qtnf_dbg_shm_stats);
+ qtnf_bringup_fw_async(bus);

return 0;

@@ -1440,11 +1425,14 @@ static void qtnf_pcie_remove(struct pci_dev *pdev)
if (!bus)
return;

+ wait_for_completion(&bus->firmware_init_complete);
+
+ if (bus->fw_state == QTNF_FW_STATE_ACTIVE)
+ qtnf_core_detach(bus);
+
priv = get_bus_priv(bus);

- qtnf_core_detach(bus);
netif_napi_del(&bus->mux_napi);
-
flush_workqueue(priv->workqueue);
destroy_workqueue(priv->workqueue);
tasklet_kill(&priv->reclaim_tq);
--
2.11.0

2018-02-27 16:11:31

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 0/6] qtnfmac: qsr10g pcie backend updates

Sergey Matyukevich <[email protected]> writes:

> Hello Kalle,
>
>> BTW, I don't see this patchset on patchwork.kernel.org[1], strangely I
>> only see v2. I don't know if it's because of problems on patchwork or
>> the submitter deleted the patches, but in case someone did delete these
>> please do not that. I want to see the old versions from patchwork to
>> check the old discussions etc.
>>
>> [1] https://patchwork.kernel.org/project/linux-wireless/list/?state=*&q=qtnfmac
>
> I guess that was me. After the first review round we reworked the patch set.
> So I posted v2 and marked v1 as 'Superseded' in Patchwork.

Odd, then I should still see them in patchwork but I don't. Oh well,
patchwork is still rough on edges so I'm not surprised. Anyway, I prefer
the patch submitters don't change the status themselves and let me do
that instead. That way it's easier for me track progress etc.

--
Kalle Valo