On Wed, 2018-07-18 at 14:14 +0200, Marcel Holtmann wrote: > Hi Sean, > > >>>>> Add a new quirk HCI_QUIRK_NON_PERSISTENT_SETUP allowing that a quirk > >>>>> that > >>>>> runs setup() after every open() and not just after the first open(). > >>>>> > >>>>> Signed-off-by: Sean Wang <[email protected]> > >>>>> --- > >>>>> include/net/bluetooth/hci.h | 9 +++++++++ > >>>>> net/bluetooth/hci_core.c | 3 ++- > >>>>> 2 files changed, 11 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h > >>>>> index 73e48be..d3ec5b2a8 100644 > >>>>> --- a/include/net/bluetooth/hci.h > >>>>> +++ b/include/net/bluetooth/hci.h > >>>>> @@ -183,6 +183,15 @@ enum { > >>>>> * during the hdev->setup vendor callback. > >>>>> */ > >>>>> HCI_QUIRK_NON_PERSISTENT_DIAG, > >>>>> + > >>>>> + /* When this quirk is set, setup() would be run after every > >>>>> + * open() and not just after the first open(). > >>>>> + * > >>>>> + * This quirk can be set before hci_register_dev is called or > >>>>> + * during the hdev->setup vendor callback. > >>>>> + * > >>>>> + */ > >>>>> + HCI_QUIRK_NON_PERSISTENT_SETUP, > >>>>> }; > >>>>> > >>>>> /* HCI device flags */ > >>>>> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > >>>>> index f5c21004..0111280 100644 > >>>>> --- a/net/bluetooth/hci_core.c > >>>>> +++ b/net/bluetooth/hci_core.c > >>>>> @@ -1396,7 +1396,8 @@ static int hci_dev_do_open(struct hci_dev *hdev) > >>>>> atomic_set(&hdev->cmd_cnt, 1); > >>>>> set_bit(HCI_INIT, &hdev->flags); > >>>>> > >>>>> - if (hci_dev_test_flag(hdev, HCI_SETUP)) { > >>>>> + if (hci_dev_test_flag(hdev, HCI_SETUP) || > >>>>> + test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks)) { > >>>>> hci_sock_dev_event(hdev, HCI_DEV_SETUP); > >>>> > >>>> can you include the extract of btmon on how the HCI_DEV_SETUP event > >>>> shows up in the traces? I remember that I asked about something like > >>>> that. > >>>> > >>>> Regards > >>>> > >>>> Marcel > >>>> > >>> > >>> No, I cannot see any event about HCI_DEV_SETUP in btmon trace, the trace > >>> I posted as below is doing some rounds of power off and then on > >> > >> it will result in this: > >> > >> case HCI_DEV_SETUP: > >> if (hdev->manufacturer == 0xffff) > >> return NULL; > >> > >> case HCI_DEV_UP: > >> skb = bt_skb_alloc(HCI_MON_INDEX_INFO_SIZE, GFP_ATOMIC); > >> > >> So we will see extra index info events, but I assume that is just fine > >> this we also see them on DEV_UP. They also do not hurt as long as not > >> magically the manufacturer information changes. > >> > >> I do wonder though if this quirk is set, then hdev->manufacturer needs to > >> be reset and allow hdev->setup() to reset it. This goes with a log of > >> other information as well. Maybe just a look if there are no races here. > >> > >> Regards > >> > >> Marcel > >> > > > > I didn't set a value to hdev->manufacture. > > > > Should I set it in hdev->setup() ? MediaTek is 0x46 according to [1]. > > it is generally useful so that it gets reported out in btmon and thus it can > assign the correct vendor specific decoder. However you better set > hdev->manufacturer = 70 in the probe() routine before calling > hci_register_dev. You have a dedicated driver and thus will always know it. > > The core tries to figure this out at some point, but then you are missing > btmon decoding for the earlier messages. See btusb.c and how other drivers > set it early on. >
thank for the point up, I will set hdev->manufacturer as 70 in probe routine before calling hci_register_dev in the next version. > Regards > > Marcel > > > _______________________________________________ > Linux-mediatek mailing list > [email protected] > http://lists.infradead.org/mailman/listinfo/linux-mediatek
