2021-11-18 14:15:06

by Anilkumar Kolli

[permalink] [raw]
Subject: [PATH v3 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.

Signed-off-by: Anilkumar Kolli <[email protected]>
---
V2:
- Use reserved-memory (Rob)

.../bindings/net/wireless/qcom,ath11k.yaml | 48 ++++++++++++++++++++++
1 file changed, 48 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
index 85c2f699d602..5a8994f6cb10 100644
--- a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
+++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
@@ -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>;
+ };
+ };
+
--
2.7.4



2021-11-18 14:15:45

by Anilkumar Kolli

[permalink] [raw]
Subject: [PATH v3 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 from DT entries.
If DT entry is available, PCI device will 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]>
---
V3:
- remove type cast and use of_property_read_u32_array() (Kalle)
V2:
- Use of_ API to read from dt node (Rob)

drivers/net/wireless/ath/ath11k/core.h | 1 +
drivers/net/wireless/ath/ath11k/mhi.c | 31 ++++++++++++++-
drivers/net/wireless/ath/ath11k/pci.c | 10 ++++-
drivers/net/wireless/ath/ath11k/qmi.c | 73 +++++++++++++++++++++++++++++-----
drivers/net/wireless/ath/ath11k/qmi.h | 1 +
5 files changed, 104 insertions(+), 12 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.h b/drivers/net/wireless/ath/ath11k/core.h
index 2f1e10b7cc17..8492ca7efb92 100644
--- a/drivers/net/wireless/ath/ath11k/core.h
+++ b/drivers/net/wireless/ath/ath11k/core.h
@@ -194,6 +194,7 @@ enum ath11k_dev_flags {
ATH11K_FLAG_REGISTERED,
ATH11K_FLAG_QMI_FAIL,
ATH11K_FLAG_HTC_SUSPEND_COMPLETE,
+ ATH11K_FLAG_FIXED_MEM_RGN,
};

enum ath11k_monitor_flags {
diff --git a/drivers/net/wireless/ath/ath11k/mhi.c b/drivers/net/wireless/ath/ath11k/mhi.c
index 26c7ae242db6..cf335c80cd11 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"
@@ -311,6 +312,26 @@ static void ath11k_mhi_op_write_reg(struct mhi_controller *mhi_cntrl,
writel(val, addr);
}

+static bool ath11k_mhi_read_addr_from_dt(struct mhi_controller *mhi_ctrl)
+{
+ struct device_node *np;
+ u32 reg[4];
+ dma_addr_t start;
+
+ np = of_find_node_by_type(NULL, "memory");
+ if (!np)
+ return false;
+
+ if (of_property_read_u32_array(np, "reg", reg, 4))
+ return false;
+
+ start = reg[0] + reg[1];
+ mhi_ctrl->iova_start = start + 0x1000000;
+ mhi_ctrl->iova_stop = start + reg[2] + reg[3];
+
+ return true;
+}
+
int ath11k_mhi_register(struct ath11k_pci *ab_pci)
{
struct ath11k_base *ab = ab_pci->ab;
@@ -339,8 +360,14 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
return ret;
}

- mhi_ctrl->iova_start = 0;
- mhi_ctrl->iova_stop = 0xffffffff;
+ if ((test_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags))) {
+ if (!ath11k_mhi_read_addr_from_dt(mhi_ctrl))
+ return -ENODATA;
+ } else {
+ mhi_ctrl->iova_start = 0;
+ mhi_ctrl->iova_stop = 0xFFFFFFFF;
+ }
+
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..26378c508d28 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))
+ set_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags);
+
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..90d11ec0d436 100644
--- a/drivers/net/wireless/ath/ath11k/qmi.c
+++ b/drivers/net/wireless/ath/ath11k/qmi.c
@@ -1749,7 +1749,9 @@ static int ath11k_qmi_respond_fw_mem_request(struct ath11k_base *ab)
* failure to FW and FW will then request mulitple blocks of small
* chunk size memory.
*/
- if (!ab->bus_params.fixed_mem_region && ab->qmi.target_mem_delayed) {
+ if (!(ab->bus_params.fixed_mem_region ||
+ test_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags)) &&
+ ab->qmi.target_mem_delayed) {
delayed = true;
ath11k_dbg(ab, ATH11K_DBG_QMI, "qmi delays mem_request %d\n",
ab->qmi.mem_seg_count);
@@ -1815,10 +1817,12 @@ 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 ||
+ test_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags)) &&
+ ab->qmi.target_mem[i].iaddr)
+ iounmap(ab->qmi.target_mem[i].iaddr);
+
if (!ab->qmi.target_mem[i].vaddr)
continue;

