Hi Marcel,
On Wed, Dec 14, 2011, Marcel Holtmann wrote:
> > +static void service_cache_off(struct work_struct *work)
> > +{
> > + struct hci_dev *hdev = container_of(work, struct hci_dev,
> > + service_cache.work);
> > +
> > + if (!test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
> > + return;
> > +
> > + hci_dev_lock_bh(hdev);
> > +
> > + update_eir(hdev);
> > + update_class(hdev);
> > +
> > + hci_dev_unlock_bh(hdev);
> > +}
> > +
> > +static void mgmt_init_hdev(struct hci_dev *hdev)
> > +{
> > + if (!test_and_set_bit(HCI_MGMT, &hdev->flags))
> > + INIT_DELAYED_WORK(&hdev->service_cache, service_cache_off);
>
> Why does this depend on MGMT being enabled? Why do we bother here? Why
> not just always initialize it when registering the controller.
Because the timer callback function is static and resides within mgmt.c
(since it needs to call update_class & update_eir which are also static
within mgmt.c).
> > - if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags))
> > + if (test_and_clear_bit(HCI_SERVICE_CACHE, &hdev->flags)) {
> > + hci_dev_unlock_bh(hdev);
> > + cancel_delayed_work_sync(&hdev->service_cache);
> > + hci_dev_lock_bh(hdev);
> > update_eir(hdev);
> > + }
>
> We have to be a bit careful here since essentially the service cache
> (actually UUID cache to be precise) is a per mgmt socket feature and not
> a per hdev feature.
>
> I have nothing against merging this first, but keep in mind that this
> needs to be updated since we can have two concurrent applications
> opening a mgmt socket.
I understand. It does get a bit tricky though since the UUID list is per
HCI device. I.e. do we keep our own list for each socket and hide it
from struct hci_dev until the (per-socket) cache gets disabled?
Johan
--
To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
