mbox series

[RFC,v2,0/4] wifi: ath10k: support board-specific firmware overrides

Message ID 20240306-wcn3990-firmware-path-v2-0-f89e98e71a57@linaro.org
Headers show
Series wifi: ath10k: support board-specific firmware overrides | expand

Message

Dmitry Baryshkov March 6, 2024, 8:16 a.m. UTC
On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
modem DSP via the TQFTPserv. These MBN files are signed by the device
vendor, can only be used with the particular SoC or device.

Unfortunately different firmware versions come with different features.
For example firmware for SDM845 doesn't use single-chan-info-per-channel
feature, while firmware for QRB2210 / QRB4210 requires that feature.

Allow board DT files to override the subdir of the fw dir used to lookup
the firmware-N.bin file decribing corresponding WiFi firmware.
For example, adding firmware-name = "qrb4210" property will make the
driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Changes in v2:
- Fixed the comment about the default board name being NULL (Kalle)
- Expanded commit message to provide examples for firmware paths (Kalle)
- Added a note regarding board-2.bin to the commit message (Kalle)
- Link to v1: https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org

---
Dmitry Baryshkov (4):
      dt-bindings: net: wireless: ath10k: describe firmware-name property
      wifi: ath10k: support board-specific firmware overrides
      arm64: dts: qcom: qrb2210-rb1: add firmware-name qualifier to WiFi node
      arm64: dts: qcom: qrb4210-rb1: add firmware-name qualifier to WiFi node

 .../devicetree/bindings/net/wireless/qcom,ath10k.yaml         |  6 ++++++
 arch/arm64/boot/dts/qcom/qrb2210-rb1.dts                      |  1 +
 arch/arm64/boot/dts/qcom/qrb4210-rb2.dts                      |  1 +
 drivers/net/wireless/ath/ath10k/core.c                        | 11 ++++++++++-
 drivers/net/wireless/ath/ath10k/core.h                        |  2 ++
 drivers/net/wireless/ath/ath10k/snoc.c                        |  3 +++
 6 files changed, 23 insertions(+), 1 deletion(-)
---
base-commit: 596764183be8ebb13352b281a442a1f1151c9b06
change-id: 20240130-wcn3990-firmware-path-7a05a0cf8107

Best regards,

Comments

Kalle Valo March 6, 2024, 9:02 a.m. UTC | #1
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> Different Qualcomm platforms using WCN3990 WiFI chip use SoC-specific
> firmware versions with different features. For example firmware for
> SDM845 doesn't use single-chan-info-per-channel feature, while firmware
> for QRB2210 / QRB4210 requires that feature. Allow board DT files to
> override the subdir of the fw dir used to lookup the firmware-N.bin file
> decribing corresponding WiFi firmware.
>
> For example:
>
> - ath10k/WCN3990/hw1.0/wlanmdsp.mbn,
>   ath10k/WCN3990/hw1.0/firmware-5.bin: main firmware files, used by default
>
> - ath10k/WCN3990/hw1.0/qcm2290/wlanmdsp.mbn,
>   ath10k/WCN3990/hw1.0/qcm2290/firmware-5.bin: SoC specific firmware
>     with different signature and feature bits

Thanks, this looks good now.

> Note, while board files lookup uses the same function and thus it is
> possible to provide board-specific board-2.bin files, this is not
> required in 99% of cases as board-2.bin already contains a way to
> provide board-specific data with finer granularity than DT overrides.

After thinking more about this I agree, we want to have just one
board-2.bin file for WCN3990 as it makes easier to share board files
across all variants.
Kalle Valo March 6, 2024, 9:04 a.m. UTC | #2
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
> modem DSP via the TQFTPserv. These MBN files are signed by the device
> vendor, can only be used with the particular SoC or device.
>
> Unfortunately different firmware versions come with different features.
> For example firmware for SDM845 doesn't use single-chan-info-per-channel
> feature, while firmware for QRB2210 / QRB4210 requires that feature.
>
> Allow board DT files to override the subdir of the fw dir used to lookup
> the firmware-N.bin file decribing corresponding WiFi firmware.
> For example, adding firmware-name = "qrb4210" property will make the
> driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
> directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> Changes in v2:
> - Fixed the comment about the default board name being NULL (Kalle)
> - Expanded commit message to provide examples for firmware paths (Kalle)
> - Added a note regarding board-2.bin to the commit message (Kalle)
> - Link to v1: https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org