@@ -1866,10 +1870,54 @@ static int ath11k_qmi_alloc_target_mem_chunk(struct ath11k_base *ab)

static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
{
+ struct device *dev = ab->dev;
+ struct device_node *hremote_node = NULL;
+ phandle hremote_phandle;
+ dma_addr_t start;
+ u32 reg[4], size, host_ddr_sz;
int i, idx;

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;
+ }
+
+ if (of_property_read_u32_array(hremote_node, "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];
+
+ if (size < ab->qmi.target_mem[i].size) {
+ ath11k_dbg(ab, ATH11K_DBG_QMI,
+ "qmi fail to assign memory of sz %u\n", size);
+ return 0;
+ }
+
+ 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 +1932,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;
@@ -2614,7 +2668,8 @@ static void ath11k_qmi_msg_mem_request_cb(struct qmi_handle *qmi_hdl,
msg->mem_seg[i].type, msg->mem_seg[i].size);
}

- if (ab->bus_params.fixed_mem_region) {
+ if (ab->bus_params.fixed_mem_region ||
+ test_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags)) {
ret = ath11k_qmi_assign_target_mem_chunk(ab);
if (ret) {
ath11k_warn(ab, "failed to assign qmi target memory: %d\n",
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


2021-11-18 22:10:00

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATH v3 1/2] dt: bindings: add new DT entry for ath11k PCI device support

On Thu, 18 Nov 2021 19:44:51 +0530, Anilkumar Kolli wrote:
> 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.
>
> 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/1556692

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.


2021-11-18 23:11:51

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATH v3 1/2] dt: bindings: add new DT entry for ath11k PCI device support

On Thu, Nov 18, 2021 at 07:44:51PM +0530, Anilkumar Kolli wrote:
> 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.
>
> Signed-off-by: Anilkumar Kolli <[email protected]>
> ---
> V2:
> - Use reserved-memory (Rob)
>
> .../bindings/net/wireless/qcom,ath11k.yaml | 48 ++++++++++++++++++++++
> 1 file changed, 48 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> index 85c2f699d602..5a8994f6cb10 100644
> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
> @@ -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>;
> };
> +

This looks like a separate example. Please split to its own entry.

> + memory {
> + device_type = "memory";
> + reg = <0x0 0x40000000 0x0 0x20000000>;
> + };

Outside the scope of what's needed in the example.

> +
> + 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>;
> + };
> + };

As is this really.

> +
> + pcie0_rp: pcie0_rp {
> + reg = <0 0 0 0 0>;

This isn't a valid PCI bus binding.

> +
> + status = "ok";

Don't need status.

> + ath11k_0: ath11k@0 {

wifi@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 >;

unit-address and reg don't match.

Why do we need 2 nodes in the first place?

> + memory-region = <&qcn9074_pcie1>;
> + };
> + };
> +
> --
> 2.7.4
>
>

2021-11-19 08:21:15

by Anilkumar Kolli

[permalink] [raw]
Subject: Re: [PATH v3 1/2] dt: bindings: add new DT entry for ath11k PCI device support

On 2021-11-19 03:39, Rob Herring wrote:
> On Thu, 18 Nov 2021 19:44:51 +0530, Anilkumar Kolli wrote:
>> 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.
>>
>> 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/1556692
>
> 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.

Sure, I will post v2 with fixes.

- Anil.

2021-11-19 08:41:30

by Anilkumar Kolli

[permalink] [raw]
Subject: Re: [PATH v3 1/2] dt: bindings: add new DT entry for ath11k PCI device support

On 2021-11-19 04:41, Rob Herring wrote:
> On Thu, Nov 18, 2021 at 07:44:51PM +0530, Anilkumar Kolli wrote:
>> 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.
>>
>> Signed-off-by: Anilkumar Kolli <[email protected]>
>> ---
>> V2:
>> - Use reserved-memory (Rob)
>>
>> .../bindings/net/wireless/qcom,ath11k.yaml | 48
>> ++++++++++++++++++++++
>> 1 file changed, 48 insertions(+)
>>
>> diff --git
>> a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
>> b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
>> index 85c2f699d602..5a8994f6cb10 100644
>> --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
>> +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath11k.yaml
>> @@ -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>;
>> };
>> +
>
> This looks like a separate example. Please split to its own entry.
>
>> + memory {
>> + device_type = "memory";
>> + reg = <0x0 0x40000000 0x0 0x20000000>;
>> + };
>
> Outside the scope of what's needed in the example.
>
Yes, memory entry is available in
"arch/arm64/boot/dts/qcom/ipq8074-hk10.dtsi"
Since its used in ath11k patch, added example.
I will remove in next version.

