[PATCH v2 0/3] wifi: ath12k: add support device recovery for WCN7850

[PATCH v2 0/3] wifi: ath12k: add support device recovery for WCN7850

[Wen Gong]

Add support for device recovery, e.g. firmware crashed.

depends on MHI patch which has applied to mhi-next:
[PATCH v4] bus: mhi: host: Skip MHI reset if device is in RDDM
https://lore.kernel.org/all/1684390959-17836-1-git-send-email-quic_qianyu@quicinc.com/

v2: change subject and commit log of "wifi: ath12k: configure RDDM size to MHI for device recovery"

Wen Gong (3):
  wifi: ath12k: configure RDDM size to MHI for device recovery
  wifi: ath12k: add ath12k_qmi_free_resource() for recovery
  wifi: ath12k: fix invalid m3 buffer address

 drivers/net/wireless/ath/ath12k/core.c | 1 +
 drivers/net/wireless/ath/ath12k/hw.c   | 3 +++
 drivers/net/wireless/ath/ath12k/hw.h   | 1 +
 drivers/net/wireless/ath/ath12k/mhi.c  | 1 +
 drivers/net/wireless/ath/ath12k/qmi.c  | 7 +++++++
 drivers/net/wireless/ath/ath12k/qmi.h  | 1 +
 6 files changed, 14 insertions(+)


base-commit: b21fe5be53eb873c02e7479372726c8aeed171e3
-- 
2.40.1


-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k

[PATCH v2 1/3] wifi: ath12k: configure RDDM size to MHI for device recovery

[Wen Gong]

RDDM is RAM DUMP DEBUG module, it is used to debug issues when firmware
encounters an error.

The rddm_size is needed by firmware while MHI enter RDDM state. Add it
to support device recovery when ath12k receive MHI_CB_EE_RDDM message.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/hw.c  | 3 +++
 drivers/net/wireless/ath/ath12k/hw.h  | 1 +
 drivers/net/wireless/ath/ath12k/mhi.c | 1 +
 3 files changed, 5 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/hw.c b/drivers/net/wireless/ath/ath12k/hw.c
