This series adds BTUSB and quirk support for the driver to confirm that
the reported LE_states can be trusted. The quirk will be used to
gradually roll out the feature to supported controllers without
affecting the stability of other controllers. If all controllers FWs
are fixed or else validated, we can consider removing the quirk over
time.
Alain Michaud (3):
bluetooth:Adding driver and quirk defs for multi-role LE
bluetooth:allow scatternet connections if supported.
bluetooth:btusb: Adding support for LE scatternet to Jfp and ThP
drivers/bluetooth/btusb.c | 11 ++++++++---
include/net/bluetooth/hci.h | 9 +++++++++
net/bluetooth/hci_event.c | 4 +++-
3 files changed, 20 insertions(+), 4 deletions(-)
--
2.26.1.301.g55bc3eb7cb9-goog
This change adds the relevant driver and quirk to allow drivers to
report the le_states as being trustworthy.
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 <[email protected]>
---
drivers/bluetooth/btusb.c | 2 +-
include/net/bluetooth/hci.h | 9 +++++++++
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 3bdec42c9612..dd27e28d4601 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -58,7 +58,7 @@ static struct usb_driver btusb_driver;
#define BTUSB_CW6622 0x100000
#define BTUSB_MEDIATEK 0x200000
#define BTUSB_WIDEBAND_SPEECH 0x400000
-
+#define BTUSB_VALID_LE_STATES 0x800000
static const struct usb_device_id btusb_table[] = {
/* Generic Bluetooth USB device */
{ USB_DEVICE_INFO(0xe0, 0x01, 0x01) },
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 5f60e135aeb6..25c2e5ee81dc 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -214,6 +214,15 @@ 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
+ * LE states reported through the HCI_LE_READ_SUPPORTED_STATES are
+ * valid. This mechanism is necessary as many controllers have
+ * been seen has having trouble initiating a connectable
+ * advertisement despite the state combination being reported as
+ * supported.
+ */
+ HCI_QUIRK_VALID_LE_STATES,
};
/* HCI device flags */
--
2.26.1.301.g55bc3eb7cb9-goog
This change allows scatternet connections to be created if the
controller reports support and the HCI_QUIRK_VALID_LE_STATES indicates
that the reported LE states can be trusted.
Signed-off-by: Alain Michaud <[email protected]>
---
net/bluetooth/hci_event.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 0a591be8b0ae..34cd98a1d370 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -5245,7 +5245,9 @@ static struct hci_conn *check_pending_le_conn(struct hci_dev *hdev,
/* 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)
+ if (hdev->conn_hash.le_num_slave > 0 &&
+ (hdev->quirks & HCI_QUIRK_VALID_LE_STATES) == 0 ||
+ !(hdev->le_states[3] & 0x10))
return NULL;
/* If we're not connectable only connect devices that we have in
--
2.26.1.301.g55bc3eb7cb9-goog
This change adds support for LE scatternet connections to Intel's JfP
and ThP controllers.
Signed-off-by: Alain Michaud <[email protected]>
---
drivers/bluetooth/btusb.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index dd27e28d4601..d801c332b85a 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -335,7 +335,8 @@ static const struct usb_device_id blacklist_table[] = {
/* Intel Bluetooth devices */
{ USB_DEVICE(0x8087, 0x0025), .driver_info = BTUSB_INTEL_NEW |
- BTUSB_WIDEBAND_SPEECH },
+ BTUSB_WIDEBAND_SPEECH |
+ BTUSB_VALID_LE_STATES },
{ USB_DEVICE(0x8087, 0x0026), .driver_info = BTUSB_INTEL_NEW |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0029), .driver_info = BTUSB_INTEL_NEW |
@@ -348,7 +349,8 @@ static const struct usb_device_id blacklist_table[] = {
{ USB_DEVICE(0x8087, 0x0aa7), .driver_info = BTUSB_INTEL |
BTUSB_WIDEBAND_SPEECH },
{ USB_DEVICE(0x8087, 0x0aaa), .driver_info = BTUSB_INTEL_NEW |
- BTUSB_WIDEBAND_SPEECH },
+ BTUSB_WIDEBAND_SPEECH |
+ BTUSB_VALID_LE_STATES },
/* Other Intel Bluetooth devices */
{ USB_VENDOR_AND_INTERFACE_INFO(0x8087, 0xe0, 0x01, 0x01),
@@ -3877,6 +3879,9 @@ static int btusb_probe(struct usb_interface *intf,
if (id->driver_info & BTUSB_WIDEBAND_SPEECH)
set_bit(HCI_QUIRK_WIDEBAND_SPEECH_SUPPORTED, &hdev->quirks);
+ if (id->driver_info & BTUSB_VALID_LE_STATES)
+ set_bit(HCI_QUIRK_VALID_LE_STATES, &hdev->quirks);
+
if (id->driver_info & BTUSB_DIGIANSWER) {
data->cmdreq_type = USB_TYPE_VENDOR;
set_bit(HCI_QUIRK_RESET_ON_CLOSE, &hdev->quirks);
--
2.26.1.301.g55bc3eb7cb9-goog
Hi Alain,
> This change allows scatternet connections to be created if the
> controller reports support and the HCI_QUIRK_VALID_LE_STATES indicates
> that the reported LE states can be trusted.
>
> Signed-off-by: Alain Michaud <[email protected]>
> ---
>
> net/bluetooth/hci_event.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 0a591be8b0ae..34cd98a1d370 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -5245,7 +5245,9 @@ static struct hci_conn *check_pending_le_conn(struct hci_dev *hdev,
> /* 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)
> + if (hdev->conn_hash.le_num_slave > 0 &&
> + (hdev->quirks & HCI_QUIRK_VALID_LE_STATES) == 0 ||
please use test_bit() here.
> + !(hdev->le_states[3] & 0x10))
> return NULL;
Regards
Marcel
Hi Alain,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on v5.7-rc2 next-20200422]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url: https://github.com/0day-ci/linux/commits/Alain-Michaud/bluetooth-Adding-driver-and-quirk-defs-for-multi-role-LE/20200423-083451
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: c6x-allyesconfig (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=c6x
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot <[email protected]>
All warnings (new ones prefixed by >>):
net/bluetooth/hci_event.c: In function 'check_pending_le_conn':
>> net/bluetooth/hci_event.c:5291:39: warning: suggest parentheses around '&&' within '||' [-Wparentheses]
5291 | if (hdev->conn_hash.le_num_slave > 0 &&
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
5292 | (hdev->quirks & HCI_QUIRK_VALID_LE_STATES) == 0 ||
| ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +5291 net/bluetooth/hci_event.c
5270
5271 /* This function requires the caller holds hdev->lock */
5272 static struct hci_conn *check_pending_le_conn(struct hci_dev *hdev,
5273 bdaddr_t *addr,
5274 u8 addr_type, u8 adv_type,
5275 bdaddr_t *direct_rpa)
5276 {
5277 struct hci_conn *conn;
5278 struct hci_conn_params *params;
5279
5280 /* If the event is not connectable don't proceed further */
5281 if (adv_type != LE_ADV_IND && adv_type != LE_ADV_DIRECT_IND)
5282 return NULL;
5283
5284 /* Ignore if the device is blocked */
5285 if (hci_bdaddr_list_lookup(&hdev->blacklist, addr, addr_type))
5286 return NULL;
5287
5288 /* Most controller will fail if we try to create new connections
5289 * while we have an existing one in slave role.
5290 */
> 5291 if (hdev->conn_hash.le_num_slave > 0 &&
5292 (hdev->quirks & HCI_QUIRK_VALID_LE_STATES) == 0 ||
5293 !(hdev->le_states[3] & 0x10))
5294 return NULL;
5295
5296 /* If we're not connectable only connect devices that we have in
5297 * our pend_le_conns list.
5298 */
5299 params = hci_pend_le_action_lookup(&hdev->pend_le_conns, addr,
5300 addr_type);
5301 if (!params)
5302 return NULL;
5303
5304 if (!params->explicit_connect) {
5305 switch (params->auto_connect) {
5306 case HCI_AUTO_CONN_DIRECT:
5307 /* Only devices advertising with ADV_DIRECT_IND are
5308 * triggering a connection attempt. This is allowing
5309 * incoming connections from slave devices.
5310 */
5311 if (adv_type != LE_ADV_DIRECT_IND)
5312 return NULL;
5313 break;
5314 case HCI_AUTO_CONN_ALWAYS:
5315 /* Devices advertising with ADV_IND or ADV_DIRECT_IND
5316 * are triggering a connection attempt. This means
5317 * that incoming connections from slave device are
5318 * accepted and also outgoing connections to slave
5319 * devices are established when found.
5320 */
5321 break;
5322 default:
5323 return NULL;
5324 }
5325 }
5326
5327 conn = hci_connect_le(hdev, addr, addr_type, BT_SECURITY_LOW,
5328 HCI_LE_AUTOCONN_TIMEOUT, HCI_ROLE_MASTER,
5329 direct_rpa);
5330 if (!IS_ERR(conn)) {
5331 /* If HCI_AUTO_CONN_EXPLICIT is set, conn is already owned
5332 * by higher layer that tried to connect, if no then
5333 * store the pointer since we don't really have any
5334 * other owner of the object besides the params that
5335 * triggered it. This way we can abort the connection if
5336 * the parameters get removed and keep the reference
5337 * count consistent once the connection is established.
5338 */
5339
5340 if (!params->explicit_connect)
5341 params->conn = hci_conn_get(conn);
5342
5343 return conn;
5344 }
5345
5346 switch (PTR_ERR(conn)) {
5347 case -EBUSY:
5348 /* If hci_connect() returns -EBUSY it means there is already
5349 * an LE connection attempt going on. Since controllers don't
5350 * support more than one connection attempt at the time, we
5351 * don't consider this an error case.
5352 */
5353 break;
5354 default:
5355 BT_DBG("Failed to connect: err %ld", PTR_ERR(conn));
5356 return NULL;
5357 }
5358
5359 return NULL;
5360 }
5361
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]