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@xxxxxxxxxxx> 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@xxxxxxxxxxx> 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 >>>>> >>>> >>>> >>> >> >> >
