| Message ID | 1713358336-29619-5-git-send-email-quic_zijuhu@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix 2 tool btattach issues for QCA controllers | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
Hi Zijun, On Wed, Apr 17, 2024 at 8:53 AM Zijun Hu <quic_zijuhu@quicinc.com> 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? > > Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> > --- > 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, > } 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 >
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@quicinc.com> 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@quicinc.com> >> --- >> 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 >> > >
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; } 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",
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. Signed-off-by: Zijun Hu <quic_zijuhu@quicinc.com> --- drivers/bluetooth/btqca.h | 1 + drivers/bluetooth/hci_qca.c | 8 +++++++- 2 files changed, 8 insertions(+), 1 deletion(-)