Hi Marcel,
On Wed, Jul 09, 2014, Marcel Holtmann wrote:
> > When we're removing the last item in the white list or adding the first
> > one to it and HCI_CONNECTABLE is not set we need to update the current
> > page scan. This patch adds a simple helper function for the purpose and
> > calls it from the respective mgmt command handlers.
> >
> > Signed-off-by: Johan Hedberg <johan.hedberg@xxxxxxxxx>
> > ---
> > net/bluetooth/mgmt.c | 31 +++++++++++++++++++++++++++++++
> > 1 file changed, 31 insertions(+)
> >
> > diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
> > index 431d5e4aa34b..f5f0763266e0 100644
> > --- a/net/bluetooth/mgmt.c
> > +++ b/net/bluetooth/mgmt.c
> > @@ -5198,6 +5198,24 @@ unlock:
> > return err;
> > }
> >
> > +/* Helper for Add/Remove Device commands */
> > +static void update_page_scan(struct hci_dev *hdev, u8 scan)
> > +{
> > + if (!test_bit(HCI_BREDR_ENABLED, &hdev->dev_flags))
> > + return;
> > +
> > + if (!hdev_is_powered(hdev))
> > + return;
> > +
> > + if (test_bit(HCI_CONNECTABLE, &hdev->dev_flags))
> > + return;
>
> wouldn't it be better to check the HCI_PSCAN flag here to see if page
> scan is actually enabled on the controller?
Maybe I was a bit too smart here, but the code is actually correct. We
also call update_page_scan() with SCAN_DISABLE (0x00) for the
remove_device case and there if HCI_CONNECTABLE is set we should return
from the function. Checking for HCI_PSCAN would do the wrong thing.
I can leave the patch as-is, or add a bit more complex logic there:
if (PSCAN && scan)
return;
if (!PSCAN && !scan)
return;
Let me know what you prefer.
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