From my point of view this looks good now but let's see what others say.
Is there a specific reason why you marked this as RFC still?
Dmitry Baryshkov March 6, 2024, 9:23 a.m. UTC | #3
On Wed, 6 Mar 2024 at 11:04, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
> > modem DSP via the TQFTPserv. These MBN files are signed by the device
> > vendor, can only be used with the particular SoC or device.
> >
> > Unfortunately different firmware versions come with different features.
> > For example firmware for SDM845 doesn't use single-chan-info-per-channel
> > feature, while firmware for QRB2210 / QRB4210 requires that feature.
> >
> > Allow board DT files to override the subdir of the fw dir used to lookup
> > the firmware-N.bin file decribing corresponding WiFi firmware.
> > For example, adding firmware-name = "qrb4210" property will make the
> > driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
> > directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> > Changes in v2:
> > - Fixed the comment about the default board name being NULL (Kalle)
> > - Expanded commit message to provide examples for firmware paths (Kalle)
> > - Added a note regarding board-2.bin to the commit message (Kalle)
> > - Link to v1: https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org
>
> From my point of view this looks good now but let's see what others say.
> Is there a specific reason why you marked this as RFC still?

No, I just forgot to remove it from the series settings, so you can
consider it as final.

I had one minor question in my head (but that's mostly for patches 3
and 4): in linux-firmware we will have ath10k/WCN3990/hw1.0/qcm2290
and make qrb4210 as a symlink to it. Is that fine from your POV? Or
should we use sensible device names (e.g. qcom-rb1)?
Kalle Valo March 6, 2024, 11:15 a.m. UTC | #4
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> On Wed, 6 Mar 2024 at 11:04, Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>>
>> > On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
>> > modem DSP via the TQFTPserv. These MBN files are signed by the device
>> > vendor, can only be used with the particular SoC or device.
>> >
>> > Unfortunately different firmware versions come with different features.
>> > For example firmware for SDM845 doesn't use single-chan-info-per-channel
>> > feature, while firmware for QRB2210 / QRB4210 requires that feature.
>> >
>> > Allow board DT files to override the subdir of the fw dir used to lookup
>> > the firmware-N.bin file decribing corresponding WiFi firmware.
>> > For example, adding firmware-name = "qrb4210" property will make the
>> > driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
>> > directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
>> >
>> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> > ---
>> > Changes in v2:
>> > - Fixed the comment about the default board name being NULL (Kalle)
>> > - Expanded commit message to provide examples for firmware paths (Kalle)
>> > - Added a note regarding board-2.bin to the commit message (Kalle)
>> > - Link to v1:
>> > https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org
>>
>> From my point of view this looks good now but let's see what others say.
>> Is there a specific reason why you marked this as RFC still?
>
> No, I just forgot to remove it from the series settings, so you can
> consider it as final.

Good, so let's ignore the RFC label for this v2.

> I had one minor question in my head (but that's mostly for patches 3
> and 4): in linux-firmware we will have ath10k/WCN3990/hw1.0/qcm2290
> and make qrb4210 as a symlink to it. Is that fine from your POV? 

Yes, I think using a symlink is a good idea.

> Or should we use sensible device names (e.g. qcom-rb1)?

I guess 'qcom-rb1' refers to 'Qualcomm Robotics RB1' board? In other
words, the question is that should we use chipset specific names like
'qcm2290' or product based names like 'qcom-rb1'?

That's a good question for which I don't have a good answer :) I'm not
very familiar with WCN3990 hardware and SoCs to have a full picture of
all this, especially how the firmware images are signed or what
different firmware branches there are etc.

To be on the safe side using 'qcom-rb1' makes sense but on the other
hand that means we need to update linux-firmware (basically add a new
symlink) everytime a new product is added. But are there going to be
that many new ath10k based products?

