linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/3] Bluetooth: add support to enumerate codec capabilities
@ 2021-04-22 14:14 Kiran K
  2021-04-22 14:14 ` [PATCH v3 2/3] Bluetooth: add support to enumerate local supports codecs v2 Kiran K
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Kiran K @ 2021-04-22 14:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Kiran K, Chethan T N, Srivatsa Ravishankar

add support to enumerate local supported codec capabilities

< 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 v3
  move codec enumeration into a new init function
* changes in v2
  add skb length check before accessing data

 include/net/bluetooth/hci.h |  7 ++++
 net/bluetooth/hci_core.c    | 16 ++++++---
 net/bluetooth/hci_event.c   | 68 +++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index ea4ae551c426..e3f7771fe84f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1314,6 +1314,13 @@ 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	codec_id[5];
+	__u8	transport;
+	__u8	direction;
+} __packed;
+
 #define HCI_OP_READ_PAGE_SCAN_ACTIVITY	0x0c1b
 struct hci_rp_read_page_scan_activity {
 	__u8     status;
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fd12f1652bdf..9419bbf55d90 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,15 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
 	return 0;
 }
 
+static int hci_init5_req(struct hci_request *req, unsigned long opt)
+{
+	struct hci_dev *hdev = req->hdev;
+
+	/* 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);
+	return 0;
+}
 static int __hci_init(struct hci_dev *hdev)
 {
 	int err;
@@ -937,6 +942,9 @@ static int __hci_init(struct hci_dev *hdev)
 	if (err < 0)
 		return err;
 
+	err = __hci_req_sync(hdev, hci_init5_req, 0, HCI_INIT_TIMEOUT, NULL);
+	if (err < 0)
+		return err;
 	/* 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.
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 5e99968939ce..a4b905a76c1b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -887,6 +887,70 @@ static void hci_cc_read_data_block_size(struct hci_dev *hdev,
 	       hdev->block_cnt, hdev->block_len);
 }
 
+static void hci_cc_read_local_codecs(struct hci_dev *hdev,
+				     struct sk_buff *skb)
+{
+	__u8 num_codecs;
+	struct hci_op_read_local_codec_caps caps;
+
+	if (skb->len < sizeof(caps))
+		return;
+
+	bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
+
+	if (skb->data[0])
+		return;
+
+	/* enumerate standard codecs */
+	skb_pull(skb, 1);
+
+	if (skb->len < 1)
+		return;
+
+	num_codecs = skb->data[0];
+
+	bt_dev_dbg(hdev, "Number of standard codecs: %u", num_codecs);
+
+	skb_pull(skb, 1);
+
+	if (skb->len < num_codecs)
+		return;
+
+	while (num_codecs--) {
+		caps.codec_id[0] = skb->data[0];
+		caps.transport = 0x00;
+		caps.direction = 0x00;
+
+		hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+			     &caps);
+
+		skb_pull(skb, 1);
+	}
+
+	/* enumerate vendor specific codecs */
+	if (skb->len < 1)
+		return;
+
+	num_codecs = skb->data[0];
+	skb_pull(skb, 1);
+
+	bt_dev_dbg(hdev, "Number of vendor specific codecs: %u", num_codecs);
+
+	if (skb->len < (num_codecs * 4))
+		return;
+
+	while (num_codecs--) {
+		caps.codec_id[0] = 0xFF;
+		memcpy(&caps.codec_id[1], skb->data, 4);
+		caps.transport = 0x00;
+		caps.direction = 0x00;
+
+		hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+			     &caps);
+		skb_pull(skb, 4);
+	}
+}
+
 static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_rp_read_clock *rp = (void *) skb->data;
@@ -3437,6 +3501,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
 		hci_cc_read_data_block_size(hdev, skb);
 		break;
 
+	case HCI_OP_READ_LOCAL_CODECS:
+		hci_cc_read_local_codecs(hdev, skb);
+		break;
+
 	case HCI_OP_READ_FLOW_CONTROL_MODE:
 		hci_cc_read_flow_control_mode(hdev, skb);
 		break;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 2/3] Bluetooth: add support to enumerate local supports codecs v2
  2021-04-22 14:14 [PATCH v3 1/3] Bluetooth: add support to enumerate codec capabilities Kiran K
@ 2021-04-22 14:14 ` Kiran K
  2021-04-23  8:00   ` Marcel Holtmann
  2021-04-22 14:14 ` [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities Kiran K
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Kiran K @ 2021-04-22 14:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Kiran K, Chethan T N, Srivatsa Ravishankar

Add support for HCI_Read_Local_Supported_Codecs_V2 and query codec
capabilities

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 v3:
  No changes

* changes in v2:
  add length check for event data before accessing

 include/net/bluetooth/hci.h      |   2 +
 include/net/bluetooth/hci_core.h |  10 +++
 net/bluetooth/hci_core.c         |   4 +-
 net/bluetooth/hci_event.c        | 110 +++++++++++++++++++++++++++++++
 4 files changed, 125 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index e3f7771fe84f..34eb9f4b027f 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1314,6 +1314,8 @@ struct hci_rp_read_local_pairing_opts {
 	__u8     max_key_size;
 } __packed;
 
+#define HCI_OP_READ_LOCAL_CODECS_V2    0x100d
+
 #define HCI_OP_READ_LOCAL_CODEC_CAPS	0x100e
 struct hci_op_read_local_codec_caps {
 	__u8	codec_id[5];
diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 8f5f390363f5..2c19b02a805d 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1848,4 +1848,14 @@ 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 LOCAL_CODEC_CIS_MASK	BIT(2)
+#define LOCAL_CODEC_BIS_MASK	BIT(3)
+
+#define LOCAL_CODEC_ACL		0x00
+#define LOCAL_CODEC_SCO		0x01
+#define LOCAL_CODEC_CIS		0x02
+#define LOCAL_CODEC_BIS		0x03
+
 #endif /* __HCI_CORE_H */
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9419bbf55d90..fda7077d0d47 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -908,7 +908,9 @@ static int hci_init5_req(struct hci_request *req, unsigned long opt)
 	struct hci_dev *hdev = req->hdev;
 
 	/* Read local codec list if the HCI command is supported */
-	if (hdev->commands[29] & 0x20)
+	if (hdev->commands[45] & 0x04)
+		hci_req_add(req, HCI_OP_READ_LOCAL_CODECS_V2, 0, NULL);
+	else if (hdev->commands[29] & 0x20)
 		hci_req_add(req, HCI_OP_READ_LOCAL_CODECS, 0, NULL);
 	return 0;
 }
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index a4b905a76c1b..7ca3535f30de 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -951,6 +951,112 @@ static void hci_cc_read_local_codecs(struct hci_dev *hdev,
 	}
 }
 
