On 4/18/2024 5:39 AM, Luiz Augusto von Dentz wrote:
> Hi Zijun,
>
> On Wed, Apr 17, 2024 at 8:53 AM Zijun Hu <quic_zijuhu@xxxxxxxxxxx> wrote:
>>
>> qca_soc_type() returns wrong soc type QCA_ROME when use tool btattach
>> for any other soc type such as QCA_QCA2066, and wrong soc type will
>> cause later initialization failure, fixed by using user specified
>> soc type for hci_uart derived from tool btattach.
>
> Then we have to fix qca_soc_type or explain what is going on that it
> can't detect the soc_type if initialized via btattach?
>
perhaps, my commit message is not precise and cause some mistook.
for tool btattach, only default QCA_ROME is used, there are no way to
get right soc type for other BT controllers. so i introduce a ioctl to let user specify
soc type info used. so i fix qca_soc_type() to use user specified soc type for tool btattach
case.
1) different soc types have different responses for VSC which is used to detect soc type
as shown by. so soc_type is can't be detected and it is needed by config by DT|ACPI or user specified.
int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
enum qca_btsoc_type soc_type)
2) soc type is a critical info, and it is used everywhere by hci_qca driver, it is also used to
decide which BT firmware to download as shown qca_uart_setup(), it soc type is not right. it will download
error BT firmware and cause serious results.
i will correct commit message for this patch.
>>
>> Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx>
>> ---
>> drivers/bluetooth/btqca.h | 1 +
>> drivers/bluetooth/hci_qca.c | 8 +++++++-
>> 2 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
>> index dc31984f71dc..a148d4c4e1bd 100644
>> --- a/drivers/bluetooth/btqca.h
>> +++ b/drivers/bluetooth/btqca.h
>> @@ -153,6 +153,7 @@ enum qca_btsoc_type {
>> QCA_WCN6750,
>> QCA_WCN6855,
>> QCA_WCN7850,
>> + QCA_MAX,
>> };
>>
>> #if IS_ENABLED(CONFIG_BT_QCA)
>> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c
>> index c04b97332bca..7c3577a4887c 100644
>> --- a/drivers/bluetooth/hci_qca.c
>> +++ b/drivers/bluetooth/hci_qca.c
>> @@ -238,12 +238,17 @@ static void qca_dmp_hdr(struct hci_dev *hdev, struct sk_buff *skb);
>>
>> static enum qca_btsoc_type qca_soc_type(struct hci_uart *hu)
>> {
>> + /* For Non-serdev device, hu->proto_data records soc type
>> + * set by ioctl HCIUARTSETPROTODATA.
>> + */
>> + int proto_data = (int)hu->proto_data;
>> enum qca_btsoc_type soc_type;
>>
>> if (hu->serdev) {
>> struct qca_serdev *qsd = serdev_device_get_drvdata(hu->serdev);
>> -
>> soc_type = qsd->btsoc_type;
>> + } else if ((proto_data > 0) && (proto_data < QCA_MAX)) {
>> + soc_type = (enum qca_btsoc_type)proto_data;
>
> Like I said a vendor specific ioctl will not gonna fly with me,
> specially since each vendor may need a different size to describe
> their controller version, etc,
>
i have comments about this part of this question in reply for [PATCH v2 3/4]
hci_uart->proto_data is a protocol specified unsigned long data, it is parsed
by specific protocol, for protocol, it is parsed as soc type. so force cast to
(enum qca_btsoc_type).
hci_uart->proto_data is mostly similar as @data of struct struct of_device_id defined by
below header file. it is assigned with misc data type and explained by specific device driver.
include/linux/mod_devicetable.h:
struct of_device_id {
char name[32];
char type[32];
char compatible[128];
const void *data;
};
>> } else {
>> soc_type = QCA_ROME;
>> }
>> @@ -2281,6 +2286,7 @@ static int qca_serdev_probe(struct serdev_device *serdev)
>> return -ENOMEM;
>>
>> qcadev->serdev_hu.serdev = serdev;
>> + qcadev->serdev_hu.proto_data = 0;
>> data = device_get_match_data(&serdev->dev);
>> serdev_device_set_drvdata(serdev, qcadev);
>> device_property_read_string(&serdev->dev, "firmware-name",
>> --
>> 2.7.4
>>
>
>