Using 'qcm2290' is easier because for a new product then there only
needs to be a change in DTS and no need to change anything
linux-firmware. But here the risk is that if there's actually two
different ath10k firmware branches for 'qcm2290'. If that ever happens
(I hope not) I guess we could solve that by adding new 'qcm2290-foo'
directory?

But I don't really know, thoughts?
Dmitry Baryshkov March 6, 2024, 2:23 p.m. UTC | #5
On Wed, 6 Mar 2024 at 13:15, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Wed, 6 Mar 2024 at 11:04, Kalle Valo <kvalo@kernel.org> wrote:
> >
> >>
> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >>
> >> > On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
> >> > modem DSP via the TQFTPserv. These MBN files are signed by the device
> >> > vendor, can only be used with the particular SoC or device.
> >> >
> >> > Unfortunately different firmware versions come with different features.
> >> > For example firmware for SDM845 doesn't use single-chan-info-per-channel
> >> > feature, while firmware for QRB2210 / QRB4210 requires that feature.
> >> >
> >> > Allow board DT files to override the subdir of the fw dir used to lookup
> >> > the firmware-N.bin file decribing corresponding WiFi firmware.
> >> > For example, adding firmware-name = "qrb4210" property will make the
> >> > driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
> >> > directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
> >> >
> >> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> >> > ---
> >> > Changes in v2:
> >> > - Fixed the comment about the default board name being NULL (Kalle)
> >> > - Expanded commit message to provide examples for firmware paths (Kalle)
> >> > - Added a note regarding board-2.bin to the commit message (Kalle)
> >> > - Link to v1:
> >> > https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org
> >>
> >> From my point of view this looks good now but let's see what others say.
> >> Is there a specific reason why you marked this as RFC still?
> >
> > No, I just forgot to remove it from the series settings, so you can
> > consider it as final.
>
> Good, so let's ignore the RFC label for this v2.
>
> > I had one minor question in my head (but that's mostly for patches 3
> > and 4): in linux-firmware we will have ath10k/WCN3990/hw1.0/qcm2290
> > and make qrb4210 as a symlink to it. Is that fine from your POV?
>
> Yes, I think using a symlink is a good idea.
>
> > Or should we use sensible device names (e.g. qcom-rb1)?
>
> I guess 'qcom-rb1' refers to 'Qualcomm Robotics RB1' board? In other
> words, the question is that should we use chipset specific names like
> 'qcm2290' or product based names like 'qcom-rb1'?
>
> That's a good question for which I don't have a good answer :) I'm not
> very familiar with WCN3990 hardware and SoCs to have a full picture of
> all this, especially how the firmware images are signed or what
> different firmware branches there are etc.

I checked Pixel-3 data, it has wlanmdsp.mbn signed by Google.

>
> To be on the safe side using 'qcom-rb1' makes sense but on the other
> hand that means we need to update linux-firmware (basically add a new
> symlink) everytime a new product is added. But are there going to be
> that many new ath10k based products?
>
> Using 'qcm2290' is easier because for a new product then there only
> needs to be a change in DTS and no need to change anything
> linux-firmware. But here the risk is that if there's actually two
> different ath10k firmware branches for 'qcm2290'. If that ever happens
> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> directory?
>
> But I don't really know, thoughts?

After some thought, I'd suggest to follow approach taken by the rest
of qcom firmware:
put a default (accepted by non-secured hardware) firmware to SoC dir
and then put a vendor-specific firmware into subdir. If any of such
vendors appear, we might even implement structural fallback: first
look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
finally just under hw1.0.

>
> --
> https://patchwork.kernel.org/project/linux-wireless/list/
>
> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


--
With best wishes
Dmitry
Jeff Johnson March 6, 2024, 10:25 p.m. UTC | #6
On 3/6/2024 6:23 AM, Dmitry Baryshkov wrote:
> After some thought, I'd suggest to follow approach taken by the rest
> of qcom firmware:
> put a default (accepted by non-secured hardware) firmware to SoC dir
> and then put a vendor-specific firmware into subdir. If any of such
> vendors appear, we might even implement structural fallback: first
> look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> finally just under hw1.0.

are there existing examples in linux-firmware?

