Host DDR memory (contiguous 45 MB in mode-0 or 15 MB in mode-2)
is reserved through DT entries for firmware usage. Send the base
address and size from DT entries.
If DT entry is available, PCI devices work with
fixed_mem_region else host allocates multiple segments.
IPQ8074 on HK10 board supports multiple PCI devices.
IPQ8074 + QCN9074 is tested with this patch.
Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.4.0.1-01838-QCAHKSWPL_SILICONZ-1
Signed-off-by: Anilkumar Kolli <[email protected]>
---
V2:
- Use of_ API to read from dt node (Rob)
drivers/net/wireless/ath/ath11k/mhi.c | 42 ++++++++++++++++++--
drivers/net/wireless/ath/ath11k/pci.c | 10 ++++-
drivers/net/wireless/ath/ath11k/qmi.c | 75 +++++++++++++++++++++++++++++++----
drivers/net/wireless/ath/ath11k/qmi.h | 1 +
4 files changed, 116 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
index 26c7ae242db6..0ea6ab7edeb9 100644
--- a/drivers/net/wireless/ath/ath11k/mhi.c
+++ b/drivers/net/wireless/ath/ath11k/mhi.c
@@ -3,6 +3,7 @@
#include <linux/msi.h>
#include <linux/pci.h>
+#include <linux/of.h>
#include "core.h"
#include "debug.h"
@@ -316,7 +317,11 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
struct ath11k_base *ab = ab_pci->ab;
struct mhi_controller *mhi_ctrl;
struct mhi_controller_config *ath11k_mhi_config;
- int ret;
+ struct device_node *np;
+ int ret, len, sw, aw;
+ u32 *reg, *reg_end;
+ unsigned long start, size;
+ bool no_dt_entry = 0;
mhi_ctrl = mhi_alloc_controller();
if (!mhi_ctrl)
@@ -339,8 +344,39 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
return ret;
}
- mhi_ctrl->iova_start = 0;
- mhi_ctrl->iova_stop = 0xffffffff;
+ np = of_find_node_by_type(NULL, "memory");
+ if (!np) {
+ no_dt_entry = 1;
+ goto no_dt_entry;
+ }
+
+ aw = of_n_addr_cells(np);
+ sw = of_n_size_cells(np);
+
+ reg = (unsigned int *)of_get_property(np, "reg", &len);
+ if (!reg) {
+ no_dt_entry = 1;
+ goto no_dt_entry;
+ }
+
+ reg_end = reg + len / (aw * sw);
+
+ do {
+ start = of_read_number(reg, aw);
+ reg += aw;
+ size = of_read_number(reg, sw);
+ reg += sw;
+ } while (reg < reg_end);
+
+no_dt_entry:
+ if (no_dt_entry) {
+ mhi_ctrl->iova_start = 0;
+ mhi_ctrl->iova_stop = 0xFFFFFFFF;
+ } else {
+ mhi_ctrl->iova_start = (dma_addr_t)(start + 0x1000000);
+ mhi_ctrl->iova_stop = (dma_addr_t)(start + size);
+ }
+
mhi_ctrl->sbl_size = SZ_512K;
mhi_ctrl->seg_len = SZ_512K;
mhi_ctrl->fbc_download = true;
diff --git a/drivers/net/wireless/ath/ath11k/pci.c b/drivers/net/wireless/ath/ath11k/pci.c
index 3d353e7c9d5c..21d0a511d9c7 100644
--- a/drivers/net/wireless/ath/ath11k/pci.c
+++ b/drivers/net/wireless/ath/ath11k/pci.c
@@ -6,6 +6,7 @@
#include <linux/module.h>
#include <linux/msi.h>
#include <linux/pci.h>
+#include <linux/of.h>
#include "pci.h"
#include "core.h"
@@ -1225,7 +1226,7 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
{
struct ath11k_base *ab;
struct ath11k_pci *ab_pci;
- u32 soc_hw_version_major, soc_hw_version_minor;
+ u32 soc_hw_version_major, soc_hw_version_minor, addr;
int ret;
ab = ath11k_core_alloc(&pdev->dev, sizeof(*ab_pci), ATH11K_BUS_PCI,
@@ -1245,6 +1246,13 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
pci_set_drvdata(pdev, ab);
spin_lock_init(&ab_pci->window_lock);
+ /* Set fixed_mem_region to true for platforms support reserved memory
+ * from DT. If memory is reserved from DT for FW, ath11k driver need not
+ * allocate memory.
+ */
+ if (!of_property_read_u32(ab->dev->of_node, "memory-region", &addr))
+ ab->bus_params.fixed_mem_region = true;
+
ret = ath11k_pci_claim(ab_pci, pdev);
if (ret) {
ath11k_err(ab, "failed to claim device: %d\n", ret);
diff --git a/drivers/net/wireless/ath/ath11k/qmi.c b/drivers/net/wireless/ath/ath11k/qmi.c
index fa73118de6db..da58ce36b477 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -1815,10 +1815,11 @@ static void ath11k_qmi_free_target_mem_chunk(struct ath11k_base *ab)
{
int i;
- if (ab->bus_params.fixed_mem_region)
- return;
-
for (i = 0; i < ab->qmi.mem_seg_count; i++) {
+ if (ab->bus_params.fixed_mem_region &&
+ ab->qmi.target_mem[i].iaddr)
+ iounmap(ab->qmi.target_mem[i].iaddr);
+
if (!ab->qmi.target_mem[i].vaddr)
continue;
@@ -1866,10 +1867,62 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab)
static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
{
- int i, idx;
+ struct device *dev = ab->dev;
+ struct device_node *hremote_node = NULL;
+ phandle hremote_phandle;
+ int i, idx, len, sw, aw, host_ddr_sz;
+ u32 *reg, *reg_end;
+ u64 start, size;
for (i = 0, idx = 0; i < ab->qmi.mem_seg_count; i++) {
switch (ab->qmi.target_mem[i].type) {
+ case HOST_DDR_REGION_TYPE:
+ if (of_property_read_u32(dev->of_node, "memory-region",
+ &hremote_phandle)) {
+ ath11k_dbg(ab, ATH11K_DBG_QMI,
+ "qmi fail to get hremote phandle\n");
+ return 0;
+ }
+
+ hremote_node = of_find_node_by_phandle(hremote_phandle);
+ if (!hremote_node) {
+ ath11k_dbg(ab, ATH11K_DBG_QMI,
+ "qmi fail to get hremote_node\n");
+ return 0;
+ }
+
+ aw = of_n_addr_cells(hremote_node);
+ sw = of_n_size_cells(hremote_node);
+
+ reg = (unsigned int *)of_get_property(hremote_node, "reg", &len);
+ if (!reg) {
+ ath11k_dbg(ab, ATH11K_DBG_QMI,
+ "qmi fail to get reg from hremote\n");
+ return 0;
+ }
+
+ reg_end = reg + len / (aw * sw);
+
+ do {
+ start = of_read_number(reg, aw);
+ reg += aw;
+ size = of_read_number(reg, sw);
+ reg += sw;
+ } while (reg < reg_end);
+
+ ath11k_dbg(ab, ATH11K_DBG_QMI,
+ "qmi ddr start addr is 0x%llx len is %lld\n",
+ start, size);
+
+ ab->qmi.target_mem[idx].paddr = start;
+ ab->qmi.target_mem[idx].iaddr =
+ ioremap(ab->qmi.target_mem[idx].paddr,
+ ab->qmi.target_mem[i].size);
+ ab->qmi.target_mem[idx].size = ab->qmi.target_mem[i].size;
+ host_ddr_sz = ab->qmi.target_mem[i].size;
+ 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].vaddr = NULL;
@@ -1884,10 +1937,16 @@ static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
}
if (ath11k_cold_boot_cal && ab->hw_params.cold_boot_calib) {
- ab->qmi.target_mem[idx].paddr =
- ATH11K_QMI_CALDB_ADDRESS;
- ab->qmi.target_mem[idx].vaddr =
- (void *)ATH11K_QMI_CALDB_ADDRESS;
+ if (hremote_node) {
+ ab->qmi.target_mem[idx].paddr =
+ start + host_ddr_sz;
+ ab->qmi.target_mem[idx].iaddr =
+ ioremap(ab->qmi.target_mem[idx].paddr,
+ ab->qmi.target_mem[i].size);
+ } else {
+ ab->qmi.target_mem[idx].paddr =
+ ATH11K_QMI_CALDB_ADDRESS;
+ }
} else {
ab->qmi.target_mem[idx].paddr = 0;
ab->qmi.target_mem[idx].vaddr = NULL;
diff --git a/drivers/net/wireless/ath/ath11k/qmi.h b/drivers/net/wireless/ath/ath11k/qmi.h
index 3bb0f9ef7996..f8d45b7dc821 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.h
+++ b/drivers/net/wireless/ath/ath11k/qmi.h
@@ -95,6 +95,7 @@ struct target_mem_chunk {
u32 type;
dma_addr_t paddr;
u32 *vaddr;
+ void __iomem *iaddr;
};
struct target_info {
--
2.7.4
Anilkumar Kolli <[email protected]> writes:
> Host DDR memory (contiguous 45 MB in mode-0 or 15 MB in mode-2)
> is reserved through DT entries for firmware usage. Send the base
> address and size from DT entries.
>
> If DT entry is available, PCI devices work with
> fixed_mem_region else host allocates multiple segments.
>
> IPQ8074 on HK10 board supports multiple PCI devices.
> IPQ8074 + QCN9074 is tested with this patch.
>
> Tested-on: QCN9074 hw1.0 PCI WLAN.HK.2.4.0.1-01838-QCAHKSWPL_SILICONZ-1
>
> Signed-off-by: Anilkumar Kolli <[email protected]>
> ---
> V2:
> - Use of_ API to read from dt node (Rob)
>
> drivers/net/wireless/ath/ath11k/mhi.c | 42 ++++++++++++++++++--
> drivers/net/wireless/ath/ath11k/pci.c | 10 ++++-
> drivers/net/wireless/ath/ath11k/qmi.c | 75 +++++++++++++++++++++++++++++++----
> drivers/net/wireless/ath/ath11k/qmi.h | 1 +
> 4 files changed, 116 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
> index 26c7ae242db6..0ea6ab7edeb9 100644
> --- a/drivers/net/wireless/ath/ath11k/mhi.c
> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
> @@ -3,6 +3,7 @@
>
> #include <linux/msi.h>
> #include <linux/pci.h>
> +#include <linux/of.h>
>
> #include "core.h"
> #include "debug.h"
> @@ -316,7 +317,11 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
> struct ath11k_base *ab = ab_pci->ab;
> struct mhi_controller *mhi_ctrl;
> struct mhi_controller_config *ath11k_mhi_config;
> - int ret;
> + struct device_node *np;
> + int ret, len, sw, aw;
> + u32 *reg, *reg_end;
> + unsigned long start, size;
> + bool no_dt_entry = 0;
Use only true or false with bools.
> mhi_ctrl = mhi_alloc_controller();
> if (!mhi_ctrl)
> @@ -339,8 +344,39 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
> return ret;
> }
>
> - mhi_ctrl->iova_start = 0;
> - mhi_ctrl->iova_stop = 0xffffffff;
> + np = of_find_node_by_type(NULL, "memory");
> + if (!np) {
> + no_dt_entry = 1;
true
> + goto no_dt_entry;
> + }
> +
> + aw = of_n_addr_cells(np);
> + sw = of_n_size_cells(np);
> +
> + reg = (unsigned int *)of_get_property(np, "reg", &len);
Always try to avoid casts. of_get_property() returns 'const void *' so
you are casting out const here, which is a bug. You don't need to do
casting with void pointers so remove it here.
But is reg property really unsigned int? And not u32 or dma_addr_t, for
example?
> + if (!reg) {
> + no_dt_entry = 1;
true
> + goto no_dt_entry;
> + }
> +
> + reg_end = reg + len / (aw * sw);
> +
> + do {
> + start = of_read_number(reg, aw);
> + reg += aw;
> + size = of_read_number(reg, sw);
of_read_number() takes 'const __be32 *cell' but reg is 'u32 *'?
> + reg += sw;
> + } while (reg < reg_end);
> +
> +no_dt_entry:
> + if (no_dt_entry) {
> + mhi_ctrl->iova_start = 0;
> + mhi_ctrl->iova_stop = 0xFFFFFFFF;
> + } else {
> + mhi_ctrl->iova_start = (dma_addr_t)(start + 0x1000000);
> + mhi_ctrl->iova_stop = (dma_addr_t)(start + size);
I don't like casts, they hide bugs like the const issue above. Is there
any way to do this without casts?
> @@ -1245,6 +1246,13 @@ static int ath11k_pci_probe(struct pci_dev *pdev,
> pci_set_drvdata(pdev, ab);
> spin_lock_init(&ab_pci->window_lock);
>
> + /* Set fixed_mem_region to true for platforms support reserved memory
> + * from DT. If memory is reserved from DT for FW, ath11k driver need not
> + * allocate memory.
> + */
> + if (!of_property_read_u32(ab->dev->of_node, "memory-region", &addr))
> + ab->bus_params.fixed_mem_region = true;
ab->bus_params is supposed to be const to catch this, but clearly it's
not. The idea of bus_params is to have static bus level configuration,
please instead use dev_flags or something else.
> static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
> {
> - int i, idx;
> + struct device *dev = ab->dev;
> + struct device_node *hremote_node = NULL;
> + phandle hremote_phandle;
> + int i, idx, len, sw, aw, host_ddr_sz;
> + u32 *reg, *reg_end;
> + u64 start, size;
>
> for (i = 0, idx = 0; i < ab->qmi.mem_seg_count; i++) {
> switch (ab->qmi.target_mem[i].type) {
> + case HOST_DDR_REGION_TYPE:
> + if (of_property_read_u32(dev->of_node, "memory-region",
> + &hremote_phandle)) {
> + ath11k_dbg(ab, ATH11K_DBG_QMI,
> + "qmi fail to get hremote phandle\n");
> + return 0;
> + }
> +
> + hremote_node = of_find_node_by_phandle(hremote_phandle);
> + if (!hremote_node) {
> + ath11k_dbg(ab, ATH11K_DBG_QMI,
> + "qmi fail to get hremote_node\n");
> + return 0;
> + }
> +
> + aw = of_n_addr_cells(hremote_node);
> + sw = of_n_size_cells(hremote_node);
> +
> + reg = (unsigned int *)of_get_property(hremote_node, "reg", &len);
No need to cast. And I guess you are again removing the const?
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On 2021-11-17 13:35, Kalle Valo wrote:
> Anilkumar Kolli <[email protected]> writes:
>
>> Host DDR memory (contiguous 45 MB in mode-0 or 15 MB in mode-2)
>> is reserved through DT entries for firmware usage. Send the base
>> address and size from DT entries.
>>
>> If DT entry is available, PCI devices work with
>> fixed_mem_region else host allocates multiple segments.
>>
>> IPQ8074 on HK10 board supports multiple PCI devices.
>> IPQ8074 + QCN9074 is tested with this patch.
>>
>> Tested-on: QCN9074 hw1.0 PCI
>> WLAN.HK.2.4.0.1-01838-QCAHKSWPL_SILICONZ-1
>>
>> Signed-off-by: Anilkumar Kolli <[email protected]>
>> ---
>> V2:
>> - Use of_ API to read from dt node (Rob)
>>
>> drivers/net/wireless/ath/ath11k/mhi.c | 42 ++++++++++++++++++--
>> drivers/net/wireless/ath/ath11k/pci.c | 10 ++++-
>> drivers/net/wireless/ath/ath11k/qmi.c | 75
>> +++++++++++++++++++++++++++++++----
>> drivers/net/wireless/ath/ath11k/qmi.h | 1 +
>> 4 files changed, 116 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath11k/mhi.c
>> b/drivers/net/wireless/ath/ath11k/mhi.c
>> index 26c7ae242db6..0ea6ab7edeb9 100644
>> --- a/drivers/net/wireless/ath/ath11k/mhi.c
>> +++ b/drivers/net/wireless/ath/ath11k/mhi.c
>> @@ -3,6 +3,7 @@
>>
>> #include <linux/msi.h>
>> #include <linux/pci.h>
>> +#include <linux/of.h>
>>
>> #include "core.h"
>> #include "debug.h"
>> @@ -316,7 +317,11 @@ int ath11k_mhi_register(struct ath11k_pci
>> *ab_pci)
>> struct ath11k_base *ab = ab_pci->ab;
>> struct mhi_controller *mhi_ctrl;
>> struct mhi_controller_config *ath11k_mhi_config;
>> - int ret;
>> + struct device_node *np;
>> + int ret, len, sw, aw;
>> + u32 *reg, *reg_end;
>> + unsigned long start, size;
>> + bool no_dt_entry = 0;
>
> Use only true or false with bools.
>
Sure.
>> mhi_ctrl = mhi_alloc_controller();
>> if (!mhi_ctrl)
>> @@ -339,8 +344,39 @@ int ath11k_mhi_register(struct ath11k_pci
>> *ab_pci)
>> return ret;
>> }
>>
>> - mhi_ctrl->iova_start = 0;
>> - mhi_ctrl->iova_stop = 0xffffffff;
>> + np = of_find_node_by_type(NULL, "memory");
>> + if (!np) {
>> + no_dt_entry = 1;
>
> true
>
>> + goto no_dt_entry;
>> + }
>> +
>> + aw = of_n_addr_cells(np);
>> + sw = of_n_size_cells(np);
>> +
>> + reg = (unsigned int *)of_get_property(np, "reg", &len);
>
> Always try to avoid casts. of_get_property() returns 'const void *' so
> you are casting out const here, which is a bug. You don't need to do
> casting with void pointers so remove it here.
>
Sure.
> But is reg property really unsigned int? And not u32 or dma_addr_t, for
> example?
>
From DT specification, each value in reg is a u32.
In our example
memory {
device_type = "memory";
reg = <0x0 0x40000000 0x0 0x20000000>;
};
0x0 is higer 32-bit addr of RAM
0x40000000 is lower 32-bit addr of RAM
0x0 is size
0x20000000 is [1GB RAM] size
>> + if (!reg) {
>> + no_dt_entry = 1;
>
> true
>
>> + goto no_dt_entry;
>> + }
>> +
>> + reg_end = reg + len / (aw * sw);
>> +
>> + do {
>> + start = of_read_number(reg, aw);
>> + reg += aw;
>> + size = of_read_number(reg, sw);
>
> of_read_number() takes 'const __be32 *cell' but reg is 'u32 *'?
>
Yes.
>> + reg += sw;
>> + } while (reg < reg_end);
>> +
>> +no_dt_entry:
>> + if (no_dt_entry) {
>> + mhi_ctrl->iova_start = 0;
>> + mhi_ctrl->iova_stop = 0xFFFFFFFF;
>> + } else {
>> + mhi_ctrl->iova_start = (dma_addr_t)(start + 0x1000000);
>> + mhi_ctrl->iova_stop = (dma_addr_t)(start + size);
>
> I don't like casts, they hide bugs like the const issue above. Is there
> any way to do this without casts?
>
u64 start, size;
if (of_property_read_u32_array(np, "reg", reg, 4)) {
ath11k_dbg(ab, ATH11K_DBG_QMI,
"qmi fail to get reg from hremote\n");
return 0;
}
start = reg[0] + reg[1];
size = reg[2] + reg[3];
I will this code in next patch. still needs typecast form u64 to
dma_addr_t. no ?
>> @@ -1245,6 +1246,13 @@ static int ath11k_pci_probe(struct pci_dev
>> *pdev,
>> pci_set_drvdata(pdev, ab);
>> spin_lock_init(&ab_pci->window_lock);
>>
>> + /* Set fixed_mem_region to true for platforms support reserved
>> memory
>> + * from DT. If memory is reserved from DT for FW, ath11k driver need
>> not
>> + * allocate memory.
>> + */
>> + if (!of_property_read_u32(ab->dev->of_node, "memory-region", &addr))
>> + ab->bus_params.fixed_mem_region = true;
>
> ab->bus_params is supposed to be const to catch this, but clearly it's
> not. The idea of bus_params is to have static bus level configuration,
> please instead use dev_flags or something else.
>
Sure.
>> static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
>> {
>> - int i, idx;
>> + struct device *dev = ab->dev;
>> + struct device_node *hremote_node = NULL;
>> + phandle hremote_phandle;
>> + int i, idx, len, sw, aw, host_ddr_sz;
>> + u32 *reg, *reg_end;
>> + u64 start, size;
>>
>> for (i = 0, idx = 0; i < ab->qmi.mem_seg_count; i++) {
>> switch (ab->qmi.target_mem[i].type) {
>> + case HOST_DDR_REGION_TYPE:
>> + if (of_property_read_u32(dev->of_node, "memory-region",
>> + &hremote_phandle)) {
>> + ath11k_dbg(ab, ATH11K_DBG_QMI,
>> + "qmi fail to get hremote phandle\n");
>> + return 0;
>> + }
>> +
>> + hremote_node = of_find_node_by_phandle(hremote_phandle);
>> + if (!hremote_node) {
>> + ath11k_dbg(ab, ATH11K_DBG_QMI,
>> + "qmi fail to get hremote_node\n");
>> + return 0;
>> + }
>> +
>> + aw = of_n_addr_cells(hremote_node);
>> + sw = of_n_size_cells(hremote_node);
>> +
>> + reg = (unsigned int *)of_get_property(hremote_node, "reg", &len);
>
> No need to cast. And I guess you are again removing the const?
- Anil.
Anilkumar Kolli <[email protected]> writes:
> On 2021-11-17 13:35, Kalle Valo wrote:
>> Anilkumar Kolli <[email protected]> writes:
>>
>>> Host DDR memory (contiguous 45 MB in mode-0 or 15 MB in mode-2)
>>> is reserved through DT entries for firmware usage. Send the base
>>> address and size from DT entries.
>>>
>>> If DT entry is available, PCI devices work with
>>> fixed_mem_region else host allocates multiple segments.
>>>
>>> IPQ8074 on HK10 board supports multiple PCI devices.
>>> IPQ8074 + QCN9074 is tested with this patch.
>>>
>>> Tested-on: QCN9074 hw1.0 PCI
>>> WLAN.HK.2.4.0.1-01838-QCAHKSWPL_SILICONZ-1
>>>
>>> Signed-off-by: Anilkumar Kolli <[email protected]>
[...]
>>> + goto no_dt_entry;
>>> + }
>>> +
>>> + reg_end = reg + len / (aw * sw);
>>> +
>>> + do {
>>> + start = of_read_number(reg, aw);
>>> + reg += aw;
>>> + size = of_read_number(reg, sw);
>>
>> of_read_number() takes 'const __be32 *cell' but reg is 'u32 *'?
>>
> Yes.
My point here was that doesn't this mixing of __be32 and u32 when cause
a sparse warning?
>
>>> + reg += sw;
>>> + } while (reg < reg_end);
>>> +
>>> +no_dt_entry:
>>> + if (no_dt_entry) {
>>> + mhi_ctrl->iova_start = 0;
>>> + mhi_ctrl->iova_stop = 0xFFFFFFFF;
>>> + } else {
>>> + mhi_ctrl->iova_start = (dma_addr_t)(start + 0x1000000);
>>> + mhi_ctrl->iova_stop = (dma_addr_t)(start + size);
>>
>> I don't like casts, they hide bugs like the const issue above. Is there
>> any way to do this without casts?
>>
>
> u64 start, size;
>
> if (of_property_read_u32_array(np, "reg", reg, 4)) {
> ath11k_dbg(ab, ATH11K_DBG_QMI,
> "qmi fail to get reg from hremote\n");
> return 0;
> }
> start = reg[0] + reg[1];
> size = reg[2] + reg[3];
>
> I will this code in next patch. still needs typecast form u64 to
> dma_addr_t. no ?
I suspect that a cast is not needed, but not sure. No time to check either.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
On Tuesday, 16 November 2021 18:00:58 CET Anilkumar Kolli wrote:
[...]
> @@ -1866,10 +1867,62 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab)
>
> static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
> {
> - int i, idx;
> + struct device *dev = ab->dev;
> + struct device_node *hremote_node = NULL;
> + phandle hremote_phandle;
> + int i, idx, len, sw, aw, host_ddr_sz;
> + u32 *reg, *reg_end;
> + u64 start, size;
>
> for (i = 0, idx = 0; i < ab->qmi.mem_seg_count; i++) {
> switch (ab->qmi.target_mem[i].type) {
> + case HOST_DDR_REGION_TYPE:
> + if (of_property_read_u32(dev->of_node, "memory-region",
> + &hremote_phandle)) {
> + ath11k_dbg(ab, ATH11K_DBG_QMI,
> + "qmi fail to get hremote phandle\n");
> + return 0;
> + }
> +
> + hremote_node = of_find_node_by_phandle(hremote_phandle);
> + if (!hremote_node) {
> + ath11k_dbg(ab, ATH11K_DBG_QMI,
> + "qmi fail to get hremote_node\n");
> + return 0;
> + }
> +
Why aren't you using something like of_address_to_resource? Or some other
__of_get_address related function.
> + aw = of_n_addr_cells(hremote_node);
> + sw = of_n_size_cells(hremote_node);
> +
> + reg = (unsigned int *)of_get_property(hremote_node, "reg", &len);
> + if (!reg) {
> + ath11k_dbg(ab, ATH11K_DBG_QMI,
> + "qmi fail to get reg from hremote\n");
> + return 0;
> + }
> +
> + reg_end = reg + len / (aw * sw);
Why are you multiplying aw with sw? And why are you then dividing len by it?
1. if you calculate (u32 *)x + 1 then then you would increase the address by 4 bytes.
2. for each address + size pair you would have (aw + sw) * 4 bytes
3. len is in bytes
So if you want to get the first byte after a full u32 reg buffer then it would be:
reg + len / sizeof(u32);
If you would want to get the amount of full address+size pairs
len / sizeof(u32) / (aw + sw)
> +
> + do {
> + start = of_read_number(reg, aw);
> + reg += aw;
> + size = of_read_number(reg, sw);
> + reg += sw;
> + } while (reg < reg_end);
What are you trying to achieve with this loop?
Kind regards,
Sven