diff mbox series

[3/3] arm64: dts: qcom: sc8280xp-crd: add wifi calibration variant

Message ID 20230320104658.22186-4-johan+linaro@kernel.org
State New
Headers
Series arm64: dts: qcom: sc8280xp-x13s: add wifi calibration variant |

Commit Message

Johan Hovold March 20, 2023, 10:46 a.m. UTC 
  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(+)

Comments

Konrad Dybcio March 20, 2023, 10:50 a.m. UTC | #1 
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 {
Johan Hovold March 20, 2023, 10:55 a.m. UTC | #2 
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
Konrad Dybcio March 20, 2023, 10:59 a.m. UTC | #3 
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
Johan Hovold March 20, 2023, 11:07 a.m. UTC | #4 
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
Kalle Valo March 20, 2023, 12:18 p.m. UTC | #5 
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.
Johan Hovold March 20, 2023, 12:34 p.m. UTC | #6 
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
diff mbox series

Patch

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 {