[PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Krzysztof Kozlowski]

There is no such property as qcom,ath12k-calibration-variant: neither in
the bindings nor in the driver.  See dtbs_check:

  x1e80100-lenovo-yoga-slim7x.dtb: wifi@0: 'qcom,ath12k-calibration-variant' does not match any of the regexes: 'pinctrl-[0-9]+'

Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
---
 arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
index a3d53f2ba2c3..9aff5a1f044d 100644
--- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
+++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
@@ -674,8 +674,6 @@ &pcie4_port0 {
 	wifi@0 {
 		compatible = "pci17cb,1107";
 		reg = <0x10000 0x0 0x0 0x0 0x0>;
-
-		qcom,ath12k-calibration-variant = "LES790";
 	};
 };
 
-- 
2.43.0

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Dmitry Baryshkov]

On Tue, Feb 25, 2025 at 10:30:51AM +0100, Krzysztof Kozlowski wrote:
> There is no such property as qcom,ath12k-calibration-variant: neither in
> the bindings nor in the driver.  See dtbs_check:
> 
>   x1e80100-lenovo-yoga-slim7x.dtb: wifi@0: 'qcom,ath12k-calibration-variant' does not match any of the regexes: 'pinctrl-[0-9]+'
> 

Adding Jeff and ath12k@ to the cc list. Is the driver able to find the
calibration variant in case it is not running on the ACPI system? I see
that it uses dmi_walk. Does it work in the non-ACPI case?

> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> index a3d53f2ba2c3..9aff5a1f044d 100644
> --- a/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> +++ b/arch/arm64/boot/dts/qcom/x1e80100-lenovo-yoga-slim7x.dts
> @@ -674,8 +674,6 @@ &pcie4_port0 {
>  	wifi@0 {
>  		compatible = "pci17cb,1107";
>  		reg = <0x10000 0x0 0x0 0x0 0x0>;
> -
> -		qcom,ath12k-calibration-variant = "LES790";
>  	};
>  };
>  
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Krzysztof Kozlowski]

On 25/02/2025 10:50, Dmitry Baryshkov wrote:
> On Tue, Feb 25, 2025 at 10:30:51AM +0100, Krzysztof Kozlowski wrote:
>> There is no such property as qcom,ath12k-calibration-variant: neither in
>> the bindings nor in the driver.  See dtbs_check:
>>
>>   x1e80100-lenovo-yoga-slim7x.dtb: wifi@0: 'qcom,ath12k-calibration-variant' does not match any of the regexes: 'pinctrl-[0-9]+'
>>
> 
> Adding Jeff and ath12k@ to the cc list. Is the driver able to find the
> calibration variant in case it is not running on the ACPI system? I see
> that it uses dmi_walk. Does it work in the non-ACPI case?


But nothing parses such string as 'qcom,ath12k-calibration-variant' (see
git grep), so how would driver use it?


Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Dmitry Baryshkov]

On Tue, Feb 25, 2025 at 11:22:25AM +0100, Krzysztof Kozlowski wrote:
> On 25/02/2025 10:50, Dmitry Baryshkov wrote:
> > On Tue, Feb 25, 2025 at 10:30:51AM +0100, Krzysztof Kozlowski wrote:
> >> There is no such property as qcom,ath12k-calibration-variant: neither in
> >> the bindings nor in the driver.  See dtbs_check:
> >>
> >>   x1e80100-lenovo-yoga-slim7x.dtb: wifi@0: 'qcom,ath12k-calibration-variant' does not match any of the regexes: 'pinctrl-[0-9]+'
> >>
> > 
> > Adding Jeff and ath12k@ to the cc list. Is the driver able to find the
> > calibration variant in case it is not running on the ACPI system? I see
> > that it uses dmi_walk. Does it work in the non-ACPI case?
> 
> 
> But nothing parses such string as 'qcom,ath12k-calibration-variant' (see
> git grep), so how would driver use it?

That's what I'm asking: is the property redundant or is it correct and
it is a driver that needs to be fixed?

-- 
With best wishes
Dmitry

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Krzysztof Kozlowski]

