RE: [RFC 2/7] ath10k: Add support to process rx packet in thread
> -Original Message- > From: Felix Fietkau > Sent: Tuesday, March 23, 2021 1:16 PM > To: Ben Greear ; Brian Norris > > Cc: Johannes Berg ; Rajkumar Manoharan > ; Rakesh Pillai ; ath10k > ; linux-wireless wirel...@vger.kernel.org>; Linux Kernel ; > Kalle Valo ; David S. Miller > ; Jakub Kicinski ; > net...@vger.kernel.org; Doug Anderson ; Evan > Green > Subject: Re: [RFC 2/7] ath10k: Add support to process rx packet in thread > > > On 2021-03-23 04:01, Ben Greear wrote: > > On 3/22/21 6:20 PM, Brian Norris wrote: > >> On Mon, Mar 22, 2021 at 4:58 PM Ben Greear > wrote: > >>> On 7/22/20 6:00 AM, Felix Fietkau wrote: > >>>> On 2020-07-22 14:55, Johannes Berg wrote: > >>>>> On Wed, 2020-07-22 at 14:27 +0200, Felix Fietkau wrote: > >>>>> > >>>>>> I'm considering testing a different approach (with mt76 initially): > >>>>>> - Add a mac80211 rx function that puts processed skbs into a list > >>>>>> instead of handing them to the network stack directly. > >>>>> > >>>>> Would this be *after* all the mac80211 processing, i.e. in place of the > >>>>> rx-up-to-stack? > >>>> Yes, it would run all the rx handlers normally and then put the > >>>> resulting skbs into a list instead of calling netif_receive_skb or > >>>> napi_gro_frags. > >>> > >>> Whatever came of this? I realized I'm running Felix's patch since his > >>> mt76 > >>> driver needs it. Any chance it will go upstream? > >> > >> If you're asking about $subject (moving NAPI/RX to a thread), this > >> landed upstream recently: > >> http://git.kernel.org/linus/adbb4fb028452b1b0488a1a7b66ab856cdf20715 > >> > >> It needs a bit of coaxing to work on a WiFi driver (including: WiFi > >> drivers tend to have a different netdev for NAPI than they expose to > >> /sys/class/net/), but it's there. > >> > >> I'm not sure if people had something else in mind in the stuff you're > >> quoting though. > > > > No, I got it confused with something Felix did: > > > > https://github.com/greearb/mt76/blob/master/patches/0001-net-add- > support-for-threaded-NAPI-polling.patch > > > > Maybe the NAPI/RX to a thread thing superceded Felix's patch? > Yes, it did and it's in linux-next already. > I sent the following change to make mt76 use it: > https://github.com/nbd168/wireless/commit/1d4ff31437e5aaa999bd7a Hi Felix / Ben, In case of ath10k (snoc based targets), we have a lot of processing in the NAPI context. Even moving this to threaded NAPI is not helping much due to the load. Breaking the tasks into multiple context (with the patch series I posted) is helping in improving the throughput. With the current rx_thread based approach, the rx processing is broken into two parallel contexts 1) reaping the packets from the HW 2) processing these packets list and handing it over to mac80211 (and later to the network stack) This is the primary reason for choosing the rx thread approach. Thanks, Rakesh. > > - Felix
RE: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
> -Original Message-
> From: Bjorn Andersson
> Sent: Wednesday, March 10, 2021 10:15 PM
> To: Rakesh Pillai
> Cc: agr...@kernel.org; o...@wizery.com; mathieu.poir...@linaro.org;
> robh...@kernel.org; p.za...@pengutronix.de; si...@codeaurora.org; linux-
> arm-...@vger.kernel.org; linux-remotep...@vger.kernel.org;
> devicet...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for
> sc7280 WPSS
>
> On Wed 10 Mar 01:28 CST 2021, Rakesh Pillai wrote:
>
> > Add support for PIL loading of WPSS processor for SC7280
> > WPSS boot will be requested by the wifi driver and hence
> > disable auto-boot for WPSS. Also add a separate shutdown
> > sequence handler for WPSS.
> >
> > Signed-off-by: Rakesh Pillai
> > ---
> > drivers/remoteproc/qcom_q6v5_adsp.c | 77
> -
> > 1 file changed, 76 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
> b/drivers/remoteproc/qcom_q6v5_adsp.c
> > index e024502..dc6b91d 100644
> > --- a/drivers/remoteproc/qcom_q6v5_adsp.c
> > +++ b/drivers/remoteproc/qcom_q6v5_adsp.c
> > @@ -58,6 +58,8 @@ struct adsp_pil_data {
> > const char *ssr_name;
> > const char *sysmon_name;
> > int ssctl_id;
> > + bool is_wpss;
> > + bool auto_boot;
> >
> > const char **clk_ids;
> > int num_clks;
> > @@ -96,8 +98,54 @@ struct qcom_adsp {
> > struct qcom_rproc_glink glink_subdev;
> > struct qcom_rproc_ssr ssr_subdev;
> > struct qcom_sysmon *sysmon;
> > +
> > + int (*shutdown)(struct qcom_adsp *adsp);
> > };
> >
> > +static int qcom_wpss_shutdown(struct qcom_adsp *adsp)
> > +{
> > + unsigned long timeout;
> > + unsigned int val;
> > + int ret;
> > +
> > + regmap_write(adsp->halt_map, adsp->halt_lpass +
> LPASS_HALTREQ_REG, 1);
> > +
> > + /* Wait for halt ACK from QDSP6 */
> > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT);
> > + for (;;) {
> > + ret = regmap_read(adsp->halt_map,
> > + adsp->halt_lpass + LPASS_HALTACK_REG,
> );
> > + if (ret || val || time_after(jiffies, timeout))
> > + break;
> > +
> > + usleep_range(1000, 1100);
> > + }
> > +
> > + /* Place the WPSS processor into reset */
> > + reset_control_assert(adsp->restart);
> > + /* wait after asserting subsystem restart from AOSS */
> > + usleep_range(100, 105);
> > + /* Remove the WPSS reset */
> > + reset_control_deassert(adsp->restart);
> > +
> > + usleep_range(100, 105);
> > +
> > + regmap_write(adsp->halt_map, adsp->halt_lpass +
> LPASS_HALTREQ_REG, 0);
> > +
> > + /* Wait for halt ACK from QDSP6 */
> > + timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT);
> > + for (;;) {
> > + ret = regmap_read(adsp->halt_map,
> > + adsp->halt_lpass + LPASS_HALTACK_REG,
> );
> > + if (ret || !val || time_after(jiffies, timeout))
> > + break;
> > +
> > + usleep_range(1000, 1100);
> > + }
> > +
> > + return 0;
> > +}
> > +
> > static int qcom_adsp_shutdown(struct qcom_adsp *adsp)
> > {
> > unsigned long timeout;
> > @@ -270,7 +318,7 @@ static int adsp_stop(struct rproc *rproc)
> > if (ret == -ETIMEDOUT)
> > dev_err(adsp->dev, "timed out on wait\n");
> >
> > - ret = qcom_adsp_shutdown(adsp);
> > + ret = adsp->shutdown(adsp);
> > if (ret)
> > dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
> >
> > @@ -439,6 +487,8 @@ static int adsp_probe(struct platform_device
> *pdev)
> > dev_err(>dev, "unable to allocate remoteproc\n");
> > return -ENOMEM;
> > }
> > +
> > + rproc->auto_boot = desc->auto_boot;
> > rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
> >
> > adsp = (struct qcom_adsp *)rproc->priv;
> > @@ -447,6 +497,11 @@ static int adsp_probe(struct platform_device
> *pdev)
> > adsp->info_name = desc->sysmon_name;
> > platform_set_drvdata(pdev, adsp);
> >
> > + if (desc->is_wpss)
> > + adsp->shutdown = qcom_wpss_shutdown;
> > + else
> > + adsp->shutdown = qcom_adsp_shutdown;
> > +
>
[PATCH] arm64: dts: qcom: sc7280: Add WPSS remoteproc node
Add the WPSS remoteproc node in dts for
PIL loading.
Signed-off-by: Rakesh Pillai
---
- This change is dependent on the below patch series
1) https://lore.kernel.org/patchwork/project/lkml/list/?series=487403
2) https://lore.kernel.org/patchwork/project/lkml/list/?series=488365
---
arch/arm64/boot/dts/qcom/sc7280-idp.dts | 4 +++
arch/arm64/boot/dts/qcom/sc7280.dtsi| 47 +
2 files changed, 51 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
index 950ecb2..603f56b 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7280-idp.dts
@@ -26,6 +26,10 @@
status = "okay";
};
+_wpss {
+ status = "okay";
+};
+
{
status = "okay";
};
diff --git a/arch/arm64/boot/dts/qcom/sc7280.dtsi
b/arch/arm64/boot/dts/qcom/sc7280.dtsi
index 8af6d77..26dd466 100644
--- a/arch/arm64/boot/dts/qcom/sc7280.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280.dtsi
@@ -53,6 +53,16 @@
no-map;
reg = <0x0 0x80b0 0x0 0x10>;
};
+
+ wlan_fw_mem: memory@80c0 {
+ no-map;
+ reg = <0x0 0x80c0 0x0 0xc0>;
+ };
+
+ wpss_mem: memory@9ae0 {
+ no-map;
+ reg = <0x0 0x9ae0 0x0 0x190>;
+ };
};
cpus {
@@ -305,6 +315,43 @@
};
};
+ remoteproc_wpss: remoteproc@8a0 {
+ compatible = "qcom,sc7280-wpss-pil";
+ reg = <0 0x08a0 0 0x1>;
+
+ interrupts-extended = < GIC_SPI 587
IRQ_TYPE_EDGE_RISING>,
+ <_smp2p_in 0 0>,
+ <_smp2p_in 1 0>,
+ <_smp2p_in 2 0>,
+ <_smp2p_in 3 0>,
+ <_smp2p_in 7 0>;
+ interrupt-names = "wdog", "fatal", "ready", "handover",
+ "stop-ack", "shutdown-ack";
+
+ memory-region = <_mem>;
+
+ qcom,smem-states = <_smp2p_out 0>;
+ qcom,smem-state-names = "stop";
+
+ resets = <_reset AOSS_CC_WCSS_RESTART>;
+ reset-names = "restart";
+
+ qcom,halt-regs = <_mutex_regs 0x37000>;
+
+ status = "disabled";
+
+ glink-edge {
+ interrupts-extended = < IPCC_CLIENT_WPSS
+
IPCC_MPROC_SIGNAL_GLINK_QMP
+
IRQ_TYPE_EDGE_RISING>;
+ mboxes = < IPCC_CLIENT_WPSS
+ IPCC_MPROC_SIGNAL_GLINK_QMP>;
+
+ label = "wpss";
+ qcom,remote-pid = <13>;
+ };
+ };
+
pdc: interrupt-controller@b22 {
compatible = "qcom,sc7280-pdc", "qcom,pdc";
reg = <0 0x0b22 0 0x3>;
--
2.7.4
[PATCH 2/2] remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS
Add support for PIL loading of WPSS processor for SC7280
WPSS boot will be requested by the wifi driver and hence
disable auto-boot for WPSS. Also add a separate shutdown
sequence handler for WPSS.
Signed-off-by: Rakesh Pillai
---
drivers/remoteproc/qcom_q6v5_adsp.c | 77 -
1 file changed, 76 insertions(+), 1 deletion(-)
diff --git a/drivers/remoteproc/qcom_q6v5_adsp.c
b/drivers/remoteproc/qcom_q6v5_adsp.c
index e024502..dc6b91d 100644
--- a/drivers/remoteproc/qcom_q6v5_adsp.c
+++ b/drivers/remoteproc/qcom_q6v5_adsp.c
@@ -58,6 +58,8 @@ struct adsp_pil_data {
const char *ssr_name;
const char *sysmon_name;
int ssctl_id;
+ bool is_wpss;
+ bool auto_boot;
const char **clk_ids;
int num_clks;
@@ -96,8 +98,54 @@ struct qcom_adsp {
struct qcom_rproc_glink glink_subdev;
struct qcom_rproc_ssr ssr_subdev;
struct qcom_sysmon *sysmon;
+
+ int (*shutdown)(struct qcom_adsp *adsp);
};
+static int qcom_wpss_shutdown(struct qcom_adsp *adsp)
+{
+ unsigned long timeout;
+ unsigned int val;
+ int ret;
+
+ regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 1);
+
+ /* Wait for halt ACK from QDSP6 */
+ timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT);
+ for (;;) {
+ ret = regmap_read(adsp->halt_map,
+ adsp->halt_lpass + LPASS_HALTACK_REG, );
+ if (ret || val || time_after(jiffies, timeout))
+ break;
+
+ usleep_range(1000, 1100);
+ }
+
+ /* Place the WPSS processor into reset */
+ reset_control_assert(adsp->restart);
+ /* wait after asserting subsystem restart from AOSS */
+ usleep_range(100, 105);
+ /* Remove the WPSS reset */
+ reset_control_deassert(adsp->restart);
+
+ usleep_range(100, 105);
+
+ regmap_write(adsp->halt_map, adsp->halt_lpass + LPASS_HALTREQ_REG, 0);
+
+ /* Wait for halt ACK from QDSP6 */
+ timeout = jiffies + msecs_to_jiffies(ACK_TIMEOUT);
+ for (;;) {
+ ret = regmap_read(adsp->halt_map,
+ adsp->halt_lpass + LPASS_HALTACK_REG, );
+ if (ret || !val || time_after(jiffies, timeout))
+ break;
+
+ usleep_range(1000, 1100);
+ }
+
+ return 0;
+}
+
static int qcom_adsp_shutdown(struct qcom_adsp *adsp)
{
unsigned long timeout;
@@ -270,7 +318,7 @@ static int adsp_stop(struct rproc *rproc)
if (ret == -ETIMEDOUT)
dev_err(adsp->dev, "timed out on wait\n");
- ret = qcom_adsp_shutdown(adsp);
+ ret = adsp->shutdown(adsp);
if (ret)
dev_err(adsp->dev, "failed to shutdown: %d\n", ret);
@@ -439,6 +487,8 @@ static int adsp_probe(struct platform_device *pdev)
dev_err(>dev, "unable to allocate remoteproc\n");
return -ENOMEM;
}
+
+ rproc->auto_boot = desc->auto_boot;
rproc_coredump_set_elf_info(rproc, ELFCLASS32, EM_NONE);
adsp = (struct qcom_adsp *)rproc->priv;
@@ -447,6 +497,11 @@ static int adsp_probe(struct platform_device *pdev)
adsp->info_name = desc->sysmon_name;
platform_set_drvdata(pdev, adsp);
+ if (desc->is_wpss)
+ adsp->shutdown = qcom_wpss_shutdown;
+ else
+ adsp->shutdown = qcom_adsp_shutdown;
+
ret = adsp_alloc_memory_region(adsp);
if (ret)
goto free_rproc;
@@ -515,6 +570,8 @@ static const struct adsp_pil_data adsp_resource_init = {
.ssr_name = "lpass",
.sysmon_name = "adsp",
.ssctl_id = 0x14,
+ .is_wpss = false,
+ .auto_boot = true;
.clk_ids = (const char*[]) {
"sway_cbcr", "lpass_ahbs_aon_cbcr", "lpass_ahbm_aon_cbcr",
"qdsp6ss_xo", "qdsp6ss_sleep", "qdsp6ss_core", NULL
@@ -528,6 +585,8 @@ static const struct adsp_pil_data cdsp_resource_init = {
.ssr_name = "cdsp",
.sysmon_name = "cdsp",
.ssctl_id = 0x17,
+ .is_wpss = false,
+ .auto_boot = true;
.clk_ids = (const char*[]) {
"sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", "q6ss_master",
"q6_axim", NULL
@@ -535,7 +594,23 @@ static const struct adsp_pil_data cdsp_resource_init = {
.num_clks = 7,
};
+static const struct adsp_pil_data wpss_resource_init = {
+ .crash_reason_smem = 626,
+ .firmware_name = "wpss.mdt",
+ .ssr_name = "wpss",
+ .sysmon_name = "wpss",
+ .ssctl_id = 0x19,
+ .is_wpss = true,
+ .a
[PATCH 1/2] dt-bindings: remoteproc: qcom: Add SC7280 WPSS support
Add WPSS PIL loading support for SC7280 SoCs. Signed-off-by: Rakesh Pillai --- .../bindings/remoteproc/qcom,hexagon-v56.txt | 35 -- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt index 1337a3d..edad5e8 100644 --- a/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt +++ b/Documentation/devicetree/bindings/remoteproc/qcom,hexagon-v56.txt @@ -9,6 +9,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core. Definition: must be one of: "qcom,qcs404-cdsp-pil", "qcom,sdm845-adsp-pil" + "qcom,sc7280-wpss-pil" - reg: Usage: required @@ -24,7 +25,13 @@ on the Qualcomm Technology Inc. Hexagon v56 core. - interrupt-names: Usage: required Value type: - Definition: must be "wdog", "fatal", "ready", "handover", "stop-ack" + Definition: The interrupts needed depends on the compatible string + qcom,sdm845-adsp-pil: + qcom,qcs404-cdsp-pil: + must be "wdog", "fatal", "ready", "handover", "stop-ack" + qcom,sc7280-wpss-pil: + must be "wdog", "fatal", "ready", "handover", "stop-ack" + "shutdown-ack" - clocks: Usage: required @@ -35,19 +42,17 @@ on the Qualcomm Technology Inc. Hexagon v56 core. - clock-names: Usage: required for SDM845 ADSP Value type: - Definition: List of clock input name strings sorted in the same - order as the clocks property. Definition must have - "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr", - "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep" - and "qdsp6ss_core". - -- clock-names: - Usage: required for QCS404 CDSP - Value type: - Definition: List of clock input name strings sorted in the same - order as the clocks property. Definition must have - "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", - "q6ss_master", "q6_axim". + Definition: The clocks needed depends on the compatible string + qcom,sdm845-adsp-pil: + must be "xo", "sway_cbcr", "lpass_ahbs_aon_cbcr", + "lpass_ahbm_aon_cbcr", "qdsp6ss_xo", "qdsp6ss_sleep", + "qdsp6ss_core" + qcom,qcs404-cdsp-pil: + must be "xo", "sway", "tbu", "bimc", "ahb_aon", "q6ss_slave", + "q6ss_master", "q6_axim" + qcom,sc7280-wpss-pil: + must be "gcc_wpss_ahb_bdg_mst_clk", "gcc_wpss_ahb_clk", + "gcc_wpss_rscp_clk" - power-domains: Usage: required @@ -65,7 +70,7 @@ on the Qualcomm Technology Inc. Hexagon v56 core. Definition: must be "pdc_sync" and "cc_lpass" - reset-names: -Usage: required for QCS404 CDSP +Usage: required for QCS404 CDSP, SC7280 WPSS Value type: Definition: must be "restart" -- 2.7.4
[PATCH 0/2] Add support for sc7280 WPSS PIL loading
Add support for PIL loading of WPSS co-processor for SC7280 SOCs. Rakesh Pillai (2): dt-bindings: remoteproc: qcom: Add SC7280 WPSS support remoteproc: qcom: q6v5_wpss: Add support for sc7280 WPSS .../bindings/remoteproc/qcom,hexagon-v56.txt | 35 +- drivers/remoteproc/qcom_q6v5_adsp.c| 77 +- 2 files changed, 96 insertions(+), 16 deletions(-) -- 2.7.4
RE: [PATCH v2 0/2] ath10k: Fixes during subsystem recovery
> -Original Message- > From: Rakesh Pillai > Sent: Friday, January 15, 2021 6:56 PM > To: 'Kalle Valo' > Cc: 'ath...@lists.infradead.org' ; 'linux- > wirel...@vger.kernel.org' ; 'linux- > ker...@vger.kernel.org' > Subject: RE: [PATCH v2 0/2] ath10k: Fixes during subsystem recovery > > > > -Original Message- > > From: Kalle Valo > > Sent: Tuesday, December 22, 2020 12:07 AM > > To: Rakesh Pillai > > Cc: ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > > ker...@vger.kernel.org > > Subject: Re: [PATCH v2 0/2] ath10k: Fixes during subsystem recovery > > > > Rakesh Pillai writes: > > > > > This patch series includes some fixes when the device > > > is in recovery mode, i.e. when the firmware goes down. > > > > > > - Pausing TX queues when FW goes down > > > - Removed unwanted/extra error logging in pkt TX path > > > - Skipping wait for FW response for delete cmds > > > - Handling the -ESHUTDOWN error code in case of SSR. > > > > > > Rakesh Pillai (2): > > > ath10k: Pause the tx queues when firmware is down > > > ath10k: Skip wait for delete response if firmware is down > > > > This has been tested only on WCN3990. How do I know that this doesn't > > break other hardware families? > > " ath10k: Pause the tx queues when firmware is down" > This is done only for SNOC (wcn3990) target and does not affect other > targets. > > " ath10k: Skip wait for delete response if firmware is down" > The skip for wmi responses is done based on the flag > "ATH10K_FLAG_CRASH_FLUSH", which is generic for all targets. > Since if the FW is down, there wont be any response from the FW for any > target. > Hi Kalle, I see that these patches are in "deferred" state. Is there any info/change needed in this patch ? Thanks, Rakesh Pillai.
RE: [PATCH v2 0/2] ath10k: Fixes during subsystem recovery
> -Original Message- > From: Kalle Valo > Sent: Tuesday, December 22, 2020 12:07 AM > To: Rakesh Pillai > Cc: ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: Re: [PATCH v2 0/2] ath10k: Fixes during subsystem recovery > > Rakesh Pillai writes: > > > This patch series includes some fixes when the device > > is in recovery mode, i.e. when the firmware goes down. > > > > - Pausing TX queues when FW goes down > > - Removed unwanted/extra error logging in pkt TX path > > - Skipping wait for FW response for delete cmds > > - Handling the -ESHUTDOWN error code in case of SSR. > > > > Rakesh Pillai (2): > > ath10k: Pause the tx queues when firmware is down > > ath10k: Skip wait for delete response if firmware is down > > This has been tested only on WCN3990. How do I know that this doesn't > break other hardware families? " ath10k: Pause the tx queues when firmware is down" This is done only for SNOC (wcn3990) target and does not affect other targets. " ath10k: Skip wait for delete response if firmware is down" The skip for wmi responses is done based on the flag "ATH10K_FLAG_CRASH_FLUSH", which is generic for all targets. Since if the FW is down, there wont be any response from the FW for any target. > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingp > atches
RE: [PATCH 0/3] mac80211: Trigger disconnect for STA during recovery
> From: Ben Greear > > On 12/15/20 9:21 AM, Youghandhar Chintala wrote: > > From: Rakesh Pillai > > > > Currently in case of target hardware restart ,we just reconfig and > > re-enable the security keys and enable the network queues to start > > data traffic back from where it was interrupted. > > Are there any known mac80211 radios/drivers that *can* support seamless > restarts? > > If not, then just could always enable this feature in mac80211? I am not aware of any mac80211 target which can restart in a seamless manner. Hence I chose to keep this optional and driver can expose this flag (if needed) based on the hardware capability. Thanks, Rakesh Pillai.
[PATCH v2] ath10k: Remove voltage regulator votes during wifi disable
When the wlan is disabled, i.e when all the interfaces are
deleted, voltage regulator votes are not removed. This leads
to more power consumption even when wlan is disabled.
Move the adding/removing of voltage regulator votes as part
of hif power on/off in SNOC targets, so that these voltage
regulator votes are there only when wlan is enabled.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
Changes from v1:
- Remove the CE pipe init failure handling. Posted as a different patch
---
drivers/net/wireless/ath/ath10k/snoc.c | 92 +-
1 file changed, 46 insertions(+), 46 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index daae470..a5443fb 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -1003,6 +1003,39 @@ static int ath10k_snoc_wlan_enable(struct ath10k *ar,
NULL);
}
+static int ath10k_hw_power_on(struct ath10k *ar)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ int ret;
+
+ ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
+
+ ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
+ if (ret)
+ return ret;
+
+ ret = clk_bulk_prepare_enable(ar_snoc->num_clks, ar_snoc->clks);
+ if (ret)
+ goto vreg_off;
+
+ return ret;
+
+vreg_off:
+ regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
+ return ret;
+}
+
+static int ath10k_hw_power_off(struct ath10k *ar)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+
+ ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power off\n");
+
+ clk_bulk_disable_unprepare(ar_snoc->num_clks, ar_snoc->clks);
+
+ return regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
+}
+
static void ath10k_snoc_wlan_disable(struct ath10k *ar)
{
struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
@@ -1024,6 +1057,7 @@ static void ath10k_snoc_hif_power_down(struct ath10k *ar)
ath10k_snoc_wlan_disable(ar);
ath10k_ce_free_rri(ar);
+ ath10k_hw_power_off(ar);
}
static int ath10k_snoc_hif_power_up(struct ath10k *ar,
@@ -1034,10 +1068,16 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar,
ath10k_dbg(ar, ATH10K_DBG_SNOC, "%s:WCN3990 driver state = %d\n",
__func__, ar->state);
+ ret = ath10k_hw_power_on(ar);
+ if (ret) {
+ ath10k_err(ar, "failed to power on device: %d\n", ret);
+ return ret;
+ }
+
ret = ath10k_snoc_wlan_enable(ar, fw_mode);
if (ret) {
ath10k_err(ar, "failed to enable wcn3990: %d\n", ret);
- return ret;
+ goto err_hw_power_off;
}
ath10k_ce_alloc_rri(ar);
@@ -1054,6 +1094,9 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar,
ath10k_ce_free_rri(ar);
ath10k_snoc_wlan_disable(ar);
+err_hw_power_off:
+ ath10k_hw_power_off(ar);
+
return ret;
}
@@ -1370,39 +1413,6 @@ static void ath10k_snoc_release_resource(struct ath10k
*ar)
ath10k_ce_free_pipe(ar, i);
}
-static int ath10k_hw_power_on(struct ath10k *ar)
-{
- struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
- int ret;
-
- ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
-
- ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
- if (ret)
- return ret;
-
- ret = clk_bulk_prepare_enable(ar_snoc->num_clks, ar_snoc->clks);
- if (ret)
- goto vreg_off;
-
- return ret;
-
-vreg_off:
- regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
- return ret;
-}
-
-static int ath10k_hw_power_off(struct ath10k *ar)
-{
- struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
-
- ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power off\n");
-
- clk_bulk_disable_unprepare(ar_snoc->num_clks, ar_snoc->clks);
-
- return regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
-}
-
static void ath10k_msa_dump_memory(struct ath10k *ar,
struct ath10k_fw_crash_data *crash_data)
{
@@ -1712,22 +1722,16 @@ static int ath10k_snoc_probe(struct platform_device
*pdev)
if (ret)
goto err_free_irq;
- ret = ath10k_hw_power_on(ar);
- if (ret) {
- ath10k_err(ar, "failed to power on device: %d\n", ret);
- goto err_free_irq;
- }
-
ret = ath10k_setup_msa_resources(ar, msa_size);
if (ret) {
ath10k_warn(ar, "failed to setup msa resources: %d\n", ret);
- goto err_power_off;
+ goto err_free_irq;
}
re
[PATCH] ath10k: Fix error handling in case of CE pipe init failure
Currently if the copy engine pipe init fails for snoc based
chipsets, the rri is not freed.
Fix this error handling for copy engine pipe init
failure.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Fixes: 4945af5b264f ("ath10k: enable SRRI/DRRI support on ddr for WCN3990")
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/snoc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index fd41f25..daae470 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -1045,12 +1045,13 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar,
ret = ath10k_snoc_init_pipes(ar);
if (ret) {
ath10k_err(ar, "failed to initialize CE: %d\n", ret);
- goto err_wlan_enable;
+ goto err_free_rri;
}
return 0;
-err_wlan_enable:
+err_free_rri:
+ ath10k_ce_free_rri(ar);
ath10k_snoc_wlan_disable(ar);
return ret;
--
2.7.4
RE: [PATCH] ath10k: Remove voltage regulator votes during wifi disable
> -Original Message-
> From: Brian Norris
> Sent: Thursday, December 10, 2020 11:44 PM
> To: Rakesh Pillai
> Cc: ath10k ; linux-wireless wirel...@vger.kernel.org>; Linux Kernel ;
> Doug Anderson ; kua...@chromium.org;
> Youghandhar Chintala
> Subject: Re: [PATCH] ath10k: Remove voltage regulator votes during wifi
> disable
>
> On Thu, Dec 10, 2020 at 7:09 AM Rakesh Pillai
> wrote:
> > --- a/drivers/net/wireless/ath/ath10k/snoc.c
> > +++ b/drivers/net/wireless/ath/ath10k/snoc.c
> > @@ -1045,14 +1085,18 @@ static int ath10k_snoc_hif_power_up(struct
> ath10k *ar,
> > ret = ath10k_snoc_init_pipes(ar);
> > if (ret) {
> > ath10k_err(ar, "failed to initialize CE: %d\n", ret);
> > - goto err_wlan_enable;
> > + goto err_free_rri;
> > }
> >
> > return 0;
> >
> > -err_wlan_enable:
> > +err_free_rri:
> > + ath10k_ce_free_rri(ar);
>
> This change in the error path seems to be an unrelated (but correct)
> fix. It deserves its own patch, I think.
Sure Brian. I will post this error handling fix as a separate patch, and also
post a v2 for this patchset.
[PATCH] ath10k: Remove voltage regulator votes during wifi disable
When the wlan is disabled, i.e when all the interfaces are
deleted, voltage regulator votes are not removed. This leads
to more power consumption even when wlan is disabled.
Move the adding/removing of voltage regulator votes as part
of hif power on/off in SNOC targets, so that these voltage
regulator votes are there only when wlan is enabled.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/snoc.c | 97 +-
1 file changed, 49 insertions(+), 48 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index fd41f25..a5443fb 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -1003,6 +1003,39 @@ static int ath10k_snoc_wlan_enable(struct ath10k *ar,
NULL);
}
+static int ath10k_hw_power_on(struct ath10k *ar)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+ int ret;
+
+ ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
+
+ ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
+ if (ret)
+ return ret;
+
+ ret = clk_bulk_prepare_enable(ar_snoc->num_clks, ar_snoc->clks);
+ if (ret)
+ goto vreg_off;
+
+ return ret;
+
+vreg_off:
+ regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
+ return ret;
+}
+
+static int ath10k_hw_power_off(struct ath10k *ar)
+{
+ struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
+
+ ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power off\n");
+
+ clk_bulk_disable_unprepare(ar_snoc->num_clks, ar_snoc->clks);
+
+ return regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
+}
+
static void ath10k_snoc_wlan_disable(struct ath10k *ar)
{
struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
@@ -1024,6 +1057,7 @@ static void ath10k_snoc_hif_power_down(struct ath10k *ar)
ath10k_snoc_wlan_disable(ar);
ath10k_ce_free_rri(ar);
+ ath10k_hw_power_off(ar);
}
static int ath10k_snoc_hif_power_up(struct ath10k *ar,
@@ -1034,10 +1068,16 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar,
ath10k_dbg(ar, ATH10K_DBG_SNOC, "%s:WCN3990 driver state = %d\n",
__func__, ar->state);
+ ret = ath10k_hw_power_on(ar);
+ if (ret) {
+ ath10k_err(ar, "failed to power on device: %d\n", ret);
+ return ret;
+ }
+
ret = ath10k_snoc_wlan_enable(ar, fw_mode);
if (ret) {
ath10k_err(ar, "failed to enable wcn3990: %d\n", ret);
- return ret;
+ goto err_hw_power_off;
}
ath10k_ce_alloc_rri(ar);
@@ -1045,14 +1085,18 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar,
ret = ath10k_snoc_init_pipes(ar);
if (ret) {
ath10k_err(ar, "failed to initialize CE: %d\n", ret);
- goto err_wlan_enable;
+ goto err_free_rri;
}
return 0;
-err_wlan_enable:
+err_free_rri:
+ ath10k_ce_free_rri(ar);
ath10k_snoc_wlan_disable(ar);
+err_hw_power_off:
+ ath10k_hw_power_off(ar);
+
return ret;
}
@@ -1369,39 +1413,6 @@ static void ath10k_snoc_release_resource(struct ath10k
*ar)
ath10k_ce_free_pipe(ar, i);
}
-static int ath10k_hw_power_on(struct ath10k *ar)
-{
- struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
- int ret;
-
- ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power on\n");
-
- ret = regulator_bulk_enable(ar_snoc->num_vregs, ar_snoc->vregs);
- if (ret)
- return ret;
-
- ret = clk_bulk_prepare_enable(ar_snoc->num_clks, ar_snoc->clks);
- if (ret)
- goto vreg_off;
-
- return ret;
-
-vreg_off:
- regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
- return ret;
-}
-
-static int ath10k_hw_power_off(struct ath10k *ar)
-{
- struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
-
- ath10k_dbg(ar, ATH10K_DBG_SNOC, "soc power off\n");
-
- clk_bulk_disable_unprepare(ar_snoc->num_clks, ar_snoc->clks);
-
- return regulator_bulk_disable(ar_snoc->num_vregs, ar_snoc->vregs);
-}
-
static void ath10k_msa_dump_memory(struct ath10k *ar,
struct ath10k_fw_crash_data *crash_data)
{
@@ -1711,22 +1722,16 @@ static int ath10k_snoc_probe(struct platform_device
*pdev)
if (ret)
goto err_free_irq;
- ret = ath10k_hw_power_on(ar);
- if (ret) {
- ath10k_err(ar, "failed to power on device: %d\n", ret);
- goto err_free_irq;
- }
-
ret = ath10k_setup_msa_resources(ar, msa_size);
if (
RE: [PATCH v3] ath10k: add option for chip-id based BDF selection
Hi, > -Original Message- > From: Abhishek Kumar > Sent: Tuesday, December 8, 2020 4:50 AM > To: kv...@codeaurora.org > Cc: linux-kernel@vger.kernel.org; kua...@chromium.org; linux- > wirel...@vger.kernel.org; ath...@lists.infradead.org; > pill...@codeaurora.org; briannor...@chromium.org; > diand...@chromium.org; David S. Miller ; Jakub > Kicinski ; net...@vger.kernel.org > Subject: [PATCH v3] ath10k: add option for chip-id based BDF selection > > In some devices difference in chip-id should be enough to pick > the right BDF. Add another support for chip-id based BDF selection. > With this new option, ath10k supports 2 fallback options. > > The board name with chip-id as option looks as follows > board name 'bus=snoc,qmi-board-id=ff,qmi-chip-id=320' > > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-QCAHLSWMTPL-1 > Tested-on: QCA6174 HW3.2 WLAN.RM.4.4.1-00157-QCARMSWPZ-1 > Signed-off-by: Abhishek Kumar > --- > > Changes in v3: > - Resurreted Patch V1 because as per discussion we do need two > fallback board names and refactored ath10k_core_create_board_name. > > drivers/net/wireless/ath/ath10k/core.c | 41 +++++++--- > 1 file changed, 30 insertions(+), 11 deletions(-) Reviewed-by: Rakesh Pillai Thanks, Rakesh Pillai.
RE: [PATCH v2 1/1] ath10k: add option for chip-id based BDF selection
> -Original Message- > From: Doug Anderson > Sent: Tuesday, December 1, 2020 12:49 AM > To: Rakesh Pillai > Cc: Abhishek Kumar ; Kalle Valo > ; LKML ; ath10k > ; Brian Norris ; > linux-wireless ; David S. Miller > ; Jakub Kicinski ; netdev > > Subject: Re: [PATCH v2 1/1] ath10k: add option for chip-id based BDF > selection > > Hi, > > On Tue, Nov 24, 2020 at 7:44 PM Rakesh Pillai > wrote: > > > > > > I missed on reviewing this change. Also I agree with Doug that this is > > > > not > > > the change I was looking for. > > > > > > > > The argument "with_variant" can be renamed to "with_extra_params". > > > There is no need for any new argument to this function. > > > > Case 1: with_extra_params=0, ar->id.bdf_ext[0] = 0 -> The > default > > > name will be used (bus=snoc,qmi_board_id=0xab) > > > > Case 2: with_extra_params=1, ar->id.bdf_ext[0] = 0 -> > > > bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd > > > > Case 3: with_extra_params=1, ar->id.bdf_ext[0] = "xyz" -> > > > bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd,variant=xyz > > > > > > > > ar->id.bdf_ext[0] depends on the DT entry for variant field. > > > > > > I'm confused about your suggestion. Maybe you can help clarify. Are > > > you suggesting: > > > > > > a) Only two calls to ath10k_core_create_board_name() > > > > > > I'm pretty sure this will fail in some cases. Specifically consider > > > the case where the device tree has a "variant" defined but the BRD > > > file only has one entry for (board-id) and one for (board-id + > > > chip-id) but no entry for (board-id + chip-id + variant). If you are > > > only making two calls then I don't think you'll pick the right one. > > > > > > Said another way... > > > > > > If the device tree has a variant: > > > 1. We should prefer a BRD entry that has board-id + chip-id + variant > > > 2. If #1 isn't there, we should prefer a BRD entry that has board-id + > > > chip- > id > > > 3. If #1 and #2 aren't there we fall back to a BRD entry that has > > > board-id. > > > > > > ...without 3 calls to ath10k_core_create_board_name() we can't handle > > > all 3 cases. > > > > This can be handled by two calls to ath10k_core_create_board_name > > 1) ath10k_core_create_board_name(ar, boardname, sizeof(boardname), > true) : As per my suggestions, this can result in two possible board names > > a) If DT have the "variant" node, it outputs the #1 from your suggestion > (1. We should prefer a BRD entry that has board-id + chip-id + variant) > > b) If DT does not have the "variant" node, it outputs the #2 from your > suggestion (2. If #1 isn't there, we should prefer a BRD entry that has board- > id + chip-id) > > > > 2) ath10k_core_create_board_name(ar, boardname, sizeof(boardname), > false): This is the second call to this function and outputs the #3 from > your > suggestion (3. If #1 and #2 aren't there we fall back to a BRD entry that has > board-id) > > What I'm trying to say is this. Imagine that: > > a) the device tree has the "variant" property. > > b) the BRD file has two entries, one for "board-id" (1) and one for > "board-id + chip-id" (2). It doesn't have one for "board-id + chip-id > + variant" (3). > > With your suggestion we'll see the "variant" property in the device > tree. That means we'll search for (1) and (3). (3) isn't there, so > we'll pick (1). ...but we really should have picked (2), right? Do we expect board-2.bin to not be populated with the bdf with variant field (if its necessary ?) Seems fine for me, if we have 2 fallback names if that is needed. > > -Doug
RE: [PATCH v2 1/1] ath10k: add option for chip-id based BDF selection
> -Original Message-
> From: Doug Anderson
> Sent: Tuesday, November 24, 2020 9:56 PM
> To: Rakesh Pillai
> Cc: Abhishek Kumar ; Kalle Valo
> ; LKML ; ath10k
> ; Brian Norris ;
> linux-wireless ; David S. Miller
> ; Jakub Kicinski ; netdev
>
> Subject: Re: [PATCH v2 1/1] ath10k: add option for chip-id based BDF
> selection
>
> Hi,
>
> On Tue, Nov 24, 2020 at 1:19 AM Rakesh Pillai
> wrote:
> >
> > > -Original Message-
> > > From: Doug Anderson
> > > Sent: Tuesday, November 24, 2020 6:27 AM
> > > To: Abhishek Kumar
> > > Cc: Kalle Valo ; Rakesh Pillai
> > > ; LKML ; ath10k
> > > ; Brian Norris ;
> > > linux-wireless ; David S. Miller
> > > ; Jakub Kicinski ; netdev
> > >
> > > Subject: Re: [PATCH v2 1/1] ath10k: add option for chip-id based BDF
> > > selection
> > >
> > > Hi,
> > >
> > > On Thu, Nov 12, 2020 at 12:09 PM Abhishek Kumar
>
> > > wrote:
> > > >
> > > > In some devices difference in chip-id should be enough to pick
> > > > the right BDF. Add another support for chip-id based BDF selection.
> > > > With this new option, ath10k supports 2 fallback options.
> > > >
> > > > The board name with chip-id as option looks as follows
> > > > board name 'bus=snoc,qmi-board-id=ff,qmi-chip-id=320'
> > > >
> > > > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-
> QCAHLSWMTPL-1
> > > > Tested-on: QCA6174 HW3.2 WLAN.RM.4.4.1-00157-QCARMSWPZ-1
> > > > Signed-off-by: Abhishek Kumar
> > > > ---
> > > >
> > > > (no changes since v1)
> > >
> > > I think you need to work on the method you're using to generate your
> > > patches. There are most definitely changes since v1. You described
> > > them in your cover letter (which you don't really need for a singleton
> > > patch) instead of here.
> > >
> > >
> > > > @@ -1438,12 +1439,17 @@ static int
> > > ath10k_core_create_board_name(struct ath10k *ar, char *name,
> > > > }
> > > >
> > > > if (ar->id.qmi_ids_valid) {
> > > > - if (with_variant && ar->id.bdf_ext[0] != '\0')
> > > > + if (with_additional_params && ar->id.bdf_ext[0] != '\0')
> > > > scnprintf(name, name_len,
> > > >
> > > > "bus=%s,qmi-board-id=%x,qmi-chip-id=%x%s",
> > > > ath10k_bus_str(ar->hif.bus),
> > > > ar->id.qmi_board_id,
> > > > ar->id.qmi_chip_id,
> > > > variant);
> > > > + else if (with_additional_params)
> > > > + scnprintf(name, name_len,
> > > > +
> > > > "bus=%s,qmi-board-id=%x,qmi-chip-id=%x",
> > > > + ath10k_bus_str(ar->hif.bus),
> > > > + ar->id.qmi_board_id,
> > > > ar->id.qmi_chip_id);
> > >
> > > I believe this is exactly opposite of what Rakesh was requesting.
> > > Specifically, he was trying to eliminate the extra scnprintf() but I
> > > think he still agreed that it was a good idea to generate 3 different
> > > strings. I believe the proper diff to apply to v1 is:
> > >
> > > https://crrev.com/c/255643
>
> Wow, I seem to have deleted the last digit from my URL. Should have been:
>
> https://crrev.com/c/2556437
>
> > >
> > > -Doug
> >
> > Hi Abhishek/Doug,
> >
> > I missed on reviewing this change. Also I agree with Doug that this is not
> the change I was looking for.
> >
> > The argument "with_variant" can be renamed to "with_extra_params".
> There is no need for any new argument to this function.
> > Case 1: with_extra_params=0, ar->id.bdf_ext[0] = 0 -> The
> > default
> name will be used (bus=snoc,qmi_board_id=0xab)
> > Case 2: with_extra_params=1, ar->id.bdf_ext[0] = 0 ->
> bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd
> > Case 3: with_extra_params=1, ar->id.bdf_ext[0] = "xyz" ->
> bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd,variant=xyz
> >
> > ar->id.bdf_ext[0] depends on the DT entry for variant
RE: [PATCH v2 1/1] ath10k: add option for chip-id based BDF selection
> -Original Message-
> From: Doug Anderson
> Sent: Tuesday, November 24, 2020 6:27 AM
> To: Abhishek Kumar
> Cc: Kalle Valo ; Rakesh Pillai
> ; LKML ; ath10k
> ; Brian Norris ;
> linux-wireless ; David S. Miller
> ; Jakub Kicinski ; netdev
>
> Subject: Re: [PATCH v2 1/1] ath10k: add option for chip-id based BDF
> selection
>
> Hi,
>
> On Thu, Nov 12, 2020 at 12:09 PM Abhishek Kumar
> wrote:
> >
> > In some devices difference in chip-id should be enough to pick
> > the right BDF. Add another support for chip-id based BDF selection.
> > With this new option, ath10k supports 2 fallback options.
> >
> > The board name with chip-id as option looks as follows
> > board name 'bus=snoc,qmi-board-id=ff,qmi-chip-id=320'
> >
> > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00696-QCAHLSWMTPL-1
> > Tested-on: QCA6174 HW3.2 WLAN.RM.4.4.1-00157-QCARMSWPZ-1
> > Signed-off-by: Abhishek Kumar
> > ---
> >
> > (no changes since v1)
>
> I think you need to work on the method you're using to generate your
> patches. There are most definitely changes since v1. You described
> them in your cover letter (which you don't really need for a singleton
> patch) instead of here.
>
>
> > @@ -1438,12 +1439,17 @@ static int
> ath10k_core_create_board_name(struct ath10k *ar, char *name,
> > }
> >
> > if (ar->id.qmi_ids_valid) {
> > - if (with_variant && ar->id.bdf_ext[0] != '\0')
> > + if (with_additional_params && ar->id.bdf_ext[0] != '\0')
> > scnprintf(name, name_len,
> > "bus=%s,qmi-board-id=%x,qmi-chip-id=%x%s",
> > ath10k_bus_str(ar->hif.bus),
> > ar->id.qmi_board_id, ar->id.qmi_chip_id,
> > variant);
> > + else if (with_additional_params)
> > + scnprintf(name, name_len,
> > + "bus=%s,qmi-board-id=%x,qmi-chip-id=%x",
> > + ath10k_bus_str(ar->hif.bus),
> > + ar->id.qmi_board_id, ar->id.qmi_chip_id);
>
> I believe this is exactly opposite of what Rakesh was requesting.
> Specifically, he was trying to eliminate the extra scnprintf() but I
> think he still agreed that it was a good idea to generate 3 different
> strings. I believe the proper diff to apply to v1 is:
>
> https://crrev.com/c/255643
>
> -Doug
Hi Abhishek/Doug,
I missed on reviewing this change. Also I agree with Doug that this is not the
change I was looking for.
The argument "with_variant" can be renamed to "with_extra_params". There is no
need for any new argument to this function.
Case 1: with_extra_params=0, ar->id.bdf_ext[0] = 0 -> The
default name will be used (bus=snoc,qmi_board_id=0xab)
Case 2: with_extra_params=1, ar->id.bdf_ext[0] = 0 ->
bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd
Case 3: with_extra_params=1, ar->id.bdf_ext[0] = "xyz" ->
bus=snoc,qmi_board_id=0xab,qmi_chip_id=0xcd,variant=xyz
ar->id.bdf_ext[0] depends on the DT entry for variant field.
Thanks,
Rakesh Pillai.
[PATCH v3] ath10k: Fix the parsing error in service available event
The wmi service available event has been
extended to contain extra 128 bit for new services
to be indicated by firmware.
Currently the presence of any optional TLVs in
the wmi service available event leads to a parsing
error with the below error message:
ath10k_snoc 1880.wifi: failed to parse svc_avail tlv: -71
The wmi service available event parsing should
not return error for the newly added optional TLV.
Fix this parsing for service available event message.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.2.2-00720-QCAHLSWMTPL-1
Fixes: cea19a6ce8bf ("ath10k: add WMI_SERVICE_AVAILABLE_EVENT support")
Signed-off-by: Rakesh Pillai
---
Changes from v2:
- Add code documentation explaining the necessity of variable
initialization for the logic to work.
---
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 4 +++-
drivers/net/wireless/ath/ath10k/wmi.c | 9 +++--
drivers/net/wireless/ath/ath10k/wmi.h | 1 +
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 932266d..7b58341 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1401,13 +1401,15 @@ static int ath10k_wmi_tlv_svc_avail_parse(struct ath10k
*ar, u16 tag, u16 len,
switch (tag) {
case WMI_TLV_TAG_STRUCT_SERVICE_AVAILABLE_EVENT:
+ arg->service_map_ext_valid = true;
arg->service_map_ext_len = *(__le32 *)ptr;
arg->service_map_ext = ptr + sizeof(__le32);
return 0;
default:
break;
}
- return -EPROTO;
+
+ return 0;
}
static int ath10k_wmi_tlv_op_pull_svc_avail(struct ath10k *ar,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
b/drivers/net/wireless/ath/ath10k/wmi.c
index 1fa7107..37b53af 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -5751,8 +5751,13 @@ void ath10k_wmi_event_service_available(struct ath10k
*ar, struct sk_buff *skb)
ret);
}
- ath10k_wmi_map_svc_ext(ar, arg.service_map_ext, ar->wmi.svc_map,
- __le32_to_cpu(arg.service_map_ext_len));
+ /*
+* Initialization of "arg.service_map_ext_valid" to ZERO is necessary
+* for the below logic to work.
+*/
+ if (arg.service_map_ext_valid)
+ ath10k_wmi_map_svc_ext(ar, arg.service_map_ext, ar->wmi.svc_map,
+ __le32_to_cpu(arg.service_map_ext_len));
}
static int ath10k_wmi_event_temperature(struct ath10k *ar, struct sk_buff *skb)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h
b/drivers/net/wireless/ath/ath10k/wmi.h
index 4898e19..66ecf09 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6917,6 +6917,7 @@ struct wmi_svc_rdy_ev_arg {
};
struct wmi_svc_avail_ev_arg {
+ bool service_map_ext_valid;
__le32 service_map_ext_len;
const __le32 *service_map_ext;
};
--
2.7.4
RE: [PATCH v2] ath10k: Fix the parsing error in service available event
> -Original Message-
> From: Doug Anderson
> Sent: Thursday, October 29, 2020 12:15 AM
> To: Rakesh Pillai
> Cc: ath10k ; linux-wireless wirel...@vger.kernel.org>; LKML ; Abhishek
> Kumar ; Brian Norris
> Subject: Re: [PATCH v2] ath10k: Fix the parsing error in service available
> event
>
> Hi,
>
> On Wed, Oct 28, 2020 at 10:01 AM Rakesh Pillai
> wrote:
> >
> > The wmi service available event has been
> > extended to contain extra 128 bit for new services
> > to be indicated by firmware.
> >
> > Currently the presence of any optional TLVs in
> > the wmi service available event leads to a parsing
> > error with the below error message:
> > ath10k_snoc 1880.wifi: failed to parse svc_avail tlv: -71
> >
> > The wmi service available event parsing should
> > not return error for the newly added optional TLV.
> > Fix this parsing for service available event message.
> >
> > Tested-on: WCN3990 hw1.0 SNOC
> >
> > Fixes: cea19a6ce8bf ("ath10k: add WMI_SERVICE_AVAILABLE_EVENT
> support")
> > Signed-off-by: Rakesh Pillai
> > ---
> > Changes from v1:
> > - Access service_map_ext only if this TLV was sent in service
> > available event.
> > ---
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 4 +++-
> > drivers/net/wireless/ath/ath10k/wmi.c | 5 +++--
> > drivers/net/wireless/ath/ath10k/wmi.h | 1 +
> > 3 files changed, 7 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > index 932266d..7b58341 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > @@ -1401,13 +1401,15 @@ static int
> ath10k_wmi_tlv_svc_avail_parse(struct ath10k *ar, u16 tag, u16 len,
> >
> > switch (tag) {
> > case WMI_TLV_TAG_STRUCT_SERVICE_AVAILABLE_EVENT:
> > + arg->service_map_ext_valid = true;
> > arg->service_map_ext_len = *(__le32 *)ptr;
> > arg->service_map_ext = ptr + sizeof(__le32);
> > return 0;
> > default:
> > break;
> > }
> > - return -EPROTO;
> > +
> > + return 0;
>
> I notice your v2 now returns 0 for _all_ unknown tags. v1 just had a
> special case for "WMI_TLV_TAG_FIRST_ARRAY_ENUM". I don't have
> enough
> experience with this driver to know which is better, but this change
> wasn't mentioned in the changes from v1. I guess you had a change of
> heart and decided this way was better?
Earlier patchset which added a case for " WMI_TLV_TAG_FIRST_ARRAY_ENUM", still
returned error if there is any other TLV except for the two cases handled.
This leaves the possibility of an error when a new TLV gets added to this
service_available message.
Since we are using the "valid" flag to indicate the validity of a particular
tag, we need not return failure in any case. This makes it scalable (and
maintains backward compatibility), in case extra TLVs are added to this message
in future.
>
>
> > }
> >
> > static int ath10k_wmi_tlv_op_pull_svc_avail(struct ath10k *ar,
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
> b/drivers/net/wireless/ath/ath10k/wmi.c
> > index 1fa7107..2e4b561 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi.c
> > @@ -5751,8 +5751,9 @@ void ath10k_wmi_event_service_available(struct
> ath10k *ar, struct sk_buff *skb)
> > ret);
> > }
> >
> > - ath10k_wmi_map_svc_ext(ar, arg.service_map_ext, ar-
> >wmi.svc_map,
> > - __le32_to_cpu(arg.service_map_ext_len));
> > + if (arg.service_map_ext_valid)
> > + ath10k_wmi_map_svc_ext(ar, arg.service_map_ext, ar-
> >wmi.svc_map,
> > +
> > __le32_to_cpu(arg.service_map_ext_len));
>
> Your new patch still requires the caller to init the
> "service_map_ext_valid" to false before calling, but I guess there's
> not a whole lot more we can do because we might be parsing more than
> one tag. It does seem nice that at least we now have a validity bit
> instead of just relying on a non-zero length to be valid.
>
> It might be nice to have a comment saying that it's up to us to init
> "arg.service_map_ext_valid" to false before calling
> ath10k_wmi_pull_svc_avail(), but I won't insist. Maybe that's obvious
> to everyone but me...
I will wait for a couple of days, if there are any other comments, to post a v3
addressing all of them together.
This approach of having a argument initialized to parse TLVs is used for many
other wmi events as well.
>
>
> -Doug
[PATCH v2] ath10k: Fix the parsing error in service available event
The wmi service available event has been
extended to contain extra 128 bit for new services
to be indicated by firmware.
Currently the presence of any optional TLVs in
the wmi service available event leads to a parsing
error with the below error message:
ath10k_snoc 1880.wifi: failed to parse svc_avail tlv: -71
The wmi service available event parsing should
not return error for the newly added optional TLV.
Fix this parsing for service available event message.
Tested-on: WCN3990 hw1.0 SNOC
Fixes: cea19a6ce8bf ("ath10k: add WMI_SERVICE_AVAILABLE_EVENT support")
Signed-off-by: Rakesh Pillai
---
Changes from v1:
- Access service_map_ext only if this TLV was sent in service
available event.
---
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 4 +++-
drivers/net/wireless/ath/ath10k/wmi.c | 5 +++--
drivers/net/wireless/ath/ath10k/wmi.h | 1 +
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index 932266d..7b58341 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -1401,13 +1401,15 @@ static int ath10k_wmi_tlv_svc_avail_parse(struct ath10k
*ar, u16 tag, u16 len,
switch (tag) {
case WMI_TLV_TAG_STRUCT_SERVICE_AVAILABLE_EVENT:
+ arg->service_map_ext_valid = true;
arg->service_map_ext_len = *(__le32 *)ptr;
arg->service_map_ext = ptr + sizeof(__le32);
return 0;
default:
break;
}
- return -EPROTO;
+
+ return 0;
}
static int ath10k_wmi_tlv_op_pull_svc_avail(struct ath10k *ar,
diff --git a/drivers/net/wireless/ath/ath10k/wmi.c
b/drivers/net/wireless/ath/ath10k/wmi.c
index 1fa7107..2e4b561 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.c
+++ b/drivers/net/wireless/ath/ath10k/wmi.c
@@ -5751,8 +5751,9 @@ void ath10k_wmi_event_service_available(struct ath10k
*ar, struct sk_buff *skb)
ret);
}
- ath10k_wmi_map_svc_ext(ar, arg.service_map_ext, ar->wmi.svc_map,
- __le32_to_cpu(arg.service_map_ext_len));
+ if (arg.service_map_ext_valid)
+ ath10k_wmi_map_svc_ext(ar, arg.service_map_ext, ar->wmi.svc_map,
+ __le32_to_cpu(arg.service_map_ext_len));
}
static int ath10k_wmi_event_temperature(struct ath10k *ar, struct sk_buff *skb)
diff --git a/drivers/net/wireless/ath/ath10k/wmi.h
b/drivers/net/wireless/ath/ath10k/wmi.h
index 4898e19..66ecf09 100644
--- a/drivers/net/wireless/ath/ath10k/wmi.h
+++ b/drivers/net/wireless/ath/ath10k/wmi.h
@@ -6917,6 +6917,7 @@ struct wmi_svc_rdy_ev_arg {
};
struct wmi_svc_avail_ev_arg {
+ bool service_map_ext_valid;
__le32 service_map_ext_len;
const __le32 *service_map_ext;
};
--
2.7.4
RE: [PATCH] ath10k: Fix the parsing error in service available event
> -Original Message-
> From: Doug Anderson
> Sent: Wednesday, October 28, 2020 9:33 PM
> To: Rakesh Pillai
> Cc: ath10k ; linux-wireless wirel...@vger.kernel.org>; LKML ; Abhishek
> Kumar ; Brian Norris
> Subject: Re: [PATCH] ath10k: Fix the parsing error in service available event
>
> Hi,
>
> On Wed, Oct 28, 2020 at 8:47 AM Rakesh Pillai
> wrote:
> >
> > > -Original Message-
> > > From: Doug Anderson
> > > Sent: Wednesday, October 28, 2020 8:07 PM
> > > To: Rakesh Pillai
> > > Cc: ath10k ; linux-wireless > > wirel...@vger.kernel.org>; LKML ;
> Abhishek
> > > Kumar ; Brian Norris
>
> > > Subject: Re: [PATCH] ath10k: Fix the parsing error in service available
> event
> > >
> > > Hi,
> > >
> > > On Tue, Oct 27, 2020 at 8:20 AM Rakesh Pillai
> > > wrote:
> > > >
> > > > The wmi service available event has been
> > > > extended to contain extra 128 bit for new services
> > > > to be indicated by firmware.
> > > >
> > > > Currently the presence of any optional TLVs in
> > > > the wmi service available event leads to a parsing
> > > > error with the below error message:
> > > > ath10k_snoc 1880.wifi: failed to parse svc_avail tlv: -71
> > > >
> > > > The wmi service available event parsing should
> > > > not return error for the newly added optional TLV.
> > > > Fix this parsing for service available event message.
> > > >
> > > > Tested-on: WCN3990 hw1.0 SNOC
> > > >
> > > > Signed-off-by: Rakesh Pillai
> > > > ---
> > > > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > > b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > > > index 932266d..3b49e29 100644
> > > > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > > > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > > > @@ -1404,9 +1404,12 @@ static int
> ath10k_wmi_tlv_svc_avail_parse(struct
> > > ath10k *ar, u16 tag, u16 len,
> > > > arg->service_map_ext_len = *(__le32 *)ptr;
> > > > arg->service_map_ext = ptr + sizeof(__le32);
> > > > return 0;
> > > > + case WMI_TLV_TAG_FIRST_ARRAY_ENUM:
> > > > + return 0;
> > >
> > > This is at least slightly worrying to me. If I were calling this
> > > function, I'd expect that if I didn't get back an error that at least
> > > "arg->service_map_ext_len" was filled in. Seems like you should do:
> > >
> > > case WMI_TLV_TAG_FIRST_ARRAY_ENUM:
> > > arg->service_map_ext_len = 0;
> > > arg->service_map_ext = NULL;
> > > return 0;
> > >
> > > ...and maybe add a comment about why you're doing that?
> > >
> > > At the moment things are working OK because
> > > ath10k_wmi_event_service_available() happens to init the structure to
> > > 0 before calling with:
> > >
> > > struct wmi_svc_avail_ev_arg arg = {};
> > >
> > > but it doesn't seem like a great idea to rely on that.
> > >
> > > That all being said, I'm just a drive-by reviewer and if everyone else
> > > likes it the way it is, feel free to ignore my comments.
> >
> >
> > Hi Doug,
> >
> > The TLV TAG " WMI_TLV_TAG_STRUCT_SERVICE_AVAILABLE_EVENT" is
> the first and a mandatory TLV in the service available event.
> > The subsequent TLVs are optional ones and may or may not be present
> (based on FW versions).
> > This patch just fixes the bug, where the presence of any other TLVs are
> leading to a failure in parsing the service available msg.
> > If, in future, we plan to use any other services from firmware, which is
> exposed in the extended TLVs, we will need to add a new variable (and not
> service_map_ext) to set the service.
>
> I'm not sure I totally understood your response, but look at it from
> the perspective of the function ath10k_wmi_event_service_available().
>
> That function calls:
>
> ret = ath10k_wmi_pull_svc_avail(ar, skb, );
>
> ...if it gets back a non-zero error code, it assumes that the
> "arg.service_map_ext" and "arg.service_map_ext_len" values are now
> valid and it can use them.
>
> Before your patch, ath10k_wmi_pull_svc_avail() was returning an error
> code. That let ath10k_wmi_event_service_available() know that it
> shouldn't look at "arg.service_map_ext" and "arg.service_map_ext_len".
> After your patch, you're not returning an error code but those fields
> aren't being filled in.
>
> Said another way, if you remove the initialization of "arg" in
> ath10k_wmi_event_service_available() then everything is broken. While
> things work because you _do_ have an initialization of "arg" in
> ath10k_wmi_event_service_available(), it feels fragile to me to rely
> on that.
Hi Doug,
Got it. I will send a v2 which will address this concern.
>
>
> -Doug
RE: [PATCH] ath10k: Fix the parsing error in service available event
> -Original Message-
> From: Doug Anderson
> Sent: Wednesday, October 28, 2020 8:07 PM
> To: Rakesh Pillai
> Cc: ath10k ; linux-wireless wirel...@vger.kernel.org>; LKML ; Abhishek
> Kumar ; Brian Norris
> Subject: Re: [PATCH] ath10k: Fix the parsing error in service available event
>
> Hi,
>
> On Tue, Oct 27, 2020 at 8:20 AM Rakesh Pillai
> wrote:
> >
> > The wmi service available event has been
> > extended to contain extra 128 bit for new services
> > to be indicated by firmware.
> >
> > Currently the presence of any optional TLVs in
> > the wmi service available event leads to a parsing
> > error with the below error message:
> > ath10k_snoc 1880.wifi: failed to parse svc_avail tlv: -71
> >
> > The wmi service available event parsing should
> > not return error for the newly added optional TLV.
> > Fix this parsing for service available event message.
> >
> > Tested-on: WCN3990 hw1.0 SNOC
> >
> > Signed-off-by: Rakesh Pillai
> > ---
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 3 +++
> > 1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > index 932266d..3b49e29 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > @@ -1404,9 +1404,12 @@ static int ath10k_wmi_tlv_svc_avail_parse(struct
> ath10k *ar, u16 tag, u16 len,
> > arg->service_map_ext_len = *(__le32 *)ptr;
> > arg->service_map_ext = ptr + sizeof(__le32);
> > return 0;
> > + case WMI_TLV_TAG_FIRST_ARRAY_ENUM:
> > + return 0;
>
> This is at least slightly worrying to me. If I were calling this
> function, I'd expect that if I didn't get back an error that at least
> "arg->service_map_ext_len" was filled in. Seems like you should do:
>
> case WMI_TLV_TAG_FIRST_ARRAY_ENUM:
> arg->service_map_ext_len = 0;
> arg->service_map_ext = NULL;
> return 0;
>
> ...and maybe add a comment about why you're doing that?
>
> At the moment things are working OK because
> ath10k_wmi_event_service_available() happens to init the structure to
> 0 before calling with:
>
> struct wmi_svc_avail_ev_arg arg = {};
>
> but it doesn't seem like a great idea to rely on that.
>
> That all being said, I'm just a drive-by reviewer and if everyone else
> likes it the way it is, feel free to ignore my comments.
Hi Doug,
The TLV TAG " WMI_TLV_TAG_STRUCT_SERVICE_AVAILABLE_EVENT" is the first and a
mandatory TLV in the service available event.
The subsequent TLVs are optional ones and may or may not be present (based on
FW versions).
This patch just fixes the bug, where the presence of any other TLVs are leading
to a failure in parsing the service available msg.
If, in future, we plan to use any other services from firmware, which is
exposed in the extended TLVs, we will need to add a new variable (and not
service_map_ext) to set the service.
>
> -Doug
RE: [PATCH] ath10k: add option for chip-id based BDF selection
> -Original Message-
> From: Doug Anderson
> Sent: Tuesday, October 27, 2020 8:26 PM
> To: Rakesh Pillai
> Cc: Abhishek Kumar ; Kalle Valo
> ; ath10k ; LKML
> ; linux-wireless wirel...@vger.kernel.org>; Brian Norris
> Subject: Re: [PATCH] ath10k: add option for chip-id based BDF selection
>
> Hi,
>
> On Mon, Oct 26, 2020 at 10:18 PM Rakesh Pillai
> wrote:
> >
> >
> >
> > > -Original Message-
> > > From: Doug Anderson
> > > Sent: Tuesday, October 27, 2020 4:21 AM
> > > To: Rakesh Pillai
> > > Cc: Abhishek Kumar ; Kalle Valo
> > > ; ath10k ; LKML
> > > ; linux-wireless > > wirel...@vger.kernel.org>; Brian Norris
> > > Subject: Re: [PATCH] ath10k: add option for chip-id based BDF selection
> > >
> > > Hi,
> > >
> > > On Sat, Oct 24, 2020 at 9:40 AM Rakesh Pillai
> wrote:
> > > >
> > > > > if (bd_ie_type == ATH10K_BD_IE_BOARD) {
> > > > > + /* With variant and chip id */
> > > > > ret = ath10k_core_create_board_name(ar, boardname,
> > > > > -
> > > > > sizeof(boardname), true);
> > > > > + sizeof(boardname),
> > > > > true, true);
> > > >
> > > > Instead of adding a lot of code to generate a second fallback name, its
> > > better to just modify the condition inside the function
> > > “ath10k_core_create_board_name” to allow the generation of BDF tag
> using
> > > chip id, even “if ar->id.bdf_ext[0] == '\0 “.
> > > >
> > > > This will make sure that the variant string is NULL, and just board-id
> > > > and
> > > chip-id is used. This will help avoid most of the code changes.
> > > > The code would look as shown below
> > > >
> > > > @@ -1493,7 +1493,7 @@ static int
> ath10k_core_create_board_name(struct
> > > ath10k *ar, char *name,
> > > > }
> > > >
> > > > if (ar->id.qmi_ids_valid) {
> > > > - if (with_variant && ar->id.bdf_ext[0] != '\0')
> > > > + if (with_variant)
> > >
> > > Wouldn't the above just be "if (with_chip_id)" instead? ...but yeah,
> > > that would be a cleaner way to do this. Abhishek: do you want to post
> > > a v2?
> >
> >
> > The parameter name passed to this function is "with_variant", since other
> non-qmi targets (eg QCA6174) use this as a flag to just add the variant field.
> > This can be renamed to something meaningful for both qmi and non-qmi
> targets.
>
> I think we still need Abhishek's change to have two booleans passed to
> this function, though, right? Thus, it'll be called 3 times:
>
> * with_chip_id = false, with_variant = false
> * with_chip_id = true, with_variant = true
> * with_chip_id = true, with_variant = false
>
> The two cases you want to combine are both with "with_chip_id = true",
> right? The "with_variant" variable being false will make the variant
> string empty.
I meant that we can use the 4th argument passed to the function "
ath10k_core_create_board_name" (currently named as with_variant) as an
indication to use the BDF name with variant.
But even if with_variant=true, we allow the variant string to be NULL, thereby
allowing us to generate a boardname with the format
"bus=snoc,qmi-board-id=0xab,qmi-chip-id=0xcd"
The combinations of args/variant-strings for generating different board names
will be as follows:
1) with_variant=false :
"bus=snoc,qmi-board-id=0xab"
2) with_variant=true, variant_string=NULL: "
bus=snoc,qmi-board-id=0xab,qmi-chip-id=0xcd"
3) with_variant=true, variant_string="variant_xyz" : "
bus=snoc,qmi-board-id=0xab,qmi-chip-id=0xcd,variant=variant_xyz"
This will minimize the code changes.
>
> -Doug
[PATCH] ath10k: Fix the parsing error in service available event
The wmi service available event has been extended to contain extra 128 bit for new services to be indicated by firmware. Currently the presence of any optional TLVs in the wmi service available event leads to a parsing error with the below error message: ath10k_snoc 1880.wifi: failed to parse svc_avail tlv: -71 The wmi service available event parsing should not return error for the newly added optional TLV. Fix this parsing for service available event message. Tested-on: WCN3990 hw1.0 SNOC Signed-off-by: Rakesh Pillai --- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c b/drivers/net/wireless/ath/ath10k/wmi-tlv.c index 932266d..3b49e29 100644 --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c @@ -1404,9 +1404,12 @@ static int ath10k_wmi_tlv_svc_avail_parse(struct ath10k *ar, u16 tag, u16 len, arg->service_map_ext_len = *(__le32 *)ptr; arg->service_map_ext = ptr + sizeof(__le32); return 0; + case WMI_TLV_TAG_FIRST_ARRAY_ENUM: + return 0; default: break; } + return -EPROTO; } -- 2.7.4
RE: [PATCH] ath10k: add option for chip-id based BDF selection
> -Original Message-
> From: Doug Anderson
> Sent: Tuesday, October 27, 2020 4:21 AM
> To: Rakesh Pillai
> Cc: Abhishek Kumar ; Kalle Valo
> ; ath10k ; LKML
> ; linux-wireless wirel...@vger.kernel.org>; Brian Norris
> Subject: Re: [PATCH] ath10k: add option for chip-id based BDF selection
>
> Hi,
>
> On Sat, Oct 24, 2020 at 9:40 AM Rakesh Pillai wrote:
> >
> > > if (bd_ie_type == ATH10K_BD_IE_BOARD) {
> > > + /* With variant and chip id */
> > > ret = ath10k_core_create_board_name(ar, boardname,
> > > - sizeof(boardname),
> > > true);
> > > + sizeof(boardname), true,
> > > true);
> >
> > Instead of adding a lot of code to generate a second fallback name, its
> better to just modify the condition inside the function
> “ath10k_core_create_board_name” to allow the generation of BDF tag using
> chip id, even “if ar->id.bdf_ext[0] == '\0 “.
> >
> > This will make sure that the variant string is NULL, and just board-id and
> chip-id is used. This will help avoid most of the code changes.
> > The code would look as shown below
> >
> > @@ -1493,7 +1493,7 @@ static int ath10k_core_create_board_name(struct
> ath10k *ar, char *name,
> > }
> >
> > if (ar->id.qmi_ids_valid) {
> > - if (with_variant && ar->id.bdf_ext[0] != '\0')
> > + if (with_variant)
>
> Wouldn't the above just be "if (with_chip_id)" instead? ...but yeah,
> that would be a cleaner way to do this. Abhishek: do you want to post
> a v2?
The parameter name passed to this function is "with_variant", since other
non-qmi targets (eg QCA6174) use this as a flag to just add the variant field.
This can be renamed to something meaningful for both qmi and non-qmi targets.
>
> -Doug
[PATCH v2] ath10k: Use bdf calibration variant for snoc targets
Board Data File (BDF) is loaded upon driver boot-up procedure.
The right board data file is identified using bus and qmi-board-id.
The problem, however, can occur when the (default) board data
file cannot fulfill with the vendor requirements and it is
necessary to use a different board data file.
Also using the chip_id for identifying the board data helps
in dealing with different variants of the board data file based
on the RF card. If the chip_id is not programmed, a default value
of 0xff will be used for parsing the board data file.
Add the support to get the variant field from DTSI and
use this information along with the chip_id to load the vendor
specific BDF.
The device tree requires addition strings to define the variant name
wifi@a00 {
status = "okay";
qcom,ath10k-calibration-variant = "xyz-v2";
};
wifi@a80 {
status = "okay";
qcom,ath10k-calibration-variant = "xyz-v1";
};
This would create the boarddata identifiers for the board-2.bin search
* bus=snoc,qmi-board-id=16,qmi-chip-id=0,variant=xyz-v1
* bus=snoc,qmi-board-id=17,qmi-chip-id=0,variant=xyz-v2
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
Changes from v1:
- Updated the commit text to add details about chip_id used for
loading bdf.
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.c | 18 +-
drivers/net/wireless/ath/ath10k/core.h | 2 ++
drivers/net/wireless/ath/ath10k/qmi.c | 8
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 5f4e121..d73ad60 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1022,7 +1022,7 @@ static int ath10k_core_check_smbios(struct ath10k *ar)
return 0;
}
-static int ath10k_core_check_dt(struct ath10k *ar)
+int ath10k_core_check_dt(struct ath10k *ar)
{
struct device_node *node;
const char *variant = NULL;
@@ -1043,6 +1043,7 @@ static int ath10k_core_check_dt(struct ath10k *ar)
return 0;
}
+EXPORT_SYMBOL(ath10k_core_check_dt);
static int ath10k_download_fw(struct ath10k *ar)
{
@@ -1437,10 +1438,17 @@ static int ath10k_core_create_board_name(struct ath10k
*ar, char *name,
}
if (ar->id.qmi_ids_valid) {
- scnprintf(name, name_len,
- "bus=%s,qmi-board-id=%x",
- ath10k_bus_str(ar->hif.bus),
- ar->id.qmi_board_id);
+ if (with_variant && ar->id.bdf_ext[0] != '\0')
+ scnprintf(name, name_len,
+ "bus=%s,qmi-board-id=%x,qmi-chip-id=%x%s",
+ ath10k_bus_str(ar->hif.bus),
+ ar->id.qmi_board_id, ar->id.qmi_chip_id,
+ variant);
+ else
+ scnprintf(name, name_len,
+ "bus=%s,qmi-board-id=%x",
+ ath10k_bus_str(ar->hif.bus),
+ ar->id.qmi_board_id);
goto out;
}
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 4cf5bd4..b50ab9e 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1076,6 +1076,7 @@ struct ath10k {
bool bmi_ids_valid;
bool qmi_ids_valid;
u32 qmi_board_id;
+ u32 qmi_chip_id;
u8 bmi_board_id;
u8 bmi_eboard_id;
u8 bmi_chip_id;
@@ -1315,6 +1316,7 @@ int ath10k_core_register(struct ath10k *ar,
const struct ath10k_bus_params *bus_params);
void ath10k_core_unregister(struct ath10k *ar);
int ath10k_core_fetch_board_file(struct ath10k *ar, int bd_ie_type);
+int ath10k_core_check_dt(struct ath10k *ar);
void ath10k_core_free_board_files(struct ath10k *ar);
#endif /* _CORE_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/qmi.c
b/drivers/net/wireless/ath/ath10k/qmi.c
index 5468a41..ae6b1f4 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -576,6 +576,8 @@ static int ath10k_qmi_cap_send_sync_msg(struct ath10k_qmi
*qmi)
if (resp->chip_info_valid) {
qmi->chip_info.chip_id = resp->chip_info.chip_id;
qmi->chip_info.chip_family = resp->chip_info.chip_family;
+ } else {
+ qmi->chip_info.chip_id = 0xFF;
}
if (resp->board_info_valid)
@@ -817,12 +819,18 @@ static void ath10k_qmi_event_server_arrive(struct
ath10k_qmi *qmi)
static int ath10k_qmi_fetch_board
RE: [PATCH v2 0/2] ath10k: Fixes during subsystem recovery
> -Original Message- > From: Rakesh Pillai > Sent: Saturday, June 27, 2020 12:24 AM > To: ath...@lists.infradead.org > Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; Rakesh > Pillai > Subject: [PATCH v2 0/2] ath10k: Fixes during subsystem recovery > > This patch series includes some fixes when the device > is in recovery mode, i.e. when the firmware goes down. > > - Pausing TX queues when FW goes down > - Removed unwanted/extra error logging in pkt TX path > - Skipping wait for FW response for delete cmds > - Handling the -ESHUTDOWN error code in case of SSR. > > Rakesh Pillai (2): > ath10k: Pause the tx queues when firmware is down > ath10k: Skip wait for delete response if firmware is down > > drivers/net/wireless/ath/ath10k/core.h | 1 + > drivers/net/wireless/ath/ath10k/mac.c | 36 ++--- > - > drivers/net/wireless/ath/ath10k/snoc.c | 3 +++ > 3 files changed, 28 insertions(+), 12 deletions(-) Hi Kalle, I see that this patch series is in Deferred state. Is there something missing or blocking this ? > > -- > 2.7.4
RE: [PATCH] ath10k: Use bdf calibration variant for snoc targets
> -Original Message-
> From: Kalle Valo
> Sent: Monday, September 7, 2020 9:48 PM
> To: Rakesh Pillai
> Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org;
> ath...@lists.infradead.org
> Subject: Re: [PATCH] ath10k: Use bdf calibration variant for snoc targets
>
> "Rakesh Pillai" writes:
>
> >> > --- a/drivers/net/wireless/ath/ath10k/qmi.c
> >> > +++ b/drivers/net/wireless/ath/ath10k/qmi.c
> >> > @@ -576,6 +576,8 @@ static int
> ath10k_qmi_cap_send_sync_msg(struct
> >> ath10k_qmi *qmi)
> >> > if (resp->chip_info_valid) {
> >> > qmi->chip_info.chip_id = resp->chip_info.chip_id;
> >> > qmi->chip_info.chip_family =
resp->chip_info.chip_family;
> >> > +} else {
> >> > +qmi->chip_info.chip_id = 0xFF;
> >> > }
> >>
> >> So you hard code chip_id to 0xff if it's not valid. Is it 100%
> >> guaranteed that there never will be a chip id with 0xff?
> >
> > 0x0 and 0xff are invalid chip id and are are not used.
> > If the chip_id read fails, we fallback to the default board data.
> > 0xff is used to go to the default board data (Also this is in alignment
with
> > the current implementation of board_id)
> >
> > Does that make sense ?
>
> I'm a bit hesitant, over the years I have seen cases so many cases where
> "foo is not used anywhere" and later that foo is actually used
> somewhere. 0xff means that there's only 254 different values, but I
> guess there are not that many chip ids? So a chip id is very different
> from a board id, right?
Yes that's correct.
Chip id is already being used in case of qca6174 (pci bus)
We are bringing this to snoc bus chipset (WCN3990)
>
> --
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingp
> atches
RE: [PATCH] ath10k: Use bdf calibration variant for snoc targets
Hi Kalle,
> -Original Message-
> From: Kalle Valo
> Sent: Wednesday, September 2, 2020 2:17 PM
> To: Rakesh Pillai
> Cc: ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: Re: [PATCH] ath10k: Use bdf calibration variant for snoc targets
>
> Rakesh Pillai writes:
>
> > Board Data File (BDF) is loaded upon driver boot-up procedure.
> > The right board data file is identified using bus and qmi-board-id.
> >
> > The problem, however, can occur when the (default) board data
> > file cannot fulfill with the vendor requirements and it is
> > necessary to use a different board data file.
> >
> > Add the support to get the variant field from DTSI and
> > use tht information to load the vendor specific BDF.
> >
> > The device tree requires addition strings to define the variant name
> >
> > wifi@a00 {
> > status = "okay";
> > qcom,ath10k-calibration-variant = "xyz-v2";
> > };
> >
> > wifi@a80 {
> > status = "okay";
> > qcom,ath10k-calibration-variant = "xyz-v1";
> > };
> >
> > This would create the boarddata identifiers for the board-2.bin search
> >
> > * bus=snoc,qmi-board-id=16,qmi-chip-id=0,variant=xyz-v1
> > * bus=snoc,qmi-board-id=17,qmi-chip-id=0,variant=xyz-v2
>
> You mention nothing about qmi-chip-id in the commit log. Please document
> what it is and also give some examples what kind of values there can be.
Let me add a bit more details about the chip-id and send v2 for this change.
>
> > --- a/drivers/net/wireless/ath/ath10k/qmi.c
> > +++ b/drivers/net/wireless/ath/ath10k/qmi.c
> > @@ -576,6 +576,8 @@ static int ath10k_qmi_cap_send_sync_msg(struct
> ath10k_qmi *qmi)
> > if (resp->chip_info_valid) {
> > qmi->chip_info.chip_id = resp->chip_info.chip_id;
> > qmi->chip_info.chip_family = resp->chip_info.chip_family;
> > + } else {
> > + qmi->chip_info.chip_id = 0xFF;
> > }
>
> So you hard code chip_id to 0xff if it's not valid. Is it 100%
> guaranteed that there never will be a chip id with 0xff?
0x0 and 0xff are invalid chip id and are are not used.
If the chip_id read fails, we fallback to the default board data.
0xff is used to go to the default board data (Also this is in alignment with
the current implementation of board_id)
Does that make sense ?
>
> >
> > if (resp->board_info_valid)
> > @@ -817,12 +819,18 @@ static void
> ath10k_qmi_event_server_arrive(struct ath10k_qmi *qmi)
> > static int ath10k_qmi_fetch_board_file(struct ath10k_qmi *qmi)
> > {
> > struct ath10k *ar = qmi->ar;
> > + int ret;
> >
> > ar->hif.bus = ATH10K_BUS_SNOC;
> > ar->id.qmi_ids_valid = true;
> > ar->id.qmi_board_id = qmi->board_info.board_id;
> > + ar->id.qmi_chip_id = qmi->chip_info.chip_id;
>
> To me a safer, and cleaner, option would be to have
> ar->id.qmi_chip_id_valid, and only add qmi-chip-id=%x to the board id if
> qmi_chip_id_valid is true. That way there's not this magic 0xff value
> hardcoded anywhere.
>
> --
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingp
> atches
RE: [PATCH] ath10k: Fix the size used in a 'dma_free_coherent()' call in an error handling path
> -Original Message-
> From: Christophe JAILLET
> Sent: Sunday, August 2, 2020 5:52 PM
> To: kv...@codeaurora.org; da...@davemloft.net; k...@kernel.org;
> pill...@codeaurora.org
> Cc: ath...@lists.infradead.org; linux-wirel...@vger.kernel.org;
> net...@vger.kernel.org; linux-kernel@vger.kernel.org; kernel-
> janit...@vger.kernel.org; Christophe JAILLET
>
> Subject: [PATCH] ath10k: Fix the size used in a 'dma_free_coherent()' call
in
> an error handling path
>
> Update the size used in 'dma_free_coherent()' in order to match the one
> used in the corresponding 'dma_alloc_coherent()'.
>
> Fixes: 1863008369ae ("ath10k: fix shadow register implementation for
> WCN3990")
> Signed-off-by: Christophe JAILLET
> ---
> This patch looks obvious to me, but commit 1863008369ae looks also simple.
> So it is surprising that such a "typo" slipped in.
Reviewed-by: Rakesh Pillai
> ---
> drivers/net/wireless/ath/ath10k/ce.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/ce.c
> b/drivers/net/wireless/ath/ath10k/ce.c
> index 294fbc1e89ab..e6e0284e4783 100644
> --- a/drivers/net/wireless/ath/ath10k/ce.c
> +++ b/drivers/net/wireless/ath/ath10k/ce.c
> @@ -1555,7 +1555,7 @@ ath10k_ce_alloc_src_ring(struct ath10k *ar,
> unsigned int ce_id,
> ret = ath10k_ce_alloc_shadow_base(ar, src_ring, nentries);
> if (ret) {
> dma_free_coherent(ar->dev,
> - (nentries * sizeof(struct
> ce_desc_64) +
> + (nentries * sizeof(struct ce_desc)
+
> CE_DESC_RING_ALIGN),
> src_ring-
> >base_addr_owner_space_unaligned,
> base_addr);
> --
> 2.25.1
RE: [PATCH v2 1/3] ath10k: Add history for tracking certain events
> -Original Message-
> From: Ben Greear
> Sent: Saturday, August 1, 2020 12:08 AM
> To: Rakesh Pillai ; ath...@lists.infradead.org
> Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org;
> kv...@codeaurora.org; da...@davemloft.net; k...@kernel.org;
> net...@vger.kernel.org
> Subject: Re: [PATCH v2 1/3] ath10k: Add history for tracking certain events
>
> On 7/31/20 11:27 AM, Rakesh Pillai wrote:
> > Add history for tracking the below events
> > - register read
> > - register write
> > - IRQ trigger
> > - NAPI poll
> > - CE service
> > - WMI cmd
> > - WMI event
> > - WMI tx completion
> >
> > This will help in debugging any crash or any
> > improper behaviour.
> >
> > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> >
> > Signed-off-by: Rakesh Pillai
> > ---
> > drivers/net/wireless/ath/ath10k/ce.c | 1 +
> > drivers/net/wireless/ath/ath10k/core.h| 74 +
> > drivers/net/wireless/ath/ath10k/debug.c | 133
> ++
> > drivers/net/wireless/ath/ath10k/debug.h | 74 +
> > drivers/net/wireless/ath/ath10k/snoc.c| 15 +++-
> > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
> > drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
> > 7 files changed, 307 insertions(+), 1 deletion(-)
> >
>
> > +void ath10k_record_wmi_event(struct ath10k *ar, enum
> ath10k_wmi_type type,
> > +u32 id, unsigned char *data)
> > +{
> > + struct ath10k_wmi_event_entry *entry;
> > + u32 idx;
> > +
> > + if (type == ATH10K_WMI_EVENT) {
> > + if (!ar->wmi_event_history.record)
> > + return;
>
> This check above is duplicated below, add it once at top of the method
> instead.
The same function is used to record WMI events and CMD, which are stored in
different memory locations.
Hence the check " if (type == ATH10K_WMI_EVENT) {" is necessary.
>
> > +
> > + spin_lock_bh(>wmi_event_history.hist_lock);
> > + idx = ath10k_core_get_next_idx(
> >reg_access_history.index,
> > + ar-
> >wmi_event_history.max_entries);
> > + spin_unlock_bh(>wmi_event_history.hist_lock);
> > + entry = >wmi_event_history.record[idx];
> > + } else {
> > + if (!ar->wmi_cmd_history.record)
> > + return;
> > +
> > + spin_lock_bh(>wmi_cmd_history.hist_lock);
> > + idx = ath10k_core_get_next_idx(
> >reg_access_history.index,
> > + ar-
> >wmi_cmd_history.max_entries);
> > + spin_unlock_bh(>wmi_cmd_history.hist_lock);
> > + entry = >wmi_cmd_history.record[idx];
> > + }
> > +
> > + entry->timestamp = ath10k_core_get_timestamp();
> > + entry->cpu_id = smp_processor_id();
> > + entry->type = type;
> > + entry->id = id;
> > + memcpy(>data, data + 4, ATH10K_WMI_DATA_LEN);
> > +}
> > +EXPORT_SYMBOL(ath10k_record_wmi_event);
>
> > @@ -1660,6 +1668,11 @@ static int ath10k_snoc_probe(struct
> platform_device *pdev)
> > ar->ce_priv = _snoc->ce;
> > msa_size = drv_data->msa_size;
> >
> > + ath10k_core_reg_access_history_init(ar,
> ATH10K_REG_ACCESS_HISTORY_MAX);
> > + ath10k_core_wmi_event_history_init(ar,
> ATH10K_WMI_EVENT_HISTORY_MAX);
> > + ath10k_core_wmi_cmd_history_init(ar,
> ATH10K_WMI_CMD_HISTORY_MAX);
> > + ath10k_core_ce_event_history_init(ar,
> ATH10K_CE_EVENT_HISTORY_MAX);
>
> Maybe only enable this once user turns it on? It sucks up a bit of memory?
This memory will be allocated only if the history is enabled via module param,
else the function just returns 0.
>
> > +
> > ath10k_snoc_quirks_init(ar);
> >
> > ret = ath10k_snoc_resource_init(ar);
> > diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > index 932266d..9df5748 100644
> > --- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> > @@ -627,6 +627,7 @@ static void ath10k_wmi_tlv_op_rx(struct ath10k *ar,
> struct sk_buff *skb)
> > if (skb_pull(skb, sizeof(struct wmi_cmd_hdr)) == NULL)
> > goto out;
> >
> > + ath10k_record_wmi_event(ar, ATH10K_WMI_EVENT, id, skb->data);
> > trace_ath10k_wmi_event(ar, id, skb->data, skb
RE: [PATCH v2 0/3]
> -Original Message- > From: Florian Fainelli > Sent: Saturday, August 1, 2020 12:17 AM > To: Rakesh Pillai ; ath...@lists.infradead.org > Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; > kv...@codeaurora.org; da...@davemloft.net; k...@kernel.org; > net...@vger.kernel.org > Subject: Re: [PATCH v2 0/3] > > On 7/31/20 11:27 AM, Rakesh Pillai wrote: > > The history recording will be compiled only if > > ATH10K_DEBUG is enabled, and also enabled via > > the module parameter. Once the history recording > > is enabled via module parameter, it can be enabled > > or disabled runtime via debugfs. > > Why not use trace prints and retrieving them via the function tracer? > This seems very ad-hoc. Tracing needs to be enabled to capture the events. But these events can be turned on in some kind of a debug build and capture the history to help us debug in case there is a crash. It wont even allocate memory if not enabled via module parameter. > > > > > --- > > Changes from v1: > > - Add module param and debugfs to enable/disable history recording. > > > > Rakesh Pillai (3): > > ath10k: Add history for tracking certain events > > ath10k: Add module param to enable history > > ath10k: Add debugfs support to enable event history > > > > drivers/net/wireless/ath/ath10k/ce.c | 1 + > > drivers/net/wireless/ath/ath10k/core.c| 3 + > > drivers/net/wireless/ath/ath10k/core.h| 82 > > drivers/net/wireless/ath/ath10k/debug.c | 207 > ++ > > drivers/net/wireless/ath/ath10k/debug.h | 75 +++ > > drivers/net/wireless/ath/ath10k/snoc.c| 15 ++- > > drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 + > > drivers/net/wireless/ath/ath10k/wmi.c | 10 ++ > > 8 files changed, 393 insertions(+), 1 deletion(-) > > > > > -- > Florian
[PATCH v2 3/3] ath10k: Add debugfs support to enable event history
Add the support to enable/disable the recording of
debug events history.
The enable/disable of the history from debugfs will
not make any affect if its not enabled via module
parameter.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/debug.c | 56 +
1 file changed, 56 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/debug.c
b/drivers/net/wireless/ath/ath10k/debug.c
index 5d08652..6785fae 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -610,6 +610,59 @@ static const struct file_operations fops_simulate_fw_crash
= {
.llseek = default_llseek,
};
+static ssize_t ath10k_read_history_enable(struct file *file,
+ char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ const char buf[] =
+ "To enable recording of certain event history, write to this
file with the enable mask\n"
+ "BIT(0): Enable Reg Access history\n"
+ " - Register write events\n"
+ " - Register read events\n"
+ "BIT(1): Enable CE events history\n"
+ " - ATH10K_IRQ_TRIGGER event\n"
+ " - ATH10K_NAPI_POLL event\n"
+ " - ATH10K_CE_SERVICE event\n"
+ " - ATH10K_NAPI_COMPLETE event\n"
+ " - ATH10K_NAPI_RESCHED event\n"
+ " - ATH10K_IRQ_SUMMARY event\n"
+ "BIT(2): Enable WMI CMD history\n"
+ " - WMI CMD event\n"
+ " - WMI CMD TX completion event\n"
+ "BIT(3): Enable WMI events history\n"
+ " - WMI Events event\n";
+
+ return simple_read_from_buffer(user_buf, count, ppos, buf, strlen(buf));
+}
+
+static ssize_t ath10k_write_history_enable(struct file *file,
+ const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ u32 history_enable_mask;
+ int i, ret;
+
+ ret = kstrtou32_from_user(user_buf, count, 0, _enable_mask);
+ if (ret)
+ return ret;
+
+ for (i = 0; i < ATH10K_HISTORY_MAX; i++)
+ if (history_enable_mask & BIT(i))
+ set_bit(i, _history_enable_mask);
+ else
+ clear_bit(i, _history_enable_mask);
+
+ return count;
+}
+
+static const struct file_operations fops_history_enable = {
+ .read = ath10k_read_history_enable,
+ .write = ath10k_write_history_enable,
+ .open = simple_open,
+ .owner = THIS_MODULE,
+ .llseek = default_llseek,
+};
+
static ssize_t ath10k_read_chip_id(struct file *file, char __user *user_buf,
size_t count, loff_t *ppos)
{
@@ -2658,6 +2711,9 @@ int ath10k_debug_register(struct ath10k *ar)
debugfs_create_file("reset_htt_stats", 0200, ar->debug.debugfs_phy, ar,
_reset_htt_stats);
+ debugfs_create_file("history_enable", 0644, ar->debug.debugfs_phy, ar,
+ _history_enable);
+
return 0;
}
--
2.7.4
[PATCH v2 1/3] ath10k: Add history for tracking certain events
Add history for tracking the below events
- register read
- register write
- IRQ trigger
- NAPI poll
- CE service
- WMI cmd
- WMI event
- WMI tx completion
This will help in debugging any crash or any
improper behaviour.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/ce.c | 1 +
drivers/net/wireless/ath/ath10k/core.h| 74 +
drivers/net/wireless/ath/ath10k/debug.c | 133 ++
drivers/net/wireless/ath/ath10k/debug.h | 74 +
drivers/net/wireless/ath/ath10k/snoc.c| 15 +++-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
7 files changed, 307 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c
b/drivers/net/wireless/ath/ath10k/ce.c
index 84ec80c..0f541de 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -1299,6 +1299,7 @@ void ath10k_ce_per_engine_service(struct ath10k *ar,
unsigned int ce_id)
struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
u32 ctrl_addr = ce_state->ctrl_addr;
+ ath10k_record_ce_event(ar, ATH10K_CE_SERVICE, ce_id);
/*
* Clear before handling
*
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 5c18f6c..46bd5aa 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -970,6 +970,75 @@ struct ath10k_bus_params {
bool hl_msdu_ids;
};
+#define ATH10K_REG_ACCESS_HISTORY_MAX 512
+#define ATH10K_CE_EVENT_HISTORY_MAX1024
+#define ATH10K_WMI_EVENT_HISTORY_MAX 512
+#define ATH10K_WMI_CMD_HISTORY_MAX 256
+
+#define ATH10K_WMI_DATA_LEN16
+
+enum ath10k_ce_event {
+ ATH10K_IRQ_TRIGGER,
+ ATH10K_NAPI_POLL,
+ ATH10K_CE_SERVICE,
+ ATH10K_NAPI_COMPLETE,
+ ATH10K_NAPI_RESCHED,
+ ATH10K_IRQ_SUMMARY,
+};
+
+enum ath10k_wmi_type {
+ ATH10K_WMI_EVENT,
+ ATH10K_WMI_CMD,
+ ATH10K_WMI_TX_COMPL,
+};
+
+struct ath10k_reg_access_entry {
+ u32 cpu_id;
+ bool write;
+ u32 offset;
+ u32 val;
+ u64 timestamp;
+};
+
+struct ath10k_wmi_event_entry {
+ u32 cpu_id;
+ enum ath10k_wmi_type type;
+ u32 id;
+ u64 timestamp;
+ unsigned char data[ATH10K_WMI_DATA_LEN];
+};
+
+struct ath10k_ce_event_entry {
+ u32 cpu_id;
+ enum ath10k_ce_event event_type;
+ u32 ce_id;
+ u64 timestamp;
+};
+
+struct ath10k_wmi_event_history {
+ struct ath10k_wmi_event_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing wmi event history */
+ spinlock_t hist_lock;
+};
+
+struct ath10k_ce_event_history {
+ struct ath10k_ce_event_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing ce event history */
+ spinlock_t hist_lock;
+};
+
+struct ath10k_reg_access_history {
+ struct ath10k_reg_access_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing register access history */
+ spinlock_t hist_lock;
+};
+
struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
@@ -1261,6 +1330,11 @@ struct ath10k {
bool coex_support;
int coex_gpio_pin;
+ struct ath10k_reg_access_history reg_access_history;
+ struct ath10k_ce_event_history ce_event_history;
+ struct ath10k_wmi_event_history wmi_event_history;
+ struct ath10k_wmi_event_history wmi_cmd_history;
+
/* must be last */
u8 drv_priv[] __aligned(sizeof(void *));
};
diff --git a/drivers/net/wireless/ath/ath10k/debug.c
b/drivers/net/wireless/ath/ath10k/debug.c
index e8250a6..9105b0b 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2722,4 +2722,137 @@ void ath10k_dbg_dump(struct ath10k *ar,
}
EXPORT_SYMBOL(ath10k_dbg_dump);
+int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->reg_access_history.record = vzalloc(max_entries *
+ sizeof(struct
ath10k_reg_access_entry));
+ if (!ar->reg_access_history.record)
+ return -ENOMEM;
+
+ ar->reg_access_history.max_entries = max_entries;
+ atomic_set(>reg_access_history.index, 0);
+ spin_lock_init(>reg_access_history.hist_lock);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_reg_access_history_init);
+
+int ath10k_core_wmi_cmd_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->wmi_cmd_history.record = vzalloc(max_entries *
+sizeof(struct
ath10k_wmi_event_entry));
+ if (!ar->wmi_cmd_history.record)
+ return -ENOMEM;
+
+ ar->wmi_cmd_
[PATCH v2 0/3]
The history recording will be compiled only if ATH10K_DEBUG is enabled, and also enabled via the module parameter. Once the history recording is enabled via module parameter, it can be enabled or disabled runtime via debugfs. --- Changes from v1: - Add module param and debugfs to enable/disable history recording. Rakesh Pillai (3): ath10k: Add history for tracking certain events ath10k: Add module param to enable history ath10k: Add debugfs support to enable event history drivers/net/wireless/ath/ath10k/ce.c | 1 + drivers/net/wireless/ath/ath10k/core.c| 3 + drivers/net/wireless/ath/ath10k/core.h| 82 drivers/net/wireless/ath/ath10k/debug.c | 207 ++ drivers/net/wireless/ath/ath10k/debug.h | 75 +++ drivers/net/wireless/ath/ath10k/snoc.c| 15 ++- drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 + drivers/net/wireless/ath/ath10k/wmi.c | 10 ++ 8 files changed, 393 insertions(+), 1 deletion(-) -- 2.7.4
[PATCH v2 2/3] ath10k: Add module param to enable history
Add a module param to enable recording history
of certain debug events. This module parameter
is a mask of the different history recording to
be enabled.
The memory for recording the history will not be
allocated if its not enabled via module parameter.
This is to avoid unnecessary memory allocation when
recording the history is not needed.
To enable all the history recording, the driver
should be loaded as below
"insmod ath10k_core.ko history_enable_mask=0xf"
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.c | 3 +++
drivers/net/wireless/ath/ath10k/core.h | 8
drivers/net/wireless/ath/ath10k/debug.c | 26 ++
drivers/net/wireless/ath/ath10k/debug.h | 1 +
4 files changed, 34 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 9104496..f91a9d0 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -29,6 +29,7 @@
unsigned int ath10k_debug_mask;
EXPORT_SYMBOL(ath10k_debug_mask);
+unsigned long ath10k_history_enable_mask;
static unsigned int ath10k_cryptmode_param;
static bool uart_print;
static bool skip_otp;
@@ -46,6 +47,7 @@ module_param(skip_otp, bool, 0644);
module_param(rawmode, bool, 0644);
module_param(fw_diag_log, bool, 0644);
module_param_named(coredump_mask, ath10k_coredump_mask, ulong, 0444);
+module_param_named(history_enable_mask, ath10k_history_enable_mask, ulong,
0444);
MODULE_PARM_DESC(debug_mask, "Debugging mask");
MODULE_PARM_DESC(uart_print, "Uart target debugging");
@@ -54,6 +56,7 @@ MODULE_PARM_DESC(cryptmode, "Crypto mode: 0-hardware,
1-software");
MODULE_PARM_DESC(rawmode, "Use raw 802.11 frame datapath");
MODULE_PARM_DESC(coredump_mask, "Bitfield of what to include in firmware crash
file");
MODULE_PARM_DESC(fw_diag_log, "Diag based fw log debugging");
+MODULE_PARM_DESC(history_enable_mask, "Enable events history recording");
static const struct ath10k_hw_params ath10k_hw_params_list[] = {
{
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 46bd5aa..ce429df 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -977,6 +977,14 @@ struct ath10k_bus_params {
#define ATH10K_WMI_DATA_LEN16
+enum ath10k_history {
+ ATH10K_REG_ACCESS_HISTORY,
+ ATH10K_CE_EVENTS_HISTORY,
+ ATH10K_WMI_CMD_HISTORY,
+ ATH10K_WMI_EVENTS_HISTORY,
+ ATH10K_HISTORY_MAX,
+};
+
enum ath10k_ce_event {
ATH10K_IRQ_TRIGGER,
ATH10K_NAPI_POLL,
diff --git a/drivers/net/wireless/ath/ath10k/debug.c
b/drivers/net/wireless/ath/ath10k/debug.c
index 9105b0b..5d08652 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2724,6 +2724,9 @@ EXPORT_SYMBOL(ath10k_dbg_dump);
int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_REG_ACCESS_HISTORY, _history_enable_mask))
+ return 0;
+
ar->reg_access_history.record = vzalloc(max_entries *
sizeof(struct
ath10k_reg_access_entry));
if (!ar->reg_access_history.record)
@@ -2739,6 +2742,9 @@ EXPORT_SYMBOL(ath10k_core_reg_access_history_init);
int ath10k_core_wmi_cmd_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_WMI_CMD_HISTORY, _history_enable_mask))
+ return 0;
+
ar->wmi_cmd_history.record = vzalloc(max_entries *
sizeof(struct
ath10k_wmi_event_entry));
if (!ar->wmi_cmd_history.record)
@@ -2754,6 +2760,9 @@ EXPORT_SYMBOL(ath10k_core_wmi_cmd_history_init);
int ath10k_core_wmi_event_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_WMI_EVENTS_HISTORY, _history_enable_mask))
+ return 0;
+
ar->wmi_event_history.record = vzalloc(max_entries *
sizeof(struct
ath10k_wmi_event_entry));
if (!ar->wmi_event_history.record)
@@ -2769,6 +2778,9 @@ EXPORT_SYMBOL(ath10k_core_wmi_event_history_init);
int ath10k_core_ce_event_history_init(struct ath10k *ar, u32 max_entries)
{
+ if (!test_bit(ATH10K_CE_EVENTS_HISTORY, _history_enable_mask))
+ return 0;
+
ar->ce_event_history.record = vzalloc(max_entries *
sizeof(struct
ath10k_ce_event_entry));
if (!ar->ce_event_history.record)
@@ -2787,7 +2799,8 @@ void ath10k_record_reg_access(struct ath10k *ar, u32
offset, u32 val, bool write
struct ath10k_reg_access_entry *entry;
u32 idx;
- if (!ar->reg_acces
RE: [RFC 0/7] Add support to process rx packets in thread
> -Original Message- > From: David Laight > Sent: Sunday, July 26, 2020 4:46 PM > To: 'Sebastian Gottschall' ; Hillf Danton > > Cc: Andrew Lunn ; Rakesh Pillai ; > net...@vger.kernel.org; linux-wirel...@vger.kernel.org; linux- > ker...@vger.kernel.org; ath...@lists.infradead.org; > diand...@chromium.org; Markus Elfring ; > evgr...@chromium.org; k...@kernel.org; johan...@sipsolutions.net; > da...@davemloft.net; kv...@codeaurora.org > Subject: RE: [RFC 0/7] Add support to process rx packets in thread > > From: Sebastian Gottschall > > Sent: 25 July 2020 16:42 > > >> i agree. i just can say that i tested this patch recently due this > > >> discussion here. and it can be changed by sysfs. but it doesnt work for > > >> wifi drivers which are mainly using dummy netdev devices. for this i > > >> made a small patch to get them working using napi_set_threaded > manually > > >> hardcoded in the drivers. (see patch bellow) > > > > By CONFIG_THREADED_NAPI, there is no need to consider what you did > here > > > in the napi core because device drivers know better and are responsible > > > for it before calling napi_schedule(n). > > > yeah. but that approach will not work for some cases. some stupid > > drivers are using locking context in the napi poll function. > > in that case the performance will runto shit. i discovered this with the > > mvneta eth driver (marvell) and mt76 tx polling (rx works) > > for mvneta is will cause very high latencies and packet drops. for mt76 > > it causes packet stop. doesnt work simply (on all cases no crashes) > > so the threading will only work for drivers which are compatible with > > that approach. it cannot be used as drop in replacement from my point of > > view. > > its all a question of the driver design > > Why should it make (much) difference whether the napi callbacks (etc) > are done in the context of the interrupted process or that of a > specific kernel thread. > The process flags (or whatever) can even be set so that it appears > to be the expected 'softint' context. > > In any case running NAPI from a thread will just show up the next > piece of code that runs for ages in softint context. > I think I've seen the tail end of memory being freed under rcu > finally happening under softint and taking absolutely ages. > > David > Hi All, Is the threaded NAPI change posted to kernel ? Is the conclusion of this discussion that " we cannot use threads for processing packets " ?? > - > Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, > MK1 1PT, UK > Registration No: 1397386 (Wales)
RE: [RFC 1/7] mac80211: Add check for napi handle before WARN_ON
> -Original Message- > From: Rakesh Pillai > Sent: Friday, July 24, 2020 11:51 AM > To: 'Johannes Berg' ; > 'ath...@lists.infradead.org' > Cc: 'linux-wirel...@vger.kernel.org' ; > 'linux-kernel@vger.kernel.org' ; > 'kv...@codeaurora.org' ; 'da...@davemloft.net' > ; 'k...@kernel.org' ; > 'net...@vger.kernel.org' ; > 'diand...@chromium.org' ; > 'evgr...@chromium.org' > Subject: RE: [RFC 1/7] mac80211: Add check for napi handle before > WARN_ON > > > > > -Original Message- > > From: Johannes Berg > > Sent: Friday, July 24, 2020 1:37 AM > > To: Rakesh Pillai ; ath...@lists.infradead.org > > Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; > > kv...@codeaurora.org; da...@davemloft.net; k...@kernel.org; > > net...@vger.kernel.org; diand...@chromium.org; > evgr...@chromium.org > > Subject: Re: [RFC 1/7] mac80211: Add check for napi handle before > > WARN_ON > > > > On Thu, 2020-07-23 at 23:56 +0530, Rakesh Pillai wrote: > > > > > > > - WARN_ON_ONCE(softirq_count() == 0); > > > > > + WARN_ON_ONCE(napi && softirq_count() == 0); > > > > > > > > FWIW, I'm pretty sure this is incorrect - we make assumptions on > > > > softirqs being disabled in mac80211 for serialization and in place of > > > > some locking, I believe. > > > > > > > > > > I checked this, but let me double confirm. > > > But after this change, no packet is submitted from driver in a softirq > > context. > > > So ideally this should take care of serialization. > > > > I'd guess that we have some reliance on BHs already being disabled, for > > things like u64 sync updates, or whatnot. I mean, we did "rx_ni()" for a > > reason ... Maybe lockdep can help catch some of the issues. > > > > But couldn't you be in a thread and have BHs disabled too? > > This would ideally beat the purpose and possibly hurt the other subsystems > running on the same core. > Hi Johannes, We do have the usage of napi_gro_receive and netif_receive_skb in mac80211. /* deliver to local stack */ if (rx->napi) napi_gro_receive(rx->napi, skb); else netif_receive_skb(skb); Also all the rx_handlers are called under the " rx->local->rx_path_lock" lock. Is the BH disable still required ? > > > > johannes
RE: [RFC 1/7] mac80211: Add check for napi handle before WARN_ON
> -Original Message- > From: Johannes Berg > Sent: Friday, July 24, 2020 1:37 AM > To: Rakesh Pillai ; ath...@lists.infradead.org > Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; > kv...@codeaurora.org; da...@davemloft.net; k...@kernel.org; > net...@vger.kernel.org; diand...@chromium.org; evgr...@chromium.org > Subject: Re: [RFC 1/7] mac80211: Add check for napi handle before > WARN_ON > > On Thu, 2020-07-23 at 23:56 +0530, Rakesh Pillai wrote: > > > > > - WARN_ON_ONCE(softirq_count() == 0); > > > > + WARN_ON_ONCE(napi && softirq_count() == 0); > > > > > > FWIW, I'm pretty sure this is incorrect - we make assumptions on > > > softirqs being disabled in mac80211 for serialization and in place of > > > some locking, I believe. > > > > > > > I checked this, but let me double confirm. > > But after this change, no packet is submitted from driver in a softirq > context. > > So ideally this should take care of serialization. > > I'd guess that we have some reliance on BHs already being disabled, for > things like u64 sync updates, or whatnot. I mean, we did "rx_ni()" for a > reason ... Maybe lockdep can help catch some of the issues. > > But couldn't you be in a thread and have BHs disabled too? This would ideally beat the purpose and possibly hurt the other subsystems running on the same core. > > johannes
RE: [RFC 0/7] Add support to process rx packets in thread
> -Original Message- > From: Florian Fainelli > Sent: Friday, July 24, 2020 12:33 AM > To: Rakesh Pillai ; 'Andrew Lunn' > > Cc: ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > ker...@vger.kernel.org; kv...@codeaurora.org; johan...@sipsolutions.net; > da...@davemloft.net; k...@kernel.org; net...@vger.kernel.org; > diand...@chromium.org; evgr...@chromium.org > Subject: Re: [RFC 0/7] Add support to process rx packets in thread > > On 7/23/20 11:21 AM, Rakesh Pillai wrote: > > > > > >> -Original Message- > >> From: Florian Fainelli > >> Sent: Tuesday, July 21, 2020 11:35 PM > >> To: Andrew Lunn ; Rakesh Pillai > > >> Cc: ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > >> ker...@vger.kernel.org; kv...@codeaurora.org; > johan...@sipsolutions.net; > >> da...@davemloft.net; k...@kernel.org; net...@vger.kernel.org; > >> diand...@chromium.org; evgr...@chromium.org > >> Subject: Re: [RFC 0/7] Add support to process rx packets in thread > >> > >> On 7/21/20 10:25 AM, Andrew Lunn wrote: > >>> On Tue, Jul 21, 2020 at 10:44:19PM +0530, Rakesh Pillai wrote: > >>>> NAPI gets scheduled on the CPU core which got the > >>>> interrupt. The linux scheduler cannot move it to a > >>>> different core, even if the CPU on which NAPI is running > >>>> is heavily loaded. This can lead to degraded wifi > >>>> performance when running traffic at peak data rates. > >>>> > >>>> A thread on the other hand can be moved to different > >>>> CPU cores, if the one on which its running is heavily > >>>> loaded. During high incoming data traffic, this gives > >>>> better performance, since the thread can be moved to a > >>>> less loaded or sometimes even a more powerful CPU core > >>>> to account for the required CPU performance in order > >>>> to process the incoming packets. > >>>> > >>>> This patch series adds the support to use a high priority > >>>> thread to process the incoming packets, as opposed to > >>>> everything being done in NAPI context. > >>> > >>> I don't see why this problem is limited to the ath10k driver. I expect > >>> it applies to all drivers using NAPI. So shouldn't you be solving this > >>> in the NAPI core? Allow a driver to request the NAPI core uses a > >>> thread? > >> > >> What's more, you should be able to configure interrupt affinity to steer > >> RX processing onto a desired CPU core, is not that working for you > >> somehow? > > > > Hi Florian, > > Yes, the affinity of IRQ does work for me. > > But the affinity of IRQ does not happen runtime based on load. > > It can if you also run irqbalance. Hi Florian, Is it some kernel feature ? Sorry I am not aware of this ? I know it can be done in userspace. > -- > Florian
RE: [RFC 7/7] ath10k: Handle rx thread suspend and resume
> -Original Message-
> From: Sebastian Gottschall
> Sent: Friday, July 24, 2020 4:36 AM
> To: Rakesh Pillai ; ath...@lists.infradead.org
> Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org;
> kv...@codeaurora.org; johan...@sipsolutions.net; da...@davemloft.net;
> k...@kernel.org; net...@vger.kernel.org; diand...@chromium.org;
> evgr...@chromium.org
> Subject: Re: [RFC 7/7] ath10k: Handle rx thread suspend and resume
>
> your patch seem to only affect the WCN3990 chipset. all other ath10k
> supported chipset are not supported here. so you see a chance to
> implement this more generic?
>
> Sebastian
Hi Sebastian,
A generic core for handling threads is added with this patchset.
So the handling of rx packet processing in thread can always be extended to
other targets, if they wish so.
The thread related APIs are in core.c which gives all the other targets access
to these APIs for using them.
Thanks,
Rakesh Pillai.
>
> Am 21.07.2020 um 19:14 schrieb Rakesh Pillai:
> > During the system suspend or resume, the rx thread
> > also needs to be suspended or resume respectively.
> >
> > Handle the rx thread as well during the system
> > suspend and resume.
> >
> > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> >
> > Signed-off-by: Rakesh Pillai
> > ---
> > drivers/net/wireless/ath/ath10k/core.c | 23 ++
> > drivers/net/wireless/ath/ath10k/core.h | 5
> > drivers/net/wireless/ath/ath10k/snoc.c | 44
> +-
> > 3 files changed, 71 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/core.c
> b/drivers/net/wireless/ath/ath10k/core.c
> > index 4064fa2..b82b355 100644
> > --- a/drivers/net/wireless/ath/ath10k/core.c
> > +++ b/drivers/net/wireless/ath/ath10k/core.c
> > @@ -668,6 +668,27 @@ static unsigned int
> ath10k_core_get_fw_feature_str(char *buf,
> > return scnprintf(buf, buf_len, "%s",
> ath10k_core_fw_feature_str[feat]);
> > }
> >
> > +int ath10k_core_thread_suspend(struct ath10k_thread *thread)
> > +{
> > + ath10k_dbg(thread->ar, ATH10K_DBG_BOOT, "Suspending thread
> %s\n",
> > + thread->name);
> > + set_bit(ATH10K_THREAD_EVENT_SUSPEND, thread->event_flags);
> > + wait_for_completion(>suspend);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(ath10k_core_thread_suspend);
> > +
> > +int ath10k_core_thread_resume(struct ath10k_thread *thread)
> > +{
> > + ath10k_dbg(thread->ar, ATH10K_DBG_BOOT, "Resuming thread
> %s\n",
> > + thread->name);
> > + complete(>resume);
> > +
> > + return 0;
> > +}
> > +EXPORT_SYMBOL(ath10k_core_thread_resume);
> > +
> > void ath10k_core_thread_post_event(struct ath10k_thread *thread,
> >enum ath10k_thread_events event)
> > {
> > @@ -700,6 +721,8 @@ int ath10k_core_thread_init(struct ath10k *ar,
> >
> > init_waitqueue_head(>wait_q);
> > init_completion(>shutdown);
> > + init_completion(>suspend);
> > + init_completion(>resume);
> > memcpy(thread->name, thread_name,
> ATH10K_THREAD_NAME_SIZE_MAX);
> > clear_bit(ATH10K_THREAD_EVENT_SHUTDOWN, thread-
> >event_flags);
> > ath10k_info(ar, "Starting thread %s\n", thread_name);
> > diff --git a/drivers/net/wireless/ath/ath10k/core.h
> b/drivers/net/wireless/ath/ath10k/core.h
> > index 596d31b..df65e75 100644
> > --- a/drivers/net/wireless/ath/ath10k/core.h
> > +++ b/drivers/net/wireless/ath/ath10k/core.h
> > @@ -976,6 +976,7 @@ enum ath10k_thread_events {
> > ATH10K_THREAD_EVENT_SHUTDOWN,
> > ATH10K_THREAD_EVENT_RX_POST,
> > ATH10K_THREAD_EVENT_TX_POST,
> > + ATH10K_THREAD_EVENT_SUSPEND,
> > ATH10K_THREAD_EVENT_MAX,
> > };
> >
> > @@ -983,6 +984,8 @@ struct ath10k_thread {
> > struct ath10k *ar;
> > struct task_struct *task;
> > struct completion shutdown;
> > + struct completion suspend;
> > + struct completion resume;
> > wait_queue_head_t wait_q;
> > DECLARE_BITMAP(event_flags, ATH10K_THREAD_EVENT_MAX);
> > char name[ATH10K_THREAD_NAME_SIZE_MAX];
> > @@ -1296,6 +1299,8 @@ static inline bool
> ath10k_peer_stats_enabled(struct ath10k *ar)
> >
> > extern unsigned long ath10k_coredump_mask;
> >
> > +int ath10k_core_thread_suspend(struct ath10k_thread *thread);
> > +int ath10k_core_thread_
RE: [RFC 1/7] mac80211: Add check for napi handle before WARN_ON
> -Original Message- > From: Johannes Berg > Sent: Wednesday, July 22, 2020 6:26 PM > To: Rakesh Pillai ; ath...@lists.infradead.org > Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org; > kv...@codeaurora.org; da...@davemloft.net; k...@kernel.org; > net...@vger.kernel.org; diand...@chromium.org; evgr...@chromium.org > Subject: Re: [RFC 1/7] mac80211: Add check for napi handle before > WARN_ON > > On Tue, 2020-07-21 at 22:44 +0530, Rakesh Pillai wrote: > > The function ieee80211_rx_napi can be now called > > from a thread context as well, with napi context > > being NULL. > > > > Hence add the napi context check before giving out > > a warning for softirq count being 0. > > > > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1 > > > > Signed-off-by: Rakesh Pillai > > --- > > net/mac80211/rx.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c > > index a88ab6f..1e703f1 100644 > > --- a/net/mac80211/rx.c > > +++ b/net/mac80211/rx.c > > @@ -4652,7 +4652,7 @@ void ieee80211_rx_napi(struct ieee80211_hw > *hw, struct ieee80211_sta *pubsta, > > struct ieee80211_supported_band *sband; > > struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb); > > > > - WARN_ON_ONCE(softirq_count() == 0); > > + WARN_ON_ONCE(napi && softirq_count() == 0); > > FWIW, I'm pretty sure this is incorrect - we make assumptions on > softirqs being disabled in mac80211 for serialization and in place of > some locking, I believe. > I checked this, but let me double confirm. But after this change, no packet is submitted from driver in a softirq context. So ideally this should take care of serialization. > johannes
RE: [RFC 2/7] ath10k: Add support to process rx packet in thread
> -Original Message- > From: Rajkumar Manoharan > Sent: Wednesday, July 22, 2020 3:23 AM > To: Rakesh Pillai > Cc: ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > ker...@vger.kernel.org; kv...@codeaurora.org; johan...@sipsolutions.net; > da...@davemloft.net; k...@kernel.org; net...@vger.kernel.org; > diand...@chromium.org; evgr...@chromium.org; linux-wireless- > ow...@vger.kernel.org > Subject: Re: [RFC 2/7] ath10k: Add support to process rx packet in thread > > On 2020-07-21 10:14, Rakesh Pillai wrote: > > NAPI instance gets scheduled on a CPU core on which > > the IRQ was triggered. The processing of rx packets > > can be CPU intensive and since NAPI cannot be moved > > to a different CPU core, to get better performance, > > its better to move the gist of rx packet processing > > in a high priority thread. > > > > Add the init/deinit part for a thread to process the > > receive packets. > > > IMHO this defeat the whole purpose of NAPI. Originally in ath10k > irq processing happened in tasklet (high priority) context which in > turn push more data to net core even though net is unable to process > driver data as both happen in different context (fast producer - slow > consumer) > issue. Why can't CPU governor schedule the interrupts in less loaded CPU > core? > Otherwise you can play with different RPS and affinity settings to meet > your > requirement. > > IMO introducing high priority tasklets/threads is not viable solution. Hi Rajkumar, In linux, the IRQs are directed to the first core which is booted. I see that all the IRQs are getting routed to CORE0 even if its heavily loaded. IRQ and NAPI are not under the scheduler's control, since it cannot be moved. NAPI is scheduled only on the same core as IRQ. But a thread can be moved around by the scheduler based on the CPU load. This is the reason I went ahead with using thread. Affinity settings are static, and cannot be done runtime without any downstream userspace entity. > > -Rajkumar
RE: [RFC 0/7] Add support to process rx packets in thread
> -Original Message- > From: Florian Fainelli > Sent: Tuesday, July 21, 2020 11:35 PM > To: Andrew Lunn ; Rakesh Pillai > Cc: ath...@lists.infradead.org; linux-wirel...@vger.kernel.org; linux- > ker...@vger.kernel.org; kv...@codeaurora.org; johan...@sipsolutions.net; > da...@davemloft.net; k...@kernel.org; net...@vger.kernel.org; > diand...@chromium.org; evgr...@chromium.org > Subject: Re: [RFC 0/7] Add support to process rx packets in thread > > On 7/21/20 10:25 AM, Andrew Lunn wrote: > > On Tue, Jul 21, 2020 at 10:44:19PM +0530, Rakesh Pillai wrote: > >> NAPI gets scheduled on the CPU core which got the > >> interrupt. The linux scheduler cannot move it to a > >> different core, even if the CPU on which NAPI is running > >> is heavily loaded. This can lead to degraded wifi > >> performance when running traffic at peak data rates. > >> > >> A thread on the other hand can be moved to different > >> CPU cores, if the one on which its running is heavily > >> loaded. During high incoming data traffic, this gives > >> better performance, since the thread can be moved to a > >> less loaded or sometimes even a more powerful CPU core > >> to account for the required CPU performance in order > >> to process the incoming packets. > >> > >> This patch series adds the support to use a high priority > >> thread to process the incoming packets, as opposed to > >> everything being done in NAPI context. > > > > I don't see why this problem is limited to the ath10k driver. I expect > > it applies to all drivers using NAPI. So shouldn't you be solving this > > in the NAPI core? Allow a driver to request the NAPI core uses a > > thread? > > What's more, you should be able to configure interrupt affinity to steer > RX processing onto a desired CPU core, is not that working for you > somehow? Hi Florian, Yes, the affinity of IRQ does work for me. But the affinity of IRQ does not happen runtime based on load. > -- > Florian
[RFC 2/7] ath10k: Add support to process rx packet in thread
NAPI instance gets scheduled on a CPU core on which
the IRQ was triggered. The processing of rx packets
can be CPU intensive and since NAPI cannot be moved
to a different CPU core, to get better performance,
its better to move the gist of rx packet processing
in a high priority thread.
Add the init/deinit part for a thread to process the
receive packets.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.c | 33 +++
drivers/net/wireless/ath/ath10k/core.h | 23 +++
drivers/net/wireless/ath/ath10k/snoc.c | 41 ++
3 files changed, 97 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 9104496..2b520a0 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -668,6 +668,39 @@ static unsigned int ath10k_core_get_fw_feature_str(char
*buf,
return scnprintf(buf, buf_len, "%s", ath10k_core_fw_feature_str[feat]);
}
+int ath10k_core_thread_shutdown(struct ath10k *ar,
+ struct ath10k_thread *thread)
+{
+ ath10k_dbg(ar, ATH10K_DBG_BOOT, "shutting down %s\n", thread->name);
+ set_bit(ATH10K_THREAD_EVENT_SHUTDOWN, thread->event_flags);
+ wake_up_process(thread->task);
+ wait_for_completion(>shutdown);
+ ath10k_info(ar, "thread %s exited\n", thread->name);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_thread_shutdown);
+
+int ath10k_core_thread_init(struct ath10k *ar,
+ struct ath10k_thread *thread,
+ int (*handler)(void *data),
+ char *thread_name)
+{
+ thread->task = kthread_create(handler, thread, thread_name);
+ if (IS_ERR(thread->task))
+ return -EINVAL;
+
+ init_waitqueue_head(>wait_q);
+ init_completion(>shutdown);
+ memcpy(thread->name, thread_name, ATH10K_THREAD_NAME_SIZE_MAX);
+ clear_bit(ATH10K_THREAD_EVENT_SHUTDOWN, thread->event_flags);
+ ath10k_info(ar, "Starting thread %s\n", thread_name);
+ wake_up_process(thread->task);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_thread_init);
+
void ath10k_core_get_fw_features_str(struct ath10k *ar,
char *buf,
size_t buf_len)
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 5c18f6c..96919e8 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -970,6 +970,22 @@ struct ath10k_bus_params {
bool hl_msdu_ids;
};
+#define ATH10K_THREAD_NAME_SIZE_MAX32
+
+enum ath10k_thread_events {
+ ATH10K_THREAD_EVENT_SHUTDOWN,
+ ATH10K_THREAD_EVENT_MAX,
+};
+
+struct ath10k_thread {
+ struct ath10k *ar;
+ struct task_struct *task;
+ struct completion shutdown;
+ wait_queue_head_t wait_q;
+ DECLARE_BITMAP(event_flags, ATH10K_THREAD_EVENT_MAX);
+ char name[ATH10K_THREAD_NAME_SIZE_MAX];
+};
+
struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
@@ -982,6 +998,7 @@ struct ath10k {
} msa;
u8 mac_addr[ETH_ALEN];
+ struct ath10k_thread rx_thread;
enum ath10k_hw_rev hw_rev;
u16 dev_id;
u32 chip_id;
@@ -1276,6 +1293,12 @@ static inline bool ath10k_peer_stats_enabled(struct
ath10k *ar)
extern unsigned long ath10k_coredump_mask;
+int ath10k_core_thread_shutdown(struct ath10k *ar,
+ struct ath10k_thread *thread);
+int ath10k_core_thread_init(struct ath10k *ar,
+ struct ath10k_thread *thread,
+ int (*handler)(void *data),
+ char *thread_name);
struct ath10k *ath10k_core_create(size_t priv_size, struct device *dev,
enum ath10k_bus bus,
enum ath10k_hw_rev hw_rev,
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index 1ef5fdb..463c34e 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -909,6 +909,31 @@ static void ath10k_snoc_buffer_cleanup(struct ath10k *ar)
}
}
+int ath10k_snoc_rx_thread_loop(void *data)
+{
+ struct ath10k_thread *rx_thread = data;
+ struct ath10k *ar = rx_thread->ar;
+ bool shutdown = false;
+
+ ath10k_dbg(ar, ATH10K_DBG_SNOC, "rx thread started\n");
+ set_user_nice(current, -1);
+
+ while (!shutdown) {
+ wait_event_interruptible(
+ rx_thread->wait_q,
+ (test_bit(ATH10K_THREAD_EVENT_SHUTDOWN,
+ rx
[RFC 7/7] ath10k: Handle rx thread suspend and resume
During the system suspend or resume, the rx thread
also needs to be suspended or resume respectively.
Handle the rx thread as well during the system
suspend and resume.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.c | 23 ++
drivers/net/wireless/ath/ath10k/core.h | 5
drivers/net/wireless/ath/ath10k/snoc.c | 44 +-
3 files changed, 71 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 4064fa2..b82b355 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -668,6 +668,27 @@ static unsigned int ath10k_core_get_fw_feature_str(char
*buf,
return scnprintf(buf, buf_len, "%s", ath10k_core_fw_feature_str[feat]);
}
+int ath10k_core_thread_suspend(struct ath10k_thread *thread)
+{
+ ath10k_dbg(thread->ar, ATH10K_DBG_BOOT, "Suspending thread %s\n",
+ thread->name);
+ set_bit(ATH10K_THREAD_EVENT_SUSPEND, thread->event_flags);
+ wait_for_completion(>suspend);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_thread_suspend);
+
+int ath10k_core_thread_resume(struct ath10k_thread *thread)
+{
+ ath10k_dbg(thread->ar, ATH10K_DBG_BOOT, "Resuming thread %s\n",
+ thread->name);
+ complete(>resume);
+
+ return 0;
+}
+EXPORT_SYMBOL(ath10k_core_thread_resume);
+
void ath10k_core_thread_post_event(struct ath10k_thread *thread,
enum ath10k_thread_events event)
{
@@ -700,6 +721,8 @@ int ath10k_core_thread_init(struct ath10k *ar,
init_waitqueue_head(>wait_q);
init_completion(>shutdown);
+ init_completion(>suspend);
+ init_completion(>resume);
memcpy(thread->name, thread_name, ATH10K_THREAD_NAME_SIZE_MAX);
clear_bit(ATH10K_THREAD_EVENT_SHUTDOWN, thread->event_flags);
ath10k_info(ar, "Starting thread %s\n", thread_name);
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 596d31b..df65e75 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -976,6 +976,7 @@ enum ath10k_thread_events {
ATH10K_THREAD_EVENT_SHUTDOWN,
ATH10K_THREAD_EVENT_RX_POST,
ATH10K_THREAD_EVENT_TX_POST,
+ ATH10K_THREAD_EVENT_SUSPEND,
ATH10K_THREAD_EVENT_MAX,
};
@@ -983,6 +984,8 @@ struct ath10k_thread {
struct ath10k *ar;
struct task_struct *task;
struct completion shutdown;
+ struct completion suspend;
+ struct completion resume;
wait_queue_head_t wait_q;
DECLARE_BITMAP(event_flags, ATH10K_THREAD_EVENT_MAX);
char name[ATH10K_THREAD_NAME_SIZE_MAX];
@@ -1296,6 +1299,8 @@ static inline bool ath10k_peer_stats_enabled(struct
ath10k *ar)
extern unsigned long ath10k_coredump_mask;
+int ath10k_core_thread_suspend(struct ath10k_thread *thread);
+int ath10k_core_thread_resume(struct ath10k_thread *thread);
void ath10k_core_thread_post_event(struct ath10k_thread *thread,
enum ath10k_thread_events event);
int ath10k_core_thread_shutdown(struct ath10k *ar,
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index 3eb5eac..a373b2b 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -932,13 +932,31 @@ int ath10k_snoc_rx_thread_loop(void *data)
rx_thread->event_flags) ||
test_and_clear_bit(ATH10K_THREAD_EVENT_TX_POST,
rx_thread->event_flags) ||
+test_bit(ATH10K_THREAD_EVENT_SUSPEND,
+ rx_thread->event_flags) ||
test_bit(ATH10K_THREAD_EVENT_SHUTDOWN,
rx_thread->event_flags)));
+ if (test_and_clear_bit(ATH10K_THREAD_EVENT_SUSPEND,
+ rx_thread->event_flags)) {
+ complete(_thread->suspend);
+ ath10k_info(ar, "rx thread suspend\n");
+ wait_for_completion(_thread->resume);
+ ath10k_info(ar, "rx thread resume\n");
+ }
+
ath10k_htt_txrx_compl_task(ar, thread_budget);
if (test_and_clear_bit(ATH10K_THREAD_EVENT_SHUTDOWN,
rx_thread->event_flags))
shutdown = true;
+
+ if (test_and_clear_bit(ATH10K_THREAD_EVENT_SUSPEND,
+ rx_thread->event_flags)) {
+ complete(_thread-
[RFC 5/7] ath10k: Handle the rx packet processing in thread
Add the support to handle the receive packet and
the tx completion processing in a thread context
if the feature has been enabled via module parameter.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.c | 8 ++
drivers/net/wireless/ath/ath10k/core.h | 4 +++
drivers/net/wireless/ath/ath10k/htt.h| 2 ++
drivers/net/wireless/ath/ath10k/htt_rx.c | 46 +++-
drivers/net/wireless/ath/ath10k/snoc.c | 12 +++--
5 files changed, 64 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 2b520a0..4064fa2 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -668,6 +668,14 @@ static unsigned int ath10k_core_get_fw_feature_str(char
*buf,
return scnprintf(buf, buf_len, "%s", ath10k_core_fw_feature_str[feat]);
}
+void ath10k_core_thread_post_event(struct ath10k_thread *thread,
+ enum ath10k_thread_events event)
+{
+ set_bit(event, thread->event_flags);
+ wake_up(>wait_q);
+}
+EXPORT_SYMBOL(ath10k_core_thread_post_event);
+
int ath10k_core_thread_shutdown(struct ath10k *ar,
struct ath10k_thread *thread)
{
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 59bdf11..596d31b 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -974,6 +974,8 @@ struct ath10k_bus_params {
enum ath10k_thread_events {
ATH10K_THREAD_EVENT_SHUTDOWN,
+ ATH10K_THREAD_EVENT_RX_POST,
+ ATH10K_THREAD_EVENT_TX_POST,
ATH10K_THREAD_EVENT_MAX,
};
@@ -1294,6 +1296,8 @@ static inline bool ath10k_peer_stats_enabled(struct
ath10k *ar)
extern unsigned long ath10k_coredump_mask;
+void ath10k_core_thread_post_event(struct ath10k_thread *thread,
+ enum ath10k_thread_events event);
int ath10k_core_thread_shutdown(struct ath10k *ar,
struct ath10k_thread *thread);
int ath10k_core_thread_init(struct ath10k *ar,
diff --git a/drivers/net/wireless/ath/ath10k/htt.h
b/drivers/net/wireless/ath/ath10k/htt.h
index cad5949..e3cb723 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1970,6 +1970,8 @@ struct ath10k_htt {
spinlock_t lock;
} rx_ring;
+ /* Protects access to in order indication queue */
+ spinlock_t rx_in_ord_q_lock;
unsigned int prefetch_len;
/* Protects access to pending_tx, num_pending_tx */
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index a4a6618..becbd56 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -796,6 +796,7 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
timer_setup(timer, ath10k_htt_rx_ring_refill_retry, 0);
spin_lock_init(>rx_ring.lock);
+ spin_lock_init(>rx_in_ord_q_lock);
htt->rx_ring.fill_cnt = 0;
htt->rx_ring.sw_rd_idx.msdu_payld = 0;
@@ -2702,10 +2703,17 @@ static void ath10k_htt_rx_tx_compl_ind(struct ath10k
*ar,
*/
if (ar->bus_param.dev_type == ATH10K_DEV_TYPE_HL) {
ath10k_txrx_tx_unref(htt, _done);
- } else if (!kfifo_put(>txdone_fifo, tx_done)) {
- ath10k_warn(ar, "txdone fifo overrun, msdu_id %d status
%d\n",
- tx_done.msdu_id, tx_done.status);
- ath10k_txrx_tx_unref(htt, _done);
+ } else {
+ if (!kfifo_put(>txdone_fifo, tx_done)) {
+ ath10k_warn(ar, "txdone fifo overrun, msdu_id
%d status %d\n",
+ tx_done.msdu_id, tx_done.status);
+ ath10k_txrx_tx_unref(htt, _done);
+ continue;
+ }
+ if (ar->rx_thread_enable)
+ ath10k_core_thread_post_event(
+ >rx_thread,
+ ATH10K_THREAD_EVENT_TX_POST);
}
}
@@ -3903,7 +3911,16 @@ bool ath10k_htt_t2h_msg_handler(struct ath10k *ar,
struct sk_buff *skb)
break;
}
case HTT_T2H_MSG_TYPE_RX_IN_ORD_PADDR_IND: {
- skb_queue_tail(>rx_in_ord_compl_q, skb);
+ if (!ar->rx_thread_enable) {
+ skb_queue_tail(>rx_in_ord_compl_q, skb);
+ } else {
+ spin_lock_bh(>rx_in_ord_q_lock);
+ skb_queue_tail(>rx_
[RFC 6/7] ath10k: Add deliver to stack from thread context
When the receive packets are submitted to the stack from a thread context, the NAPI handle should be passed as NULL to the function ieee80211_rx_napi. This will make sure that the packets are submitted to stack via non-napi method Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1 Signed-off-by: Rakesh Pillai --- drivers/net/wireless/ath/ath10k/htt_rx.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c index becbd56..85c169c 100644 --- a/drivers/net/wireless/ath/ath10k/htt_rx.c +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c @@ -1321,7 +1321,10 @@ static void ath10k_process_rx(struct ath10k *ar, struct sk_buff *skb) trace_ath10k_rx_hdr(ar, skb->data, skb->len); trace_ath10k_rx_payload(ar, skb->data, skb->len); - ieee80211_rx_napi(ar->hw, NULL, skb, >napi); + if (in_serving_softirq()) + ieee80211_rx_napi(ar->hw, NULL, skb, >napi); + else + ieee80211_rx_napi(ar->hw, NULL, skb, NULL); } static int ath10k_htt_rx_nwifi_hdrlen(struct ath10k *ar, -- 2.7.4
[RFC 3/7] ath10k: Add module param to enable rx thread
Add a module parameter to enable or disable
the processing of received packets in rx thread.
To enable rx packet processing in a thread
context, use the belo command to load driver:
insmod ath10k_snoc.ko rx_thread_enable=1
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/snoc.c | 24 +---
2 files changed, 18 insertions(+), 7 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 96919e8..59bdf11 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -998,6 +998,7 @@ struct ath10k {
} msa;
u8 mac_addr[ETH_ALEN];
+ bool rx_thread_enable;
struct ath10k_thread rx_thread;
enum ath10k_hw_rev hw_rev;
u16 dev_id;
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index 463c34e..f01725b 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -26,6 +26,12 @@
#define CE_POLL_PIPE 4
#define ATH10K_SNOC_WAKE_IRQ 2
+static bool ath10k_rx_thread_enable;
+
+module_param_named(rx_thread_enable, ath10k_rx_thread_enable, bool, 0644);
+
+MODULE_PARM_DESC(rx_thread_enable, "Receive packet processing in thread");
+
static char *const ce_name[] = {
"WLAN_CE_0",
"WLAN_CE_1",
@@ -941,7 +947,8 @@ static void ath10k_snoc_hif_stop(struct ath10k *ar)
napi_synchronize(>napi);
napi_disable(>napi);
- ath10k_core_thread_shutdown(ar, >rx_thread);
+ if (ar->rx_thread_enable)
+ ath10k_core_thread_shutdown(ar, >rx_thread);
ath10k_snoc_buffer_cleanup(ar);
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif stop\n");
}
@@ -954,12 +961,14 @@ static int ath10k_snoc_hif_start(struct ath10k *ar)
bitmap_clear(ar_snoc->pending_ce_irqs, 0, CE_COUNT_MAX);
napi_enable(>napi);
- ret = ath10k_core_thread_init(ar, >rx_thread,
- ath10k_snoc_rx_thread_loop,
- "ath10k_rx_thread");
- if (ret) {
- ath10k_err(ar, "failed to start rx thread\n");
- goto rx_thread_fail;
+ if (ar->rx_thread_enable) {
+ ret = ath10k_core_thread_init(ar, >rx_thread,
+ ath10k_snoc_rx_thread_loop,
+ "ath10k_rx_thread");
+ if (ret) {
+ ath10k_err(ar, "failed to start rx thread\n");
+ goto rx_thread_fail;
+ }
}
ath10k_snoc_irq_enable(ar);
@@ -1693,6 +1702,7 @@ static int ath10k_snoc_probe(struct platform_device *pdev)
}
ar->rx_thread.ar = ar;
+ ar->rx_thread_enable = ath10k_rx_thread_enable;
ar_snoc = ath10k_snoc_priv(ar);
ar_snoc->dev = pdev;
platform_set_drvdata(pdev, ar);
--
2.7.4
[RFC 4/7] ath10k: Do not exhaust budget on process tx completion
Currently the entire NAPI budget is marked as exhausted
if any tx completion is processed.
In scenarios of bi-directional traffic, this leads to a
situation where the irqs are never enabled and the NAPI
is rescheuled again and again.
Increase the work done quota by the number of tx completions
which are processed in the NAPI context.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/htt_rx.c | 15 ++-
1 file changed, 6 insertions(+), 9 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c
b/drivers/net/wireless/ath/ath10k/htt_rx.c
index cac05e7..a4a6618 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -4077,21 +4077,18 @@ int ath10k_htt_txrx_compl_task(struct ath10k *ar, int
budget)
/* Deliver received data after processing data from hardware */
quota = ath10k_htt_rx_deliver_msdu(ar, quota, budget);
- /* From NAPI documentation:
-* The napi poll() function may also process TX completions, in which
-* case if it processes the entire TX ring then it should count that
-* work as the rest of the budget.
-*/
- if ((quota < budget) && !kfifo_is_empty(>txdone_fifo))
- quota = budget;
-
/* kfifo_get: called only within txrx_tasklet so it's neatly serialized.
* From kfifo_get() documentation:
* Note that with only one concurrent reader and one concurrent writer,
* you don't need extra locking to use these macro.
*/
- while (kfifo_get(>txdone_fifo, _done))
+ while (kfifo_get(>txdone_fifo, _done)) {
ath10k_txrx_tx_unref(htt, _done);
+ quota++;
+ }
+
+ if (quota > budget)
+ resched_napi = true;
ath10k_mac_tx_push_pending(ar);
--
2.7.4
[RFC 1/7] mac80211: Add check for napi handle before WARN_ON
The function ieee80211_rx_napi can be now called from a thread context as well, with napi context being NULL. Hence add the napi context check before giving out a warning for softirq count being 0. Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1 Signed-off-by: Rakesh Pillai --- net/mac80211/rx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c index a88ab6f..1e703f1 100644 --- a/net/mac80211/rx.c +++ b/net/mac80211/rx.c @@ -4652,7 +4652,7 @@ void ieee80211_rx_napi(struct ieee80211_hw *hw, struct ieee80211_sta *pubsta, struct ieee80211_supported_band *sband; struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb); - WARN_ON_ONCE(softirq_count() == 0); + WARN_ON_ONCE(napi && softirq_count() == 0); if (WARN_ON(status->band >= NUM_NL80211_BANDS)) goto drop; -- 2.7.4
[RFC 0/7] Add support to process rx packets in thread
NAPI gets scheduled on the CPU core which got the interrupt. The linux scheduler cannot move it to a different core, even if the CPU on which NAPI is running is heavily loaded. This can lead to degraded wifi performance when running traffic at peak data rates. A thread on the other hand can be moved to different CPU cores, if the one on which its running is heavily loaded. During high incoming data traffic, this gives better performance, since the thread can be moved to a less loaded or sometimes even a more powerful CPU core to account for the required CPU performance in order to process the incoming packets. This patch series adds the support to use a high priority thread to process the incoming packets, as opposed to everything being done in NAPI context. The rx thread can be enabled by using a module parameter when loading the ath10k_snoc module. --- This patch series is dependent on the below patch series https://patchwork.kernel.org/project/ath10k/list/?series=315759 Rakesh Pillai (7): mac80211: Add check for napi handle before WARN_ON ath10k: Add support to process rx packet in thread ath10k: Add module param to enable rx thread ath10k: Do not exhaust budget on process tx completion ath10k: Handle the rx packet processing in thread ath10k: Add deliver to stack from thread context ath10k: Handle rx thread suspend and resume drivers/net/wireless/ath/ath10k/core.c | 64 +++ drivers/net/wireless/ath/ath10k/core.h | 33 ++ drivers/net/wireless/ath/ath10k/htt.h| 2 + drivers/net/wireless/ath/ath10k/htt_rx.c | 66 ++- drivers/net/wireless/ath/ath10k/snoc.c | 105 ++- net/mac80211/rx.c| 2 +- 6 files changed, 253 insertions(+), 19 deletions(-) -- 2.7.4
RE: [PATCH] ath10k: Add interrupt summary based CE processing
> -Original Message- > From: Peter Oh > Sent: Tuesday, July 21, 2020 7:03 AM > To: Kalle Valo > Cc: Brian Norris ; Doug Anderson > ; linux-wireless wirel...@vger.kernel.org>; Rakesh Pillai ; ath10k > ; LKML > Subject: Re: [PATCH] ath10k: Add interrupt summary based CE processing > > I'll take my word back. > It's not this patch problem, but by others. > I have 2 extra patches before the 3 patches so my system looks like > > backports from ath.git 5.6-rc1 + linux kernel 4.4 (similar to OpenWrt) > On top of the working system, I cherry-picked these 5. > > #1. > ath10k: Avoid override CE5 configuration for QCA99X0 chipsets > ath.git commit 521fc37be3d879561ca5ab42d64719cf94116af0 > #2. > ath10k: Fix NULL pointer dereference in AHB device probe > wireless-drivers.git commit 1cfd3426ef989b83fa6176490a38777057e57f6c > #3. > ath10k: Add interrupt summary based CE processing > https://patchwork.kernel.org/patch/11628299/ Hi Peter, This patch is applicable only for snoc target WCN3990, since there is a check for per_ce_irq. For PCI targets, per_ce_irq is false, and hence follows a different path. Thanks, Rakesh Pillai. > #4. > ath10k: Keep track of which interrupts fired, don't poll them > https://patchwork.kernel.org/patch/11654631/ > #5. > ath10k: Get rid of "per_ce_irq" hw param > https://patchwork.kernel.org/patch/11654633/ > > The error "[ 14.226184] ath10k_ahb a00.wifi: failed to receive > initialized event from target: 8000" is because of #1 and #2, > since this happens even after I reverted #3~#5. > Once I reverted all except #1 I got another crash. > > [ 11.179595] !#% P<__ath10k_ce_rx_post_buf+0x14/0x98 > [ath10k_core]> L<0x4bc00> F<005> [000c] > [ 11.179643] Unable to handle kernel NULL pointer dereference at > virtual address 000c > [ 11.439207] [<7f15a69c>] (__ath10k_ce_rx_post_buf [ath10k_core]) > from [<7f15a874>] (ath10k_ce_rx_post_buf+0x3c/0x50 [ath10k_core]) > [ 11.447204] [<7f15a874>] (ath10k_ce_rx_post_buf [ath10k_core]) from > [<7f2889a4>] (ath10k_pci_diag_read_mem+0x104/0x2a8 [ath10k_pci]) > [ 11.458706] [<7f2889a4>] (ath10k_pci_diag_read_mem [ath10k_pci]) > from [<7f288b68>] (ath10k_pci_diag_read32+0x1c/0x2c [ath10k_pci]) > [ 11.470767] [<7f288b68>] (ath10k_pci_diag_read32 [ath10k_pci]) from > [<7f28abe8>] (ath10k_pci_init_config+0x2c/0x290 [ath10k_pci]) > [ 11.482314] [<7f28abe8>] (ath10k_pci_init_config [ath10k_pci]) from > [<7f28d160>] (ath10k_ahb_hif_power_up+0x7c/0xe8 [ath10k_pci]) > [ 11.494153] [<7f28d160>] (ath10k_ahb_hif_power_up [ath10k_pci]) > from [<7f135348>] (ath10k_core_register_work+0x84/0x8f8 [ath10k_core]) > [ 11.505766] [<7f135348>] (ath10k_core_register_work [ath10k_core]) > from [<8023b614>] (process_one_work+0x1c0/0x2f8) > [ 11.517594] [<8023b614>] (process_one_work) from [<8023c650>] > (worker_thread+0x280/0x3c0) > [ 11.527919] [<8023c650>] (worker_thread) from [<802408f8>] > (kthread+0xd8/0xe8) > [ 11.536247] [<802408f8>] (kthread) from [<80209ce8>] > (ret_from_fork+0x14/0x2c) > > When I revert #1 eventually, my system is back to working. > So I'm blaming the #1 and #2 could have potential bugs or require > ath.git branch up-to-date. > > On Mon, Jul 20, 2020 at 5:58 PM Peter Oh wrote: > > > > My previous email wasn't sent out. > > > > At first I gave these 3 patches. > > ath10k: Add interrupt summary based CE processing > > https://patchwork.kernel.org/patch/11628299/ > > ath10k: Keep track of which interrupts fired, don't poll them > > https://patchwork.kernel.org/patch/11654631/ > > ath10k: Get rid of "per_ce_irq" hw param > > https://patchwork.kernel.org/patch/11654633/ > > and saw the crash happen and then reverted the top 2 and used the very > > first one, but it is still happening. > > > > On Mon, Jul 20, 2020 at 5:56 PM Peter Oh wrote: > > > > > > Since IPQ4019 doesn't support per CE based interrupt summary, I doubt > > > if this change is correct. > > > + ath10k_ce_engine_int_status_clear(ar, ctrl_addr, > > > + wm_regs->cc_mask | > > > wm_regs->wm_mask); > > > > > > > > > On Mon, Jul 20, 2020 at 5:53 PM Peter Oh wrote: > > > > > > > > At first I gave these 3 patches. > > > > ath10k: Add interrupt summary based CE processing > > > > https://patchwork.kernel.org/patch/11628299/ > > > > ath10k: Keep track of which interrupts fired, d
RE: [PATCH] ath10k: Add history for tracking certain events
> -Original Message- > From: Ben Greear > Sent: Sunday, June 28, 2020 10:56 PM > To: Rakesh Pillai ; ath...@lists.infradead.org > Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] ath10k: Add history for tracking certain events > > > > On 06/27/2020 10:12 PM, Rakesh Pillai wrote: > > > > > >> -Original Message- > >> From: Ben Greear > >> Sent: Saturday, June 27, 2020 8:58 PM > >> To: Rakesh Pillai ; ath...@lists.infradead.org > >> Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org > >> Subject: Re: [PATCH] ath10k: Add history for tracking certain events > >> > >> > >> > >> On 06/26/2020 11:22 PM, Rakesh Pillai wrote: > >>> For debugging many issues, a history of the > >>> below mentioned events can help get an idea > >>> of what exactly was going on just before any > >>> issue occurred in the system. These event > >>> history will be collected only when the host > >>> driver is run in debug mode (i.e. with the > >>> config ATH10K_DEBUG enabled). > >> > >> This should be disabled by default unless user specifically pokes some > >> debugfs > >> value to turn it on so that it does not impact performance. > > > > Hi Ben, > > This history is enabled only if the user compiles the kernel with > > ATH10K_DEBUG. > > Making it runtime, adds a lot of "if" conditions for this history record. > > Do you suggest to add support to enable/disable it runtime even in > > ATH10K_DEBUG ? > > Yes, because you are adding lots of locks/unlocks. That is way more > expensive > than an if statement. You can add an 'unlikely' to the if check as well, so > compiler will optimize for this feature not being enabled. Hi Ben, I missed this mail somehow. Yes sure, I will make the necessary changes and upload a new patchset. Thanks, Rakesh Pillai. > > Thanks, > Ben > > > > >> > >> Thanks, > >> Ben > >> > >>> > >>> Add history for tracking the below events > >>> - register read > >>> - register write > >>> - IRQ trigger > >>> - IRQ Enable > >>> - IRQ Disable > >>> - NAPI poll > >>> - CE service > >>> - WMI cmd > >>> - WMI event > >>> - WMI tx completion > >>> > >>> This will help in debugging any crash or any > >>> improper behaviour. > >> > >> > >> -- > >> Ben Greear > >> Candela Technologies Inc http://www.candelatech.com > > > > -- > Ben Greear > Candela Technologies Inc http://www.candelatech.com
[PATCH v2] arm64: dts: qcom: sc7180: Add missing properties for Wifi node
The wlan firmware memory is statically mapped in
the Trusted Firmware, hence the wlan driver does
not need to map/unmap this region dynamically.
Hence add the property to indicate the wlan driver
to not map/unamp the firmware memory region
dynamically.
Also add the chain1 voltage supply for wlan.
Signed-off-by: Rakesh Pillai
---
Changes from v1:
- Add the wifi mac alias
This patch is created on top of the change by
Douglas Anderson.
https://lkml.org/lkml/2020/6/25/817
Also the dt-bindings for the chain1 voltage supply
is added by the below patch series:
https://patchwork.kernel.org/project/linux-wireless/list/?series=309137
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 472f7f4..c042d61 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -19,6 +19,7 @@
aliases {
bluetooth0 =
+ wifi0 =
hsuart0 =
serial0 =
};
@@ -391,10 +392,12 @@
{
status = "okay";
+ qcom,msa-fixed-perm;
vdd-0.8-cx-mx-supply = <_l9a_0p6>;
vdd-1.8-xo-supply = <_l1c_1p8>;
vdd-1.3-rfa-supply = <_l2c_1p3>;
vdd-3.3-ch0-supply = <_l10c_3p3>;
+ vdd-3.3-ch1-supply = <_l11c_3p3>;
wifi-firmware {
iommus = <_smmu 0xc2 0x1>;
};
--
2.7.4
RE: [PATCH] arm64: dts: qcom: sc7180: Add missing properties for Wifi node
> -Original Message-
> From: Doug Anderson
> Sent: Friday, July 10, 2020 1:36 AM
> To: Rakesh Pillai
> Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS
> ; Evan Green ;
> Andy Gross ; Bjorn Andersson
> ; Rob Herring ; linux-
> arm-msm ; LKML ker...@vger.kernel.org>; Sibi Sankar
> Subject: Re: [PATCH] arm64: dts: qcom: sc7180: Add missing properties for
> Wifi node
>
> Hi,
>
> On Thu, Jul 9, 2020 at 2:18 AM Rakesh Pillai wrote:
> >
> > The wlan firmware memory is statically mapped in
> > the Trusted Firmware, hence the wlan driver does
> > not need to map/unmap this region dynamically.
> >
> > Hence add the property to indicate the wlan driver
> > to not map/unamp the firmware memory region
> > dynamically.
> >
> > Also add the chain1 voltage supply for wlan.
> >
> > Signed-off-by: Rakesh Pillai
> > ---
> > This patch is created on top of the change by
> > Douglas Anderson.
> > https://lkml.org/lkml/2020/6/25/817
> >
> > Also the dt-bindings for the chain1 voltage supply
> > is added by the below patch series:
> > https://patchwork.kernel.org/project/linux-wireless/list/?series=309137
> > ---
> > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > index 472f7f4..4c64bc1 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > @@ -391,10 +391,12 @@
> >
> > {
> > status = "okay";
> > + qcom,msa-fixed-perm;
>
> At one point in time I thought +Sibi said that this wouldn't be needed
> once the firmware was fixed. ...afterwards you said that it was
> needed for SSR (subsystem reset). Would be good to get confirmation
> from Sibi that this matches his understanding.
Hi Doug,
This is now needed as the firmware memory mapping was moved to Trusted firmware.
This region is now statically mapped to avoid access from driver.
>
>
> > vdd-0.8-cx-mx-supply = <_l9a_0p6>;
> > vdd-1.8-xo-supply = <_l1c_1p8>;
> > vdd-1.3-rfa-supply = <_l2c_1p3>;
> > vdd-3.3-ch0-supply = <_l10c_3p3>;
> > + vdd-3.3-ch1-supply = <_l11c_3p3>;
> > wifi-firmware {
> > iommus = <_smmu 0xc2 0x1>;
> > };
>
> Other than the one question this looks good to me.
>
> -Doug
[PATCH] arm64: dts: qcom: sc7180: Add missing properties for Wifi node
The wlan firmware memory is statically mapped in
the Trusted Firmware, hence the wlan driver does
not need to map/unmap this region dynamically.
Hence add the property to indicate the wlan driver
to not map/unamp the firmware memory region
dynamically.
Also add the chain1 voltage supply for wlan.
Signed-off-by: Rakesh Pillai
---
This patch is created on top of the change by
Douglas Anderson.
https://lkml.org/lkml/2020/6/25/817
Also the dt-bindings for the chain1 voltage supply
is added by the below patch series:
https://patchwork.kernel.org/project/linux-wireless/list/?series=309137
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 2 ++
1 file changed, 2 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 472f7f4..4c64bc1 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -391,10 +391,12 @@
{
status = "okay";
+ qcom,msa-fixed-perm;
vdd-0.8-cx-mx-supply = <_l9a_0p6>;
vdd-1.8-xo-supply = <_l1c_1p8>;
vdd-1.3-rfa-supply = <_l2c_1p3>;
vdd-3.3-ch0-supply = <_l10c_3p3>;
+ vdd-3.3-ch1-supply = <_l11c_3p3>;
wifi-firmware {
iommus = <_smmu 0xc2 0x1>;
};
--
2.7.4
RE: [REPOST PATCH] arm64: dts: qcom: Fix WiFi supplies on sc7180-idp
> -Original Message-
> From: Rakesh Pillai
> Sent: Friday, June 26, 2020 2:14 PM
> To: 'Douglas Anderson' ; 'Andy Gross'
> ; 'Bjorn Andersson'
> Cc: 'Evan Green' ; 'Sibi Sankar'
> ; 'Rob Herring' ;
> 'devicet...@vger.kernel.org' ; 'linux-arm-
> m...@vger.kernel.org' ; 'linux-
> ker...@vger.kernel.org'
> Subject: RE: [REPOST PATCH] arm64: dts: qcom: Fix WiFi supplies on sc7180-
> idp
>
>
>
> > -Original Message-
> > From: Douglas Anderson
> > Sent: Friday, June 26, 2020 1:47 AM
> > To: Andy Gross ; Bjorn Andersson
> >
> > Cc: Evan Green ; Sibi Sankar
> > ; Rakesh Pillai ; Douglas
> > Anderson ; Rob Herring ;
> > devicet...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> > ker...@vger.kernel.org
> > Subject: [REPOST PATCH] arm64: dts: qcom: Fix WiFi supplies on
sc7180-idp
> >
> > The WiFi supplies that were added recently can't have done anything
> > useful because they were missing the "-supply" suffix. Booting
> > without the "-supply" suffix would give these messages:
> >
> > ath10k_snoc 1880.wifi: 1880.wifi supply vdd-0.8-cx-mx not found,
> > using dummy regulator
> > ath10k_snoc 1880.wifi: 1880.wifi supply vdd-1.8-xo not found,
> using
> > dummy regulator
> > ath10k_snoc 1880.wifi: 1880.wifi supply vdd-1.3-rfa not found,
> using
> > dummy regulator
> > ath10k_snoc 1880.wifi: 1880.wifi supply vdd-3.3-ch0 not found,
> > using dummy regulator
> >
> > Let's add the "-supply" suffix.
> >
> > Fixes: 1e7594a38f37 ("arm64: dts: qcom: sc7180: Add WCN3990 WLAN
> > module device node")
> > Signed-off-by: Douglas Anderson
> > ---
> Reviewed-by: Rakesh Pillai
Tested-by: Rakesh Pillai
>
> I missed this in the DTSI patch, while manually rebasing to the kernel
tip.
>
> > I don't have an IDP setup but I have a similar board. Testing on IDP
> > would, of course, be appreciated.
> >
> > Repost because I screwed up the "after-the-cut" notes on first post.
> >
> > arch/arm64/boot/dts/qcom/sc7180-idp.dts | 8
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > index 39dbfc89689e..472f7f41cc93 100644
> > --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> > @@ -391,10 +391,10 @@ video-firmware {
> >
> > {
> > status = "okay";
> > - vdd-0.8-cx-mx = <_l9a_0p6>;
> > - vdd-1.8-xo = <_l1c_1p8>;
> > - vdd-1.3-rfa = <_l2c_1p3>;
> > - vdd-3.3-ch0 = <_l10c_3p3>;
> > + vdd-0.8-cx-mx-supply = <_l9a_0p6>;
> > + vdd-1.8-xo-supply = <_l1c_1p8>;
> > + vdd-1.3-rfa-supply = <_l2c_1p3>;
> > + vdd-3.3-ch0-supply = <_l10c_3p3>;
> > wifi-firmware {
> > iommus = <_smmu 0xc2 0x1>;
> > };
> > --
> > 2.27.0.212.ge8ba1cc988-goog
RE: [PATCH] ath10k: Keep track of which interrupts fired, don't poll them
> -Original Message-
> From: Douglas Anderson
> Sent: Tuesday, July 7, 2020 10:48 PM
> To: kv...@codeaurora.org; ath...@lists.infradead.org
> Cc: saiprakash.ran...@codeaurora.org; linux-arm-...@vger.kernel.org;
> linux-wirel...@vger.kernel.org; pill...@codeaurora.org;
> kua...@google.com; Douglas Anderson ; David
> S. Miller ; Jakub Kicinski ; linux-
> ker...@vger.kernel.org; net...@vger.kernel.org
> Subject: [PATCH] ath10k: Keep track of which interrupts fired, don't poll
> them
>
> If we have a per CE (Copy Engine) IRQ then we have no summary
> register. Right now the code generates a summary register by
> iterating over all copy engines and seeing if they have an interrupt
> pending.
>
> This has a problem. Specifically if _none_ if the Copy Engines have
> an interrupt pending then they might go into low power mode and
> reading from their address space will cause a full system crash. This
> was seen to happen when two interrupts went off at nearly the same
> time. Both were handled by a single call of ath10k_snoc_napi_poll()
> but, because there were two interrupts handled and thus two calls to
> napi_schedule() there was still a second call to
> ath10k_snoc_napi_poll() which ran with no interrupts pending.
>
> Instead of iterating over all the copy engines, let's just keep track
> of the IRQs that fire. Then we can effectively generate our own
> summary without ever needing to read the Copy Engines.
>
> Tested-on: WCN3990 SNOC WLAN.HL.3.2.2-00490-QCAHLSWMTPL-1
>
> Signed-off-by: Douglas Anderson
Reviewed-by: Rakesh Pillai
> ---
> This patch continues work to try to squash all instances of the crash
> we've been seeing while reading CE registers and hopefully this patch
> addresses the true root of the issue.
>
> The first patch that attempted to address these problems landed as
> commit 8f9ed93d09a9 ("ath10k: Wait until copy complete is actually
> done before completing"). After that Rakesh Pillai posted ("ath10k:
> Add interrupt summary based CE processing") [1] and this patch is
> based atop that one. Both of those patches significantly reduced the
> instances of problems but didn't fully eliminate them. Crossing my
> fingers that they're all gone now.
>
> [1] https://lore.kernel.org/r/1593193967-29897-1-git-send-email-
> pill...@codeaurora.org
>
> drivers/net/wireless/ath/ath10k/ce.c | 84 ++
> drivers/net/wireless/ath/ath10k/ce.h | 14 ++---
> drivers/net/wireless/ath/ath10k/snoc.c | 18 --
> drivers/net/wireless/ath/ath10k/snoc.h | 1 +
> 4 files changed, 51 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/ce.c
> b/drivers/net/wireless/ath/ath10k/ce.c
> index 1e16f263854a..84ec80c6d08f 100644
> --- a/drivers/net/wireless/ath/ath10k/ce.c
> +++ b/drivers/net/wireless/ath/ath10k/ce.c
> @@ -481,38 +481,6 @@ static inline void
> ath10k_ce_engine_int_status_clear(struct ath10k *ar,
> ath10k_ce_write32(ar, ce_ctrl_addr + wm_regs->addr, mask);
> }
>
> -static bool ath10k_ce_engine_int_status_check(struct ath10k *ar, u32
> ce_ctrl_addr,
> - unsigned int mask)
> -{
> - struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs-
> >wm_regs;
> -
> - return ath10k_ce_read32(ar, ce_ctrl_addr + wm_regs->addr) &
> mask;
> -}
> -
> -u32 ath10k_ce_gen_interrupt_summary(struct ath10k *ar)
> -{
> - struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs-
> >wm_regs;
> - struct ath10k_ce_pipe *ce_state;
> - struct ath10k_ce *ce;
> - u32 irq_summary = 0;
> - u32 ctrl_addr;
> - u32 ce_id;
> -
> - ce = ath10k_ce_priv(ar);
> -
> - for (ce_id = 0; ce_id < CE_COUNT; ce_id++) {
> - ce_state = >ce_states[ce_id];
> - ctrl_addr = ce_state->ctrl_addr;
> - if (ath10k_ce_engine_int_status_check(ar, ctrl_addr,
> - wm_regs->cc_mask)) {
> - irq_summary |= BIT(ce_id);
> - }
> - }
> -
> - return irq_summary;
> -}
> -EXPORT_SYMBOL(ath10k_ce_gen_interrupt_summary);
> -
> /*
> * Guts of ath10k_ce_send.
> * The caller takes responsibility for any needed locking.
> @@ -1399,45 +1367,55 @@ static void
> ath10k_ce_per_engine_handler_adjust(struct ath10k_ce_pipe *ce_state)
> ath10k_ce_watermark_intr_disable(ar, ctrl_addr);
> }
>
> -int ath10k_ce_disable_interrupts(struct ath10k *ar)
> +void ath10k_ce_disable_interrupt(struct ath10k *ar, int ce_id)
> {
> struct ath10k_ce *ce = ath10k_ce_priv(ar);
> struct ath10k
RE: [PATCH 2/2] ath10k: Add support for chain1 regulator supply voting
> -Original Message-
> From: Doug Anderson
> Sent: Saturday, June 27, 2020 3:22 AM
> To: Rakesh Pillai ; Kalle Valo
>
> Cc: ath...@lists.infradead.org; open list:OPEN FIRMWARE AND FLATTENED
> DEVICE TREE BINDINGS ; linux-wireless wirel...@vger.kernel.org>; LKML
> Subject: Re: [PATCH 2/2] ath10k: Add support for chain1 regulator supply
> voting
>
> Hi,
>
> On Fri, Jun 26, 2020 at 11:02 AM Rakesh Pillai wrote:
> >
> > Add support to vote for chain-1 voltage regulator
> > in WCN3990.
> >
> > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> >
> > Signed-off-by: Rakesh Pillai
> > ---
> > drivers/net/wireless/ath/ath10k/snoc.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
> b/drivers/net/wireless/ath/ath10k/snoc.c
> > index 645ed5f..407a074 100644
> > --- a/drivers/net/wireless/ath/ath10k/snoc.c
> > +++ b/drivers/net/wireless/ath/ath10k/snoc.c
> > @@ -45,6 +45,7 @@ static const char * const ath10k_regulators[] = {
> > "vdd-1.8-xo",
> > "vdd-1.3-rfa",
> > "vdd-3.3-ch0",
> > + "vdd-3.3-ch1",
>
> Reviewed-by: Douglas Anderson
>
> ...with the slight nit that ${SUBJECT} and description should probably
> call it "chan1" and not "chain1". Presumably the maintainer can fix
> when applying.
>
> -Doug
Hi Doug,
It has to be chain1 only, not chan1.
This is the power supply rail for the wlan-chain1
RE: [PATCH] ath10k: Add history for tracking certain events
> -Original Message- > From: Ben Greear > Sent: Saturday, June 27, 2020 8:58 PM > To: Rakesh Pillai ; ath...@lists.infradead.org > Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org > Subject: Re: [PATCH] ath10k: Add history for tracking certain events > > > > On 06/26/2020 11:22 PM, Rakesh Pillai wrote: > > For debugging many issues, a history of the > > below mentioned events can help get an idea > > of what exactly was going on just before any > > issue occurred in the system. These event > > history will be collected only when the host > > driver is run in debug mode (i.e. with the > > config ATH10K_DEBUG enabled). > > This should be disabled by default unless user specifically pokes some > debugfs > value to turn it on so that it does not impact performance. Hi Ben, This history is enabled only if the user compiles the kernel with ATH10K_DEBUG. Making it runtime, adds a lot of "if" conditions for this history record. Do you suggest to add support to enable/disable it runtime even in ATH10K_DEBUG ? > > Thanks, > Ben > > > > > Add history for tracking the below events > > - register read > > - register write > > - IRQ trigger > > - IRQ Enable > > - IRQ Disable > > - NAPI poll > > - CE service > > - WMI cmd > > - WMI event > > - WMI tx completion > > > > This will help in debugging any crash or any > > improper behaviour. > > > -- > Ben Greear > Candela Technologies Inc http://www.candelatech.com
[PATCH] ath10k: Add history for tracking certain events
For debugging many issues, a history of the
below mentioned events can help get an idea
of what exactly was going on just before any
issue occurred in the system. These event
history will be collected only when the host
driver is run in debug mode (i.e. with the
config ATH10K_DEBUG enabled).
Add history for tracking the below events
- register read
- register write
- IRQ trigger
- IRQ Enable
- IRQ Disable
- NAPI poll
- CE service
- WMI cmd
- WMI event
- WMI tx completion
This will help in debugging any crash or any
improper behaviour.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/ce.c | 2 +
drivers/net/wireless/ath/ath10k/core.h| 77 +
drivers/net/wireless/ath/ath10k/debug.c | 134 ++
drivers/net/wireless/ath/ath10k/debug.h | 74 +
drivers/net/wireless/ath/ath10k/snoc.c| 17
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 1 +
drivers/net/wireless/ath/ath10k/wmi.c | 10 +++
7 files changed, 315 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c
b/drivers/net/wireless/ath/ath10k/ce.c
index 1e16f26..ddf2765 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -509,6 +509,7 @@ u32 ath10k_ce_gen_interrupt_summary(struct ath10k *ar)
}
}
+ ath10k_record_ce_event(ar, ATH10K_IRQ_SUMMARY, irq_summary);
return irq_summary;
}
EXPORT_SYMBOL(ath10k_ce_gen_interrupt_summary);
@@ -1331,6 +1332,7 @@ void ath10k_ce_per_engine_service(struct ath10k *ar,
unsigned int ce_id)
struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
u32 ctrl_addr = ce_state->ctrl_addr;
+ ath10k_record_ce_event(ar, ATH10K_CE_SERVICE, ce_id);
/*
* Clear before handling
*
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 040053a..eecb5cc 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -971,6 +971,78 @@ struct ath10k_bus_params {
bool hl_msdu_ids;
};
+#define ATH10K_REG_ACCESS_HISTORY_MAX 512
+#define ATH10K_CE_EVENT_HISTORY_MAX1024
+#define ATH10K_WMI_EVENT_HISTORY_MAX 512
+#define ATH10K_WMI_CMD_HISTORY_MAX 256
+
+#define ATH10K_WMI_DATA_LEN16
+
+enum ath10k_ce_event {
+ ATH10K_IRQ_ENTRY,
+ ATH10K_IRQ_EXIT,
+ ATH10K_IRQ_ENABLE,
+ ATH10K_IRQ_DISABLE,
+ ATH10K_NAPI_POLL,
+ ATH10K_CE_SERVICE,
+ ATH10K_NAPI_COMPLETE,
+ ATH10K_NAPI_RESCHED,
+ ATH10K_IRQ_SUMMARY,
+};
+
+enum ath10k_wmi_type {
+ ATH10K_WMI_EVENT,
+ ATH10K_WMI_CMD,
+ ATH10K_WMI_TX_COMPL,
+};
+
+struct ath10k_reg_access_entry {
+ bool write;
+ u32 cpu_id;
+ u32 offset;
+ u32 val;
+ u64 timestamp;
+};
+
+struct ath10k_wmi_event_entry {
+ enum ath10k_wmi_type type;
+ u32 cpu_id;
+ u32 id;
+ u64 timestamp;
+ unsigned char data[ATH10K_WMI_DATA_LEN];
+};
+
+struct ath10k_ce_event_entry {
+ enum ath10k_ce_event event_type;
+ u32 cpu_id;
+ u32 ce_id;
+ u64 timestamp;
+};
+
+struct ath10k_wmi_event_history {
+ struct ath10k_wmi_event_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing wmi event history */
+ spinlock_t hist_lock;
+};
+
+struct ath10k_ce_event_history {
+ struct ath10k_ce_event_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing ce event history */
+ spinlock_t hist_lock;
+};
+
+struct ath10k_reg_access_history {
+ struct ath10k_reg_access_entry *record;
+ u32 max_entries;
+ atomic_t index;
+ /* lock for accessing register access history */
+ spinlock_t hist_lock;
+};
+
struct ath10k {
struct ath_common ath_common;
struct ieee80211_hw *hw;
@@ -1263,6 +1335,11 @@ struct ath10k {
bool coex_support;
int coex_gpio_pin;
+ struct ath10k_reg_access_history reg_access_history;
+ struct ath10k_ce_event_history ce_event_history;
+ struct ath10k_wmi_event_history wmi_event_history;
+ struct ath10k_wmi_event_history wmi_cmd_history;
+
/* must be last */
u8 drv_priv[] __aligned(sizeof(void *));
};
diff --git a/drivers/net/wireless/ath/ath10k/debug.c
b/drivers/net/wireless/ath/ath10k/debug.c
index e8250a6..1be4883 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2722,4 +2722,138 @@ void ath10k_dbg_dump(struct ath10k *ar,
}
EXPORT_SYMBOL(ath10k_dbg_dump);
+int ath10k_core_reg_access_history_init(struct ath10k *ar, u32 max_entries)
+{
+ ar->reg_access_history.record = vzalloc(max_entries *
+ sizeof(struct
ath10k_reg_access_entry));
+
[PATCH v2 2/2] ath10k: Skip wait for delete response if firmware is down
Currently the driver waits for response from the
firmware for all the delete cmds, eg: vdev_delete,
peer delete. If the firmware is down, these wait
will always timeout and return an error.
Also during subsytems recovery, any attempt to
send a WMI cmd to the FW will return the -ESHUTDOWN
status, which when returned to mac80211, can cause
unnecessary warnings to be printed on to the console,
as shown below
[ 2559.529565] Call trace:
[ 2559.532214] __sta_info_destroy_part2+0x160/0x168 [mac80211]
[ 2559.538157] __sta_info_flush+0x124/0x180 [mac80211]
[ 2559.543402] ieee80211_set_disassoc+0x130/0x2c0 [mac80211]
[ 2559.549172] ieee80211_mgd_deauth+0x238/0x25c [mac80211]
[ 2559.554764] ieee80211_deauth+0x24/0x30 [mac80211]
[ 2559.559860] cfg80211_mlme_deauth+0x258/0x2b0 [cfg80211]
[ 2559.565446] nl80211_deauthenticate+0xe4/0x110 [cfg80211]
[ 2559.571064] genl_rcv_msg+0x3a0/0x440
[ 2559.574888] netlink_rcv_skb+0xb4/0x11c
[ 2559.578877] genl_rcv+0x34/0x48
[ 2559.582162] netlink_unicast+0x14c/0x1e4
[ 2559.586235] netlink_sendmsg+0x2f0/0x360
[ 2559.590317] sock_sendmsg+0x44/0x5c
[ 2559.593951] sys_sendmsg+0x1c8/0x290
[ 2559.598029] ___sys_sendmsg+0xa8/0xfc
[ 2559.601840] __sys_sendmsg+0x8c/0xd0
[ 2559.605572] __arm64_compat_sys_sendmsg+0x2c/0x38
[ 2559.610468] el0_svc_common+0xa8/0x160
[ 2559.614372] el0_svc_compat_handler+0x2c/0x38
[ 2559.618905] el0_svc_compat+0x8/0x10
Skip the wait for delete response from the
firmware if the firmware is down. Also return
success to the mac80211 calls when the peer delete
cmd fails with return status -ESHUTDOWN.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/mac.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c
b/drivers/net/wireless/ath/ath10k/mac.c
index dc7befc..6882fcc 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -701,7 +701,8 @@ static void ath10k_wait_for_peer_delete_done(struct ath10k
*ar, u32 vdev_id,
unsigned long time_left;
int ret;
- if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)) {
+ if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map) &&
+ !test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
ret = ath10k_wait_for_peer_deleted(ar, vdev_id, addr);
if (ret) {
ath10k_warn(ar, "failed wait for peer deleted");
@@ -841,7 +842,8 @@ static int ath10k_peer_delete(struct ath10k *ar, u32
vdev_id, const u8 *addr)
if (ret)
return ret;
- if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)) {
+ if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map) &&
+ !test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
unsigned long time_left;
time_left = wait_for_completion_timeout
@@ -5673,7 +5675,8 @@ static void ath10k_remove_interface(struct ieee80211_hw
*hw,
ath10k_warn(ar, "failed to delete WMI vdev %i: %d\n",
arvif->vdev_id, ret);
- if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)) {
+ if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map) &&
+ !test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
time_left = wait_for_completion_timeout(>vdev_delete_done,
ATH10K_VDEV_DELETE_TIMEOUT_HZ);
if (time_left == 0) {
@@ -6110,6 +6113,11 @@ static int ath10k_hw_scan(struct ieee80211_hw *hw,
goto exit;
}
+ if (test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
+ ret = -ESHUTDOWN;
+ goto exit;
+ }
+
spin_lock_bh(>data_lock);
switch (ar->scan.state) {
case ATH10K_SCAN_IDLE:
@@ -6758,7 +6766,9 @@ static int ath10k_sta_state(struct ieee80211_hw *hw,
}
ret = ath10k_peer_delete(ar, arvif->vdev_id, sta->addr);
- if (ret)
+ if (ret == -ESHUTDOWN)
+ ret = 0;
+ else if (ret)
ath10k_warn(ar, "failed to delete peer %pM for vdev %d:
%i\n",
sta->addr, arvif->vdev_id, ret);
--
2.7.4
[PATCH v2 1/2] ath10k: Pause the tx queues when firmware is down
When the FW goes down, the transmission of
packets cannot be done. Currently we are not
pausing the tx queues, hence there will be too
many attempts to trasmit packets and each of
them resulting in failure and thereby logging
unwanted failure messages onto the console,
as shown below
[ 60.842192] ath10k_snoc 1880.wifi: failed to transmit packet, dropping:
-108
[ 60.850181] ath10k_snoc 1880.wifi: failed to submit frame: -108
[ 60.857058] ath10k_snoc 1880.wifi: failed to push frame: -108
[ 60.936217] ath10k_snoc 1880.wifi: failed to transmit packet, dropping:
-108
[ 60.944496] ath10k_snoc 1880.wifi: failed to submit frame: -108
[ 60.951265] ath10k_snoc 1880.wifi: failed to push frame: -108
[ 61.976589] ath10k_snoc 1880.wifi: failed to transmit packet, dropping:
-108
Stop the tx queues when the wlan firmware is
down and also remove the unwanted duplicate/extra
logging in the packet transmission path.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 18 ++
drivers/net/wireless/ath/ath10k/snoc.c | 3 +++
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 62b1502..040053a 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -900,6 +900,7 @@ static inline const char *ath10k_scan_state_str(enum
ath10k_scan_state state)
enum ath10k_tx_pause_reason {
ATH10K_TX_PAUSE_Q_FULL,
+ ATH10K_TX_PAUSE_FW_DOWN,
ATH10K_TX_PAUSE_MAX,
};
diff --git a/drivers/net/wireless/ath/ath10k/mac.c
b/drivers/net/wireless/ath/ath10k/mac.c
index 3e38962..dc7befc 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3348,6 +3348,7 @@ void ath10k_mac_tx_lock(struct ath10k *ar, int reason)
ar->tx_paused |= BIT(reason);
ieee80211_stop_queues(ar->hw);
}
+EXPORT_SYMBOL(ath10k_mac_tx_lock);
static void ath10k_mac_tx_unlock_iter(void *data, u8 *mac,
struct ieee80211_vif *vif)
@@ -3378,6 +3379,7 @@ void ath10k_mac_tx_unlock(struct ath10k *ar, int reason)
ieee80211_wake_queue(ar->hw, ar->hw->offchannel_tx_hw_queue);
}
+EXPORT_SYMBOL(ath10k_mac_tx_unlock);
void ath10k_mac_vif_tx_lock(struct ath10k_vif *arvif, int reason)
{
@@ -3748,11 +3750,8 @@ static int ath10k_mac_tx_submit(struct ath10k *ar,
break;
}
- if (ret) {
- ath10k_warn(ar, "failed to transmit packet, dropping: %d\n",
- ret);
+ if (ret)
ieee80211_free_txskb(ar->hw, skb);
- }
return ret;
}
@@ -3806,10 +3805,8 @@ static int ath10k_mac_tx(struct ath10k *ar,
}
ret = ath10k_mac_tx_submit(ar, txmode, txpath, skb);
- if (ret) {
- ath10k_warn(ar, "failed to submit frame: %d\n", ret);
+ if (ret)
return ret;
- }
return 0;
}
@@ -4441,7 +4438,12 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
ret = ath10k_mac_tx(ar, vif, txmode, txpath, skb, false);
if (ret) {
- ath10k_warn(ar, "failed to transmit frame: %d\n", ret);
+ if (ret == -ESHUTDOWN)
+ ath10k_dbg(ar, ATH10K_DBG_MAC, "failed to transmit
frame: %d\n",
+ ret);
+ else
+ ath10k_warn(ar, "failed to transmit frame: %d\n", ret);
+
if (is_htt) {
spin_lock_bh(>htt.tx_lock);
ath10k_htt_tx_dec_pending(htt);
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index 407a074..8440e64 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -20,6 +20,7 @@
#include "hif.h"
#include "htc.h"
#include "snoc.h"
+#include "mac.h"
#define ATH10K_SNOC_RX_POST_RETRY_MS 50
#define CE_POLL_PIPE 4
@@ -1296,6 +1297,7 @@ int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type)
switch (type) {
case ATH10K_QMI_EVENT_FW_READY_IND:
if (test_bit(ATH10K_SNOC_FLAG_REGISTERED, _snoc->flags)) {
+ ath10k_mac_tx_unlock(ar, ATH10K_TX_PAUSE_FW_DOWN);
queue_work(ar->workqueue, >restart_work);
break;
}
@@ -1313,6 +1315,7 @@ int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type)
case ATH10K_QMI_EVENT_FW_DOWN_IND:
set_bit(ATH10K_SNOC_FLAG_RECOVERY, _snoc->flags);
set_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags);
+ ath10
[PATCH v2 0/2] ath10k: Fixes during subsystem recovery
This patch series includes some fixes when the device is in recovery mode, i.e. when the firmware goes down. - Pausing TX queues when FW goes down - Removed unwanted/extra error logging in pkt TX path - Skipping wait for FW response for delete cmds - Handling the -ESHUTDOWN error code in case of SSR. Rakesh Pillai (2): ath10k: Pause the tx queues when firmware is down ath10k: Skip wait for delete response if firmware is down drivers/net/wireless/ath/ath10k/core.h | 1 + drivers/net/wireless/ath/ath10k/mac.c | 36 ++ drivers/net/wireless/ath/ath10k/snoc.c | 3 +++ 3 files changed, 28 insertions(+), 12 deletions(-) -- 2.7.4
RE: [PATCH 2/2] ath10k: Skip wait for delete response if firmware is down
> -Original Message-
> From: Ben Greear
> Sent: Friday, June 26, 2020 11:57 PM
> To: Rakesh Pillai ; ath...@lists.infradead.org
> Cc: linux-wirel...@vger.kernel.org; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH 2/2] ath10k: Skip wait for delete response if firmware
is
> down
>
>
>
> On 06/26/2020 11:11 AM, Rakesh Pillai wrote:
> > Currently the driver waits for response from the
> > firmware for all the delete cmds, eg: vdev_delete,
> > peer delete. If the firmware is down, these wait
> > will always timeout and return an error.
> >
> > Also during subsytems recovery, any attempt to
> > send a WMI cmd to the FW will return the -ESHUTDOWN
> > status, which when returned to mac80211, can cause
> > unnecessary warnings to be printed on to the console,
> > as shown below
> >
> > [ 2559.529565] Call trace:
> > [ 2559.532214] __sta_info_destroy_part2+0x160/0x168 [mac80211]
> > [ 2559.538157] __sta_info_flush+0x124/0x180 [mac80211]
> > [ 2559.543402] ieee80211_set_disassoc+0x130/0x2c0 [mac80211]
> > [ 2559.549172] ieee80211_mgd_deauth+0x238/0x25c [mac80211]
> > [ 2559.554764] ieee80211_deauth+0x24/0x30 [mac80211]
> > [ 2559.559860] cfg80211_mlme_deauth+0x258/0x2b0 [cfg80211]
> > [ 2559.565446] nl80211_deauthenticate+0xe4/0x110 [cfg80211]
> > [ 2559.571064] genl_rcv_msg+0x3a0/0x440
> > [ 2559.574888] netlink_rcv_skb+0xb4/0x11c
> > [ 2559.578877] genl_rcv+0x34/0x48
> > [ 2559.582162] netlink_unicast+0x14c/0x1e4
> > [ 2559.586235] netlink_sendmsg+0x2f0/0x360
> > [ 2559.590317] sock_sendmsg+0x44/0x5c
> > [ 2559.593951] sys_sendmsg+0x1c8/0x290
> > [ 2559.598029] ___sys_sendmsg+0xa8/0xfc
> > [ 2559.601840] __sys_sendmsg+0x8c/0xd0
> > [ 2559.605572] __arm64_compat_sys_sendmsg+0x2c/0x38
> > [ 2559.610468] el0_svc_common+0xa8/0x160
> > [ 2559.614372] el0_svc_compat_handler+0x2c/0x38
> > [ 2559.618905] el0_svc_compat+0x8/0x10
> >
> > Skip the wait for delete response from the
> > firmware if the firmware is down. Also return
> > success to the mac80211 calls when the peer delete
> > cmd fails with return status -ESHUTDOWN.
> >
> > Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
> >
> > Signed-off-by: Rakesh Pillai
> > ---
> > drivers/net/wireless/ath/ath10k/mac.c | 18 ++
> > 1 file changed, 14 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/wireless/ath/ath10k/mac.c
> b/drivers/net/wireless/ath/ath10k/mac.c
> > index dc7befc..7ac6549 100644
> > --- a/drivers/net/wireless/ath/ath10k/mac.c
> > +++ b/drivers/net/wireless/ath/ath10k/mac.c
> > @@ -701,7 +701,8 @@ static void
> ath10k_wait_for_peer_delete_done(struct ath10k *ar, u32 vdev_id,
> > unsigned long time_left;
> > int ret;
> >
> > - if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map))
> {
> > + if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)
> &&
> > + test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
>
> Don't you mean !test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags))
> ???
>
> Or maybe I'm just mis-reading your patch?
Hi Ben,
Yes, it should be !test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)).
I will send out a v2.
>
> Thanks,
> Ben
>
> --
> Ben Greear
> Candela Technologies Inc http://www.candelatech.com
[PATCH 1/2] ath10k: Pause the tx queues when firmware is down
When the FW goes down, the transmission of
packets cannot be done. Currently we are not
pausing the tx queues, hence there will be too
many attempts to trasmit packets and each of
them resulting in failure and thereby logging
unwanted failure messages onto the console,
as shown below
[ 60.842192] ath10k_snoc 1880.wifi: failed to transmit packet, dropping:
-108
[ 60.850181] ath10k_snoc 1880.wifi: failed to submit frame: -108
[ 60.857058] ath10k_snoc 1880.wifi: failed to push frame: -108
[ 60.936217] ath10k_snoc 1880.wifi: failed to transmit packet, dropping:
-108
[ 60.944496] ath10k_snoc 1880.wifi: failed to submit frame: -108
[ 60.951265] ath10k_snoc 1880.wifi: failed to push frame: -108
[ 61.976589] ath10k_snoc 1880.wifi: failed to transmit packet, dropping:
-108
Stop the tx queues when the wlan firmware is
down and also remove the unwanted duplicate/extra
logging in the packet transmission path.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.h | 1 +
drivers/net/wireless/ath/ath10k/mac.c | 18 ++
drivers/net/wireless/ath/ath10k/snoc.c | 3 +++
3 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 62b1502..040053a 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -900,6 +900,7 @@ static inline const char *ath10k_scan_state_str(enum
ath10k_scan_state state)
enum ath10k_tx_pause_reason {
ATH10K_TX_PAUSE_Q_FULL,
+ ATH10K_TX_PAUSE_FW_DOWN,
ATH10K_TX_PAUSE_MAX,
};
diff --git a/drivers/net/wireless/ath/ath10k/mac.c
b/drivers/net/wireless/ath/ath10k/mac.c
index 3e38962..dc7befc 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -3348,6 +3348,7 @@ void ath10k_mac_tx_lock(struct ath10k *ar, int reason)
ar->tx_paused |= BIT(reason);
ieee80211_stop_queues(ar->hw);
}
+EXPORT_SYMBOL(ath10k_mac_tx_lock);
static void ath10k_mac_tx_unlock_iter(void *data, u8 *mac,
struct ieee80211_vif *vif)
@@ -3378,6 +3379,7 @@ void ath10k_mac_tx_unlock(struct ath10k *ar, int reason)
ieee80211_wake_queue(ar->hw, ar->hw->offchannel_tx_hw_queue);
}
+EXPORT_SYMBOL(ath10k_mac_tx_unlock);
void ath10k_mac_vif_tx_lock(struct ath10k_vif *arvif, int reason)
{
@@ -3748,11 +3750,8 @@ static int ath10k_mac_tx_submit(struct ath10k *ar,
break;
}
- if (ret) {
- ath10k_warn(ar, "failed to transmit packet, dropping: %d\n",
- ret);
+ if (ret)
ieee80211_free_txskb(ar->hw, skb);
- }
return ret;
}
@@ -3806,10 +3805,8 @@ static int ath10k_mac_tx(struct ath10k *ar,
}
ret = ath10k_mac_tx_submit(ar, txmode, txpath, skb);
- if (ret) {
- ath10k_warn(ar, "failed to submit frame: %d\n", ret);
+ if (ret)
return ret;
- }
return 0;
}
@@ -4441,7 +4438,12 @@ static void ath10k_mac_op_tx(struct ieee80211_hw *hw,
ret = ath10k_mac_tx(ar, vif, txmode, txpath, skb, false);
if (ret) {
- ath10k_warn(ar, "failed to transmit frame: %d\n", ret);
+ if (ret == -ESHUTDOWN)
+ ath10k_dbg(ar, ATH10K_DBG_MAC, "failed to transmit
frame: %d\n",
+ ret);
+ else
+ ath10k_warn(ar, "failed to transmit frame: %d\n", ret);
+
if (is_htt) {
spin_lock_bh(>htt.tx_lock);
ath10k_htt_tx_dec_pending(htt);
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index 407a074..8440e64 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -20,6 +20,7 @@
#include "hif.h"
#include "htc.h"
#include "snoc.h"
+#include "mac.h"
#define ATH10K_SNOC_RX_POST_RETRY_MS 50
#define CE_POLL_PIPE 4
@@ -1296,6 +1297,7 @@ int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type)
switch (type) {
case ATH10K_QMI_EVENT_FW_READY_IND:
if (test_bit(ATH10K_SNOC_FLAG_REGISTERED, _snoc->flags)) {
+ ath10k_mac_tx_unlock(ar, ATH10K_TX_PAUSE_FW_DOWN);
queue_work(ar->workqueue, >restart_work);
break;
}
@@ -1313,6 +1315,7 @@ int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type)
case ATH10K_QMI_EVENT_FW_DOWN_IND:
set_bit(ATH10K_SNOC_FLAG_RECOVERY, _snoc->flags);
set_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags);
+ ath10
[PATCH 2/2] ath10k: Skip wait for delete response if firmware is down
Currently the driver waits for response from the
firmware for all the delete cmds, eg: vdev_delete,
peer delete. If the firmware is down, these wait
will always timeout and return an error.
Also during subsytems recovery, any attempt to
send a WMI cmd to the FW will return the -ESHUTDOWN
status, which when returned to mac80211, can cause
unnecessary warnings to be printed on to the console,
as shown below
[ 2559.529565] Call trace:
[ 2559.532214] __sta_info_destroy_part2+0x160/0x168 [mac80211]
[ 2559.538157] __sta_info_flush+0x124/0x180 [mac80211]
[ 2559.543402] ieee80211_set_disassoc+0x130/0x2c0 [mac80211]
[ 2559.549172] ieee80211_mgd_deauth+0x238/0x25c [mac80211]
[ 2559.554764] ieee80211_deauth+0x24/0x30 [mac80211]
[ 2559.559860] cfg80211_mlme_deauth+0x258/0x2b0 [cfg80211]
[ 2559.565446] nl80211_deauthenticate+0xe4/0x110 [cfg80211]
[ 2559.571064] genl_rcv_msg+0x3a0/0x440
[ 2559.574888] netlink_rcv_skb+0xb4/0x11c
[ 2559.578877] genl_rcv+0x34/0x48
[ 2559.582162] netlink_unicast+0x14c/0x1e4
[ 2559.586235] netlink_sendmsg+0x2f0/0x360
[ 2559.590317] sock_sendmsg+0x44/0x5c
[ 2559.593951] sys_sendmsg+0x1c8/0x290
[ 2559.598029] ___sys_sendmsg+0xa8/0xfc
[ 2559.601840] __sys_sendmsg+0x8c/0xd0
[ 2559.605572] __arm64_compat_sys_sendmsg+0x2c/0x38
[ 2559.610468] el0_svc_common+0xa8/0x160
[ 2559.614372] el0_svc_compat_handler+0x2c/0x38
[ 2559.618905] el0_svc_compat+0x8/0x10
Skip the wait for delete response from the
firmware if the firmware is down. Also return
success to the mac80211 calls when the peer delete
cmd fails with return status -ESHUTDOWN.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/mac.c | 18 ++
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c
b/drivers/net/wireless/ath/ath10k/mac.c
index dc7befc..7ac6549 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -701,7 +701,8 @@ static void ath10k_wait_for_peer_delete_done(struct ath10k
*ar, u32 vdev_id,
unsigned long time_left;
int ret;
- if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)) {
+ if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map) &&
+ test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
ret = ath10k_wait_for_peer_deleted(ar, vdev_id, addr);
if (ret) {
ath10k_warn(ar, "failed wait for peer deleted");
@@ -841,7 +842,8 @@ static int ath10k_peer_delete(struct ath10k *ar, u32
vdev_id, const u8 *addr)
if (ret)
return ret;
- if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)) {
+ if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map) &&
+ test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
unsigned long time_left;
time_left = wait_for_completion_timeout
@@ -5673,7 +5675,8 @@ static void ath10k_remove_interface(struct ieee80211_hw
*hw,
ath10k_warn(ar, "failed to delete WMI vdev %i: %d\n",
arvif->vdev_id, ret);
- if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map)) {
+ if (test_bit(WMI_SERVICE_SYNC_DELETE_CMDS, ar->wmi.svc_map) &&
+ test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
time_left = wait_for_completion_timeout(>vdev_delete_done,
ATH10K_VDEV_DELETE_TIMEOUT_HZ);
if (time_left == 0) {
@@ -6110,6 +6113,11 @@ static int ath10k_hw_scan(struct ieee80211_hw *hw,
goto exit;
}
+ if (test_bit(ATH10K_FLAG_CRASH_FLUSH, >dev_flags)) {
+ ret = -ESHUTDOWN;
+ goto exit;
+ }
+
spin_lock_bh(>data_lock);
switch (ar->scan.state) {
case ATH10K_SCAN_IDLE:
@@ -6758,7 +6766,9 @@ static int ath10k_sta_state(struct ieee80211_hw *hw,
}
ret = ath10k_peer_delete(ar, arvif->vdev_id, sta->addr);
- if (ret)
+ if (ret == -ESHUTDOWN)
+ ret = 0;
+ else if (ret)
ath10k_warn(ar, "failed to delete peer %pM for vdev %d:
%i\n",
sta->addr, arvif->vdev_id, ret);
--
2.7.4
[PATCH 0/2] ath10k: Fixes during subsystem recovery
This patch series includes some fixes when the device is in recovery mode, i.e. when the firmware goes down. - Pausing TX queues when FW goes down - Removed unwanted/extra error logging in pkt TX path - Skipping wait for FW response for delete cmds - Handling the -ESHUTDOWN error code in case of SSR. Rakesh Pillai (2): ath10k: Pause the tx queues when firmware is down ath10k: Skip wait for delete response if firmware is down drivers/net/wireless/ath/ath10k/core.h | 1 + drivers/net/wireless/ath/ath10k/mac.c | 36 ++ drivers/net/wireless/ath/ath10k/snoc.c | 3 +++ 3 files changed, 28 insertions(+), 12 deletions(-) -- 2.7.4
[PATCH 1/2] dt: bindings: Add new regulator as optional property for WCN3990
Add an additional regulator supply as an optional
property for WCN3990.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
index 65ee68e..b7188d3 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt
@@ -65,7 +65,8 @@ Optional properties:
the length can vary between hw versions.
- -supply: handle to the regulator device tree node
optional "supply-name" are "vdd-0.8-cx-mx",
- "vdd-1.8-xo", "vdd-1.3-rfa" and "vdd-3.3-ch0".
+ "vdd-1.8-xo", "vdd-1.3-rfa", "vdd-3.3-ch0"
+ and "vdd-3.3-ch1".
- memory-region:
Usage: optional
Value type:
@@ -204,6 +205,7 @@ wifi@1800 {
vdd-1.8-xo-supply = <_l7a_1p8>;
vdd-1.3-rfa-supply = <_l17a_1p3>;
vdd-3.3-ch0-supply = <_l25a_3p3>;
+ vdd-3.3-ch1-supply = <_l26a_3p3>;
memory-region = <_msa_mem>;
iommus = <_smmu 0x0040 0x1>;
qcom,msa-fixed-perm;
--
2.7.4
[PATCH 2/2] ath10k: Add support for chain1 regulator supply voting
Add support to vote for chain-1 voltage regulator
in WCN3990.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/snoc.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index 645ed5f..407a074 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -45,6 +45,7 @@ static const char * const ath10k_regulators[] = {
"vdd-1.8-xo",
"vdd-1.3-rfa",
"vdd-3.3-ch0",
+ "vdd-3.3-ch1",
};
static const char * const ath10k_clocks[] = {
--
2.7.4
[PATCH 0/2] ath10k: Add chain-1 voltage regulator voting
Add the support to vote for the chain-1 voltage regulator for WCN3990. This is added as an optional property. Rakesh Pillai (2): dt: bindings: Add new regulator as optional property for WCN3990 ath10k: Add support for chain1 regulator supply voting Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt | 4 +++- drivers/net/wireless/ath/ath10k/snoc.c | 1 + 2 files changed, 4 insertions(+), 1 deletion(-) -- 2.7.4
[PATCH] ath10k: Add interrupt summary based CE processing
Currently the NAPI processing loops through all
the copy engines and processes a particular copy
engine is the copy completion is set for that copy
engine. The host driver is not supposed to access
any copy engine register after clearing the interrupt
status register.
This might result in kernel crash like the one below
[ 1159.220143] Call trace:
[ 1159.220170] ath10k_snoc_read32+0x20/0x40 [ath10k_snoc]
[ 1159.220193] ath10k_ce_per_engine_service_any+0x78/0x130 [ath10k_core]
[ 1159.220203] ath10k_snoc_napi_poll+0x38/0x8c [ath10k_snoc]
[ 1159.220270] net_rx_action+0x100/0x3b0
[ 1159.220312] __do_softirq+0x164/0x30c
[ 1159.220345] run_ksoftirqd+0x2c/0x64
[ 1159.220380] smpboot_thread_fn+0x1b0/0x288
[ 1159.220405] kthread+0x11c/0x12c
[ 1159.220423] ret_from_fork+0x10/0x18
To avoid such a scenario, we generate an interrupt
summary by reading the copy completion for all the
copy engine before actually processing any of them.
This will avoid reading the interrupt status register
for any CE after the interrupt status is cleared.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/ce.c | 63 ++--
drivers/net/wireless/ath/ath10k/ce.h | 5 +--
2 files changed, 42 insertions(+), 26 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/ce.c
b/drivers/net/wireless/ath/ath10k/ce.c
index ffdd4b9..1e16f26 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -481,15 +481,38 @@ static inline void
ath10k_ce_engine_int_status_clear(struct ath10k *ar,
ath10k_ce_write32(ar, ce_ctrl_addr + wm_regs->addr, mask);
}
-static inline bool ath10k_ce_engine_int_status_check(struct ath10k *ar,
-u32 ce_ctrl_addr,
-unsigned int mask)
+static bool ath10k_ce_engine_int_status_check(struct ath10k *ar, u32
ce_ctrl_addr,
+ unsigned int mask)
{
struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
return ath10k_ce_read32(ar, ce_ctrl_addr + wm_regs->addr) & mask;
}
+u32 ath10k_ce_gen_interrupt_summary(struct ath10k *ar)
+{
+ struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
+ struct ath10k_ce_pipe *ce_state;
+ struct ath10k_ce *ce;
+ u32 irq_summary = 0;
+ u32 ctrl_addr;
+ u32 ce_id;
+
+ ce = ath10k_ce_priv(ar);
+
+ for (ce_id = 0; ce_id < CE_COUNT; ce_id++) {
+ ce_state = >ce_states[ce_id];
+ ctrl_addr = ce_state->ctrl_addr;
+ if (ath10k_ce_engine_int_status_check(ar, ctrl_addr,
+ wm_regs->cc_mask)) {
+ irq_summary |= BIT(ce_id);
+ }
+ }
+
+ return irq_summary;
+}
+EXPORT_SYMBOL(ath10k_ce_gen_interrupt_summary);
+
/*
* Guts of ath10k_ce_send.
* The caller takes responsibility for any needed locking.
@@ -1308,32 +1331,24 @@ void ath10k_ce_per_engine_service(struct ath10k *ar,
unsigned int ce_id)
struct ath10k_hw_ce_host_wm_regs *wm_regs = ar->hw_ce_regs->wm_regs;
u32 ctrl_addr = ce_state->ctrl_addr;
- spin_lock_bh(>ce_lock);
-
- if (ath10k_ce_engine_int_status_check(ar, ctrl_addr,
- wm_regs->cc_mask)) {
- /* Clear before handling */
- ath10k_ce_engine_int_status_clear(ar, ctrl_addr,
- wm_regs->cc_mask);
-
- spin_unlock_bh(>ce_lock);
-
- if (ce_state->recv_cb)
- ce_state->recv_cb(ce_state);
-
- if (ce_state->send_cb)
- ce_state->send_cb(ce_state);
-
- spin_lock_bh(>ce_lock);
- }
-
/*
+* Clear before handling
+*
* Misc CE interrupts are not being handled, but still need
* to be cleared.
+*
+* NOTE: When the last copy engine interrupt is cleared the
+* hardware will go to sleep. Once this happens any access to
+* the CE registers can cause a hardware fault.
*/
- ath10k_ce_engine_int_status_clear(ar, ctrl_addr, wm_regs->wm_mask);
+ ath10k_ce_engine_int_status_clear(ar, ctrl_addr,
+ wm_regs->cc_mask | wm_regs->wm_mask);
- spin_unlock_bh(>ce_lock);
+ if (ce_state->recv_cb)
+ ce_state->recv_cb(ce_state);
+
+ if (ce_state->send_cb)
+ ce_state->send_cb(ce_state);
}
EXPORT_SYMBOL(ath10k_ce_per_engine_service);
diff --git a/drivers/net/wireless/ath/ath10k/ce.h
b/drivers/net/wireless/ath/ath10k/ce.h
index 75df79d..a440aaf 100644
--- a/drivers/n
[PATCH] ath10k: Use bdf calibration variant for snoc targets
Board Data File (BDF) is loaded upon driver boot-up procedure.
The right board data file is identified using bus and qmi-board-id.
The problem, however, can occur when the (default) board data
file cannot fulfill with the vendor requirements and it is
necessary to use a different board data file.
Add the support to get the variant field from DTSI and
use tht information to load the vendor specific BDF.
The device tree requires addition strings to define the variant name
wifi@a00 {
status = "okay";
qcom,ath10k-calibration-variant = "xyz-v2";
};
wifi@a80 {
status = "okay";
qcom,ath10k-calibration-variant = "xyz-v1";
};
This would create the boarddata identifiers for the board-2.bin search
* bus=snoc,qmi-board-id=16,qmi-chip-id=0,variant=xyz-v1
* bus=snoc,qmi-board-id=17,qmi-chip-id=0,variant=xyz-v2
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/core.c | 18 +-
drivers/net/wireless/ath/ath10k/core.h | 2 ++
drivers/net/wireless/ath/ath10k/qmi.c | 8
3 files changed, 23 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c
b/drivers/net/wireless/ath/ath10k/core.c
index 22b6937..bd60913 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -1024,7 +1024,7 @@ static int ath10k_core_check_smbios(struct ath10k *ar)
return 0;
}
-static int ath10k_core_check_dt(struct ath10k *ar)
+int ath10k_core_check_dt(struct ath10k *ar)
{
struct device_node *node;
const char *variant = NULL;
@@ -1045,6 +1045,7 @@ static int ath10k_core_check_dt(struct ath10k *ar)
return 0;
}
+EXPORT_SYMBOL(ath10k_core_check_dt);
static int ath10k_download_fw(struct ath10k *ar)
{
@@ -1439,10 +1440,17 @@ static int ath10k_core_create_board_name(struct ath10k
*ar, char *name,
}
if (ar->id.qmi_ids_valid) {
- scnprintf(name, name_len,
- "bus=%s,qmi-board-id=%x",
- ath10k_bus_str(ar->hif.bus),
- ar->id.qmi_board_id);
+ if (with_variant && ar->id.bdf_ext[0] != '\0')
+ scnprintf(name, name_len,
+ "bus=%s,qmi-board-id=%x,qmi-chip-id=%x%s",
+ ath10k_bus_str(ar->hif.bus),
+ ar->id.qmi_board_id, ar->id.qmi_chip_id,
+ variant);
+ else
+ scnprintf(name, name_len,
+ "bus=%s,qmi-board-id=%x",
+ ath10k_bus_str(ar->hif.bus),
+ ar->id.qmi_board_id);
goto out;
}
diff --git a/drivers/net/wireless/ath/ath10k/core.h
b/drivers/net/wireless/ath/ath10k/core.h
index 5c18f6c..62b1502 100644
--- a/drivers/net/wireless/ath/ath10k/core.h
+++ b/drivers/net/wireless/ath/ath10k/core.h
@@ -1056,6 +1056,7 @@ struct ath10k {
bool bmi_ids_valid;
bool qmi_ids_valid;
u32 qmi_board_id;
+ u32 qmi_chip_id;
u8 bmi_board_id;
u8 bmi_eboard_id;
u8 bmi_chip_id;
@@ -1295,6 +1296,7 @@ int ath10k_core_register(struct ath10k *ar,
const struct ath10k_bus_params *bus_params);
void ath10k_core_unregister(struct ath10k *ar);
int ath10k_core_fetch_board_file(struct ath10k *ar, int bd_ie_type);
+int ath10k_core_check_dt(struct ath10k *ar);
void ath10k_core_free_board_files(struct ath10k *ar);
#endif /* _CORE_H_ */
diff --git a/drivers/net/wireless/ath/ath10k/qmi.c
b/drivers/net/wireless/ath/ath10k/qmi.c
index 5468a41..ae6b1f4 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -576,6 +576,8 @@ static int ath10k_qmi_cap_send_sync_msg(struct ath10k_qmi
*qmi)
if (resp->chip_info_valid) {
qmi->chip_info.chip_id = resp->chip_info.chip_id;
qmi->chip_info.chip_family = resp->chip_info.chip_family;
+ } else {
+ qmi->chip_info.chip_id = 0xFF;
}
if (resp->board_info_valid)
@@ -817,12 +819,18 @@ static void ath10k_qmi_event_server_arrive(struct
ath10k_qmi *qmi)
static int ath10k_qmi_fetch_board_file(struct ath10k_qmi *qmi)
{
struct ath10k *ar = qmi->ar;
+ int ret;
ar->hif.bus = ATH10K_BUS_SNOC;
ar->id.qmi_ids_valid = true;
ar->id.qmi_board_id = qmi->board_info.board_id;
+ ar->id.qmi_chip_id = qmi->chip_info.chip_id;
ar->hw_params.fw.dir = WCN3990_HW_1_0_FW_DIR;
+ ret = ath10k_core_check_dt(ar);
+
[PATCH] ath10k: Register shutdown handler
As a part of device shutdown the smmu driver will be
stopped and henceforth any IOVA address translation
will not be done. The wlan driver, being one of the
smmu driver consumer, should stop all the dma related
activity as a part of shutdown, and thereby ensuring
that no dma activity is done once the smmu driver
shuts down.
During the device shutdown, the smmu calls shutdown
for all its consumers in order to indicate them to
stop all their dma activities.
Register the shutdown handler to stop the wlan
driver and avoid any dma operations.
Tested-on: WCN3990 hw1.0 SNOC WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/snoc.c | 9 +
1 file changed, 9 insertions(+)
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c
b/drivers/net/wireless/ath/ath10k/snoc.c
index 354d49b..645ed5f 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -1772,9 +1772,18 @@ static int ath10k_snoc_remove(struct platform_device
*pdev)
return 0;
}
+static void ath10k_snoc_shutdown(struct platform_device *pdev)
+{
+ struct ath10k *ar = platform_get_drvdata(pdev);
+
+ ath10k_dbg(ar, ATH10K_DBG_SNOC, "snoc shutdown\n");
+ ath10k_snoc_remove(pdev);
+}
+
static struct platform_driver ath10k_snoc_driver = {
.probe = ath10k_snoc_probe,
.remove = ath10k_snoc_remove,
+ .shutdown = ath10k_snoc_shutdown,
.driver = {
.name = "ath10k_snoc",
.of_match_table = ath10k_snoc_dt_match,
--
2.7.4
RE: [REPOST PATCH] arm64: dts: qcom: Fix WiFi supplies on sc7180-idp
> -Original Message-
> From: Douglas Anderson
> Sent: Friday, June 26, 2020 1:47 AM
> To: Andy Gross ; Bjorn Andersson
>
> Cc: Evan Green ; Sibi Sankar
> ; Rakesh Pillai ; Douglas
> Anderson ; Rob Herring ;
> devicet...@vger.kernel.org; linux-arm-...@vger.kernel.org; linux-
> ker...@vger.kernel.org
> Subject: [REPOST PATCH] arm64: dts: qcom: Fix WiFi supplies on sc7180-idp
>
> The WiFi supplies that were added recently can't have done anything
> useful because they were missing the "-supply" suffix. Booting
> without the "-supply" suffix would give these messages:
>
> ath10k_snoc 1880.wifi: 1880.wifi supply vdd-0.8-cx-mx not found,
> using dummy regulator
> ath10k_snoc 1880.wifi: 1880.wifi supply vdd-1.8-xo not found,
using
> dummy regulator
> ath10k_snoc 1880.wifi: 1880.wifi supply vdd-1.3-rfa not found,
using
> dummy regulator
> ath10k_snoc 1880.wifi: 1880.wifi supply vdd-3.3-ch0 not found,
> using dummy regulator
>
> Let's add the "-supply" suffix.
>
> Fixes: 1e7594a38f37 ("arm64: dts: qcom: sc7180: Add WCN3990 WLAN
> module device node")
> Signed-off-by: Douglas Anderson
> ---
Reviewed-by: Rakesh Pillai
I missed this in the DTSI patch, while manually rebasing to the kernel tip.
> I don't have an IDP setup but I have a similar board. Testing on IDP
> would, of course, be appreciated.
>
> Repost because I screwed up the "after-the-cut" notes on first post.
>
> arch/arm64/boot/dts/qcom/sc7180-idp.dts | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> index 39dbfc89689e..472f7f41cc93 100644
> --- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> +++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
> @@ -391,10 +391,10 @@ video-firmware {
>
> {
> status = "okay";
> - vdd-0.8-cx-mx = <_l9a_0p6>;
> - vdd-1.8-xo = <_l1c_1p8>;
> - vdd-1.3-rfa = <_l2c_1p3>;
> - vdd-3.3-ch0 = <_l10c_3p3>;
> + vdd-0.8-cx-mx-supply = <_l9a_0p6>;
> + vdd-1.8-xo-supply = <_l1c_1p8>;
> + vdd-1.3-rfa-supply = <_l2c_1p3>;
> + vdd-3.3-ch0-supply = <_l10c_3p3>;
> wifi-firmware {
> iommus = <_smmu 0xc2 0x1>;
> };
> --
> 2.27.0.212.ge8ba1cc988-goog
RE: [PATCH v11] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
> -Original Message- > From: Bjorn Andersson > Sent: Monday, June 22, 2020 12:36 AM > To: Evan Green > Cc: Rakesh Pillai ; open list:OPEN FIRMWARE AND > FLATTENED DEVICE TREE BINDINGS ; linux-arm > Mailing List ; LKML ker...@vger.kernel.org>; linux-arm-msm m...@vger.kernel.org> > Subject: Re: [PATCH v11] arm64: dts: qcom: sc7180: Add WCN3990 WLAN > module device node > > On Wed 17 Jun 15:45 PDT 2020, Evan Green wrote: > > > On Thu, May 21, 2020 at 9:19 AM Evan Green > wrote: > > > > > > On Tue, May 19, 2020 at 8:57 PM Rakesh Pillai > wrote: > > > > > > > > Add device node for the ath10k SNOC platform driver probe > > > > and add resources required for WCN3990 on sc7180 soc. > > > > > > > > Signed-off-by: Rakesh Pillai > > > > > > Reviewed-by: Evan Green > > > > Looks like this never landed anywhere. Is it blocked on something? > > I remember waiting for some reviews, but I see those are in place. Then > as I was applying this I saw that a v12 had shown up, with regulators. > > So, I applied v12. Thanks Bjorn. > > Thanks, > Bjorn
[PATCH v12] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
Add device node for the ath10k SNOC platform driver probe
and add resources required for WCN3990 on sc7180 soc.
Signed-off-by: Rakesh Pillai
---
Changes from v11:
- Add the optional regulator votes which are needed in case of SSR.
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 11 +++
arch/arm64/boot/dts/qcom/sc7180.dtsi| 22 ++
2 files changed, 33 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 4e9149d..39dbfc8 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -389,6 +389,17 @@
};
};
+ {
+ status = "okay";
+ vdd-0.8-cx-mx = <_l9a_0p6>;
+ vdd-1.8-xo = <_l1c_1p8>;
+ vdd-1.3-rfa = <_l2c_1p3>;
+ vdd-3.3-ch0 = <_l10c_3p3>;
+ wifi-firmware {
+ iommus = <_smmu 0xc2 0x1>;
+ };
+};
+
/* PINCTRL - additions to nodes defined in sc7180.dtsi */
_clk {
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 31b9217..cd6d3b5 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2814,6 +2814,28 @@
#freq-domain-cells = <1>;
};
+
+ wifi: wifi@1880 {
+ compatible = "qcom,wcn3990-wifi";
+ reg = <0 0x1880 0 0x80>;
+ reg-names = "membase";
+ iommus = <_smmu 0xc0 0x1>;
+ interrupts =
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ;
+ memory-region = <_mem>;
+ status = "disabled";
+ };
};
thermal-zones {
--
2.7.4
RE: [PATCH v11] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
Hi Evan, All the dependent patches are already merged. Thanks, Rakesh Pillai. > -Original Message- > From: Evan Green > Sent: Thursday, June 18, 2020 4:15 AM > To: Rakesh Pillai ; Bjorn Andersson > > Cc: open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS > ; linux-arm Mailing List ker...@lists.infradead.org>; LKML ; linux- > arm-msm > Subject: Re: [PATCH v11] arm64: dts: qcom: sc7180: Add WCN3990 WLAN > module device node > > On Thu, May 21, 2020 at 9:19 AM Evan Green > wrote: > > > > On Tue, May 19, 2020 at 8:57 PM Rakesh Pillai > wrote: > > > > > > Add device node for the ath10k SNOC platform driver probe > > > and add resources required for WCN3990 on sc7180 soc. > > > > > > Signed-off-by: Rakesh Pillai > > > > Reviewed-by: Evan Green > > Looks like this never landed anywhere. Is it blocked on something? > -Evan
[PATCH v11] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
Add device node for the ath10k SNOC platform driver probe
and add resources required for WCN3990 on sc7180 soc.
Signed-off-by: Rakesh Pillai
---
Changes from v10:
- Corrected the position of wifi node, as per address
- Removed the wlan_fw_mem from reserved memory, since
its already added as reserved memory in board DT file.
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 7 +++
arch/arm64/boot/dts/qcom/sc7180.dtsi| 22 ++
2 files changed, 29 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 4e9149d..38b102e 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -389,6 +389,13 @@
};
};
+ {
+ status = "okay";
+ wifi-firmware {
+ iommus = <_smmu 0xc2 0x1>;
+ };
+};
+
/* PINCTRL - additions to nodes defined in sc7180.dtsi */
_clk {
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index 6b12c60..da79f8f 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -2811,6 +2811,28 @@
#freq-domain-cells = <1>;
};
+
+ wifi: wifi@1880 {
+ compatible = "qcom,wcn3990-wifi";
+ reg = <0 0x1880 0 0x80>;
+ reg-names = "membase";
+ iommus = <_smmu 0xc0 0x1>;
+ interrupts =
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ;
+ memory-region = <_mem>;
+ status = "disabled";
+ };
};
thermal-zones {
--
2.7.4
[PATCH v10] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
Add device node for the ath10k SNOC platform driver probe
and add resources required for WCN3990 on sc7180 soc.
Signed-off-by: Rakesh Pillai
---
Changes from v9:
- Place the wlan_fw_mem under reserved-memory node
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 7 +++
arch/arm64/boot/dts/qcom/sc7180.dtsi| 27 +++
2 files changed, 34 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 4e9149d..38b102e 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -389,6 +389,13 @@
};
};
+ {
+ status = "okay";
+ wifi-firmware {
+ iommus = <_smmu 0xc2 0x1>;
+ };
+};
+
/* PINCTRL - additions to nodes defined in sc7180.dtsi */
_clk {
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index f1280e0..19bd7d0 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -106,6 +106,11 @@
no-map;
};
+ wlan_fw_mem: memory@9410 {
+ reg = <0 0x9410 0 0x20>;
+ no-map;
+ };
+
rmtfs_mem: memory@8440 {
compatible = "qcom,rmtfs-mem";
reg = <0x0 0x8440 0x0 0x20>;
@@ -944,6 +949,28 @@
};
};
+ wifi: wifi@1880 {
+ compatible = "qcom,wcn3990-wifi";
+ reg = <0 0x1880 0 0x80>;
+ reg-names = "membase";
+ iommus = <_smmu 0xc0 0x1>;
+ interrupts =
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ;
+ memory-region = <_fw_mem>;
+ status = "disabled";
+ };
+
config_noc: interconnect@150 {
compatible = "qcom,sc7180-config-noc";
reg = <0 0x0150 0 0x28000>;
--
2.7.4
[PATCH v9] arm64: dts: qcom: sc7180: Add WCN3990 WLAN module device node
Add device node for the ath10k SNOC platform driver probe
and add resources required for WCN3990 on sc7180 soc.
Signed-off-by: Rakesh Pillai
---
Changes from v8:
- Removed the qcom,msa-fixed-perm
---
arch/arm64/boot/dts/qcom/sc7180-idp.dts | 7 +++
arch/arm64/boot/dts/qcom/sc7180.dtsi| 27 +++
2 files changed, 34 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
index 4e9149d..38b102e 100644
--- a/arch/arm64/boot/dts/qcom/sc7180-idp.dts
+++ b/arch/arm64/boot/dts/qcom/sc7180-idp.dts
@@ -389,6 +389,13 @@
};
};
+ {
+ status = "okay";
+ wifi-firmware {
+ iommus = <_smmu 0xc2 0x1>;
+ };
+};
+
/* PINCTRL - additions to nodes defined in sc7180.dtsi */
_clk {
diff --git a/arch/arm64/boot/dts/qcom/sc7180.dtsi
b/arch/arm64/boot/dts/qcom/sc7180.dtsi
index f1280e0..dd4e095 100644
--- a/arch/arm64/boot/dts/qcom/sc7180.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7180.dtsi
@@ -63,6 +63,11 @@
clock-frequency = <32764>;
#clock-cells = <0>;
};
+
+ wlan_fw_mem: memory@9410 {
+ reg = <0 0x9410 0 0x20>;
+ no-map;
+ };
};
reserved_memory: reserved-memory {
@@ -944,6 +949,28 @@
};
};
+ wifi: wifi@1880 {
+ compatible = "qcom,wcn3990-wifi";
+ reg = <0 0x1880 0 0x80>;
+ reg-names = "membase";
+ iommus = <_smmu 0xc0 0x1>;
+ interrupts =
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ,
+ ;
+ memory-region = <_fw_mem>;
+ status = "disabled";
+ };
+
config_noc: interconnect@150 {
compatible = "qcom,sc7180-config-noc";
reg = <0 0x0150 0 0x28000>;
--
2.7.4
[PATCH v2] ath10k: Remove msdu from idr when management pkt send fails
Currently when the sending of any management pkt
via wmi command fails, the packet is being unmapped
freed in the error handling. But the idr entry added,
which is used to track these packet is not getting removed.
Hence, during unload, in wmi cleanup, all the entries
in IDR are removed and the corresponding buffer is
attempted to be freed. This can cause a situation where
one packet is attempted to be freed twice.
Fix this error by rmeoving the msdu from the idr
list when the sending of a management packet over
wmi fails.
Tested HW: WCN3990
Tested FW: WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Fixes: 1807da49733e ("ath10k: wmi: add management tx by reference support over
wmi")
Signed-off-by: Rakesh Pillai
---
Changes from v1:
- Added a helper function in wmi-ops for cleanup_mgmt_tx_send
---
drivers/net/wireless/ath/ath10k/wmi-ops.h | 15 ++-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 15 +++
2 files changed, 29 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h
b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index 1491c25..8c3a656 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -133,6 +133,7 @@ struct wmi_ops {
struct sk_buff *(*gen_mgmt_tx_send)(struct ath10k *ar,
struct sk_buff *skb,
dma_addr_t paddr);
+ int (*cleanup_mgmt_tx_send)(struct ath10k *ar, struct sk_buff *msdu);
struct sk_buff *(*gen_dbglog_cfg)(struct ath10k *ar, u64 module_enable,
u32 log_level);
struct sk_buff *(*gen_pktlog_enable)(struct ath10k *ar, u32 filter);
@@ -442,6 +443,15 @@ ath10k_wmi_get_txbf_conf_scheme(struct ath10k *ar)
}
static inline int
+ath10k_wmi_cleanup_mgmt_tx_send(struct ath10k *ar, struct sk_buff *msdu)
+{
+ if (!ar->wmi.ops->cleanup_mgmt_tx_send)
+ return -EOPNOTSUPP;
+
+ return ar->wmi.ops->cleanup_mgmt_tx_send(ar, msdu);
+}
+
+static inline int
ath10k_wmi_mgmt_tx_send(struct ath10k *ar, struct sk_buff *msdu,
dma_addr_t paddr)
{
@@ -457,8 +467,11 @@ ath10k_wmi_mgmt_tx_send(struct ath10k *ar, struct sk_buff
*msdu,
ret = ath10k_wmi_cmd_send(ar, skb,
ar->wmi.cmd->mgmt_tx_send_cmdid);
- if (ret)
+ if (ret) {
+ /* remove this msdu from idr tracking */
+ ath10k_wmi_cleanup_mgmt_tx_send(ar, msdu);
return ret;
+ }
return 0;
}
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e1ab900f..2a31a42 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -2898,6 +2898,18 @@ ath10k_wmi_tlv_op_gen_request_stats(struct ath10k *ar,
u32 stats_mask)
}
static int
+ath10k_wmi_tlv_op_cleanup_mgmt_tx_send(struct ath10k *ar,
+ struct sk_buff *msdu)
+{
+ struct ath10k_skb_cb *cb = ATH10K_SKB_CB(msdu);
+ struct ath10k_wmi *wmi = >wmi;
+
+ idr_remove(>mgmt_pending_tx, cb->msdu_id);
+
+ return 0;
+}
+
+static int
ath10k_wmi_mgmt_tx_alloc_msdu_id(struct ath10k *ar, struct sk_buff *skb,
dma_addr_t paddr)
{
@@ -2971,6 +2983,8 @@ ath10k_wmi_tlv_op_gen_mgmt_tx_send(struct ath10k *ar,
struct sk_buff *msdu,
if (desc_id < 0)
goto err_free_skb;
+ cb->msdu_id = desc_id;
+
ptr = (void *)skb->data;
tlv = ptr;
tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_MGMT_TX_CMD);
@@ -4419,6 +4433,7 @@ static const struct wmi_ops wmi_tlv_ops = {
.gen_force_fw_hang = ath10k_wmi_tlv_op_gen_force_fw_hang,
/* .gen_mgmt_tx = not implemented; HTT is used */
.gen_mgmt_tx_send = ath10k_wmi_tlv_op_gen_mgmt_tx_send,
+ .cleanup_mgmt_tx_send = ath10k_wmi_tlv_op_cleanup_mgmt_tx_send,
.gen_dbglog_cfg = ath10k_wmi_tlv_op_gen_dbglog_cfg,
.gen_pktlog_enable = ath10k_wmi_tlv_op_gen_pktlog_enable,
.gen_pktlog_disable = ath10k_wmi_tlv_op_gen_pktlog_disable,
--
2.7.4
[PATCH] ath10k: Skip handling del_server during driver exit
The qmi infrastructure sends the client a del_server
event when the client releases its qmi handle. This
is not the msg indicating the actual qmi server exiting.
In such cases the del_server msg should not be processed,
since the wifi firmware does not reset its qmi state.
Hence skip the processing of del_server event when the
driver is unloading.
Tested HW: WCN3990
Tested FW: WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Fixes: ba94c753ccb4 ("ath10k: add QMI message handshake for wcn3990 client")
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/qmi.c | 13 -
drivers/net/wireless/ath/ath10k/qmi.h | 6 ++
2 files changed, 18 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/qmi.c
b/drivers/net/wireless/ath/ath10k/qmi.c
index 85dce43..7abdef8 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.c
+++ b/drivers/net/wireless/ath/ath10k/qmi.c
@@ -961,7 +961,16 @@ static void ath10k_qmi_del_server(struct qmi_handle
*qmi_hdl,
container_of(qmi_hdl, struct ath10k_qmi, qmi_hdl);
qmi->fw_ready = false;
- ath10k_qmi_driver_event_post(qmi, ATH10K_QMI_EVENT_SERVER_EXIT, NULL);
+
+ /*
+* The del_server event is to be processed only if coming from
+* the qmi server. The qmi infrastructure sends del_server, when
+* any client releases the qmi handle. In this case do not process
+* this del_server event.
+*/
+ if (qmi->state == ATH10K_QMI_STATE_INIT_DONE)
+ ath10k_qmi_driver_event_post(qmi, ATH10K_QMI_EVENT_SERVER_EXIT,
+NULL);
}
static struct qmi_ops ath10k_qmi_ops = {
@@ -1091,6 +1100,7 @@ int ath10k_qmi_init(struct ath10k *ar, u32 msa_size)
if (ret)
goto err_qmi_lookup;
+ qmi->state = ATH10K_QMI_STATE_INIT_DONE;
return 0;
err_qmi_lookup:
@@ -1109,6 +1119,7 @@ int ath10k_qmi_deinit(struct ath10k *ar)
struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar);
struct ath10k_qmi *qmi = ar_snoc->qmi;
+ qmi->state = ATH10K_QMI_STATE_DEINIT;
qmi_handle_release(>qmi_hdl);
cancel_work_sync(>event_work);
destroy_workqueue(qmi->event_wq);
diff --git a/drivers/net/wireless/ath/ath10k/qmi.h
b/drivers/net/wireless/ath/ath10k/qmi.h
index dc25737..b597205 100644
--- a/drivers/net/wireless/ath/ath10k/qmi.h
+++ b/drivers/net/wireless/ath/ath10k/qmi.h
@@ -83,6 +83,11 @@ struct ath10k_qmi_driver_event {
void *data;
};
+enum ath10k_qmi_state {
+ ATH10K_QMI_STATE_INIT_DONE,
+ ATH10K_QMI_STATE_DEINIT,
+};
+
struct ath10k_qmi {
struct ath10k *ar;
struct qmi_handle qmi_hdl;
@@ -105,6 +110,7 @@ struct ath10k_qmi {
char fw_build_timestamp[MAX_TIMESTAMP_LEN + 1];
struct ath10k_qmi_cal_data cal_data[MAX_NUM_CAL_V01];
bool msa_fixed_perm;
+ enum ath10k_qmi_state state;
};
int ath10k_qmi_wlan_enable(struct ath10k *ar,
--
2.7.4
[PATCH] ath10k: Remove msdu from idr when management pkt send fails
Currently when the sending of any management pkt
via wmi command fails, the packet is being unmapped
freed in the error handling. But the idr entry added,
which is used to track these packet is not getting removed.
Hence, during unload, in wmi cleanup, all the entries
in IDR are removed and the corresponding buffer is
attempted to be freed. This can cause a situation where
one packet is attempted to be freed twice.
Fix this error by rmeoving the msdu from the idr
list when the sending of a management packet over
wmi fails.
Tested HW: WCN3990
Tested FW: WLAN.HL.3.1-01040-QCAHLSWMTPLZ-1
Fixes: 1807da49733e ("ath10k: wmi: add management tx by reference support over
wmi")
Signed-off-by: Rakesh Pillai
---
drivers/net/wireless/ath/ath10k/wmi-ops.h | 7 ++-
drivers/net/wireless/ath/ath10k/wmi-tlv.c | 15 +++
2 files changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/ath10k/wmi-ops.h
b/drivers/net/wireless/ath/ath10k/wmi-ops.h
index 1491c25..de15e62 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-ops.h
+++ b/drivers/net/wireless/ath/ath10k/wmi-ops.h
@@ -216,6 +216,7 @@ struct wmi_ops {
struct sk_buff *(*gen_bb_timing)
(struct ath10k *ar,
const struct wmi_bb_timing_cfg_arg *arg);
+ int (*cleanup_mgmt_tx_send)(struct ath10k *ar, struct sk_buff *msdu);
};
@@ -457,8 +458,12 @@ ath10k_wmi_mgmt_tx_send(struct ath10k *ar, struct sk_buff
*msdu,
ret = ath10k_wmi_cmd_send(ar, skb,
ar->wmi.cmd->mgmt_tx_send_cmdid);
- if (ret)
+ if (ret) {
+ /* remove this msdu from idr tracking */
+ if (ar->wmi.ops->cleanup_mgmt_tx_send)
+ ar->wmi.ops->cleanup_mgmt_tx_send(ar, msdu);
return ret;
+ }
return 0;
}
diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e1ab900f..b2a4a44 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -2898,6 +2898,18 @@ ath10k_wmi_tlv_op_gen_request_stats(struct ath10k *ar,
u32 stats_mask)
}
static int
+ath10k_wmi_tlv_op_cleanup_mgmt_tx_send(struct ath10k *ar,
+ struct sk_buff *msdu)
+{
+ struct ath10k_skb_cb *cb = ATH10K_SKB_CB(msdu);
+ struct ath10k_wmi *wmi = >wmi;
+
+ idr_remove(>mgmt_pending_tx, cb->msdu_id);
+
+ return 0;
+}
+
+static int
ath10k_wmi_mgmt_tx_alloc_msdu_id(struct ath10k *ar, struct sk_buff *skb,
dma_addr_t paddr)
{
@@ -2971,6 +2983,8 @@ ath10k_wmi_tlv_op_gen_mgmt_tx_send(struct ath10k *ar,
struct sk_buff *msdu,
if (desc_id < 0)
goto err_free_skb;
+ cb->msdu_id = desc_id;
+
ptr = (void *)skb->data;
tlv = ptr;
tlv->tag = __cpu_to_le16(WMI_TLV_TAG_STRUCT_MGMT_TX_CMD);
@@ -4447,6 +4461,7 @@ static const struct wmi_ops wmi_tlv_ops = {
.gen_echo = ath10k_wmi_tlv_op_gen_echo,
.gen_vdev_spectral_conf = ath10k_wmi_tlv_op_gen_vdev_spectral_conf,
.gen_vdev_spectral_enable = ath10k_wmi_tlv_op_gen_vdev_spectral_enable,
+ .cleanup_mgmt_tx_send = ath10k_wmi_tlv_op_cleanup_mgmt_tx_send,
};
static const struct wmi_peer_flags_map wmi_tlv_peer_flags_map = {
--
2.7.4
