* [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details
@ 2021-06-08 12:24 Kiran K
2021-06-08 12:24 ` [PATCH v9 02/10] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
` (11 more replies)
0 siblings, 12 replies; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K, Chethan T N, Srivatsa Ravishankar
Move reading of supported local codecs into a separate init function,
query codecs capabilities and cache the data
Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
Reported-by: kernel test robot <lkp@intel.com>
---
* changes in v9:
- use shortname vnd instead of ven
* changes in v8:
- add comments
- split __u8 codec_id[5] into {__u8 id; __le16 cid, vid }
- address review comment related codec caps structure
* changes in v7:
- keep codec enumeration call in hci_init instead of having a separate
function
- Remove unused bitmasks defined for LE transports
* changes in v6:
- fix compiler warning reported for ARCH=arc
* changes in v5:
- fix review comments
- move code used to read standard/vendor codecs caps into single function
* changes in v4:
- convert reading of codecs and codecs caps calls from async to sync
* changes in v3
move codec enumeration into a new init function
* changes in v2
add skb length check before accessing data
include/net/bluetooth/hci.h | 41 +++++++
include/net/bluetooth/hci_core.h | 17 +++
net/bluetooth/hci_core.c | 199 ++++++++++++++++++++++++++++++-
3 files changed, 253 insertions(+), 4 deletions(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 2dc947341502..3eb723765669 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1307,6 +1307,28 @@ struct hci_rp_read_data_block_size {
} __packed;
#define HCI_OP_READ_LOCAL_CODECS 0x100b
+struct hci_std_codecs {
+ __u8 num;
+ __u8 codec[];
+} __packed;
+
+struct hci_vnd_codec {
+ /* company id */
+ __le16 cid;
+ /* vendor codec id */
+ __le16 vid;
+} __packed;
+
+struct hci_vnd_codecs {
+ __u8 num;
+ struct hci_vnd_codec codec[];
+} __packed;
+
+struct hci_rp_read_local_supported_codecs {
+ __u8 status;
+ struct hci_std_codecs std_codecs;
+ struct hci_vnd_codecs vnd_codecs;
+} __packed;
#define HCI_OP_READ_LOCAL_PAIRING_OPTS 0x100c
struct hci_rp_read_local_pairing_opts {
@@ -1315,6 +1337,25 @@ struct hci_rp_read_local_pairing_opts {
__u8 max_key_size;
} __packed;
+#define HCI_OP_READ_LOCAL_CODEC_CAPS 0x100e
+struct hci_op_read_local_codec_caps {
+ __u8 id;
+ __le16 cid;
+ __le16 vid;
+ __u8 transport;
+ __u8 direction;
+} __packed;
+
+struct hci_codec_caps {
+ __u8 len;
+ __u8 data[];
+} __packed;
+
+struct hci_rp_read_local_codec_caps {
+ __u8 status;
+ __u8 num_caps;
+} __packed;
+
#define HCI_OP_READ_PAGE_SCAN_ACTIVITY 0x0c1b
struct hci_rp_read_page_scan_activity {
__u8 status;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 212f46806ce7..3284044c3dd7 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -131,6 +131,17 @@ struct bdaddr_list {
u8 bdaddr_type;
};
+struct codec_list {
+ struct list_head list;
+ u8 id;
+ __le16 cid;
+ __le16 vid;
+ u8 transport;
+ u8 num_caps;
+ u32 len;
+ struct hci_codec_caps caps[];
+};
+
struct bdaddr_list_with_irk {
struct list_head list;
bdaddr_t bdaddr;
@@ -535,6 +546,7 @@ struct hci_dev {
struct list_head pend_le_conns;
struct list_head pend_le_reports;
struct list_head blocked_keys;
+ struct list_head local_codecs;
struct hci_dev_stats stat;
@@ -1849,4 +1861,9 @@ void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
#define SCO_AIRMODE_CVSD 0x0000
#define SCO_AIRMODE_TRANSP 0x0003
+#define LOCAL_CODEC_ACL_MASK BIT(0)
+#define LOCAL_CODEC_SCO_MASK BIT(1)
+
+#define TRANSPORT_TYPE_MAX 0x04
+
#endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 1eb7ffd0dd29..3f77ce1e9dd6 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -838,10 +838,6 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
if (hdev->commands[22] & 0x04)
hci_set_event_mask_page_2(req);
- /* Read local codec list if the HCI command is supported */
- if (hdev->commands[29] & 0x20)
- hci_req_add(req, HCI_OP_READ_LOCAL_CODECS, 0, NULL);
-
/* Read local pairing options if the HCI command is supported */
if (hdev->commands[41] & 0x08)
hci_req_add(req, HCI_OP_READ_LOCAL_PAIRING_OPTS, 0, NULL);
@@ -907,6 +903,195 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
return 0;
}
+static int hci_codec_list_add(struct list_head *list,
+ struct hci_op_read_local_codec_caps *sent,
+ struct hci_rp_read_local_codec_caps *rp,
+ void *caps,
+ __u32 len)
+{
+ struct codec_list *entry;
+
+ entry = kzalloc(sizeof(*entry) + len, GFP_KERNEL);
+ if (!entry)
+ return -ENOMEM;
+
+ entry->id = sent->id;
+ if (sent->id == 0xFF) {
+ entry->cid = __le16_to_cpu(sent->cid);
+ entry->vid = __le16_to_cpu(sent->vid);
+ }
+ entry->transport = sent->transport;
+ entry->len = len;
+ entry->num_caps = rp->num_caps;
+ if (rp->num_caps)
+ memcpy(entry->caps, caps, len);
+ list_add(&entry->list, list);
+
+ return 0;
+}
+
+static void hci_codec_list_clear(struct list_head *codec_list)
+{
+ struct codec_list *c, *n;
+
+ list_for_each_entry_safe(c, n, codec_list, list) {
+ list_del(&c->list);
+ kfree(c);
+ }
+}
+
+static void hci_read_codec_capabilities(struct hci_dev *hdev, void *codec_id,
+ __u8 transport, bool is_vnd_codec)
+{
+ struct hci_op_read_local_codec_caps cmd;
+ __u8 i;
+
+ memset(&cmd, 0, sizeof(cmd));
+
+ if (is_vnd_codec) {
+ struct hci_vnd_codec *vnd_codec;
+
+ vnd_codec = codec_id;
+ cmd.id = 0xFF;
+ cmd.cid = vnd_codec->cid;
+ cmd.vid = vnd_codec->vid;
+ } else {
+ cmd.id = *(__u8 *)codec_id;
+ }
+
+ cmd.direction = 0x00;
+
+ for (i = 0; i < TRANSPORT_TYPE_MAX; i++) {
+ if (transport & BIT(i)) {
+ struct hci_rp_read_local_codec_caps *rp;
+ struct hci_codec_caps *caps;
+ struct sk_buff *skb;
+ __u8 j;
+ __u32 len;
+
+ cmd.transport = i;
+ skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS,
+ sizeof(cmd), &cmd,
+ HCI_CMD_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Failed to read codec capabilities (%ld)",
+ PTR_ERR(skb));
+ continue;
+ }
+
+ if (skb->len < sizeof(*rp))
+ goto error;
+
+ rp = (void *)skb->data;
+
+ if (rp->status)
+ goto error;
+
+ if (!rp->num_caps) {
+ len = 0;
+ /* this codec doesn't have capabilities */
+ goto skip_caps_parse;
+ }
+
+ skb_pull(skb, sizeof(*rp));
+
+ for (j = 0, len = 0; j < rp->num_caps; j++) {
+ caps = (void *)skb->data;
+ if (skb->len < sizeof(*caps))
+ goto error;
+ if (skb->len < caps->len)
+ goto error;
+ len += sizeof(caps->len) + caps->len;
+ skb_pull(skb, sizeof(caps->len) + caps->len);
+ }
+
+skip_caps_parse:
+ hci_dev_lock(hdev);
+ hci_codec_list_add(&hdev->local_codecs, &cmd, rp,
+ (__u8 *)rp + sizeof(*rp), len);
+ hci_dev_unlock(hdev);
+error:
+ kfree_skb(skb);
+ }
+ }
+}
+
+static void hci_codec_list_parse(struct hci_dev *hdev, __u8 num_codecs,
+ void *codec_list, bool is_vnd_codec)
+{
+ __u8 i;
+
+ for (i = 0; i < num_codecs; i++) {
+ if (!is_vnd_codec) {
+ struct hci_std_codecs *codecs = codec_list;
+
+ hci_read_codec_capabilities(hdev, &codecs->codec[i],
+ LOCAL_CODEC_ACL_MASK,
+ is_vnd_codec);
+ } else {
+ struct hci_vnd_codecs *codecs = codec_list;
+
+ hci_read_codec_capabilities(hdev, &codecs->codec[i],
+ LOCAL_CODEC_ACL_MASK,
+ is_vnd_codec);
+ }
+ }
+}
+
+static void hci_read_supported_codecs(struct hci_dev *hdev)
+{
+ struct sk_buff *skb;
+ struct hci_rp_read_local_supported_codecs *rp;
+ struct hci_std_codecs *std_codecs;
+ struct hci_vnd_codecs *vnd_codecs;
+
+ skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_CODECS, 0, NULL,
+ HCI_CMD_TIMEOUT);
+
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
+ PTR_ERR(skb));
+ return;
+ }
+
+ if (skb->len < sizeof(*rp))
+ goto error;
+
+ rp = (void *)skb->data;
+
+ if (rp->status)
+ goto error;
+
+ skb_pull(skb, sizeof(rp->status));
+
+ std_codecs = (void *)skb->data;
+
+ /* validate codecs length before accessing */
+ if (skb->len < flex_array_size(std_codecs, codec, std_codecs->num)
+ + sizeof(std_codecs->num))
+ goto error;
+
+ /* enumerate codec capabilities of standard codecs */
+ hci_codec_list_parse(hdev, std_codecs->num, std_codecs, false);
+
+ skb_pull(skb, flex_array_size(std_codecs, codec, std_codecs->num)
+ + sizeof(std_codecs->num));
+
+ vnd_codecs = (void *)skb->data;
+
+ /* validate vendor codecs length before accessing */
+ if (skb->len <
+ flex_array_size(vnd_codecs, codec, vnd_codecs->num)
+ + sizeof(vnd_codecs->num))
+ goto error;
+
+ /* enumerate vendor codec capabilities */
+ hci_codec_list_parse(hdev, vnd_codecs->num, vnd_codecs, true);
+
+error:
+ kfree_skb(skb);
+}
+
static int __hci_init(struct hci_dev *hdev)
{
int err;
@@ -937,6 +1122,10 @@ static int __hci_init(struct hci_dev *hdev)
if (err < 0)
return err;
+ /* Read local codec list if the HCI command is supported */
+ if (hdev->commands[29] & 0x20)
+ hci_read_supported_codecs(hdev);
+
/* This function is only called when the controller is actually in
* configured state. When the controller is marked as unconfigured,
* this initialization procedure is not run.
@@ -1836,6 +2025,7 @@ int hci_dev_do_close(struct hci_dev *hdev)
memset(hdev->eir, 0, sizeof(hdev->eir));
memset(hdev->dev_class, 0, sizeof(hdev->dev_class));
bacpy(&hdev->random_addr, BDADDR_ANY);
+ hci_codec_list_clear(&hdev->local_codecs);
hci_req_sync_unlock(hdev);
@@ -3837,6 +4027,7 @@ struct hci_dev *hci_alloc_dev(void)
INIT_LIST_HEAD(&hdev->conn_hash.list);
INIT_LIST_HEAD(&hdev->adv_instances);
INIT_LIST_HEAD(&hdev->blocked_keys);
+ INIT_LIST_HEAD(&hdev->local_codecs);
INIT_WORK(&hdev->rx_work, hci_rx_work);
INIT_WORK(&hdev->cmd_work, hci_cmd_work);
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 02/10] Bluetooth: Add support for Read Local Supported Codecs V2
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-08 12:24 ` [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path Kiran K
` (10 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K, Chethan T N, Srivatsa Ravishankar
Use V2 version of read local supported command is controller
supports
snoop:
> HCI Event: Command Complete (0x0e) plen 20
Read Local Supported Codecs V2 (0x04|0x000d) ncmd 1
Status: Success (0x00)
Number of supported codecs: 7
Codec: u-law log (0x00)
Logical Transport Type: 0x02
Codec supported over BR/EDR SCO and eSCO
Codec: A-law log (0x01)
Logical Transport Type: 0x02
Codec supported over BR/EDR SCO and eSCO
Codec: CVSD (0x02)
Logical Transport Type: 0x02
Codec supported over BR/EDR SCO and eSCO
Codec: Transparent (0x03)
Logical Transport Type: 0x02
Codec supported over BR/EDR SCO and eSCO
Codec: Linear PCM (0x04)
Logical Transport Type: 0x02
Codec supported over BR/EDR SCO and eSCO
Codec: Reserved (0x08)
Logical Transport Type: 0x03
Codec supported over BR/EDR ACL
Codec supported over BR/EDR SCO and eSCO
Codec: mSBC (0x05)
Logical Transport Type: 0x03
Codec supported over BR/EDR ACL
Codec supported over BR/EDR SCO and eSCO
Number of vendor codecs: 0
......
< HCI Command: Read Local Suppor.. (0x04|0x000e) plen 7
Codec: mSBC (0x05)
Logical Transport Type: 0x00
Direction: Input (Host to Controller) (0x00)
> HCI Event: Command Complete (0x0e) plen 12
Read Local Supported Codec Capabilities (0x04|0x000e) ncmd 1
Status: Success (0x00)
Number of codec capabilities: 1
Capabilities #0:
00 00 11 15 02 33
Signed-off-by: Kiran K <kiran.k@intel.com>
Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
* changes in v9:
use vnd as shortcut name for vendor instead of ven
* changes in v8:
no changes
* changes in v7:
call codec enumeration code in hci_init instead of having it in a separate
function
* changes in v6
no changes
* changes in v5:
fix review comments
* changes in v4:
converts codec read capabilities calls from async to sync
* changes in v3:
No changes
* changes in v2:
add length check for event data before accessing
include/net/bluetooth/hci.h | 29 ++++++++++++++
net/bluetooth/hci_core.c | 78 ++++++++++++++++++++++++++++++++++++-
2 files changed, 106 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 3eb723765669..45bd9af4ce61 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1337,6 +1337,35 @@ struct hci_rp_read_local_pairing_opts {
__u8 max_key_size;
} __packed;
+#define HCI_OP_READ_LOCAL_CODECS_V2 0x100d
+struct hci_std_codec_v2 {
+ __u8 id;
+ __u8 transport;
+} __packed;
+
+struct hci_std_codecs_v2 {
+ __u8 num;
+ struct hci_std_codec_v2 codec[];
+} __packed;
+
+struct hci_vnd_codec_v2 {
+ __u8 id;
+ __le16 cid;
+ __le16 vid;
+ __u8 transport;
+} __packed;
+
+struct hci_vnd_codecs_v2 {
+ __u8 num;
+ struct hci_vnd_codec_v2 codec[];
+} __packed;
+
+struct hci_rp_read_local_supported_codecs_v2 {
+ __u8 status;
+ struct hci_std_codecs_v2 std_codecs;
+ struct hci_vnd_codecs_v2 vendor_codecs;
+} __packed;
+
#define HCI_OP_READ_LOCAL_CODEC_CAPS 0x100e
struct hci_op_read_local_codec_caps {
__u8 id;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 3f77ce1e9dd6..186b347ae9d3 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -1038,6 +1038,28 @@ static void hci_codec_list_parse(struct hci_dev *hdev, __u8 num_codecs,
}
}
+static void hci_codec_list_parse_v2(struct hci_dev *hdev, __u8 num_codecs,
+ void *codec_list, bool is_vnd_codec)
+{
+ __u8 i;
+
+ for (i = 0; i < num_codecs; i++) {
+ if (!is_vnd_codec) {
+ struct hci_std_codecs_v2 *codecs = codec_list;
+
+ hci_read_codec_capabilities(hdev, &codecs->codec[i],
+ codecs->codec[i].transport,
+ is_vnd_codec);
+ } else {
+ struct hci_vnd_codecs_v2 *codecs = codec_list;
+
+ hci_read_codec_capabilities(hdev, &codecs->codec[i],
+ codecs->codec[i].transport,
+ is_vnd_codec);
+ }
+ }
+}
+
static void hci_read_supported_codecs(struct hci_dev *hdev)
{
struct sk_buff *skb;
@@ -1092,6 +1114,58 @@ static void hci_read_supported_codecs(struct hci_dev *hdev)
kfree_skb(skb);
}
+static void hci_read_supported_codecs_v2(struct hci_dev *hdev)
+{
+ struct sk_buff *skb;
+ struct hci_rp_read_local_supported_codecs_v2 *rp;
+ struct hci_std_codecs_v2 *std_codecs;
+ struct hci_vnd_codecs_v2 *vnd_codecs;
+
+ skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_CODECS_V2, 0, NULL,
+ HCI_CMD_TIMEOUT);
+
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Failed to read local supported codecs (%ld)",
+ PTR_ERR(skb));
+ return;
+ }
+
+ if (skb->len < sizeof(*rp))
+ goto error;
+
+ rp = (void *)skb->data;
+
+ if (rp->status)
+ goto error;
+
+ skb_pull(skb, sizeof(rp->status));
+
+ std_codecs = (void *)skb->data;
+
+ /* check for payload data length before accessing */
+ if (skb->len < flex_array_size(std_codecs, codec, std_codecs->num)
+ + sizeof(std_codecs->num))
+ goto error;
+
+ hci_codec_list_parse_v2(hdev, std_codecs->num, std_codecs, false);
+
+ skb_pull(skb, flex_array_size(std_codecs, codec, std_codecs->num)
+ + sizeof(std_codecs->num));
+
+ vnd_codecs = (void *)skb->data;
+
+ /* check for payload data length before accessing */
+ if (skb->len <
+ flex_array_size(vnd_codecs, codec, vnd_codecs->num)
+ + sizeof(vnd_codecs->num))
+ goto error;
+
+ hci_codec_list_parse_v2(hdev, vnd_codecs->num, vnd_codecs, true);
+
+error:
+ kfree_skb(skb);
+}
+
static int __hci_init(struct hci_dev *hdev)
{
int err;
@@ -1123,7 +1197,9 @@ static int __hci_init(struct hci_dev *hdev)
return err;
/* Read local codec list if the HCI command is supported */
- if (hdev->commands[29] & 0x20)
+ if (hdev->commands[45] & 0x04)
+ hci_read_supported_codecs_v2(hdev);
+ else if (hdev->commands[29] & 0x20)
hci_read_supported_codecs(hdev);
/* This function is only called when the controller is actually in
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
2021-06-08 12:24 ` [PATCH v9 02/10] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-15 19:26 ` Marcel Holtmann
2021-06-08 12:24 ` [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback Kiran K
` (9 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K
There is no standard HCI command to retrieve data path for transport.
Add a new callback function to retrieve data path which is used
in offload usecase. This needs to be set at setup stage if controller
supports offload codecs
Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
* changes in v9:
- define a separate patch for core changes
include/net/bluetooth/hci_core.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 3284044c3dd7..641477396da3 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -617,6 +617,7 @@ struct hci_dev {
int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
void (*cmd_timeout)(struct hci_dev *hdev);
bool (*prevent_wake)(struct hci_dev *hdev);
+ int (*get_data_path)(struct hci_dev *hdev);
};
#define HCI_PHY_HANDLE(handle) (handle & 0xff)
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
2021-06-08 12:24 ` [PATCH v9 02/10] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
2021-06-08 12:24 ` [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-15 19:30 ` Marcel Holtmann
2021-06-08 12:24 ` [PATCH v9 05/10] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket Kiran K
` (8 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K
Read supported offload usecases and set get_data_path callback if
controller suppports offload codecs.
Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
drivers/bluetooth/btintel.c | 36 ++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btintel.h | 18 ++++++++++++++++++
drivers/bluetooth/btusb.c | 8 ++++++++
3 files changed, 62 insertions(+)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index e44b6993cf91..95c6a1bef35e 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1272,6 +1272,42 @@ int btintel_set_debug_features(struct hci_dev *hdev,
}
EXPORT_SYMBOL_GPL(btintel_set_debug_features);
+int btintel_get_data_path(struct hci_dev *hdev)
+{
+ return 1;
+}
+EXPORT_SYMBOL_GPL(btintel_get_data_path);
+
+int btintel_read_offload_usecases(struct hci_dev *hdev,
+ struct intel_offload_usecases *usecases)
+{
+ struct sk_buff *skb;
+ int err = 0;
+
+ skb = __hci_cmd_sync(hdev, 0xfc86, 0, NULL, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "Reading offload usecases failed (%ld)",
+ PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+
+ if (skb->len < sizeof(*usecases)) {
+ err = -EIO;
+ goto error;
+ }
+
+ if (skb->data[0]) {
+ err = -bt_to_errno(skb->data[0]);
+ goto error;
+ }
+
+ memcpy(usecases, skb->data, sizeof(*usecases));
+error:
+ kfree_skb(skb);
+ return err;
+}
+EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
+
MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
MODULE_VERSION(VERSION);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index d184064a5e7c..9bcc489680db 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -132,6 +132,11 @@ struct intel_debug_features {
__u8 page1[16];
} __packed;
+struct intel_offload_usecases {
+ __u8 status;
+ __u8 preset[8];
+} __packed;
+
#define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) >> 8))
#define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) >> 16))
#define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff)
@@ -175,6 +180,9 @@ int btintel_read_debug_features(struct hci_dev *hdev,
struct intel_debug_features *features);
int btintel_set_debug_features(struct hci_dev *hdev,
const struct intel_debug_features *features);
+int btintel_read_offload_usecases(struct hci_dev *hdev,
+ struct intel_offload_usecases *usecases);
+int btintel_get_data_path(struct hci_dev *hdev);
#else
static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -307,4 +315,14 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
return -EOPNOTSUPP;
}
+static int btintel_read_offload_usecases(struct hci_dev *hdev,
+ struct intel_offload_usecases *usecases)
+{
+ return -EOPNOTSUPP;
+}
+
+static int btintel_get_data_path(struct hci_dev *hdev)
+{
+ return -EOPNOTSUPP;
+}
#endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index a9855a2dd561..1e51beec5776 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -2952,6 +2952,7 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
int err;
struct intel_debug_features features;
struct intel_version_tlv version;
+ struct intel_offload_usecases usecases;
bt_dev_dbg(hdev, "");
@@ -3008,6 +3009,13 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
/* Set DDC mask for available debug features */
btintel_set_debug_features(hdev, &features);
+ err = btintel_read_offload_usecases(hdev, &usecases);
+ if (!err) {
+ /* set get_data_path callback if offload is supported */
+ if (usecases.preset[0] & 0x03)
+ hdev->get_data_path = btintel_get_data_path;
+ }
+
/* Read the Intel version information after loading the FW */
err = btintel_read_version_tlv(hdev, &version);
if (err)
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 05/10] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
` (2 preceding siblings ...)
2021-06-08 12:24 ` [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-15 19:37 ` Marcel Holtmann
2021-06-08 12:24 ` [PATCH v9 06/10] Bluetooth: Add a callback function to set data path Kiran K
` (7 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K
Add BT_CODEC option for getsockopt systemcall over SCO socket
to expose the codecs supported by controller
Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
* changes on v9:
- fix typos,review comments, remove quirk
include/net/bluetooth/bluetooth.h | 20 ++++++
include/net/bluetooth/hci.h | 4 ++
net/bluetooth/sco.c | 111 +++++++++++++++++++++++++++++-
3 files changed, 134 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 9125effbf448..1840756958ce 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -153,6 +153,26 @@ struct bt_voice {
#define BT_SCM_PKT_STATUS 0x03
+#define BT_CODEC 19
+
+struct bt_codec_caps {
+ __u8 len;
+ __u8 data[];
+} __packed;
+
+struct bt_codec {
+ __u8 id;
+ __le16 cid;
+ __le16 vid;
+ __u8 data_path;
+ __u8 num_caps;
+} __packed;
+
+struct bt_codecs {
+ __u8 num_codecs;
+ struct bt_codec codecs[];
+} __packed;
+
__printf(1, 2)
void bt_info(const char *fmt, ...);
__printf(1, 2)
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 45bd9af4ce61..31a5ac8918fc 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -2619,6 +2619,10 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
#define hci_iso_data_len(h) ((h) & 0x3fff)
#define hci_iso_data_flags(h) ((h) >> 14)
+/* codec transport types */
+#define TRANSPORT_ACL 0x00
+#define TRANSPORT_SCO_ESCO 0x01
+
/* le24 support */
static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3])
{
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index d9a4e88dacbb..98d5e24e5680 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -944,10 +944,15 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
char __user *optval, int __user *optlen)
{
struct sock *sk = sock->sk;
- int len, err = 0;
+ int len, err = 0, buf_len;
struct bt_voice voice;
u32 phys;
int pkt_status;
+ struct codec_list *c;
+ u8 num_codecs, i, __user *ptr;
+ struct hci_dev *hdev;
+ struct hci_codec_caps *caps;
+ __u8 data_path;
BT_DBG("sk %p", sk);
@@ -1012,6 +1017,110 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
err = -EFAULT;
break;
+ case BT_CODEC:
+ num_codecs = 0;
+ buf_len = 0;
+
+ hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
+ if (!hdev) {
+ err = -EBADFD;
+ break;
+ }
+
+ if (!hdev->get_data_path) {
+ err = -EOPNOTSUPP;
+ break;
+ }
+
+ hci_dev_lock(hdev);
+ /* find total buffer size required to copy codec + caps */
+ list_for_each_entry(c, &hdev->local_codecs, list) {
+ if (c->transport != TRANSPORT_SCO_ESCO)
+ continue;
+ num_codecs++;
+ for (i = 0, caps = c->caps; i < c->num_caps; i++) {
+ buf_len += 1 + caps->len;
+ caps = (void *)&caps->data[caps->len];
+ }
+ buf_len += sizeof(struct bt_codec);
+ }
+ hci_dev_unlock(hdev);
+
+ buf_len += sizeof(struct bt_codecs);
+ if (buf_len > len) {
+ err = -ENOBUFS;
+ break;
+ }
+ ptr = optval;
+
+ if (put_user(num_codecs, ptr)) {
+ err = -EFAULT;
+ break;
+ }
+ ptr += sizeof(num_codecs);
+
+ hci_dev_lock(hdev);
+ list_for_each_entry(c, &hdev->local_codecs, list) {
+ if (c->transport != TRANSPORT_SCO_ESCO)
+ continue;
+
+ if (put_user(c->id, ptr)) {
+ err = -EFAULT;
+ goto unlock;
+ }
+ ptr += sizeof(c->id);
+
+ if (put_user(c->cid, ptr)) {
+ err = -EFAULT;
+ goto unlock;
+ }
+ ptr += sizeof(c->cid);
+
+ if (put_user(c->vid, ptr)) {
+ err = -EFAULT;
+ goto unlock;
+ }
+ ptr += sizeof(c->vid);
+
+ err = hdev->get_data_path(hdev);
+ if (err < 0) {
+ err = -EFAULT;
+ goto unlock;
+ }
+
+ data_path = (__u8)err;
+ if (put_user(data_path, ptr)) {
+ err = -EFAULT;
+ goto unlock;
+ }
+ ptr += sizeof(data_path);
+
+ if (put_user(c->num_caps, ptr)) {
+ err = -EFAULT;
+ goto unlock;
+ }
+ ptr += sizeof(c->num_caps);
+
+ len = 0;
+ for (i = 0, caps = c->caps; i < c->num_caps; i++) {
+ len += 1 + caps->len;
+ caps = (void *)&caps->data[caps->len];
+ }
+
+ if (len && copy_to_user(ptr, c->caps, len)) {
+ err = -EFAULT;
+ goto unlock;
+ }
+ ptr += len;
+ }
+
+ if (put_user(buf_len, optlen))
+ err = -EFAULT;
+unlock:
+ hci_dev_unlock(hdev);
+
+ break;
+
default:
err = -ENOPROTOOPT;
break;
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 06/10] Bluetooth: Add a callback function to set data path
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
` (3 preceding siblings ...)
2021-06-08 12:24 ` [PATCH v9 05/10] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-15 19:37 ` Marcel Holtmann
2021-06-08 12:24 ` [PATCH v9 07/10] Bluetooth: btintel: define callback " Kiran K
` (6 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K
In HFP offload usecase, Intel controllers require offload use
case id (NBS or WBS) to be set before opening SCO connection. Define
a new callback which gets called on setsockopt SCO socket. User space
audio module is expected to set codec via setsockopt(sk, BT_CODEC, ....)
before opening SCO connection.
Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
include/net/bluetooth/hci_core.h | 2 ++
1 file changed, 2 insertions(+)
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 641477396da3..ad0024891447 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -618,6 +618,8 @@ struct hci_dev {
void (*cmd_timeout)(struct hci_dev *hdev);
bool (*prevent_wake)(struct hci_dev *hdev);
int (*get_data_path)(struct hci_dev *hdev);
+ int (*set_data_path)(struct hci_dev *hdev, __u8 type,
+ struct bt_codec *codec);
};
#define HCI_PHY_HANDLE(handle) (handle & 0xff)
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 07/10] Bluetooth: btintel: define callback to set data path
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
` (4 preceding siblings ...)
2021-06-08 12:24 ` [PATCH v9 06/10] Bluetooth: Add a callback function to set data path Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-15 19:39 ` Marcel Holtmann
2021-06-08 12:24 ` [PATCH v9 08/10] Bluetooth: Add BT_CODEC option for setsockopt over SCO Kiran K
` (5 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K
Adds callback function which is called to set the data path
for HFP offload case before opening SCO connection
Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
drivers/bluetooth/btintel.c | 50 +++++++++++++++++++++++++++++++++++++
drivers/bluetooth/btintel.h | 8 ++++++
drivers/bluetooth/btusb.c | 4 ++-
include/net/bluetooth/hci.h | 8 ++++++
4 files changed, 69 insertions(+), 1 deletion(-)
diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 95c6a1bef35e..3eba5c587ef6 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct hci_dev *hdev,
}
EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
+int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
+ struct bt_codec *codec)
+{
+ __u8 preset;
+ struct hci_op_configure_data_path *cmd;
+ __u8 buffer[255];
+ struct sk_buff *skb;
+
+ if (type != SCO_LINK && type != ESCO_LINK)
+ return -EINVAL;
+
+ switch (codec->id) {
+ case 0x02:
+ preset = 0x00;
+ break;
+ case 0x05:
+ preset = 0x01;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ cmd = (void *)buffer;
+ cmd->data_path_id = 0x01;
+ cmd->len = 1;
+ cmd->data[0] = preset;
+
+ cmd->direction = 0x00;
+ skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH, sizeof(*cmd) + 1,
+ cmd, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "configure input data path failed (%ld)",
+ PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+ kfree_skb(skb);
+
+ cmd->direction = 0x01;
+ skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH, sizeof(*cmd) + 1,
+ cmd, HCI_INIT_TIMEOUT);
+ if (IS_ERR(skb)) {
+ bt_dev_err(hdev, "configure output data path failed (%ld)",
+ PTR_ERR(skb));
+ return PTR_ERR(skb);
+ }
+ kfree_skb(skb);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(btintel_set_data_path);
+
MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
MODULE_VERSION(VERSION);
diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
index 9bcc489680db..9806970c9871 100644
--- a/drivers/bluetooth/btintel.h
+++ b/drivers/bluetooth/btintel.h
@@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev *hdev,
int btintel_read_offload_usecases(struct hci_dev *hdev,
struct intel_offload_usecases *usecases);
int btintel_get_data_path(struct hci_dev *hdev);
+int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
+ struct bt_codec *codec);
#else
static inline int btintel_check_bdaddr(struct hci_dev *hdev)
@@ -325,4 +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev)
{
return -EOPNOTSUPP;
}
+
+static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
+ struct bt_codec *codec)
+{
+ return -EOPNOTSUPP;
+}
#endif
diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 1e51beec5776..afafa44752a1 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
err = btintel_read_offload_usecases(hdev, &usecases);
if (!err) {
/* set get_data_path callback if offload is supported */
- if (usecases.preset[0] & 0x03)
+ if (usecases.preset[0] & 0x03) {
hdev->get_data_path = btintel_get_data_path;
+ hdev->set_data_path = btintel_set_data_path;
+ }
}
/* Read the Intel version information after loading the FW */
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 31a5ac8918fc..42963188dcea 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {
__u8 rand256[16];
} __packed;
+#define HCI_CONFIGURE_DATA_PATH 0x0c83
+struct hci_op_configure_data_path {
+ __u8 direction;
+ __u8 data_path_id;
+ __u8 len;
+ __u8 data[];
+} __packed;
+
#define HCI_OP_READ_LOCAL_VERSION 0x1001
struct hci_rp_read_local_version {
__u8 status;
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 08/10] Bluetooth: Add BT_CODEC option for setsockopt over SCO
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
` (5 preceding siblings ...)
2021-06-08 12:24 ` [PATCH v9 07/10] Bluetooth: btintel: define callback " Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-08 12:24 ` [PATCH v9 09/10] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command Kiran K
` (4 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K
Add BT_CODEC option on setsockopt system call to allow user space
audio modules to set codec. Driver also sets data path if non-HCI is
selected.
Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
include/net/bluetooth/bluetooth.h | 2 ++
net/bluetooth/sco.c | 59 +++++++++++++++++++++++++++++++
2 files changed, 61 insertions(+)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 1840756958ce..0e8802d09068 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -173,6 +173,8 @@ struct bt_codecs {
struct bt_codec codecs[];
} __packed;
+#define CODING_FORMAT_CVSD 0x02
+
__printf(1, 2)
void bt_info(const char *fmt, ...);
__printf(1, 2)
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 98d5e24e5680..5aa6808c1a22 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -67,6 +67,7 @@ struct sco_pinfo {
__u32 flags;
__u16 setting;
__u8 cmsg_mask;
+ struct bt_codec codec;
struct sco_conn *conn;
};
@@ -438,6 +439,7 @@ static void __sco_sock_close(struct sock *sk)
sock_set_flag(sk, SOCK_ZAPPED);
break;
}
+
}
/* Must be called on unlocked socket. */
@@ -499,6 +501,10 @@ static struct sock *sco_sock_alloc(struct net *net, struct socket *sock,
sk->sk_state = BT_OPEN;
sco_pi(sk)->setting = BT_VOICE_CVSD_16BIT;
+ sco_pi(sk)->codec.id = CODING_FORMAT_CVSD;
+ sco_pi(sk)->codec.cid = 0xffff;
+ sco_pi(sk)->codec.vid = 0xffff;
+ sco_pi(sk)->codec.data_path = 0x00;
timer_setup(&sk->sk_timer, sco_sock_timeout, 0);
@@ -808,6 +814,9 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
struct sock *sk = sock->sk;
int len, err = 0;
struct bt_voice voice;
+ struct bt_codecs *codecs;
+ struct hci_dev *hdev;
+ __u8 buffer[255];
u32 opt;
BT_DBG("sk %p", sk);
@@ -870,6 +879,56 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
sco_pi(sk)->cmsg_mask &= SCO_CMSG_PKT_STATUS;
break;
+ case BT_CODEC:
+ if (sk->sk_state != BT_OPEN && sk->sk_state != BT_BOUND &&
+ sk->sk_state != BT_CONNECT2) {
+ err = -EINVAL;
+ break;
+ }
+
+ hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
+ if (!hdev) {
+ err = -EBADFD;
+ break;
+ }
+
+ if (!hdev->set_data_path) {
+ err = -EOPNOTSUPP;
+ break;
+ }
+
+ if (optlen < sizeof(struct bt_codecs) || optlen > 255) {
+ err = -EINVAL;
+ break;
+ }
+
+ if (copy_from_sockptr(buffer, optval, optlen)) {
+ err = -EFAULT;
+ break;
+ }
+
+ codecs = (void *)buffer;
+
+ if (codecs->num_codecs > 1) {
+ err = -EINVAL;
+ break;
+ }
+
+ if (codecs->codecs[0].data_path) {
+ err = hdev->set_data_path(hdev, SCO_LINK,
+ codecs->codecs);
+ if (err < 0)
+ break;
+
+ if (codecs->codecs[0].id == 0xff) {
+ sco_pi(sk)->codec.cid = codecs->codecs[0].cid;
+ sco_pi(sk)->codec.vid = codecs->codecs[0].vid;
+ }
+ }
+ sco_pi(sk)->codec.id = codecs->codecs[0].id;
+ sco_pi(sk)->codec.data_path = codecs->codecs[0].data_path;
+ break;
+
default:
err = -ENOPROTOOPT;
break;
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 09/10] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
` (6 preceding siblings ...)
2021-06-08 12:24 ` [PATCH v9 08/10] Bluetooth: Add BT_CODEC option for setsockopt over SCO Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-08 12:24 ` [PATCH v9 10/10] Bluetooth: Add support for msbc coding format Kiran K
` (3 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K
< HCI Command: Enhanced Setup Synchronous Connection (0x01|0x003d) plen 59
Handle: 256
Transmit bandwidth: 8000
Receive bandwidth: 8000
Max latency: 13
Packet type: 0x0380
3-EV3 may not be used
2-EV5 may not be used
3-EV5 may not be used
Retransmission effort: Optimize for link quality (0x02)
> HCI Event: Command Status (0x0f) plen 4
Enhanced Setup Synchronous Connection (0x01|0x003d) ncmd 1
Status: Success (0x00)
> HCI Event: Synchronous Connect Complete (0x2c) plen 17
Status: Success (0x00)
Handle: 257
Address: CC:98:8B:92:04:FD (SONY Visual Products Inc.)
Link type: eSCO (0x02)
Transmission interval: 0x0c
Retransmission window: 0x06
RX packet length: 60
TX packet length: 60
Air mode: Transparent (0x03)
Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
* changes in v9:
- Fix review comments, use bt_dev_dbg instead of BT_DBG
include/net/bluetooth/bluetooth.h | 3 +-
include/net/bluetooth/hci.h | 34 ++++++++++
include/net/bluetooth/hci_core.h | 7 +-
net/bluetooth/hci_conn.c | 107 ++++++++++++++++++++++++++++--
net/bluetooth/hci_event.c | 48 +++++++++++++-
net/bluetooth/sco.c | 11 ++-
6 files changed, 201 insertions(+), 9 deletions(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index 0e8802d09068..af2809121571 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -173,7 +173,8 @@ struct bt_codecs {
struct bt_codec codecs[];
} __packed;
-#define CODING_FORMAT_CVSD 0x02
+#define CODING_FORMAT_CVSD 0x02
+#define CODING_FORMAT_TRANSPARENT 0x03
__printf(1, 2)
void bt_info(const char *fmt, ...);
diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 42963188dcea..03241c15b5fb 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -871,6 +871,40 @@ struct hci_cp_logical_link_cancel {
__u8 flow_spec_id;
} __packed;
+#define HCI_OP_ENHANCED_SETUP_SYNC_CONN 0x043D
+struct hci_coding_format {
+ __u8 id;
+ __le16 cid;
+ __le16 vid;
+} __packed;
+
+struct hci_cp_enhanced_setup_sync_conn {
+ __le16 handle;
+ __le32 tx_bandwidth;
+ __le32 rx_bandwidth;
+ struct hci_coding_format tx_coding_format;
+ struct hci_coding_format rx_coding_format;
+ __le16 tx_codec_frame_size;
+ __le16 rx_codec_frame_size;
+ __le32 in_bandwidth;
+ __le32 out_bandwidth;
+ struct hci_coding_format in_coding_format;
+ struct hci_coding_format out_coding_format;
+ __le16 in_coded_data_size;
+ __le16 out_coded_data_size;
+ __u8 in_pcm_data_format;
+ __u8 out_pcm_data_format;
+ __u8 in_pcm_sample_payload_msb_pos;
+ __u8 out_pcm_sample_payload_msb_pos;
+ __u8 in_data_path;
+ __u8 out_data_path;
+ __u8 in_trasnport_unit_size;
+ __u8 out_trasnport_unit_size;
+ __le16 max_latency;
+ __le16 pkt_type;
+ __u8 retrans_effort;
+} __packed;
+
struct hci_rp_logical_link_cancel {
__u8 status;
__u8 phy_handle;
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index ad0024891447..4c2514135a5d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -712,6 +712,7 @@ struct hci_conn {
struct amp_mgr *amp_mgr;
struct hci_conn *link;
+ struct bt_codec codec;
void (*connect_cfm_cb) (struct hci_conn *conn, u8 status);
void (*security_cfm_cb) (struct hci_conn *conn, u8 status);
@@ -1094,6 +1095,7 @@ static inline struct hci_conn *hci_lookup_le_connect(struct hci_dev *hdev)
int hci_disconnect(struct hci_conn *conn, __u8 reason);
bool hci_setup_sync(struct hci_conn *conn, __u16 handle);
+bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle);
void hci_sco_setup(struct hci_conn *conn, __u8 status);
struct hci_conn *hci_conn_add(struct hci_dev *hdev, int type, bdaddr_t *dst,
@@ -1118,7 +1120,7 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
u8 sec_level, u8 auth_type,
enum conn_reasons conn_reason);
struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
- __u16 setting);
+ __u16 setting, struct bt_codec *codec);
int hci_conn_check_link_mode(struct hci_conn *conn);
int hci_conn_check_secure(struct hci_conn *conn, __u8 sec_level);
int hci_conn_security(struct hci_conn *conn, __u8 sec_level, __u8 auth_type,
@@ -1439,6 +1441,9 @@ void hci_conn_del_sysfs(struct hci_conn *conn);
/* Use LL Privacy based address resolution if supported */
#define use_ll_privacy(dev) ((dev)->le_features[0] & HCI_LE_LL_PRIVACY)
+/* Use enhanced synchronous connection if command is supported */
+#define use_enhanced_sco_conn(dev) ((dev)->commands[29] & 0x08)
+
/* Use ext scanning if set ext scan param and ext scan enable is supported */
#define use_ext_scan(dev) (((dev)->commands[37] & 0x20) && \
((dev)->commands[37] & 0x40))
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2b5059a56cda..9569b21adb88 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -307,6 +307,97 @@ static bool find_next_esco_param(struct hci_conn *conn,
return conn->attempt <= size;
}
+bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
+{
+ struct hci_dev *hdev = conn->hdev;
+ struct hci_cp_enhanced_setup_sync_conn cp;
+ const struct sco_param *param;
+
+ bt_dev_dbg(hdev, "hcon %p", conn);
+
+ conn->state = BT_CONNECT;
+ conn->out = true;
+
+ conn->attempt++;
+
+ memset(&cp, 0x00, sizeof(cp));
+
+ cp.handle = cpu_to_le16(handle);
+
+ cp.tx_bandwidth = cpu_to_le32(0x00001f40);
+ cp.rx_bandwidth = cpu_to_le32(0x00001f40);
+
+ switch (conn->codec.id) {
+ case CODING_FORMAT_TRANSPARENT:
+ if (!find_next_esco_param(conn, esco_param_msbc,
+ ARRAY_SIZE(esco_param_msbc)))
+ return false;
+ param = &esco_param_msbc[conn->attempt - 1];
+ cp.tx_coding_format.id = 0x03;
+ cp.rx_coding_format.id = 0x03;
+ cp.tx_codec_frame_size = __cpu_to_le16(60);
+ cp.rx_codec_frame_size = __cpu_to_le16(60);
+ cp.in_bandwidth = __cpu_to_le32(0x1f40);
+ cp.out_bandwidth = __cpu_to_le32(0x1f40);
+ cp.in_coding_format.id = 0x03;
+ cp.out_coding_format.id = 0x03;
+ cp.in_coded_data_size = __cpu_to_le16(16);
+ cp.out_coded_data_size = __cpu_to_le16(16);
+ cp.in_pcm_data_format = 2;
+ cp.out_pcm_data_format = 2;
+ cp.in_pcm_sample_payload_msb_pos = 0;
+ cp.out_pcm_sample_payload_msb_pos = 0;
+ cp.in_data_path = conn->codec.data_path;
+ cp.out_data_path = conn->codec.data_path;
+ cp.in_trasnport_unit_size = 1;
+ cp.out_trasnport_unit_size = 1;
+ break;
+
+ case CODING_FORMAT_CVSD:
+ if (lmp_esco_capable(conn->link)) {
+ if (!find_next_esco_param(conn, esco_param_cvsd,
+ ARRAY_SIZE(esco_param_cvsd)))
+ return false;
+ param = &esco_param_cvsd[conn->attempt - 1];
+ } else {
+ if (conn->attempt > ARRAY_SIZE(sco_param_cvsd))
+ return false;
+ param = &sco_param_cvsd[conn->attempt - 1];
+ }
+ cp.tx_coding_format.id = 2;
+ cp.rx_coding_format.id = 2;
+ cp.tx_codec_frame_size = __cpu_to_le16(60);
+ cp.rx_codec_frame_size = __cpu_to_le16(60);
+ cp.in_bandwidth = __cpu_to_le32(16000);
+ cp.out_bandwidth = __cpu_to_le32(16000);
+ cp.in_coding_format.id = 4;
+ cp.out_coding_format.id = 4;
+ cp.in_coded_data_size = __cpu_to_le16(16);
+ cp.out_coded_data_size = __cpu_to_le16(16);
+ cp.in_pcm_data_format = 2;
+ cp.out_pcm_data_format = 2;
+ cp.in_pcm_sample_payload_msb_pos = 0;
+ cp.out_pcm_sample_payload_msb_pos = 0;
+ cp.in_data_path = conn->codec.data_path;
+ cp.out_data_path = conn->codec.data_path;
+ cp.in_trasnport_unit_size = 16;
+ cp.out_trasnport_unit_size = 16;
+ break;
+
+ default:
+ return false;
+ }
+
+ cp.retrans_effort = param->retrans_effort;
+ cp.pkt_type = __cpu_to_le16(param->pkt_type);
+ cp.max_latency = __cpu_to_le16(param->max_latency);
+
+ if (hci_send_cmd(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN, sizeof(cp), &cp) < 0)
+ return false;
+
+ return true;
+}
+
bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
{
struct hci_dev *hdev = conn->hdev;
@@ -424,10 +515,14 @@ void hci_sco_setup(struct hci_conn *conn, __u8 status)
BT_DBG("hcon %p", conn);
if (!status) {
- if (lmp_esco_capable(conn->hdev))
- hci_setup_sync(sco, conn->handle);
- else
+ if (lmp_esco_capable(conn->hdev)) {
+ if (use_enhanced_sco_conn(conn->hdev))
+ hci_enhanced_setup_sync(sco, conn->handle);
+ else
+ hci_setup_sync(sco, conn->handle);
+ } else {
hci_add_sco(sco, conn->handle);
+ }
} else {
hci_connect_cfm(sco, status);
hci_conn_del(sco);
@@ -1319,7 +1414,7 @@ struct hci_conn *hci_connect_acl(struct hci_dev *hdev, bdaddr_t *dst,
}
struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
- __u16 setting)
+ __u16 setting, struct bt_codec *codec)
{
struct hci_conn *acl;
struct hci_conn *sco;
@@ -1344,6 +1439,10 @@ struct hci_conn *hci_connect_sco(struct hci_dev *hdev, int type, bdaddr_t *dst,
hci_conn_hold(sco);
sco->setting = setting;
+ sco->codec.id = codec->id;
+ sco->codec.cid = codec->cid;
+ sco->codec.vid = codec->vid;
+ sco->codec.data_path = codec->data_path;
if (acl->state == BT_CONNECTED &&
(sco->state == BT_OPEN || sco->state == BT_CLOSED)) {
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 98ec486743ba..29a769a1a5e7 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -2236,6 +2236,41 @@ static void hci_cs_setup_sync_conn(struct hci_dev *hdev, __u8 status)
hci_dev_unlock(hdev);
}
+static void hci_cs_enhanced_setup_sync_conn(struct hci_dev *hdev, __u8 status)
+{
+ struct hci_cp_enhanced_setup_sync_conn *cp;
+ struct hci_conn *acl, *sco;
+ __u16 handle;
+
+ bt_dev_dbg(hdev, "status 0x%2.2x", status);
+
+ if (!status)
+ return;
+
+ cp = hci_sent_cmd_data(hdev, HCI_OP_ENHANCED_SETUP_SYNC_CONN);
+ if (!cp)
+ return;
+
+ handle = __le16_to_cpu(cp->handle);
+
+ bt_dev_dbg(hdev, "handle 0x%4.4x", handle);
+
+ hci_dev_lock(hdev);
+
+ acl = hci_conn_hash_lookup_handle(hdev, handle);
+ if (acl) {
+ sco = acl->link;
+ if (sco) {
+ sco->state = BT_CLOSED;
+
+ hci_connect_cfm(sco, status);
+ hci_conn_del(sco);
+ }
+ }
+
+ hci_dev_unlock(hdev);
+}
+
static void hci_cs_sniff_mode(struct hci_dev *hdev, __u8 status)
{
struct hci_cp_sniff_mode *cp;
@@ -3715,6 +3750,10 @@ static void hci_cmd_status_evt(struct hci_dev *hdev, struct sk_buff *skb,
hci_cs_setup_sync_conn(hdev, ev->status);
break;
+ case HCI_OP_ENHANCED_SETUP_SYNC_CONN:
+ hci_cs_enhanced_setup_sync_conn(hdev, ev->status);
+ break;
+
case HCI_OP_SNIFF_MODE:
hci_cs_sniff_mode(hdev, ev->status);
break;
@@ -4401,8 +4440,13 @@ static void hci_sync_conn_complete_evt(struct hci_dev *hdev,
if (conn->out) {
conn->pkt_type = (hdev->esco_type & SCO_ESCO_MASK) |
(hdev->esco_type & EDR_ESCO_MASK);
- if (hci_setup_sync(conn, conn->link->handle))
- goto unlock;
+ if (use_enhanced_sco_conn(hdev)) {
+ if (hci_enhanced_setup_sync(conn, conn->link->handle))
+ goto unlock;
+ } else {
+ if (hci_setup_sync(conn, conn->link->handle))
+ goto unlock;
+ }
}
fallthrough;
diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
index 5aa6808c1a22..f4eea0ae3af2 100644
--- a/net/bluetooth/sco.c
+++ b/net/bluetooth/sco.c
@@ -240,7 +240,7 @@ static int sco_connect(struct sock *sk)
}
hcon = hci_connect_sco(hdev, type, &sco_pi(sk)->dst,
- sco_pi(sk)->setting);
+ sco_pi(sk)->setting, &sco_pi(sk)->codec);
if (IS_ERR(hcon)) {
err = PTR_ERR(hcon);
goto done;
@@ -865,6 +865,15 @@ static int sco_sock_setsockopt(struct socket *sock, int level, int optname,
}
sco_pi(sk)->setting = voice.setting;
+ hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src,
+ BDADDR_BREDR);
+ if (!hdev) {
+ err = -EBADFD;
+ break;
+ }
+ if (use_enhanced_sco_conn(hdev) &&
+ voice.setting == BT_VOICE_TRANSPARENT)
+ sco_pi(sk)->codec.id = CODING_FORMAT_TRANSPARENT;
break;
case BT_PKT_STATUS:
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v9 10/10] Bluetooth: Add support for msbc coding format
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
` (7 preceding siblings ...)
2021-06-08 12:24 ` [PATCH v9 09/10] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command Kiran K
@ 2021-06-08 12:24 ` Kiran K
2021-06-15 19:43 ` Marcel Holtmann
2021-06-08 13:39 ` [v9,01/10] Bluetooth: enumerate local supported codec and cache details bluez.test.bot
` (2 subsequent siblings)
11 siblings, 1 reply; 31+ messages in thread
From: Kiran K @ 2021-06-08 12:24 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Kiran K
In Enhanced_Setup_Synchronous_Command, add support for msbc
coding format
Signed-off-by: Kiran K <kiran.k@intel.com>
Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
---
include/net/bluetooth/bluetooth.h | 1 +
net/bluetooth/hci_conn.c | 27 ++++++++++++++++++++++++++-
2 files changed, 27 insertions(+), 1 deletion(-)
diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
index af2809121571..056699224da7 100644
--- a/include/net/bluetooth/bluetooth.h
+++ b/include/net/bluetooth/bluetooth.h
@@ -175,6 +175,7 @@ struct bt_codecs {
#define CODING_FORMAT_CVSD 0x02
#define CODING_FORMAT_TRANSPARENT 0x03
+#define CODING_FORMAT_MSBC 0x05
__printf(1, 2)
void bt_info(const char *fmt, ...);
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 9569b21adb88..73c134459361 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -328,6 +328,32 @@ bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
cp.rx_bandwidth = cpu_to_le32(0x00001f40);
switch (conn->codec.id) {
+ case CODING_FORMAT_MSBC:
+ if (!find_next_esco_param(conn, esco_param_msbc,
+ ARRAY_SIZE(esco_param_msbc)))
+ return false;
+
+ param = &esco_param_msbc[conn->attempt - 1];
+ cp.tx_coding_format.id = 0x05;
+ cp.rx_coding_format.id = 0x05;
+ cp.tx_codec_frame_size = __cpu_to_le16(60);
+ cp.rx_codec_frame_size = __cpu_to_le16(60);
+ cp.in_bandwidth = __cpu_to_le32(32000);
+ cp.out_bandwidth = __cpu_to_le32(32000);
+ cp.in_coding_format.id = 0x04;
+ cp.out_coding_format.id = 0x04;
+ cp.in_coded_data_size = __cpu_to_le16(16);
+ cp.out_coded_data_size = __cpu_to_le16(16);
+ cp.in_pcm_data_format = 2;
+ cp.out_pcm_data_format = 2;
+ cp.in_pcm_sample_payload_msb_pos = 0;
+ cp.out_pcm_sample_payload_msb_pos = 0;
+ cp.in_data_path = conn->codec.data_path;
+ cp.out_data_path = conn->codec.data_path;
+ cp.in_trasnport_unit_size = 1;
+ cp.out_trasnport_unit_size = 1;
+ break;
+
case CODING_FORMAT_TRANSPARENT:
if (!find_next_esco_param(conn, esco_param_msbc,
ARRAY_SIZE(esco_param_msbc)))
@@ -383,7 +409,6 @@ bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
cp.in_trasnport_unit_size = 16;
cp.out_trasnport_unit_size = 16;
break;
-
default:
return false;
}
--
2.17.1
^ permalink raw reply related [flat|nested] 31+ messages in thread
* RE: [v9,01/10] Bluetooth: enumerate local supported codec and cache details
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
` (8 preceding siblings ...)
2021-06-08 12:24 ` [PATCH v9 10/10] Bluetooth: Add support for msbc coding format Kiran K
@ 2021-06-08 13:39 ` bluez.test.bot
2021-06-08 18:49 ` kernel test robot
2021-06-15 19:25 ` Marcel Holtmann
11 siblings, 0 replies; 31+ messages in thread
From: bluez.test.bot @ 2021-06-08 13:39 UTC (permalink / raw)
To: linux-bluetooth, kiran.k
[-- Attachment #1: Type: text/plain, Size: 2826 bytes --]
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=496181
---Test result---
Test Summary:
CheckPatch PASS 8.22 seconds
GitLint FAIL 1.15 seconds
BuildKernel PASS 689.43 seconds
TestRunner: Setup PASS 444.12 seconds
TestRunner: l2cap-tester PASS 3.26 seconds
TestRunner: bnep-tester PASS 2.18 seconds
TestRunner: mgmt-tester PASS 33.21 seconds
TestRunner: rfcomm-tester PASS 2.59 seconds
TestRunner: sco-tester PASS 2.41 seconds
TestRunner: smp-tester PASS 2.57 seconds
TestRunner: userchan-tester PASS 2.26 seconds
Details
##############################
Test: CheckPatch - PASS - 8.22 seconds
Run checkpatch.pl script with rule in .checkpatch.conf
##############################
Test: GitLint - FAIL - 1.15 seconds
Run gitlint with rule in .gitlint
Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command
1: T1 Title exceeds max length (76>72): "Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command"
##############################
Test: BuildKernel - PASS - 689.43 seconds
Build Kernel with minimal configuration supports Bluetooth
##############################
Test: TestRunner: Setup - PASS - 444.12 seconds
Setup environment for running Test Runner
##############################
Test: TestRunner: l2cap-tester - PASS - 3.26 seconds
Run test-runner with l2cap-tester
Total: 40, Passed: 40 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: bnep-tester - PASS - 2.18 seconds
Run test-runner with bnep-tester
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: mgmt-tester - PASS - 33.21 seconds
Run test-runner with mgmt-tester
Total: 446, Passed: 433 (97.1%), Failed: 0, Not Run: 13
##############################
Test: TestRunner: rfcomm-tester - PASS - 2.59 seconds
Run test-runner with rfcomm-tester
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: sco-tester - PASS - 2.41 seconds
Run test-runner with sco-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: smp-tester - PASS - 2.57 seconds
Run test-runner with smp-tester
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0
##############################
Test: TestRunner: userchan-tester - PASS - 2.26 seconds
Run test-runner with userchan-tester
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0
---
Regards,
Linux Bluetooth
[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 51530 bytes --]
[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3898 bytes --]
[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 612981 bytes --]
[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 14754 bytes --]
[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9911 bytes --]
[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11822 bytes --]
[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 6478 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
@ 2021-06-08 18:49 ` kernel test robot
2021-06-08 12:24 ` [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path Kiran K
` (10 subsequent siblings)
11 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2021-06-08 18:49 UTC (permalink / raw)
To: Kiran K, linux-bluetooth
Cc: kbuild-all, Kiran K, Chethan T N, Srivatsa Ravishankar
[-- Attachment #1: Type: text/plain, Size: 4052 bytes --]
Hi Kiran,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on bluetooth/master v5.13-rc5 next-20210608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kiran-K/Bluetooth-enumerate-local-supported-codec-and-cache-details/20210608-202239
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: mips-randconfig-s032-20210608 (attached as .config)
compiler: mipsel-linux-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
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/38ec55e0fc2fabf67d0b5f151700afae12db44f7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kiran-K/Bluetooth-enumerate-local-supported-codec-and-cache-details/20210608-202239
git checkout 38ec55e0fc2fabf67d0b5f151700afae12db44f7
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
command-line: note: in included file:
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_ACQUIRE redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_SEQ_CST redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_ACQ_REL redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_RELEASE redefined
builtin:0:0: sparse: this was the original definition
net/bluetooth/hci_core.c: note: in included file:
include/net/bluetooth/hci_core.h:142:35: sparse: sparse: array of flexible structures
>> net/bluetooth/hci_core.c:920:28: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] cid @@ got unsigned short [usertype] @@
net/bluetooth/hci_core.c:920:28: sparse: expected restricted __le16 [usertype] cid
net/bluetooth/hci_core.c:920:28: sparse: got unsigned short [usertype]
>> net/bluetooth/hci_core.c:921:28: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] vid @@ got unsigned short [usertype] @@
net/bluetooth/hci_core.c:921:28: sparse: expected restricted __le16 [usertype] vid
net/bluetooth/hci_core.c:921:28: sparse: got unsigned short [usertype]
vim +920 net/bluetooth/hci_core.c
905
906 static int hci_codec_list_add(struct list_head *list,
907 struct hci_op_read_local_codec_caps *sent,
908 struct hci_rp_read_local_codec_caps *rp,
909 void *caps,
910 __u32 len)
911 {
912 struct codec_list *entry;
913
914 entry = kzalloc(sizeof(*entry) + len, GFP_KERNEL);
915 if (!entry)
916 return -ENOMEM;
917
918 entry->id = sent->id;
919 if (sent->id == 0xFF) {
> 920 entry->cid = __le16_to_cpu(sent->cid);
> 921 entry->vid = __le16_to_cpu(sent->vid);
922 }
923 entry->transport = sent->transport;
924 entry->len = len;
925 entry->num_caps = rp->num_caps;
926 if (rp->num_caps)
927 memcpy(entry->caps, caps, len);
928 list_add(&entry->list, list);
929
930 return 0;
931 }
932
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24016 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details
@ 2021-06-08 18:49 ` kernel test robot
0 siblings, 0 replies; 31+ messages in thread
From: kernel test robot @ 2021-06-08 18:49 UTC (permalink / raw)
To: kbuild-all
[-- Attachment #1: Type: text/plain, Size: 4137 bytes --]
Hi Kiran,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on bluetooth-next/master]
[also build test WARNING on bluetooth/master v5.13-rc5 next-20210608]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Kiran-K/Bluetooth-enumerate-local-supported-codec-and-cache-details/20210608-202239
base: https://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git master
config: mips-randconfig-s032-20210608 (attached as .config)
compiler: mipsel-linux-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
# apt-get install sparse
# sparse version: v0.6.3-341-g8af24329-dirty
# https://github.com/0day-ci/linux/commit/38ec55e0fc2fabf67d0b5f151700afae12db44f7
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Kiran-K/Bluetooth-enumerate-local-supported-codec-and-cache-details/20210608-202239
git checkout 38ec55e0fc2fabf67d0b5f151700afae12db44f7
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' W=1 ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
sparse warnings: (new ones prefixed by >>)
command-line: note: in included file:
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_ACQUIRE redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_SEQ_CST redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_ACQ_REL redefined
builtin:0:0: sparse: this was the original definition
builtin:1:9: sparse: sparse: preprocessor token __ATOMIC_RELEASE redefined
builtin:0:0: sparse: this was the original definition
net/bluetooth/hci_core.c: note: in included file:
include/net/bluetooth/hci_core.h:142:35: sparse: sparse: array of flexible structures
>> net/bluetooth/hci_core.c:920:28: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] cid @@ got unsigned short [usertype] @@
net/bluetooth/hci_core.c:920:28: sparse: expected restricted __le16 [usertype] cid
net/bluetooth/hci_core.c:920:28: sparse: got unsigned short [usertype]
>> net/bluetooth/hci_core.c:921:28: sparse: sparse: incorrect type in assignment (different base types) @@ expected restricted __le16 [usertype] vid @@ got unsigned short [usertype] @@
net/bluetooth/hci_core.c:921:28: sparse: expected restricted __le16 [usertype] vid
net/bluetooth/hci_core.c:921:28: sparse: got unsigned short [usertype]
vim +920 net/bluetooth/hci_core.c
905
906 static int hci_codec_list_add(struct list_head *list,
907 struct hci_op_read_local_codec_caps *sent,
908 struct hci_rp_read_local_codec_caps *rp,
909 void *caps,
910 __u32 len)
911 {
912 struct codec_list *entry;
913
914 entry = kzalloc(sizeof(*entry) + len, GFP_KERNEL);
915 if (!entry)
916 return -ENOMEM;
917
918 entry->id = sent->id;
919 if (sent->id == 0xFF) {
> 920 entry->cid = __le16_to_cpu(sent->cid);
> 921 entry->vid = __le16_to_cpu(sent->vid);
922 }
923 entry->transport = sent->transport;
924 entry->len = len;
925 entry->num_caps = rp->num_caps;
926 if (rp->num_caps)
927 memcpy(entry->caps, caps, len);
928 list_add(&entry->list, list);
929
930 return 0;
931 }
932
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 24016 bytes --]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
` (10 preceding siblings ...)
2021-06-08 18:49 ` kernel test robot
@ 2021-06-15 19:25 ` Marcel Holtmann
2021-06-16 2:51 ` K, Kiran
11 siblings, 1 reply; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-15 19:25 UTC (permalink / raw)
To: Kiran K; +Cc: Bluez mailing list, Chethan T N, Srivatsa Ravishankar
Hi Kiran,
> Move reading of supported local codecs into a separate init function,
> query codecs capabilities and cache the data
>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> Reported-by: kernel test robot <lkp@intel.com>
what is Reported-by? This makes no sense since this is original code.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path
2021-06-08 12:24 ` [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path Kiran K
@ 2021-06-15 19:26 ` Marcel Holtmann
2021-06-16 2:56 ` K, Kiran
0 siblings, 1 reply; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-15 19:26 UTC (permalink / raw)
To: Kiran K; +Cc: linux-bluetooth
Hi Kiran,
> There is no standard HCI command to retrieve data path for transport.
> Add a new callback function to retrieve data path which is used
> in offload usecase. This needs to be set at setup stage if controller
> supports offload codecs
>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> ---
> * changes in v9:
> - define a separate patch for core changes
>
> include/net/bluetooth/hci_core.h | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 3284044c3dd7..641477396da3 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -617,6 +617,7 @@ struct hci_dev {
> int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> void (*cmd_timeout)(struct hci_dev *hdev);
> bool (*prevent_wake)(struct hci_dev *hdev);
> + int (*get_data_path)(struct hci_dev *hdev);
> };
and where is the code using hdev->get_data_path. That code needs to be in this patch.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback
2021-06-08 12:24 ` [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback Kiran K
@ 2021-06-15 19:30 ` Marcel Holtmann
2021-06-16 3:02 ` K, Kiran
0 siblings, 1 reply; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-15 19:30 UTC (permalink / raw)
To: Kiran K; +Cc: linux-bluetooth
Hi Kiran,
> Read supported offload usecases and set get_data_path callback if
> controller suppports offload codecs.
>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> ---
> drivers/bluetooth/btintel.c | 36 ++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h | 18 ++++++++++++++++++
> drivers/bluetooth/btusb.c | 8 ++++++++
> 3 files changed, 62 insertions(+)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index e44b6993cf91..95c6a1bef35e 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1272,6 +1272,42 @@ int btintel_set_debug_features(struct hci_dev *hdev,
> }
> EXPORT_SYMBOL_GPL(btintel_set_debug_features);
>
> +int btintel_get_data_path(struct hci_dev *hdev)
> +{
> + return 1;
> +}
> +EXPORT_SYMBOL_GPL(btintel_get_data_path);
> +
> +int btintel_read_offload_usecases(struct hci_dev *hdev,
> + struct intel_offload_usecases *usecases)
> +{
> + struct sk_buff *skb;
> + int err = 0;
> +
> + skb = __hci_cmd_sync(hdev, 0xfc86, 0, NULL, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev, "Reading offload usecases failed (%ld)",
> + PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> +
> + if (skb->len < sizeof(*usecases)) {
> + err = -EIO;
> + goto error;
> + }
> +
> + if (skb->data[0]) {
> + err = -bt_to_errno(skb->data[0]);
> + goto error;
> + }
> +
> + memcpy(usecases, skb->data, sizeof(*usecases));
> +error:
> + kfree_skb(skb);
> + return err;
> +}
> +EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
> +
> MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
> MODULE_VERSION(VERSION);
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index d184064a5e7c..9bcc489680db 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -132,6 +132,11 @@ struct intel_debug_features {
> __u8 page1[16];
> } __packed;
>
> +struct intel_offload_usecases {
> + __u8 status;
> + __u8 preset[8];
> +} __packed;
> +
> #define INTEL_HW_PLATFORM(cnvx_bt) ((u8)(((cnvx_bt) & 0x0000ff00) >> 8))
> #define INTEL_HW_VARIANT(cnvx_bt) ((u8)(((cnvx_bt) & 0x003f0000) >> 16))
> #define INTEL_CNVX_TOP_TYPE(cnvx_top) ((cnvx_top) & 0x00000fff)
> @@ -175,6 +180,9 @@ int btintel_read_debug_features(struct hci_dev *hdev,
> struct intel_debug_features *features);
> int btintel_set_debug_features(struct hci_dev *hdev,
> const struct intel_debug_features *features);
> +int btintel_read_offload_usecases(struct hci_dev *hdev,
> + struct intel_offload_usecases *usecases);
> +int btintel_get_data_path(struct hci_dev *hdev);
> #else
>
> static inline int btintel_check_bdaddr(struct hci_dev *hdev)
> @@ -307,4 +315,14 @@ static inline int btintel_set_debug_features(struct hci_dev *hdev,
> return -EOPNOTSUPP;
> }
>
> +static int btintel_read_offload_usecases(struct hci_dev *hdev,
> + struct intel_offload_usecases *usecases)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static int btintel_get_data_path(struct hci_dev *hdev)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index a9855a2dd561..1e51beec5776 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -2952,6 +2952,7 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
> int err;
> struct intel_debug_features features;
> struct intel_version_tlv version;
> + struct intel_offload_usecases usecases;
>
> bt_dev_dbg(hdev, "");
>
> @@ -3008,6 +3009,13 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
> /* Set DDC mask for available debug features */
> btintel_set_debug_features(hdev, &features);
>
> + err = btintel_read_offload_usecases(hdev, &usecases);
> + if (!err) {
> + /* set get_data_path callback if offload is supported */
> + if (usecases.preset[0] & 0x03)
> + hdev->get_data_path = btintel_get_data_path;
> + }
> +
I really wonder if this exporting everything single detail to be just used by btusb is really a good idea. Would it be just enough to have a btintel_configure_offload(hdev); helper that does everything and be done with it. There is already too much Intel specifics bleeding into btusb.c and I think we need to correct that going forward.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 05/10] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket
2021-06-08 12:24 ` [PATCH v9 05/10] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket Kiran K
@ 2021-06-15 19:37 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-15 19:37 UTC (permalink / raw)
To: Kiran K; +Cc: linux-bluetooth
Hi Kiran,
> Add BT_CODEC option for getsockopt systemcall over SCO socket
> to expose the codecs supported by controller
>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> ---
> * changes on v9:
> - fix typos,review comments, remove quirk
>
> include/net/bluetooth/bluetooth.h | 20 ++++++
> include/net/bluetooth/hci.h | 4 ++
> net/bluetooth/sco.c | 111 +++++++++++++++++++++++++++++-
> 3 files changed, 134 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index 9125effbf448..1840756958ce 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -153,6 +153,26 @@ struct bt_voice {
>
> #define BT_SCM_PKT_STATUS 0x03
>
> +#define BT_CODEC 19
> +
> +struct bt_codec_caps {
> + __u8 len;
> + __u8 data[];
> +} __packed;
> +
> +struct bt_codec {
> + __u8 id;
> + __le16 cid;
> + __le16 vid;
> + __u8 data_path;
> + __u8 num_caps;
> +} __packed;
> +
> +struct bt_codecs {
> + __u8 num_codecs;
> + struct bt_codec codecs[];
> +} __packed;
> +
> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> __printf(1, 2)
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 45bd9af4ce61..31a5ac8918fc 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -2619,6 +2619,10 @@ static inline struct hci_sco_hdr *hci_sco_hdr(const struct sk_buff *skb)
> #define hci_iso_data_len(h) ((h) & 0x3fff)
> #define hci_iso_data_flags(h) ((h) >> 14)
>
> +/* codec transport types */
> +#define TRANSPORT_ACL 0x00
> +#define TRANSPORT_SCO_ESCO 0x01
> +
> /* le24 support */
> static inline void hci_cpu_to_le24(__u32 val, __u8 dst[3])
> {
> diff --git a/net/bluetooth/sco.c b/net/bluetooth/sco.c
> index d9a4e88dacbb..98d5e24e5680 100644
> --- a/net/bluetooth/sco.c
> +++ b/net/bluetooth/sco.c
> @@ -944,10 +944,15 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> char __user *optval, int __user *optlen)
> {
> struct sock *sk = sock->sk;
> - int len, err = 0;
> + int len, err = 0, buf_len;
> struct bt_voice voice;
> u32 phys;
> int pkt_status;
> + struct codec_list *c;
> + u8 num_codecs, i, __user *ptr;
> + struct hci_dev *hdev;
> + struct hci_codec_caps *caps;
> + __u8 data_path;
>
> BT_DBG("sk %p", sk);
>
> @@ -1012,6 +1017,110 @@ static int sco_sock_getsockopt(struct socket *sock, int level, int optname,
> err = -EFAULT;
> break;
>
> + case BT_CODEC:
> + num_codecs = 0;
> + buf_len = 0;
> +
> + hdev = hci_get_route(&sco_pi(sk)->dst, &sco_pi(sk)->src, BDADDR_BREDR);
> + if (!hdev) {
> + err = -EBADFD;
> + break;
> + }
> +
> + if (!hdev->get_data_path) {
> + err = -EOPNOTSUPP;
> + break;
> + }
> +
> + hci_dev_lock(hdev);
> + /* find total buffer size required to copy codec + caps */
please check for simple style mistakes like double spaces. Also I would put the comment above the hci_dev_lock().
> + list_for_each_entry(c, &hdev->local_codecs, list) {
> + if (c->transport != TRANSPORT_SCO_ESCO)
> + continue;
> + num_codecs++;
> + for (i = 0, caps = c->caps; i < c->num_caps; i++) {
> + buf_len += 1 + caps->len;
> + caps = (void *)&caps->data[caps->len];
> + }
> + buf_len += sizeof(struct bt_codec);
> + }
> + hci_dev_unlock(hdev);
> +
> + buf_len += sizeof(struct bt_codecs);
> + if (buf_len > len) {
> + err = -ENOBUFS;
> + break;
> + }
> + ptr = optval;
> +
> + if (put_user(num_codecs, ptr)) {
> + err = -EFAULT;
> + break;
> + }
> + ptr += sizeof(num_codecs);
> +
And this is missing comment as well.
> + hci_dev_lock(hdev);
> + list_for_each_entry(c, &hdev->local_codecs, list) {
> + if (c->transport != TRANSPORT_SCO_ESCO)
> + continue;
> +
> + if (put_user(c->id, ptr)) {
> + err = -EFAULT;
> + goto unlock;
> + }
> + ptr += sizeof(c->id);
> +
> + if (put_user(c->cid, ptr)) {
> + err = -EFAULT;
> + goto unlock;
> + }
> + ptr += sizeof(c->cid);
> +
> + if (put_user(c->vid, ptr)) {
> + err = -EFAULT;
> + goto unlock;
> + }
> + ptr += sizeof(c->vid);
> +
> + err = hdev->get_data_path(hdev);
> + if (err < 0) {
> + err = -EFAULT;
> + goto unlock;
> + }
Using the variable name err is really bad here. It is also not an EFAULT type of error. They are really specific. I really don’t get why not prepare the data in advance and have a single put_user call.
> +
> + data_path = (__u8)err;
> + if (put_user(data_path, ptr)) {
> + err = -EFAULT;
> + goto unlock;
> + }
> + ptr += sizeof(data_path);
> +
> + if (put_user(c->num_caps, ptr)) {
> + err = -EFAULT;
> + goto unlock;
> + }
> + ptr += sizeof(c->num_caps);
> +
> + len = 0;
> + for (i = 0, caps = c->caps; i < c->num_caps; i++) {
> + len += 1 + caps->len;
> + caps = (void *)&caps->data[caps->len];
> + }
> +
> + if (len && copy_to_user(ptr, c->caps, len)) {
> + err = -EFAULT;
> + goto unlock;
> + }
> + ptr += len;
> + }
> +
> + if (put_user(buf_len, optlen))
> + err = -EFAULT;
> +unlock:
> + hci_dev_unlock(hdev);
Jumping from within a for-loop is nothing something that I actually like. It is better you break out of via break.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 06/10] Bluetooth: Add a callback function to set data path
2021-06-08 12:24 ` [PATCH v9 06/10] Bluetooth: Add a callback function to set data path Kiran K
@ 2021-06-15 19:37 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-15 19:37 UTC (permalink / raw)
To: Kiran K; +Cc: linux-bluetooth
Hi Kiran,
> In HFP offload usecase, Intel controllers require offload use
> case id (NBS or WBS) to be set before opening SCO connection. Define
> a new callback which gets called on setsockopt SCO socket. User space
> audio module is expected to set codec via setsockopt(sk, BT_CODEC, ....)
> before opening SCO connection.
>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> ---
> include/net/bluetooth/hci_core.h | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
> index 641477396da3..ad0024891447 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -618,6 +618,8 @@ struct hci_dev {
> void (*cmd_timeout)(struct hci_dev *hdev);
> bool (*prevent_wake)(struct hci_dev *hdev);
> int (*get_data_path)(struct hci_dev *hdev);
> + int (*set_data_path)(struct hci_dev *hdev, __u8 type,
> + struct bt_codec *codec);
> };
>
same as the other one, this needs to also provide the user of hdev->set_data_path.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data path
2021-06-08 12:24 ` [PATCH v9 07/10] Bluetooth: btintel: define callback " Kiran K
@ 2021-06-15 19:39 ` Marcel Holtmann
2021-06-16 3:10 ` K, Kiran
0 siblings, 1 reply; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-15 19:39 UTC (permalink / raw)
To: Kiran K; +Cc: linux-bluetooth
Hi Kiran,
> Adds callback function which is called to set the data path
> for HFP offload case before opening SCO connection
>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> ---
> drivers/bluetooth/btintel.c | 50 +++++++++++++++++++++++++++++++++++++
> drivers/bluetooth/btintel.h | 8 ++++++
> drivers/bluetooth/btusb.c | 4 ++-
> include/net/bluetooth/hci.h | 8 ++++++
> 4 files changed, 69 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> index 95c6a1bef35e..3eba5c587ef6 100644
> --- a/drivers/bluetooth/btintel.c
> +++ b/drivers/bluetooth/btintel.c
> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct hci_dev *hdev,
> }
> EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
>
> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> + struct bt_codec *codec)
> +{
> + __u8 preset;
> + struct hci_op_configure_data_path *cmd;
> + __u8 buffer[255];
> + struct sk_buff *skb;
> +
> + if (type != SCO_LINK && type != ESCO_LINK)
> + return -EINVAL;
> +
> + switch (codec->id) {
> + case 0x02:
> + preset = 0x00;
> + break;
> + case 0x05:
> + preset = 0x01;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + cmd = (void *)buffer;
> + cmd->data_path_id = 0x01;
> + cmd->len = 1;
> + cmd->data[0] = preset;
> +
> + cmd->direction = 0x00;
> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH, sizeof(*cmd) + 1,
> + cmd, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev, "configure input data path failed (%ld)",
> + PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> + kfree_skb(skb);
> +
> + cmd->direction = 0x01;
> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH, sizeof(*cmd) + 1,
> + cmd, HCI_INIT_TIMEOUT);
> + if (IS_ERR(skb)) {
> + bt_dev_err(hdev, "configure output data path failed (%ld)",
> + PTR_ERR(skb));
> + return PTR_ERR(skb);
> + }
> + kfree_skb(skb);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(btintel_set_data_path);
> +
> MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver " VERSION);
> MODULE_VERSION(VERSION);
> diff --git a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h
> index 9bcc489680db..9806970c9871 100644
> --- a/drivers/bluetooth/btintel.h
> +++ b/drivers/bluetooth/btintel.h
> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev *hdev,
> int btintel_read_offload_usecases(struct hci_dev *hdev,
> struct intel_offload_usecases *usecases);
> int btintel_get_data_path(struct hci_dev *hdev);
> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> + struct bt_codec *codec);
> #else
>
> static inline int btintel_check_bdaddr(struct hci_dev *hdev)
> @@ -325,4 +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev)
> {
> return -EOPNOTSUPP;
> }
> +
> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> + struct bt_codec *codec)
> +{
> + return -EOPNOTSUPP;
> +}
> #endif
> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> index 1e51beec5776..afafa44752a1 100644
> --- a/drivers/bluetooth/btusb.c
> +++ b/drivers/bluetooth/btusb.c
> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct hci_dev *hdev)
> err = btintel_read_offload_usecases(hdev, &usecases);
> if (!err) {
> /* set get_data_path callback if offload is supported */
> - if (usecases.preset[0] & 0x03)
> + if (usecases.preset[0] & 0x03) {
> hdev->get_data_path = btintel_get_data_path;
> + hdev->set_data_path = btintel_set_data_path;
> + }
> }
> /* Read the Intel version information after loading the FW */
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 31a5ac8918fc..42963188dcea 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {
> __u8 rand256[16];
> } __packed;
>
> +#define HCI_CONFIGURE_DATA_PATH 0x0c83
> +struct hci_op_configure_data_path {
> + __u8 direction;
> + __u8 data_path_id;
> + __u8 len;
> + __u8 data[];
> +} __packed;
> +
if this is a standard HCI command, why is this done as hdev->set_data_path? This makes no sense too me.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 10/10] Bluetooth: Add support for msbc coding format
2021-06-08 12:24 ` [PATCH v9 10/10] Bluetooth: Add support for msbc coding format Kiran K
@ 2021-06-15 19:43 ` Marcel Holtmann
2021-06-23 3:24 ` K, Kiran
0 siblings, 1 reply; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-15 19:43 UTC (permalink / raw)
To: Kiran K; +Cc: linux-bluetooth
Hi Kiran,
> In Enhanced_Setup_Synchronous_Command, add support for msbc
> coding format
>
> Signed-off-by: Kiran K <kiran.k@intel.com>
> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> ---
> include/net/bluetooth/bluetooth.h | 1 +
> net/bluetooth/hci_conn.c | 27 ++++++++++++++++++++++++++-
> 2 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/include/net/bluetooth/bluetooth.h b/include/net/bluetooth/bluetooth.h
> index af2809121571..056699224da7 100644
> --- a/include/net/bluetooth/bluetooth.h
> +++ b/include/net/bluetooth/bluetooth.h
> @@ -175,6 +175,7 @@ struct bt_codecs {
>
> #define CODING_FORMAT_CVSD 0x02
> #define CODING_FORMAT_TRANSPARENT 0x03
> +#define CODING_FORMAT_MSBC 0x05
>
> __printf(1, 2)
> void bt_info(const char *fmt, ...);
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 9569b21adb88..73c134459361 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -328,6 +328,32 @@ bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
> cp.rx_bandwidth = cpu_to_le32(0x00001f40);
>
> switch (conn->codec.id) {
> + case CODING_FORMAT_MSBC:
> + if (!find_next_esco_param(conn, esco_param_msbc,
> + ARRAY_SIZE(esco_param_msbc)))
> + return false;
> +
> + param = &esco_param_msbc[conn->attempt - 1];
> + cp.tx_coding_format.id = 0x05;
> + cp.rx_coding_format.id = 0x05;
> + cp.tx_codec_frame_size = __cpu_to_le16(60);
> + cp.rx_codec_frame_size = __cpu_to_le16(60);
> + cp.in_bandwidth = __cpu_to_le32(32000);
> + cp.out_bandwidth = __cpu_to_le32(32000);
> + cp.in_coding_format.id = 0x04;
> + cp.out_coding_format.id = 0x04;
> + cp.in_coded_data_size = __cpu_to_le16(16);
> + cp.out_coded_data_size = __cpu_to_le16(16);
> + cp.in_pcm_data_format = 2;
> + cp.out_pcm_data_format = 2;
> + cp.in_pcm_sample_payload_msb_pos = 0;
> + cp.out_pcm_sample_payload_msb_pos = 0;
> + cp.in_data_path = conn->codec.data_path;
> + cp.out_data_path = conn->codec.data_path;
> + cp.in_trasnport_unit_size = 1;
> + cp.out_trasnport_unit_size = 1;
so s/trasnport/transport/
Please spellcheck your structs.
> + break;
> +
> case CODING_FORMAT_TRANSPARENT:
> if (!find_next_esco_param(conn, esco_param_msbc,
> ARRAY_SIZE(esco_param_msbc)))
> @@ -383,7 +409,6 @@ bool hci_enhanced_setup_sync(struct hci_conn *conn, __u16 handle)
> cp.in_trasnport_unit_size = 16;
> cp.out_trasnport_unit_size = 16;
> break;
> -
We can not have these random hunks in patches. You need to review your final set before sending it out.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details
2021-06-15 19:25 ` Marcel Holtmann
@ 2021-06-16 2:51 ` K, Kiran
2021-06-16 5:15 ` Marcel Holtmann
0 siblings, 1 reply; 31+ messages in thread
From: K, Kiran @ 2021-06-16 2:51 UTC (permalink / raw)
To: Marcel Holtmann
Cc: Bluez mailing list, Tumkur Narayan, Chethan, Srivatsa, Ravishankar
Hi Marcel,
>
> Hi Kiran,
>
> > Move reading of supported local codecs into a separate init function,
> > query codecs capabilities and cache the data
> >
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> > Reported-by: kernel test robot <lkp@intel.com>
>
> what is Reported-by? This makes no sense since this is original code.
Got a compiler warning in one of the patchset. Hence added "Reported-by". Ok to remove this in the next patchset.
>
> Regards
>
> Marcel
Regards,
Kiran
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path
2021-06-15 19:26 ` Marcel Holtmann
@ 2021-06-16 2:56 ` K, Kiran
2021-06-16 5:17 ` Marcel Holtmann
0 siblings, 1 reply; 31+ messages in thread
From: K, Kiran @ 2021-06-16 2:56 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
> Hi Kiran,
>
> > There is no standard HCI command to retrieve data path for transport.
> > Add a new callback function to retrieve data path which is used in
> > offload usecase. This needs to be set at setup stage if controller
> > supports offload codecs
> >
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> > ---
> > * changes in v9:
> > - define a separate patch for core changes
> >
> > include/net/bluetooth/hci_core.h | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/include/net/bluetooth/hci_core.h
> > b/include/net/bluetooth/hci_core.h
> > index 3284044c3dd7..641477396da3 100644
> > --- a/include/net/bluetooth/hci_core.h
> > +++ b/include/net/bluetooth/hci_core.h
> > @@ -617,6 +617,7 @@ struct hci_dev {
> > int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
> > void (*cmd_timeout)(struct hci_dev *hdev);
> > bool (*prevent_wake)(struct hci_dev *hdev);
> > + int (*get_data_path)(struct hci_dev *hdev);
> > };
>
> and where is the code using hdev->get_data_path. That code needs to be in
> this patch.
In the previous patchset, there was a comment to separate out driver and core changes. Let me know if I am missing something here.
https://patchwork.kernel.org/project/bluetooth/patch/20210518104232.5431-3-kiran.k@intel.com/
>
> Regards
>
> Marcel
Regards,
Kiran
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback
2021-06-15 19:30 ` Marcel Holtmann
@ 2021-06-16 3:02 ` K, Kiran
0 siblings, 0 replies; 31+ messages in thread
From: K, Kiran @ 2021-06-16 3:02 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
> Hi Kiran,
>
> > Read supported offload usecases and set get_data_path callback if
> > controller suppports offload codecs.
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index a9855a2dd561..1e51beec5776 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -2952,6 +2952,7 @@ static int btusb_setup_intel_newgen(struct
> hci_dev *hdev)
> > int err;
> > struct intel_debug_features features;
> > struct intel_version_tlv version;
> > + struct intel_offload_usecases usecases;
> >
> > bt_dev_dbg(hdev, "");
> >
> > @@ -3008,6 +3009,13 @@ static int btusb_setup_intel_newgen(struct
> hci_dev *hdev)
> > /* Set DDC mask for available debug features */
> > btintel_set_debug_features(hdev, &features);
> >
> > + err = btintel_read_offload_usecases(hdev, &usecases);
> > + if (!err) {
> > + /* set get_data_path callback if offload is supported */
> > + if (usecases.preset[0] & 0x03)
> > + hdev->get_data_path = btintel_get_data_path;
> > + }
> > +
>
> I really wonder if this exporting everything single detail to be just used by
> btusb is really a good idea. Would it be just enough to have a
> btintel_configure_offload(hdev); helper that does everything and be done
> with it. There is already too much Intel specifics bleeding into btusb.c and I
> think we need to correct that going forward.
>
Ok. I will refactor this to move Intel specific code to btintel*
> Regards
>
> Marcel
Thanks,
Kiran
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data path
2021-06-15 19:39 ` Marcel Holtmann
@ 2021-06-16 3:10 ` K, Kiran
2021-06-16 5:18 ` Marcel Holtmann
0 siblings, 1 reply; 31+ messages in thread
From: K, Kiran @ 2021-06-16 3:10 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
>
> Hi Kiran,
>
> > Adds callback function which is called to set the data path for HFP
> > offload case before opening SCO connection
> >
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> > ---
> > drivers/bluetooth/btintel.c | 50
> +++++++++++++++++++++++++++++++++++++
> > drivers/bluetooth/btintel.h | 8 ++++++
> > drivers/bluetooth/btusb.c | 4 ++-
> > include/net/bluetooth/hci.h | 8 ++++++
> > 4 files changed, 69 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
> > index 95c6a1bef35e..3eba5c587ef6 100644
> > --- a/drivers/bluetooth/btintel.c
> > +++ b/drivers/bluetooth/btintel.c
> > @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct
> > hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
> >
> > +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> > + struct bt_codec *codec)
> > +{
> > + __u8 preset;
> > + struct hci_op_configure_data_path *cmd;
> > + __u8 buffer[255];
> > + struct sk_buff *skb;
> > +
> > + if (type != SCO_LINK && type != ESCO_LINK)
> > + return -EINVAL;
> > +
> > + switch (codec->id) {
> > + case 0x02:
> > + preset = 0x00;
> > + break;
> > + case 0x05:
> > + preset = 0x01;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + cmd = (void *)buffer;
> > + cmd->data_path_id = 0x01;
> > + cmd->len = 1;
> > + cmd->data[0] = preset;
> > +
> > + cmd->direction = 0x00;
> > + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
> sizeof(*cmd) + 1,
> > + cmd, HCI_INIT_TIMEOUT);
> > + if (IS_ERR(skb)) {
> > + bt_dev_err(hdev, "configure input data path failed (%ld)",
> > + PTR_ERR(skb));
> > + return PTR_ERR(skb);
> > + }
> > + kfree_skb(skb);
> > +
> > + cmd->direction = 0x01;
> > + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
> sizeof(*cmd) + 1,
> > + cmd, HCI_INIT_TIMEOUT);
> > + if (IS_ERR(skb)) {
> > + bt_dev_err(hdev, "configure output data path failed (%ld)",
> > + PTR_ERR(skb));
> > + return PTR_ERR(skb);
> > + }
> > + kfree_skb(skb);
> > + return 0;
> > +}
> > +EXPORT_SYMBOL_GPL(btintel_set_data_path);
> > +
> > MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
> > MODULE_DESCRIPTION("Bluetooth support for Intel devices ver "
> > VERSION); MODULE_VERSION(VERSION); diff --git
> > a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index
> > 9bcc489680db..9806970c9871 100644
> > --- a/drivers/bluetooth/btintel.h
> > +++ b/drivers/bluetooth/btintel.h
> > @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev
> > *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,
> > struct intel_offload_usecases *usecases); int
> > btintel_get_data_path(struct hci_dev *hdev);
> > +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> > + struct bt_codec *codec);
> > #else
> >
> > static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@ -325,4
> > +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {
> > return -EOPNOTSUPP;
> > }
> > +
> > +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> > + struct bt_codec *codec)
> > +{
> > + return -EOPNOTSUPP;
> > +}
> > #endif
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 1e51beec5776..afafa44752a1 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct
> hci_dev *hdev)
> > err = btintel_read_offload_usecases(hdev, &usecases);
> > if (!err) {
> > /* set get_data_path callback if offload is supported */
> > - if (usecases.preset[0] & 0x03)
> > + if (usecases.preset[0] & 0x03) {
> > hdev->get_data_path = btintel_get_data_path;
> > + hdev->set_data_path = btintel_set_data_path;
> > + }
> > }
>
> > /* Read the Intel version information after loading the FW */ diff
> > --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> > index 31a5ac8918fc..42963188dcea 100644
> > --- a/include/net/bluetooth/hci.h
> > +++ b/include/net/bluetooth/hci.h
> > @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {
> > __u8 rand256[16];
> > } __packed;
> >
> > +#define HCI_CONFIGURE_DATA_PATH 0x0c83
> > +struct hci_op_configure_data_path {
> > + __u8 direction;
> > + __u8 data_path_id;
> > + __u8 len;
> > + __u8 data[];
> > +} __packed;
> > +
>
> if this is a standard HCI command, why is this done as hdev->set_data_path?
> This makes no sense too me.
Intel uses HCI_CONFIGURE_DATA_PATH to command to set the preset ID (NBS, WBS, ...). Here len and data[] are vendor specific. I should have prefixed these fields with vnd_. I will address this in next patchset.
>
> Regards
>
> Marcel
Thanks,
Kiran
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details
2021-06-16 2:51 ` K, Kiran
@ 2021-06-16 5:15 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-16 5:15 UTC (permalink / raw)
To: K, Kiran
Cc: Bluez mailing list, Tumkur Narayan, Chethan, Srivatsa, Ravishankar
Hi Kiran,
>>> Move reading of supported local codecs into a separate init function,
>>> query codecs capabilities and cache the data
>>>
>>> Signed-off-by: Kiran K <kiran.k@intel.com>
>>> Signed-off-by: Chethan T N <chethan.tumkur.narayan@intel.com>
>>> Signed-off-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
>>> Reported-by: kernel test robot <lkp@intel.com>
>>
>> what is Reported-by? This makes no sense since this is original code.
>
> Got a compiler warning in one of the patchset. Hence added "Reported-by". Ok to remove this in the next patchset.
yes since they have not been yet upstream.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path
2021-06-16 2:56 ` K, Kiran
@ 2021-06-16 5:17 ` Marcel Holtmann
0 siblings, 0 replies; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-16 5:17 UTC (permalink / raw)
To: K, Kiran; +Cc: linux-bluetooth
Hi Kiran,
>>> There is no standard HCI command to retrieve data path for transport.
>>> Add a new callback function to retrieve data path which is used in
>>> offload usecase. This needs to be set at setup stage if controller
>>> supports offload codecs
>>>
>>> Signed-off-by: Kiran K <kiran.k@intel.com>
>>> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
>>> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
>>> ---
>>> * changes in v9:
>>> - define a separate patch for core changes
>>>
>>> include/net/bluetooth/hci_core.h | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/include/net/bluetooth/hci_core.h
>>> b/include/net/bluetooth/hci_core.h
>>> index 3284044c3dd7..641477396da3 100644
>>> --- a/include/net/bluetooth/hci_core.h
>>> +++ b/include/net/bluetooth/hci_core.h
>>> @@ -617,6 +617,7 @@ struct hci_dev {
>>> int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
>>> void (*cmd_timeout)(struct hci_dev *hdev);
>>> bool (*prevent_wake)(struct hci_dev *hdev);
>>> + int (*get_data_path)(struct hci_dev *hdev);
>>> };
>>
>> and where is the code using hdev->get_data_path. That code needs to be in
>> this patch.
>
> In the previous patchset, there was a comment to separate out driver and core changes. Let me know if I am missing something here.
> https://patchwork.kernel.org/project/bluetooth/patch/20210518104232.5431-3-kiran.k@intel.com/
>
I know that and this is not contradictory. Introducing such a callback must come with the usage of said callback. Usage means the core side and not the driver side of it.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data path
2021-06-16 3:10 ` K, Kiran
@ 2021-06-16 5:18 ` Marcel Holtmann
2021-06-17 7:53 ` K, Kiran
0 siblings, 1 reply; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-16 5:18 UTC (permalink / raw)
To: K, Kiran; +Cc: linux-bluetooth
Hi Kiran,
>>> Adds callback function which is called to set the data path for HFP
>>> offload case before opening SCO connection
>>>
>>> Signed-off-by: Kiran K <kiran.k@intel.com>
>>> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
>>> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
>>> ---
>>> drivers/bluetooth/btintel.c | 50
>> +++++++++++++++++++++++++++++++++++++
>>> drivers/bluetooth/btintel.h | 8 ++++++
>>> drivers/bluetooth/btusb.c | 4 ++-
>>> include/net/bluetooth/hci.h | 8 ++++++
>>> 4 files changed, 69 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
>>> index 95c6a1bef35e..3eba5c587ef6 100644
>>> --- a/drivers/bluetooth/btintel.c
>>> +++ b/drivers/bluetooth/btintel.c
>>> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct
>>> hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
>>>
>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
>>> + struct bt_codec *codec)
>>> +{
>>> + __u8 preset;
>>> + struct hci_op_configure_data_path *cmd;
>>> + __u8 buffer[255];
>>> + struct sk_buff *skb;
>>> +
>>> + if (type != SCO_LINK && type != ESCO_LINK)
>>> + return -EINVAL;
>>> +
>>> + switch (codec->id) {
>>> + case 0x02:
>>> + preset = 0x00;
>>> + break;
>>> + case 0x05:
>>> + preset = 0x01;
>>> + break;
>>> + default:
>>> + return -EINVAL;
>>> + }
>>> +
>>> + cmd = (void *)buffer;
>>> + cmd->data_path_id = 0x01;
>>> + cmd->len = 1;
>>> + cmd->data[0] = preset;
>>> +
>>> + cmd->direction = 0x00;
>>> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
>> sizeof(*cmd) + 1,
>>> + cmd, HCI_INIT_TIMEOUT);
>>> + if (IS_ERR(skb)) {
>>> + bt_dev_err(hdev, "configure input data path failed (%ld)",
>>> + PTR_ERR(skb));
>>> + return PTR_ERR(skb);
>>> + }
>>> + kfree_skb(skb);
>>> +
>>> + cmd->direction = 0x01;
>>> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
>> sizeof(*cmd) + 1,
>>> + cmd, HCI_INIT_TIMEOUT);
>>> + if (IS_ERR(skb)) {
>>> + bt_dev_err(hdev, "configure output data path failed (%ld)",
>>> + PTR_ERR(skb));
>>> + return PTR_ERR(skb);
>>> + }
>>> + kfree_skb(skb);
>>> + return 0;
>>> +}
>>> +EXPORT_SYMBOL_GPL(btintel_set_data_path);
>>> +
>>> MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
>>> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver "
>>> VERSION); MODULE_VERSION(VERSION); diff --git
>>> a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index
>>> 9bcc489680db..9806970c9871 100644
>>> --- a/drivers/bluetooth/btintel.h
>>> +++ b/drivers/bluetooth/btintel.h
>>> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev
>>> *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,
>>> struct intel_offload_usecases *usecases); int
>>> btintel_get_data_path(struct hci_dev *hdev);
>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
>>> + struct bt_codec *codec);
>>> #else
>>>
>>> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@ -325,4
>>> +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {
>>> return -EOPNOTSUPP;
>>> }
>>> +
>>> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
>>> + struct bt_codec *codec)
>>> +{
>>> + return -EOPNOTSUPP;
>>> +}
>>> #endif
>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>> index 1e51beec5776..afafa44752a1 100644
>>> --- a/drivers/bluetooth/btusb.c
>>> +++ b/drivers/bluetooth/btusb.c
>>> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct
>> hci_dev *hdev)
>>> err = btintel_read_offload_usecases(hdev, &usecases);
>>> if (!err) {
>>> /* set get_data_path callback if offload is supported */
>>> - if (usecases.preset[0] & 0x03)
>>> + if (usecases.preset[0] & 0x03) {
>>> hdev->get_data_path = btintel_get_data_path;
>>> + hdev->set_data_path = btintel_set_data_path;
>>> + }
>>> }
>>
>>> /* Read the Intel version information after loading the FW */ diff
>>> --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>> index 31a5ac8918fc..42963188dcea 100644
>>> --- a/include/net/bluetooth/hci.h
>>> +++ b/include/net/bluetooth/hci.h
>>> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {
>>> __u8 rand256[16];
>>> } __packed;
>>>
>>> +#define HCI_CONFIGURE_DATA_PATH 0x0c83
>>> +struct hci_op_configure_data_path {
>>> + __u8 direction;
>>> + __u8 data_path_id;
>>> + __u8 len;
>>> + __u8 data[];
>>> +} __packed;
>>> +
>>
>> if this is a standard HCI command, why is this done as hdev->set_data_path?
>> This makes no sense too me.
> Intel uses HCI_CONFIGURE_DATA_PATH to command to set the preset ID (NBS, WBS, ...). Here len and data[] are vendor specific. I should have prefixed these fields with vnd_. I will address this in next patchset.
if the command is defined by the Bluetooth SIG, it is handle in the core. However if it needs vendor specific input that we need a callback for just that data.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data path
2021-06-16 5:18 ` Marcel Holtmann
@ 2021-06-17 7:53 ` K, Kiran
2021-06-17 10:19 ` Marcel Holtmann
0 siblings, 1 reply; 31+ messages in thread
From: K, Kiran @ 2021-06-17 7:53 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
> -----Original Message-----
> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Wednesday, June 16, 2021 10:48 AM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data
> path
>
> Hi Kiran,
>
> >>> Adds callback function which is called to set the data path for HFP
> >>> offload case before opening SCO connection
> >>>
> >>> Signed-off-by: Kiran K <kiran.k@intel.com>
> >>> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> >>> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> >>> ---
> >>> drivers/bluetooth/btintel.c | 50
> >> +++++++++++++++++++++++++++++++++++++
> >>> drivers/bluetooth/btintel.h | 8 ++++++
> >>> drivers/bluetooth/btusb.c | 4 ++-
> >>> include/net/bluetooth/hci.h | 8 ++++++
> >>> 4 files changed, 69 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/bluetooth/btintel.c
> >>> b/drivers/bluetooth/btintel.c index 95c6a1bef35e..3eba5c587ef6
> >>> 100644
> >>> --- a/drivers/bluetooth/btintel.c
> >>> +++ b/drivers/bluetooth/btintel.c
> >>> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct
> >>> hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
> >>>
> >>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> >>> + struct bt_codec *codec)
> >>> +{
> >>> + __u8 preset;
> >>> + struct hci_op_configure_data_path *cmd;
> >>> + __u8 buffer[255];
> >>> + struct sk_buff *skb;
> >>> +
> >>> + if (type != SCO_LINK && type != ESCO_LINK)
> >>> + return -EINVAL;
> >>> +
> >>> + switch (codec->id) {
> >>> + case 0x02:
> >>> + preset = 0x00;
> >>> + break;
> >>> + case 0x05:
> >>> + preset = 0x01;
> >>> + break;
> >>> + default:
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + cmd = (void *)buffer;
> >>> + cmd->data_path_id = 0x01;
> >>> + cmd->len = 1;
> >>> + cmd->data[0] = preset;
> >>> +
> >>> + cmd->direction = 0x00;
> >>> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
> >> sizeof(*cmd) + 1,
> >>> + cmd, HCI_INIT_TIMEOUT);
> >>> + if (IS_ERR(skb)) {
> >>> + bt_dev_err(hdev, "configure input data path failed (%ld)",
> >>> + PTR_ERR(skb));
> >>> + return PTR_ERR(skb);
> >>> + }
> >>> + kfree_skb(skb);
> >>> +
> >>> + cmd->direction = 0x01;
> >>> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
> >> sizeof(*cmd) + 1,
> >>> + cmd, HCI_INIT_TIMEOUT);
> >>> + if (IS_ERR(skb)) {
> >>> + bt_dev_err(hdev, "configure output data path failed (%ld)",
> >>> + PTR_ERR(skb));
> >>> + return PTR_ERR(skb);
> >>> + }
> >>> + kfree_skb(skb);
> >>> + return 0;
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(btintel_set_data_path);
> >>> +
> >>> MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
> >>> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver "
> >>> VERSION); MODULE_VERSION(VERSION); diff --git
> >>> a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index
> >>> 9bcc489680db..9806970c9871 100644
> >>> --- a/drivers/bluetooth/btintel.h
> >>> +++ b/drivers/bluetooth/btintel.h
> >>> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev
> >>> *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,
> >>> struct intel_offload_usecases *usecases); int
> >>> btintel_get_data_path(struct hci_dev *hdev);
> >>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> >>> + struct bt_codec *codec);
> >>> #else
> >>>
> >>> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@
> >>> -325,4
> >>> +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {
> >>> return -EOPNOTSUPP;
> >>> }
> >>> +
> >>> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> >>> + struct bt_codec *codec)
> >>> +{
> >>> + return -EOPNOTSUPP;
> >>> +}
> >>> #endif
> >>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >>> index 1e51beec5776..afafa44752a1 100644
> >>> --- a/drivers/bluetooth/btusb.c
> >>> +++ b/drivers/bluetooth/btusb.c
> >>> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct
> >> hci_dev *hdev)
> >>> err = btintel_read_offload_usecases(hdev, &usecases);
> >>> if (!err) {
> >>> /* set get_data_path callback if offload is supported */
> >>> - if (usecases.preset[0] & 0x03)
> >>> + if (usecases.preset[0] & 0x03) {
> >>> hdev->get_data_path = btintel_get_data_path;
> >>> + hdev->set_data_path = btintel_set_data_path;
> >>> + }
> >>> }
> >>
> >>> /* Read the Intel version information after loading the FW */ diff
> >>> --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> >>> index 31a5ac8918fc..42963188dcea 100644
> >>> --- a/include/net/bluetooth/hci.h
> >>> +++ b/include/net/bluetooth/hci.h
> >>> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {
> >>> __u8 rand256[16];
> >>> } __packed;
> >>>
> >>> +#define HCI_CONFIGURE_DATA_PATH 0x0c83
> >>> +struct hci_op_configure_data_path {
> >>> + __u8 direction;
> >>> + __u8 data_path_id;
> >>> + __u8 len;
> >>> + __u8 data[];
> >>> +} __packed;
> >>> +
> >>
> >> if this is a standard HCI command, why is this done as hdev-
> >set_data_path?
> >> This makes no sense too me.
> > Intel uses HCI_CONFIGURE_DATA_PATH to command to set the preset ID
> (NBS, WBS, ...). Here len and data[] are vendor specific. I should have
> prefixed these fields with vnd_. I will address this in next patchset.
>
> if the command is defined by the Bluetooth SIG, it is handle in the core.
> However if it needs vendor specific input that we need a callback for just that
> data.
The current design uses HCI_CONFIGURE_DATA_PATH inside set_data_path callback and its not used at core. I have leveraged SIG command here to minimize defining of new vendor command as vnd_data[] gives flexibility to pass in non-standard values. Other vendors may not have same command/flow to configure data path.
If we are not supposed to use Bluetooth SIG command at driver level, then I need to come up with a new vendor specific command. Please help with your input.
>
> Regards
>
> Marcel
Thanks,
Kiran
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data path
2021-06-17 7:53 ` K, Kiran
@ 2021-06-17 10:19 ` Marcel Holtmann
2021-06-23 3:21 ` K, Kiran
0 siblings, 1 reply; 31+ messages in thread
From: Marcel Holtmann @ 2021-06-17 10:19 UTC (permalink / raw)
To: K, Kiran; +Cc: linux-bluetooth
Hi Kiran,
>>>>> Adds callback function which is called to set the data path for HFP
>>>>> offload case before opening SCO connection
>>>>>
>>>>> Signed-off-by: Kiran K <kiran.k@intel.com>
>>>>> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
>>>>> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
>>>>> ---
>>>>> drivers/bluetooth/btintel.c | 50
>>>> +++++++++++++++++++++++++++++++++++++
>>>>> drivers/bluetooth/btintel.h | 8 ++++++
>>>>> drivers/bluetooth/btusb.c | 4 ++-
>>>>> include/net/bluetooth/hci.h | 8 ++++++
>>>>> 4 files changed, 69 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/bluetooth/btintel.c
>>>>> b/drivers/bluetooth/btintel.c index 95c6a1bef35e..3eba5c587ef6
>>>>> 100644
>>>>> --- a/drivers/bluetooth/btintel.c
>>>>> +++ b/drivers/bluetooth/btintel.c
>>>>> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct
>>>>> hci_dev *hdev, } EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
>>>>>
>>>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
>>>>> + struct bt_codec *codec)
>>>>> +{
>>>>> + __u8 preset;
>>>>> + struct hci_op_configure_data_path *cmd;
>>>>> + __u8 buffer[255];
>>>>> + struct sk_buff *skb;
>>>>> +
>>>>> + if (type != SCO_LINK && type != ESCO_LINK)
>>>>> + return -EINVAL;
>>>>> +
>>>>> + switch (codec->id) {
>>>>> + case 0x02:
>>>>> + preset = 0x00;
>>>>> + break;
>>>>> + case 0x05:
>>>>> + preset = 0x01;
>>>>> + break;
>>>>> + default:
>>>>> + return -EINVAL;
>>>>> + }
>>>>> +
>>>>> + cmd = (void *)buffer;
>>>>> + cmd->data_path_id = 0x01;
>>>>> + cmd->len = 1;
>>>>> + cmd->data[0] = preset;
>>>>> +
>>>>> + cmd->direction = 0x00;
>>>>> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
>>>> sizeof(*cmd) + 1,
>>>>> + cmd, HCI_INIT_TIMEOUT);
>>>>> + if (IS_ERR(skb)) {
>>>>> + bt_dev_err(hdev, "configure input data path failed (%ld)",
>>>>> + PTR_ERR(skb));
>>>>> + return PTR_ERR(skb);
>>>>> + }
>>>>> + kfree_skb(skb);
>>>>> +
>>>>> + cmd->direction = 0x01;
>>>>> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
>>>> sizeof(*cmd) + 1,
>>>>> + cmd, HCI_INIT_TIMEOUT);
>>>>> + if (IS_ERR(skb)) {
>>>>> + bt_dev_err(hdev, "configure output data path failed (%ld)",
>>>>> + PTR_ERR(skb));
>>>>> + return PTR_ERR(skb);
>>>>> + }
>>>>> + kfree_skb(skb);
>>>>> + return 0;
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(btintel_set_data_path);
>>>>> +
>>>>> MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
>>>>> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver "
>>>>> VERSION); MODULE_VERSION(VERSION); diff --git
>>>>> a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index
>>>>> 9bcc489680db..9806970c9871 100644
>>>>> --- a/drivers/bluetooth/btintel.h
>>>>> +++ b/drivers/bluetooth/btintel.h
>>>>> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev
>>>>> *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,
>>>>> struct intel_offload_usecases *usecases); int
>>>>> btintel_get_data_path(struct hci_dev *hdev);
>>>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
>>>>> + struct bt_codec *codec);
>>>>> #else
>>>>>
>>>>> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@
>>>>> -325,4
>>>>> +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev) {
>>>>> return -EOPNOTSUPP;
>>>>> }
>>>>> +
>>>>> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
>>>>> + struct bt_codec *codec)
>>>>> +{
>>>>> + return -EOPNOTSUPP;
>>>>> +}
>>>>> #endif
>>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
>>>>> index 1e51beec5776..afafa44752a1 100644
>>>>> --- a/drivers/bluetooth/btusb.c
>>>>> +++ b/drivers/bluetooth/btusb.c
>>>>> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct
>>>> hci_dev *hdev)
>>>>> err = btintel_read_offload_usecases(hdev, &usecases);
>>>>> if (!err) {
>>>>> /* set get_data_path callback if offload is supported */
>>>>> - if (usecases.preset[0] & 0x03)
>>>>> + if (usecases.preset[0] & 0x03) {
>>>>> hdev->get_data_path = btintel_get_data_path;
>>>>> + hdev->set_data_path = btintel_set_data_path;
>>>>> + }
>>>>> }
>>>>
>>>>> /* Read the Intel version information after loading the FW */ diff
>>>>> --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
>>>>> index 31a5ac8918fc..42963188dcea 100644
>>>>> --- a/include/net/bluetooth/hci.h
>>>>> +++ b/include/net/bluetooth/hci.h
>>>>> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {
>>>>> __u8 rand256[16];
>>>>> } __packed;
>>>>>
>>>>> +#define HCI_CONFIGURE_DATA_PATH 0x0c83
>>>>> +struct hci_op_configure_data_path {
>>>>> + __u8 direction;
>>>>> + __u8 data_path_id;
>>>>> + __u8 len;
>>>>> + __u8 data[];
>>>>> +} __packed;
>>>>> +
>>>>
>>>> if this is a standard HCI command, why is this done as hdev-
>>> set_data_path?
>>>> This makes no sense too me.
>>> Intel uses HCI_CONFIGURE_DATA_PATH to command to set the preset ID
>> (NBS, WBS, ...). Here len and data[] are vendor specific. I should have
>> prefixed these fields with vnd_. I will address this in next patchset.
>>
>> if the command is defined by the Bluetooth SIG, it is handle in the core.
>> However if it needs vendor specific input that we need a callback for just that
>> data.
>
> The current design uses HCI_CONFIGURE_DATA_PATH inside set_data_path callback and its not used at core. I have leveraged SIG command here to minimize defining of new vendor command as vnd_data[] gives flexibility to pass in non-standard values. Other vendors may not have same command/flow to configure data path.
>
> If we are not supposed to use Bluetooth SIG command at driver level, then I need to come up with a new vendor specific command. Please help with your input.
I don’t understand this argumentation. The Bluetooth standard defined HCI_Configure_Data_Path with Vendor_Specific_Config for exactly this reason. So just use it especially if our controllers already support it.
Now I am starting to wonder if this design is making things complicated for no reason. Isn’t it enough to have a hdev->get_data_path_config callback that allows to retrieve such data from the driver.
Frankly, the only thing you need from a driver is that it tells you the values of data_path_id and the vendor_config so that you can feed it back into the controller. Or am I missing anything here?
Let me be clear, if there is a SIG defined command, we implement support for in the core and not the driver. I do not want that other vendors have to define everything over and over again. That is what a standard is for. Only the vendor specific portions are handed off to the driver or userspace to provide.
Regards
Marcel
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data path
2021-06-17 10:19 ` Marcel Holtmann
@ 2021-06-23 3:21 ` K, Kiran
0 siblings, 0 replies; 31+ messages in thread
From: K, Kiran @ 2021-06-23 3:21 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
> -----Original Message-----
> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Thursday, June 17, 2021 3:50 PM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH v9 07/10] Bluetooth: btintel: define callback to set data
> path
>
> Hi Kiran,
>
> >>>>> Adds callback function which is called to set the data path for
> >>>>> HFP offload case before opening SCO connection
> >>>>>
> >>>>> Signed-off-by: Kiran K <kiran.k@intel.com>
> >>>>> Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> >>>>> Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> >>>>> ---
> >>>>> drivers/bluetooth/btintel.c | 50
> >>>> +++++++++++++++++++++++++++++++++++++
> >>>>> drivers/bluetooth/btintel.h | 8 ++++++
> >>>>> drivers/bluetooth/btusb.c | 4 ++-
> >>>>> include/net/bluetooth/hci.h | 8 ++++++
> >>>>> 4 files changed, 69 insertions(+), 1 deletion(-)
> >>>>>
> >>>>> diff --git a/drivers/bluetooth/btintel.c
> >>>>> b/drivers/bluetooth/btintel.c index 95c6a1bef35e..3eba5c587ef6
> >>>>> 100644
> >>>>> --- a/drivers/bluetooth/btintel.c
> >>>>> +++ b/drivers/bluetooth/btintel.c
> >>>>> @@ -1308,6 +1308,56 @@ int btintel_read_offload_usecases(struct
> >>>>> hci_dev *hdev, }
> EXPORT_SYMBOL_GPL(btintel_read_offload_usecases);
> >>>>>
> >>>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> >>>>> + struct bt_codec *codec)
> >>>>> +{
> >>>>> + __u8 preset;
> >>>>> + struct hci_op_configure_data_path *cmd;
> >>>>> + __u8 buffer[255];
> >>>>> + struct sk_buff *skb;
> >>>>> +
> >>>>> + if (type != SCO_LINK && type != ESCO_LINK)
> >>>>> + return -EINVAL;
> >>>>> +
> >>>>> + switch (codec->id) {
> >>>>> + case 0x02:
> >>>>> + preset = 0x00;
> >>>>> + break;
> >>>>> + case 0x05:
> >>>>> + preset = 0x01;
> >>>>> + break;
> >>>>> + default:
> >>>>> + return -EINVAL;
> >>>>> + }
> >>>>> +
> >>>>> + cmd = (void *)buffer;
> >>>>> + cmd->data_path_id = 0x01;
> >>>>> + cmd->len = 1;
> >>>>> + cmd->data[0] = preset;
> >>>>> +
> >>>>> + cmd->direction = 0x00;
> >>>>> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
> >>>> sizeof(*cmd) + 1,
> >>>>> + cmd, HCI_INIT_TIMEOUT);
> >>>>> + if (IS_ERR(skb)) {
> >>>>> + bt_dev_err(hdev, "configure input data path failed
> (%ld)",
> >>>>> + PTR_ERR(skb));
> >>>>> + return PTR_ERR(skb);
> >>>>> + }
> >>>>> + kfree_skb(skb);
> >>>>> +
> >>>>> + cmd->direction = 0x01;
> >>>>> + skb = __hci_cmd_sync(hdev, HCI_CONFIGURE_DATA_PATH,
> >>>> sizeof(*cmd) + 1,
> >>>>> + cmd, HCI_INIT_TIMEOUT);
> >>>>> + if (IS_ERR(skb)) {
> >>>>> + bt_dev_err(hdev, "configure output data path failed
> (%ld)",
> >>>>> + PTR_ERR(skb));
> >>>>> + return PTR_ERR(skb);
> >>>>> + }
> >>>>> + kfree_skb(skb);
> >>>>> + return 0;
> >>>>> +}
> >>>>> +EXPORT_SYMBOL_GPL(btintel_set_data_path);
> >>>>> +
> >>>>> MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
> >>>>> MODULE_DESCRIPTION("Bluetooth support for Intel devices ver "
> >>>>> VERSION); MODULE_VERSION(VERSION); diff --git
> >>>>> a/drivers/bluetooth/btintel.h b/drivers/bluetooth/btintel.h index
> >>>>> 9bcc489680db..9806970c9871 100644
> >>>>> --- a/drivers/bluetooth/btintel.h
> >>>>> +++ b/drivers/bluetooth/btintel.h
> >>>>> @@ -183,6 +183,8 @@ int btintel_set_debug_features(struct hci_dev
> >>>>> *hdev, int btintel_read_offload_usecases(struct hci_dev *hdev,
> >>>>> struct intel_offload_usecases *usecases); int
> >>>>> btintel_get_data_path(struct hci_dev *hdev);
> >>>>> +int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> >>>>> + struct bt_codec *codec);
> >>>>> #else
> >>>>>
> >>>>> static inline int btintel_check_bdaddr(struct hci_dev *hdev) @@
> >>>>> -325,4
> >>>>> +327,10 @@ static int btintel_get_data_path(struct hci_dev *hdev)
> >>>>> +{
> >>>>> return -EOPNOTSUPP;
> >>>>> }
> >>>>> +
> >>>>> +static int btintel_set_data_path(struct hci_dev *hdev, __u8 type,
> >>>>> + struct bt_codec *codec)
> >>>>> +{
> >>>>> + return -EOPNOTSUPP;
> >>>>> +}
> >>>>> #endif
> >>>>> diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> >>>>> index 1e51beec5776..afafa44752a1 100644
> >>>>> --- a/drivers/bluetooth/btusb.c
> >>>>> +++ b/drivers/bluetooth/btusb.c
> >>>>> @@ -3012,8 +3012,10 @@ static int btusb_setup_intel_newgen(struct
> >>>> hci_dev *hdev)
> >>>>> err = btintel_read_offload_usecases(hdev, &usecases);
> >>>>> if (!err) {
> >>>>> /* set get_data_path callback if offload is supported */
> >>>>> - if (usecases.preset[0] & 0x03)
> >>>>> + if (usecases.preset[0] & 0x03) {
> >>>>> hdev->get_data_path = btintel_get_data_path;
> >>>>> + hdev->set_data_path =
> btintel_set_data_path;
> >>>>> + }
> >>>>> }
> >>>>
> >>>>> /* Read the Intel version information after loading the FW */
> >>>>> diff --git a/include/net/bluetooth/hci.h
> >>>>> b/include/net/bluetooth/hci.h index 31a5ac8918fc..42963188dcea
> >>>>> 100644
> >>>>> --- a/include/net/bluetooth/hci.h
> >>>>> +++ b/include/net/bluetooth/hci.h
> >>>>> @@ -1250,6 +1250,14 @@ struct hci_rp_read_local_oob_ext_data {
> >>>>> __u8 rand256[16];
> >>>>> } __packed;
> >>>>>
> >>>>> +#define HCI_CONFIGURE_DATA_PATH 0x0c83
> >>>>> +struct hci_op_configure_data_path {
> >>>>> + __u8 direction;
> >>>>> + __u8 data_path_id;
> >>>>> + __u8 len;
> >>>>> + __u8 data[];
> >>>>> +} __packed;
> >>>>> +
> >>>>
> >>>> if this is a standard HCI command, why is this done as hdev-
> >>> set_data_path?
> >>>> This makes no sense too me.
> >>> Intel uses HCI_CONFIGURE_DATA_PATH to command to set the preset
> ID
> >> (NBS, WBS, ...). Here len and data[] are vendor specific. I should
> >> have prefixed these fields with vnd_. I will address this in next patchset.
> >>
> >> if the command is defined by the Bluetooth SIG, it is handle in the core.
> >> However if it needs vendor specific input that we need a callback for
> >> just that data.
> >
> > The current design uses HCI_CONFIGURE_DATA_PATH inside
> set_data_path callback and its not used at core. I have leveraged SIG
> command here to minimize defining of new vendor command as vnd_data[]
> gives flexibility to pass in non-standard values. Other vendors may not have
> same command/flow to configure data path.
> >
> > If we are not supposed to use Bluetooth SIG command at driver level, then
> I need to come up with a new vendor specific command. Please help with
> your input.
>
> I don’t understand this argumentation. The Bluetooth standard defined
> HCI_Configure_Data_Path with Vendor_Specific_Config for exactly this
> reason. So just use it especially if our controllers already support it.
>
> Now I am starting to wonder if this design is making things complicated for
> no reason. Isn’t it enough to have a hdev->get_data_path_config callback
> that allows to retrieve such data from the driver.
>
> Frankly, the only thing you need from a driver is that it tells you the values of
> data_path_id and the vendor_config so that you can feed it back into the
> controller. Or am I missing anything here?
>
> Let me be clear, if there is a SIG defined command, we implement support
> for in the core and not the driver. I do not want that other vendors have to
> define everything over and over again. That is what a standard is for. Only
> the vendor specific portions are handed off to the driver or userspace to
> provide.
Agree. I will make the suggested changes in the next patchset.
>
> Regards
>
> Marcel
Thanks,
Kiran
^ permalink raw reply [flat|nested] 31+ messages in thread
* RE: [PATCH v9 10/10] Bluetooth: Add support for msbc coding format
2021-06-15 19:43 ` Marcel Holtmann
@ 2021-06-23 3:24 ` K, Kiran
0 siblings, 0 replies; 31+ messages in thread
From: K, Kiran @ 2021-06-23 3:24 UTC (permalink / raw)
To: Marcel Holtmann; +Cc: linux-bluetooth
Hi Marcel,
> -----Original Message-----
> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Wednesday, June 16, 2021 1:14 AM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org
> Subject: Re: [PATCH v9 10/10] Bluetooth: Add support for msbc coding
> format
>
> Hi Kiran,
>
> > In Enhanced_Setup_Synchronous_Command, add support for msbc coding
> > format
> >
> > Signed-off-by: Kiran K <kiran.k@intel.com>
> > Reviewed-by: Chethan T N <chethan.tumkur.narayan@intel.com>
> > Reviewed-by: Srivatsa Ravishankar <ravishankar.srivatsa@intel.com>
> > ---
> > include/net/bluetooth/bluetooth.h | 1 +
> > net/bluetooth/hci_conn.c | 27 ++++++++++++++++++++++++++-
> > 2 files changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/net/bluetooth/bluetooth.h
> > b/include/net/bluetooth/bluetooth.h
> > index af2809121571..056699224da7 100644
> > --- a/include/net/bluetooth/bluetooth.h
> > +++ b/include/net/bluetooth/bluetooth.h
> > @@ -175,6 +175,7 @@ struct bt_codecs {
> >
> > #define CODING_FORMAT_CVSD 0x02
> > #define CODING_FORMAT_TRANSPARENT 0x03
> > +#define CODING_FORMAT_MSBC 0x05
> >
> > __printf(1, 2)
> > void bt_info(const char *fmt, ...);
> > diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c index
> > 9569b21adb88..73c134459361 100644
> > --- a/net/bluetooth/hci_conn.c
> > +++ b/net/bluetooth/hci_conn.c
> > @@ -328,6 +328,32 @@ bool hci_enhanced_setup_sync(struct hci_conn
> *conn, __u16 handle)
> > cp.rx_bandwidth = cpu_to_le32(0x00001f40);
> >
> > switch (conn->codec.id) {
> > + case CODING_FORMAT_MSBC:
> > + if (!find_next_esco_param(conn, esco_param_msbc,
> > + ARRAY_SIZE(esco_param_msbc)))
> > + return false;
> > +
> > + param = &esco_param_msbc[conn->attempt - 1];
> > + cp.tx_coding_format.id = 0x05;
> > + cp.rx_coding_format.id = 0x05;
> > + cp.tx_codec_frame_size = __cpu_to_le16(60);
> > + cp.rx_codec_frame_size = __cpu_to_le16(60);
> > + cp.in_bandwidth = __cpu_to_le32(32000);
> > + cp.out_bandwidth = __cpu_to_le32(32000);
> > + cp.in_coding_format.id = 0x04;
> > + cp.out_coding_format.id = 0x04;
> > + cp.in_coded_data_size = __cpu_to_le16(16);
> > + cp.out_coded_data_size = __cpu_to_le16(16);
> > + cp.in_pcm_data_format = 2;
> > + cp.out_pcm_data_format = 2;
> > + cp.in_pcm_sample_payload_msb_pos = 0;
> > + cp.out_pcm_sample_payload_msb_pos = 0;
> > + cp.in_data_path = conn->codec.data_path;
> > + cp.out_data_path = conn->codec.data_path;
> > + cp.in_trasnport_unit_size = 1;
> > + cp.out_trasnport_unit_size = 1;
>
> so s/trasnport/transport/
>
> Please spellcheck your structs.
Ack.
>
> > + break;
> > +
> > case CODING_FORMAT_TRANSPARENT:
> > if (!find_next_esco_param(conn, esco_param_msbc,
> > ARRAY_SIZE(esco_param_msbc)))
> > @@ -383,7 +409,6 @@ bool hci_enhanced_setup_sync(struct hci_conn
> *conn, __u16 handle)
> > cp.in_trasnport_unit_size = 16;
> > cp.out_trasnport_unit_size = 16;
> > break;
> > -
>
> We can not have these random hunks in patches. You need to review your
> final set before sending it out.
Ack.
>
> Regards
>
> Marcel
Thanks,
Kiran
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2021-06-23 3:24 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 12:24 [PATCH v9 01/10] Bluetooth: enumerate local supported codec and cache details Kiran K
2021-06-08 12:24 ` [PATCH v9 02/10] Bluetooth: Add support for Read Local Supported Codecs V2 Kiran K
2021-06-08 12:24 ` [PATCH v9 03/10] Bluetooth: Add a callback function to retireve data path Kiran K
2021-06-15 19:26 ` Marcel Holtmann
2021-06-16 2:56 ` K, Kiran
2021-06-16 5:17 ` Marcel Holtmann
2021-06-08 12:24 ` [PATCH v9 04/10] Bluetooth: btintel: set get_data_path callback Kiran K
2021-06-15 19:30 ` Marcel Holtmann
2021-06-16 3:02 ` K, Kiran
2021-06-08 12:24 ` [PATCH v9 05/10] Bluetooth: Add BT_CODEC option for getsockopt for SCO socket Kiran K
2021-06-15 19:37 ` Marcel Holtmann
2021-06-08 12:24 ` [PATCH v9 06/10] Bluetooth: Add a callback function to set data path Kiran K
2021-06-15 19:37 ` Marcel Holtmann
2021-06-08 12:24 ` [PATCH v9 07/10] Bluetooth: btintel: define callback " Kiran K
2021-06-15 19:39 ` Marcel Holtmann
2021-06-16 3:10 ` K, Kiran
2021-06-16 5:18 ` Marcel Holtmann
2021-06-17 7:53 ` K, Kiran
2021-06-17 10:19 ` Marcel Holtmann
2021-06-23 3:21 ` K, Kiran
2021-06-08 12:24 ` [PATCH v9 08/10] Bluetooth: Add BT_CODEC option for setsockopt over SCO Kiran K
2021-06-08 12:24 ` [PATCH v9 09/10] Bluetooth: Add support for HCI_Enhanced_Setup_Synchronous_Connection command Kiran K
2021-06-08 12:24 ` [PATCH v9 10/10] Bluetooth: Add support for msbc coding format Kiran K
2021-06-15 19:43 ` Marcel Holtmann
2021-06-23 3:24 ` K, Kiran
2021-06-08 13:39 ` [v9,01/10] Bluetooth: enumerate local supported codec and cache details bluez.test.bot
2021-06-08 18:49 ` [PATCH v9 01/10] " kernel test robot
2021-06-08 18:49 ` kernel test robot
2021-06-15 19:25 ` Marcel Holtmann
2021-06-16 2:51 ` K, Kiran
2021-06-16 5:15 ` Marcel Holtmann
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.