index 5991cc91cd00..c69cfca67f7d 100644
--- a/drivers/net/wireless/ath/ath12k/hw.c
+++ b/drivers/net/wireless/ath/ath12k/hw.c
@@ -907,6 +907,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = {
 		.hal_ops = &hal_qcn9274_ops,
 
 		.qmi_cnss_feature_bitmap = BIT(CNSS_QDSS_CFG_MISS_V01),
+		.rddm_size = 0,
 	},
 	{
 		.name = "wcn7850 hw2.0",
@@ -964,6 +965,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = {
 
 		.qmi_cnss_feature_bitmap = BIT(CNSS_QDSS_CFG_MISS_V01) |
 					   BIT(CNSS_PCIE_PERST_NO_PULL_V01),
+		.rddm_size = 0x780000,
 	},
 	{
 		.name = "qcn9274 hw2.0",
@@ -1019,6 +1021,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = {
 		.hal_ops = &hal_qcn9274_ops,
 
 		.qmi_cnss_feature_bitmap = BIT(CNSS_QDSS_CFG_MISS_V01),
+		.rddm_size = 0,
 	},
 };
 
diff --git a/drivers/net/wireless/ath/ath12k/hw.h b/drivers/net/wireless/ath/ath12k/hw.h
index e6c4223c283c..7d82b76ad021 100644
--- a/drivers/net/wireless/ath/ath12k/hw.h
+++ b/drivers/net/wireless/ath/ath12k/hw.h
@@ -186,6 +186,7 @@ struct ath12k_hw_params {
 	const struct hal_ops *hal_ops;
 
 	u64 qmi_cnss_feature_bitmap;
+	u32 rddm_size;
 };
 
 struct ath12k_hw_ops {
diff --git a/drivers/net/wireless/ath/ath12k/mhi.c b/drivers/net/wireless/ath/ath12k/mhi.c
index 42f1140baa4f..91b2abf6da44 100644
--- a/drivers/net/wireless/ath/ath12k/mhi.c
+++ b/drivers/net/wireless/ath/ath12k/mhi.c
@@ -366,6 +366,7 @@ int ath12k_mhi_register(struct ath12k_pci *ab_pci)
 	mhi_ctrl->fw_image = ab_pci->amss_path;
 	mhi_ctrl->regs = ab->mem;
 	mhi_ctrl->reg_len = ab->mem_len;
+	mhi_ctrl->rddm_size = ab->hw_params->rddm_size;
 
 	ret = ath12k_mhi_get_msi(ab_pci);
 	if (ret) {
-- 
2.40.1


-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k

Re: [PATCH v2 1/3] wifi: ath12k: configure RDDM size to MHI for device recovery

[Kalle Valo]

Wen Gong <quic_wgong@quicinc.com> wrote:

> RDDM is RAM DUMP DEBUG module, it is used to debug issues when firmware
> encounters an error.
> 
> The rddm_size is needed by firmware while MHI enter RDDM state. Add it
> to support device recovery when ath12k receive MHI_CB_EE_RDDM message.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
> 
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

I'm not sure what "support device recovery" means exactly. How does this patch
change functionality from user's point of view?

No need to resend because of this, I can add that to the commit log.

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230721055305.20420-2-quic_wgong@quicinc.com/

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


-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k

Re: [PATCH v2 1/3] wifi: ath12k: configure RDDM size to MHI for device recovery

[Wen Gong]

On 8/3/2023 5:24 PM, Kalle Valo wrote:
> Wen Gong <quic_wgong@quicinc.com> wrote:
>
>> RDDM is RAM DUMP DEBUG module, it is used to debug issues when firmware
>> encounters an error.
>>
>> The rddm_size is needed by firmware while MHI enter RDDM state. Add it
>> to support device recovery when ath12k receive MHI_CB_EE_RDDM message.
>>
>> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>>
>> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
> I'm not sure what "support device recovery" means exactly. How does this patch
> change functionality from user's point of view?
>
> No need to resend because of this, I can add that to the commit log.
Device recovery means SSR(subsystem restart), when firmware happen 
crash, ath12k
will receive the RDDM event, and then ath12k/mac80211 begin to re-start 
wifi/firmware,
after that, the wifi become normal again.

This patch is to let firmware report RDDM event correctly to ath12k. 
Without this patch,
firmware will not report RDDM event to ath12k correctly, then ath12k 
will not begin SSR
process.

I think it should be changed like this:
The rddm_size is needed by firmware while MHI enter RDDM state. Add it
and then firmware will report MHI_CB_EE_RDDM correctly while firmware
encounters an error, then ath12k could start the device recovery process.

-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k

Re: [PATCH v2 1/3] wifi: ath12k: configure RDDM size to MHI for device recovery

[Kalle Valo]

Wen Gong <quic_wgong@quicinc.com> writes:

> On 8/3/2023 5:24 PM, Kalle Valo wrote:
>> Wen Gong <quic_wgong@quicinc.com> wrote:
>>
>>> RDDM is RAM DUMP DEBUG module, it is used to debug issues when firmware
>>> encounters an error.
>>>
>>> The rddm_size is needed by firmware while MHI enter RDDM state. Add it
>>> to support device recovery when ath12k receive MHI_CB_EE_RDDM message.
>>>
>>> Tested-on: WCN7850 hw2.0 PCI
>>> WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>>>
>>> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>> I'm not sure what "support device recovery" means exactly. How does this patch
>> change functionality from user's point of view?
>>
>> No need to resend because of this, I can add that to the commit log.
> Device recovery means SSR(subsystem restart), when firmware happen
> crash, ath12k
> will receive the RDDM event, and then ath12k/mac80211 begin to
> re-start wifi/firmware,
> after that, the wifi become normal again.
>
> This patch is to let firmware report RDDM event correctly to ath12k.
> Without this patch,
> firmware will not report RDDM event to ath12k correctly, then ath12k
> will not begin SSR
> process.
>
> I think it should be changed like this:
>
> The rddm_size is needed by firmware while MHI enter RDDM state. Add it
> and then firmware will report MHI_CB_EE_RDDM correctly while firmware
> encounters an error, then ath12k could start the device recovery process.

How about this:

"RDDM is Ram Dump Debug Module which is used to debug issues when the
firmware encounters an error. The rddm_size is needed by the firmware
while MHI goes to the RDDM state. Provide the size to MHI subsystem so
that the firmware restart works when the firmware crashes."

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

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

-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k

Re: [PATCH v2 1/3] wifi: ath12k: configure RDDM size to MHI for device recovery

[Wen Gong]

On 8/3/2023 7:32 PM, Kalle Valo wrote:
> Wen Gong <quic_wgong@quicinc.com> writes:
>
>> On 8/3/2023 5:24 PM, Kalle Valo wrote:
>>> Wen Gong <quic_wgong@quicinc.com> wrote:
>>>
>>>> RDDM is RAM DUMP DEBUG module, it is used to debug issues when firmware
>>>> encounters an error.
>>>>
>>>> The rddm_size is needed by firmware while MHI enter RDDM state. Add it
>>>> to support device recovery when ath12k receive MHI_CB_EE_RDDM message.
>>>>
>>>> Tested-on: WCN7850 hw2.0 PCI
>>>> WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
>>>>
>>>> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
>>>> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>
>>> I'm not sure what "support device recovery" means exactly. How does this patch
>>> change functionality from user's point of view?
>>>
>>> No need to resend because of this, I can add that to the commit log.
>> Device recovery means SSR(subsystem restart), when firmware happen
>> crash, ath12k
>> will receive the RDDM event, and then ath12k/mac80211 begin to
>> re-start wifi/firmware,
>> after that, the wifi become normal again.
>>
>> This patch is to let firmware report RDDM event correctly to ath12k.
>> Without this patch,
>> firmware will not report RDDM event to ath12k correctly, then ath12k
>> will not begin SSR
>> process.
>>
>> I think it should be changed like this:
>>
>> The rddm_size is needed by firmware while MHI enter RDDM state. Add it
>> and then firmware will report MHI_CB_EE_RDDM correctly while firmware
>> encounters an error, then ath12k could start the device recovery process.
> How about this:
>
> "RDDM is Ram Dump Debug Module which is used to debug issues when the
> firmware encounters an error. The rddm_size is needed by the firmware
> while MHI goes to the RDDM state. Provide the size to MHI subsystem so
> that the firmware restart works when the firmware crashes."

Thanks.

Yes, it is OK for me.


-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k

Re: [PATCH v2 1/3] wifi: ath12k: configure RDDM size to MHI for device recovery

[Kalle Valo]

Wen Gong <quic_wgong@quicinc.com> wrote:

> RDDM is Ram Dump Debug Module which is used to debug issues when the
> firmware encounters an error. The rddm_size is needed by the firmware
> while MHI goes to the RDDM state. Provide the size to MHI subsystem so
> that the firmware restart works when the firmware crashes.
> 
> Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4
> 
> Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
> Signed-off-by: Kalle Valo <quic_kvalo@quicinc.com>

3 patches applied to ath-next branch of ath.git, thanks.

ae3ed72020de wifi: ath12k: configure RDDM size to MHI for device recovery
92448f8718ba wifi: ath12k: add ath12k_qmi_free_resource() for recovery
c42c2b8224c4 wifi: ath12k: fix invalid m3 buffer address

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20230721055305.20420-2-quic_wgong@quicinc.com/

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


-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k

[PATCH v2 2/3] wifi: ath12k: add ath12k_qmi_free_resource() for recovery

[Wen Gong]

ath12k_qmi_free_target_mem_chunk() and ath12k_qmi_m3_free() is static
in qmi.c, they are needed for recovery, export them in a new function

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/core.c | 1 +
 drivers/net/wireless/ath/ath12k/qmi.c  | 6 ++++++
 drivers/net/wireless/ath/ath12k/qmi.h  | 1 +
 3 files changed, 8 insertions(+)

diff --git a/drivers/net/wireless/ath/ath12k/core.c b/drivers/net/wireless/ath/ath12k/core.c
index 3df8059d5512..85e49f9ff4c4 100644
--- a/drivers/net/wireless/ath/ath12k/core.c
+++ b/drivers/net/wireless/ath/ath12k/core.c
@@ -839,6 +839,7 @@ static void ath12k_core_reset(struct work_struct *work)
 						ATH12K_RECOVER_START_TIMEOUT_HZ);
 
 	ath12k_hif_power_down(ab);
+	ath12k_qmi_free_resource(ab);
 	ath12k_hif_power_up(ab);
 
 	ath12k_dbg(ab, ATH12K_DBG_BOOT, "reset started\n");
diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index 4afba76b90fe..d0da9b63a114 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -3089,3 +3089,9 @@ void ath12k_qmi_deinit_service(struct ath12k_base *ab)
 	ath12k_qmi_m3_free(ab);
 	ath12k_qmi_free_target_mem_chunk(ab);
 }
+
+void ath12k_qmi_free_resource(struct ath12k_base *ab)
+{
+	ath12k_qmi_free_target_mem_chunk(ab);
+	ath12k_qmi_m3_free(ab);
+}
diff --git a/drivers/net/wireless/ath/ath12k/qmi.h b/drivers/net/wireless/ath/ath12k/qmi.h
index df76149c49f5..6d4a5ab72d45 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.h
+++ b/drivers/net/wireless/ath/ath12k/qmi.h
@@ -566,5 +566,6 @@ void ath12k_qmi_event_work(struct work_struct *work);
 void ath12k_qmi_msg_recv_work(struct work_struct *work);
 void ath12k_qmi_deinit_service(struct ath12k_base *ab);
 int ath12k_qmi_init_service(struct ath12k_base *ab);
+void ath12k_qmi_free_resource(struct ath12k_base *ab);
 
 #endif
-- 
2.40.1


-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k

[PATCH v2 3/3] wifi: ath12k: fix invalid m3 buffer address

[Wen Gong]

This is to fix m3 buffer reuse issue as m3_mem->size isn't set to
ZERO in free function, which leads invalid m3 downloading to
firmware and firmware crashed.

Tested-on: WCN7850 hw2.0 PCI WLAN.HMT.1.0-03427-QCAHMTSWPL_V1.0_V2.0_SILICONZ-1.15378.4

Signed-off-by: Wen Gong <quic_wgong@quicinc.com>
---
 drivers/net/wireless/ath/ath12k/qmi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c
index d0da9b63a114..edc45d78b5d5 100644
--- a/drivers/net/wireless/ath/ath12k/qmi.c
+++ b/drivers/net/wireless/ath/ath12k/qmi.c
@@ -2535,6 +2535,7 @@ static void ath12k_qmi_m3_free(struct ath12k_base *ab)
 	dma_free_coherent(ab->dev, m3_mem->size,
 			  m3_mem->vaddr, m3_mem->paddr);
 	m3_mem->vaddr = NULL;
+	m3_mem->size = 0;
 }
 
 static int ath12k_qmi_wlanfw_m3_info_send(struct ath12k_base *ab)
-- 
2.40.1


-- 
ath12k mailing list
ath12k@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/ath12k