This patch series are to support SCO offload for QCA2066, ALL BTHOST
needs to do is specifying both Input_Data_Path and Output_Data_Path
as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
to configure data path by HCI_Configure_Data_Path at all.
Zijun Hu (2):
Bluetooth: hci_conn: Check non NULL before calling
hdev->get_codec_config_data()
Bluetooth: qca: Support SCO offload for QCA2066
drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
net/bluetooth/hci_conn.c | 2 +-
2 files changed, 20 insertions(+), 1 deletion(-)
--
The Qualcomm Innovation Center
Explicitly add non NULL checking before calling
hdev->get_codec_config_data().
Signed-off-by: Zijun Hu <[email protected]>
---
net/bluetooth/hci_conn.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index 2cee330188ce..d647eb4606a7 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -346,7 +346,7 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
bt_dev_dbg(hdev, "hcon %p", conn);
/* for offload use case, codec needs to configured before opening SCO */
- if (conn->codec.data_path)
+ if (conn->codec.data_path && hdev->get_codec_config_data)
configure_datapath_sync(hdev, &conn->codec);
conn->state = BT_CONNECT;
--
The Qualcomm Innovation Center
In order to support SCO offload for QCA2066, ALL BTHOST needs to do
is specifying both Input_Data_Path and Output_Data_Path as 0x01 for
HCI_Enhanced_Setup_Synchronous_Connection, and it is implemented by
this change.
Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 35f74f209d1f..68347b2a204a 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1815,6 +1815,21 @@ static void hci_coredump_qca(struct hci_dev *hdev)
kfree_skb(skb);
}
+static int qca_get_data_path_id(struct hci_dev *hdev, __u8 *data_path_id)
+{
+ /* QCA uses 1 as data path id for SCO offload */
+ *data_path_id = 1;
+ return 0;
+}
+
+static int qca_configure_sco_offload(struct hci_dev *hdev)
+{
+ bt_dev_info(hdev, "SCO offload is supported");
+ hdev->get_data_path_id = qca_get_data_path_id;
+ hdev->get_codec_config_data = NULL;
+ return 0;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1969,6 +1984,10 @@ static int qca_setup(struct hci_uart *hu)
hu->hdev->set_bdaddr = qca_set_bdaddr_rome;
else
hu->hdev->set_bdaddr = qca_set_bdaddr;
+
+ if (soc_type == QCA_QCA2066)
+ qca_configure_sco_offload(hdev);
+
qca->fw_version = le16_to_cpu(ver.patch_ver);
qca->controller_id = le16_to_cpu(ver.rom_ver);
hci_devcd_register(hdev, hci_coredump_qca, qca_dmp_hdr, NULL);
--
The Qualcomm Innovation Center
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=798985
---Test result---
Test Summary:
CheckPatch PASS 1.25 seconds
GitLint FAIL 0.94 seconds
SubjectPrefix PASS 0.25 seconds
BuildKernel PASS 27.61 seconds
CheckAllWarning PASS 30.20 seconds
CheckSparse PASS 35.48 seconds
CheckSmatch PASS 98.58 seconds
BuildKernel32 PASS 29.28 seconds
TestRunnerSetup PASS 427.69 seconds
TestRunner_l2cap-tester PASS 23.08 seconds
TestRunner_iso-tester PASS 46.62 seconds
TestRunner_bnep-tester PASS 7.08 seconds
TestRunner_mgmt-tester FAIL 180.03 seconds
TestRunner_rfcomm-tester PASS 11.19 seconds
TestRunner_sco-tester PASS 14.49 seconds
TestRunner_ioctl-tester PASS 12.27 seconds
TestRunner_mesh-tester PASS 8.76 seconds
TestRunner_smp-tester PASS 9.78 seconds
TestRunner_userchan-tester PASS 7.43 seconds
IncrementalBuild PASS 31.31 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v1,1/2] Bluetooth: hci_conn: Check non NULL before calling hdev->get_codec_config_data()
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (89>80): "[v1,1/2] Bluetooth: hci_conn: Check non NULL before calling hdev->get_codec_config_data()"
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 497, Passed: 496 (99.8%), Failed: 1, Not Run: 0
Failed Test Cases
Pairing Acceptor - SMP over BR/EDR 2 Timed out 2.358 seconds
---
Regards,
Linux Bluetooth
Hi,
On Mon, Nov 6, 2023 at 1:19 AM Zijun Hu <[email protected]> wrote:
>
> This patch series are to support SCO offload for QCA2066, ALL BTHOST
> needs to do is specifying both Input_Data_Path and Output_Data_Path
> as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
> to configure data path by HCI_Configure_Data_Path at all.
This part it doesn't need to use HCI_Configure_Data_Path seems to be
non-standard, if so it needs to be handled by the driver, also it is
probably a good idea to explain how it works, what are the commands
used and the result traffic using btmon to collect the HCI trace.
> Zijun Hu (2):
> Bluetooth: hci_conn: Check non NULL before calling
> hdev->get_codec_config_data()
> Bluetooth: qca: Support SCO offload for QCA2066
>
> drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
> net/bluetooth/hci_conn.c | 2 +-
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> --
> The Qualcomm Innovation Center
>
--
Luiz Augusto von Dentz
On 11/7/2023 12:16 AM, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Mon, Nov 6, 2023 at 1:19 AM Zijun Hu <[email protected]> wrote:
>>
>> This patch series are to support SCO offload for QCA2066, ALL BTHOST
>> needs to do is specifying both Input_Data_Path and Output_Data_Path
>> as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
>> to configure data path by HCI_Configure_Data_Path at all.
>
> This part it doesn't need to use HCI_Configure_Data_Path seems to be
> non-standard, if so it needs to be handled by the driver, also it is
> probably a good idea to explain how it works, what are the commands
> used and the result traffic using btmon to collect the HCI trace.
>
My change does NOT touch current BT core driver logic at all. i just assign NULL to
hdev->get_codec_config_data within QCA device driver. so it follows current kernel
offload design.
BTW, Core spec also does not specify standard procedures for SCO offload since it is
vendor specific.
>> Zijun Hu (2):
>> Bluetooth: hci_conn: Check non NULL before calling
>> hdev->get_codec_config_data()
>> Bluetooth: qca: Support SCO offload for QCA2066
>>
>> drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
>> net/bluetooth/hci_conn.c | 2 +-
>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>
>> --
>> The Qualcomm Innovation Center
>>
>
>
Hi,
On Mon, Nov 6, 2023 at 9:47 PM quic_zijuhu <[email protected]> wrote:
>
> On 11/7/2023 12:16 AM, Luiz Augusto von Dentz wrote:
> > Hi,
> >
> > On Mon, Nov 6, 2023 at 1:19 AM Zijun Hu <[email protected]> wrote:
> >>
> >> This patch series are to support SCO offload for QCA2066, ALL BTHOST
> >> needs to do is specifying both Input_Data_Path and Output_Data_Path
> >> as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
> >> to configure data path by HCI_Configure_Data_Path at all.
> >
> > This part it doesn't need to use HCI_Configure_Data_Path seems to be
> > non-standard, if so it needs to be handled by the driver, also it is
> > probably a good idea to explain how it works, what are the commands
> > used and the result traffic using btmon to collect the HCI trace.
> >
> My change does NOT touch current BT core driver logic at all. i just assign NULL to
> hdev->get_codec_config_data within QCA device driver. so it follows current kernel
> offload design.
>
> BTW, Core spec also does not specify standard procedures for SCO offload since it is
> vendor specific.
We should probably document the expectation so it is clearer to the
driver what to expect, also the offload must be selected by the
application via socket interface as the HCI routing is the default, so
if the controller defaults to offload that needs fixing.
As for this change in specific, the configure data path function can
check if the driver does implement it, so we don't have to check it
before calling avoiding duplicate code.
> >> Zijun Hu (2):
> >> Bluetooth: hci_conn: Check non NULL before calling
> >> hdev->get_codec_config_data()
> >> Bluetooth: qca: Support SCO offload for QCA2066
> >>
> >> drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
> >> net/bluetooth/hci_conn.c | 2 +-
> >> 2 files changed, 20 insertions(+), 1 deletion(-)
> >>
> >> --
> >> The Qualcomm Innovation Center
> >>
> >
> >
>
--
Luiz Augusto von Dentz
On 11/18/2023 12:02 AM, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Mon, Nov 6, 2023 at 9:47 PM quic_zijuhu <[email protected]> wrote:
>>
>> On 11/7/2023 12:16 AM, Luiz Augusto von Dentz wrote:
>>> Hi,
>>>
>>> On Mon, Nov 6, 2023 at 1:19 AM Zijun Hu <[email protected]> wrote:
>>>>
>>>> This patch series are to support SCO offload for QCA2066, ALL BTHOST
>>>> needs to do is specifying both Input_Data_Path and Output_Data_Path
>>>> as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
>>>> to configure data path by HCI_Configure_Data_Path at all.
>>>
>>> This part it doesn't need to use HCI_Configure_Data_Path seems to be
>>> non-standard, if so it needs to be handled by the driver, also it is
>>> probably a good idea to explain how it works, what are the commands
>>> used and the result traffic using btmon to collect the HCI trace.
>>>
>> My change does NOT touch current BT core driver logic at all. i just assign NULL to
>> hdev->get_codec_config_data within QCA device driver. so it follows current kernel
>> offload design.
>>
>> BTW, Core spec also does not specify standard procedures for SCO offload since it is
>> vendor specific.
>
> We should probably document the expectation so it is clearer to the
> driver what to expect, also the offload must be selected by the
> application via socket interface as the HCI routing is the default, so
> if the controller defaults to offload that needs fixing.
>
i will add comments within hci_qca.c to document QC offload usage as you suggested.
the controller(firmware) supports both offload or non-offload, if setup SCO/eSCO by
HCI_Enhanced_Setup_Synchronous_Connection with data path parameter as 0x00, then
controller will use HCI for audio data. if as 0x01, it will use non-HCI such as PCM.
From current kernel HFP offload design,if hdev->get_data_path_id() is implemented by
device driver. it means HFP offload is ALSO SUPPORTED. whether to use offload or not is
decided by user (upper BT application) as shown below:
if user wants to use offload, then they must include offload UUID in config option KernelExperimental
within /etc/bluetooth/main.conf and use setsockopt to config BT_CODEC.
if user does not want to use offload. then they just need to remove offload UUID from option
KernelExperimental
based on above understanding, i have below points even if out of discussion scope for this change.
1) term data path selection is more suitable than offload for current design.
offload related to performing HFP coding/decoding within controller. data path selection related to
transferring audio data by HCI or non-HCI such as PCM.
2) perhaps, use setsockopt to config BT_CODEC for offload AS DEFAULT within upper application or BLUEZ,
and just ONLY use the config option KernelExperimental to controller if to use offload.
> As for this change in specific, the configure data path function can
> check if the driver does implement it, so we don't have to check it
> before calling avoiding duplicate code.
>
current checking is simpler and more suitable than below 2 alternatives to prevent
send HCI_Configure_Data_Path since they need to take this scenario as error and return error code.
even if we indeed can't take vendor special design as error wrongly.
alternative 1): check and return error code within configure_datapath_sync().
alternative 2): implement hev->get_codec_config_data by returning error code within device driver hci_qca.c
besides, there are no suitable error code for this case.
>>>> Zijun Hu (2):
>>>> Bluetooth: hci_conn: Check non NULL before calling
>>>> hdev->get_codec_config_data()
>>>> Bluetooth: qca: Support SCO offload for QCA2066
>>>>
>>>> drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
>>>> net/bluetooth/hci_conn.c | 2 +-
>>>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>>>
>>>> --
>>>> The Qualcomm Innovation Center
>>>>
>>>
>>>
>>
>
>
In order to support SCO offload for QCA2066, ALL BTHOST needs to do
is specifying both Input_Data_Path and Output_Data_Path as 0x01 for
HCI_Enhanced_Setup_Synchronous_Connection, and it is implemented by
this change.
Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 35f74f209d1f..68347b2a204a 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1815,6 +1815,21 @@ static void hci_coredump_qca(struct hci_dev *hdev)
kfree_skb(skb);
}
+static int qca_get_data_path_id(struct hci_dev *hdev, __u8 *data_path_id)
+{
+ /* QCA uses 1 as data path id for SCO offload */
+ *data_path_id = 1;
+ return 0;
+}
+
+static int qca_configure_sco_offload(struct hci_dev *hdev)
+{
+ bt_dev_info(hdev, "SCO offload is supported");
+ hdev->get_data_path_id = qca_get_data_path_id;
+ hdev->get_codec_config_data = NULL;
+ return 0;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1969,6 +1984,10 @@ static int qca_setup(struct hci_uart *hu)
hu->hdev->set_bdaddr = qca_set_bdaddr_rome;
else
hu->hdev->set_bdaddr = qca_set_bdaddr;
+
+ if (soc_type == QCA_QCA2066)
+ qca_configure_sco_offload(hdev);
+
qca->fw_version = le16_to_cpu(ver.patch_ver);
qca->controller_id = le16_to_cpu(ver.rom_ver);
hci_devcd_register(hdev, hci_coredump_qca, qca_dmp_hdr, NULL);
--
The Qualcomm Innovation Center
From: Zijun Hu <[email protected]>
This patch series are to support SCO offload for QCA2066, ALL BTHOST
needs to do is specifying both Input_Data_Path and Output_Data_Path
as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
to configure data path by HCI_Configure_Data_Path at all.
Changes form V1->V2:
- Bluetooth: qca:
- - Add and correct inline comments
Zijun Hu (2):
Bluetooth: hci_conn: Check non NULL before calling
hdev->get_codec_config_data()
Bluetooth: qca: Support SCO offload for QCA2066
drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
net/bluetooth/hci_conn.c | 2 +-
2 files changed, 20 insertions(+), 1 deletion(-)
--
The Qualcomm Innovation Center
In order to support SCO offload for QCA2066, ALL BTHOST needs to do
is specifying both Input_Data_Path and Output_Data_Path as 0x01 for
HCI_Enhanced_Setup_Synchronous_Connection, and it is implemented by
this change.
Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 35f74f209d1f..68347b2a204a 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1815,6 +1815,21 @@ static void hci_coredump_qca(struct hci_dev *hdev)
kfree_skb(skb);
}
+static int qca_get_data_path_id(struct hci_dev *hdev, __u8 *data_path_id)
+{
+ /* QCA uses 1 as data path id for SCO offload */
+ *data_path_id = 1;
+ return 0;
+}
+
+static int qca_configure_sco_offload(struct hci_dev *hdev)
+{
+ bt_dev_info(hdev, "SCO offload is supported");
+ hdev->get_data_path_id = qca_get_data_path_id;
+ hdev->get_codec_config_data = NULL;
+ return 0;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1969,6 +1984,10 @@ static int qca_setup(struct hci_uart *hu)
hu->hdev->set_bdaddr = qca_set_bdaddr_rome;
else
hu->hdev->set_bdaddr = qca_set_bdaddr;
+
+ if (soc_type == QCA_QCA2066)
+ qca_configure_sco_offload(hdev);
+
qca->fw_version = le16_to_cpu(ver.patch_ver);
qca->controller_id = le16_to_cpu(ver.rom_ver);
hci_devcd_register(hdev, hci_coredump_qca, qca_dmp_hdr, NULL);
--
The Qualcomm Innovation Center
From: Zijun Hu <[email protected]>
This patch series are to support SCO offload for QCA2066, ALL BTHOST
needs to do is specifying both Input_Data_Path and Output_Data_Path
as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
to configure data path by HCI_Configure_Data_Path at all.
Changes form V1->V2:
- Bluetooth: qca:
- - Add and correct inline comments
Zijun Hu (2):
Bluetooth: hci_conn: Check non NULL before calling
hdev->get_codec_config_data()
Bluetooth: qca: Support SCO offload for QCA2066
drivers/bluetooth/hci_qca.c | 24 ++++++++++++++++++++++++
net/bluetooth/hci_conn.c | 2 +-
2 files changed, 25 insertions(+), 1 deletion(-)
--
The Qualcomm Innovation Center
In order to support SCO offload for QCA2066, ALL BTHOST needs to do
is specifying both Input_Data_Path and Output_Data_Path as 0x01 for
HCI_Enhanced_Setup_Synchronous_Connection, and it is implemented by
this change.
Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 35f74f209d1f..970b1bcf7dde 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1815,6 +1815,26 @@ static void hci_coredump_qca(struct hci_dev *hdev)
kfree_skb(skb);
}
+static int qca_get_data_path_id(struct hci_dev *hdev, __u8 *data_path_id)
+{
+ /* QCA uses 1 as non-HCI data path id for HFP */
+ *data_path_id = 1;
+ return 0;
+}
+
+static int qca_configure_sco_offload(struct hci_dev *hdev)
+{
+ bt_dev_info(hdev, "HFP non-HCI data transport is supported");
+ hdev->get_data_path_id = qca_get_data_path_id;
+ /* Do not need to send HCI_Configure_Data_Path to configure QCA non-HCI
+ * data transport since HCI_Enhanced_Setup_Synchronous_Connection is
+ * enough for QCA controller to configure HFP data transport, so set
+ * below field as NULL.
+ */
+ hdev->get_codec_config_data = NULL;
+ return 0;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1969,6 +1989,10 @@ static int qca_setup(struct hci_uart *hu)
hu->hdev->set_bdaddr = qca_set_bdaddr_rome;
else
hu->hdev->set_bdaddr = qca_set_bdaddr;
+
+ if (soc_type == QCA_QCA2066)
+ qca_configure_sco_offload(hdev);
+
qca->fw_version = le16_to_cpu(ver.patch_ver);
qca->controller_id = le16_to_cpu(ver.rom_ver);
hci_devcd_register(hdev, hci_coredump_qca, qca_dmp_hdr, NULL);
--
The Qualcomm Innovation Center
From: Zijun Hu <[email protected]>
This patch series are to support SCO offload for QCA2066, ALL BTHOST
needs to do is specifying both Input_Data_Path and Output_Data_Path
as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
to configure data path by HCI_Configure_Data_Path at all.
Changes form V2->V3:
- Bluetooth: qca:
- - Add and correct inline comments
Changes form V1->V2:
- No changes, sent by mistake
Zijun Hu (2):
Bluetooth: hci_conn: Check non NULL before calling
hdev->get_codec_config_data()
Bluetooth: qca: Support SCO offload for QCA2066
drivers/bluetooth/hci_qca.c | 24 ++++++++++++++++++++++++
net/bluetooth/hci_conn.c | 2 +-
2 files changed, 25 insertions(+), 1 deletion(-)
--
The Qualcomm Innovation Center
Explicitly add non NULL checking before calling
hdev->get_codec_config_data().
Signed-off-by: Zijun Hu <[email protected]>
---
net/bluetooth/hci_conn.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index a09071059214..21b59d674717 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -346,7 +346,7 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
bt_dev_dbg(hdev, "hcon %p", conn);
/* for offload use case, codec needs to configured before opening SCO */
- if (conn->codec.data_path)
+ if (conn->codec.data_path && hdev->get_codec_config_data)
configure_datapath_sync(hdev, &conn->codec);
conn->state = BT_CONNECT;
--
The Qualcomm Innovation Center
In order to support SCO offload for QCA2066, ALL BTHOST needs to do
is specifying both Input_Data_Path and Output_Data_Path as 0x01 for
HCI_Enhanced_Setup_Synchronous_Connection, and it is implemented by
this change.
Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 24 ++++++++++++++++++++++++
1 file changed, 24 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 35f74f209d1f..970b1bcf7dde 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1815,6 +1815,26 @@ static void hci_coredump_qca(struct hci_dev *hdev)
kfree_skb(skb);
}
+static int qca_get_data_path_id(struct hci_dev *hdev, __u8 *data_path_id)
+{
+ /* QCA uses 1 as non-HCI data path id for HFP */
+ *data_path_id = 1;
+ return 0;
+}
+
+static int qca_configure_sco_offload(struct hci_dev *hdev)
+{
+ bt_dev_info(hdev, "HFP non-HCI data transport is supported");
+ hdev->get_data_path_id = qca_get_data_path_id;
+ /* Do not need to send HCI_Configure_Data_Path to configure QCA non-HCI
+ * data transport since HCI_Enhanced_Setup_Synchronous_Connection is
+ * enough for QCA controller to configure HFP data transport, so set
+ * below field as NULL.
+ */
+ hdev->get_codec_config_data = NULL;
+ return 0;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1969,6 +1989,10 @@ static int qca_setup(struct hci_uart *hu)
hu->hdev->set_bdaddr = qca_set_bdaddr_rome;
else
hu->hdev->set_bdaddr = qca_set_bdaddr;
+
+ if (soc_type == QCA_QCA2066)
+ qca_configure_sco_offload(hdev);
+
qca->fw_version = le16_to_cpu(ver.patch_ver);
qca->controller_id = le16_to_cpu(ver.rom_ver);
hci_devcd_register(hdev, hci_coredump_qca, qca_dmp_hdr, NULL);
--
The Qualcomm Innovation Center
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=805152
---Test result---
Test Summary:
CheckPatch PASS 1.22 seconds
GitLint FAIL 1.01 seconds
SubjectPrefix PASS 0.24 seconds
BuildKernel PASS 27.79 seconds
CheckAllWarning PASS 30.51 seconds
CheckSparse PASS 35.74 seconds
CheckSmatch PASS 99.40 seconds
BuildKernel32 PASS 26.99 seconds
TestRunnerSetup PASS 420.48 seconds
TestRunner_l2cap-tester PASS 23.13 seconds
TestRunner_iso-tester PASS 47.68 seconds
TestRunner_bnep-tester PASS 7.08 seconds
TestRunner_mgmt-tester PASS 164.12 seconds
TestRunner_rfcomm-tester PASS 11.06 seconds
TestRunner_sco-tester PASS 14.38 seconds
TestRunner_ioctl-tester PASS 12.09 seconds
TestRunner_mesh-tester PASS 11.00 seconds
TestRunner_smp-tester PASS 9.95 seconds
TestRunner_userchan-tester PASS 7.34 seconds
IncrementalBuild PASS 30.77 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v3,1/2] Bluetooth: hci_conn: Check non NULL before calling hdev->get_codec_config_data()
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (89>80): "[v3,1/2] Bluetooth: hci_conn: Check non NULL before calling hdev->get_codec_config_data()"
---
Regards,
Linux Bluetooth
Hi Luiz,
FYI.
On 11/29/2023 11:29 AM, quic_zijuhu wrote:
> On 11/18/2023 12:02 AM, Luiz Augusto von Dentz wrote:
>> Hi,
>>
>> On Mon, Nov 6, 2023 at 9:47 PM quic_zijuhu <[email protected]> wrote:
>>>
>>> On 11/7/2023 12:16 AM, Luiz Augusto von Dentz wrote:
>>>> Hi,
>>>>
>>>> On Mon, Nov 6, 2023 at 1:19 AM Zijun Hu <[email protected]> wrote:
>>>>>
>>>>> This patch series are to support SCO offload for QCA2066, ALL BTHOST
>>>>> needs to do is specifying both Input_Data_Path and Output_Data_Path
>>>>> as 0x01 for HCI_Enhanced_Setup_Synchronous_Connection, does NOT need
>>>>> to configure data path by HCI_Configure_Data_Path at all.
>>>>
>>>> This part it doesn't need to use HCI_Configure_Data_Path seems to be
>>>> non-standard, if so it needs to be handled by the driver, also it is
>>>> probably a good idea to explain how it works, what are the commands
>>>> used and the result traffic using btmon to collect the HCI trace.
>>>>
>>> My change does NOT touch current BT core driver logic at all. i just assign NULL to
>>> hdev->get_codec_config_data within QCA device driver. so it follows current kernel
>>> offload design.
>>>
>>> BTW, Core spec also does not specify standard procedures for SCO offload since it is
>>> vendor specific.
>>
>> We should probably document the expectation so it is clearer to the
>> driver what to expect, also the offload must be selected by the
>> application via socket interface as the HCI routing is the default, so
>> if the controller defaults to offload that needs fixing.
>>
> i will add comments within hci_qca.c to document QC offload usage as you suggested.
> the controller(firmware) supports both offload or non-offload, if setup SCO/eSCO by
> HCI_Enhanced_Setup_Synchronous_Connection with data path parameter as 0x00, then
> controller will use HCI for audio data. if as 0x01, it will use non-HCI such as PCM.
>
> From current kernel HFP offload design,if hdev->get_data_path_id() is implemented by
> device driver. it means HFP offload is ALSO SUPPORTED. whether to use offload or not is
> decided by user (upper BT application) as shown below:
>
> if user wants to use offload, then they must include offload UUID in config option KernelExperimental
> within /etc/bluetooth/main.conf and use setsockopt to config BT_CODEC.
>
> if user does not want to use offload. then they just need to remove offload UUID from option
> KernelExperimental
>
> based on above understanding, i have below points even if out of discussion scope for this change.
> 1) term data path selection is more suitable than offload for current design.
> offload related to performing HFP coding/decoding within controller. data path selection related to
> transferring audio data by HCI or non-HCI such as PCM.
>
> 2) perhaps, use setsockopt to config BT_CODEC for offload AS DEFAULT within upper application or BLUEZ,
> and just ONLY use the config option KernelExperimental to controller if to use offload.
>
>> As for this change in specific, the configure data path function can
>> check if the driver does implement it, so we don't have to check it
>> before calling avoiding duplicate code.
>>
> current checking is simpler and more suitable than below 2 alternatives to prevent
> send HCI_Configure_Data_Path since they need to take this scenario as error and return error code.
> even if we indeed can't take vendor special design as error wrongly.
>
> alternative 1): check and return error code within configure_datapath_sync().
> alternative 2): implement hev->get_codec_config_data by returning error code within device driver hci_qca.c
> besides, there are no suitable error code for this case.
>>>>> Zijun Hu (2):
>>>>> Bluetooth: hci_conn: Check non NULL before calling
>>>>> hdev->get_codec_config_data()
>>>>> Bluetooth: qca: Support SCO offload for QCA2066
>>>>>
>>>>> drivers/bluetooth/hci_qca.c | 19 +++++++++++++++++++
>>>>> net/bluetooth/hci_conn.c | 2 +-
>>>>> 2 files changed, 20 insertions(+), 1 deletion(-)
>>>>>
>>>>> --
>>>>> The Qualcomm Innovation Center
>>>>>
>>>>
>>>>
>>>
>>
>>
>
This patch series are to support HFP offload for QCA2066.
Changes form V3->V4:
- Bluetooth: hci_conn:
- - Move non NULL checking into configure_datapath_sync()
- Bluetooth: qca:
- - Optimize commmit messages and comments and function names
Changes form V2->V3:
- Bluetooth: qca:
- - Add and correct inline comments
Changes form V1->V2:
- No changes, sent by mistake
Zijun Hu (2):
Bluetooth: hci_conn: Check non NULL function before calling for HFP
offload
Bluetooth: qca: Support HFP offload for QCA2066
drivers/bluetooth/hci_qca.c | 22 ++++++++++++++++++++++
net/bluetooth/hci_conn.c | 4 ++++
2 files changed, 26 insertions(+)
--
The Qualcomm Innovation Center
For some controllers such as QCA2066, it does not need to send
HCI_Configure_Data_Path to configure non-HCI data transport path to support
HFP offload, their device drivers may set hdev->get_codec_config_data as
NULL, so Explicitly add this non NULL checking before calling the function.
Signed-off-by: Zijun Hu <[email protected]>
---
net/bluetooth/hci_conn.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index a09071059214..0b757c3a7331 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -300,6 +300,10 @@ static int configure_datapath_sync(struct hci_dev *hdev, struct bt_codec *codec)
__u8 vnd_len, *vnd_data = NULL;
struct hci_op_configure_data_path *cmd = NULL;
+ /* Do not take me as error */
+ if (!hdev->get_codec_config_data)
+ return 0;
+
err = hdev->get_codec_config_data(hdev, ESCO_LINK, codec, &vnd_len,
&vnd_data);
if (err < 0)
--
The Qualcomm Innovation Center
For QCA2066 HFP offload, HCI_Configure_Data_Path is not required since
present HCI_Enhanced_Setup_Synchronous_Connection is enough to configure
non-HCI data transport path when set both Input_Data_Path and
Output_Data_Path parameters as 0x01, as is implemented by this change.
Signed-off-by: Zijun Hu <[email protected]>
---
drivers/bluetooth/hci_qca.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
index 35f74f209d1f..94b8c406f0c0 100644
--- a/drivers/bluetooth/hci_qca.c
+++ b/drivers/bluetooth/hci_qca.c
@@ -1815,6 +1815,24 @@ static void hci_coredump_qca(struct hci_dev *hdev)
kfree_skb(skb);
}
+static int qca_get_data_path_id(struct hci_dev *hdev, __u8 *data_path_id)
+{
+ /* QCA uses 1 as non-HCI data path id for HFP */
+ *data_path_id = 1;
+ return 0;
+}
+
+static int qca_configure_hfp_offload(struct hci_dev *hdev)
+{
+ bt_dev_info(hdev, "HFP non-HCI data transport is supported");
+ hdev->get_data_path_id = qca_get_data_path_id;
+ /* Do not need to send HCI_Configure_Data_Path to configure non-HCI
+ * data transport path for QCA controllers, so set below field as NULL.
+ */
+ hdev->get_codec_config_data = NULL;
+ return 0;
+}
+
static int qca_setup(struct hci_uart *hu)
{
struct hci_dev *hdev = hu->hdev;
@@ -1969,6 +1987,10 @@ static int qca_setup(struct hci_uart *hu)
hu->hdev->set_bdaddr = qca_set_bdaddr_rome;
else
hu->hdev->set_bdaddr = qca_set_bdaddr;
+
+ if (soc_type == QCA_QCA2066)
+ qca_configure_hfp_offload(hdev);
+
qca->fw_version = le16_to_cpu(ver.patch_ver);
qca->controller_id = le16_to_cpu(ver.rom_ver);
hci_devcd_register(hdev, hci_coredump_qca, qca_dmp_hdr, NULL);
--
The Qualcomm Innovation Center
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=808064
---Test result---
Test Summary:
CheckPatch PASS 1.30 seconds
GitLint FAIL 0.87 seconds
SubjectPrefix PASS 0.26 seconds
BuildKernel PASS 27.21 seconds
CheckAllWarning PASS 30.70 seconds
CheckSparse PASS 35.45 seconds
CheckSmatch PASS 98.65 seconds
BuildKernel32 PASS 26.75 seconds
TestRunnerSetup PASS 420.51 seconds
TestRunner_l2cap-tester PASS 22.91 seconds
TestRunner_iso-tester PASS 44.71 seconds
TestRunner_bnep-tester PASS 7.03 seconds
TestRunner_mgmt-tester PASS 166.15 seconds
TestRunner_rfcomm-tester PASS 10.97 seconds
TestRunner_sco-tester PASS 14.63 seconds
TestRunner_ioctl-tester PASS 12.14 seconds
TestRunner_mesh-tester PASS 8.86 seconds
TestRunner_smp-tester PASS 9.88 seconds
TestRunner_userchan-tester PASS 7.41 seconds
IncrementalBuild PASS 30.27 seconds
Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v4,1/2] Bluetooth: hci_conn: Check non NULL function before calling for HFP offload
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (84>80): "[v4,1/2] Bluetooth: hci_conn: Check non NULL function before calling for HFP offload"
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Fri, 8 Dec 2023 09:51:25 +0800 you wrote:
> This patch series are to support HFP offload for QCA2066.
>
> Changes form V3->V4:
> - Bluetooth: hci_conn:
> - - Move non NULL checking into configure_datapath_sync()
> - Bluetooth: qca:
> - - Optimize commmit messages and comments and function names
>
> [...]
Here is the summary with links:
- [v4,1/2] Bluetooth: hci_conn: Check non NULL function before calling for HFP offload
(no matching commit)
- [v4,2/2] Bluetooth: qca: Support HFP offload for QCA2066
https://git.kernel.org/bluetooth/bluetooth-next/c/3d41844703fe
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html