All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant
@ 2023-03-20 10:46 Johan Hovold
  2023-03-20 10:46 ` [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Johan Hovold
                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Johan Hovold @ 2023-03-20 10:46 UTC (permalink / raw)
  To: Kalle Valo, Bjorn Andersson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Andy Gross, Konrad Dybcio,
	linux-wireless, netdev, devicetree, linux-arm-msm, linux-kernel,
	Johan Hovold

This series adds the missing calibration variant devicetree property
which is needed to load the (just released) calibration data and use the
ath11k wifi on the X13s and sc8280xp-crd.

Kalle, do you want to take the binding through your tree or should Bjorn
take it all through the Qualcomm tree?

Johan


Johan Hovold (3):
  dt-bindings: wireless: add ath11k pcie bindings
  arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant
  arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant

 .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts     | 17 ++++++
 .../qcom/sc8280xp-lenovo-thinkpad-x13s.dts    | 17 ++++++
 3 files changed, 90 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml

-- 
2.39.2


^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-20 10:46 [PATCH 0/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
@ 2023-03-20 10:46 ` Johan Hovold
  2023-03-20 12:22   ` Kalle Valo
  2023-03-21  8:14   ` Krzysztof Kozlowski
  2023-03-20 10:46 ` [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
  2023-03-20 10:46 ` [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: " Johan Hovold
  2 siblings, 2 replies; 24+ messages in thread
From: Johan Hovold @ 2023-03-20 10:46 UTC (permalink / raw)
  To: Kalle Valo, Bjorn Andersson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Andy Gross, Konrad Dybcio,
	linux-wireless, netdev, devicetree, linux-arm-msm, linux-kernel,
	Johan Hovold

Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
for which the calibration data variant may need to be described.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml

diff --git a/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml b/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
new file mode 100644
index 000000000000..df67013822c6
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (c) 2023 Linaro Limited
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/wireless/pci17cb,1103.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Technologies ath11k wireless devices (PCIe)
+
+maintainers:
+  - Kalle Valo <kvalo@kernel.org>
+
+description: |
+  Qualcomm Technologies IEEE 802.11ax PCIe devices.
+
+properties:
+  compatible:
+    enum:
+      - pci17cb,1103  # WCN6856
+
+  reg:
+    maxItems: 1
+
+  qcom,ath11k-calibration-variant:
+    $ref: /schemas/types.yaml#/definitions/string
+    description: calibration data variant
+
+required:
+  - compatible
+  - reg
+
+additionalProperties: false
+
+examples:
+  - |
+    pcie {
+        #address-cells = <3>;
+        #size-cells = <2>;
+
+        pcie@0 {
+            device_type = "pci";
+            reg = <0x0 0x0 0x0 0x0 0x0>;
+            #address-cells = <3>;
+            #size-cells = <2>;
+            ranges;
+
+            bus-range = <0x01 0xff>;
+
+            wifi@0 {
+                compatible = "pci17cb,1103";
+                reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+                qcom,ath11k-calibration-variant = "LE_X13S";
+            };
+        };
+    };
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant
  2023-03-20 10:46 [PATCH 0/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
  2023-03-20 10:46 ` [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Johan Hovold
@ 2023-03-20 10:46 ` Johan Hovold
  2023-03-20 10:59   ` Konrad Dybcio
  2023-03-20 12:15   ` Steev Klimaszewski
  2023-03-20 10:46 ` [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: " Johan Hovold
  2 siblings, 2 replies; 24+ messages in thread
From: Johan Hovold @ 2023-03-20 10:46 UTC (permalink / raw)
  To: Kalle Valo, Bjorn Andersson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Andy Gross, Konrad Dybcio,
	linux-wireless, netdev, devicetree, linux-arm-msm, linux-kernel,
	Johan Hovold

Describe the bus topology for PCIe domain 6 and add the ath11k
calibration variant so that the board file (calibration data) can be
loaded.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216246
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts  | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index 150f51f1db37..0051025e0aa8 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -711,6 +711,23 @@ &pcie4 {
 	pinctrl-0 = <&pcie4_default>;
 
 	status = "okay";
+
+	pcie@0 {
+		device_type = "pci";
+		reg = <0x0 0x0 0x0 0x0 0x0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
+
+		bus-range = <0x01 0xff>;
+
+		wifi@0 {
+			compatible = "pci17cb,1103";
+			reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+			qcom,ath11k-calibration-variant = "LE_X13S";
+		};
+	};
 };
 
 &pcie4_phy {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant
  2023-03-20 10:46 [PATCH 0/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
  2023-03-20 10:46 ` [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Johan Hovold
  2023-03-20 10:46 ` [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
@ 2023-03-20 10:46 ` Johan Hovold
  2023-03-20 10:50   ` Konrad Dybcio
  2 siblings, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2023-03-20 10:46 UTC (permalink / raw)
  To: Kalle Valo, Bjorn Andersson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Andy Gross, Konrad Dybcio,
	linux-wireless, netdev, devicetree, linux-arm-msm, linux-kernel,
	Johan Hovold

Describe the bus topology for PCIe domain 6 and add the ath11k
calibration variant so that the board file (calibration data) can be
loaded.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=216036
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
index 90a5df9c7a24..5dfda12f669b 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
@@ -579,6 +579,23 @@ &pcie4 {
 	pinctrl-0 = <&pcie4_default>;
 
 	status = "okay";
+
+	pcie@0 {
+		device_type = "pci";
+		reg = <0x0 0x0 0x0 0x0 0x0>;
+		#address-cells = <3>;
+		#size-cells = <2>;
+		ranges;
+
+		bus-range = <0x01 0xff>;
+
+		wifi@0 {
+			compatible = "pci17cb,1103";
+			reg = <0x10000 0x0 0x0 0x0 0x0>;
+
+			qcom,ath11k-calibration-variant = "LE_X13S";
+		};
+	};
 };
 
 &pcie4_phy {
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant
  2023-03-20 10:46 ` [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: " Johan Hovold
@ 2023-03-20 10:50   ` Konrad Dybcio
  2023-03-20 10:55     ` Johan Hovold
  0 siblings, 1 reply; 24+ messages in thread
From: Konrad Dybcio @ 2023-03-20 10:50 UTC (permalink / raw)
  To: Johan Hovold, Kalle Valo, Bjorn Andersson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Andy Gross, linux-wireless,
	netdev, devicetree, linux-arm-msm, linux-kernel



On 20.03.2023 11:46, Johan Hovold wrote:
> Describe the bus topology for PCIe domain 6 and add the ath11k
> calibration variant so that the board file (calibration data) can be
> loaded.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216036
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> index 90a5df9c7a24..5dfda12f669b 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts


Was mixing
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts

this /\

[...]

and this \/
> +			qcom,ath11k-calibration-variant = "LE_X13S";
Intentional? Especially given Kalle's comment on bugzilla?

Konrad
> +		};
> +	};
>  };
>  
>  &pcie4_phy {

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant
  2023-03-20 10:50   ` Konrad Dybcio
@ 2023-03-20 10:55     ` Johan Hovold
  2023-03-20 10:59       ` Konrad Dybcio
  2023-03-20 11:07       ` Johan Hovold
  0 siblings, 2 replies; 24+ messages in thread
From: Johan Hovold @ 2023-03-20 10:55 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Johan Hovold, Kalle Valo, Bjorn Andersson, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, linux-wireless, netdev,
	devicetree, linux-arm-msm, linux-kernel

On Mon, Mar 20, 2023 at 11:50:30AM +0100, Konrad Dybcio wrote:
> 
> 
> On 20.03.2023 11:46, Johan Hovold wrote:
> > Describe the bus topology for PCIe domain 6 and add the ath11k
> > calibration variant so that the board file (calibration data) can be
> > loaded.
> > 
> > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216036
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > index 90a5df9c7a24..5dfda12f669b 100644
> > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> 
> 
> Was mixing
> > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> 
> this /\
> 
> [...]
> 
> and this \/
> > +			qcom,ath11k-calibration-variant = "LE_X13S";
> Intentional? Especially given Kalle's comment on bugzilla?

Yes, it is intentional. The corresponding calibration data allows the
wifi to be used on the CRD. I measure 150 MBits/s which may a bit lower
than expected, but it's better than having no wifi at all.

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant
  2023-03-20 10:55     ` Johan Hovold
@ 2023-03-20 10:59       ` Konrad Dybcio
  2023-03-20 11:07       ` Johan Hovold
  1 sibling, 0 replies; 24+ messages in thread
From: Konrad Dybcio @ 2023-03-20 10:59 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Kalle Valo, Bjorn Andersson, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, linux-wireless, netdev,
	devicetree, linux-arm-msm, linux-kernel



On 20.03.2023 11:55, Johan Hovold wrote:
> On Mon, Mar 20, 2023 at 11:50:30AM +0100, Konrad Dybcio wrote:
>>
>>
>> On 20.03.2023 11:46, Johan Hovold wrote:
>>> Describe the bus topology for PCIe domain 6 and add the ath11k
>>> calibration variant so that the board file (calibration data) can be
>>> loaded.
>>>
>>> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216036
>>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>>> ---
>>>  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 17 +++++++++++++++++
>>>  1 file changed, 17 insertions(+)
>>>
>>> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>>> index 90a5df9c7a24..5dfda12f669b 100644
>>> --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>>
>>
>> Was mixing
>>> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>>
>> this /\
>>
>> [...]
>>
>> and this \/
>>> +			qcom,ath11k-calibration-variant = "LE_X13S";
>> Intentional? Especially given Kalle's comment on bugzilla?
> 
> Yes, it is intentional. The corresponding calibration data allows the
> wifi to be used on the CRD. I measure 150 MBits/s which may a bit lower
> than expected, but it's better than having no wifi at all.
OK, sounds great!

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
> 
> Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant
  2023-03-20 10:46 ` [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
@ 2023-03-20 10:59   ` Konrad Dybcio
  2023-03-20 12:15   ` Steev Klimaszewski
  1 sibling, 0 replies; 24+ messages in thread
From: Konrad Dybcio @ 2023-03-20 10:59 UTC (permalink / raw)
  To: Johan Hovold, Kalle Valo, Bjorn Andersson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Andy Gross, linux-wireless,
	netdev, devicetree, linux-arm-msm, linux-kernel



On 20.03.2023 11:46, Johan Hovold wrote:
> Describe the bus topology for PCIe domain 6 and add the ath11k
> calibration variant so that the board file (calibration data) can be
> loaded.
> 
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216246
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>

Konrad
>  .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts  | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> index 150f51f1db37..0051025e0aa8 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> @@ -711,6 +711,23 @@ &pcie4 {
>  	pinctrl-0 = <&pcie4_default>;
>  
>  	status = "okay";
> +
> +	pcie@0 {
> +		device_type = "pci";
> +		reg = <0x0 0x0 0x0 0x0 0x0>;
> +		#address-cells = <3>;
> +		#size-cells = <2>;
> +		ranges;
> +
> +		bus-range = <0x01 0xff>;
> +
> +		wifi@0 {
> +			compatible = "pci17cb,1103";
> +			reg = <0x10000 0x0 0x0 0x0 0x0>;
> +
> +			qcom,ath11k-calibration-variant = "LE_X13S";
> +		};
> +	};
>  };
>  
>  &pcie4_phy {

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant
  2023-03-20 10:55     ` Johan Hovold
  2023-03-20 10:59       ` Konrad Dybcio
@ 2023-03-20 11:07       ` Johan Hovold
  2023-03-20 12:18         ` Kalle Valo
  1 sibling, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2023-03-20 11:07 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Johan Hovold, Kalle Valo, Bjorn Andersson, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, linux-wireless, netdev,
	devicetree, linux-arm-msm, linux-kernel

On Mon, Mar 20, 2023 at 11:55:48AM +0100, Johan Hovold wrote:
> On Mon, Mar 20, 2023 at 11:50:30AM +0100, Konrad Dybcio wrote:
> > 
> > 
> > On 20.03.2023 11:46, Johan Hovold wrote:
> > > Describe the bus topology for PCIe domain 6 and add the ath11k
> > > calibration variant so that the board file (calibration data) can be
> > > loaded.
> > > 
> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216036
> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > > ---
> > >  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 17 +++++++++++++++++
> > >  1 file changed, 17 insertions(+)
> > > 
> > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > > index 90a5df9c7a24..5dfda12f669b 100644
> > > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > 
> > 
> > Was mixing
> > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
> > 
> > this /\
> > 
> > [...]
> > 
> > and this \/
> > > +			qcom,ath11k-calibration-variant = "LE_X13S";
> > Intentional? Especially given Kalle's comment on bugzilla?
> 
> Yes, it is intentional. The corresponding calibration data allows the
> wifi to be used on the CRD. I measure 150 MBits/s which may a bit lower
> than expected, but it's better than having no wifi at all.

I was going back and forth about mentioning this in the commit message
and we could off on this one until someone confirms that the
corresponding calibration data can (or should) be used for the X13s.

Note that there is no other match for

	'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=0108,qmi-chip-id=2,qmi-board-id=140'

in the new board-2.bin.

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant
  2023-03-20 10:46 ` [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
  2023-03-20 10:59   ` Konrad Dybcio
@ 2023-03-20 12:15   ` Steev Klimaszewski
  1 sibling, 0 replies; 24+ messages in thread
From: Steev Klimaszewski @ 2023-03-20 12:15 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Kalle Valo, Bjorn Andersson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Andy Gross, Konrad Dybcio, linux-wireless, netdev, devicetree,
	linux-arm-msm, linux-kernel

On Mon, Mar 20, 2023 at 5:52 AM Johan Hovold <johan+linaro@kernel.org> wrote:
>
> Describe the bus topology for PCIe domain 6 and add the ath11k
> calibration variant so that the board file (calibration data) can be
> loaded.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=216246
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts  | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> index 150f51f1db37..0051025e0aa8 100644
> --- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> +++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
> @@ -711,6 +711,23 @@ &pcie4 {
>         pinctrl-0 = <&pcie4_default>;
>
>         status = "okay";
> +
> +       pcie@0 {
> +               device_type = "pci";
> +               reg = <0x0 0x0 0x0 0x0 0x0>;
> +               #address-cells = <3>;
> +               #size-cells = <2>;
> +               ranges;
> +
> +               bus-range = <0x01 0xff>;
> +
> +               wifi@0 {
> +                       compatible = "pci17cb,1103";
> +                       reg = <0x10000 0x0 0x0 0x0 0x0>;
> +
> +                       qcom,ath11k-calibration-variant = "LE_X13S";
> +               };
> +       };
>  };
>
>  &pcie4_phy {
> --
> 2.39.2
>

So glad to finally see this and confirm that it works!

Tested-by: Steev Klimaszewski <steev@kali.org> #Thinkpad X13s

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant
  2023-03-20 11:07       ` Johan Hovold
@ 2023-03-20 12:18         ` Kalle Valo
  2023-03-20 12:34           ` Johan Hovold
  0 siblings, 1 reply; 24+ messages in thread
From: Kalle Valo @ 2023-03-20 12:18 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Konrad Dybcio, Johan Hovold, Bjorn Andersson, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, linux-wireless, netdev,
	devicetree, linux-arm-msm, linux-kernel

Johan Hovold <johan@kernel.org> writes:

> On Mon, Mar 20, 2023 at 11:55:48AM +0100, Johan Hovold wrote:
>
>> On Mon, Mar 20, 2023 at 11:50:30AM +0100, Konrad Dybcio wrote:
>> > 
>> > 
>> > On 20.03.2023 11:46, Johan Hovold wrote:
>> > > Describe the bus topology for PCIe domain 6 and add the ath11k
>> > > calibration variant so that the board file (calibration data) can be
>> > > loaded.
>> > > 
>> > > Link: https://bugzilla.kernel.org/show_bug.cgi?id=216036
>> > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> > > ---
>> > >  arch/arm64/boot/dts/qcom/sc8280xp-crd.dts | 17 +++++++++++++++++
>> > >  1 file changed, 17 insertions(+)
>> > > 
>> > > diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>> > > b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>> > > index 90a5df9c7a24..5dfda12f669b 100644
>> > > --- a/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>> > 
>> > 
>> > Was mixing
>> > > +++ b/arch/arm64/boot/dts/qcom/sc8280xp-crd.dts
>> > 
>> > this /\
>> > 
>> > [...]
>> > 
>> > and this \/
>> > > +			qcom,ath11k-calibration-variant = "LE_X13S";
>> > Intentional? Especially given Kalle's comment on bugzilla?
>> 
>> Yes, it is intentional. The corresponding calibration data allows the
>> wifi to be used on the CRD. I measure 150 MBits/s which may a bit lower
>> than expected, but it's better than having no wifi at all.
>
> I was going back and forth about mentioning this in the commit message
> and we could off on this one until someone confirms that the
> corresponding calibration data can (or should) be used for the X13s.
>
> Note that there is no other match for
>
> 	'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=0108,qmi-chip-id=2,qmi-board-id=140'
>
> in the new board-2.bin.

If the device in question is something else than Lenovo X13s, I would
prefer that the variant is not set. Just in case we need different board
files for different models. It's easy to add aliases to board-2.bin.

I need to check internally what board file should be used for this CRD.
If the speed is only 150 Mbit/s I suspect it needs a different board
file.

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-20 10:46 ` [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Johan Hovold
@ 2023-03-20 12:22   ` Kalle Valo
  2023-03-20 12:42     ` Johan Hovold
  2023-03-21  8:14   ` Krzysztof Kozlowski
  1 sibling, 1 reply; 24+ messages in thread
From: Kalle Valo @ 2023-03-20 12:22 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Bjorn Andersson, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Rob Herring, Krzysztof Kozlowski, Andy Gross,
	Konrad Dybcio, linux-wireless, netdev, devicetree, linux-arm-msm,
	linux-kernel

Johan Hovold <johan+linaro@kernel.org> writes:

> Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
> for which the calibration data variant may need to be described.
>
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml

I'm confused (as usual), how does this differ from
bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files?

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant
  2023-03-20 12:18         ` Kalle Valo
@ 2023-03-20 12:34           ` Johan Hovold
  0 siblings, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2023-03-20 12:34 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Konrad Dybcio, Johan Hovold, Bjorn Andersson, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, linux-wireless, netdev,
	devicetree, linux-arm-msm, linux-kernel

On Mon, Mar 20, 2023 at 02:18:31PM +0200, Kalle Valo wrote:
> Johan Hovold <johan@kernel.org> writes:

> >> > > +			qcom,ath11k-calibration-variant = "LE_X13S";
> >> > Intentional? Especially given Kalle's comment on bugzilla?
> >> 
> >> Yes, it is intentional. The corresponding calibration data allows the
> >> wifi to be used on the CRD. I measure 150 MBits/s which may a bit lower
> >> than expected, but it's better than having no wifi at all.
> >
> > I was going back and forth about mentioning this in the commit message
> > and we could off on this one until someone confirms that the
> > corresponding calibration data can (or should) be used for the X13s.

Hopefully clear from context, but that was supposed to say "CRD" and not
"X13s"...

> > Note that there is no other match for
> >
> > 	'bus=pci,vendor=17cb,device=1103,subsystem-vendor=17cb,subsystem-device=0108,qmi-chip-id=2,qmi-board-id=140'
> >
> > in the new board-2.bin.
> 
> If the device in question is something else than Lenovo X13s, I would
> prefer that the variant is not set. Just in case we need different board
> files for different models. It's easy to add aliases to board-2.bin.

The sc8280xp CRD is the Qualcomm "compute" reference design for this
platform and is very similar to the X13s but they are not identical.

For ath11k and wcn6855, the CRD I have reports a chip_id of 2 and
"hw2.0", while the X13s reports chip_id 18 and "hw2.1".

The new board-2.bin notably adds two entries that match these chip_ids
but with the variant specified as "LE_X13S" for both.

> I need to check internally what board file should be used for this CRD.
> If the speed is only 150 Mbit/s I suspect it needs a different board
> file.

Sounds good. Let's drop this one for now then.

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-20 12:22   ` Kalle Valo
@ 2023-03-20 12:42     ` Johan Hovold
  2023-03-20 18:41         ` Kalle Valo
  0 siblings, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2023-03-20 12:42 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Johan Hovold, Bjorn Andersson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Andy Gross, Konrad Dybcio, linux-wireless, netdev, devicetree,
	linux-arm-msm, linux-kernel

On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote:
> Johan Hovold <johan+linaro@kernel.org> writes:
> 
> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
> > for which the calibration data variant may need to be described.
> >
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> 
> I'm confused (as usual), how does this differ from
> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files?

Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856
when using PCIe (e.g. as most properties are then discoverable).

We could try to encode everything in one file, but that would likely
just result in a big mess of a schema with conditionals all over.

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-20 12:42     ` Johan Hovold
@ 2023-03-20 18:41         ` Kalle Valo
  0 siblings, 0 replies; 24+ messages in thread
From: Kalle Valo @ 2023-03-20 18:41 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Bjorn Andersson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Andy Gross, Konrad Dybcio, linux-wireless, netdev, devicetree,
	linux-arm-msm, linux-kernel, ath11k

+ ath11k list

Johan Hovold <johan@kernel.org> writes:

> On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote:
>> Johan Hovold <johan+linaro@kernel.org> writes:
>> 
>> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
>> > for which the calibration data variant may need to be described.
>> >
>> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> > ---
>> >  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
>> >  1 file changed, 56 insertions(+)
>> >  create mode 100644
>> > Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
>> 
>> I'm confused (as usual), how does this differ from
>> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files?
>
> Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856
> when using PCIe (e.g. as most properties are then discoverable).
>
> We could try to encode everything in one file, but that would likely
> just result in a big mess of a schema with conditionals all over.

Ah, so the current qcom,ath11k.yaml would be only for ath11k AHB devices
and this new file is only for ath11k PCI devices? But why still the odd
name pci17cb,1103.yaml? It's not really descriptive and I'm for sure
will not remember that pci17cb,1103.yaml is for ath11k :)

Also it doesn't look good that we have qcom,ath11k-calibration-variant
documented twice now. I'm no DT expert but isn't there any other way? Is
it possible to include other files? For example, if we would have three
files:

qcom,ath11k.yaml
qcom,ath11k-ahb.yaml
qcom,ath11k-pci.yaml

Then have the common properties like ath11k-calibration-variant in the
first file and ahb/pci files would include that.

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

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

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
@ 2023-03-20 18:41         ` Kalle Valo
  0 siblings, 0 replies; 24+ messages in thread
From: Kalle Valo @ 2023-03-20 18:41 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Bjorn Andersson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Andy Gross, Konrad Dybcio, linux-wireless, netdev, devicetree,
	linux-arm-msm, linux-kernel, ath11k

+ ath11k list

Johan Hovold <johan@kernel.org> writes:

> On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote:
>> Johan Hovold <johan+linaro@kernel.org> writes:
>> 
>> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
>> > for which the calibration data variant may need to be described.
>> >
>> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> > ---
>> >  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
>> >  1 file changed, 56 insertions(+)
>> >  create mode 100644
>> > Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
>> 
>> I'm confused (as usual), how does this differ from
>> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files?
>
> Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856
> when using PCIe (e.g. as most properties are then discoverable).
>
> We could try to encode everything in one file, but that would likely
> just result in a big mess of a schema with conditionals all over.

Ah, so the current qcom,ath11k.yaml would be only for ath11k AHB devices
and this new file is only for ath11k PCI devices? But why still the odd
name pci17cb,1103.yaml? It's not really descriptive and I'm for sure
will not remember that pci17cb,1103.yaml is for ath11k :)

Also it doesn't look good that we have qcom,ath11k-calibration-variant
documented twice now. I'm no DT expert but isn't there any other way? Is
it possible to include other files? For example, if we would have three
files:

qcom,ath11k.yaml
qcom,ath11k-ahb.yaml
qcom,ath11k-pci.yaml

Then have the common properties like ath11k-calibration-variant in the
first file and ahb/pci files would include that.

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-20 10:46 ` [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Johan Hovold
  2023-03-20 12:22   ` Kalle Valo
@ 2023-03-21  8:14   ` Krzysztof Kozlowski
  2023-03-21  8:16     ` Krzysztof Kozlowski
  2023-03-21  8:27     ` Johan Hovold
  1 sibling, 2 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-21  8:14 UTC (permalink / raw)
  To: Johan Hovold, Kalle Valo, Bjorn Andersson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Andy Gross, Konrad Dybcio,
	linux-wireless, netdev, devicetree, linux-arm-msm, linux-kernel

On 20/03/2023 11:46, Johan Hovold wrote:
> Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
> for which the calibration data variant may need to be described.
> 
> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> ---
>  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
>  1 file changed, 56 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml b/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> new file mode 100644
> index 000000000000..df67013822c6
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml

PCI devices are kind of exception in the naming, so this should be
qcom,ath11k-pci.yaml or qcom,wcn6856.yaml (or something similar)


> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (c) 2023 Linaro Limited
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/wireless/pci17cb,1103.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Qualcomm Technologies ath11k wireless devices (PCIe)
> +
> +maintainers:
> +  - Kalle Valo <kvalo@kernel.org>
> +
> +description: |
> +  Qualcomm Technologies IEEE 802.11ax PCIe devices.
> +
> +properties:
> +  compatible:
> +    enum:
> +      - pci17cb,1103  # WCN6856
> +
> +  reg:
> +    maxItems: 1
> +
> +  qcom,ath11k-calibration-variant:

qcom,calibration-variant

> +    $ref: /schemas/types.yaml#/definitions/string
> +    description: calibration data variant

Your description copies the name of property. Instead say something more...

> +

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-21  8:14   ` Krzysztof Kozlowski
@ 2023-03-21  8:16     ` Krzysztof Kozlowski
  2023-03-21  8:27     ` Johan Hovold
  1 sibling, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-21  8:16 UTC (permalink / raw)
  To: Johan Hovold, Kalle Valo, Bjorn Andersson
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Rob Herring, Krzysztof Kozlowski, Andy Gross, Konrad Dybcio,
	linux-wireless, netdev, devicetree, linux-arm-msm, linux-kernel

On 21/03/2023 09:14, Krzysztof Kozlowski wrote:
> On 20/03/2023 11:46, Johan Hovold wrote:
>> Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
>> for which the calibration data variant may need to be described.
>>
>> Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> ---
>>  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
>>  1 file changed, 56 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml b/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
>> new file mode 100644
>> index 000000000000..df67013822c6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> 
> PCI devices are kind of exception in the naming, so this should be
> qcom,ath11k-pci.yaml or qcom,wcn6856.yaml (or something similar)
> 
> 
>> @@ -0,0 +1,56 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (c) 2023 Linaro Limited
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/wireless/pci17cb,1103.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Qualcomm Technologies ath11k wireless devices (PCIe)
>> +
>> +maintainers:
>> +  - Kalle Valo <kvalo@kernel.org>
>> +
>> +description: |
>> +  Qualcomm Technologies IEEE 802.11ax PCIe devices.
>> +
>> +properties:
>> +  compatible:
>> +    enum:
>> +      - pci17cb,1103  # WCN6856
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +  qcom,ath11k-calibration-variant:
> 
> qcom,calibration-variant

Ah, so there is already property with ath11k, then let's go with
existing name.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-20 18:41         ` Kalle Valo
@ 2023-03-21  8:22           ` Johan Hovold
  -1 siblings, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2023-03-21  8:22 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Johan Hovold, Bjorn Andersson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Andy Gross, Konrad Dybcio, linux-wireless, netdev, devicetree,
	linux-arm-msm, linux-kernel, ath11k

On Mon, Mar 20, 2023 at 08:41:21PM +0200, Kalle Valo wrote:
> + ath11k list
> 
> Johan Hovold <johan@kernel.org> writes:
> 
> > On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote:
> >> Johan Hovold <johan+linaro@kernel.org> writes:
> >> 
> >> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
> >> > for which the calibration data variant may need to be described.
> >> >
> >> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> >> > ---
> >> >  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
> >> >  1 file changed, 56 insertions(+)
> >> >  create mode 100644
> >> > Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> >> 
> >> I'm confused (as usual), how does this differ from
> >> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files?
> >
> > Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856
> > when using PCIe (e.g. as most properties are then discoverable).
> >
> > We could try to encode everything in one file, but that would likely
> > just result in a big mess of a schema with conditionals all over.
> 
> Ah, so the current qcom,ath11k.yaml would be only for ath11k AHB devices
> and this new file is only for ath11k PCI devices?

Right, there would two separate schema files for the two device classes.

> But why still the odd
> name pci17cb,1103.yaml? It's not really descriptive and I'm for sure
> will not remember that pci17cb,1103.yaml is for ath11k :)

Yeah, it's not the best name from that perspective, but it follows the
current convention of naming the schema files after the first compatible
added.

That said, we don't have many schemas for PCI devices so perhaps we can
establish a new convention for those. Perhaps by replacing the numerical
ids with what we'd use if these were platform devices (e.g.
'qcom,wcn6855.yaml').

As long as the DT maintainers are OK with it, I'd also be happy with
something like you suggest below:

	qcom,ath11k-ahb.yaml
	qcom,ath11k-pci.yaml

(or simply not renaming the current file 'qcom,ath11k.yaml') but I have
gotten push back on that in the past.

> Also it doesn't look good that we have qcom,ath11k-calibration-variant
> documented twice now. I'm no DT expert but isn't there any other way? Is
> it possible to include other files? For example, if we would have three
> files:
> 
> qcom,ath11k.yaml
> qcom,ath11k-ahb.yaml
> qcom,ath11k-pci.yaml
> 
> Then have the common properties like ath11k-calibration-variant in the
> first file and ahb/pci files would include that.

That should be possible, but it's not necessarily better as you'd then
have to look up two files to see the bindings for either device class
(and as far as I can tell there would not be much sharing beyond this
single property).

Note that the property could just have well have been named
'qcom,calibration-variant' and then it would be shared also with the
ath10k set of devices which currently holds another definition of what
is essentially the same property ('qcom,ath10k-calibration-variant').

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
@ 2023-03-21  8:22           ` Johan Hovold
  0 siblings, 0 replies; 24+ messages in thread
From: Johan Hovold @ 2023-03-21  8:22 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Johan Hovold, Bjorn Andersson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Andy Gross, Konrad Dybcio, linux-wireless, netdev, devicetree,
	linux-arm-msm, linux-kernel, ath11k

On Mon, Mar 20, 2023 at 08:41:21PM +0200, Kalle Valo wrote:
> + ath11k list
> 
> Johan Hovold <johan@kernel.org> writes:
> 
> > On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote:
> >> Johan Hovold <johan+linaro@kernel.org> writes:
> >> 
> >> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
> >> > for which the calibration data variant may need to be described.
> >> >
> >> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> >> > ---
> >> >  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
> >> >  1 file changed, 56 insertions(+)
> >> >  create mode 100644
> >> > Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> >> 
> >> I'm confused (as usual), how does this differ from
> >> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files?
> >
> > Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856
> > when using PCIe (e.g. as most properties are then discoverable).
> >
> > We could try to encode everything in one file, but that would likely
> > just result in a big mess of a schema with conditionals all over.
> 
> Ah, so the current qcom,ath11k.yaml would be only for ath11k AHB devices
> and this new file is only for ath11k PCI devices?

Right, there would two separate schema files for the two device classes.

> But why still the odd
> name pci17cb,1103.yaml? It's not really descriptive and I'm for sure
> will not remember that pci17cb,1103.yaml is for ath11k :)

Yeah, it's not the best name from that perspective, but it follows the
current convention of naming the schema files after the first compatible
added.

That said, we don't have many schemas for PCI devices so perhaps we can
establish a new convention for those. Perhaps by replacing the numerical
ids with what we'd use if these were platform devices (e.g.
'qcom,wcn6855.yaml').

As long as the DT maintainers are OK with it, I'd also be happy with
something like you suggest below:

	qcom,ath11k-ahb.yaml
	qcom,ath11k-pci.yaml

(or simply not renaming the current file 'qcom,ath11k.yaml') but I have
gotten push back on that in the past.

> Also it doesn't look good that we have qcom,ath11k-calibration-variant
> documented twice now. I'm no DT expert but isn't there any other way? Is
> it possible to include other files? For example, if we would have three
> files:
> 
> qcom,ath11k.yaml
> qcom,ath11k-ahb.yaml
> qcom,ath11k-pci.yaml
> 
> Then have the common properties like ath11k-calibration-variant in the
> first file and ahb/pci files would include that.

That should be possible, but it's not necessarily better as you'd then
have to look up two files to see the bindings for either device class
(and as far as I can tell there would not be much sharing beyond this
single property).

Note that the property could just have well have been named
'qcom,calibration-variant' and then it would be shared also with the
ath10k set of devices which currently holds another definition of what
is essentially the same property ('qcom,ath10k-calibration-variant').

Johan

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-21  8:14   ` Krzysztof Kozlowski
  2023-03-21  8:16     ` Krzysztof Kozlowski
@ 2023-03-21  8:27     ` Johan Hovold
  2023-03-21  8:29       ` Krzysztof Kozlowski
  1 sibling, 1 reply; 24+ messages in thread
From: Johan Hovold @ 2023-03-21  8:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski
  Cc: Johan Hovold, Kalle Valo, Bjorn Andersson, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, Konrad Dybcio, linux-wireless,
	netdev, devicetree, linux-arm-msm, linux-kernel

On Tue, Mar 21, 2023 at 09:14:15AM +0100, Krzysztof Kozlowski wrote:
> On 20/03/2023 11:46, Johan Hovold wrote:
> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
> > for which the calibration data variant may need to be described.
> > 
> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
> > ---
> >  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
> >  1 file changed, 56 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml b/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> > new file mode 100644
> > index 000000000000..df67013822c6
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
> 
> PCI devices are kind of exception in the naming, so this should be
> qcom,ath11k-pci.yaml or qcom,wcn6856.yaml (or something similar)

Heh, I suggested something similar in my reply to Kalle. Let's go with
'qcom,ath11k-pci.yaml' then as he first suggested (and keeping the
current schema file unchanged?).

> > @@ -0,0 +1,56 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# Copyright (c) 2023 Linaro Limited
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/net/wireless/pci17cb,1103.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Qualcomm Technologies ath11k wireless devices (PCIe)
> > +
> > +maintainers:
> > +  - Kalle Valo <kvalo@kernel.org>
> > +
> > +description: |
> > +  Qualcomm Technologies IEEE 802.11ax PCIe devices.
> > +
> > +properties:
> > +  compatible:
> > +    enum:
> > +      - pci17cb,1103  # WCN6856
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +  qcom,ath11k-calibration-variant:
> 
> qcom,calibration-variant

This one is already in use as you noticed.

> > +    $ref: /schemas/types.yaml#/definitions/string
> > +    description: calibration data variant
> 
> Your description copies the name of property. Instead say something more...

Yeah, I was actively avoiding trying to say too much (e.g. mentioning
the name of the current firmware file). See the definition in
qcom,ath11k.yaml.

I can try to find some middle ground unless you prefer copying the
current definition.

Johan

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-21  8:27     ` Johan Hovold
@ 2023-03-21  8:29       ` Krzysztof Kozlowski
  0 siblings, 0 replies; 24+ messages in thread
From: Krzysztof Kozlowski @ 2023-03-21  8:29 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Kalle Valo, Bjorn Andersson, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Rob Herring,
	Krzysztof Kozlowski, Andy Gross, Konrad Dybcio, linux-wireless,
	netdev, devicetree, linux-arm-msm, linux-kernel

On 21/03/2023 09:27, Johan Hovold wrote:
> 
>>> +    $ref: /schemas/types.yaml#/definitions/string
>>> +    description: calibration data variant
>>
>> Your description copies the name of property. Instead say something more...
> 
> Yeah, I was actively avoiding trying to say too much (e.g. mentioning
> the name of the current firmware file). See the definition in
> qcom,ath11k.yaml.
> 
> I can try to find some middle ground unless you prefer copying the
> current definition.

So just copy the description or its parts.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
  2023-03-21  8:22           ` Johan Hovold
@ 2023-03-22  5:59             ` Kalle Valo
  -1 siblings, 0 replies; 24+ messages in thread
From: Kalle Valo @ 2023-03-22  5:59 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Bjorn Andersson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Andy Gross, Konrad Dybcio, linux-wireless, netdev, devicetree,
	linux-arm-msm, linux-kernel, ath11k

Johan Hovold <johan@kernel.org> writes:

> On Mon, Mar 20, 2023 at 08:41:21PM +0200, Kalle Valo wrote:
>
>> + ath11k list
>> 
>> Johan Hovold <johan@kernel.org> writes:
>> 
>> > On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote:
>> >> Johan Hovold <johan+linaro@kernel.org> writes:
>> >> 
>> >> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
>> >> > for which the calibration data variant may need to be described.
>> >> >
>> >> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> >> > ---
>> >> >  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
>> >> >  1 file changed, 56 insertions(+)
>> >> >  create mode 100644
>> >> > Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
>> >> 
>> >> I'm confused (as usual), how does this differ from
>> >> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files?
>> >
>> > Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856
>> > when using PCIe (e.g. as most properties are then discoverable).
>> >
>> > We could try to encode everything in one file, but that would likely
>> > just result in a big mess of a schema with conditionals all over.
>> 
>> Ah, so the current qcom,ath11k.yaml would be only for ath11k AHB devices
>> and this new file is only for ath11k PCI devices?
>
> Right, there would two separate schema files for the two device classes.
>
>> But why still the odd
>> name pci17cb,1103.yaml? It's not really descriptive and I'm for sure
>> will not remember that pci17cb,1103.yaml is for ath11k :)
>
> Yeah, it's not the best name from that perspective, but it follows the
> current convention of naming the schema files after the first compatible
> added.
>
> That said, we don't have many schemas for PCI devices so perhaps we can
> establish a new convention for those. Perhaps by replacing the numerical
> ids with what we'd use if these were platform devices (e.g.
> 'qcom,wcn6855.yaml').
>
> As long as the DT maintainers are OK with it, I'd also be happy with
> something like you suggest below:
>
> 	qcom,ath11k-ahb.yaml
> 	qcom,ath11k-pci.yaml
>
> (or simply not renaming the current file 'qcom,ath11k.yaml') but I have
> gotten push back on that in the past.

Ok, maybe it's then better not to try renaming qcom,ath11k.yaml and keep
it as is.

>> Also it doesn't look good that we have qcom,ath11k-calibration-variant
>> documented twice now. I'm no DT expert but isn't there any other way? Is
>> it possible to include other files? For example, if we would have three
>> files:
>> 
>> qcom,ath11k.yaml
>> qcom,ath11k-ahb.yaml
>> qcom,ath11k-pci.yaml
>> 
>> Then have the common properties like ath11k-calibration-variant in the
>> first file and ahb/pci files would include that.
>
> That should be possible, but it's not necessarily better as you'd then
> have to look up two files to see the bindings for either device class
> (and as far as I can tell there would not be much sharing beyond this
> single property).
>
> Note that the property could just have well have been named
> 'qcom,calibration-variant' and then it would be shared also with the
> ath10k set of devices which currently holds another definition of what
> is essentially the same property ('qcom,ath10k-calibration-variant').

Oh man, having it as 'qcom,calibration-variant' would have been so much
better. Oh well, too late now :(

Thanks for explaining all this.

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

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

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings
@ 2023-03-22  5:59             ` Kalle Valo
  0 siblings, 0 replies; 24+ messages in thread
From: Kalle Valo @ 2023-03-22  5:59 UTC (permalink / raw)
  To: Johan Hovold
  Cc: Johan Hovold, Bjorn Andersson, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Rob Herring, Krzysztof Kozlowski,
	Andy Gross, Konrad Dybcio, linux-wireless, netdev, devicetree,
	linux-arm-msm, linux-kernel, ath11k

Johan Hovold <johan@kernel.org> writes:

> On Mon, Mar 20, 2023 at 08:41:21PM +0200, Kalle Valo wrote:
>
>> + ath11k list
>> 
>> Johan Hovold <johan@kernel.org> writes:
>> 
>> > On Mon, Mar 20, 2023 at 02:22:12PM +0200, Kalle Valo wrote:
>> >> Johan Hovold <johan+linaro@kernel.org> writes:
>> >> 
>> >> > Add devicetree bindings for Qualcomm ath11k PCIe devices such as WCN6856
>> >> > for which the calibration data variant may need to be described.
>> >> >
>> >> > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
>> >> > ---
>> >> >  .../bindings/net/wireless/pci17cb,1103.yaml   | 56 +++++++++++++++++++
>> >> >  1 file changed, 56 insertions(+)
>> >> >  create mode 100644
>> >> > Documentation/devicetree/bindings/net/wireless/pci17cb,1103.yaml
>> >> 
>> >> I'm confused (as usual), how does this differ from
>> >> bindings/net/wireless/qcom,ath11k.yaml? Why we need two .yaml files?
>> >
>> > Almost none of bindings/net/wireless/qcom,ath11k.yaml applies to WCN6856
>> > when using PCIe (e.g. as most properties are then discoverable).
>> >
>> > We could try to encode everything in one file, but that would likely
>> > just result in a big mess of a schema with conditionals all over.
>> 
>> Ah, so the current qcom,ath11k.yaml would be only for ath11k AHB devices
>> and this new file is only for ath11k PCI devices?
>
> Right, there would two separate schema files for the two device classes.
>
>> But why still the odd
>> name pci17cb,1103.yaml? It's not really descriptive and I'm for sure
>> will not remember that pci17cb,1103.yaml is for ath11k :)
>
> Yeah, it's not the best name from that perspective, but it follows the
> current convention of naming the schema files after the first compatible
> added.
>
> That said, we don't have many schemas for PCI devices so perhaps we can
> establish a new convention for those. Perhaps by replacing the numerical
> ids with what we'd use if these were platform devices (e.g.
> 'qcom,wcn6855.yaml').
>
> As long as the DT maintainers are OK with it, I'd also be happy with
> something like you suggest below:
>
> 	qcom,ath11k-ahb.yaml
> 	qcom,ath11k-pci.yaml
>
> (or simply not renaming the current file 'qcom,ath11k.yaml') but I have
> gotten push back on that in the past.

