Re: [PATCH] Bluetooth: Don't force DISCOVERY_STOPPED state in inquiry_cache_flush

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Andre,

On Fri, Mar 02, 2012, Andre Guedes wrote:
> > On Thu, Mar 01, 2012, Andre Guedes wrote:
> >> We are not supposed to force DISCOVERY_STOPPED in inquiry_cache_flush
> >> because we may break the discovery state machine. For instance,
> >> during interleaved discovery, when we are about to start inquiry,
> >> the state machine forcibly goes to DISCOVERY_STOPPED while it
> >> should stay in DISCOVERY_FINDING state.
> >>
> >> This problem results in unexpected behaviors such as sending two
> >> mgmt_discovering events to userspace (when only one event is
> >> expected) and Stop Discovery failures.
> >>
> >> Signed-off-by: Andre Guedes <andre.guedes@openbossa.org>
> >> ---
> >>  net/bluetooth/hci_core.c |    1 -
> >>  1 file changed, 1 deletion(-)
> >>
> >> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> >> index d3ddc0b..661d65f 100644
> >> --- a/net/bluetooth/hci_core.c
> >> +++ b/net/bluetooth/hci_core.c
> >> @@ -413,7 +413,6 @@ static void inquiry_cache_flush(struct hci_dev *hdev)
> >>
> >>       INIT_LIST_HEAD(&cache->unknown);
> >>       INIT_LIST_HEAD(&cache->resolve);
> >> -     cache->state = DISCOVERY_STOPPED;
> >>  }
> >
> > Nack.
> >
> > The reason why this was there is hci_dev_do_close() and hci_dev_reset()
> > which call inquiry_cache_flush(). If the discovery state is not set
> > correctly through these code paths you might get into a situation where
> > you can't start discovery again after doing "hciconfig hci0 reset" or
> > "hciconfig hci0 down" while discovery was active. So I agree that some
> > fix is needed but you need to ensure that you don't break these use
> > cases.
> 
> This issue is already address by upstream patch "Bluetooth: Set
> DISCOVERY_STOPPED if controller resets". So, the discovery state machine
> will be right even if we run "hciconfig hci0 reset" or "hciconfig hci0
> down" while discovery is active.

I didn't realize this has been added to the cc_reset function. In that
case I think both use cases should be fine with this patch, i.e. I
change my nack to an ack :)

Johan