RE: [PATCH v2] Patch for CYW4373 hci up fail issue

[<Nobuaki.Tsunashima@infineon.com>]

Hi Paul,

Thanks a lot for your comments.

>> CYW4373 ROM FW has an issue that it claims LE_Read_Transmit_Power 
>> command as supported in a response of Read_Local_Supported_Command 
>> command but rejects the LE_Read_Transmit_Power command with "Unknown HCI Command" status.
>> Due to the issue, Bluetooth driver of 5.15 and later kernel fails to hci up.
>
>Why not 5.14?

More precisely, there was no problem with 5.10 and the issue was firstly
reported with 5.15. So, not sure for the interim versions. If the exact version
the issue was firstly introduced is needed, I will take some effort for it.

I will follow your recommendations below.

> First, please prefix the subject with `Bluetooth:`, and be more specific about the change:
> Please use your full name:
> Please re-flow for 75 characters per line and add a blank between paragraphs.

Regards,
Nobuaki Tsunashima

-----Original Message-----
From: Paul Menzel <pmenzel@molgen.mpg.de> 
Sent: Tuesday, May 21, 2024 1:53 AM
To: Tsunashima Nobuaki (SMD C3 JP RM WLS AE) <Nobuaki.Tsunashima@infineon.com>
Cc: marcel@holtmann.org; luiz.dentz@gmail.com; linux-bluetooth@vger.kernel.org
Subject: Re: [PATCH v2] Patch for CYW4373 hci up fail issue

Caution: This e-mail originated outside Infineon Technologies. Do not click on links or open attachments unless you validate it is safe<https://intranet-content.infineon.com/explore/aboutinfineon/rules/informationsecurity/ug/SocialEngineering/Pages/SocialEngineeringElements_en.aspx>.



Dear Nobuaki,


Thank you for your patch. Some comments on formalities.

First, please prefix the subject with `Bluetooth:`, and be more specific about the change:

Bluetooth: Apply HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER to CYW4373

Am 20.05.24 um 10:22 schrieb Nobuaki.Tsunashima@infineon.com:

Please use your full name:

     git config --global user.name "Nobuaki Tsunashima"
     git commit --amend --author="Nobuaki Tsunashima <nobuaki.tsunashima@infineon.com>" -s

> CYW4373 ROM FW has an issue that it claims LE_Read_Transmit_Power 
> command as supported in a response of Read_Local_Supported_Command 
> command but rejects the LE_Read_Transmit_Power command with "Unknown HCI Command" status.
> Due to the issue, Bluetooth driver of 5.15 and later kernel fails to hci up.

Why not 5.14?

> Especially in USB i/f case, it would be difficult to download patch FW that includes Its fix unless hci is up.
> The patch forces the driver to skip LE_Read_Transmit_Power Command 
> when it detects CYW4373 with ROM FW build.

Please re-flow for 75 characters per line and add a blank between paragraphs.


Kind regards,

Paul


> Signed-off-by: Nobuaki Tsunashima <nobuaki.tsunashima@infineon.com>
>
> ---
> V1 -> V2: Fix several coding style warnings.
>
>   drivers/bluetooth/btbcm.c | 33 ++++++++++++++++++++++++++++++++-
>   drivers/bluetooth/btusb.c |  4 ++++
>   2 files changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btbcm.c b/drivers/bluetooth/btbcm.c 
> index 0a5445ac5e1b..da4718a268d0 100644
> --- a/drivers/bluetooth/btbcm.c
> +++ b/drivers/bluetooth/btbcm.c
> @@ -437,18 +437,49 @@ static const struct dmi_system_id disable_broken_read_transmit_power[] = {
>       { }
>   };
>
> +struct bcm_chip_version_table {
> +     u8      chip_id;
> +     u16 baseline;
> +};
> +#define BCM_ROMFW_BASELINE_NUM       0xFFFF
> +static const struct bcm_chip_version_table disable_broken_read_transmit_power_by_chip_ver[] = {
> +     {0x87, BCM_ROMFW_BASELINE_NUM}          /* CYW4373/4373E */
> +};
> +static bool btbcm_is_disable_broken_read_tx_power_by_chip_ver(u8 
> +chip_id, u16 baseline) {
> +     int i;
> +     int table_size = ARRAY_SIZE(disable_broken_read_transmit_power_by_chip_ver);
> +     const struct bcm_chip_version_table *entry =
> +                                                     
> +&disable_broken_read_transmit_power_by_chip_ver[0];
> +
> +     for (i = 0 ; i < table_size ; i++, entry++)
> +     {
> +             if ((chip_id == entry->chip_id) && (baseline == entry->baseline))
> +                     return true;
> +     }
> +
> +     return false;
> +}
> +
>   static int btbcm_read_info(struct hci_dev *hdev)
>   {
>       struct sk_buff *skb;
> +     u8 chip_id;
> +     u16 baseline;
>
>       /* Read Verbose Config Version Info */
>       skb = btbcm_read_verbose_config(hdev);
>       if (IS_ERR(skb))
>               return PTR_ERR(skb);
> -
> +     chip_id = skb->data[1];
> +     baseline = skb->data[3] | (skb->data[4] << 8);
>       bt_dev_info(hdev, "BCM: chip id %u", skb->data[1]);
>       kfree_skb(skb);
>
> +     /* Check Chip ID and disable broken Read LE Min/Max Tx Power */
> +     if (btbcm_is_disable_broken_read_tx_power_by_chip_ver(chip_id, baseline))
> +             set_bit(HCI_QUIRK_BROKEN_READ_TRANSMIT_POWER, 
> + &hdev->quirks);
> +
>       return 0;
>   }
>
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c 
> index d31edad7a056..52561c8d8828 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -142,6 +142,10 @@ static const struct usb_device_id btusb_table[] = {
>       { USB_VENDOR_AND_INTERFACE_INFO(0x04ca, 0xff, 0x01, 0x01),
>         .driver_info = BTUSB_BCM_PATCHRAM },
>
> +     /* Cypress devices with vendor specific id */
> +     { USB_VENDOR_AND_INTERFACE_INFO(0x04b4, 0xff, 0x01, 0x01),
> +       .driver_info = BTUSB_BCM_PATCHRAM },
> +
>       /* Broadcom devices with vendor specific id */
>       { USB_VENDOR_AND_INTERFACE_INFO(0x0a5c, 0xff, 0x01, 0x01),
>         .driver_info = BTUSB_BCM_PATCHRAM },