Ok, maybe it's then better not to try renaming qcom,ath11k.yaml and keep
it as is.

>> Also it doesn't look good that we have qcom,ath11k-calibration-variant
>> documented twice now. I'm no DT expert but isn't there any other way? Is
>> it possible to include other files? For example, if we would have three
>> files:
>> 
>> qcom,ath11k.yaml
>> qcom,ath11k-ahb.yaml
>> qcom,ath11k-pci.yaml
>> 
>> Then have the common properties like ath11k-calibration-variant in the
>> first file and ahb/pci files would include that.
>
> That should be possible, but it's not necessarily better as you'd then
> have to look up two files to see the bindings for either device class
> (and as far as I can tell there would not be much sharing beyond this
> single property).
>
> Note that the property could just have well have been named
> 'qcom,calibration-variant' and then it would be shared also with the
> ath10k set of devices which currently holds another definition of what
> is essentially the same property ('qcom,ath10k-calibration-variant').

Oh man, having it as 'qcom,calibration-variant' would have been so much
better. Oh well, too late now :(

Thanks for explaining all this.

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

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

-- 
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2023-03-22  6:00 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-20 10:46 [PATCH 0/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
2023-03-20 10:46 ` [PATCH 1/3] dt-bindings: wireless: add ath11k pcie bindings Johan Hovold
2023-03-20 12:22   ` Kalle Valo
2023-03-20 12:42     ` Johan Hovold
2023-03-20 18:41       ` Kalle Valo
2023-03-20 18:41         ` Kalle Valo
2023-03-21  8:22         ` Johan Hovold
2023-03-21  8:22           ` Johan Hovold
2023-03-22  5:59           ` Kalle Valo
2023-03-22  5:59             ` Kalle Valo
2023-03-21  8:14   ` Krzysztof Kozlowski
2023-03-21  8:16     ` Krzysztof Kozlowski
2023-03-21  8:27     ` Johan Hovold
2023-03-21  8:29       ` Krzysztof Kozlowski
2023-03-20 10:46 ` [PATCH 2/3] arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant Johan Hovold
2023-03-20 10:59   ` Konrad Dybcio
2023-03-20 12:15   ` Steev Klimaszewski
2023-03-20 10:46 ` [PATCH 3/3] arm64: dts: qcom: sc8280xp-crd: " Johan Hovold
2023-03-20 10:50   ` Konrad Dybcio
2023-03-20 10:55     ` Johan Hovold
2023-03-20 10:59       ` Konrad Dybcio
2023-03-20 11:07       ` Johan Hovold
2023-03-20 12:18         ` Kalle Valo
2023-03-20 12:34           ` Johan Hovold

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.