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

Commit Message

quic_zijuhu Nov. 14, 2024, 6:26 a.m. UTC

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 <[e-mail]>
Cc: Aiqun Yu (Maria) <[e-mail]>
Cc: Cheng Jiang <[e-mail]>
Cc: Johan Hovold <[e-mail]>
Cc: Jens Glathe <[e-mail]>
Cc: Steev Klimaszewski <[e-mail]>
Signed-off-by: Zijun Hu <[e-mail]>
---
 drivers/bluetooth/btqca.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)


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

Best regards,

Comments

Paul Menzel Nov. 14, 2024, 9:49 a.m. UTC | #1

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.

> Cc: Bjorn Andersson <[e-mail]>
> Cc: Aiqun Yu (Maria) <[e-mail]>
> Cc: Cheng Jiang <[e-mail]>
> Cc: Johan Hovold <[e-mail]>
> Cc: Jens Glathe <[e-mail]>
> Cc: Steev Klimaszewski <[e-mail]>
> Signed-off-by: Zijun Hu <[e-mail]>
> ---
>   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);


Kind regards,

Paul

Jens Glathe Nov. 15, 2024, 6:40 a.m. UTC | #2

On 14.11.24 07:26, 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 <[e-mail]>
> Cc: Aiqun Yu (Maria) <[e-mail]>
> Cc: Cheng Jiang <[e-mail]>
> Cc: Johan Hovold <[e-mail]>
> Cc: Jens Glathe <[e-mail]>
> Cc: Steev Klimaszewski <[e-mail]>
> Signed-off-by: Zijun Hu <[e-mail]>
> ---
>   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,

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 <[e-mail]>

with best regards

Jens

Jens Glathe Nov. 15, 2024, 7:13 a.m. UTC | #3

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/[e-mail]/

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

Steev Klimaszewski Nov. 15, 2024, 7:17 a.m. UTC | #4

Hi Zijun,

On Thu, Nov 14, 2024 at 12:27 AM Zijun Hu <[e-mail]> 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 <[e-mail]>
> Cc: Aiqun Yu (Maria) <[e-mail]>
> Cc: Cheng Jiang <[e-mail]>
> Cc: Johan Hovold <[e-mail]>
> Cc: Jens Glathe <[e-mail]>
> Cc: Steev Klimaszewski <[e-mail]>
> Signed-off-by: Zijun Hu <[e-mail]>
> ---
>  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 <[e-mail]>
>

Thank you for this, I'd had something similar written, so it's nice to
see I was kind of on the right track!  I tested this on my Thinkpad
X13s with an H2GO bluetooth Speaker and the range I can get is far
greater when it properly loads the b8c file (with this patch), than
with the .bin (without this patch).  Thanks again!

Tested-by: Steev Klimaszewski <[e-mail]>

Johan Hovold Nov. 15, 2024, 10:01 a.m. UTC | #5

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 <[e-mail]>
> Cc: Aiqun Yu (Maria) <[e-mail]>
> Cc: Cheng Jiang <[e-mail]>
> Cc: Johan Hovold <[e-mail]>
> Cc: Jens Glathe <[e-mail]>
> Cc: Steev Klimaszewski <[e-mail]>
> Signed-off-by: Zijun Hu <[e-mail]>

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)?

Fixes: 095327fede00 ("Bluetooth: hci_qca: Add support for QTI Bluetooth chip wcn6855")
Cc: [e-mail]	# 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.

Either way:

Reviewed-by: Johan Hovold <[e-mail]>
Tested-by: Johan Hovold <[e-mail]>

Johan

Johan Hovold Nov. 15, 2024, 10:07 a.m. UTC | #6

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

Luiz Augusto von Dentz Nov. 15, 2024, 4:40 p.m. UTC | #7

Hi Zijun,

On Thu, Nov 14, 2024 at 1:27 AM Zijun Hu <[e-mail]> 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 <[e-mail]>
> Cc: Aiqun Yu (Maria) <[e-mail]>
> Cc: Cheng Jiang <[e-mail]>
> Cc: Johan Hovold <[e-mail]>
> Cc: Jens Glathe <[e-mail]>
> Cc: Steev Klimaszewski <[e-mail]>
> Signed-off-by: Zijun Hu <[e-mail]>

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 <[e-mail]>
>

quic_zijuhu Nov. 16, 2024, 3:57 p.m. UTC | #8

On 11/16/2024 12:40 AM, Luiz Augusto von Dentz wrote:
> Hi Zijun,
> 
> On Thu, Nov 14, 2024 at 1:27 AM Zijun Hu <[e-mail]> 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 <[e-mail]>
>> Cc: Aiqun Yu (Maria) <[e-mail]>
>> Cc: Cheng Jiang <[e-mail]>
>> Cc: Johan Hovold <[e-mail]>
>> Cc: Jens Glathe <[e-mail]>
>> Cc: Steev Klimaszewski <[e-mail]>
>> Signed-off-by: Zijun Hu <[e-mail]>
> How about adding the following:
> 
> Fixes: 095327fede00 ("Bluetooth: hci_qca: Add support for QTI
> Bluetooth chip wcn6855")
> 

good suggestion. will add it within v2.

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

i will help to backport this change ASAP once it is mainlined.

thank you luiz for code review. (^^)(^^).

>> ---
>>  drivers/bluetooth/btqca.c | 35 ++++++++++++++++++++++++++++++++---
>>  1 file changed, 32 insertions(+), 3 deletions(-)

quic_zijuhu Nov. 16, 2024, 4:07 p.m. UTC | #9

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 <[e-mail]>
>> Cc: Aiqun Yu (Maria) <[e-mail]>
>> Cc: Cheng Jiang <[e-mail]>
>> Cc: Johan Hovold <[e-mail]>
>> Cc: Jens Glathe <[e-mail]>
>> Cc: Steev Klimaszewski <[e-mail]>
>> Signed-off-by: Zijun Hu <[e-mail]>
> 
> 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: [e-mail]	# 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 <[e-mail]>
> Tested-by: Johan Hovold <[e-mail]>

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

> 
> Johan

quic_zijuhu Nov. 16, 2024, 4:10 p.m. UTC | #10

On 11/14/2024 5:49 PM, Paul Menzel wrote:
> 
> 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.
> 

will do it within v2 as you suggest.

> What is variant *g*?
> 
> Maybe also describe the file naming convention in the commit message.

v2 comments under --- will contain reply about this.

thank you for code review (^^) (^^)

quic_zijuhu Nov. 16, 2024, 4:14 p.m. UTC | #11

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 <[e-mail]>
> 

thank you Jens for helping verification with your machines.

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

quic_zijuhu Nov. 16, 2024, 4:17 p.m. UTC | #12

On 11/15/2024 3:17 PM, Steev Klimaszewski wrote:
> Thank you for this, I'd had something similar written, so it's nice to
> see I was kind of on the right track!  I tested this on my Thinkpad
> X13s with an H2GO bluetooth Speaker and the range I can get is far
> greater when it properly loads the b8c file (with this patch), than
> with the .bin (without this patch).  Thanks again!
> 
> Tested-by: Steev Klimaszewski <[e-mail]>

i am glad to heard that the issue can be fixed
thank you Steev for help verification with your machines.
(^^)(^^).

Patch

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);