or is the whole point being only the default firmware is in
linux-firmware and vendors would follow this pattern if they add their
own firmware?
Dmitry Baryshkov March 7, 2024, 8:22 a.m. UTC | #7
On Thu, 7 Mar 2024 at 00:25, Jeff Johnson <quic_jjohnson@quicinc.com> wrote:
>
> On 3/6/2024 6:23 AM, Dmitry Baryshkov wrote:
> > After some thought, I'd suggest to follow approach taken by the rest
> > of qcom firmware:
> > put a default (accepted by non-secured hardware) firmware to SoC dir
> > and then put a vendor-specific firmware into subdir. If any of such
> > vendors appear, we might even implement structural fallback: first
> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> > finally just under hw1.0.
>
> are there existing examples in linux-firmware?

Yes. QCM2290 / QRB4210 platforms have "updated" wlanmdsp.mbn file,
which actually prompted us to work on these overrides. I have opened
the MR against linux-firmware, marked as draft for now, until all
discussions are finished:

https://gitlab.com/kernel-firmware/linux-firmware/-/merge_requests/165

> or is the whole point being only the default firmware is in
> linux-firmware and vendors would follow this pattern if they add their
> own firmware?

Unfortunately not.
Kalle Valo March 8, 2024, 3:19 p.m. UTC | #8
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

>> To be on the safe side using 'qcom-rb1' makes sense but on the other
>> hand that means we need to update linux-firmware (basically add a new
>> symlink) everytime a new product is added. But are there going to be
>> that many new ath10k based products?
>>
>> Using 'qcm2290' is easier because for a new product then there only
>> needs to be a change in DTS and no need to change anything
>> linux-firmware. But here the risk is that if there's actually two
>> different ath10k firmware branches for 'qcm2290'. If that ever happens
>> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
>> directory?
>>
>> But I don't really know, thoughts?
>
> After some thought, I'd suggest to follow approach taken by the rest
> of qcom firmware:

Can you provide pointers to those cases?

> put a default (accepted by non-secured hardware) firmware to SoC dir
> and then put a vendor-specific firmware into subdir. If any of such
> vendors appear, we might even implement structural fallback: first
> look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> finally just under hw1.0.

Honestly that looks quite compilicated compared to having just one
sub-directory. How will ath10k find the directory names (or I vendor and
model names) like 'Google' or 'blueline' in this example?
Dmitry Baryshkov March 9, 2024, 3:07 p.m. UTC | #9
On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
> >> hand that means we need to update linux-firmware (basically add a new
> >> symlink) everytime a new product is added. But are there going to be
> >> that many new ath10k based products?
> >>
> >> Using 'qcm2290' is easier because for a new product then there only
> >> needs to be a change in DTS and no need to change anything
> >> linux-firmware. But here the risk is that if there's actually two
> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> >> directory?
> >>
> >> But I don't really know, thoughts?
> >
> > After some thought, I'd suggest to follow approach taken by the rest
> > of qcom firmware:
>
> Can you provide pointers to those cases?

https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX

>
> > put a default (accepted by non-secured hardware) firmware to SoC dir
> > and then put a vendor-specific firmware into subdir. If any of such
> > vendors appear, we might even implement structural fallback: first
> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> > finally just under hw1.0.
>
> Honestly that looks quite compilicated compared to having just one
> sub-directory. How will ath10k find the directory names (or I vendor and
> model names) like 'Google' or 'blueline' in this example?

I was thinking about the firmware-name = "sdm845/Google/blueline". But
this can be really simpler, firmware-name = "blueline" or
"sdm845/blueline" with no need for fallbacks.

