Bluetooth: qca: Support downloading board id specific NVM for WCN6855

Download board id specific NVM instead of default for WCN6855 if board
id is available, and that is required by Lenovo ThinkPad X13s.

Cc: Bjorn Andersson <bjorande@quicinc.com>
Cc: Aiqun Yu (Maria) <quic_aiquny@quicinc.com>
Cc: Cheng Jiang <quic_chejiang@quicinc.com>
Cc: Johan Hovold <johan@kernel.org>
Cc: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
Cc: Steev Klimaszewski <steev@kali.org>
Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
---
 drivers/bluetooth/btqca.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)


---
base-commit: e88b020190bf5bc3e7ce5bd8003fc39b23cc95fe
change-id: 20241113-x13s_wcn6855_fix-53c573ff7878

Best regards,

On 14.11.24 10:49, Paul Menzel wrote:
> Dear Zijun,
>
>
> Thank you for your patch.
>
> Am 14.11.24 um 07:26 schrieb Zijun Hu:
>> Download board id specific NVM instead of default for WCN6855 if board
>> id is available, and that is required by Lenovo ThinkPad X13s.
>
> Could you please start by describing the problem/motivation. What does
> not work with the Lenovo ThinkPad X13s before your pacth.
>
> What is variant *g*?
>
> Maybe also describe the file naming convention in the commit message.
>
>
Hi Paul,

Zijun was so kind to review my RFC patch [1] and post an alternate
implementation. The problem is/was that the default firmware patch files
for WCN6855 don't enable the possible quality and range that you get
with board specific files, which are now [2] available in
linux-firmware. It is not only the Lenovo Thinkpad X13s that is
affected, it is quite a range of devices.

The variant *g* is a SoC variant with some extended capabilities as it
seems. The X13s doesn't have it, the Windows Dev Kit 2023 and the HP
Omnibook X14 have it. I have no real information about what the
difference is, but there is code in btqca.c to generate distinct
firmware names.

with best regards

Jens

[1]
https://lore.kernel.org/all/20241003-bt-nvm-firmware-v1-1-79028931214f@oldschoolsolutions.biz/

[2]
https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/qca?id=77a11ffc5a0aaaadc870793d02f6c6781ee9f598

On Fri, Nov 15, 2024 at 08:13:55AM +0100, Jens Glathe wrote:

> The variant *g* is a SoC variant with some extended capabilities as it
> seems. The X13s doesn't have it, the Windows Dev Kit 2023 and the HP
> Omnibook X14 have it. 

Actually my X13s has the GF (or g) variant, so perhaps also other
machines can come with one or the other.

Understanding what the difference is between GF and non-GF would indeed
be interesting.

Johan

Hi Zijun,

On Thu, Nov 14, 2024 at 1:27 AM Zijun Hu <quic_zijuhu@quicinc.com> wrote:
>
> Download board id specific NVM instead of default for WCN6855 if board
> id is available, and that is required by Lenovo ThinkPad X13s.
>
> Cc: Bjorn Andersson <bjorande@quicinc.com>
> Cc: Aiqun Yu (Maria) <quic_aiquny@quicinc.com>
> Cc: Cheng Jiang <quic_chejiang@quicinc.com>
> Cc: Johan Hovold <johan@kernel.org>
> Cc: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> Cc: Steev Klimaszewski <steev@kali.org>
> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>

How about adding the following:

