diff mbox series

[PATH,v3,1/2] dt: bindings: add new DT entry for ath11k PCI device support

Message ID 1637244892-27267-1-git-send-email-akolli@codeaurora.org (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series [PATH,v3,1/2] dt: bindings: add new DT entry for ath11k PCI device support | expand

Commit Message

Anilkumar Kolli Nov. 18, 2021, 2:14 p.m. UTC
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 <akolli@codeaurora.org>
---
V2:
  - Use reserved-memory (Rob)

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

Comments

Rob Herring (Arm) Nov. 18, 2021, 10:09 p.m. UTC | #1
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 <akolli@codeaurora.org>
> ---
> 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.
Rob Herring (Arm) Nov. 18, 2021, 11:11 p.m. UTC | #2
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 <akolli@codeaurora.org>
> ---
> 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
> 
>
Anilkumar Kolli Nov. 19, 2021, 8:21 a.m. UTC | #3
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 <akolli@codeaurora.org>
>> ---
>> 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.
Anilkumar Kolli Nov. 19, 2021, 8:41 a.m. UTC | #4
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 <akolli@codeaurora.org>
>> ---
>> 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.
diff mbox series

Patch

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