WCN6750 uses static window mapping to access the HW registers.
Unlike QCN9074 which uses 3rd window for UMAC and 2nd window
for CE register access, WCN6750 uses 1st window for UMAC
and 2nd window for CE registers.
Also, refactor the code so that WCN6750 can use the existing
ath11k_pci_read32/write32() APIs for accessing the registers.
Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1
Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-00192-QCAHKSWPL_SILICONZ-1
Signed-off-by: Manikanta Pubbisetty <[email protected]>
---
drivers/net/wireless/ath/ath11k/core.c | 15 +++++++
drivers/net/wireless/ath/ath11k/hw.h | 2 +
drivers/net/wireless/ath/ath11k/pcic.c | 54 +++++++++-----------------
3 files changed, 36 insertions(+), 35 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index dc1084532cef..3be4327b4d9c 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -107,6 +107,8 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fixed_mem_region = true,
.static_window_map = false,
.hybrid_bus_type = false,
+ .dp_window_idx = 0,
+ .ce_window_idx = 0,
},
{
.hw_rev = ATH11K_HW_IPQ6018_HW10,
@@ -179,6 +181,8 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fixed_mem_region = true,
.static_window_map = false,
.hybrid_bus_type = false,
+ .dp_window_idx = 0,
+ .ce_window_idx = 0,
},
{
.name = "qca6390 hw2.0",
@@ -250,6 +254,8 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fixed_mem_region = false,
.static_window_map = false,
.hybrid_bus_type = false,
+ .dp_window_idx = 0,
+ .ce_window_idx = 0,
},
{
.name = "qcn9074 hw1.0",
@@ -321,6 +327,8 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fixed_mem_region = false,
.static_window_map = true,
.hybrid_bus_type = false,
+ .dp_window_idx = 3,
+ .ce_window_idx = 2,
},
{
.name = "wcn6855 hw2.0",
@@ -392,6 +400,8 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fixed_mem_region = false,
.static_window_map = false,
.hybrid_bus_type = false,
+ .dp_window_idx = 0,
+ .ce_window_idx = 0,
},
{
.name = "wcn6855 hw2.1",
@@ -462,6 +472,8 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.fixed_mem_region = false,
.static_window_map = false,
.hybrid_bus_type = false,
+ .dp_window_idx = 0,
+ .ce_window_idx = 0,
},
{
.name = "wcn6750 hw1.0",
@@ -527,7 +539,10 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
.m3_fw_support = false,
.fixed_bdf_addr = false,
.fixed_mem_region = false,
+ .static_window_map = true,
.hybrid_bus_type = true,
+ .dp_window_idx = 1,
+ .ce_window_idx = 2,
},
};
diff --git a/drivers/net/wireless/ath/ath11k/hw.h b/drivers/net/wireless/ath/ath11k/hw.h
index 03eb5dfd4a5e..b63538084215 100644
--- a/drivers/net/wireless/ath/ath11k/hw.h
+++ b/drivers/net/wireless/ath/ath11k/hw.h
@@ -201,6 +201,8 @@ struct ath11k_hw_params {
bool fixed_mem_region;
bool static_window_map;
bool hybrid_bus_type;
+ u8 dp_window_idx;
+ u8 ce_window_idx;
};
struct ath11k_hw_ops {
diff --git a/drivers/net/wireless/ath/ath11k/pcic.c b/drivers/net/wireless/ath/ath11k/pcic.c
index 7a920d65023f..46cf96d3e1d2 100644
--- a/drivers/net/wireless/ath/ath11k/pcic.c
+++ b/drivers/net/wireless/ath/ath11k/pcic.c
@@ -134,16 +134,13 @@ EXPORT_SYMBOL(ath11k_pcic_init_msi_config);
static inline u32 ath11k_pcic_get_window_start(struct ath11k_base *ab,
u32 offset)
{
- u32 window_start;
+ u32 window_start = 0;
- /* If offset lies within DP register range, use 3rd window */
if ((offset ^ HAL_SEQ_WCSS_UMAC_OFFSET) < ATH11K_PCI_WINDOW_RANGE_MASK)
- window_start = 3 * ATH11K_PCI_WINDOW_START;
- /* If offset lies within CE register range, use 2nd window */
- else if ((offset ^ HAL_CE_WFSS_CE_REG_BASE) < ATH11K_PCI_WINDOW_RANGE_MASK)
- window_start = 2 * ATH11K_PCI_WINDOW_START;
- else
- window_start = ATH11K_PCI_WINDOW_START;
+ window_start = ab->hw_params.dp_window_idx * ATH11K_PCI_WINDOW_START;
+ else if ((offset ^ HAL_SEQ_WCSS_UMAC_CE0_SRC_REG(ab)) <
+ ATH11K_PCI_WINDOW_RANGE_MASK)
+ window_start = ab->hw_params.ce_window_idx * ATH11K_PCI_WINDOW_START;
return window_start;
}
@@ -162,19 +159,12 @@ void ath11k_pcic_write32(struct ath11k_base *ab, u32 offset, u32 value)
if (offset < ATH11K_PCI_WINDOW_START) {
iowrite32(value, ab->mem + offset);
- } else {
- if (ab->hw_params.static_window_map)
- window_start = ath11k_pcic_get_window_start(ab, offset);
- else
- window_start = ATH11K_PCI_WINDOW_START;
-
- if (window_start == ATH11K_PCI_WINDOW_START &&
- ab->pci.ops->window_write32) {
- ab->pci.ops->window_write32(ab, offset, value);
- } else {
- iowrite32(value, ab->mem + window_start +
- (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
- }
+ } else if (ab->hw_params.static_window_map) {
+ window_start = ath11k_pcic_get_window_start(ab, offset);
+ iowrite32(value, ab->mem + window_start +
+ (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
+ } else if (ab->pci.ops->window_write32) {
+ ab->pci.ops->window_write32(ab, offset, value);
}
if (test_bit(ATH11K_FLAG_DEVICE_INIT_DONE, &ab->dev_flags) &&
@@ -185,7 +175,8 @@ void ath11k_pcic_write32(struct ath11k_base *ab, u32 offset, u32 value)
u32 ath11k_pcic_read32(struct ath11k_base *ab, u32 offset)
{
- u32 val, window_start;
+ u32 val = 0;
+ u32 window_start;
int ret = 0;
/* for offset beyond BAR + 4K - 32, may
@@ -197,19 +188,12 @@ u32 ath11k_pcic_read32(struct ath11k_base *ab, u32 offset)
if (offset < ATH11K_PCI_WINDOW_START) {
val = ioread32(ab->mem + offset);
- } else {
- if (ab->hw_params.static_window_map)
- window_start = ath11k_pcic_get_window_start(ab, offset);
- else
- window_start = ATH11K_PCI_WINDOW_START;
-
- if (window_start == ATH11K_PCI_WINDOW_START &&
- ab->pci.ops->window_read32) {
- val = ab->pci.ops->window_read32(ab, offset);
- } else {
- val = ioread32(ab->mem + window_start +
- (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
- }
+ } else if (ab->hw_params.static_window_map) {
+ window_start = ath11k_pcic_get_window_start(ab, offset);
+ val = ioread32(ab->mem + window_start +
+ (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
+ } else if (ab->pci.ops->window_read32) {
+ val = ab->pci.ops->window_read32(ab, offset);
}
if (test_bit(ATH11K_FLAG_DEVICE_INIT_DONE, &ab->dev_flags) &&
--
2.35.1
Manikanta Pubbisetty <[email protected]> writes:
> WCN6750 uses static window mapping to access the HW registers.
> Unlike QCN9074 which uses 3rd window for UMAC and 2nd window
> for CE register access, WCN6750 uses 1st window for UMAC
> and 2nd window for CE registers.
>
> Also, refactor the code so that WCN6750 can use the existing
> ath11k_pci_read32/write32() APIs for accessing the registers.
>
> Tested-on: WCN6750 hw1.0 AHB WLAN.MSL.1.0.1-00887-QCAMSLSWPLZ-1
> Tested-on: WCN6855 hw2.0 PCI WLAN.HSP.1.1-01720.1-QCAHSPSWPL_V1_V2_SILICONZ_LITE-1
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-QCAHKSWPL_SILICONZ-1
> Tested-on: IPQ8074 hw2.0 AHB WLAN.HK.2.4.0.1-00192-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Manikanta Pubbisetty <[email protected]>
[...]
> {
> .name = "wcn6750 hw1.0",
> @@ -527,7 +539,10 @@ static const struct ath11k_hw_params ath11k_hw_params[] = {
> .m3_fw_support = false,
> .fixed_bdf_addr = false,
> .fixed_mem_region = false,
> + .static_window_map = true,
> .hybrid_bus_type = true,
> + .dp_window_idx = 1,
> + .ce_window_idx = 2,
> },
In the pending branch I moved setting of static_window_map to patch 3.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 5/26/2022 12:16 AM, Maxime Bizon wrote:
>
> On Fri, 2022-04-29 at 22:34 +0530, Manikanta Pubbisetty wrote:
>
> Hello Manikanta,
>
>> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-
>> QCAHKSWPL_SILICONZ-1
>
> Nope your patch breaks QCN9074:
>
> [ 13.660963] ath11k_pci 0000:03:00.0: failed to set pcie link register 0x01e0e0a8: 0xffffffff != 0x00000010
> [ 13.675994] ath11k_pci 0000:03:00.0: failed to set sysclk: -110
>
> device still seem to work though
Thanks for letting me know about this, IIRC I don't remember
encountering this problem in my testing. Just for my understanding, have
you reverted this change and confirmed that these errors go away ?
>
>> @@ -134,16 +134,13 @@ EXPORT_SYMBOL(ath11k_pcic_init_msi_config);
>> static inline u32 ath11k_pcic_get_window_start(struct ath11k_base *ab,
>> u32 offset)
>> {
>> - u32 window_start;
>> + u32 window_start = 0;
>>
>> - /* If offset lies within DP register range, use 3rd window */
>> if ((offset ^ HAL_SEQ_WCSS_UMAC_OFFSET) < ATH11K_PCI_WINDOW_RANGE_MASK)
>> - window_start = 3 * ATH11K_PCI_WINDOW_START;
>> - /* If offset lies within CE register range, use 2nd window */
>> - else if ((offset ^ HAL_CE_WFSS_CE_REG_BASE) < ATH11K_PCI_WINDOW_RANGE_MASK)
>> - window_start = 2 * ATH11K_PCI_WINDOW_START;
>> - else
>> - window_start = ATH11K_PCI_WINDOW_START;
>> + window_start = ab->hw_params.dp_window_idx * ATH11K_PCI_WINDOW_START;
>> + else if ((offset ^ HAL_SEQ_WCSS_UMAC_CE0_SRC_REG(ab)) <
>> + ATH11K_PCI_WINDOW_RANGE_MASK)
>> + window_start = ab->hw_params.ce_window_idx * ATH11K_PCI_WINDOW_START;
>>
>> return window_start;
>> }
>
>
> for some offsets, previous code could return ATH11K_PCI_WINDOW_START,
> whereas new code now returns 0
>
>
>> @@ -162,19 +159,12 @@ void ath11k_pcic_write32(struct ath11k_base *ab, u32 offset, u32 value)
>>
>> if (offset < ATH11K_PCI_WINDOW_START) {
>> iowrite32(value, ab->mem + offset);
>> - } else {
>> - if (ab->hw_params.static_window_map)
>> - window_start = ath11k_pcic_get_window_start(ab, offset);
>> - else
>> - window_start = ATH11K_PCI_WINDOW_START;
>> -
>> - if (window_start == ATH11K_PCI_WINDOW_START &&
>> - ab->pci.ops->window_write32) {
>> - ab->pci.ops->window_write32(ab, offset, value);
>> - } else {
>> - iowrite32(value, ab->mem + window_start +
>> - (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
>> - }
>> + } else if (ab->hw_params.static_window_map) {
>> + window_start = ath11k_pcic_get_window_start(ab, offset);
>> + iowrite32(value, ab->mem + window_start +
>> + (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
>> + } else if (ab->pci.ops->window_write32) {
>> + ab->pci.ops->window_write32(ab, offset, value);
>> }
>>
>
> with previous code on QCN9074, when ath11k_pcic_get_window_start()
> returned ATH11K_PCI_WINDOW_START, then it would call window_write32()
>
> with new code on QCN9074, static_window_map is true, so window_write32
> will never be called.
>
>> u32 ath11k_pcic_read32(struct ath11k_base *ab, u32 offset)
>
> ditto here
>
If you could please confirm that reverting this change helps, I can send
a patch to fix this.
Thanks,
Manikanta
On Fri, 2022-04-29 at 22:34 +0530, Manikanta Pubbisetty wrote:
Hello Manikanta,
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.5.0.1-01100-
> QCAHKSWPL_SILICONZ-1
Nope your patch breaks QCN9074:
[ 13.660963] ath11k_pci 0000:03:00.0: failed to set pcie link register 0x01e0e0a8: 0xffffffff != 0x00000010
[ 13.675994] ath11k_pci 0000:03:00.0: failed to set sysclk: -110
device still seem to work though
> @@ -134,16 +134,13 @@ EXPORT_SYMBOL(ath11k_pcic_init_msi_config);
> static inline u32 ath11k_pcic_get_window_start(struct ath11k_base *ab,
> u32 offset)
> {
> - u32 window_start;
> + u32 window_start = 0;
>
> - /* If offset lies within DP register range, use 3rd window */
> if ((offset ^ HAL_SEQ_WCSS_UMAC_OFFSET) < ATH11K_PCI_WINDOW_RANGE_MASK)
> - window_start = 3 * ATH11K_PCI_WINDOW_START;
> - /* If offset lies within CE register range, use 2nd window */
> - else if ((offset ^ HAL_CE_WFSS_CE_REG_BASE) < ATH11K_PCI_WINDOW_RANGE_MASK)
> - window_start = 2 * ATH11K_PCI_WINDOW_START;
> - else
> - window_start = ATH11K_PCI_WINDOW_START;
> + window_start = ab->hw_params.dp_window_idx * ATH11K_PCI_WINDOW_START;
> + else if ((offset ^ HAL_SEQ_WCSS_UMAC_CE0_SRC_REG(ab)) <
> + ATH11K_PCI_WINDOW_RANGE_MASK)
> + window_start = ab->hw_params.ce_window_idx * ATH11K_PCI_WINDOW_START;
>
> return window_start;
> }
for some offsets, previous code could return ATH11K_PCI_WINDOW_START,
whereas new code now returns 0
> @@ -162,19 +159,12 @@ void ath11k_pcic_write32(struct ath11k_base *ab, u32 offset, u32 value)
>
> if (offset < ATH11K_PCI_WINDOW_START) {
> iowrite32(value, ab->mem + offset);
> - } else {
> - if (ab->hw_params.static_window_map)
> - window_start = ath11k_pcic_get_window_start(ab, offset);
> - else
> - window_start = ATH11K_PCI_WINDOW_START;
> -
> - if (window_start == ATH11K_PCI_WINDOW_START &&
> - ab->pci.ops->window_write32) {
> - ab->pci.ops->window_write32(ab, offset, value);
> - } else {
> - iowrite32(value, ab->mem + window_start +
> - (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
> - }
> + } else if (ab->hw_params.static_window_map) {
> + window_start = ath11k_pcic_get_window_start(ab, offset);
> + iowrite32(value, ab->mem + window_start +
> + (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
> + } else if (ab->pci.ops->window_write32) {
> + ab->pci.ops->window_write32(ab, offset, value);
> }
>
with previous code on QCN9074, when ath11k_pcic_get_window_start()
returned ATH11K_PCI_WINDOW_START, then it would call window_write32()
with new code on QCN9074, static_window_map is true, so window_write32
will never be called.
> u32 ath11k_pcic_read32(struct ath11k_base *ab, u32 offset)
ditto here
--
Maxime
On Mon, 2022-05-30 at 19:17 +0300, Kalle Valo wrote:
> Looks good to me. Can you submit that as a proper patch?
I'd prefer that patch to come from someone with actual knowledge of the
hardware, I basically did a blind revert
--
Maxime
Maxime Bizon <[email protected]> writes:
> On Thu, 2022-05-26 at 09:12 +0530, Manikanta Pubbisetty wrote:
>
>>
>> Thanks for letting me know about this, IIRC I don't remember
>> encountering this problem in my testing. Just for my understanding,
>> have you reverted this change and confirmed that these errors go away
>> ?
>
> I first confirmed the register location was indeed incorrect, then I
> fixed it like this:
Looks good to me. Can you submit that as a proper patch?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Thu, 2022-05-26 at 09:12 +0530, Manikanta Pubbisetty wrote:
>
> Thanks for letting me know about this, IIRC I don't remember
> encountering this problem in my testing. Just for my understanding,
> have you reverted this change and confirmed that these errors go away
> ?
I first confirmed the register location was indeed incorrect, then I
fixed it like this:
--- a/drivers/net/wireless/ath/ath11k/pcic.c
+++ b/drivers/net/wireless/ath/ath11k/pcic.c
@@ -143,7 +143,7 @@ EXPORT_SYMBOL(ath11k_pcic_init_msi_config);
static inline u32 ath11k_pcic_get_window_start(struct ath11k_base *ab,
u32 offset)
{
- u32 window_start = 0;
+ u32 window_start = ATH11K_PCI_WINDOW_START;
if ((offset ^ HAL_SEQ_WCSS_UMAC_OFFSET) < ATH11K_PCI_WINDOW_RANGE_MASK)
window_start = ab->hw_params.dp_window_idx * ATH11K_PCI_WINDOW_START;
@@ -170,8 +170,12 @@ void ath11k_pcic_write32(struct ath11k_base *ab, u32 offset, u32 value)
iowrite32(value, ab->mem + offset);
} else if (ab->hw_params.static_window_map) {
window_start = ath11k_pcic_get_window_start(ab, offset);
- iowrite32(value, ab->mem + window_start +
- (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
+ if (window_start == ATH11K_PCI_WINDOW_START &&
+ ab->pci.ops->window_write32)
+ ab->pci.ops->window_write32(ab, offset, value);
+ else
+ iowrite32(value, ab->mem + window_start +
+ (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
} else if (ab->pci.ops->window_write32) {
ab->pci.ops->window_write32(ab, offset, value);
}
@@ -200,8 +204,12 @@ u32 ath11k_pcic_read32(struct ath11k_base *ab, u32 offset)
val = ioread32(ab->mem + offset);
} else if (ab->hw_params.static_window_map) {
window_start = ath11k_pcic_get_window_start(ab, offset);
- val = ioread32(ab->mem + window_start +
- (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
+ if (window_start == ATH11K_PCI_WINDOW_START &&
+ ab->pci.ops->window_read32)
+ val = ab->pci.ops->window_read32(ab, offset);
+ else
+ val = ioread32(ab->mem + window_start +
+ (offset & ATH11K_PCI_WINDOW_RANGE_MASK));
} else if (ab->pci.ops->window_read32) {
val = ab->pci.ops->window_read32(ab, offset);
}
--
Maxime