diff mbox series

ath10k: Introduce a devicetree quirk to skip host cap QMI requests

Message ID 1601058581-19461-1-git-send-email-amit.pundir@linaro.org (mailing list archive)
State Changes Requested
Delegated to: Kalle Valo
Headers show
Series ath10k: Introduce a devicetree quirk to skip host cap QMI requests | expand

Commit Message

Amit Pundir Sept. 25, 2020, 6:29 p.m. UTC
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(-)

Comments

Bjorn Andersson Sept. 26, 2020, midnight UTC | #1
On Fri 25 Sep 11:29 PDT 2020, 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.
> 
> 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(-)
> 
> 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)) {

Could have made this an early return inside
ath10k_qmi_host_cap_send_sync(), but this works.

Reviewed-by: Bjorn Andersson <bjorn.andersson@linaro.org>

Regards,
Bjorn

> +		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;
> -- 
> 2.7.4
>
Rob Herring (Arm) Sept. 29, 2020, 7:08 p.m. UTC | #2
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
Amit Pundir Sept. 30, 2020, 11:09 a.m. UTC | #3
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
Bjorn Andersson Oct. 29, 2020, 1:40 p.m. UTC | #4
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
Amit Pundir Nov. 3, 2020, 7:48 a.m. UTC | #5
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
Bjorn Andersson Nov. 24, 2020, 5:51 p.m. UTC | #6
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
Kalle Valo Dec. 7, 2020, 4:55 p.m. UTC | #7
Bjorn Andersson <bjorn.andersson@linaro.org> writes:

> 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.

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.
Amit Pundir Feb. 2, 2021, 11:11 a.m. UTC | #8
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
Kalle Valo Feb. 8, 2021, 5:21 p.m. UTC | #9
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.
Bjorn Andersson Feb. 8, 2021, 5:48 p.m. UTC | #10
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
Kalle Valo Feb. 9, 2021, 8:10 a.m. UTC | #11
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
Amit Pundir May 20, 2021, 11:47 a.m. UTC | #12
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 mbox series

Patch

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;