Re: [RFC PATCH] Bluetooth: btintel: Fix broken LED quirk for legacy ROM device

["An, Tedd" <tedd.an@intel.com>]

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