| Message ID | 1699251565-28759-1-git-send-email-quic_zijuhu@quicinc.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Bluetooth: Support SCO offload for QCA2066 | expand |
Hi, On Mon, Nov 6, 2023 at 1:19 AM Zijun Hu <quic_zijuhu@quicinc.com> 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 >
On 11/7/2023 12:16 AM, Luiz Augusto von Dentz wrote: > Hi, > > On Mon, Nov 6, 2023 at 1:19 AM Zijun Hu <quic_zijuhu@quicinc.com> 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 <quic_zijuhu@quicinc.com> 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 <quic_zijuhu@quicinc.com> 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 > >> > > > > >
On 11/18/2023 12:02 AM, Luiz Augusto von Dentz wrote: > Hi, > > On Mon, Nov 6, 2023 at 9:47 PM quic_zijuhu <quic_zijuhu@quicinc.com> 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 <quic_zijuhu@quicinc.com> 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 >>>> >>> >>> >> > >
From: Zijun Hu <zijuhu@qti.qualcomm.com>
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(-)
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 <quic_zijuhu@quicinc.com> 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 <quic_zijuhu@quicinc.com> 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 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(-)