On 25/02/2025 12:45, Dmitry Baryshkov wrote:
> On Tue, Feb 25, 2025 at 11:22:25AM +0100, Krzysztof Kozlowski wrote:
>> On 25/02/2025 10:50, Dmitry Baryshkov wrote:
>>> On Tue, Feb 25, 2025 at 10:30:51AM +0100, Krzysztof Kozlowski wrote:
>>>> There is no such property as qcom,ath12k-calibration-variant: neither in
>>>> the bindings nor in the driver.  See dtbs_check:
>>>>
>>>>   x1e80100-lenovo-yoga-slim7x.dtb: wifi@0: 'qcom,ath12k-calibration-variant' does not match any of the regexes: 'pinctrl-[0-9]+'
>>>>
>>>
>>> Adding Jeff and ath12k@ to the cc list. Is the driver able to find the
>>> calibration variant in case it is not running on the ACPI system? I see
>>> that it uses dmi_walk. Does it work in the non-ACPI case?
>>
>>
>> But nothing parses such string as 'qcom,ath12k-calibration-variant' (see
>> git grep), so how would driver use it?
> 
> That's what I'm asking: is the property redundant or is it correct and
> it is a driver that needs to be fixed?

I assume driver will need something like that property, but that's not a
reason to accept incorrect one in DTS. One cannot add properties to DTS
without bindings, so bypassing bindings review, and then claim "but my
driver needs them". Send proper patches for driver first which will get
a review.

This could be instead renamed to final correct property, but since there
is no user, no indication it is needed or correct, I cannot prepare such
patch. I would not know what to write (e.g. "rename to qcom,foo-bar,
because I have no clue").

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Jeff Johnson]

On 2/25/2025 4:14 AM, Krzysztof Kozlowski wrote:
> On 25/02/2025 12:45, Dmitry Baryshkov wrote:
>> On Tue, Feb 25, 2025 at 11:22:25AM +0100, Krzysztof Kozlowski wrote:
>>> On 25/02/2025 10:50, Dmitry Baryshkov wrote:
>>>> On Tue, Feb 25, 2025 at 10:30:51AM +0100, Krzysztof Kozlowski wrote:
>>>>> There is no such property as qcom,ath12k-calibration-variant: neither in
>>>>> the bindings nor in the driver.  See dtbs_check:
>>>>>
>>>>>   x1e80100-lenovo-yoga-slim7x.dtb: wifi@0: 'qcom,ath12k-calibration-variant' does not match any of the regexes: 'pinctrl-[0-9]+'
>>>>>
>>>>
>>>> Adding Jeff and ath12k@ to the cc list. Is the driver able to find the
>>>> calibration variant in case it is not running on the ACPI system? I see
>>>> that it uses dmi_walk. Does it work in the non-ACPI case?
>>>
>>>
>>> But nothing parses such string as 'qcom,ath12k-calibration-variant' (see
>>> git grep), so how would driver use it?
>>
>> That's what I'm asking: is the property redundant or is it correct and
>> it is a driver that needs to be fixed?
> 
> I assume driver will need something like that property, but that's not a
> reason to accept incorrect one in DTS. One cannot add properties to DTS
> without bindings, so bypassing bindings review, and then claim "but my
> driver needs them". Send proper patches for driver first which will get
> a review.

We definitely need a calibration variant entry.
I've pinged the development team to get the driver patch.

I'm also verifying internally that there are no issues with your renaming
proposal: qcom,ath1*k-calibration-variant => qcom,calibration-variant
https://msgid.link/20250225-b-wifi-qcom-calibration-variant-v1-0-3b2aa3f89c53@linaro.org

/jeff

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Krzysztof Kozlowski]

On 25/02/2025 17:44, Jeff Johnson wrote:
>>>>
>>>> But nothing parses such string as 'qcom,ath12k-calibration-variant' (see
>>>> git grep), so how would driver use it?
>>>
>>> That's what I'm asking: is the property redundant or is it correct and
>>> it is a driver that needs to be fixed?
>>
>> I assume driver will need something like that property, but that's not a
>> reason to accept incorrect one in DTS. One cannot add properties to DTS
>> without bindings, so bypassing bindings review, and then claim "but my
>> driver needs them". Send proper patches for driver first which will get
>> a review.
> 
> We definitely need a calibration variant entry.
> I've pinged the development team to get the driver patch.


