| Message ID | ebbda69c-63c1-4003-bf97-c3adf3ccb9e3@freebox.fr |
|---|---|
| Headers | show |
| Series | Work around missing MSA_READY indicator for msm8998 devices | expand |
On 29/04/2024 15:01, Marc Gonzalez wrote: > Work around missing MSA_READY indicator in ath10k driver > (apply work-around for all msm8998 devices) > > CHANGELOG v3 > - Add a paragraph in binding commit to explain why we use > a DT property instead of a firmware feature bit. > - Warn if the "no_msa_ready_indicator" property is true, > but we actually receive the indicator. > > Marc Gonzalez (3): > dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop > wifi: ath10k: do not always wait for MSA_READY indicator > arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi > > Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml | 5 +++++ > arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 + > drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++ > drivers/net/wireless/ath/ath10k/qmi.h | 1 + > 4 files changed, 18 insertions(+) > I wonder if you could infer the workaround based on firmware version, instead of kernel passed flag ? --- bod
On Tue, Apr 30, 2024 at 12:24:40AM +0100, Bryan O'Donoghue wrote: > On 29/04/2024 15:01, Marc Gonzalez wrote: > > Work around missing MSA_READY indicator in ath10k driver > > (apply work-around for all msm8998 devices) > > > > CHANGELOG v3 > > - Add a paragraph in binding commit to explain why we use > > a DT property instead of a firmware feature bit. > > - Warn if the "no_msa_ready_indicator" property is true, > > but we actually receive the indicator. > > > > Marc Gonzalez (3): > > dt-bindings: net: wireless: ath10k: add qcom,no-msa-ready-indicator prop > > wifi: ath10k: do not always wait for MSA_READY indicator > > arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi > > > > Documentation/devicetree/bindings/net/wireless/qcom,ath10k.yaml | 5 +++++ > > arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 + > > drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++ > > drivers/net/wireless/ath/ath10k/qmi.h | 1 + > > 4 files changed, 18 insertions(+) > > > > I wonder if you could infer the workaround based on firmware version, > instead of kernel passed flag ? > It's been a while, but I attempted this for the similar workaround for SDM845 RB3 et al. I vaguely remember concluding that the different devices I worked on, had firmware from different branches without any suitable version scheme to use for such comparison - which is why we have "qcom,snoc-host-cap-8bit-quirk;" in those nodes (which apparently is not documented in the yaml). I'd also rather see us ask Marc spending time on some more fruitful exercise... Regards, Bjorn
On Mon, Apr 29, 2024 at 04:06:29PM +0200, Marc Gonzalez wrote: > The ath10k driver waits for an "MSA_READY" indicator > to complete initialization. If the indicator is not > received, then the device remains unusable. > > Several msm8998-based devices are affected by this issue. > Oddly, it seems safe to NOT wait for the indicator, and > proceed immediately when QMI_EVENT_SERVER_ARRIVE. > > fw_version 0x100204b2 > fw_build_timestamp 2019-09-04 03:01 > fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HL.1.0-01202-QCAHLSWMTPLZ-1.221523.2 > > Jeff Johnson wrote: > > The feedback I received was "it might be ok to change all ath10k qmi > to skip waiting for msa_ready", and it was pointed out that ath11k > (and ath12k) do not wait for it. > > However with so many deployed devices, "might be ok" isn't a strong > argument for changing the default behavior. > > Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> As with patch 1, please address the s-o-b and accept my: Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> Regards, Bjorn > --- > drivers/net/wireless/ath/ath10k/qmi.c | 11 +++++++++++ > drivers/net/wireless/ath/ath10k/qmi.h | 1 + > 2 files changed, 12 insertions(+) > > diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c > index 38e939f572a9e..f1f33af0170a0 100644 > --- a/drivers/net/wireless/ath/ath10k/qmi.c > +++ b/drivers/net/wireless/ath/ath10k/qmi.c > @@ -1040,6 +1040,10 @@ static void ath10k_qmi_driver_event_work(struct work_struct *work) > switch (event->type) { > case ATH10K_QMI_EVENT_SERVER_ARRIVE: > ath10k_qmi_event_server_arrive(qmi); > + if (qmi->no_msa_ready_indicator) { > + ath10k_info(ar, "qmi not waiting for msa_ready indicator"); > + ath10k_qmi_event_msa_ready(qmi); > + } > break; > case ATH10K_QMI_EVENT_SERVER_EXIT: > ath10k_qmi_event_server_exit(qmi); > @@ -1048,6 +1052,10 @@ static void ath10k_qmi_driver_event_work(struct work_struct *work) > ath10k_qmi_event_fw_ready_ind(qmi); > break; > case ATH10K_QMI_EVENT_MSA_READY_IND: > + if (qmi->no_msa_ready_indicator) { > + ath10k_warn(ar, "qmi unexpected msa_ready indicator"); > + break; > + } > ath10k_qmi_event_msa_ready(qmi); > break; > default: > @@ -1077,6 +1085,9 @@ int ath10k_qmi_init(struct ath10k *ar, u32 msa_size) > if (of_property_read_bool(dev->of_node, "qcom,msa-fixed-perm")) > qmi->msa_fixed_perm = true; > > + if (of_property_read_bool(dev->of_node, "qcom,no-msa-ready-indicator")) > + qmi->no_msa_ready_indicator = true; > + > ret = qmi_handle_init(&qmi->qmi_hdl, > WLFW_BDF_DOWNLOAD_REQ_MSG_V01_MAX_MSG_LEN, > &ath10k_qmi_ops, qmi_msg_handler); > diff --git a/drivers/net/wireless/ath/ath10k/qmi.h b/drivers/net/wireless/ath/ath10k/qmi.h > index 89464239fe96a..0816eb4e4a18a 100644 > --- a/drivers/net/wireless/ath/ath10k/qmi.h > +++ b/drivers/net/wireless/ath/ath10k/qmi.h > @@ -107,6 +107,7 @@ struct ath10k_qmi { > char fw_build_timestamp[MAX_TIMESTAMP_LEN + 1]; > struct ath10k_qmi_cal_data cal_data[MAX_NUM_CAL_V01]; > bool msa_fixed_perm; > + bool no_msa_ready_indicator; > enum ath10k_qmi_state state; > }; > > -- > 2.34.1 > >
On 30/04/2024 04:24, Bjorn Andersson wrote: > On Mon, Apr 29, 2024 at 04:06:29PM +0200, Marc Gonzalez wrote: > >> The ath10k driver waits for an "MSA_READY" indicator >> to complete initialization. If the indicator is not >> received, then the device remains unusable. >> >> Several msm8998-based devices are affected by this issue. >> Oddly, it seems safe to NOT wait for the indicator, and >> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >> >> fw_version 0x100204b2 >> fw_build_timestamp 2019-09-04 03:01 >> fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HL.1.0-01202-QCAHLSWMTPLZ-1.221523.2 >> >> Jeff Johnson wrote: >> >> The feedback I received was "it might be ok to change all ath10k qmi >> to skip waiting for msa_ready", and it was pointed out that ath11k >> (and ath12k) do not wait for it. >> >> However with so many deployed devices, "might be ok" isn't a strong >> argument for changing the default behavior. >> >> Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> >> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > > As with patch 1, please address the s-o-b and accept my: > > Reviewed-by: Bjorn Andersson <quic_bjorande@quicinc.com> As with patch 1, I typed this patch all by myself with my grubby little paws. You can drop PH's S-o-b. Regards
On 4/29/2024 7:06 AM, Marc Gonzalez wrote: > The ath10k driver waits for an "MSA_READY" indicator > to complete initialization. If the indicator is not > received, then the device remains unusable. > > Several msm8998-based devices are affected by this issue. > Oddly, it seems safe to NOT wait for the indicator, and > proceed immediately when QMI_EVENT_SERVER_ARRIVE. > > fw_version 0x100204b2 > fw_build_timestamp 2019-09-04 03:01 > fw_build_id QC_IMAGE_VERSION_STRING=WLAN.HL.1.0-01202-QCAHLSWMTPLZ-1.221523.2 > > Jeff Johnson wrote: > > The feedback I received was "it might be ok to change all ath10k qmi > to skip waiting for msa_ready", and it was pointed out that ath11k > (and ath12k) do not wait for it. > > However with so many deployed devices, "might be ok" isn't a strong > argument for changing the default behavior. > > Signed-off-by: Pierre-Hugues Husson <phhusson@freebox.fr> > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > --- with SOB cleaned up: Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
On 4/29/2024 7:07 AM, Marc Gonzalez wrote: > The ath10k driver waits for an "MSA_READY" indicator > to complete initialization. If the indicator is not > received, then the device remains unusable. > > cf. ath10k_qmi_driver_event_work() > > Several msm8998-based devices are affected by this issue. > Oddly, it seems safe to NOT wait for the indicator, and > proceed immediately when QMI_EVENT_SERVER_ARRIVE. > > Jeff Johnson wrote: > > The feedback I received was "it might be ok to change all ath10k qmi > to skip waiting for msa_ready", and it was pointed out that ath11k > (and ath12k) do not wait for it. > > However with so many deployed devices, "might be ok" isn't a strong > argument for changing the default behavior. > > cf. also > https://wiki.postmarketos.org/wiki/Qualcomm_Snapdragon_835_(MSM8998)#WLAN > > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> Acked-by: Jeff Johnson <quic_jjohnson@quicinc.com>
On 29/04/2024 16:07, Marc Gonzalez wrote: > The ath10k driver waits for an "MSA_READY" indicator > to complete initialization. If the indicator is not > received, then the device remains unusable. > > cf. ath10k_qmi_driver_event_work() > > Several msm8998-based devices are affected by this issue. > Oddly, it seems safe to NOT wait for the indicator, and > proceed immediately when QMI_EVENT_SERVER_ARRIVE. > > Jeff Johnson wrote: > > The feedback I received was "it might be ok to change all ath10k qmi > to skip waiting for msa_ready", and it was pointed out that ath11k > (and ath12k) do not wait for it. > > However with so many deployed devices, "might be ok" isn't a strong > argument for changing the default behavior. > > cf. also > https://wiki.postmarketos.org/wiki/Qualcomm_Snapdragon_835_(MSM8998)#WLAN > > Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> > --- > arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi > index 67b8374ddf02f..4e6245095adfc 100644 > --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi > +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi > @@ -3234,6 +3234,7 @@ wifi: wifi@18800000 { > iommus = <&anoc2_smmu 0x1900>, > <&anoc2_smmu 0x1901>; > qcom,snoc-host-cap-8bit-quirk; > + qcom,no-msa-ready-indicator; > }; > }; > }; Bjorn, This patch is supposed to go through your tree, right? Regards
On 5/6/24 12:39, Marc Gonzalez wrote: > On 29/04/2024 16:07, Marc Gonzalez wrote: > >> The ath10k driver waits for an "MSA_READY" indicator >> to complete initialization. If the indicator is not >> received, then the device remains unusable. >> >> cf. ath10k_qmi_driver_event_work() >> >> Several msm8998-based devices are affected by this issue. >> Oddly, it seems safe to NOT wait for the indicator, and >> proceed immediately when QMI_EVENT_SERVER_ARRIVE. >> >> Jeff Johnson wrote: >> >> The feedback I received was "it might be ok to change all ath10k qmi >> to skip waiting for msa_ready", and it was pointed out that ath11k >> (and ath12k) do not wait for it. >> >> However with so many deployed devices, "might be ok" isn't a strong >> argument for changing the default behavior. >> >> cf. also >> https://wiki.postmarketos.org/wiki/Qualcomm_Snapdragon_835_(MSM8998)#WLAN >> >> Signed-off-by: Marc Gonzalez <mgonzalez@freebox.fr> >> --- >> arch/arm64/boot/dts/qcom/msm8998.dtsi | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/arm64/boot/dts/qcom/msm8998.dtsi b/arch/arm64/boot/dts/qcom/msm8998.dtsi >> index 67b8374ddf02f..4e6245095adfc 100644 >> --- a/arch/arm64/boot/dts/qcom/msm8998.dtsi >> +++ b/arch/arm64/boot/dts/qcom/msm8998.dtsi >> @@ -3234,6 +3234,7 @@ wifi: wifi@18800000 { >> iommus = <&anoc2_smmu 0x1900>, >> <&anoc2_smmu 0x1901>; >> qcom,snoc-host-cap-8bit-quirk; >> + qcom,no-msa-ready-indicator; >> }; >> }; >> }; > > > Bjorn, > > This patch is supposed to go through your tree, right? Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org> Konrad
On Mon, 29 Apr 2024 16:01:41 +0200, Marc Gonzalez wrote: > Work around missing MSA_READY indicator in ath10k driver > (apply work-around for all msm8998 devices) > > CHANGELOG v3 > - Add a paragraph in binding commit to explain why we use > a DT property instead of a firmware feature bit. > - Warn if the "no_msa_ready_indicator" property is true, > but we actually receive the indicator. > > [...] Applied, thanks! [3/3] arm64: dts: qcom: msm8998: set qcom,no-msa-ready-indicator for wifi commit: 737abcabe97bb37e38be2504acd28ad779dbaf3d Best regards,