Hi,
On Mon, Feb 13, 2017 at 10:34:26AM +0100, Marcel Holtmann wrote:
> > While working on the nokia BT driver, I found a strange bug in
> > bluetooth core support. If the driver's setup routine fails,
> > a hci0 interface is created with status DOWN, which looks ok.
> > The management interface is not happy, though:
> >
> > n950# btmgmt info ; btmgmt config
> > Index list with 0 items
> > Unconfigured index list with 0 items
> >
> > It becomes even more interesting, if I do "hciconfig hci0 up"
> > (the setup routine only failed due to FW not yet being available
> > during early boot). At that point btmgmt still fails:
> >
> > n950# btmgmt info; btmgmt config
> > Index list with 0 items
> > Unconfigured index list with 0 items
> >
> > But the device is up and running from hciconfig's perspective
> > and hcitool can be used to scan for devices.
> >
> > I added a few debug prints to the management interface and the
> > problem is, that HCI_SETUP flag is never cleared, when initial
> > setup fails. This should be easily reproducible by adding this
> > to any BT driver's setup routine:
> >
> > static bool fail = true;
> > if (fail) {
> > fail = false;
> > return -ENOENT;
> > }
> >
> > I'm not sure about the correct solution to fix this. Initially I
> > thought the flag just needs to be cleared in the path used by
> > "hciconfig up". But this would mean, that such a device cannot be
> > enabled using the management API (since HCI_SETUP would be set
> > until its cleared by hciconfig).
>
> so the legacy (aka old and don’t use it anymore) ioctl that
> hciconfig triggers for bringing an interface up should in theory
> do the same as Set Power from mgmt. At least from a BR/EDR
> perspective since for LE it is a lot more complicated. However
> this also includes Secure Connections and OOB pairing where you
> need mgmt to function properly.
ioctl calls hci_dev_do_open() directly and the mgmt interface uses
hci_power_on(), which does a few extra things. As far as I can tell
the ioctl should also do the extra things from hci_power_on().
Basically the extra things are: clear HCI_SETUP flag & call
mgmt_index_added. Otherwise using the legacy API results in
inconsistent mgmt state.
> That all said, there might be some race condition here in case
> hdev->setup returns a negative error. Actually if hdev->setup
> fails the controller should be marked as non-functional. That
> would mean mark it as RAW only device. I think that is how we have
> treated this. Which means you can bring it up with hciconfig’s
> ioctl or use it as HCI User Channel, but the Bluetooth core stack
> will never touch it again.
For me it happens reliably if setup fails initially. Also as far as
I can tell this can potentially happen with all BT drivers loading a
FW in their setup routine.
I added a few more debug prints for further information. During
initial start hci_power_on() is called and fails when calling
hci_dev_do_open(). As a result mgmt_set_powered_failed() is called.
The HCI_SETUP flag is only removed after succesfully calling the
setup routine.
Now in the mgmt API the index lists (read_index_list,
read_unconf_index_list and read_ext_index_list) all
use the following loop, which ignores devices with
the HCI_SETUP flag set:
list_for_each_entry(d, &hci_dev_list, list) {
if (hci_dev_test_flag(d, HCI_SETUP) ||
hci_dev_test_flag(d, HCI_CONFIG) ||
hci_dev_test_flag(d, HCI_USER_CHANNEL))
continue;
...
}
I wonder if it would be ok to just list a device with HCI_SETUP
flag _and_ power status=failed as unpowered device in the management
interface? For using the device it would have to be powered on
first, which will result in running setup routine again. If it
fails again mgmt_set_powered_failed() will be called.
-- Sebastian
Attachment:
signature.asc
Description: PGP signature