My point is that the firmware-name provides the possibility to handle
that in different ways.
Dmitry Baryshkov March 30, 2024, 4:47 a.m. UTC | #10
On Wed, 6 Mar 2024 at 10:16, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
> modem DSP via the TQFTPserv. These MBN files are signed by the device
> vendor, can only be used with the particular SoC or device.
>
> Unfortunately different firmware versions come with different features.
> For example firmware for SDM845 doesn't use single-chan-info-per-channel
> feature, while firmware for QRB2210 / QRB4210 requires that feature.
>
> Allow board DT files to override the subdir of the fw dir used to lookup
> the firmware-N.bin file decribing corresponding WiFi firmware.
> For example, adding firmware-name = "qrb4210" property will make the
> driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
> directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
> Changes in v2:
> - Fixed the comment about the default board name being NULL (Kalle)
> - Expanded commit message to provide examples for firmware paths (Kalle)
> - Added a note regarding board-2.bin to the commit message (Kalle)
> - Link to v1: https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org
>
> ---
> Dmitry Baryshkov (4):
>       dt-bindings: net: wireless: ath10k: describe firmware-name property
>       wifi: ath10k: support board-specific firmware overrides
>       arm64: dts: qcom: qrb2210-rb1: add firmware-name qualifier to WiFi node
>       arm64: dts: qcom: qrb4210-rb1: add firmware-name qualifier to WiFi node

Kalle, Jeff, is there anything pending on me on this series?
Jeff Johnson April 2, 2024, 11:40 p.m. UTC | #11
On 3/29/2024 9:47 PM, Dmitry Baryshkov wrote:
> On Wed, 6 Mar 2024 at 10:16, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
>>
>> On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
>> modem DSP via the TQFTPserv. These MBN files are signed by the device
>> vendor, can only be used with the particular SoC or device.
>>
>> Unfortunately different firmware versions come with different features.
>> For example firmware for SDM845 doesn't use single-chan-info-per-channel
>> feature, while firmware for QRB2210 / QRB4210 requires that feature.
>>
>> Allow board DT files to override the subdir of the fw dir used to lookup
>> the firmware-N.bin file decribing corresponding WiFi firmware.
>> For example, adding firmware-name = "qrb4210" property will make the
>> driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
>> directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> ---
>> Changes in v2:
>> - Fixed the comment about the default board name being NULL (Kalle)
>> - Expanded commit message to provide examples for firmware paths (Kalle)
>> - Added a note regarding board-2.bin to the commit message (Kalle)
>> - Link to v1: https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org
>>
>> ---
>> Dmitry Baryshkov (4):
>>       dt-bindings: net: wireless: ath10k: describe firmware-name property
>>       wifi: ath10k: support board-specific firmware overrides
>>       arm64: dts: qcom: qrb2210-rb1: add firmware-name qualifier to WiFi node
>>       arm64: dts: qcom: qrb4210-rb1: add firmware-name qualifier to WiFi node
> 
> Kalle, Jeff, is there anything pending on me on this series?
> 
Nothing from me -- this is outside my area of expertise so I'm deferring to Kalle.

Kalle?
Kalle Valo April 4, 2024, 11:48 a.m. UTC | #12
Jeff Johnson <quic_jjohnson@quicinc.com> writes:

> On 3/29/2024 9:47 PM, Dmitry Baryshkov wrote:
>
>> On Wed, 6 Mar 2024 at 10:16, Dmitry Baryshkov
>> <dmitry.baryshkov@linaro.org> wrote:
>>>
>>> On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
>>> modem DSP via the TQFTPserv. These MBN files are signed by the device
>>> vendor, can only be used with the particular SoC or device.
>>>
>>> Unfortunately different firmware versions come with different features.
>>> For example firmware for SDM845 doesn't use single-chan-info-per-channel
>>> feature, while firmware for QRB2210 / QRB4210 requires that feature.
>>>
>>> Allow board DT files to override the subdir of the fw dir used to lookup
>>> the firmware-N.bin file decribing corresponding WiFi firmware.
>>> For example, adding firmware-name = "qrb4210" property will make the
>>> driver look for the firmware-N.bin first in ath10k/WCN3990/hw1.0/qrb4210
>>> directory and then fallback to the default ath10k/WCN3990/hw1.0 dir.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> ---
>>> Changes in v2:
>>> - Fixed the comment about the default board name being NULL (Kalle)
>>> - Expanded commit message to provide examples for firmware paths (Kalle)
>>> - Added a note regarding board-2.bin to the commit message (Kalle)
>>> - Link to v1:
>>> https://lore.kernel.org/r/20240130-wcn3990-firmware-path-v1-0-826b93202964@linaro.org
>>>
>>> ---
>>> Dmitry Baryshkov (4):
>>>       dt-bindings: net: wireless: ath10k: describe firmware-name property
>>>       wifi: ath10k: support board-specific firmware overrides
>>>       arm64: dts: qcom: qrb2210-rb1: add firmware-name qualifier to WiFi node
>>>       arm64: dts: qcom: qrb4210-rb1: add firmware-name qualifier to WiFi node
>> 
>> Kalle, Jeff, is there anything pending on me on this series?
>> 
> Nothing from me -- this is outside my area of expertise so I'm deferring to Kalle.

