diff mbox series

[v1] bluetooth:Adding driver and quirk defs for multi-role LE

Message ID 20200403133806.246918-1-alainm@chromium.org (mailing list archive)
State Rejected
Delegated to: Marcel Holtmann
Headers show
Series [v1] bluetooth:Adding driver and quirk defs for multi-role LE | expand

Commit Message

Alain Michaud April 3, 2020, 1:38 p.m. UTC
This change adds the relevant driver and quirk to allow drivers to
report that concurrent roles are well supported by the controller.

This has historically been disabled as controllers did not reliably
support this. In particular, this will be used to relax this condition
for controllers that have been well tested and reliable.

	/* Most controller will fail if we try to create new connections
	 * while we have an existing one in slave role.
	 */
	if (hdev->conn_hash.le_num_slave > 0)
		return NULL;

Signed-off-by: Alain Michaud <alainm@chromium.org>
---

 drivers/bluetooth/btusb.c   | 2 ++
 include/net/bluetooth/hci.h | 8 ++++++++
 2 files changed, 10 insertions(+)

Comments

Marcel Holtmann April 3, 2020, 1:49 p.m. UTC | #1
Hi Alain,

> This change adds the relevant driver and quirk to allow drivers to
> report that concurrent roles are well supported by the controller.
> 
> This has historically been disabled as controllers did not reliably
> support this. In particular, this will be used to relax this condition
> for controllers that have been well tested and reliable.
> 
> 	/* Most controller will fail if we try to create new connections
> 	 * while we have an existing one in slave role.
> 	 */
> 	if (hdev->conn_hash.le_num_slave > 0)
> 		return NULL;
> 
> Signed-off-by: Alain Michaud <alainm@chromium.org>
> ---
> 
> drivers/bluetooth/btusb.c   | 2 ++
> include/net/bluetooth/hci.h | 8 ++++++++
> 2 files changed, 10 insertions(+)
> 
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 3bdec42c9612..22e61a502d40 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -58,6 +58,8 @@ static struct usb_driver btusb_driver;
> #define BTUSB_CW6622		0x100000
> #define BTUSB_MEDIATEK		0x200000
> #define BTUSB_WIDEBAND_SPEECH	0x400000
> +#define BTUSB_LE_CONCURRENT_ROLES_SUPPORTED \
> +				0x800000
> 
> static const struct usb_device_id btusb_table[] = {
> 	/* Generic Bluetooth USB device */
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 5f60e135aeb6..b2c76cde7cd4 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -214,6 +214,14 @@ enum {
> 	 * This quirk must be set before hci_register_dev is called.
> 	 */
> 	HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> +
> +	/* When this quirk is set, the controller has validated that
> +	 * concurrent LE roles are supported.  This mechanism is necessary
> +	 * as many controllers have been seen has having trouble initiating
> +	 * a connectable advertisement after at least one connection in
> +	 * central had already been established.
> +	 */
> +	HCI_QUIRK_LE_CONCURRENT_ROLES_SUPPORTED,
> };

lets do this the other way around. Lets remove the limitation we have in our HCI core code. And then we see which controllers report errors. Trying to enable each controller individually is cumbersome. I rather later on blacklist the one or two historic controllers that don’t support it.

Regards

Marcel
Alain Michaud April 3, 2020, 1:54 p.m. UTC | #2
Hi Marcel,

On Fri, Apr 3, 2020 at 9:49 AM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Alain,
>
> > This change adds the relevant driver and quirk to allow drivers to
> > report that concurrent roles are well supported by the controller.
> >
> > This has historically been disabled as controllers did not reliably
> > support this. In particular, this will be used to relax this condition
> > for controllers that have been well tested and reliable.
> >
> >       /* Most controller will fail if we try to create new connections
> >        * while we have an existing one in slave role.
> >        */
> >       if (hdev->conn_hash.le_num_slave > 0)
> >               return NULL;
> >
> > Signed-off-by: Alain Michaud <alainm@chromium.org>
> > ---
> >
> > drivers/bluetooth/btusb.c   | 2 ++
> > include/net/bluetooth/hci.h | 8 ++++++++
> > 2 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 3bdec42c9612..22e61a502d40 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -58,6 +58,8 @@ static struct usb_driver btusb_driver;
> > #define BTUSB_CW6622          0x100000
> > #define BTUSB_MEDIATEK                0x200000
> > #define BTUSB_WIDEBAND_SPEECH 0x400000
> > +#define BTUSB_LE_CONCURRENT_ROLES_SUPPORTED \
> > +                             0x800000
> >
> > static const struct usb_device_id btusb_table[] = {
> >       /* Generic Bluetooth USB device */
> > diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 5f60e135aeb6..b2c76cde7cd4 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -214,6 +214,14 @@ enum {
> >        * This quirk must be set before hci_register_dev is called.
> >        */
> >       HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> > +
> > +     /* When this quirk is set, the controller has validated that
> > +      * concurrent LE roles are supported.  This mechanism is necessary
> > +      * as many controllers have been seen has having trouble initiating
> > +      * a connectable advertisement after at least one connection in
> > +      * central had already been established.
> > +      */
> > +     HCI_QUIRK_LE_CONCURRENT_ROLES_SUPPORTED,
> > };
>
> lets do this the other way around. Lets remove the limitation we have in our HCI core code. And then we see which controllers report errors. Trying to enable each controller individually is cumbersome. I rather later on blacklist the one or two historic controllers that don’t support it.
>

I agree it's cumbersome but given we don't have much data or existing
tests around this, we don't have too many options.  I strongly
disagree with the approach of simply enabling it and seeing what
breaks as scenarios using this functionality are expected to scale out
quickly, likely before we have time to fine all controller issues.

My team is building test infrastructure to help collect data with
this, we already know it's already problematic on a range of
controllers and we have a good idea on which controllers which will be
set.  We will contribute to the cumbersome part of this.

> Regards
>
> Marcel
>
Marcel Holtmann April 3, 2020, 5:05 p.m. UTC | #3
Hi Alain,

>>> This change adds the relevant driver and quirk to allow drivers to
>>> report that concurrent roles are well supported by the controller.
>>> 
>>> This has historically been disabled as controllers did not reliably
>>> support this. In particular, this will be used to relax this condition
>>> for controllers that have been well tested and reliable.
>>> 
>>>      /* Most controller will fail if we try to create new connections
>>>       * while we have an existing one in slave role.
>>>       */
>>>      if (hdev->conn_hash.le_num_slave > 0)
>>>              return NULL;
>>> 
>>> Signed-off-by: Alain Michaud <alainm@chromium.org>
>>> ---
>>> 
>>> drivers/bluetooth/btusb.c   | 2 ++
>>> include/net/bluetooth/hci.h | 8 ++++++++
>>> 2 files changed, 10 insertions(+)
>>> 
>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>> index 3bdec42c9612..22e61a502d40 100644
>>> --- a/drivers/bluetooth/btusb.c
>>> +++ b/drivers/bluetooth/btusb.c
>>> @@ -58,6 +58,8 @@ static struct usb_driver btusb_driver;
>>> #define BTUSB_CW6622          0x100000
>>> #define BTUSB_MEDIATEK                0x200000
>>> #define BTUSB_WIDEBAND_SPEECH 0x400000
>>> +#define BTUSB_LE_CONCURRENT_ROLES_SUPPORTED \
>>> +                             0x800000
>>> 
>>> static const struct usb_device_id btusb_table[] = {
>>>      /* Generic Bluetooth USB device */
>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>> index 5f60e135aeb6..b2c76cde7cd4 100644
>>> --- a/include/net/bluetooth/hci.h
>>> +++ b/include/net/bluetooth/hci.h
>>> @@ -214,6 +214,14 @@ enum {
>>>       * This quirk must be set before hci_register_dev is called.
>>>       */
>>>      HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
>>> +
>>> +     /* When this quirk is set, the controller has validated that
>>> +      * concurrent LE roles are supported.  This mechanism is necessary
>>> +      * as many controllers have been seen has having trouble initiating
>>> +      * a connectable advertisement after at least one connection in
>>> +      * central had already been established.
>>> +      */
>>> +     HCI_QUIRK_LE_CONCURRENT_ROLES_SUPPORTED,
>>> };
>> 
>> lets do this the other way around. Lets remove the limitation we have in our HCI core code. And then we see which controllers report errors. Trying to enable each controller individually is cumbersome. I rather later on blacklist the one or two historic controllers that don’t support it.
>> 
> 
> I agree it's cumbersome but given we don't have much data or existing
> tests around this, we don't have too many options.  I strongly
> disagree with the approach of simply enabling it and seeing what
> breaks as scenarios using this functionality are expected to scale out
> quickly, likely before we have time to fine all controller issues.
> 
> My team is building test infrastructure to help collect data with
> this, we already know it's already problematic on a range of
> controllers and we have a good idea on which controllers which will be
> set.  We will contribute to the cumbersome part of this.

can we at least do something a bit smarter based on the LE Read Supported States command?

If that command in general indicates that central and peripheral mode is supported concurrently, we provide the support for advertising. If it is not supported, we just don’t indicate support for advertising.

So I have a really bad experience with the move from Bluetooth 1.0b to 1.1 and the HCI Reset on init. We got that wrong by whitelisting hardware and it came to bite us badly when the number of controllers increased that just got the spec right.

Regards

Marcel
Alain Michaud April 3, 2020, 5:11 p.m. UTC | #4
Hi Marcel,

On Fri, Apr 3, 2020 at 1:05 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Alain,
>
> >>> This change adds the relevant driver and quirk to allow drivers to
> >>> report that concurrent roles are well supported by the controller.
> >>>
> >>> This has historically been disabled as controllers did not reliably
> >>> support this. In particular, this will be used to relax this condition
> >>> for controllers that have been well tested and reliable.
> >>>
> >>>      /* Most controller will fail if we try to create new connections
> >>>       * while we have an existing one in slave role.
> >>>       */
> >>>      if (hdev->conn_hash.le_num_slave > 0)
> >>>              return NULL;
> >>>
> >>> Signed-off-by: Alain Michaud <alainm@chromium.org>
> >>> ---
> >>>
> >>> drivers/bluetooth/btusb.c   | 2 ++
> >>> include/net/bluetooth/hci.h | 8 ++++++++
> >>> 2 files changed, 10 insertions(+)
> >>>
> >>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >>> index 3bdec42c9612..22e61a502d40 100644
> >>> --- a/drivers/bluetooth/btusb.c
> >>> +++ b/drivers/bluetooth/btusb.c
> >>> @@ -58,6 +58,8 @@ static struct usb_driver btusb_driver;
> >>> #define BTUSB_CW6622          0x100000
> >>> #define BTUSB_MEDIATEK                0x200000
> >>> #define BTUSB_WIDEBAND_SPEECH 0x400000
> >>> +#define BTUSB_LE_CONCURRENT_ROLES_SUPPORTED \
> >>> +                             0x800000
> >>>
> >>> static const struct usb_device_id btusb_table[] = {
> >>>      /* Generic Bluetooth USB device */
> >>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >>> index 5f60e135aeb6..b2c76cde7cd4 100644
> >>> --- a/include/net/bluetooth/hci.h
> >>> +++ b/include/net/bluetooth/hci.h
> >>> @@ -214,6 +214,14 @@ enum {
> >>>       * This quirk must be set before hci_register_dev is called.
> >>>       */
> >>>      HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> >>> +
> >>> +     /* When this quirk is set, the controller has validated that
> >>> +      * concurrent LE roles are supported.  This mechanism is necessary
> >>> +      * as many controllers have been seen has having trouble initiating
> >>> +      * a connectable advertisement after at least one connection in
> >>> +      * central had already been established.
> >>> +      */
> >>> +     HCI_QUIRK_LE_CONCURRENT_ROLES_SUPPORTED,
> >>> };
> >>
> >> lets do this the other way around. Lets remove the limitation we have in our HCI core code. And then we see which controllers report errors. Trying to enable each controller individually is cumbersome. I rather later on blacklist the one or two historic controllers that don’t support it.
> >>
> >
> > I agree it's cumbersome but given we don't have much data or existing
> > tests around this, we don't have too many options.  I strongly
> > disagree with the approach of simply enabling it and seeing what
> > breaks as scenarios using this functionality are expected to scale out
> > quickly, likely before we have time to fine all controller issues.
> >
> > My team is building test infrastructure to help collect data with
> > this, we already know it's already problematic on a range of
> > controllers and we have a good idea on which controllers which will be
> > set.  We will contribute to the cumbersome part of this.
>
> can we at least do something a bit smarter based on the LE Read Supported States command?
>
> If that command in general indicates that central and peripheral mode is supported concurrently, we provide the support for advertising. If it is not supported, we just don’t indicate support for advertising.
In an ideal case I'd say yes, but that's also proven not to be a
reliable metric.  Unfortunately it doesn't seem to be covered by
qualification program and as a result, it is largely not trustworthy.

>
> So I have a really bad experience with the move from Bluetooth 1.0b to 1.1 and the HCI Reset on init. We got that wrong by whitelisting
hardware and it came to bite us badly when the number of controllers
increased that just got the spec right.
I'm afraid I don't have a good exit plan for this at the moment.  It
may be something we either need to have some sort of extention for the
hardware to report the appropriate support or better, fix the
compliance gap so after a particular HCI version the bit field becomes
trustworthy.
>
> Regards
>
> Marcel
>
Marcel Holtmann April 3, 2020, 6:04 p.m. UTC | #5
Hi Alain,

>>>>> This change adds the relevant driver and quirk to allow drivers to
>>>>> report that concurrent roles are well supported by the controller.
>>>>> 
>>>>> This has historically been disabled as controllers did not reliably
>>>>> support this. In particular, this will be used to relax this condition
>>>>> for controllers that have been well tested and reliable.
>>>>> 
>>>>>     /* Most controller will fail if we try to create new connections
>>>>>      * while we have an existing one in slave role.
>>>>>      */
>>>>>     if (hdev->conn_hash.le_num_slave > 0)
>>>>>             return NULL;
>>>>> 
>>>>> Signed-off-by: Alain Michaud <alainm@chromium.org>
>>>>> ---
>>>>> 
>>>>> drivers/bluetooth/btusb.c   | 2 ++
>>>>> include/net/bluetooth/hci.h | 8 ++++++++
>>>>> 2 files changed, 10 insertions(+)
>>>>> 
>>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>>>> index 3bdec42c9612..22e61a502d40 100644
>>>>> --- a/drivers/bluetooth/btusb.c
>>>>> +++ b/drivers/bluetooth/btusb.c
>>>>> @@ -58,6 +58,8 @@ static struct usb_driver btusb_driver;
>>>>> #define BTUSB_CW6622          0x100000
>>>>> #define BTUSB_MEDIATEK                0x200000
>>>>> #define BTUSB_WIDEBAND_SPEECH 0x400000
>>>>> +#define BTUSB_LE_CONCURRENT_ROLES_SUPPORTED \
>>>>> +                             0x800000
>>>>> 
>>>>> static const struct usb_device_id btusb_table[] = {
>>>>>     /* Generic Bluetooth USB device */
>>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>>> index 5f60e135aeb6..b2c76cde7cd4 100644
>>>>> --- a/include/net/bluetooth/hci.h
>>>>> +++ b/include/net/bluetooth/hci.h
>>>>> @@ -214,6 +214,14 @@ enum {
>>>>>      * This quirk must be set before hci_register_dev is called.
>>>>>      */
>>>>>     HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
>>>>> +
>>>>> +     /* When this quirk is set, the controller has validated that
>>>>> +      * concurrent LE roles are supported.  This mechanism is necessary
>>>>> +      * as many controllers have been seen has having trouble initiating
>>>>> +      * a connectable advertisement after at least one connection in
>>>>> +      * central had already been established.
>>>>> +      */
>>>>> +     HCI_QUIRK_LE_CONCURRENT_ROLES_SUPPORTED,
>>>>> };
>>>> 
>>>> lets do this the other way around. Lets remove the limitation we have in our HCI core code. And then we see which controllers report errors. Trying to enable each controller individually is cumbersome. I rather later on blacklist the one or two historic controllers that don’t support it.
>>>> 
>>> 
>>> I agree it's cumbersome but given we don't have much data or existing
>>> tests around this, we don't have too many options.  I strongly
>>> disagree with the approach of simply enabling it and seeing what
>>> breaks as scenarios using this functionality are expected to scale out
>>> quickly, likely before we have time to fine all controller issues.
>>> 
>>> My team is building test infrastructure to help collect data with
>>> this, we already know it's already problematic on a range of
>>> controllers and we have a good idea on which controllers which will be
>>> set.  We will contribute to the cumbersome part of this.
>> 
>> can we at least do something a bit smarter based on the LE Read Supported States command?
>> 
>> If that command in general indicates that central and peripheral mode is supported concurrently, we provide the support for advertising. If it is not supported, we just don’t indicate support for advertising.
> In an ideal case I'd say yes, but that's also proven not to be a
> reliable metric.  Unfortunately it doesn't seem to be covered by
> qualification program and as a result, it is largely not trustworthy.
> 
>> 
>> So I have a really bad experience with the move from Bluetooth 1.0b to 1.1 and the HCI Reset on init. We got that wrong by whitelisting
> hardware and it came to bite us badly when the number of controllers
> increased that just got the spec right.
> I'm afraid I don't have a good exit plan for this at the moment.  It
> may be something we either need to have some sort of extention for the
> hardware to report the appropriate support or better, fix the
> compliance gap so after a particular HCI version the bit field becomes
> trustworthy.

why do we always end up in these rock vs hard-place situations ;)

You might realize that I am really reluctant with whitelisting of behavior that should be default. Do you happen to have a bit more data that you can share?

Or can we do at least something like QUIRK_VALID_LE_STATES and if that quirk is not set, then only central mode gets enabled. We can then provide a debugfs setting for that quirk.

Regards

Marcel
Alain Michaud April 3, 2020, 6:08 p.m. UTC | #6
On Fri, Apr 3, 2020 at 2:04 PM Marcel Holtmann <marcel@holtmann.org> wrote:
>
> Hi Alain,
>
> >>>>> This change adds the relevant driver and quirk to allow drivers to
> >>>>> report that concurrent roles are well supported by the controller.
> >>>>>
> >>>>> This has historically been disabled as controllers did not reliably
> >>>>> support this. In particular, this will be used to relax this condition
> >>>>> for controllers that have been well tested and reliable.
> >>>>>
> >>>>>     /* Most controller will fail if we try to create new connections
> >>>>>      * while we have an existing one in slave role.
> >>>>>      */
> >>>>>     if (hdev->conn_hash.le_num_slave > 0)
> >>>>>             return NULL;
> >>>>>
> >>>>> Signed-off-by: Alain Michaud <alainm@chromium.org>
> >>>>> ---
> >>>>>
> >>>>> drivers/bluetooth/btusb.c   | 2 ++
> >>>>> include/net/bluetooth/hci.h | 8 ++++++++
> >>>>> 2 files changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >>>>> index 3bdec42c9612..22e61a502d40 100644
> >>>>> --- a/drivers/bluetooth/btusb.c
> >>>>> +++ b/drivers/bluetooth/btusb.c
> >>>>> @@ -58,6 +58,8 @@ static struct usb_driver btusb_driver;
> >>>>> #define BTUSB_CW6622          0x100000
> >>>>> #define BTUSB_MEDIATEK                0x200000
> >>>>> #define BTUSB_WIDEBAND_SPEECH 0x400000
> >>>>> +#define BTUSB_LE_CONCURRENT_ROLES_SUPPORTED \
> >>>>> +                             0x800000
> >>>>>
> >>>>> static const struct usb_device_id btusb_table[] = {
> >>>>>     /* Generic Bluetooth USB device */
> >>>>> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >>>>> index 5f60e135aeb6..b2c76cde7cd4 100644
> >>>>> --- a/include/net/bluetooth/hci.h
> >>>>> +++ b/include/net/bluetooth/hci.h
> >>>>> @@ -214,6 +214,14 @@ enum {
> >>>>>      * This quirk must be set before hci_register_dev is called.
> >>>>>      */
> >>>>>     HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
> >>>>> +
> >>>>> +     /* When this quirk is set, the controller has validated that
> >>>>> +      * concurrent LE roles are supported.  This mechanism is necessary
> >>>>> +      * as many controllers have been seen has having trouble initiating
> >>>>> +      * a connectable advertisement after at least one connection in
> >>>>> +      * central had already been established.
> >>>>> +      */
> >>>>> +     HCI_QUIRK_LE_CONCURRENT_ROLES_SUPPORTED,
> >>>>> };
> >>>>
> >>>> lets do this the other way around. Lets remove the limitation we have in our HCI core code. And then we see which controllers report errors. Trying to enable each controller individually is cumbersome. I rather later on blacklist the one or two historic controllers that don’t support it.
> >>>>
> >>>
> >>> I agree it's cumbersome but given we don't have much data or existing
> >>> tests around this, we don't have too many options.  I strongly
> >>> disagree with the approach of simply enabling it and seeing what
> >>> breaks as scenarios using this functionality are expected to scale out
> >>> quickly, likely before we have time to fine all controller issues.
> >>>
> >>> My team is building test infrastructure to help collect data with
> >>> this, we already know it's already problematic on a range of
> >>> controllers and we have a good idea on which controllers which will be
> >>> set.  We will contribute to the cumbersome part of this.
> >>
> >> can we at least do something a bit smarter based on the LE Read Supported States command?
> >>
> >> If that command in general indicates that central and peripheral mode is supported concurrently, we provide the support for advertising. If it is not supported, we just don’t indicate support for advertising.
> > In an ideal case I'd say yes, but that's also proven not to be a
> > reliable metric.  Unfortunately it doesn't seem to be covered by
> > qualification program and as a result, it is largely not trustworthy.
> >
> >>
> >> So I have a really bad experience with the move from Bluetooth 1.0b to 1.1 and the HCI Reset on init. We got that wrong by whitelisting
> > hardware and it came to bite us badly when the number of controllers
> > increased that just got the spec right.
> > I'm afraid I don't have a good exit plan for this at the moment.  It
> > may be something we either need to have some sort of extention for the
> > hardware to report the appropriate support or better, fix the
> > compliance gap so after a particular HCI version the bit field becomes
> > trustworthy.
>
> why do we always end up in these rock vs hard-place situations ;)
This is, IMO, a direct result of poor quality of the qualification
program, especially around HCI contract validation.

>
> You might realize that I am really reluctant with whitelisting of behavior that should be default. Do you happen to have a bit more data that you can share?
We are still collecting the data.  We can share the data around when
we'd be ready to set the driver flag :)

>
> Or can we do at least something like QUIRK_VALID_LE_STATES and if that quirk is not set, then only central mode gets enabled. We can then provide a debugfs setting for that quirk.
I'm happy to change this into QUIK_VALID_LE_STATES, but I admit that
testing some of the directed advertisement states are beyond the scope
of what I'm comfortable signing up for.

>
> Regards
>
> Marcel
>
diff mbox series

Patch

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 3bdec42c9612..22e61a502d40 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -58,6 +58,8 @@  static struct usb_driver btusb_driver;
 #define BTUSB_CW6622		0x100000
 #define BTUSB_MEDIATEK		0x200000
 #define BTUSB_WIDEBAND_SPEECH	0x400000
+#define BTUSB_LE_CONCURRENT_ROLES_SUPPORTED \
+				0x800000
 
 static const struct usb_device_id btusb_table[] = {
 	/* Generic Bluetooth USB device */
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5f60e135aeb6..b2c76cde7cd4 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -214,6 +214,14 @@  enum {
 	 * This quirk must be set before hci_register_dev is called.
 	 */
 	HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED,
+
+	/* When this quirk is set, the controller has validated that
+	 * concurrent LE roles are supported.  This mechanism is necessary
+	 * as many controllers have been seen has having trouble initiating
+	 * a connectable advertisement after at least one connection in
+	 * central had already been established.
+	 */
+	HCI_QUIRK_LE_CONCURRENT_ROLES_SUPPORTED,
 };
 
 /* HCI device flags */