>> +
>> + 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>;
>> + };
>> + };
>
> As is this really.
>
ipq8074-hk10.dtsi board supports two PCI bus and QCN9074 on each PCI.
So added two separate entries to reserves memory for each QCN9074.

>> +
>> + pcie0_rp: pcie0_rp {
>> + reg = <0 0 0 0 0>;
>
> This isn't a valid PCI bus binding.
>
Got it, let me rework in next patch.

>> +
>> + status = "ok";
>
> Don't need status.

Sure, will remove in next patch.

>
>> + ath11k_0: ath11k@0 {
>
> wifi@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 >;
>
> unit-address and reg don't match.
>
will update in next patch.

> Why do we need 2 nodes in the first place?
>
ipq8074-hk10.dtsi board supports two PCI bus and QCN9074 on each PCI.
So added two separate entries to reserves memory for each QCN9074.

Since its example, Shall I remove one ?

>> + memory-region = <&qcn9074_pcie1>;
>> + };
>> + };
>> +
>> --
>> 2.7.4
>>

- Anil.

2021-11-19 13:00:55

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATH v3 2/2] ath11k: Use reserved host DDR addresses from DT for PCI devices

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 from DT entries.
> If DT entry is available, PCI device will 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]>

Not sure how I missed these in the last round:

> +static bool ath11k_mhi_read_addr_from_dt(struct mhi_controller *mhi_ctrl)

static int ....

> +{
> + struct device_node *np;
> + u32 reg[4];
> + dma_addr_t start;
> +
> + np = of_find_node_by_type(NULL, "memory");
> + if (!np)
> + return false;
> +
> + if (of_property_read_u32_array(np, "reg", reg, 4))
> + return false;

ret = of_property_read_u32_array(np, "reg", reg, 4);
if (ret)
return ret;

> + start = reg[0] + reg[1];
> + mhi_ctrl->iova_start = start + 0x1000000;
> + mhi_ctrl->iova_stop = start + reg[2] + reg[3];
> +
> + return true;

return 0;

> +}
> +
> int ath11k_mhi_register(struct ath11k_pci *ab_pci)
> {
> struct ath11k_base *ab = ab_pci->ab;
> @@ -339,8 +360,14 @@ int ath11k_mhi_register(struct ath11k_pci *ab_pci)
> return ret;
> }
>
> - mhi_ctrl->iova_start = 0;
> - mhi_ctrl->iova_stop = 0xffffffff;
> + if ((test_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags))) {
> + if (!ath11k_mhi_read_addr_from_dt(mhi_ctrl))
> + return -ENODATA;

ret = ath11k_mhi_read_addr_from_dt(mhi_ctrl);
if (ret)
return ret;

> @@ -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))
> + set_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags);

ret = of_property_read_u32(ab->dev->of_node, "memory-region", &addr);
if (!ret)
set_bit(ATH11K_FLAG_FIXED_MEM_RGN, &ab->dev_flags);

> static int ath11k_qmi_assign_target_mem_chunk(struct ath11k_base *ab)
> {
> + struct device *dev = ab->dev;
> + struct device_node *hremote_node = NULL;
> + phandle hremote_phandle;
> + dma_addr_t start;
> + u32 reg[4], size, host_ddr_sz;
> int i, idx;
>
> 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;
> + }
> +
> + if (of_property_read_u32_array(hremote_node, "reg", reg, 4)) {
> + ath11k_dbg(ab, ATH11K_DBG_QMI,
> + "qmi fail to get reg from hremote\n");
> + return 0;
> + }

ret = of_property_read_u32_array(hremote_node, "reg", reg, 4);
if (ret) {
ath11k_dbg(ab, ATH11K_DBG_QMI,
"qmi fail to get reg from hremote\n");
return 0;
}

--
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2021-11-19 13:57:02

by Sven Eckelmann

[permalink] [raw]
Subject: Re: [PATH v3 2/2] ath11k: Use reserved host DDR addresses from DT for PCI devices

On Thursday, 18 November 2021 15:14:52 CET Anilkumar Kolli wrote:
> + if (of_property_read_u32_array(hremote_node, "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];

That cannot be correct. Since when can upper 32 bit and lower 32 bit of an u64
be combined with a simple "+" and no shifting? And why can you operate on the
reg without getting the address + size cell count?

Kind regards,
Sven


Attachments:
signature.asc (833.00 B)
This is a digitally signed message part.