Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-277-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Message-ID: <bdb15439-52cd-4727-843a-cf6250a5a984@quicinc.com>
Date: Wed, 29 Nov 2023 11:29:55 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 0/2] Bluetooth: Support SCO offload for QCA2066
Content-Language: en-US
To: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
CC: <marcel@holtmann.org>, <johan.hedberg@gmail.com>,
        <linux-bluetooth@vger.kernel.org>
References: <1699251565-28759-1-git-send-email-quic_zijuhu@quicinc.com>
 <CABBYNZK6UNm9SWhmJtvp-bzObTk-UR7GhK2U16ffFEe-bdpOoA@mail.gmail.com>
 <b767f792-912d-0386-c891-4f571217e846@quicinc.com>
 <CABBYNZLrdX5pxWNL86NcE7DvSzvLN0xMTaMvJ265=CvKYmMMcw@mail.gmail.com>
From: quic_zijuhu <quic_zijuhu@quicinc.com>
In-Reply-To: <CABBYNZLrdX5pxWNL86NcE7DvSzvLN0xMTaMvJ265=CvKYmMMcw@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit

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