* [PATCH v2] Patch for CYW4373 hci up fail issue @ 2024-05-20 8:22 Nobuaki.Tsunashima 2024-05-20 8:58 ` [v2] " bluez.test.bot 2024-05-20 16:52 ` [PATCH v2] " Paul Menzel 0 siblings, 2 replies; 5+ messages in thread From: Nobuaki.Tsunashima @ 2024-05-20 8:22 UTC (permalink / raw) To: marcel, luiz.dentz; +Cc: linux-bluetooth 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. 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. 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 }, -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* RE: [v2] Patch for CYW4373 hci up fail issue 2024-05-20 8:22 [PATCH v2] Patch for CYW4373 hci up fail issue Nobuaki.Tsunashima @ 2024-05-20 8:58 ` bluez.test.bot 2024-05-20 16:52 ` [PATCH v2] " Paul Menzel 1 sibling, 0 replies; 5+ messages in thread From: bluez.test.bot @ 2024-05-20 8:58 UTC (permalink / raw) To: linux-bluetooth, Nobuaki.Tsunashima [-- Attachment #1: Type: text/plain, Size: 5691 bytes --] This is automated email and please do not reply to this email! Dear submitter, Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series: PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=854351 ---Test result--- Test Summary: CheckPatch FAIL 1.29 seconds GitLint FAIL 0.54 seconds SubjectPrefix FAIL 0.35 seconds BuildKernel PASS 31.19 seconds CheckAllWarning PASS 34.29 seconds CheckSparse PASS 39.28 seconds CheckSmatch FAIL 36.39 seconds BuildKernel32 PASS 29.13 seconds TestRunnerSetup PASS 534.76 seconds TestRunner_l2cap-tester PASS 18.82 seconds TestRunner_iso-tester FAIL 33.51 seconds TestRunner_bnep-tester PASS 4.84 seconds TestRunner_mgmt-tester FAIL 110.56 seconds TestRunner_rfcomm-tester PASS 7.54 seconds TestRunner_sco-tester PASS 15.07 seconds TestRunner_ioctl-tester PASS 8.13 seconds TestRunner_mesh-tester PASS 6.02 seconds TestRunner_smp-tester PASS 7.20 seconds TestRunner_userchan-tester PASS 4.96 seconds IncrementalBuild PASS 28.38 seconds Details ############################## Test: CheckPatch - FAIL Desc: Run checkpatch.pl script Output: [v2] Patch for CYW4373 hci up fail issue WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?) #80: CYW4373 ROM FW has an issue that it claims LE_Read_Transmit_Power command as supported in a response ERROR: trailing whitespace #116: FILE: drivers/bluetooth/btbcm.c:452: +^Iconst struct bcm_chip_version_table *entry = $ WARNING: line length of 107 exceeds 100 columns #117: FILE: drivers/bluetooth/btbcm.c:453: + &disable_broken_read_transmit_power_by_chip_ver[0]; ERROR: that open brace { should be on the previous line #119: FILE: drivers/bluetooth/btbcm.c:455: + for (i = 0 ; i < table_size ; i++, entry++) + { WARNING: From:/Signed-off-by: email name mismatch: 'From: Nobuaki.Tsunashima@infineon.com' != 'Signed-off-by: Nobuaki Tsunashima <nobuaki.tsunashima@infineon.com>' total: 2 errors, 3 warnings, 60 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. NOTE: Whitespace errors detected. You may wish to use scripts/cleanpatch or scripts/cleanfile /github/workspace/src/src/13668129.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS. ############################## Test: GitLint - FAIL Desc: Run gitlint Output: [v2] Patch for CYW4373 hci up fail issue WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 3: B1 Line exceeds max length (100>80): "CYW4373 ROM FW has an issue that it claims LE_Read_Transmit_Power command as supported in a response" 4: B1 Line exceeds max length (91>80): "of Read_Local_Supported_Command command but rejects the LE_Read_Transmit_Power command with" 7: B1 Line exceeds max length (110>80): "Especially in USB i/f case, it would be difficult to download patch FW that includes Its fix unless hci is up." 8: B1 Line exceeds max length (99>80): "The patch forces the driver to skip LE_Read_Transmit_Power Command when it detects CYW4373 with ROM" ############################## Test: SubjectPrefix - FAIL Desc: Check subject contains "Bluetooth" prefix Output: "Bluetooth: " prefix is not specified in the subject ############################## Test: CheckSmatch - FAIL Desc: Run smatch tool with source Output: Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139 make[4]: *** Deleting file 'net/bluetooth/hci_core.o' make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2 make[2]: *** [scripts/Makefile.build:485: net] Error 2 make[2]: *** Waiting for unfinished jobs.... Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139 make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o' make[4]: *** Waiting for unfinished jobs.... Segmentation fault (core dumped) make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bpa10x.o] Error 139 make[4]: *** Deleting file 'drivers/bluetooth/bpa10x.o' make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2 make[2]: *** [scripts/Makefile.build:485: drivers] Error 2 make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2 make: *** [Makefile:240: __sub-make] Error 2 ############################## Test: TestRunner_iso-tester - FAIL Desc: Run iso-tester with test-runner Output: Total: 122, Passed: 117 (95.9%), Failed: 1, Not Run: 4 Failed Test Cases ISO Connect2 Suspend - Success Failed 4.239 seconds ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 492, Passed: 489 (99.4%), Failed: 1, Not Run: 2 Failed Test Cases LL Privacy - Remove Device 4 (Disable Adv) Timed out 2.734 seconds --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Patch for CYW4373 hci up fail issue 2024-05-20 8:22 [PATCH v2] Patch for CYW4373 hci up fail issue Nobuaki.Tsunashima 2024-05-20 8:58 ` [v2] " bluez.test.bot @ 2024-05-20 16:52 ` Paul Menzel 2024-05-21 1:43 ` Nobuaki.Tsunashima 1 sibling, 1 reply; 5+ messages in thread From: Paul Menzel @ 2024-05-20 16:52 UTC (permalink / raw) To: Nobuaki Tsunashima; +Cc: marcel, luiz.dentz, linux-bluetooth 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 }, ^ permalink raw reply [flat|nested] 5+ messages in thread
* RE: [PATCH v2] Patch for CYW4373 hci up fail issue 2024-05-20 16:52 ` [PATCH v2] " Paul Menzel @ 2024-05-21 1:43 ` Nobuaki.Tsunashima 2024-05-22 4:19 ` Paul Menzel 0 siblings, 1 reply; 5+ messages in thread From: Nobuaki.Tsunashima @ 2024-05-21 1:43 UTC (permalink / raw) To: pmenzel; +Cc: marcel, luiz.dentz, linux-bluetooth 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 }, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2] Patch for CYW4373 hci up fail issue 2024-05-21 1:43 ` Nobuaki.Tsunashima @ 2024-05-22 4:19 ` Paul Menzel 0 siblings, 0 replies; 5+ messages in thread From: Paul Menzel @ 2024-05-22 4:19 UTC (permalink / raw) To: Nobuaki Tsunashima; +Cc: marcel, luiz.dentz, linux-bluetooth Dear Nobuaki, Thank you for your reply. Am 21.05.24 um 03:43 schrieb Nobuaki.Tsunashima@infineon.com: >>> 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. As you have the test setup, and bisecting is normally very quick, it’d be great if you could pinpoint the commit causing the regression. […] Kind regards, Paul ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-05-22 4:19 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-05-20 8:22 [PATCH v2] Patch for CYW4373 hci up fail issue Nobuaki.Tsunashima 2024-05-20 8:58 ` [v2] " bluez.test.bot 2024-05-20 16:52 ` [PATCH v2] " Paul Menzel 2024-05-21 1:43 ` Nobuaki.Tsunashima 2024-05-22 4:19 ` Paul Menzel
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox