dt: bindings: add new DT entry for ath11k PCI device support
Subsystems (3)
PATCH v2 1/2 dt: bindings: add new DT entry for ath11k PCI device support
Ath11k driver supports PCI devices such as QCN9074/QCA6390. Ath11k firmware uses host DDR memory. DT entry is used to reserve these host DDR memory regions, send these memory base addresses using DT entries.
---
V2:
- Use reserved-memory (Rob)
.../bindings/net/wireless/qcom,ath11k.yaml | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+)
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml +48 -0
@@ -150,6 +150,12 @@ properties: string to uniquely identify variant of the calibration data in the board-2.bin for designs with colliding bus and device specific ids + memory-region: + maxItems: 1 + description: + phandle to a node describing reserved memory (System RAM memory) + used by ath11k firmware (see bindings/reserved-memory/reserved-memory.txt) + required: - compatible - reg
@@ -279,3 +285,45 @@ examples: "tcl2host-status-ring"; qcom,rproc = <&q6v5_wcss>; }; + + memory { + device_type = "memory"; + reg = <0x0 0x40000000 0x0 0x20000000>; + }; + + reserved-memory { + #address-cells = <1>; + #size-cells = <1>; + ranges; + + qcn9074_pcie0: qcn9074_pcie0@51100000 { + no-map; + reg = <0x0 0x51100000 0x0 0x03500000>; + }; + + qcn9074_pcie1: qcn9074_pcie1@54600000 { + no-map; + reg = <0x0 0x54600000 0x0 0x03500000>; + }; + }; + + pcie0_rp: pcie0_rp { + reg = <0 0 0 0 0>; + + status = "ok"; + ath11k_0: ath11k@0 { + reg = <0 0 0 0 0 >; + memory-region = <&qcn9074_pcie0>; + }; + }; + + pcie1_rp: pcie1_rp { + reg = <0 0 0 0 0>; + + status = "ok"; + ath11k_1: ath11k@1 { + reg = <0 0 0 0 0 >; + memory-region = <&qcn9074_pcie1>; + }; + }; +
Signature · 1 line
2.7.4
PATCH v2 2/2 ath11k: Use reserved host DDR addresses from DT for PCI devices
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
---
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(-)
drivers/net/wireless/ath/ath11k/mhi.c +39 -3
@@ -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;
drivers/net/wireless/ath/ath11k/pci.c +9 -1
@@ -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);
drivers/net/wireless/ath/ath11k/qmi.c +67 -8
97 lines not on this page · raw message
drivers/net/wireless/ath/ath11k/qmi.h +1 -0
@@ -95,6 +95,7 @@ struct target_mem_chunk {u32 type;dma_addr_t paddr;u32 *vaddr; +void __iomem *iaddr; };struct target_info {
Signature · 1 line
2.7.4
Re: [PATCH v2 2/2] ath11k: Use reserved host DDR addresses from DT for PCI devices
Quoted 36 lines · Anilkumar Kolli
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_regionelse 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:
- Useof_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(-)drivers/net/wireless/ath/ath11k/mhi.c +1
-0@@ -3,6 +3,7 @@#include <linux/msi.h> #include <linux/pci.h> +#include <linux/of.h> #include "core.h" #include "debug.h"
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 324
Quoted 9 lines
drivers/net/wireless/ath/ath11k/mhi.c +5
-1@@ -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.
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 349
Quoted 11 lines
drivers/net/wireless/ath/ath11k/mhi.c +3
-2mhi_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
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 356
Quoted 7 lines
drivers/net/wireless/ath/ath11k/mhi.c +7
-0+ 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?
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 358
Quoted 2 lines
drivers/net/wireless/ath/ath11k/mhi.c +2
-0+if (!reg) { + no_dt_entry =1 ;
true
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 367
Quoted 9 lines
drivers/net/wireless/ath/ath11k/mhi.c +9
-0+ 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 *'?
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 377
Quoted 10 lines
drivers/net/wireless/ath/ath11k/mhi.c +10
-0+ 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?
drivers/net/wireless/ath/ath11k/mhi.c static int ath11k_pci_probe(struct pci_dev *pdev, · line 1254
Quoted 10 lines
drivers/net/wireless/ath/ath11k/mhi.c +6
-0@@ -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.
Quoted 21 lines
drivers/net/wireless/ath/ath11k/mhi.c +15
-1static 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);
drivers/net/wireless/ath/ath11k/mhi.c static int ath11k_pci_probe(struct pci_dev *pdev,
Quoted 10 lines
drivers/net/wireless/ath/ath11k/mhi.c +10
-0+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?
Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2 2/2] ath11k: Use reserved host DDR addresses from DT for PCI devices
Quoted 36 lines · Kalle Valo
Quoted 34 lines · Anilkumar Kolli
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_regionelse 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:
- Useof_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(-)drivers/net/wireless/ath/ath11k/mhi.c +1
-0@@ -3,6 +3,7 @@#include <linux/msi.h> #include <linux/pci.h> +#include <linux/of.h>
Quoted 16 lines
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci · line 325
Quoted 13 lines
drivers/net/wireless/ath/ath11k/mhi.c +5
-1#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.
Quoted 15 lines
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci · line 350
Quoted 12 lines
drivers/net/wireless/ath/ath11k/mhi.c +3
-2mhi_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
Quoted 12 lines
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 356
Quoted 7 lines
drivers/net/wireless/ath/ath11k/mhi.c +7
-0+ 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.
Quoted 3 lines
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] sizeQuoted 5 lines
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 358
Quoted 2 lines
drivers/net/wireless/ath/ath11k/mhi.c +2
-0+if (!reg) { + no_dt_entry =1 ;true
Quoted 12 lines
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 367
Quoted 9 lines
drivers/net/wireless/ath/ath11k/mhi.c +9
-0+ 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.
Quoted 5 lines
Quoted 5 lines
drivers/net/wireless/ath/ath11k/mhi.c +5
-0+ reg += sw; + }while (reg < reg_end); + +no_dt_entry: +if (no_dt_entry) {
Quoted 9 lines
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 377
Quoted 5 lines
drivers/net/wireless/ath/ath11k/mhi.c +5
-0+ 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 ?
Quoted 5 lines
Quoted 5 lines
drivers/net/wireless/ath/ath11k/mhi.c +0
-0@@ -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);
Quoted 13 lines
drivers/net/wireless/ath/ath11k/mhi.c
Quoted 8 lines
drivers/net/wireless/ath/ath11k/mhi.c +1
-0+/* Set fixed_mem_region to true for platforms support reserved memory
drivers/net/wireless/ath/ath11k/mhi.c +1
-0+* from DT. If memory is reserved from DT for FW, ath11k driver need not
drivers/net/wireless/ath/ath11k/mhi.c +4
-0+* allocate memory. +*/ +if (!of_property_read_u32 (ab->dev->of_node,"memory-region" , &addr)) + ab->bus_params.fixed_mem_region = true;
ab->bus_paramsis supposed to be const to catch this, but clearly it's not. The idea ofbus_paramsis to have static bus level configuration, please instead usedev_flagsor something else.
Sure.
Quoted 25 lines
Quoted 25 lines
static int
ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab){drivers/net/wireless/ath/ath11k/mhi.c +19
-1-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 ;
Quoted 8 lines
drivers/net/wireless/ath/ath11k/mhi.c
Quoted 6 lines
drivers/net/wireless/ath/ath11k/mhi.c +6
-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.
Signature · 3 lines
ath11k mailing list [email protected] http://lists.infradead.org/mailman/listinfo/ath11k
Re: [PATCH v2 2/2] ath11k: Use reserved host DDR addresses from DT for PCI devices
Quoted 17 lines · Anilkumar Kolli
Quoted 16 lines · Kalle Valo
Quoted 14 lines · Anilkumar Kolli
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_regionelse 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-1Signed-off-by Anilkumar Kolli <[email protected]>
[...]
Quoted 12 lines
Quoted 12 lines
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 367
Quoted 9 lines
drivers/net/wireless/ath/ath11k/mhi.c +9
-0+ 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 *'?
Quoted 1 line
Yes.
My point here was that doesn't this mixing of __be32 and u32 when cause a sparse warning?
Quoted 26 lines
Quoted 15 lines
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 377
Quoted 11 lines
drivers/net/wireless/ath/ath11k/mhi.c +10
-0+ 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];
Quoted 2 lines
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.
Signature · 3 lines
https://patchwork.kernel.org/project/linux-wireless/list/ https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH v2 2/2] ath11k: Use reserved host DDR addresses from DT for PCI devices
In reply to Anilkumar KolliOn Tuesday, 16 November 2021 18:00:58 CET Anilkumar Kolli wrote:
[...]
Quoted 19 lines
drivers/net/wireless/ath/ath11k/qmi.c +11
-1@@ -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" );
drivers/net/wireless/ath/ath11k/qmi.c static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab) · line 1893
Quoted 10 lines
drivers/net/wireless/ath/ath11k/qmi.c +10
-0+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.
drivers/net/wireless/ath/ath11k/qmi.c static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab) · line 1904
Quoted 11 lines
drivers/net/wireless/ath/ath11k/qmi.c +11
-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);
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)
drivers/net/wireless/ath/ath11k/mhi.c int ath11k_mhi_register(struct ath11k_pci *ab_pci) · line 369
Quoted 7 lines
drivers/net/wireless/ath/ath11k/mhi.c +7
-0+ +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
Rob Herring
maintainerQuoted 5 lines · Anilkumar Kolli
Ath11k driver supports PCI devices such as QCN9074/QCA6390. Ath11k firmware uses host DDR memory. DT entry is used to reserve these host DDR memory regions, send these memory base addresses using DT entries.
Quoted 8 lines
Signed-off-by Anilkumar Kolli <[email protected]>---
V2:
- Use reserved-memory (Rob).../bindings/net/wireless/qcom,ath11k.yaml | 48 ++++++++++++++++++++++ 1 file changed, 48 insertions(+)
My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):
yamllint warnings/errors:
dtschema/dtc warnings/errors:
Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:160.13-31: Warning (reg_format): /example-0/pcie0_rp:reg: property has invalid length (20 bytes) (#address-cells == 1, #size-cells == 1) Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:164.17-36: Warning (reg_format): /example-0/pcie0_rp/ath11k@0:reg: property has invalid length (20 bytes) (#address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:170.13-31: Warning (reg_format): /example-0/pcie1_rp:reg: property has invalid length (20 bytes) (#address-cells == 1, #size-cells == 1) Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:174.17-36: Warning (reg_format): /example-0/pcie1_rp/ath11k@1:reg: property has invalid length (20 bytes) (#address-cells == 2, #size-cells == 1) Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:138.16-141.11: Warning (unit_address_vs_reg): /example-0/memory: node has a reg or ranges property, but no unit name Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:159.28-167.11: Warning (unit_address_vs_reg): /example-0/pcie0_rp: node has a reg or ranges property, but no unit name Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:169.28-177.11: Warning (unit_address_vs_reg): /example-0/pcie1_rp: node has a reg or ranges property, but no unit name Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dt.yaml: Warning (pci_device_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dt.yaml: Warning (pci_device_bus_num): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dt.yaml: Warning (simple_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dt.yaml: Warning (i2c_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dt.yaml: Warning (spi_bus_reg): Failed prerequisite 'reg_format' Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:163.32-166.15: Warning (avoid_default_addr_size): /example-0/pcie0_rp/ath11k@0: Relying on default #address-cells value Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:163.32-166.15: Warning (avoid_default_addr_size): /example-0/pcie0_rp/ath11k@0: Relying on default #size-cells value Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:173.32-176.15: Warning (avoid_default_addr_size): /example-0/pcie1_rp/ath11k@1: Relying on default #address-cells value Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dts:173.32-176.15: Warning (avoid_default_addr_size): /example-0/pcie1_rp/ath11k@1: Relying on default #size-cells value Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dt.yaml: Warning (unique_unit_address): Failed prerequisite 'avoid_default_addr_size' /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dt.yaml: reserved-memory: qcn9074_pcie0@51100000:reg:0: [0, 1360003072, 0, 55574528] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.example.dt.yaml: reserved-memory: qcn9074_pcie1@54600000:reg:0: [0, 1415577600, 0, 55574528] is too long
From schema: /usr/local/lib/python3.8/dist-packages/dtschema/schemas/reg.yaml
doc reference errors (make refcheckdocs):
See https://patchwork.ozlabs.org/patch/1555958
This check can fail if there are any dependencies. The base for a patch series is generally the most recent rc1.
If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure 'yamllint' is installed and dt-schema is up to date:
pip3 install dtschema --upgrade
Please check and re-submit.