+static void hci_cc_read_local_codecs_v2(struct hci_dev *hdev,
+					struct sk_buff *skb)
+{
+	__u8 num_codecs, transport;
+	struct hci_op_read_local_codec_caps caps;
+
+	if (skb->len < sizeof(caps))
+		return;
+
+	bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
+
+	if (skb->data[0])
+		return;
+
+	/* enumerate standard codecs */
+	skb_pull(skb, 1);
+
+	if (skb->len < 1)
+		return;
+
+	num_codecs = skb->data[0];
+
+	bt_dev_info(hdev, "Number of standard codecs: %u", num_codecs);
+
+	skb_pull(skb, 1);
+
+	if (skb->len < (num_codecs * 2))
+		return;
+
+	while (num_codecs--) {
+		caps.codec_id[0] = skb->data[0];
+		transport = skb->data[1];
+		caps.direction = 0x00;
+
+		if (transport & LOCAL_CODEC_ACL_MASK) {
+			caps.transport = LOCAL_CODEC_ACL;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_SCO_MASK) {
+			caps.transport = LOCAL_CODEC_SCO;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_BIS_MASK) {
+			caps.transport = LOCAL_CODEC_BIS;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_CIS_MASK) {
+			caps.transport = LOCAL_CODEC_CIS;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		skb_pull(skb, 2);
+	}
+
+	/* enumerate vendor specific codecs */
+	if (skb->len < 1)
+		return;
+
+	num_codecs = skb->data[0];
+
+	skb_pull(skb, 1);
+
+	if (skb->len < (num_codecs * 5))
+		return;
+
+	bt_dev_info(hdev, "Number of vendor specific codecs: %u", num_codecs);
+
+	while (num_codecs--) {
+		caps.codec_id[0] = 0xFF;
+		memcpy(&caps.codec_id[1], skb->data, 4);
+		transport = skb->data[4];
+		caps.direction = 0x00;
+
+		if (transport & LOCAL_CODEC_ACL_MASK) {
+			caps.transport = LOCAL_CODEC_ACL;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_SCO_MASK) {
+			caps.transport = LOCAL_CODEC_SCO;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_BIS) {
+			caps.transport = LOCAL_CODEC_BIS;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+
+		if (transport & LOCAL_CODEC_CIS_MASK) {
+			caps.transport = LOCAL_CODEC_CIS;
+			hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
+				     &caps);
+		}
+	}
+}
+
 static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_rp_read_clock *rp = (void *) skb->data;
@@ -3505,6 +3611,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
 		hci_cc_read_local_codecs(hdev, skb);
 		break;
 
+	case HCI_OP_READ_LOCAL_CODECS_V2:
+		hci_cc_read_local_codecs_v2(hdev, skb);
+		break;
+
 	case HCI_OP_READ_FLOW_CONTROL_MODE:
 		hci_cc_read_flow_control_mode(hdev, skb);
 		break;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities
  2021-04-22 14:14 [PATCH v3 1/3] Bluetooth: add support to enumerate codec capabilities Kiran K
  2021-04-22 14:14 ` [PATCH v3 2/3] Bluetooth: add support to enumerate local supports codecs v2 Kiran K
@ 2021-04-22 14:14 ` Kiran K
  2021-04-23  8:04   ` Marcel Holtmann
  2021-04-22 15:08 ` [v3,1/3] Bluetooth: add support to enumerate " bluez.test.bot
  2021-04-23  7:58 ` [PATCH v3 1/3] " Marcel Holtmann
  3 siblings, 1 reply; 10+ messages in thread
From: Kiran K @ 2021-04-22 14:14 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Kiran K, Chethan T N, Srivatsa Ravishankar

Cache the codec information in the driver and this data can
be exposed to user space audio modules via getsockopt

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 v3
  remove unwanted log

* changes in v2
  add skb length check before accessing data

 include/net/bluetooth/hci.h      | 11 +++++++++++
 include/net/bluetooth/hci_core.h | 13 +++++++++++++
 net/bluetooth/hci_core.c         | 31 +++++++++++++++++++++++++++++++
 net/bluetooth/hci_event.c        | 32 ++++++++++++++++++++++++++++++++
 4 files changed, 87 insertions(+)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index 34eb9f4b027f..6b4344639ff7 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -1323,6 +1323,17 @@ struct hci_op_read_local_codec_caps {
 	__u8	direction;
 } __packed;
 
+struct hci_codec_caps {
+	__u8	len;
+	__u8	caps[];
+} __packed;
+
+struct hci_rp_read_local_codec_caps {
+	__u8	status;
+	__u8	num_caps;
+	struct hci_codec_caps 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 2c19b02a805d..b40c7ed38d18 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -131,6 +131,14 @@ struct bdaddr_list {
 	u8 bdaddr_type;
 };
 
+struct codec_list {
+	struct list_head list;
+	u8	transport;
+	u8	codec_id[5];
+	u8	num_caps;
+	struct hci_codec_caps caps[];
+};
+
 struct bdaddr_list_with_irk {
 	struct list_head list;
 	bdaddr_t bdaddr;
@@ -534,6 +542,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;
 
@@ -1843,6 +1852,10 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,
 
 void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
 			       u8 *bdaddr_type);
+int hci_codec_list_add(struct list_head *list, struct hci_rp_read_local_codec_caps *rp,
+		       __u32 len,
+		       struct hci_op_read_local_codec_caps *sent);
+void hci_codec_list_clear(struct list_head *codec_list);
 
 #define SCO_AIRMODE_MASK       0x0003
 #define SCO_AIRMODE_CVSD       0x0000
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index fda7077d0d47..17dc342f4486 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -3569,6 +3569,35 @@ void hci_conn_params_clear_disabled(struct hci_dev *hdev)
 	BT_DBG("All LE disabled connection parameters were removed");
 }
 
+int hci_codec_list_add(struct list_head *list, struct hci_rp_read_local_codec_caps *rp,
+		       __u32 len,
+		       struct hci_op_read_local_codec_caps *sent)
+{
+	struct codec_list *entry;
+
+	entry = kzalloc(sizeof(*entry) + len, GFP_KERNEL);
+	if (!entry)
+		return -ENOMEM;
+
+	memcpy(entry->codec_id, sent->codec_id, 5);
+	entry->transport = sent->transport;
+	entry->num_caps = rp->num_caps;
+	if (rp->num_caps)
+		memcpy(entry->caps, rp->caps, len);
+	list_add(&entry->list, list);
+
+	return 0;
+}
+
+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);
+	}
+}
 /* This function requires the caller holds hdev->lock */
 static void hci_conn_params_clear_all(struct hci_dev *hdev)
 {
@@ -3828,6 +3857,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);
@@ -4046,6 +4076,7 @@ void hci_unregister_dev(struct hci_dev *hdev)
 	hci_conn_params_clear_all(hdev);
 	hci_discovery_filter_clear(hdev);
 	hci_blocked_keys_clear(hdev);
+	hci_codec_list_clear(&hdev->local_codecs);
 	hci_dev_unlock(hdev);
 
 	hci_dev_put(hdev);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 7ca3535f30de..b55cd02abd02 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -1057,6 +1057,34 @@ static void hci_cc_read_local_codecs_v2(struct hci_dev *hdev,
 	}
 }
 
+static void hci_cc_read_local_codec_caps(struct hci_dev *hdev,
+					 struct sk_buff *skb)
+{
+	struct hci_op_read_local_codec_caps *sent;
+	struct hci_rp_read_local_codec_caps *rp;
+
+	if (skb->len < sizeof(*rp))
+		return;
+
+	rp = (void *)skb->data;
+
+	bt_dev_dbg(hdev, "status 0x%2.2x", rp->status);
+
+	if (rp->status)
+		return;
+
+	sent = hci_sent_cmd_data(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS);
+
+	if (!sent)
+		return;
+
+	hci_dev_lock(hdev);
+
+	hci_codec_list_add(&hdev->local_codecs, rp, skb->len - 2, sent);
+
+	hci_dev_unlock(hdev);
+}
+
 static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_rp_read_clock *rp = (void *) skb->data;
@@ -3615,6 +3643,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
 		hci_cc_read_local_codecs_v2(hdev, skb);
 		break;
 
+	case HCI_OP_READ_LOCAL_CODEC_CAPS:
+		hci_cc_read_local_codec_caps(hdev, skb);
+		break;
+
 	case HCI_OP_READ_FLOW_CONTROL_MODE:
 		hci_cc_read_flow_control_mode(hdev, skb);
 		break;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* RE: [v3,1/3] Bluetooth: add support to enumerate codec capabilities
  2021-04-22 14:14 [PATCH v3 1/3] Bluetooth: add support to enumerate codec capabilities Kiran K
  2021-04-22 14:14 ` [PATCH v3 2/3] Bluetooth: add support to enumerate local supports codecs v2 Kiran K
  2021-04-22 14:14 ` [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities Kiran K
@ 2021-04-22 15:08 ` bluez.test.bot
  2021-04-23  7:58 ` [PATCH v3 1/3] " Marcel Holtmann
  3 siblings, 0 replies; 10+ messages in thread
From: bluez.test.bot @ 2021-04-22 15:08 UTC (permalink / raw)
  To: linux-bluetooth, kiran.k

[-- Attachment #1: Type: text/plain, Size: 1467 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=471665

---Test result---

##############################
Test: CheckPatch - PASS


##############################
Test: CheckGitLint - PASS


##############################
Test: CheckBuildK - PASS


##############################
Test: CheckTestRunner: Setup - PASS


##############################
Test: CheckTestRunner: l2cap-tester - PASS
Total: 40, Passed: 34 (85.0%), Failed: 0, Not Run: 6

##############################
Test: CheckTestRunner: bnep-tester - PASS
Total: 1, Passed: 1 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: mgmt-tester - PASS
Total: 416, Passed: 402 (96.6%), Failed: 0, Not Run: 14

##############################
Test: CheckTestRunner: rfcomm-tester - PASS
Total: 9, Passed: 9 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: sco-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: smp-tester - PASS
Total: 8, Passed: 8 (100.0%), Failed: 0, Not Run: 0

##############################
Test: CheckTestRunner: userchan-tester - PASS
Total: 3, Passed: 3 (100.0%), Failed: 0, Not Run: 0



---
Regards,
Linux Bluetooth


[-- Attachment #2: l2cap-tester.log --]
[-- Type: application/octet-stream, Size: 43346 bytes --]

[-- Attachment #3: bnep-tester.log --]
[-- Type: application/octet-stream, Size: 3537 bytes --]

[-- Attachment #4: mgmt-tester.log --]
[-- Type: application/octet-stream, Size: 546864 bytes --]

[-- Attachment #5: rfcomm-tester.log --]
[-- Type: application/octet-stream, Size: 11657 bytes --]

[-- Attachment #6: sco-tester.log --]
[-- Type: application/octet-stream, Size: 9892 bytes --]

[-- Attachment #7: smp-tester.log --]
[-- Type: application/octet-stream, Size: 11803 bytes --]

[-- Attachment #8: userchan-tester.log --]
[-- Type: application/octet-stream, Size: 5433 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 1/3] Bluetooth: add support to enumerate codec capabilities
  2021-04-22 14:14 [PATCH v3 1/3] Bluetooth: add support to enumerate codec capabilities Kiran K
                   ` (2 preceding siblings ...)
  2021-04-22 15:08 ` [v3,1/3] Bluetooth: add support to enumerate " bluez.test.bot
@ 2021-04-23  7:58 ` Marcel Holtmann
  3 siblings, 0 replies; 10+ messages in thread
From: Marcel Holtmann @ 2021-04-23  7:58 UTC (permalink / raw)
  To: Kiran K; +Cc: Bluetooth Kernel Mailing List, Chethan T N, Srivatsa Ravishankar

Hi Kiran,

> add support to enumerate local supported codec capabilities
> 
> < 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 v3
>  move codec enumeration into a new init function
> * changes in v2
>  add skb length check before accessing data
> 
> include/net/bluetooth/hci.h |  7 ++++
> net/bluetooth/hci_core.c    | 16 ++++++---
> net/bluetooth/hci_event.c   | 68 +++++++++++++++++++++++++++++++++++++
> 3 files changed, 87 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index ea4ae551c426..e3f7771fe84f 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1314,6 +1314,13 @@ 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	codec_id[5];
> +	__u8	transport;
> +	__u8	direction;
> +} __packed;
> +
> #define HCI_OP_READ_PAGE_SCAN_ACTIVITY	0x0c1b
> struct hci_rp_read_page_scan_activity {
> 	__u8     status;
> diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
> index fd12f1652bdf..9419bbf55d90 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);
> -

