Re: [PATCH v2 3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi
On Thu, Apr 25, 2024 at 06:42:16PM +0300, Kalle Valo wrote: > Marc Gonzalez writes: > > > On 25/04/2024 11:42, Kalle Valo wrote: > > > >> Marc Gonzalez wrote: > >> > >>> Do you prefer: > >>> > >>> Option A = never waiting for the MSA_READY indicator for ANYONE > >>> Option B = not waiting for the MSA_READY indicator when > >>> qcom,no-msa-ready-indicator is defined > >>> Option C = not waiting for the MSA_READY indicator for certain > >>> platforms (based on root compatible) > >>> Option D = some other solution not yet discussed > >> > >> After firmware-N.bin solution didn't work (sorry about that!) my > >> preference is option B. > > > > Actually, Option B is this patch series. > > Could you formally review it? > > I'm happy with this series and would take it to ath.git, just need an > ack from DT maintainers: As far as I can tell, Krzysztof wanted a commit message update for 1/3. Usually this dts patch would go via the qcom dts tree, so presumably there's a need for an Ack from Bjorn or Konrad? > > https://patchwork.kernel.org/project/linux-wireless/patch/84f20fb5-5d48-419c-8eff-d7044afb8...@freebox.fr/ > > > Perhaps one thing I could do slightly differently is to NOT call > > ath10k_qmi_event_msa_ready() a second time if we DO receive the > > indicator later. > > Good point. And maybe add an ath10k_warn() message so that we notice if > there's a mismatch. > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches signature.asc Description: PGP signature
Re: [PATCH 1/2] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
On Mon, Mar 04, 2024 at 09:59:13PM +0200, Dmitry Baryshkov wrote: > On Mon, 4 Mar 2024 at 21:46, Conor Dooley wrote: > > On Mon, Mar 04, 2024 at 09:37:00PM +0200, Dmitry Baryshkov wrote: > > > On Mon, 4 Mar 2024 at 21:34, Conor Dooley wrote: > > > > On Mon, Mar 04, 2024 at 05:21:37PM +0100, Marc Gonzalez wrote: > > > > > 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? > > > > > > No. Ath10k uses WiFi SoC as an SoC designator rather than the main SoC. > > > > Suitability of either fix aside, can you explain this to me? Is the "WiFi > > SoC" accessible from the "main SoC" at a regular MMIO address? The > > "ath10k" compatible says it is SDIO-based & the other two compatibles > > seem to be MMIO. > > Yes, this is correct. MSM8996 uses PCI to access WiFi chip, MSM8998 uses MMIO. Thanks. A SoC-specific compatible sounds like it would be suitable in that case then, to deal with integration quirks for that specific SoC? I usually leave the ins and outs of these qcom SoCs to Krzysztof, but I can't help but wanna know what the justification is here for not using one. signature.asc Description: PGP signature
Re: [PATCH 1/2] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
On Mon, Mar 04, 2024 at 09:37:00PM +0200, Dmitry Baryshkov wrote: > On Mon, 4 Mar 2024 at 21: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? > > No. Ath10k uses WiFi SoC as an SoC designator rather than the main SoC. Suitability of either fix aside, can you explain this to me? Is the "WiFi SoC" accessible from the "main SoC" at a regular MMIO address? The "ath10k" compatible says it is SDIO-based & the other two compatibles seem to be MMIO. signature.asc Description: PGP signature
Re: [PATCH 1/2] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
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) signature.asc Description: PGP signature
Re: [PATCH 1/2] dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop
On Wed, Feb 28, 2024 at 06:37:08PM +0200, Kalle Valo wrote: > Marc Gonzalez writes: > > 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. signature.asc Description: PGP signature
