| Message ID | 1601058581-19461-1-git-send-email-amit.pundir@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | ath10k: Introduce a devicetree quirk to skip host cap QMI requests | expand |
On Fri, Sep 25, 2020 at 11:59:41PM +0530, Amit Pundir wrote: > There are firmware versions which do not support host capability > QMI request. We suspect either the host cap is not implemented or > there may be firmware specific issues, but apparently there seem > to be a generation of firmware that has this particular behavior. > > For example, firmware build on Xiaomi Poco F1 (sdm845) phone: > "QC_IMAGE_VERSION_STRING=WLAN.HL.2.0.c3-00257-QCAHLSWMTPLZ-1" > > If we do not skip the host cap QMI request on Poco F1, then we > get a QMI_ERR_MALFORMED_MSG_V01 error message in the > ath10k_qmi_host_cap_send_sync(). But this error message is not > fatal to the firmware nor to the ath10k driver and we can still > bring up the WiFi services successfully if we just ignore it. > > Hence introducing this DeviceTree quirk to skip host capability > QMI request for the firmware versions which do not support this > feature. So if you change the WiFi firmware, you may force a DT change too. Those are pretty independent things otherwise. Why can't you just always ignore this error? If you can't deal with this entirely in the driver, then it should be part of the WiFi firmware so it's always in sync. Rob
On Wed, 30 Sep 2020 at 00:38, Rob Herring <robh@kernel.org> wrote: > > On Fri, Sep 25, 2020 at 11:59:41PM +0530, Amit Pundir wrote: > > There are firmware versions which do not support host capability > > QMI request. We suspect either the host cap is not implemented or > > there may be firmware specific issues, but apparently there seem > > to be a generation of firmware that has this particular behavior. > > > > For example, firmware build on Xiaomi Poco F1 (sdm845) phone: > > "QC_IMAGE_VERSION_STRING=WLAN.HL.2.0.c3-00257-QCAHLSWMTPLZ-1" > > > > If we do not skip the host cap QMI request on Poco F1, then we > > get a QMI_ERR_MALFORMED_MSG_V01 error message in the > > ath10k_qmi_host_cap_send_sync(). But this error message is not > > fatal to the firmware nor to the ath10k driver and we can still > > bring up the WiFi services successfully if we just ignore it. > > > > Hence introducing this DeviceTree quirk to skip host capability > > QMI request for the firmware versions which do not support this > > feature. > > So if you change the WiFi firmware, you may force a DT change too. Those > are pretty independent things otherwise. This is a valid concern and I'm not sure about the other devices, but on PocoF1 I have tried all the three released firmware version updates: WLAN.HL.2.0.c3-00257-QCAHLSWMTPLZ-1 WLAN.HL.2.0.c3-00445-QCAHLSWMTPLZ-1 WLAN.HL.2.0.c3-00534-QCAHLSWMTPLZ-1 and none of them works without this skip host-cap patch or equivalent hack. PocoF1 is already 2+ years old device and sadly I do not expect any major vendor update coming its way. > > Why can't you just always ignore this error? If you can't deal with this > entirely in the driver, then it should be part of the WiFi firmware so > it's always in sync. I don't know the technical details of the ath10k/qmi driver, but I'm OK if we just ignore the return value of ath10k_qmi_host_cap_send_sync() and move along. Regards, Amit Pundir > > Rob
On Tue 29 Sep 14:08 CDT 2020, Rob Herring wrote: > On Fri, Sep 25, 2020 at 11:59:41PM +0530, Amit Pundir wrote: > > There are firmware versions which do not support host capability > > QMI request. We suspect either the host cap is not implemented or > > there may be firmware specific issues, but apparently there seem > > to be a generation of firmware that has this particular behavior. > > > > For example, firmware build on Xiaomi Poco F1 (sdm845) phone: > > "QC_IMAGE_VERSION_STRING=WLAN.HL.2.0.c3-00257-QCAHLSWMTPLZ-1" > > > > If we do not skip the host cap QMI request on Poco F1, then we > > get a QMI_ERR_MALFORMED_MSG_V01 error message in the > > ath10k_qmi_host_cap_send_sync(). But this error message is not > > fatal to the firmware nor to the ath10k driver and we can still > > bring up the WiFi services successfully if we just ignore it. > > > > Hence introducing this DeviceTree quirk to skip host capability > > QMI request for the firmware versions which do not support this > > feature. > > So if you change the WiFi firmware, you may force a DT change too. Those > are pretty independent things otherwise. > Yes and that's not good. But I looked at somehow derive this from firmware version numbers etc and it's not working out, so I'm out of ideas for alternatives. > Why can't you just always ignore this error? If you can't deal with this > entirely in the driver, then it should be part of the WiFi firmware so > it's always in sync. > Unfortunately the firmware versions I've hit this problem on has gone belly up when receiving this request, that's why I asked Amit to add a flag to skip it. That said, in the devices I've hit this I've managed to get newer firmware working, which doesn't have either problem. Regards, Bjorn
Hi Rob, Bjorn, Kalle, On Thu, 29 Oct 2020 at 19:10, Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Tue 29 Sep 14:08 CDT 2020, Rob Herring wrote: > > > On Fri, Sep 25, 2020 at 11:59:41PM +0530, Amit Pundir wrote: > > > There are firmware versions which do not support host capability > > > QMI request. We suspect either the host cap is not implemented or > > > there may be firmware specific issues, but apparently there seem > > > to be a generation of firmware that has this particular behavior. > > > > > > For example, firmware build on Xiaomi Poco F1 (sdm845) phone: > > > "QC_IMAGE_VERSION_STRING=WLAN.HL.2.0.c3-00257-QCAHLSWMTPLZ-1" > > > > > > If we do not skip the host cap QMI request on Poco F1, then we > > > get a QMI_ERR_MALFORMED_MSG_V01 error message in the > > > ath10k_qmi_host_cap_send_sync(). But this error message is not > > > fatal to the firmware nor to the ath10k driver and we can still > > > bring up the WiFi services successfully if we just ignore it. > > > > > > Hence introducing this DeviceTree quirk to skip host capability > > > QMI request for the firmware versions which do not support this > > > feature. > > > > So if you change the WiFi firmware, you may force a DT change too. Those > > are pretty independent things otherwise. > > > > Yes and that's not good. But I looked at somehow derive this from > firmware version numbers etc and it's not working out, so I'm out of > ideas for alternatives. > > > Why can't you just always ignore this error? If you can't deal with this > > entirely in the driver, then it should be part of the WiFi firmware so > > it's always in sync. > > > > Unfortunately the firmware versions I've hit this problem on has gone > belly up when receiving this request, that's why I asked Amit to add a > flag to skip it. So what is next for this DT quirk? I'm OK to go back to my previous of_machine_is_compatible() device specific hack, for now, https://patchwork.kernel.org/project/linux-wireless/patch/1600328501-8832-1-git-send-email-amit.pundir@linaro.org/ till we have a reasonable fix in place or receive a vendor firmware drop which fixes this problem (which I believe is highly unlikely though, for this 2+ years old device). Regards, Amit Pundir > > That said, in the devices I've hit this I've managed to get newer > firmware working, which doesn't have either problem. > > Regards, > Bjorn
On Tue 03 Nov 01:48 CST 2020, Amit Pundir wrote: > Hi Rob, Bjorn, Kalle, > > On Thu, 29 Oct 2020 at 19:10, Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Tue 29 Sep 14:08 CDT 2020, Rob Herring wrote: > > > > > On Fri, Sep 25, 2020 at 11:59:41PM +0530, Amit Pundir wrote: > > > > There are firmware versions which do not support host capability > > > > QMI request. We suspect either the host cap is not implemented or > > > > there may be firmware specific issues, but apparently there seem > > > > to be a generation of firmware that has this particular behavior. > > > > > > > > For example, firmware build on Xiaomi Poco F1 (sdm845) phone: > > > > "QC_IMAGE_VERSION_STRING=WLAN.HL.2.0.c3-00257-QCAHLSWMTPLZ-1" > > > > > > > > If we do not skip the host cap QMI request on Poco F1, then we > > > > get a QMI_ERR_MALFORMED_MSG_V01 error message in the > > > > ath10k_qmi_host_cap_send_sync(). But this error message is not > > > > fatal to the firmware nor to the ath10k driver and we can still > > > > bring up the WiFi services successfully if we just ignore it. > > > > > > > > Hence introducing this DeviceTree quirk to skip host capability > > > > QMI request for the firmware versions which do not support this > > > > feature. > > > > > > So if you change the WiFi firmware, you may force a DT change too. Those > > > are pretty independent things otherwise. > > > > > > > Yes and that's not good. But I looked at somehow derive this from > > firmware version numbers etc and it's not working out, so I'm out of > > ideas for alternatives. > > > > > Why can't you just always ignore this error? If you can't deal with this > > > entirely in the driver, then it should be part of the WiFi firmware so > > > it's always in sync. > > > > > > > Unfortunately the firmware versions I've hit this problem on has gone > > belly up when receiving this request, that's why I asked Amit to add a > > flag to skip it. > > So what is next for this DT quirk? > Rob, we still have this problem and we've not come up with any way to determine in runtime that we need to skip this part of the initialization. Regards, Bjorn > I'm OK to go back to my previous of_machine_is_compatible() > device specific hack, for now, > https://patchwork.kernel.org/project/linux-wireless/patch/1600328501-8832-1-git-send-email-amit.pundir@linaro.org/ > till we have a reasonable fix in place or receive a vendor firmware > drop which fixes this problem (which I believe is highly unlikely > though, for this 2+ years old device). > > Regards, > Amit Pundir > > > > > That said, in the devices I've hit this I've managed to get newer > > firmware working, which doesn't have either problem. > > > > Regards, > > Bjorn
Hi Kalle, On Mon, 7 Dec 2020 at 22:25, Kalle Valo <kvalo@codeaurora.org> wrote: > > This is firmware version specific, right? There's also enum > ath10k_fw_features which is embedded within firmware-N.bin, we could add > a new flag there. But that means that a correct firmware-N.bin is needed > for each firmware version, not sure if that would work out. Just > throwing out ideas here. Apologies for this late reply. I was out for a while. If by that (the firmware version) you mean "QC_IMAGE_VERSION_STRING", then that may be a bit tricky. Pocophone F1 use the same firmware family version (WLAN.HL.2.0.XXX), used by Dragonboard 845c (which has Wi-Fi working upstream). Regards, Amit Pundir > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Amit Pundir <amit.pundir@linaro.org> writes: > Hi Kalle, > > On Mon, 7 Dec 2020 at 22:25, Kalle Valo <kvalo@codeaurora.org> wrote: >> >> This is firmware version specific, right? There's also enum >> ath10k_fw_features which is embedded within firmware-N.bin, we could add >> a new flag there. But that means that a correct firmware-N.bin is needed >> for each firmware version, not sure if that would work out. Just >> throwing out ideas here. > > Apologies for this late reply. I was out for a while. No worries. > If by that (the firmware version) you mean "QC_IMAGE_VERSION_STRING", > then that may be a bit tricky. Pocophone F1 use the same firmware > family version (WLAN.HL.2.0.XXX), used by Dragonboard 845c (which has > Wi-Fi working upstream). I'm meaning the ath10k firmware meta data we have in firmware-N.bin (N=2,3,4...) file. A quick summary: Every ath10k firmware release should have firmware-N.bin. The file is created with this tool: https://github.com/qca/qca-swiss-army-knife/blob/master/tools/scripts/ath10k/ath10k-fwencoder firmware-N.bin contains various metadata, one of those being firmware feature flags: enum ath10k_fw_features { /* wmi_mgmt_rx_hdr contains extra RSSI information */ ATH10K_FW_FEATURE_EXT_WMI_MGMT_RX = 0, /* Firmware from 10X branch. Deprecated, don't use in new code. */ ATH10K_FW_FEATURE_WMI_10X = 1, [...] So what you could is add a new flag enum ath10k_fw_features, create a new firmware-N.bin for your device and enable the flag on the firmware releases for your device only. I don't know if this is usable, but one solution which came to my mind.
On Mon 08 Feb 11:21 CST 2021, Kalle Valo wrote: > Amit Pundir <amit.pundir@linaro.org> writes: > > > Hi Kalle, > > > > On Mon, 7 Dec 2020 at 22:25, Kalle Valo <kvalo@codeaurora.org> wrote: > >> > >> This is firmware version specific, right? There's also enum > >> ath10k_fw_features which is embedded within firmware-N.bin, we could add > >> a new flag there. But that means that a correct firmware-N.bin is needed > >> for each firmware version, not sure if that would work out. Just > >> throwing out ideas here. > > > > Apologies for this late reply. I was out for a while. > > No worries. > > > If by that (the firmware version) you mean "QC_IMAGE_VERSION_STRING", > > then that may be a bit tricky. Pocophone F1 use the same firmware > > family version (WLAN.HL.2.0.XXX), used by Dragonboard 845c (which has > > Wi-Fi working upstream). > > I'm meaning the ath10k firmware meta data we have in firmware-N.bin > (N=2,3,4...) file. A quick summary: > > Every ath10k firmware release should have firmware-N.bin. The file is > created with this tool: > > https://github.com/qca/qca-swiss-army-knife/blob/master/tools/scripts/ath10k/ath10k-fwencoder > > firmware-N.bin contains various metadata, one of those being firmware > feature flags: > > enum ath10k_fw_features { > /* wmi_mgmt_rx_hdr contains extra RSSI information */ > ATH10K_FW_FEATURE_EXT_WMI_MGMT_RX = 0, > > /* Firmware from 10X branch. Deprecated, don't use in new code. */ > ATH10K_FW_FEATURE_WMI_10X = 1, > > [...] > > So what you could is add a new flag enum ath10k_fw_features, create a > new firmware-N.bin for your device and enable the flag on the firmware > releases for your device only. > > I don't know if this is usable, but one solution which came to my mind. It sounds quite reasonable to pass this using firmawre-N.bin instead of DT, however that would imply that we need to find firmware-N.bin in the device-specific directory, where we keep the wlanmdsp.mbn as well - and not under /lib/firmware/ath10k For other devices (e.g. ADSP, modem or wlanmdsp.mbn) we're putting these in e.g. /lib/firmware/qcom/LENOVO/81JL/ and specifies the location using a firmware-name property in DT. Regards, Bjorn > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Hi, Reviving this old thread again, to check if there are still any hopes of landing this patch upstream. Based on the feedback I have got so far, there are no easy way to skip this part of the initialization at runtime. Bjorn and Kalle discussed the possibility of creating device specific firmware-N.bin firmware file but that would mean firmware-N.bin has to be loaded from the device-specific directory along with wlanmdsp.bin. And ideally making ath10k/ath11k in-charge of firmware loading, but there doesn't seem to be a consensus on this either(?) Regards, Amit Pundir On Tue, 9 Feb 2021 at 13:41, Kalle Valo <kvalo@codeaurora.org> wrote: > > Bjorn Andersson <bjorn.andersson@linaro.org> writes: > > > On Mon 08 Feb 11:21 CST 2021, Kalle Valo wrote: > > > >> Amit Pundir <amit.pundir@linaro.org> writes: > >> > >> > Hi Kalle, > >> > > >> > On Mon, 7 Dec 2020 at 22:25, Kalle Valo <kvalo@codeaurora.org> wrote: > >> >> > >> >> This is firmware version specific, right? There's also enum > >> >> ath10k_fw_features which is embedded within firmware-N.bin, we could add > >> >> a new flag there. But that means that a correct firmware-N.bin is needed > >> >> for each firmware version, not sure if that would work out. Just > >> >> throwing out ideas here. > >> > > >> > Apologies for this late reply. I was out for a while. > >> > >> No worries. > >> > >> > If by that (the firmware version) you mean "QC_IMAGE_VERSION_STRING", > >> > then that may be a bit tricky. Pocophone F1 use the same firmware > >> > family version (WLAN.HL.2.0.XXX), used by Dragonboard 845c (which has > >> > Wi-Fi working upstream). > >> > >> I'm meaning the ath10k firmware meta data we have in firmware-N.bin > >> (N=2,3,4...) file. A quick summary: > >> > >> Every ath10k firmware release should have firmware-N.bin. The file is > >> created with this tool: > >> > >> https://github.com/qca/qca-swiss-army-knife/blob/master/tools/scripts/ath10k/ath10k-fwencoder > >> > >> firmware-N.bin contains various metadata, one of those being firmware > >> feature flags: > >> > >> enum ath10k_fw_features { > >> /* wmi_mgmt_rx_hdr contains extra RSSI information */ > >> ATH10K_FW_FEATURE_EXT_WMI_MGMT_RX = 0, > >> > >> /* Firmware from 10X branch. Deprecated, don't use in new code. */ > >> ATH10K_FW_FEATURE_WMI_10X = 1, > >> > >> [...] > >> > >> So what you could is add a new flag enum ath10k_fw_features, create a > >> new firmware-N.bin for your device and enable the flag on the firmware > >> releases for your device only. > >> > >> I don't know if this is usable, but one solution which came to my mind. > > > > It sounds quite reasonable to pass this using firmawre-N.bin instead of > > DT, however that would imply that we need to find firmware-N.bin in the > > device-specific directory, where we keep the wlanmdsp.mbn as well - and > > not under /lib/firmware/ath10k > > > > For other devices (e.g. ADSP, modem or wlanmdsp.mbn) we're putting these > > in e.g. /lib/firmware/qcom/LENOVO/81JL/ and specifies the location using > > a firmware-name property in DT. > > Ah, I didn't realise that. > > Actually I would like to have ath10k in control[1] of QMI/rproc firmware > loading as the firmware releases have different constraints, like the > issue we are now discussing. Ideally firmware-N.bin would contain all > firmware files, for example wlanmdsp.mbn, and from the meta data > ath10k/ath11k would know what version of the firmware interface should > be used. > > I remember we discussed this briefly a year or two ago and there was no > easy solution, but I really wish we could find one. More these kind of > firmware interface incompatibilities will most likely pop up, also in > ath11k, so it would be great to find a clean and easily maneagable > solution. > > [1] With control I mean that ath10k/ath11k can choose which firmware > should be loaded > > -- > https://patchwork.kernel.org/project/linux-wireless/list/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff --git a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt index 65ee68efd574..135c7ecd4487 100644 --- a/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt +++ b/Documentation/devicetree/bindings/net/wireless/qcom,ath10k.txt @@ -86,6 +86,11 @@ Optional properties: Value type: <empty> Definition: Quirk specifying that the firmware expects the 8bit version of the host capability QMI request +- qcom,snoc-host-cap-skip-quirk: + Usage: Optional + Value type: <empty> + Definition: Quirk specifying that the firmware wants to skip the host + capability QMI request - qcom,xo-cal-data: xo cal offset to be configured in xo trim register. - qcom,msa-fixed-perm: Boolean context flag to disable SCM call for statically diff --git a/drivers/net/wireless/ath/ath10k/qmi.c b/drivers/net/wireless/ath/ath10k/qmi.c index 5468a41e928e..5adff7695e18 100644 --- a/drivers/net/wireless/ath/ath10k/qmi.c +++ b/drivers/net/wireless/ath/ath10k/qmi.c @@ -770,6 +770,7 @@ ath10k_qmi_ind_register_send_sync_msg(struct ath10k_qmi *qmi) static void ath10k_qmi_event_server_arrive(struct ath10k_qmi *qmi) { struct ath10k *ar = qmi->ar; + struct ath10k_snoc *ar_snoc = ath10k_snoc_priv(ar); int ret; ret = ath10k_qmi_ind_register_send_sync_msg(qmi); @@ -781,9 +782,15 @@ static void ath10k_qmi_event_server_arrive(struct ath10k_qmi *qmi) return; } - ret = ath10k_qmi_host_cap_send_sync(qmi); - if (ret) - return; + /* + * Skip the host capability request for the firmware versions which + * do not support this feature. + */ + if (!test_bit(ATH10K_SNOC_FLAG_SKIP_HOST_CAP_QUIRK, &ar_snoc->flags)) { + ret = ath10k_qmi_host_cap_send_sync(qmi); + if (ret) + return; + } ret = ath10k_qmi_msa_mem_info_send_sync_msg(qmi); if (ret) diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c index 354d49b1cd45..4efbf1339c80 100644 --- a/drivers/net/wireless/ath/ath10k/snoc.c +++ b/drivers/net/wireless/ath/ath10k/snoc.c @@ -1281,6 +1281,9 @@ static void ath10k_snoc_quirks_init(struct ath10k *ar) if (of_property_read_bool(dev->of_node, "qcom,snoc-host-cap-8bit-quirk")) set_bit(ATH10K_SNOC_FLAG_8BIT_HOST_CAP_QUIRK, &ar_snoc->flags); + + if (of_property_read_bool(dev->of_node, "qcom,snoc-host-cap-skip-quirk")) + set_bit(ATH10K_SNOC_FLAG_SKIP_HOST_CAP_QUIRK, &ar_snoc->flags); } int ath10k_snoc_fw_indication(struct ath10k *ar, u64 type) diff --git a/drivers/net/wireless/ath/ath10k/snoc.h b/drivers/net/wireless/ath/ath10k/snoc.h index a3dd06f6ac62..2a0045f0af7e 100644 --- a/drivers/net/wireless/ath/ath10k/snoc.h +++ b/drivers/net/wireless/ath/ath10k/snoc.h @@ -47,6 +47,7 @@ enum ath10k_snoc_flags { ATH10K_SNOC_FLAG_UNREGISTERING, ATH10K_SNOC_FLAG_RECOVERY, ATH10K_SNOC_FLAG_8BIT_HOST_CAP_QUIRK, + ATH10K_SNOC_FLAG_SKIP_HOST_CAP_QUIRK, }; struct clk_bulk_data;
There are firmware versions which do not support host capability QMI request. We suspect either the host cap is not implemented or there may be firmware specific issues, but apparently there seem to be a generation of firmware that has this particular behavior. For example, firmware build on Xiaomi Poco F1 (sdm845) phone: "QC_IMAGE_VERSION_STRING=WLAN.HL.2.0.c3-00257-QCAHLSWMTPLZ-1" If we do not skip the host cap QMI request on Poco F1, then we get a QMI_ERR_MALFORMED_MSG_V01 error message in the ath10k_qmi_host_cap_send_sync(). But this error message is not fatal to the firmware nor to the ath10k driver and we can still bring up the WiFi services successfully if we just ignore it. Hence introducing this DeviceTree quirk to skip host capability QMI request for the firmware versions which do not support this feature. Suggested-by: Bjorn Andersson <bjorn.andersson@linaro.org> Signed-off-by: Amit Pundir <amit.pundir@linaro.org> --- .../devicetree/bindings/net/wireless/qcom,ath10k.txt | 5 +++++ drivers/net/wireless/ath/ath10k/qmi.c | 13 ++++++++++--- drivers/net/wireless/ath/ath10k/snoc.c | 3 +++ drivers/net/wireless/ath/ath10k/snoc.h | 1 + 4 files changed, 19 insertions(+), 3 deletions(-)