| Message ID | 20211216033453.9806-1-hj.tedd.an@gmail.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
| Series | [RFC] Bluetooth: btintel: Fix broken LED quirk for legacy ROM device | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/checkpatch | success | Checkpatch PASS |
| tedd_an/gitlint | success | Gitlint PASS |
| tedd_an/buildkernel | success | Build Kernel PASS |
| tedd_an/incremental_build | success | Pass |
| tedd_an/testrunnersetup | success | Test Runner Setup PASS |
| tedd_an/testrunnerl2cap-tester | success | Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerbnep-tester | success | Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnermgmt-tester | success | Total: 493, Passed: 493 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnerrfcomm-tester | success | Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersco-tester | success | Total: 12, Passed: 12 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunnersmp-tester | success | Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0 |
| tedd_an/testrunneruserchan-tester | success | Total: 4, Passed: 4 (100.0%), Failed: 0, Not Run: 0 |
Dear Tedd, Am 16.12.21 um 04:34 schrieb Tedd Ho-Jeong An: > From: Tedd Ho-Jeong An <tedd.an@intel.com> > > This patch fix the broken LED quirk for Intel legacy ROM devices. fix*es* Excuse my ignorance, but what are Intel legacy ROM devices? Maybe also mention the IDs 0x0a2a and 0x0aa7 in the commit message? > Legacy device sends the SW RFKILL while shutting down the device (like > HCI interface down) to turn off the LED by putting the device in assert. > > Once the SW RFKILL is on, it needs the HCI_Reset to exit from the SW > RFKILL state. This patch checks the quirk and send the HCI_Reset before send*s* > sending the HCI_Intel_Read_Version command. Is that document in some datasheet/specification? If so, please mention it. Also, in the commit message summary you write “Fix”. If it fixes an old commit, please add a Fixes: tag. > Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com> > --- > drivers/bluetooth/btintel.c | 13 ++++++------- > drivers/bluetooth/btusb.c | 10 ++++++++-- > 2 files changed, 14 insertions(+), 9 deletions(-) I have a Dell Latitude E7250 with the 8087:0a2a. How can I test this? > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > index 8f9109b40961..0a5aa637bf6f 100644 > --- a/drivers/bluetooth/btintel.c > +++ b/drivers/bluetooth/btintel.c > @@ -2355,8 +2355,13 @@ static int btintel_setup_combined(struct hci_dev *hdev) > * As a workaround, send HCI Reset command first which will reset the > * number of completed commands and allow normal command processing > * from now on. > + * > + * For INTEL_BROKEN_LED, these devices have an issue with LED which > + * doesn't go off immediately during shutdown. Set the flag > + * here to send the LED OFF command during shutdown. Please reflow all lines for the allowed line length limit (doesn’t fits on the line above). > */ > - if (btintel_test_flag(hdev, INTEL_BROKEN_INITIAL_NCMD)) { > + if (btintel_test_flag(hdev, INTEL_BROKEN_INITIAL_NCMD) || > + btintel_test_flag(hdev, INTEL_BROKEN_LED)) { > skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, > HCI_INIT_TIMEOUT); > if (IS_ERR(skb)) { > @@ -2428,12 +2433,6 @@ static int btintel_setup_combined(struct hci_dev *hdev) > set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, > &hdev->quirks); > > - /* These devices have an issue with LED which doesn't > - * go off immediately during shutdown. Set the flag > - * here to send the LED OFF command during shutdown. > - */ > - btintel_set_flag(hdev, INTEL_BROKEN_LED); > - > err = btintel_legacy_rom_setup(hdev, &ver); > break; > case 0x0b: /* SfP */ > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > index d1bd9ee0a6ab..c6a070d5284f 100644 > --- a/drivers/bluetooth/btusb.c > +++ b/drivers/bluetooth/btusb.c > @@ -60,6 +60,7 @@ static struct usb_driver btusb_driver; > #define BTUSB_WIDEBAND_SPEECH 0x400000 > #define BTUSB_VALID_LE_STATES 0x800000 > #define BTUSB_QCA_WCN6855 0x1000000 > +#define BTUSB_INTEL_BROKEN_LED 0x2000000 > #define BTUSB_INTEL_BROKEN_INITIAL_NCMD 0x4000000 > > static const struct usb_device_id btusb_table[] = { > @@ -382,9 +383,11 @@ static const struct usb_device_id blacklist_table[] = { > { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR }, > { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED | > BTUSB_INTEL_BROKEN_INITIAL_NCMD }, > - { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED }, > + { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED | > + BTUSB_INTEL_BROKEN_LED }, > { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_COMBINED }, > - { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED }, > + { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED | > + BTUSB_INTEL_BROKEN_LED }, > { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_COMBINED }, > > /* Other Intel Bluetooth devices */ > @@ -3724,6 +3727,9 @@ static int btusb_probe(struct usb_interface *intf, > > if (id->driver_info & BTUSB_INTEL_BROKEN_INITIAL_NCMD) > btintel_set_flag(hdev, INTEL_BROKEN_INITIAL_NCMD); > + > + if (id->driver_info & BTUSB_INTEL_BROKEN_LED) > + btintel_set_flag(hdev, INTEL_BROKEN_LED); > } > > if (id->driver_info & BTUSB_MARVELL)
Hi Paul On Thu, 2021-12-16 at 08:52 +0100, Paul Menzel wrote: > Dear Tedd, > > > Am 16.12.21 um 04:34 schrieb Tedd Ho-Jeong An: > > From: Tedd Ho-Jeong An <tedd.an@intel.com> > > > > This patch fix the broken LED quirk for Intel legacy ROM devices. > > fix*es* > > Excuse my ignorance, but what are Intel legacy ROM devices? Maybe also > mention the IDs 0x0a2a and 0x0aa7 in the commit message? > > > Legacy device sends the SW RFKILL while shutting down the device (like > > HCI interface down) to turn off the LED by putting the device in assert. > > > > Once the SW RFKILL is on, it needs the HCI_Reset to exit from the SW > > RFKILL state. This patch checks the quirk and send the HCI_Reset before > > send*s* > > > sending the HCI_Intel_Read_Version command. > > Is that document in some datasheet/specification? If so, please mention it. > > Also, in the commit message summary you write “Fix”. If it fixes an old > commit, please add a Fixes: tag. > > > Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com> > > --- > > drivers/bluetooth/btintel.c | 13 ++++++------- > > drivers/bluetooth/btusb.c | 10 ++++++++-- > > 2 files changed, 14 insertions(+), 9 deletions(-) > > I have a Dell Latitude E7250 with the 8087:0a2a. How can I test this? Apply this patch on top of the bluetooth-next (or current mainline) The BT interface should be available after reboot. > > > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c > > index 8f9109b40961..0a5aa637bf6f 100644 > > --- a/drivers/bluetooth/btintel.c > > +++ b/drivers/bluetooth/btintel.c > > @@ -2355,8 +2355,13 @@ static int btintel_setup_combined(struct hci_dev *hdev) > > * As a workaround, send HCI Reset command first which will reset the > > * number of completed commands and allow normal command processing > > * from now on. > > + * > > + * For INTEL_BROKEN_LED, these devices have an issue with LED which > > + * doesn't go off immediately during shutdown. Set the flag > > + * here to send the LED OFF command during shutdown. > > Please reflow all lines for the allowed line length limit (doesn’t fits > on the line above). > > > */ > > - if (btintel_test_flag(hdev, INTEL_BROKEN_INITIAL_NCMD)) { > > + if (btintel_test_flag(hdev, INTEL_BROKEN_INITIAL_NCMD) || > > + btintel_test_flag(hdev, INTEL_BROKEN_LED)) { > > skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, > > HCI_INIT_TIMEOUT); > > if (IS_ERR(skb)) { > > @@ -2428,12 +2433,6 @@ static int btintel_setup_combined(struct hci_dev *hdev) > > set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, > > &hdev->quirks); > > > > - /* These devices have an issue with LED which doesn't > > - * go off immediately during shutdown. Set the flag > > - * here to send the LED OFF command during shutdown. > > - */ > > - btintel_set_flag(hdev, INTEL_BROKEN_LED); > > - > > err = btintel_legacy_rom_setup(hdev, &ver); > > break; > > case 0x0b: /* SfP */ > > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c > > index d1bd9ee0a6ab..c6a070d5284f 100644 > > --- a/drivers/bluetooth/btusb.c > > +++ b/drivers/bluetooth/btusb.c > > @@ -60,6 +60,7 @@ static struct usb_driver btusb_driver; > > #define BTUSB_WIDEBAND_SPEECH 0x400000 > > #define BTUSB_VALID_LE_STATES 0x800000 > > #define BTUSB_QCA_WCN6855 0x1000000 > > +#define BTUSB_INTEL_BROKEN_LED 0x2000000 > > #define BTUSB_INTEL_BROKEN_INITIAL_NCMD 0x4000000 > > > > static const struct usb_device_id btusb_table[] = { > > @@ -382,9 +383,11 @@ static const struct usb_device_id blacklist_table[] = { > > { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR }, > > { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED | > > BTUSB_INTEL_BROKEN_INITIAL_NCMD }, > > - { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED }, > > + { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED | > > + BTUSB_INTEL_BROKEN_LED }, > > { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_COMBINED }, > > - { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED }, > > + { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED | > > + BTUSB_INTEL_BROKEN_LED }, > > { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_COMBINED }, > > > > /* Other Intel Bluetooth devices */ > > @@ -3724,6 +3727,9 @@ static int btusb_probe(struct usb_interface *intf, > > > > if (id->driver_info & BTUSB_INTEL_BROKEN_INITIAL_NCMD) > > btintel_set_flag(hdev, INTEL_BROKEN_INITIAL_NCMD); > > + > > + if (id->driver_info & BTUSB_INTEL_BROKEN_LED) > > + btintel_set_flag(hdev, INTEL_BROKEN_LED); > > } > > > > if (id->driver_info & BTUSB_MARVELL) Regards, Tedd
Dear Tedd, Am 16.12.21 um 19:38 schrieb An, Tedd: > On Thu, 2021-12-16 at 08:52 +0100, Paul Menzel wrote: >> Am 16.12.21 um 04:34 schrieb Tedd Ho-Jeong An: >>> From: Tedd Ho-Jeong An <tedd.an@intel.com> >>> >>> This patch fix the broken LED quirk for Intel legacy ROM devices. >> >> fix*es* >> >> Excuse my ignorance, but what are Intel legacy ROM devices? Maybe also >> mention the IDs 0x0a2a and 0x0aa7 in the commit message? >> >>> Legacy device sends the SW RFKILL while shutting down the device (like >>> HCI interface down) to turn off the LED by putting the device in assert. >>> >>> Once the SW RFKILL is on, it needs the HCI_Reset to exit from the SW >>> RFKILL state. This patch checks the quirk and send the HCI_Reset before >> >> send*s* >> >>> sending the HCI_Intel_Read_Version command. >> >> Is that document in some datasheet/specification? If so, please mention it. >> >> Also, in the commit message summary you write “Fix”. If it fixes an old >> commit, please add a Fixes: tag. >> >>> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com> >>> --- >>> drivers/bluetooth/btintel.c | 13 ++++++------- >>> drivers/bluetooth/btusb.c | 10 ++++++++-- >>> 2 files changed, 14 insertions(+), 9 deletions(-) >> >> I have a Dell Latitude E7250 with the 8087:0a2a. How can I test this? > > Apply this patch on top of the bluetooth-next (or current mainline) > The BT interface should be available after reboot. On the laptop, I do not have any problems with missing BT interface after reboot with Linux 5.16-rc5. Are not all devices affected? […] Kind regards, Paul
Hi Paul, On Fri, Dec 17, 2021 at 7:33 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote: > > Dear Tedd, > > > Am 16.12.21 um 19:38 schrieb An, Tedd: > > > On Thu, 2021-12-16 at 08:52 +0100, Paul Menzel wrote: > > >> Am 16.12.21 um 04:34 schrieb Tedd Ho-Jeong An: > >>> From: Tedd Ho-Jeong An <tedd.an@intel.com> > >>> > >>> This patch fix the broken LED quirk for Intel legacy ROM devices. > >> > >> fix*es* > >> > >> Excuse my ignorance, but what are Intel legacy ROM devices? Maybe also > >> mention the IDs 0x0a2a and 0x0aa7 in the commit message? > >> > >>> Legacy device sends the SW RFKILL while shutting down the device (like > >>> HCI interface down) to turn off the LED by putting the device in assert. > >>> > >>> Once the SW RFKILL is on, it needs the HCI_Reset to exit from the SW > >>> RFKILL state. This patch checks the quirk and send the HCI_Reset before > >> > >> send*s* > >> > >>> sending the HCI_Intel_Read_Version command. > >> > >> Is that document in some datasheet/specification? If so, please mention it. > >> > >> Also, in the commit message summary you write “Fix”. If it fixes an old > >> commit, please add a Fixes: tag. > >> > >>> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com> > >>> --- > >>> drivers/bluetooth/btintel.c | 13 ++++++------- > >>> drivers/bluetooth/btusb.c | 10 ++++++++-- > >>> 2 files changed, 14 insertions(+), 9 deletions(-) > >> > >> I have a Dell Latitude E7250 with the 8087:0a2a. How can I test this? > > > > Apply this patch on top of the bluetooth-next (or current mainline) > > The BT interface should be available after reboot. > > On the laptop, I do not have any problems with missing BT interface > after reboot with Linux 5.16-rc5. Are not all devices affected? If the problem is fixed for you please confirm it by replying with a Tested-by line.
Dear Luiz, dear Tedd, Am 17.12.21 um 20:27 schrieb Luiz Augusto von Dentz: > On Fri, Dec 17, 2021 at 7:33 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote: >> Am 16.12.21 um 19:38 schrieb An, Tedd: >> >>> On Thu, 2021-12-16 at 08:52 +0100, Paul Menzel wrote: >> >>>> Am 16.12.21 um 04:34 schrieb Tedd Ho-Jeong An: >>>>> From: Tedd Ho-Jeong An <tedd.an@intel.com> >>>>> >>>>> This patch fix the broken LED quirk for Intel legacy ROM devices. >>>> >>>> fix*es* >>>> >>>> Excuse my ignorance, but what are Intel legacy ROM devices? Maybe also >>>> mention the IDs 0x0a2a and 0x0aa7 in the commit message? >>>> >>>>> Legacy device sends the SW RFKILL while shutting down the device (like >>>>> HCI interface down) to turn off the LED by putting the device in assert. >>>>> >>>>> Once the SW RFKILL is on, it needs the HCI_Reset to exit from the SW >>>>> RFKILL state. This patch checks the quirk and send the HCI_Reset before >>>> >>>> send*s* >>>> >>>>> sending the HCI_Intel_Read_Version command. >>>> >>>> Is that document in some datasheet/specification? If so, please mention it. >>>> >>>> Also, in the commit message summary you write “Fix”. If it fixes an old >>>> commit, please add a Fixes: tag. >>>> >>>>> Signed-off-by: Tedd Ho-Jeong An <tedd.an@intel.com> >>>>> --- >>>>> drivers/bluetooth/btintel.c | 13 ++++++------- >>>>> drivers/bluetooth/btusb.c | 10 ++++++++-- >>>>> 2 files changed, 14 insertions(+), 9 deletions(-) >>>> >>>> I have a Dell Latitude E7250 with the 8087:0a2a. How can I test this? >>> >>> Apply this patch on top of the bluetooth-next (or current mainline) >>> The BT interface should be available after reboot. >> >> On the laptop, I do not have any problems with missing BT interface >> after reboot with Linux 5.16-rc5. Are not all devices affected? > > If the problem is fixed for you please confirm it by replying with a > Tested-by line. Sorry, there seems to be a misunderstanding. I do not know what the problem is in the first place. On the Dell Latitude E7250 with Linux 5.16-rc5 the Bluetooth interface shows up after a warm reboot just fine. Kind regards, Paul
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c index 8f9109b40961..0a5aa637bf6f 100644 --- a/drivers/bluetooth/btintel.c +++ b/drivers/bluetooth/btintel.c @@ -2355,8 +2355,13 @@ static int btintel_setup_combined(struct hci_dev *hdev) * As a workaround, send HCI Reset command first which will reset the * number of completed commands and allow normal command processing * from now on. + * + * For INTEL_BROKEN_LED, these devices have an issue with LED which + * doesn't go off immediately during shutdown. Set the flag + * here to send the LED OFF command during shutdown. */ - if (btintel_test_flag(hdev, INTEL_BROKEN_INITIAL_NCMD)) { + if (btintel_test_flag(hdev, INTEL_BROKEN_INITIAL_NCMD) || + btintel_test_flag(hdev, INTEL_BROKEN_LED)) { skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT); if (IS_ERR(skb)) { @@ -2428,12 +2433,6 @@ static int btintel_setup_combined(struct hci_dev *hdev) set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks); - /* These devices have an issue with LED which doesn't - * go off immediately during shutdown. Set the flag - * here to send the LED OFF command during shutdown. - */ - btintel_set_flag(hdev, INTEL_BROKEN_LED); - err = btintel_legacy_rom_setup(hdev, &ver); break; case 0x0b: /* SfP */ diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c index d1bd9ee0a6ab..c6a070d5284f 100644 --- a/drivers/bluetooth/btusb.c +++ b/drivers/bluetooth/btusb.c @@ -60,6 +60,7 @@ static struct usb_driver btusb_driver; #define BTUSB_WIDEBAND_SPEECH 0x400000 #define BTUSB_VALID_LE_STATES 0x800000 #define BTUSB_QCA_WCN6855 0x1000000 +#define BTUSB_INTEL_BROKEN_LED 0x2000000 #define BTUSB_INTEL_BROKEN_INITIAL_NCMD 0x4000000 static const struct usb_device_id btusb_table[] = { @@ -382,9 +383,11 @@ static const struct usb_device_id blacklist_table[] = { { USB_DEVICE(0x8087, 0x07da), .driver_info = BTUSB_CSR }, { USB_DEVICE(0x8087, 0x07dc), .driver_info = BTUSB_INTEL_COMBINED | BTUSB_INTEL_BROKEN_INITIAL_NCMD }, - { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED }, + { USB_DEVICE(0x8087, 0x0a2a), .driver_info = BTUSB_INTEL_COMBINED | + BTUSB_INTEL_BROKEN_LED }, { USB_DEVICE(0x8087, 0x0a2b), .driver_info = BTUSB_INTEL_COMBINED }, - { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED }, + { USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL_COMBINED | + BTUSB_INTEL_BROKEN_LED }, { USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_COMBINED }, /* Other Intel Bluetooth devices */ @@ -3724,6 +3727,9 @@ static int btusb_probe(struct usb_interface *intf, if (id->driver_info & BTUSB_INTEL_BROKEN_INITIAL_NCMD) btintel_set_flag(hdev, INTEL_BROKEN_INITIAL_NCMD); + + if (id->driver_info & BTUSB_INTEL_BROKEN_LED) + btintel_set_flag(hdev, INTEL_BROKEN_LED); } if (id->driver_info & BTUSB_MARVELL)