diff mbox series

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

Message ID 20230721055305.20420-2-quic_wgong@quicinc.com
State Superseded
Headers show
Series wifi: ath12k: add support device recovery for WCN7850 | expand

Commit Message

Wen Gong July 21, 2023, 5:53 a.m. UTC
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(+)

Comments

Kalle Valo Aug. 3, 2023, 9:24 a.m. UTC | #1
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.
Wen Gong Aug. 3, 2023, 9:40 a.m. UTC | #2
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.
Kalle Valo Aug. 3, 2023, 11:32 a.m. UTC | #3
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."
Wen Gong Aug. 7, 2023, 2:56 a.m. UTC | #4
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.
Kalle Valo Oct. 12, 2023, 4:07 p.m. UTC | #5
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
diff mbox series

Patch

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) {