* [PATCH v2] ath10k: fix module load regression with iram-recovery feature
@ 2021-10-20 7:50 ` Kalle Valo
0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2021-10-20 7:50 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
From: Abinaya Kalaiselvan <akalaise@codeaurora.org>
Commit 9af7c32ceca8 ("ath10k: add target IRAM recovery feature support")
introduced a new firmware feature flag ATH10K_FW_FEATURE_IRAM_RECOVERY. But
this caused ath10k_pci module load to fail if ATH10K_FW_CRASH_DUMP_RAM_DATA bit
was not enabled in the ath10k coredump_mask module parameter:
[ 2209.328190] ath10k_pci 0000:02:00.0: qca9984/qca9994 hw1.0 target 0x01000000 chip_id 0x00000000 sub 168c:cafe
[ 2209.434414] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 1 testmode 1
[ 2209.547191] ath10k_pci 0000:02:00.0: firmware ver 10.4-3.9.0.2-00099 api 5 features no-p2p,mfp,peer-flow-ctrl,btcoex-param,allows-mesh-bcast,no-ps,peer-fixed-rate,iram-recovery crc32 cbade90a
[ 2210.896485] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id 0:1 crc32 a040efc2
[ 2213.603339] ath10k_pci 0000:02:00.0: failed to copy target iram contents: -12
[ 2213.839027] ath10k_pci 0000:02:00.0: could not init core (-12)
[ 2213.933910] ath10k_pci 0000:02:00.0: could not probe fw (-12)
And by default coredump_mask does not have ATH10K_FW_CRASH_DUMP_RAM_DATA
enabled so anyone using a firmware with iram-recovery feature would fail. To my
knowledge only QCA9984 firmwares starting from release 10.4-3.9.0.2-00099
enabled the feature.
The reason for regression was that ath10k_core_copy_target_iram() used
ath10k_coredump_get_mem_layout() to get the memory layout, but when
ATH10K_FW_CRASH_DUMP_RAM_DATA was disabled it would get just NULL and bail out
with an error.
While looking at all this I noticed another bug: if CONFIG_DEV_COREDUMP is
disabled but the firmware has iram-recovery enabled the module load fails with
similar error messages. I fixed that by returning 0 from
ath10k_core_copy_target_iram() when _ath10k_coredump_get_mem_layout() returns
NULL.
Tested-on: QCA9984 hw2.0 PCI 10.4-3.9.0.2-00139
Fixes: 9af7c32ceca8 ("ath10k: add target IRAM recovery feature support")
Signed-off-by: Abinaya Kalaiselvan <akalaise@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
v2:
* rewrite the commit log
* more understandable naming for functions
* fix both the compile and runtime issues when CONFIG_DEV_COREDUMP is disabled
drivers/net/wireless/ath/ath10k/core.c | 11 +++++++++--
drivers/net/wireless/ath/ath10k/coredump.c | 11 ++++++++---
drivers/net/wireless/ath/ath10k/coredump.h | 7 +++++++
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index c21e05549f61..d0b37c21d7f8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2690,9 +2690,16 @@ static int ath10k_core_copy_target_iram(struct ath10k *ar)
int i, ret;
u32 len, remaining_len;
- hw_mem = ath10k_coredump_get_mem_layout(ar);
+ /* copy target iram feature must work also when
+ * ATH10K_FW_CRASH_DUMP_RAM_DATA is disabled, so
+ * _ath10k_coredump_get_mem_layout() to accomplist that
+ */
+ hw_mem = _ath10k_coredump_get_mem_layout(ar);
if (!hw_mem)
- return -ENOMEM;
+ /* if CONFIG_DEV_COREDUMP is disabled we get NULL, then
+ * just silently disable the feature by doing nothing
+ */
+ return 0;
for (i = 0; i < hw_mem->region_table.size; i++) {
tmp = &hw_mem->region_table.regions[i];
diff --git a/drivers/net/wireless/ath/ath10k/coredump.c b/drivers/net/wireless/ath/ath10k/coredump.c
index 7eb72290a925..55e7e11d06d9 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.c
+++ b/drivers/net/wireless/ath/ath10k/coredump.c
@@ -1447,11 +1447,17 @@ static u32 ath10k_coredump_get_ramdump_size(struct ath10k *ar)
const struct ath10k_hw_mem_layout *ath10k_coredump_get_mem_layout(struct ath10k *ar)
{
- int i;
-
if (!test_bit(ATH10K_FW_CRASH_DUMP_RAM_DATA, &ath10k_coredump_mask))
return NULL;
+ return _ath10k_coredump_get_mem_layout(ar);
+}
+EXPORT_SYMBOL(ath10k_coredump_get_mem_layout);
+
+const struct ath10k_hw_mem_layout *_ath10k_coredump_get_mem_layout(struct ath10k *ar)
+{
+ int i;
+
if (WARN_ON(ar->target_version == 0))
return NULL;
@@ -1464,7 +1470,6 @@ const struct ath10k_hw_mem_layout *ath10k_coredump_get_mem_layout(struct ath10k
return NULL;
}
-EXPORT_SYMBOL(ath10k_coredump_get_mem_layout);
struct ath10k_fw_crash_data *ath10k_coredump_new(struct ath10k *ar)
{
diff --git a/drivers/net/wireless/ath/ath10k/coredump.h b/drivers/net/wireless/ath/ath10k/coredump.h
index 42404e246e0e..240d70515088 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.h
+++ b/drivers/net/wireless/ath/ath10k/coredump.h
@@ -176,6 +176,7 @@ int ath10k_coredump_register(struct ath10k *ar);
void ath10k_coredump_unregister(struct ath10k *ar);
void ath10k_coredump_destroy(struct ath10k *ar);
+const struct ath10k_hw_mem_layout *_ath10k_coredump_get_mem_layout(struct ath10k *ar);
const struct ath10k_hw_mem_layout *ath10k_coredump_get_mem_layout(struct ath10k *ar);
#else /* CONFIG_DEV_COREDUMP */
@@ -214,6 +215,12 @@ ath10k_coredump_get_mem_layout(struct ath10k *ar)
return NULL;
}
+static inline const struct ath10k_hw_mem_layout *
+_ath10k_coredump_get_mem_layout(struct ath10k *ar)
+{
+ return NULL;
+}
+
#endif /* CONFIG_DEV_COREDUMP */
#endif /* _COREDUMP_H_ */
--
2.20.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2] ath10k: fix module load regression with iram-recovery feature
@ 2021-10-20 7:50 ` Kalle Valo
0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2021-10-20 7:50 UTC (permalink / raw)
To: ath10k; +Cc: linux-wireless
From: Abinaya Kalaiselvan <akalaise@codeaurora.org>
Commit 9af7c32ceca8 ("ath10k: add target IRAM recovery feature support")
introduced a new firmware feature flag ATH10K_FW_FEATURE_IRAM_RECOVERY. But
this caused ath10k_pci module load to fail if ATH10K_FW_CRASH_DUMP_RAM_DATA bit
was not enabled in the ath10k coredump_mask module parameter:
[ 2209.328190] ath10k_pci 0000:02:00.0: qca9984/qca9994 hw1.0 target 0x01000000 chip_id 0x00000000 sub 168c:cafe
[ 2209.434414] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 1 testmode 1
[ 2209.547191] ath10k_pci 0000:02:00.0: firmware ver 10.4-3.9.0.2-00099 api 5 features no-p2p,mfp,peer-flow-ctrl,btcoex-param,allows-mesh-bcast,no-ps,peer-fixed-rate,iram-recovery crc32 cbade90a
[ 2210.896485] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id 0:1 crc32 a040efc2
[ 2213.603339] ath10k_pci 0000:02:00.0: failed to copy target iram contents: -12
[ 2213.839027] ath10k_pci 0000:02:00.0: could not init core (-12)
[ 2213.933910] ath10k_pci 0000:02:00.0: could not probe fw (-12)
And by default coredump_mask does not have ATH10K_FW_CRASH_DUMP_RAM_DATA
enabled so anyone using a firmware with iram-recovery feature would fail. To my
knowledge only QCA9984 firmwares starting from release 10.4-3.9.0.2-00099
enabled the feature.
The reason for regression was that ath10k_core_copy_target_iram() used
ath10k_coredump_get_mem_layout() to get the memory layout, but when
ATH10K_FW_CRASH_DUMP_RAM_DATA was disabled it would get just NULL and bail out
with an error.
While looking at all this I noticed another bug: if CONFIG_DEV_COREDUMP is
disabled but the firmware has iram-recovery enabled the module load fails with
similar error messages. I fixed that by returning 0 from
ath10k_core_copy_target_iram() when _ath10k_coredump_get_mem_layout() returns
NULL.
Tested-on: QCA9984 hw2.0 PCI 10.4-3.9.0.2-00139
Fixes: 9af7c32ceca8 ("ath10k: add target IRAM recovery feature support")
Signed-off-by: Abinaya Kalaiselvan <akalaise@codeaurora.org>
Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
---
v2:
* rewrite the commit log
* more understandable naming for functions
* fix both the compile and runtime issues when CONFIG_DEV_COREDUMP is disabled
drivers/net/wireless/ath/ath10k/core.c | 11 +++++++++--
drivers/net/wireless/ath/ath10k/coredump.c | 11 ++++++++---
drivers/net/wireless/ath/ath10k/coredump.h | 7 +++++++
3 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index c21e05549f61..d0b37c21d7f8 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -2690,9 +2690,16 @@ static int ath10k_core_copy_target_iram(struct ath10k *ar)
int i, ret;
u32 len, remaining_len;
- hw_mem = ath10k_coredump_get_mem_layout(ar);
+ /* copy target iram feature must work also when
+ * ATH10K_FW_CRASH_DUMP_RAM_DATA is disabled, so
+ * _ath10k_coredump_get_mem_layout() to accomplist that
+ */
+ hw_mem = _ath10k_coredump_get_mem_layout(ar);
if (!hw_mem)
- return -ENOMEM;
+ /* if CONFIG_DEV_COREDUMP is disabled we get NULL, then
+ * just silently disable the feature by doing nothing
+ */
+ return 0;
for (i = 0; i < hw_mem->region_table.size; i++) {
tmp = &hw_mem->region_table.regions[i];
diff --git a/drivers/net/wireless/ath/ath10k/coredump.c b/drivers/net/wireless/ath/ath10k/coredump.c
index 7eb72290a925..55e7e11d06d9 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.c
+++ b/drivers/net/wireless/ath/ath10k/coredump.c
@@ -1447,11 +1447,17 @@ static u32 ath10k_coredump_get_ramdump_size(struct ath10k *ar)
const struct ath10k_hw_mem_layout *ath10k_coredump_get_mem_layout(struct ath10k *ar)
{
- int i;
-
if (!test_bit(ATH10K_FW_CRASH_DUMP_RAM_DATA, &ath10k_coredump_mask))
return NULL;
+ return _ath10k_coredump_get_mem_layout(ar);
+}
+EXPORT_SYMBOL(ath10k_coredump_get_mem_layout);
+
+const struct ath10k_hw_mem_layout *_ath10k_coredump_get_mem_layout(struct ath10k *ar)
+{
+ int i;
+
if (WARN_ON(ar->target_version == 0))
return NULL;
@@ -1464,7 +1470,6 @@ const struct ath10k_hw_mem_layout *ath10k_coredump_get_mem_layout(struct ath10k
return NULL;
}
-EXPORT_SYMBOL(ath10k_coredump_get_mem_layout);
struct ath10k_fw_crash_data *ath10k_coredump_new(struct ath10k *ar)
{
diff --git a/drivers/net/wireless/ath/ath10k/coredump.h b/drivers/net/wireless/ath/ath10k/coredump.h
index 42404e246e0e..240d70515088 100644
--- a/drivers/net/wireless/ath/ath10k/coredump.h
+++ b/drivers/net/wireless/ath/ath10k/coredump.h
@@ -176,6 +176,7 @@ int ath10k_coredump_register(struct ath10k *ar);
void ath10k_coredump_unregister(struct ath10k *ar);
void ath10k_coredump_destroy(struct ath10k *ar);
+const struct ath10k_hw_mem_layout *_ath10k_coredump_get_mem_layout(struct ath10k *ar);
const struct ath10k_hw_mem_layout *ath10k_coredump_get_mem_layout(struct ath10k *ar);
#else /* CONFIG_DEV_COREDUMP */
@@ -214,6 +215,12 @@ ath10k_coredump_get_mem_layout(struct ath10k *ar)
return NULL;
}
+static inline const struct ath10k_hw_mem_layout *
+_ath10k_coredump_get_mem_layout(struct ath10k *ar)
+{
+ return NULL;
+}
+
#endif /* CONFIG_DEV_COREDUMP */
#endif /* _COREDUMP_H_ */
--
2.20.1
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ath10k: fix module load regression with iram-recovery feature
2021-10-20 7:50 ` Kalle Valo
@ 2021-10-25 13:05 ` Kalle Valo
-1 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2021-10-25 13:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath10k, linux-wireless
Kalle Valo <kvalo@codeaurora.org> wrote:
> Commit 9af7c32ceca8 ("ath10k: add target IRAM recovery feature support")
> introduced a new firmware feature flag ATH10K_FW_FEATURE_IRAM_RECOVERY. But
> this caused ath10k_pci module load to fail if ATH10K_FW_CRASH_DUMP_RAM_DATA bit
> was not enabled in the ath10k coredump_mask module parameter:
>
> [ 2209.328190] ath10k_pci 0000:02:00.0: qca9984/qca9994 hw1.0 target 0x01000000 chip_id 0x00000000 sub 168c:cafe
> [ 2209.434414] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 1 testmode 1
> [ 2209.547191] ath10k_pci 0000:02:00.0: firmware ver 10.4-3.9.0.2-00099 api 5 features no-p2p,mfp,peer-flow-ctrl,btcoex-param,allows-mesh-bcast,no-ps,peer-fixed-rate,iram-recovery crc32 cbade90a
> [ 2210.896485] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id 0:1 crc32 a040efc2
> [ 2213.603339] ath10k_pci 0000:02:00.0: failed to copy target iram contents: -12
> [ 2213.839027] ath10k_pci 0000:02:00.0: could not init core (-12)
> [ 2213.933910] ath10k_pci 0000:02:00.0: could not probe fw (-12)
>
> And by default coredump_mask does not have ATH10K_FW_CRASH_DUMP_RAM_DATA
> enabled so anyone using a firmware with iram-recovery feature would fail. To my
> knowledge only QCA9984 firmwares starting from release 10.4-3.9.0.2-00099
> enabled the feature.
>
> The reason for regression was that ath10k_core_copy_target_iram() used
> ath10k_coredump_get_mem_layout() to get the memory layout, but when
> ATH10K_FW_CRASH_DUMP_RAM_DATA was disabled it would get just NULL and bail out
> with an error.
>
> While looking at all this I noticed another bug: if CONFIG_DEV_COREDUMP is
> disabled but the firmware has iram-recovery enabled the module load fails with
> similar error messages. I fixed that by returning 0 from
> ath10k_core_copy_target_iram() when _ath10k_coredump_get_mem_layout() returns
> NULL.
>
> Tested-on: QCA9984 hw2.0 PCI 10.4-3.9.0.2-00139
>
> Fixes: 9af7c32ceca8 ("ath10k: add target IRAM recovery feature support")
> Signed-off-by: Abinaya Kalaiselvan <akalaise@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
6f8c8bf4c7c9 ath10k: fix module load regression with iram-recovery feature
--
https://patchwork.kernel.org/project/linux-wireless/patch/20211020075054.23061-1-kvalo@codeaurora.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ath10k: fix module load regression with iram-recovery feature
@ 2021-10-25 13:05 ` Kalle Valo
0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2021-10-25 13:05 UTC (permalink / raw)
To: Kalle Valo; +Cc: ath10k, linux-wireless
Kalle Valo <kvalo@codeaurora.org> wrote:
> Commit 9af7c32ceca8 ("ath10k: add target IRAM recovery feature support")
> introduced a new firmware feature flag ATH10K_FW_FEATURE_IRAM_RECOVERY. But
> this caused ath10k_pci module load to fail if ATH10K_FW_CRASH_DUMP_RAM_DATA bit
> was not enabled in the ath10k coredump_mask module parameter:
>
> [ 2209.328190] ath10k_pci 0000:02:00.0: qca9984/qca9994 hw1.0 target 0x01000000 chip_id 0x00000000 sub 168c:cafe
> [ 2209.434414] ath10k_pci 0000:02:00.0: kconfig debug 1 debugfs 1 tracing 1 dfs 1 testmode 1
> [ 2209.547191] ath10k_pci 0000:02:00.0: firmware ver 10.4-3.9.0.2-00099 api 5 features no-p2p,mfp,peer-flow-ctrl,btcoex-param,allows-mesh-bcast,no-ps,peer-fixed-rate,iram-recovery crc32 cbade90a
> [ 2210.896485] ath10k_pci 0000:02:00.0: board_file api 1 bmi_id 0:1 crc32 a040efc2
> [ 2213.603339] ath10k_pci 0000:02:00.0: failed to copy target iram contents: -12
> [ 2213.839027] ath10k_pci 0000:02:00.0: could not init core (-12)
> [ 2213.933910] ath10k_pci 0000:02:00.0: could not probe fw (-12)
>
> And by default coredump_mask does not have ATH10K_FW_CRASH_DUMP_RAM_DATA
> enabled so anyone using a firmware with iram-recovery feature would fail. To my
> knowledge only QCA9984 firmwares starting from release 10.4-3.9.0.2-00099
> enabled the feature.
>
> The reason for regression was that ath10k_core_copy_target_iram() used
> ath10k_coredump_get_mem_layout() to get the memory layout, but when
> ATH10K_FW_CRASH_DUMP_RAM_DATA was disabled it would get just NULL and bail out
> with an error.
>
> While looking at all this I noticed another bug: if CONFIG_DEV_COREDUMP is
> disabled but the firmware has iram-recovery enabled the module load fails with
> similar error messages. I fixed that by returning 0 from
> ath10k_core_copy_target_iram() when _ath10k_coredump_get_mem_layout() returns
> NULL.
>
> Tested-on: QCA9984 hw2.0 PCI 10.4-3.9.0.2-00139
>
> Fixes: 9af7c32ceca8 ("ath10k: add target IRAM recovery feature support")
> Signed-off-by: Abinaya Kalaiselvan <akalaise@codeaurora.org>
> Signed-off-by: Jouni Malinen <jouni@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
Patch applied to ath-next branch of ath.git, thanks.
6f8c8bf4c7c9 ath10k: fix module load regression with iram-recovery feature
--
https://patchwork.kernel.org/project/linux-wireless/patch/20211020075054.23061-1-kvalo@codeaurora.org/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-10-25 13:06 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20 7:50 [PATCH v2] ath10k: fix module load regression with iram-recovery feature Kalle Valo
2021-10-20 7:50 ` Kalle Valo
2021-10-25 13:05 ` Kalle Valo
2021-10-25 13:05 ` Kalle Valo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.