I was on Easter vacation and now catching up, that's why the delay.
Kalle Valo April 5, 2024, 12:01 p.m. UTC | #13
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
>>
>> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>>
>> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
>> >> hand that means we need to update linux-firmware (basically add a new
>> >> symlink) everytime a new product is added. But are there going to be
>> >> that many new ath10k based products?
>> >>
>> >> Using 'qcm2290' is easier because for a new product then there only
>> >> needs to be a change in DTS and no need to change anything
>> >> linux-firmware. But here the risk is that if there's actually two
>> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
>> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
>> >> directory?
>> >>
>> >> But I don't really know, thoughts?
>> >
>> > After some thought, I'd suggest to follow approach taken by the rest
>> > of qcom firmware:
>>
>> Can you provide pointers to those cases?
>
> https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
>
>>
>> > put a default (accepted by non-secured hardware) firmware to SoC dir
>> > and then put a vendor-specific firmware into subdir. If any of such
>> > vendors appear, we might even implement structural fallback: first
>> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
>> > finally just under hw1.0.
>>
>> Honestly that looks quite compilicated compared to having just one
>> sub-directory. How will ath10k find the directory names (or I vendor and
>> model names) like 'Google' or 'blueline' in this example?
>
> I was thinking about the firmware-name = "sdm845/Google/blueline". But
> this can be really simpler, firmware-name = "blueline" or
> "sdm845/blueline" with no need for fallbacks.

I have been also thinking about this and I would prefer not to have the
fallbacks. But good if you agree with that.

IMHO just "sdm845-blueline" would be the most simple. I don't see the
point of having a directory structure when there are not that many
directories really. But this is just cosmetics.

> My point is that the firmware-name provides the possibility to handle
> that in different ways.

Very good, thanks.
Dmitry Baryshkov April 5, 2024, 12:34 p.m. UTC | #14
On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
> >>
> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >>
> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
> >> >> hand that means we need to update linux-firmware (basically add a new
> >> >> symlink) everytime a new product is added. But are there going to be
> >> >> that many new ath10k based products?
> >> >>
> >> >> Using 'qcm2290' is easier because for a new product then there only
> >> >> needs to be a change in DTS and no need to change anything
> >> >> linux-firmware. But here the risk is that if there's actually two
> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> >> >> directory?
> >> >>
> >> >> But I don't really know, thoughts?
> >> >
> >> > After some thought, I'd suggest to follow approach taken by the rest
> >> > of qcom firmware:
> >>
> >> Can you provide pointers to those cases?
> >
> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
> >
> >>
> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
> >> > and then put a vendor-specific firmware into subdir. If any of such
> >> > vendors appear, we might even implement structural fallback: first
> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> >> > finally just under hw1.0.
> >>
> >> Honestly that looks quite compilicated compared to having just one
> >> sub-directory. How will ath10k find the directory names (or I vendor and
> >> model names) like 'Google' or 'blueline' in this example?
> >
> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
> > this can be really simpler, firmware-name = "blueline" or
> > "sdm845/blueline" with no need for fallbacks.
>
> I have been also thinking about this and I would prefer not to have the
> fallbacks. But good if you agree with that.
>
> IMHO just "sdm845-blueline" would be the most simple. I don't see the
> point of having a directory structure when there are not that many
> directories really. But this is just cosmetics.

It is "not many directories" if we are thinking about the
linux-firmware or open devices. But once embedded distros start
picking this up for the supported devices, this can quickly become a
nuisance. We have been there for Qualcomm DSP firmware and we ended up
adopting the SoC/vendor/device structure, because otherwise it becomes
a bedlam.

