Hi Mattijs, > Some HCI devices which have the HCI_QUIRK_NON_PERSISTENT_SETUP [1] > require a call to setup() to be ran after every open(). > > During the setup() stage, these devices expect the chip to acknowledge > its setup() completion via vendor specific frames. > > If userspace opens() such HCI device in HCI_USER_CHANNEL [2] mode, > the vendor specific frames are never tranmitted to the driver, as > they are filtered in hci_rx_work(). > > Allow HCI devices which have HCI_QUIRK_NON_PERSISTENT_SETUP to process > frames if the HCI device is is HCI_INIT state. > > [1] https://lore.kernel.org/patchwork/patch/965071/ > [2] https://www.spinics.net/lists/linux-bluetooth/msg37345.html > > Fixes: 740011cfe948 ("Bluetooth: Add new quirk for non-persistent setup settings") > Signed-off-by: Mattijs Korpershoek <mkorpershoek@xxxxxxxxxxxx> > --- > Some more background on the change follows: > > The Android bluetooth stack (Bluedroid) also has a HAL implementation > which follows Linux's standard rfkill interface [1]. > > This implementation relies on the HCI_CHANNEL_USER feature to get > exclusive access to the underlying bluetooth device. > > When testing this along with the btkmtksdio driver, the > chip appeared unresponsive when calling the following from userspace: > > struct sockaddr_hci addr; > int fd; > > fd = socket(AF_BLUETOOTH, SOCK_RAW, BTPROTO_HCI); > > memset(&addr, 0, sizeof(addr)); > addr.hci_family = AF_BLUETOOTH; > addr.hci_dev = 0; > addr.hci_channel = HCI_CHANNEL_USER; > > bind(fd, (struct sockaddr *) &addr, sizeof(addr)); # device hangs > > In the case of bluetooth drivers exposing QUIRK_NON_PERSISTENT_SETUP > such as btmtksdio, setup() is called each multiple times. > In particular, when userspace calls bind(), the setup() is called again > and vendor specific commands might be send to re-initialize the chip. > > Those commands are filtered out by hci_core in HCI_CHANNEL_USER mode, > preventing setup() from completing successfully. > > This has been tested on a 4.19 kernel based on Android Common Kernel. > It has also been compile tested on bluetooth-next. > > [1] https://android.googlesource.com/platform/system/bt/+/refs/heads/master/vendor_libs/linux/interface/ > > net/bluetooth/hci_core.c | 15 +++++++++++++-- > 1 file changed, 13 insertions(+), 2 deletions(-) > > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index 04bc79359a17..5f12e8574d54 100644 > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -4440,9 +4440,20 @@ static void hci_rx_work(struct work_struct *work) > hci_send_to_sock(hdev, skb); > } > > + /* If the device has been opened in HCI_USER_CHANNEL, > + * the userspace has exclusive access to device. > + * When HCI_QUIRK_NON_PERSISTENT_SETUP is set and > + * device is HCI_INIT, we still need to process > + * the data packets to the driver in order > + * to complete its setup(). > + */ > if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL)) { > - kfree_skb(skb); > - continue; > + if (!test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, > + &hdev->quirks) || > + !test_bit(HCI_INIT, &hdev->flags)) { > + kfree_skb(skb); > + continue; > + } > } if (hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && !test_bit(HCI_INIT, &hdev->flags)) { kfree_skb(skb); continue; } Wouldn’t it be enough to just add a check for HCI_INIT to this. I mean it makes no difference if ->setup is repeated on each device open or not. We want to process event during HCI_INIT when in user channel mode. Regards Marcel