Fixes: 095327fede00 ("Bluetooth: hci_qca: Add support for QTI
Bluetooth chip wcn6855")

Not sure if this would be simple to backport given that there are
things like 691d54d0f7cb ("Bluetooth: qca: use switch case for soc
type behavior") that may have to be backported as well.

> ---
>  drivers/bluetooth/btqca.c | 35 ++++++++++++++++++++++++++++++++---
>  1 file changed, 32 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index dfbbac92242a..4f8576cbbab9 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -717,6 +717,29 @@ static void qca_generate_hsp_nvm_name(char *fwname, size_t max_size,
>                 snprintf(fwname, max_size, "qca/hpnv%02x%s.%x", rom_ver, variant, bid);
>  }
>
> +static void qca_get_hsp_nvm_name_generic(struct qca_fw_config *cfg,
> +                                        struct qca_btsoc_version ver,
> +                                        u8 rom_ver, u16 bid)
> +{
> +       const char *variant;
> +
> +       /* hsp gf chip */
> +       if ((le32_to_cpu(ver.soc_id) & QCA_HSP_GF_SOC_MASK) == QCA_HSP_GF_SOC_ID)
> +               variant = "g";
> +       else
> +               variant = "";
> +
> +       if (bid == 0x0)
> +               snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/hpnv%02x%s.bin",
> +                        rom_ver, variant);
> +       else if (bid & 0xff00)
> +               snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/hpnv%02x%s.b%x",
> +                        rom_ver, variant, bid);
> +       else
> +               snprintf(cfg->fwname, sizeof(cfg->fwname), "qca/hpnv%02x%s.b%02x",
> +                        rom_ver, variant, bid);
> +}
> +
>  static inline void qca_get_nvm_name_generic(struct qca_fw_config *cfg,
>                                             const char *stem, u8 rom_ver, u16 bid)
>  {
> @@ -810,8 +833,15 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>         /* Give the controller some time to get ready to receive the NVM */
>         msleep(10);
>
> -       if (soc_type == QCA_QCA2066 || soc_type == QCA_WCN7850)
> +       switch (soc_type) {
> +       case QCA_QCA2066:
> +       case QCA_WCN6855:
> +       case QCA_WCN7850:
>                 qca_read_fw_board_id(hdev, &boardid);
> +               break;
> +       default:
> +               break;
> +       }
>
>         /* Download NVM configuration */
>         config.type = TLV_TYPE_NVM;
> @@ -848,8 +878,7 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>                                  "qca/msnv%02x.bin", rom_ver);
>                         break;
>                 case QCA_WCN6855:
> -                       snprintf(config.fwname, sizeof(config.fwname),
> -                                "qca/hpnv%02x.bin", rom_ver);
> +                       qca_get_hsp_nvm_name_generic(&config, ver, rom_ver, boardid);
>                         break;
>                 case QCA_WCN7850:
>                         qca_get_nvm_name_generic(&config, "hmt", rom_ver, boardid);
>
> ---
> base-commit: e88b020190bf5bc3e7ce5bd8003fc39b23cc95fe
> change-id: 20241113-x13s_wcn6855_fix-53c573ff7878
>
> Best regards,
> --
> Zijun Hu <quic_zijuhu@quicinc.com>
>

On 11/15/2024 6:01 PM, Johan Hovold wrote:
> On Wed, Nov 13, 2024 at 10:26:56PM -0800, Zijun Hu wrote:
>> Download board id specific NVM instead of default for WCN6855 if board
>> id is available, and that is required by Lenovo ThinkPad X13s.
>>
>> Cc: Bjorn Andersson <bjorande@quicinc.com>
>> Cc: Aiqun Yu (Maria) <quic_aiquny@quicinc.com>
>> Cc: Cheng Jiang <quic_chejiang@quicinc.com>
>> Cc: Johan Hovold <johan@kernel.org>
>> Cc: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
>> Cc: Steev Klimaszewski <steev@kali.org>
>> Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com>
> 
> This works like a charm on my X13s which has the GF variant.
> 
> Unlike with the "default" NVM file, the range is excellent with the
> board-specific file now pushed to linux-firmware (similar to what I see
> when using the Windows driver NVM file). Specifically, the range with
> the headphones I use for testing increases from about two meters to 20 m
> (around a bend).
> 
> Even if these NVM files didn't make it into the November release of
> linux-firmware and therefore won't make it into the distros for another
> month, I think we should mark this one as a fix and backport it to
> stable as soon as possible.
> 
> Zijun, could you amend the commit message with some details about why
> this needs to be fixed and backported (e.g. refer to my range example
> above)?
> 

will do it within v2 as you suggest.

> Fixes: 095327fede00 ("Bluetooth: hci_qca: Add support for QTI Bluetooth chip wcn6855")
> Cc: stable@vger.kernel.org	# 6.4
> 
> It's possible to add a comment after the stable tag to delay backporting
> until the next linux-firmware release, but in this case it may be better
> to break existing setups and force people to update to the correct radio
> calibration data.
> 

i would like to temporarily add fallback logic within v2, then i will
help to backport it ASAP without breaking existing setups even if the
current default NVM are not for WCN6855.

> Either way:
> 
> Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
> Tested-by: Johan Hovold <johan+linaro@kernel.org>

thank you johan for verification and suggestion.
(^^)(^^)

> 
> Johan

On 11/15/2024 2:40 PM, Jens Glathe wrote:
> Hi Zijun,
> 
> I tested his patch on the HP Omnibook X14 and on the Windows Dev Kit
> 2023, it works well. Thank you!
> 
> Tested-by: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
> 

thank you Jens for helping verification with your machines.

(^^)(^^)
> with best regards