> > My point is that the firmware-name provides the possibility to handle
> > that in different ways.
>
> Very good, thanks.

Thanks for your suggestions and for picking the patches.

Bjorn, could you please pick up the DT patches now?
Kalle Valo April 5, 2024, 12:41 p.m. UTC | #15
Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:

> On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
>
>>
>> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>>
>> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
>> >>
>> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>> >>
>> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
>> >> >> hand that means we need to update linux-firmware (basically add a new
>> >> >> symlink) everytime a new product is added. But are there going to be
>> >> >> that many new ath10k based products?
>> >> >>
>> >> >> Using 'qcm2290' is easier because for a new product then there only
>> >> >> needs to be a change in DTS and no need to change anything
>> >> >> linux-firmware. But here the risk is that if there's actually two
>> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
>> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
>> >> >> directory?
>> >> >>
>> >> >> But I don't really know, thoughts?
>> >> >
>> >> > After some thought, I'd suggest to follow approach taken by the rest
>> >> > of qcom firmware:
>> >>
>> >> Can you provide pointers to those cases?
>> >
>> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
>> >
>> >>
>> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
>> >> > and then put a vendor-specific firmware into subdir. If any of such
>> >> > vendors appear, we might even implement structural fallback: first
>> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
>> >> > finally just under hw1.0.
>> >>
>> >> Honestly that looks quite compilicated compared to having just one
>> >> sub-directory. How will ath10k find the directory names (or I vendor and
>> >> model names) like 'Google' or 'blueline' in this example?
>> >
>> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
>> > this can be really simpler, firmware-name = "blueline" or
>> > "sdm845/blueline" with no need for fallbacks.
>>
>> I have been also thinking about this and I would prefer not to have the
>> fallbacks. But good if you agree with that.
>>
>> IMHO just "sdm845-blueline" would be the most simple. I don't see the
>> point of having a directory structure when there are not that many
>> directories really. But this is just cosmetics.
>
> It is "not many directories" if we are thinking about the
> linux-firmware or open devices. But once embedded distros start
> picking this up for the supported devices, this can quickly become a
> nuisance.

Ok. Just out of curiosity, any ideas how many devices/products are there
with wcn3990 who want to use ath10k?
Dmitry Baryshkov April 5, 2024, 12:49 p.m. UTC | #16
On Fri, 5 Apr 2024 at 15:41, Kalle Valo <kvalo@kernel.org> wrote:
>
> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
>
> > On Fri, 5 Apr 2024 at 15:01, Kalle Valo <kvalo@kernel.org> wrote:
> >
> >>
> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >>
> >> > On Fri, 8 Mar 2024 at 17:19, Kalle Valo <kvalo@kernel.org> wrote:
> >> >>
> >> >> Dmitry Baryshkov <dmitry.baryshkov@linaro.org> writes:
> >> >>
> >> >> >> To be on the safe side using 'qcom-rb1' makes sense but on the other
> >> >> >> hand that means we need to update linux-firmware (basically add a new
> >> >> >> symlink) everytime a new product is added. But are there going to be
> >> >> >> that many new ath10k based products?
> >> >> >>
> >> >> >> Using 'qcm2290' is easier because for a new product then there only
> >> >> >> needs to be a change in DTS and no need to change anything
> >> >> >> linux-firmware. But here the risk is that if there's actually two
> >> >> >> different ath10k firmware branches for 'qcm2290'. If that ever happens
> >> >> >> (I hope not) I guess we could solve that by adding new 'qcm2290-foo'
> >> >> >> directory?
> >> >> >>
> >> >> >> But I don't really know, thoughts?
> >> >> >
> >> >> > After some thought, I'd suggest to follow approach taken by the rest
> >> >> > of qcom firmware:
> >> >>
> >> >> Can you provide pointers to those cases?
> >> >
> >> > https://gitlab.com/kernel-firmware/linux-firmware/-/tree/main/qcom/sc8280xp/LENOVO/21BX
> >> >
> >> >>
> >> >> > put a default (accepted by non-secured hardware) firmware to SoC dir
> >> >> > and then put a vendor-specific firmware into subdir. If any of such
> >> >> > vendors appear, we might even implement structural fallback: first
> >> >> > look into sdm845/Google/blueline, then in sdm845/Google, sdm845/ and
> >> >> > finally just under hw1.0.
> >> >>
> >> >> Honestly that looks quite compilicated compared to having just one
> >> >> sub-directory. How will ath10k find the directory names (or I vendor and
> >> >> model names) like 'Google' or 'blueline' in this example?
> >> >
> >> > I was thinking about the firmware-name = "sdm845/Google/blueline". But
> >> > this can be really simpler, firmware-name = "blueline" or
> >> > "sdm845/blueline" with no need for fallbacks.
> >>
> >> I have been also thinking about this and I would prefer not to have the
> >> fallbacks. But good if you agree with that.
> >>
> >> IMHO just "sdm845-blueline" would be the most simple. I don't see the
> >> point of having a directory structure when there are not that many
> >> directories really. But this is just cosmetics.
> >
> > It is "not many directories" if we are thinking about the
> > linux-firmware or open devices. But once embedded distros start
> > picking this up for the supported devices, this can quickly become a
> > nuisance.
>
> Ok. Just out of curiosity, any ideas how many devices/products are there
> with wcn3990 who want to use ath10k?