The patches were on the lists but were not accepted. Therefore DTS
property cannot get into the kernel. I am sorry, but this is not somehow
fluid or flexible that internal team can squeeze something into the kernel.

Also post factum reasoning is not correct, because this would open the
gate to bypass any sort of review. Just squeeze your stuff into the DTS
and then you can bypass all DT maintainers :/

All properties must be documented and bindings must be accepted *before*
DTS patch is applied.

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Jeff Johnson]

On 2/25/2025 9:07 AM, Krzysztof Kozlowski wrote:
> On 25/02/2025 17:44, Jeff Johnson wrote:
>>>>>
>>>>> But nothing parses such string as 'qcom,ath12k-calibration-variant' (see
>>>>> git grep), so how would driver use it?
>>>>
>>>> That's what I'm asking: is the property redundant or is it correct and
>>>> it is a driver that needs to be fixed?
>>>
>>> I assume driver will need something like that property, but that's not a
>>> reason to accept incorrect one in DTS. One cannot add properties to DTS
>>> without bindings, so bypassing bindings review, and then claim "but my
>>> driver needs them". Send proper patches for driver first which will get
>>> a review.
>>
>> We definitely need a calibration variant entry.
>> I've pinged the development team to get the driver patch.
> 
> 
> The patches were on the lists but were not accepted. Therefore DTS
> property cannot get into the kernel. I am sorry, but this is not somehow
> fluid or flexible that internal team can squeeze something into the kernel.

I see bindings and DTS patches but no driver patch, even in my internal queue.

> 
> Also post factum reasoning is not correct, because this would open the
> gate to bypass any sort of review. Just squeeze your stuff into the DTS
> and then you can bypass all DT maintainers :/
> 
> All properties must be documented and bindings must be accepted *before*
> DTS patch is applied.

There is no intention to bypass DT maintainers. We are just trying to upstream
a large amount of downstream code, and in the process some pieces are coming
out of order. And there is also confusion if binding, driver, and DTS changes
should be in one series or three separate series.

We are moving towards an upstream-first model, but we still have to address
the existing technical debt.

/jeff

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Krzysztof Kozlowski]

On 25/02/2025 18:36, Jeff Johnson wrote:
> 
>>
>> Also post factum reasoning is not correct, because this would open the
>> gate to bypass any sort of review. Just squeeze your stuff into the DTS
>> and then you can bypass all DT maintainers :/
>>
>> All properties must be documented and bindings must be accepted *before*
>> DTS patch is applied.
> 
> There is no intention to bypass DT maintainers. We are just trying to upstream
> a large amount of downstream code, and in the process some pieces are coming

I don't see how this is related here - patch was not sent by anyone from
Qualcomm.

> out of order. And there is also confusion if binding, driver, and DTS changes
> should be in one series or three separate series.

How is it related to incorrect property here? It feels like this topic
is being hijacked for some other point. I am not happy with this because
then Bjorn will see that discussion is going so he will ignore the patch.

BTW, I gave my statement multiple times, writing bindings also mention
this, so is anything going to change if I say it 100th time here? In one
month there will be the same question :/

DTS must be applied via ARM SoC, thus you cannot combine DTS into
patchsets being entirely applied by driver subsystem maintainers (Greg,
netdev, sometimes maybe watchdog). For other maintainers, you can
combine it, because they know to skip DTS.

Bindings always go via subsystem, so they must be part of driver
patchset, unless of course there is no driver (but then bindings are
"the driver" patchset).


Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Krzysztof Kozlowski]