that is not what I actually meant. The initial command to read the codec list can clearly stay in this init phase. Just the subsequent calls to read further details require an extra phase.

> 	/* 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,15 @@ static int hci_init4_req(struct hci_request *req, unsigned long opt)
> 	return 0;
> }
> 
> +static int hci_init5_req(struct hci_request *req, unsigned long opt)
> +{
> +	struct hci_dev *hdev = req->hdev;
> +
> +	/* 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);

So here instead you go through your list of codec ids and read its capabilities.

> +	return 0;
> +}
> static int __hci_init(struct hci_dev *hdev)
> {
> 	int err;
> @@ -937,6 +942,9 @@ static int __hci_init(struct hci_dev *hdev)
> 	if (err < 0)
> 		return err;
> 
> +	err = __hci_req_sync(hdev, hci_init5_req, 0, HCI_INIT_TIMEOUT, NULL);
> +	if (err < 0)
> +		return err;
> 	/* 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.
> diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
> index 5e99968939ce..a4b905a76c1b 100644
> --- a/net/bluetooth/hci_event.c
> +++ b/net/bluetooth/hci_event.c
> @@ -887,6 +887,70 @@ static void hci_cc_read_data_block_size(struct hci_dev *hdev,
> 	       hdev->block_cnt, hdev->block_len);
> }
> 
> +static void hci_cc_read_local_codecs(struct hci_dev *hdev,
> +				     struct sk_buff *skb)
> +{
> +	__u8 num_codecs;
> +	struct hci_op_read_local_codec_caps caps;
> +
> +	if (skb->len < sizeof(caps))
> +		return;
> +
> +	bt_dev_dbg(hdev, "status 0x%2.2x", skb->data[0]);
> +
> +	if (skb->data[0])
> +		return;
> +
> +	/* enumerate standard codecs */
> +	skb_pull(skb, 1);
> +
> +	if (skb->len < 1)
> +		return;
> +
> +	num_codecs = skb->data[0];
> +
> +	bt_dev_dbg(hdev, "Number of standard codecs: %u", num_codecs);
> +
> +	skb_pull(skb, 1);
> +
> +	if (skb->len < num_codecs)
> +		return;
> +
> +	while (num_codecs--) {
> +		caps.codec_id[0] = skb->data[0];
> +		caps.transport = 0x00;
> +		caps.direction = 0x00;
> +
> +		hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +			     &caps);
> +