Just for the DT in mainline I can count about 30 devices that have
ath10k WiFi node:

arch/arm64/boot/dts/qcom/msm8998-clamshell.dtsi:&wifi {
arch/arm64/boot/dts/qcom/msm8998-fxtec-pro1.dts:&wifi {
arch/arm64/boot/dts/qcom/msm8998-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/msm8998-oneplus-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/msm8998-xiaomi-sagit.dts:&wifi {
arch/arm64/boot/dts/qcom/qcs404-evb.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-acer-aspire1.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-idp.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-homestar.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-kingoftown.dts:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-lazor.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-pazquel360.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-pompom.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc7180-trogdor-wormdingler.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sc8180x-lenovo-flex-5g.dts:&wifi {
arch/arm64/boot/dts/qcom/sc8180x-primus.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-cheza.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-db845c.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-oneplus-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-samsung-starqltechn.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-shift-axolotl.dts:&wifi {
arch/arm64/boot/dts/qcom/sdm845-xiaomi-beryllium-common.dtsi:&wifi {
arch/arm64/boot/dts/qcom/sdm845-xiaomi-polaris.dts:&wifi {
arch/arm64/boot/dts/qcom/sm6375-sony-xperia-murray-pdx225.dts:&wifi {
arch/arm64/boot/dts/qcom/sm8150-microsoft-surface-duo.dts:&wifi {
arch/arm64/boot/dts/qcom/sm8150-mtp.dts:&wifi {
arch/arm64/boot/dts/qcom/qrb2210-rb1.dts:&wifi {
arch/arm64/boot/dts/qcom/qrb4210-rb2.dts:&wifi {

The list doesn't include e.g. PIxel 2-3-4-5 or some other phones which
are still on their way for the proper mainline support.
--
With best wishes
Dmitry
Bjorn Andersson April 22, 2024, 1:15 p.m. UTC | #17
On Wed, 06 Mar 2024 10:16:44 +0200, Dmitry Baryshkov wrote:
> On WCN3990 platforms actual firmware, wlanmdsp.mbn, is sideloaded to the
> modem DSP via the TQFTPserv. These MBN files are signed by the device
> vendor, can only be used with the particular SoC or device.
> 
> Unfortunately different firmware versions come with different features.
> For example firmware for SDM845 doesn't use single-chan-info-per-channel
> feature, while firmware for QRB2210 / QRB4210 requires that feature.
> 
> [...]

Applied, thanks!

[3/4] arm64: dts: qcom: qrb2210-rb1: add firmware-name qualifier to WiFi node
      commit: 57ce4b27a12c827a24aaa18aa444bcb8733cb053
[4/4] arm64: dts: qcom: qrb4210-rb1: add firmware-name qualifier to WiFi node
      commit: 673b174b5b2ca2fb99fe52bf7bad3cc348432170

Best regards,