On 25/02/2025 18:59, Krzysztof Kozlowski wrote:
> On 25/02/2025 18:36, Jeff Johnson wrote:
>>
>>>
>>> Also post factum reasoning is not correct, because this would open the
>>> gate to bypass any sort of review. Just squeeze your stuff into the DTS
>>> and then you can bypass all DT maintainers :/
>>>
>>> All properties must be documented and bindings must be accepted *before*
>>> DTS patch is applied.
>>
>> There is no intention to bypass DT maintainers. We are just trying to upstream
>> a large amount of downstream code, and in the process some pieces are coming
> 
> I don't see how this is related here - patch was not sent by anyone from
> Qualcomm.
> 
>> out of order. And there is also confusion if binding, driver, and DTS changes
>> should be in one series or three separate series.
> 
> How is it related to incorrect property here? It feels like this topic
> is being hijacked for some other point. I am not happy with this because
> then Bjorn will see that discussion is going so he will ignore the patch.
> 
> BTW, I gave my statement multiple times, writing bindings also mention
> this, so is anything going to change if I say it 100th time here? In one
> month there will be the same question :/


Another BTW, not helpful to community, but if you asked above for
Qualcomm, that Qualcomm does not know where to post DTS, then you are
lucky, because your extensive internal guideline has it already very
clearly documented (detailed in "Driver upstreaming process" and a bit
in "feedback/review on lists"). The guide is quite comprehensive and
covers all typical cases, like that one.

Best regards,
Krzysztof

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Dmitry Baryshkov]

On Tue, Feb 25, 2025 at 08:44:57AM -0800, Jeff Johnson wrote:
> On 2/25/2025 4:14 AM, Krzysztof Kozlowski wrote:
> > On 25/02/2025 12:45, Dmitry Baryshkov wrote:
> >> On Tue, Feb 25, 2025 at 11:22:25AM +0100, Krzysztof Kozlowski wrote:
> >>> On 25/02/2025 10:50, Dmitry Baryshkov wrote:
> >>>> On Tue, Feb 25, 2025 at 10:30:51AM +0100, Krzysztof Kozlowski wrote:
> >>>>> There is no such property as qcom,ath12k-calibration-variant: neither in
> >>>>> the bindings nor in the driver.  See dtbs_check:
> >>>>>
> >>>>>   x1e80100-lenovo-yoga-slim7x.dtb: wifi@0: 'qcom,ath12k-calibration-variant' does not match any of the regexes: 'pinctrl-[0-9]+'
> >>>>>
> >>>>
> >>>> Adding Jeff and ath12k@ to the cc list. Is the driver able to find the
> >>>> calibration variant in case it is not running on the ACPI system? I see
> >>>> that it uses dmi_walk. Does it work in the non-ACPI case?
> >>>
> >>>
> >>> But nothing parses such string as 'qcom,ath12k-calibration-variant' (see
> >>> git grep), so how would driver use it?
> >>
> >> That's what I'm asking: is the property redundant or is it correct and
> >> it is a driver that needs to be fixed?
> > 
> > I assume driver will need something like that property, but that's not a
> > reason to accept incorrect one in DTS. One cannot add properties to DTS
> > without bindings, so bypassing bindings review, and then claim "but my
> > driver needs them". Send proper patches for driver first which will get
> > a review.
> 
> We definitely need a calibration variant entry.
> I've pinged the development team to get the driver patch.

I think we need a confirmation from sobody using Slim7x if the driver
can read info from DMI or if it can not. In the end, DMI != ACPI.

> 
> I'm also verifying internally that there are no issues with your renaming
> proposal: qcom,ath1*k-calibration-variant => qcom,calibration-variant
> https://msgid.link/20250225-b-wifi-qcom-calibration-variant-v1-0-3b2aa3f89c53@linaro.org
> 
> /jeff

-- 
With best wishes
Dmitry

Re: [PATCH] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant

[Bjorn Andersson]

On Tue, 25 Feb 2025 10:30:51 +0100, Krzysztof Kozlowski wrote:
> There is no such property as qcom,ath12k-calibration-variant: neither in
> the bindings nor in the driver.  See dtbs_check:
> 
>   x1e80100-lenovo-yoga-slim7x.dtb: wifi@0: 'qcom,ath12k-calibration-variant' does not match any of the regexes: 'pinctrl-[0-9]+'
> 
> 

Applied, thanks!

[1/1] arm64: dts: qcom: x1e80100-slim7x: Drop incorrect qcom,ath12k-calibration-variant
      commit: fb03174d17ec98e939dac81e29b372c9b568fca9

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>