Here you just store the codec ids.

> +		skb_pull(skb, 1);
> +	}
> +
> +	/* enumerate vendor specific codecs */
> +	if (skb->len < 1)
> +		return;
> +
> +	num_codecs = skb->data[0];
> +	skb_pull(skb, 1);
> +
> +	bt_dev_dbg(hdev, "Number of vendor specific codecs: %u", num_codecs);
> +
> +	if (skb->len < (num_codecs * 4))
> +		return;
> +
> +	while (num_codecs--) {
> +		caps.codec_id[0] = 0xFF;
> +		memcpy(&caps.codec_id[1], skb->data, 4);
> +		caps.transport = 0x00;
> +		caps.direction = 0x00;
> +
> +		hci_send_cmd(hdev, HCI_OP_READ_LOCAL_CODEC_CAPS, sizeof(caps),
> +			     &caps);
> +		skb_pull(skb, 4);
> +	}
> +}
> +
> static void hci_cc_read_clock(struct hci_dev *hdev, struct sk_buff *skb)
> {
> 	struct hci_rp_read_clock *rp = (void *) skb->data;
> @@ -3437,6 +3501,10 @@ static void hci_cmd_complete_evt(struct hci_dev *hdev, struct sk_buff *skb,
> 		hci_cc_read_data_block_size(hdev, skb);
> 		break;
> 
> +	case HCI_OP_READ_LOCAL_CODECS:
> +		hci_cc_read_local_codecs(hdev, skb);
> +		break;
> +
> 	case HCI_OP_READ_FLOW_CONTROL_MODE:
> 		hci_cc_read_flow_control_mode(hdev, skb);
> 		break;

Regards

Marcel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 2/3] Bluetooth: add support to enumerate local supports codecs v2
  2021-04-22 14:14 ` [PATCH v3 2/3] Bluetooth: add support to enumerate local supports codecs v2 Kiran K
@ 2021-04-23  8:00   ` Marcel Holtmann
  0 siblings, 0 replies; 10+ messages in thread
From: Marcel Holtmann @ 2021-04-23  8:00 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, Chethan T N, Srivatsa Ravishankar

Hi Kiran,

> Add support for HCI_Read_Local_Supported_Codecs_V2 and query codec
> capabilities
> 
> 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 v3:
>  No changes
> 
> * changes in v2:
>  add length check for event data before accessing
> 
> include/net/bluetooth/hci.h      |   2 +
> include/net/bluetooth/hci_core.h |  10 +++
> net/bluetooth/hci_core.c         |   4 +-
> net/bluetooth/hci_event.c        | 110 +++++++++++++++++++++++++++++++
> 4 files changed, 125 insertions(+), 1 deletion(-)

I would rather move the v2 support to 3/3 in this series.

Regards

Marcel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities
  2021-04-22 14:14 ` [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities Kiran K
@ 2021-04-23  8:04   ` Marcel Holtmann
  2021-04-28 13:53     ` K, Kiran
  0 siblings, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2021-04-23  8:04 UTC (permalink / raw)
  To: Kiran K; +Cc: linux-bluetooth, Chethan T N, Srivatsa Ravishankar

Hi Kiran,

> Cache the codec information in the driver and this data can
> be exposed to user space audio modules via getsockopt
> 
> 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 v3
>  remove unwanted log
> 
> * changes in v2
>  add skb length check before accessing data
> 
> include/net/bluetooth/hci.h      | 11 +++++++++++
> include/net/bluetooth/hci_core.h | 13 +++++++++++++
> net/bluetooth/hci_core.c         | 31 +++++++++++++++++++++++++++++++
> net/bluetooth/hci_event.c        | 32 ++++++++++++++++++++++++++++++++
> 4 files changed, 87 insertions(+)
> 
> diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
> index 34eb9f4b027f..6b4344639ff7 100644
> --- a/include/net/bluetooth/hci.h
> +++ b/include/net/bluetooth/hci.h
> @@ -1323,6 +1323,17 @@ struct hci_op_read_local_codec_caps {
> 	__u8	direction;
> } __packed;
> 
> +struct hci_codec_caps {
> +	__u8	len;
> +	__u8	caps[];
> +} __packed;
> +
> +struct hci_rp_read_local_codec_caps {
> +	__u8	status;
> +	__u8	num_caps;
> +	struct hci_codec_caps 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 2c19b02a805d..b40c7ed38d18 100644
> --- a/include/net/bluetooth/hci_core.h
> +++ b/include/net/bluetooth/hci_core.h
> @@ -131,6 +131,14 @@ struct bdaddr_list {
> 	u8 bdaddr_type;
> };
> 
> +struct codec_list {
> +	struct list_head list;
> +	u8	transport;
> +	u8	codec_id[5];
> +	u8	num_caps;
> +	struct hci_codec_caps caps[];
> +};
> +
> struct bdaddr_list_with_irk {
> 	struct list_head list;
> 	bdaddr_t bdaddr;
> @@ -534,6 +542,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;
> 
> @@ -1843,6 +1852,10 @@ void hci_le_start_enc(struct hci_conn *conn, __le16 ediv, __le64 rand,
> 
> void hci_copy_identity_address(struct hci_dev *hdev, bdaddr_t *bdaddr,
> 			       u8 *bdaddr_type);
> +int hci_codec_list_add(struct list_head *list, struct hci_rp_read_local_codec_caps *rp,
> +		       __u32 len,
> +		       struct hci_op_read_local_codec_caps *sent);

I think you need to redo the whole patch series, since 1/3 should have hci_codec_list_add in that it adds the codec id from reading the codec list.

And then reading the capabilities just updates the codec.

Our problem is that the whole init phase is rather async than sync in it procedure. And the reason for that is purely historic from the times when Linus had no work queues and we had to process everything in tasklets or spawn kthreads.

Frankly if we moved the whole init procedure to use __hci_cmd_sync we could fold the complete init{1-4} phases into one. And there is no reason we don’t do that. However one problem at a time.

Regards

Marcel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities
  2021-04-23  8:04   ` Marcel Holtmann
@ 2021-04-28 13:53     ` K, Kiran
  2021-04-28 14:49       ` Marcel Holtmann
  0 siblings, 1 reply; 10+ messages in thread
From: K, Kiran @ 2021-04-28 13:53 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth, Tumkur Narayan, Chethan, Srivatsa, Ravishankar

Hi Marcel,

> > +int hci_codec_list_add(struct list_head *list, struct
> hci_rp_read_local_codec_caps *rp,
> > +		       __u32 len,
> > +		       struct hci_op_read_local_codec_caps *sent);
> 
> I think you need to redo the whole patch series, since 1/3 should have
> hci_codec_list_add in that it adds the codec id from reading the codec list.
> 
Ack

> And then reading the capabilities just updates the codec.
> 
With async calls converted to sync,  can we add codec details to the list on reading codec caps as same codec can be supported on multiple transport types ?

> Our problem is that the whole init phase is rather async than sync in it
> procedure. And the reason for that is purely historic from the times when
> Linus had no work queues and we had to process everything in tasklets or
> spawn kthreads.
> 
> Frankly if we moved the whole init procedure to use __hci_cmd_sync we
> could fold the complete init{1-4} phases into one. And there is no reason we
> don’t do that. However one problem at a time.
> 
Ack. I will define init5 for reading codecs and codec caps  using __hci_cmd_sync  calls.

> Regards
> 
> Marcel

Thanks,
Kiran


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities
  2021-04-28 13:53     ` K, Kiran
@ 2021-04-28 14:49       ` Marcel Holtmann
  2021-04-28 15:11         ` K, Kiran
  0 siblings, 1 reply; 10+ messages in thread
From: Marcel Holtmann @ 2021-04-28 14:49 UTC (permalink / raw)
  To: K, Kiran; +Cc: linux-bluetooth, Tumkur Narayan, Chethan, Srivatsa, Ravishankar

Hi Kiran,

>>> +int hci_codec_list_add(struct list_head *list, struct
>> hci_rp_read_local_codec_caps *rp,
>>> +		       __u32 len,
>>> +		       struct hci_op_read_local_codec_caps *sent);
>> 
>> I think you need to redo the whole patch series, since 1/3 should have
>> hci_codec_list_add in that it adds the codec id from reading the codec list.
>> 
> Ack
> 
>> And then reading the capabilities just updates the codec.
>> 
> With async calls converted to sync,  can we add codec details to the list on reading codec caps as same codec can be supported on multiple transport types ?
> 
>> Our problem is that the whole init phase is rather async than sync in it
>> procedure. And the reason for that is purely historic from the times when
>> Linus had no work queues and we had to process everything in tasklets or
>> spawn kthreads.
>> 
>> Frankly if we moved the whole init procedure to use __hci_cmd_sync we
>> could fold the complete init{1-4} phases into one. And there is no reason we
>> don’t do that. However one problem at a time.
>> 
> Ack. I will define init5 for reading codecs and codec caps  using __hci_cmd_sync  calls.

I think this is too early. I only looked at the intermingling of hci_update_background_scan. I have not gotten the whole init handling. I suspect some looking issues that would have to be cleaned up first.

Regards

Marcel


^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities
  2021-04-28 14:49       ` Marcel Holtmann
@ 2021-04-28 15:11         ` K, Kiran
  0 siblings, 0 replies; 10+ messages in thread
From: K, Kiran @ 2021-04-28 15:11 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: linux-bluetooth, Tumkur Narayan, Chethan, Srivatsa, Ravishankar

Hi Marcel,

> -----Original Message-----
> From: Marcel Holtmann <marcel@holtmann.org>
> Sent: Wednesday, April 28, 2021 8:20 PM
> To: K, Kiran <kiran.k@intel.com>
> Cc: linux-bluetooth@vger.kernel.org; Tumkur Narayan, Chethan
> <chethan.tumkur.narayan@intel.com>; Srivatsa, Ravishankar
> <ravishankar.srivatsa@intel.com>
> Subject: Re: [PATCH v3 3/3] Bluetooth: cache local supported codec
> capabilities
> 
> Hi Kiran,
> 
> >>> +int hci_codec_list_add(struct list_head *list, struct
> >> hci_rp_read_local_codec_caps *rp,
> >>> +		       __u32 len,
> >>> +		       struct hci_op_read_local_codec_caps *sent);
> >>
> >> I think you need to redo the whole patch series, since 1/3 should
> >> have hci_codec_list_add in that it adds the codec id from reading the
> codec list.
> >>
> > Ack
> >
> >> And then reading the capabilities just updates the codec.
> >>
> > With async calls converted to sync,  can we add codec details to the list on
> reading codec caps as same codec can be supported on multiple transport
> types ?
> >
> >> Our problem is that the whole init phase is rather async than sync in
> >> it procedure. And the reason for that is purely historic from the
> >> times when Linus had no work queues and we had to process everything
> >> in tasklets or spawn kthreads.
> >>
> >> Frankly if we moved the whole init procedure to use __hci_cmd_sync we
> >> could fold the complete init{1-4} phases into one. And there is no
> >> reason we don’t do that. However one problem at a time.
> >>
> > Ack. I will define init5 for reading codecs and codec caps  using
> __hci_cmd_sync  calls.
> 
> I think this is too early. I only looked at the intermingling of
> hci_update_background_scan. I have not gotten the whole init handling. I
> suspect some looking issues that would have to be cleaned up first.

In my test, I didn't see any issue. In that case, let me know how to proceed further. I am happy to amend as per your comments.

Thanks,
Kiran


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-04-28 15:11 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-22 14:14 [PATCH v3 1/3] Bluetooth: add support to enumerate codec capabilities Kiran K
2021-04-22 14:14 ` [PATCH v3 2/3] Bluetooth: add support to enumerate local supports codecs v2 Kiran K
2021-04-23  8:00   ` Marcel Holtmann
2021-04-22 14:14 ` [PATCH v3 3/3] Bluetooth: cache local supported codec capabilities Kiran K
2021-04-23  8:04   ` Marcel Holtmann
2021-04-28 13:53     ` K, Kiran
2021-04-28 14:49       ` Marcel Holtmann
2021-04-28 15:11         ` K, Kiran
2021-04-22 15:08 ` [v3,1/3] Bluetooth: add support to enumerate " bluez.test.bot
2021-04-23  7:58 ` [PATCH v3 1/3] " Marcel Holtmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).