Received-SPF: pass (google.com: domain of linux-wireless+bounces-4380-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <4df78b7e-92dc-45e4-9d5f-9ec8793c4fd3@freebox.fr>
Date: Tue, 5 Mar 2024 14:41:08 +0100
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/2] dt-bindings: net: wireless: ath10k: add
 qcom,no-msa-ready-indicator prop
Content-Language: en-US
To: Conor Dooley <conor@kernel.org>
Cc: Kalle Valo <kvalo@kernel.org>, Jeff Johnson <quic_jjohnson@quicinc.com>,
 ath10k <ath10k@lists.infradead.org>,
 wireless <linux-wireless@vger.kernel.org>, DT <devicetree@vger.kernel.org>,
 Rob Herring <robh+dt@kernel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
 Conor Dooley <conor+dt@kernel.org>,
 Pierre-Hugues Husson <phhusson@freebox.fr>,
 Jami Kettunen <jamipkettunen@gmail.com>,
 Jeffrey Hugo <quic_jhugo@quicinc.com>,
 Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
References: <14daa98e-7fd3-4ebb-87bb-5d2c1fba679f@freebox.fr>
 <b8de96c7-cbb6-4a09-a4d4-2c11b3ab3e01@freebox.fr> <871q8wk7o3.fsf@kernel.org>
 <3392f356-7b19-483d-b9f8-3bd84068fa52@freebox.fr> <87wmqoilzf.fsf@kernel.org>
 <20240229-ageless-primal-7a0544420949@spud>
 <68a49964-7c05-4575-a4f3-35848c08fefc@freebox.fr>
 <20240304-component-animator-e2ee0ab7574a@spud>
From: Marc Gonzalez <mgonzalez@freebox.fr>
In-Reply-To: <20240304-component-animator-e2ee0ab7574a@spud>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit

On 04/03/2024 20:34, Conor Dooley wrote:

> On Mon, Mar 04, 2024 at 05:21:37PM +0100, Marc Gonzalez wrote:
>
>> On 29/02/2024 19:40, Conor Dooley wrote:
>>
>>> On Wed, Feb 28, 2024 at 06:37:08PM +0200, Kalle Valo wrote:
>>>
>>>> Marc Gonzalez wrote:
>>>>
>>>>> As mentioned in my other reply, there are several msm8998-based
>>>>> devices affected by this issue. Is it not appropriate to consider
>>>>> a kernel-based work-around?
>>>>
>>>> Sorry, not following you here. But I'll try to answer anyway:
>>>>
>>>> I have understood that Device Tree is supposed to describe hardware, not
>>>> software. This is why having this property in DT does not look right
>>>> place for this. For example, if the ath10k firmware is fixed then DT
>>>> would have to be changed even though nothing changed in hardware. But of
>>>> course DT maintainers have the final say.
>>>
>>> I dunno, if the firmware affects the functionality of the hardware in a
>>> way that cannot be detected from the operating system at runtime how
>>> else is it supposed to deal with that?
>>> The devicetree is supposed to describe hardware, yes, but at a certain
>>> point the line between firmware and hardware is invisible :)
>>> Not describing software is mostly about not using it to determine
>>> software policy in the operating system.
>>
>> Recording here what was discussed a few days ago on IRC:
>>
>> If all msm8998 boards are affected, then it /might/ make sense
>> to work around the issue for ALL msm8998 boards:
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c
>> index 0776e79b25f3a..9da06da518fb6 100644
>> --- a/drivers/net/wireless/ath/ath10k/qmi.c
>> +++ b/drivers/net/wireless/ath/ath10k/qmi.c
>> @@ -1076,6 +1076,9 @@ int ath10k_qmi_init(struct ath10k *ar, u32 msa_size)
>>  	qmi->ar = ar;
>>  	ar_snoc->qmi = qmi;
>>  
>> +	if (of_device_is_compatible(of_root, "qcom,msm8998")
>> +		qmi->no_point_in_waiting_for_msa_ready_indicator = true;
>> +
>>  	if (of_property_read_bool(dev->of_node, "qcom,msa-fixed-perm"))
>>  		qmi->msa_fixed_perm = true;
>>  
>>
>> Thus, anyone porting an msm8998 board to mainline would automatically
>> get the work-around, without having to hunt down the feature bit,
>> and tweak the FW files.
> 
> How come the root node comes into this, don't you have a soc-specific
> compatible for the integration on this SoC?
> (I am assuming that this is not the SDIO variant, given then it'd not be
> fixed to this particular implementation)

I see only
"qcom,ipq4019-wifi"
"qcom,wcn3990-wifi"
"qcom,wcn6750-wifi"

arch/arm64/boot/dts/qcom/msm8998.dtsi uses "qcom,wcn3990-wifi"
(which is also used for 9 other SoCs).

IIUC, you're saying there probably should have been a generic
compatible AND a board-specific compatible, like for ufshc:

$ git grep qcom,ufshc arch/arm64/boot/dts/qcom
arch/arm64/boot/dts/qcom/msm8996.dtsi:                  compatible = "qcom,msm8996-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/msm8998.dtsi:                  compatible = "qcom,msm8998-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
arch/arm64/boot/dts/qcom/sa8775p.dtsi:                  compatible = "qcom,sa8775p-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
arch/arm64/boot/dts/qcom/sc7280.dtsi:                   compatible = "qcom,sc7280-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sc8180x.dtsi:                  compatible = "qcom,sc8180x-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sc8280xp.dtsi:                 compatible = "qcom,sc8280xp-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sc8280xp.dtsi:                 compatible = "qcom,sc8280xp-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sdm845.dtsi:                   compatible = "qcom,sdm845-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sm6115.dtsi:                   compatible = "qcom,sm6115-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
arch/arm64/boot/dts/qcom/sm6125.dtsi:                   compatible = "qcom,sm6125-ufshc", "qcom,ufshc", "jedec,ufs-2.0";
arch/arm64/boot/dts/qcom/sm6350.dtsi:                   compatible = "qcom,sm6350-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sm8150.dtsi:                   compatible = "qcom,sm8150-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sm8250.dtsi:                   compatible = "qcom,sm8250-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sm8350.dtsi:                   compatible = "qcom,sm8350-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sm8450.dtsi:                   compatible = "qcom,sm8450-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sm8550.dtsi:                   compatible = "qcom,sm8550-ufshc", "qcom,ufshc",
arch/arm64/boot/dts/qcom/sm8650.dtsi:                   compatible = "qcom,sm8650-ufshc", "qcom,ufshc", "jedec,ufs-2.0";


If all msm8998-based device trees had (for example)
a qcom,msm8998-wcn3990-wifi compatible, we could have
matched that to apply a quirk to all msm8998 boards.

Did I understand correctly?

(In the toy code I provided, I matched the of_root because
there is no SoC-specific compatible for the device itself.)

-- 
Regards