| Message ID | 20241015182637.955753-16-quic_rajkbhag@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | wifi: ath12k: add Ath12k AHB driver support for IPQ5332 | expand |
On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote: > The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE > memory during QMI memory requests. This memory is part of the > HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the > hardware parameter and provide this memory address to the firmware > during QMI memory requests. Sounds like something to put in the device tree, no? Konrad
On 11/4/2024 7:46 PM, Konrad Dybcio wrote: > On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote: >> The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE >> memory during QMI memory requests. This memory is part of the >> HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the >> hardware parameter and provide this memory address to the firmware >> during QMI memory requests. > > Sounds like something to put in the device tree, no? > This BDF memory address is the RAM offset. We did add this in device tree in version 1. This is removed from device tree in v2 based on the review comment that DT should not store RAM offset. refer below link: Link: https://lore.kernel.org/all/f8cd9c3d-47e1-4709-9334-78e4790acef0@kernel.org/
On 3.12.2024 10:18 AM, Raj Kumar Bhagat wrote: > On 11/4/2024 7:46 PM, Konrad Dybcio wrote: >> On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote: >>> The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE >>> memory during QMI memory requests. This memory is part of the >>> HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the >>> hardware parameter and provide this memory address to the firmware >>> during QMI memory requests. >> >> Sounds like something to put in the device tree, no? >> > > This BDF memory address is the RAM offset. We did add this in device tree in > version 1. This is removed from device tree in v2 based on the review comment that > DT should not store RAM offset. > > refer below link: > Link: https://lore.kernel.org/all/f8cd9c3d-47e1-4709-9334-78e4790acef0@kernel.org/ Right, I think this could be something under /reserved-memory instead Konrad
On 12/5/2024 11:12 PM, Konrad Dybcio wrote: > On 3.12.2024 10:18 AM, Raj Kumar Bhagat wrote: >> On 11/4/2024 7:46 PM, Konrad Dybcio wrote: >>> On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote: >>>> The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE >>>> memory during QMI memory requests. This memory is part of the >>>> HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the >>>> hardware parameter and provide this memory address to the firmware >>>> during QMI memory requests. >>> >>> Sounds like something to put in the device tree, no? >>> >> >> This BDF memory address is the RAM offset. We did add this in device tree in >> version 1. This is removed from device tree in v2 based on the review comment that >> DT should not store RAM offset. >> >> refer below link: >> Link: https://lore.kernel.org/all/f8cd9c3d-47e1-4709-9334-78e4790acef0@kernel.org/ > > Right, I think this could be something under /reserved-memory instead > Thanks for the suggestion. However, the BDF_MEM_REGION_TYPE is already within the memory reserved for HOST_DDR_REGION_TYPE through /reserved-memory. Therefore, reserving the memory for BDF_MEM_REGION_TYPE again in the Device Tree (DT) will cause a warning for 'overlapping memory reservation'.
On 6.12.2024 5:34 AM, Raj Kumar Bhagat wrote: > On 12/5/2024 11:12 PM, Konrad Dybcio wrote: >> On 3.12.2024 10:18 AM, Raj Kumar Bhagat wrote: >>> On 11/4/2024 7:46 PM, Konrad Dybcio wrote: >>>> On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote: >>>>> The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE >>>>> memory during QMI memory requests. This memory is part of the >>>>> HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the >>>>> hardware parameter and provide this memory address to the firmware >>>>> during QMI memory requests. >>>> >>>> Sounds like something to put in the device tree, no? >>>> >>> >>> This BDF memory address is the RAM offset. We did add this in device tree in >>> version 1. This is removed from device tree in v2 based on the review comment that >>> DT should not store RAM offset. >>> >>> refer below link: >>> Link: https://lore.kernel.org/all/f8cd9c3d-47e1-4709-9334-78e4790acef0@kernel.org/ >> >> Right, I think this could be something under /reserved-memory instead >> > > Thanks for the suggestion. However, the BDF_MEM_REGION_TYPE is already within the > memory reserved for HOST_DDR_REGION_TYPE through /reserved-memory. Therefore, reserving > the memory for BDF_MEM_REGION_TYPE again in the Device Tree (DT) will cause a warning > for 'overlapping memory reservation'. Then you can grab a handle to it with of_reserved_mem_lookup() and of_reserved_mem_device_init_by_idx() Konrad
On 12/6/2024 4:19 PM, Konrad Dybcio wrote: > On 6.12.2024 5:34 AM, Raj Kumar Bhagat wrote: >> On 12/5/2024 11:12 PM, Konrad Dybcio wrote: >>> On 3.12.2024 10:18 AM, Raj Kumar Bhagat wrote: >>>> On 11/4/2024 7:46 PM, Konrad Dybcio wrote: >>>>> On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote: >>>>>> The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE >>>>>> memory during QMI memory requests. This memory is part of the >>>>>> HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the >>>>>> hardware parameter and provide this memory address to the firmware >>>>>> during QMI memory requests. >>>>> >>>>> Sounds like something to put in the device tree, no? >>>>> >>>> >>>> This BDF memory address is the RAM offset. We did add this in device tree in >>>> version 1. This is removed from device tree in v2 based on the review comment that >>>> DT should not store RAM offset. >>>> >>>> refer below link: >>>> Link: https://lore.kernel.org/all/f8cd9c3d-47e1-4709-9334-78e4790acef0@kernel.org/ >>> >>> Right, I think this could be something under /reserved-memory instead >>> >> >> Thanks for the suggestion. However, the BDF_MEM_REGION_TYPE is already within the >> memory reserved for HOST_DDR_REGION_TYPE through /reserved-memory. Therefore, reserving >> the memory for BDF_MEM_REGION_TYPE again in the Device Tree (DT) will cause a warning >> for 'overlapping memory reservation'. > > Then you can grab a handle to it with of_reserved_mem_lookup() > and of_reserved_mem_device_init_by_idx() > The memory HOST_DDR_REGION_TYPE is a bigger memory around 43MB, while the memory BDF_MEM_REGION_TYPE is smaller around 256KB within HOST_DDR_REGION_TYPE, Using the above mentioned API we still have to store the offset in ath12k to point at memory BDF_MEM_REGION_TYPE from the start of HOST_DDR_REGION_TYPE.
On 9.12.2024 5:23 AM, Raj Kumar Bhagat wrote: > On 12/6/2024 4:19 PM, Konrad Dybcio wrote: >> On 6.12.2024 5:34 AM, Raj Kumar Bhagat wrote: >>> On 12/5/2024 11:12 PM, Konrad Dybcio wrote: >>>> On 3.12.2024 10:18 AM, Raj Kumar Bhagat wrote: >>>>> On 11/4/2024 7:46 PM, Konrad Dybcio wrote: >>>>>> On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote: >>>>>>> The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE >>>>>>> memory during QMI memory requests. This memory is part of the >>>>>>> HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the >>>>>>> hardware parameter and provide this memory address to the firmware >>>>>>> during QMI memory requests. >>>>>> >>>>>> Sounds like something to put in the device tree, no? >>>>>> >>>>> >>>>> This BDF memory address is the RAM offset. We did add this in device tree in >>>>> version 1. This is removed from device tree in v2 based on the review comment that >>>>> DT should not store RAM offset. >>>>> >>>>> refer below link: >>>>> Link: https://lore.kernel.org/all/f8cd9c3d-47e1-4709-9334-78e4790acef0@kernel.org/ >>>> >>>> Right, I think this could be something under /reserved-memory instead >>>> >>> >>> Thanks for the suggestion. However, the BDF_MEM_REGION_TYPE is already within the >>> memory reserved for HOST_DDR_REGION_TYPE through /reserved-memory. Therefore, reserving >>> the memory for BDF_MEM_REGION_TYPE again in the Device Tree (DT) will cause a warning >>> for 'overlapping memory reservation'. >> >> Then you can grab a handle to it with of_reserved_mem_lookup() >> and of_reserved_mem_device_init_by_idx() >> > > The memory HOST_DDR_REGION_TYPE is a bigger memory around 43MB, while the memory > BDF_MEM_REGION_TYPE is smaller around 256KB within HOST_DDR_REGION_TYPE, Using the > above mentioned API we still have to store the offset in ath12k to point at memory > BDF_MEM_REGION_TYPE from the start of HOST_DDR_REGION_TYPE. That's still way better than hardcoding platform specifics in the common driver Konrad
On 12/13/2024 5:48 AM, Konrad Dybcio wrote: > On 9.12.2024 5:23 AM, Raj Kumar Bhagat wrote: >> On 12/6/2024 4:19 PM, Konrad Dybcio wrote: >>> On 6.12.2024 5:34 AM, Raj Kumar Bhagat wrote: >>>> On 12/5/2024 11:12 PM, Konrad Dybcio wrote: >>>>> On 3.12.2024 10:18 AM, Raj Kumar Bhagat wrote: >>>>>> On 11/4/2024 7:46 PM, Konrad Dybcio wrote: >>>>>>> On 15.10.2024 8:26 PM, Raj Kumar Bhagat wrote: >>>>>>>> The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE >>>>>>>> memory during QMI memory requests. This memory is part of the >>>>>>>> HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the >>>>>>>> hardware parameter and provide this memory address to the firmware >>>>>>>> during QMI memory requests. >>>>>>> >>>>>>> Sounds like something to put in the device tree, no? >>>>>>> >>>>>> >>>>>> This BDF memory address is the RAM offset. We did add this in device tree in >>>>>> version 1. This is removed from device tree in v2 based on the review comment that >>>>>> DT should not store RAM offset. >>>>>> >>>>>> refer below link: >>>>>> Link: https://lore.kernel.org/all/f8cd9c3d-47e1-4709-9334-78e4790acef0@kernel.org/ >>>>> >>>>> Right, I think this could be something under /reserved-memory instead >>>>> >>>> >>>> Thanks for the suggestion. However, the BDF_MEM_REGION_TYPE is already within the >>>> memory reserved for HOST_DDR_REGION_TYPE through /reserved-memory. Therefore, reserving >>>> the memory for BDF_MEM_REGION_TYPE again in the Device Tree (DT) will cause a warning >>>> for 'overlapping memory reservation'. >>> >>> Then you can grab a handle to it with of_reserved_mem_lookup() >>> and of_reserved_mem_device_init_by_idx() >>> >> >> The memory HOST_DDR_REGION_TYPE is a bigger memory around 43MB, while the memory >> BDF_MEM_REGION_TYPE is smaller around 256KB within HOST_DDR_REGION_TYPE, Using the >> above mentioned API we still have to store the offset in ath12k to point at memory >> BDF_MEM_REGION_TYPE from the start of HOST_DDR_REGION_TYPE. > > That's still way better than hardcoding platform specifics in the common > driver > Sure, I agree. I'll update in latest version to store the offset for BDF_MEM_REGION_TYPE. Thanks!
diff --git a/drivers/net/wireless/ath/ath12k/hw.c b/drivers/net/wireless/ath/ath12k/hw.c index 4117a4b718e3..8fc191b0b467 100644 --- a/drivers/net/wireless/ath/ath12k/hw.c +++ b/drivers/net/wireless/ath/ath12k/hw.c @@ -1320,6 +1320,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = { .cmem_remap = NULL, .ce_ie_addr = NULL, .ce_remap = NULL, + .bdf_addr = 0, }, { .name = "wcn7850 hw2.0", @@ -1405,6 +1406,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = { .cmem_remap = NULL, .ce_ie_addr = NULL, .ce_remap = NULL, + .bdf_addr = 0, }, { .name = "qcn9274 hw2.0", @@ -1486,6 +1488,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = { .cmem_remap = NULL, .ce_ie_addr = NULL, .ce_remap = NULL, + .bdf_addr = 0, }, { .name = "ipq5332 hw1.0", @@ -1562,6 +1565,7 @@ static const struct ath12k_hw_params ath12k_hw_params[] = { .cmem_remap = &ath12k_cmem_remap_ipq5332, .ce_ie_addr = &ath12k_ce_ie_addr_ipq5332, .ce_remap = &ath12k_ce_remap_ipq5332, + .bdf_addr = 0x4B500000, }, }; diff --git a/drivers/net/wireless/ath/ath12k/hw.h b/drivers/net/wireless/ath/ath12k/hw.h index 580c7be109e0..038fe1b30d11 100644 --- a/drivers/net/wireless/ath/ath12k/hw.h +++ b/drivers/net/wireless/ath/ath12k/hw.h @@ -225,6 +225,7 @@ struct ath12k_hw_params { const struct cmem_remap *cmem_remap; const struct ce_ie_addr *ce_ie_addr; const struct ce_remap *ce_remap; + u32 bdf_addr; }; struct ath12k_hw_ops { diff --git a/drivers/net/wireless/ath/ath12k/qmi.c b/drivers/net/wireless/ath/ath12k/qmi.c index ec8859031824..b5cad6656722 100644 --- a/drivers/net/wireless/ath/ath12k/qmi.c +++ b/drivers/net/wireless/ath/ath12k/qmi.c @@ -2491,6 +2491,14 @@ static int ath12k_qmi_assign_target_mem_chunk(struct ath12k_base *ab) ab->qmi.target_mem[idx].type = ab->qmi.target_mem[i].type; idx++; break; + case BDF_MEM_REGION_TYPE: + ab->qmi.target_mem[idx].paddr = ab->hw_params->bdf_addr; + ab->qmi.target_mem[idx].v.ioaddr = NULL; + ab->qmi.target_mem[idx].size = ab->qmi.target_mem[i].size; + ab->qmi.target_mem[idx].type = ab->qmi.target_mem[i].type; + idx++; + break; + case CALDB_MEM_REGION_TYPE: /* Cold boot calibration is not enabled in Ath12k. Hence, * assign paddr = 0.
The Ath2k AHB device (IPQ5332) firmware requests BDF_MEM_REGION_TYPE memory during QMI memory requests. This memory is part of the HOST_DDR_REGION_TYPE. Therefore, add the BDF memory address to the hardware parameter and provide this memory address to the firmware during QMI memory requests. Tested-on: IPQ5332 hw1.0 AHB WLAN.WBE.1.3.1-00130-QCAHKSWPL_SILICONZ-1 Signed-off-by: Raj Kumar Bhagat <quic_rajkbhag@quicinc.com> --- drivers/net/wireless/ath/ath12k/hw.c | 4 ++++ drivers/net/wireless/ath/ath12k/hw.h | 1 + drivers/net/wireless/ath/ath12k/qmi.c | 8 ++++++++ 3 files changed, 13 insertions(+)