Received-SPF: pass (google.com: domain of linux-bluetooth+bounces-444-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <0bb83c21-97f5-4588-a834-ddc433d82f00@quicinc.com>
Date: Fri, 8 Dec 2023 05:59:05 +0800
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1 0/2] Bluetooth: Support SCO offload for QCA2066
From: quic_zijuhu <quic_zijuhu@quicinc.com>
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>
 <bdb15439-52cd-4727-843a-cf6250a5a984@quicinc.com>
Content-Language: en-US
In-Reply-To: <bdb15439-52cd-4727-843a-cf6250a5a984@quicinc.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit

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