| Message ID | 1713564212-21725-3-git-send-email-quic_zijuhu@quicinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix two regression issues for QCA controllers | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | fail | WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 1: T1 Title exceeds max length (93>80): "[v3,2/2] Bluetooth: qca: Fix BT enable failure for QCA_QCA6390 after disable then warm reboot" |
| tedd_an/SubjectPrefix | success | Gitlint PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
On 20/04/2024 00:03, Zijun Hu wrote: > From: Zijun Hu <zijuhu@qti.qualcomm.com> > > Commit 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed > serdev") will cause below regression issue: > > BT can't be enabled after below steps: > cold boot -> enable BT -> disable BT -> warm reboot -> BT enable failure > if BT reset pin is not configured within DT|ACPI for QCA_QCA6390. Please mention if QCA6390 requires reset pin, according to datasheet or some hardware guideline. > > Fixed by sending the VSC to reset controller within qca_serdev_shutdown() > once BT was ever enabled. > > Fixes: 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed serdev") > Reported-by: Wren Turkal <wt@penguintechs.org> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218726 > Signed-off-by: Zijun Hu <zijuhu@qti.qualcomm.com> > Tested-by: Wren Turkal <wt@penguintechs.org> > --- > drivers/bluetooth/hci_qca.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c > index 0934e74112a6..3f5173f1180b 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -2438,13 +2438,12 @@ static void qca_serdev_shutdown(struct device *dev) > struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev); > struct hci_uart *hu = &qcadev->serdev_hu; > struct hci_dev *hdev = hu->hdev; > - struct qca_data *qca = hu->priv; > const u8 ibs_wake_cmd[] = { 0xFD }; > const u8 edl_reset_soc_cmd[] = { 0x01, 0x00, 0xFC, 0x01, 0x05 }; > > if (qcadev->btsoc_type == QCA_QCA6390) { > - if (test_bit(QCA_BT_OFF, &qca->flags) || > - !test_bit(HCI_RUNNING, &hdev->flags)) > + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks) || > + hci_dev_test_flag(hdev, HCI_SETUP)) > return; I am sorry, but why do we need to perform shutdown procedure now if device is off? I raised questions about this and I still don't understand. Not much got better comparing to previous version. Actually, I have no clue what changed. Where is the changelog? Best regards, Krzysztof
On 4/20/2024 7:07 PM, Krzysztof Kozlowski wrote: > On 20/04/2024 00:03, Zijun Hu wrote: >> From: Zijun Hu <zijuhu@qti.qualcomm.com> >> >> Commit 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed >> serdev") will cause below regression issue: >> >> BT can't be enabled after below steps: >> cold boot -> enable BT -> disable BT -> warm reboot -> BT enable failure >> if BT reset pin is not configured within DT|ACPI for QCA_QCA6390. > > Please mention if QCA6390 requires reset pin, according to datasheet or > some hardware guideline. > will update commit message. >> >> Fixed by sending the VSC to reset controller within qca_serdev_shutdown() >> once BT was ever enabled. >> >> Fixes: 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed serdev") >> Reported-by: Wren Turkal <wt@penguintechs.org> >> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218726 >> Signed-off-by: Zijun Hu <zijuhu@qti.qualcomm.com> >> Tested-by: Wren Turkal <wt@penguintechs.org> >> --- >> drivers/bluetooth/hci_qca.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c >> index 0934e74112a6..3f5173f1180b 100644 >> --- a/drivers/bluetooth/hci_qca.c >> +++ b/drivers/bluetooth/hci_qca.c >> @@ -2438,13 +2438,12 @@ static void qca_serdev_shutdown(struct device *dev) >> struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev); >> struct hci_uart *hu = &qcadev->serdev_hu; >> struct hci_dev *hdev = hu->hdev; >> - struct qca_data *qca = hu->priv; >> const u8 ibs_wake_cmd[] = { 0xFD }; >> const u8 edl_reset_soc_cmd[] = { 0x01, 0x00, 0xFC, 0x01, 0x05 }; >> >> if (qcadev->btsoc_type == QCA_QCA6390) { >> - if (test_bit(QCA_BT_OFF, &qca->flags) || >> - !test_bit(HCI_RUNNING, &hdev->flags)) >> + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks) || >> + hci_dev_test_flag(hdev, HCI_SETUP)) >> return; > > I am sorry, but why do we need to perform shutdown procedure now if > device is off? > this is shutdown of serdev and not hdev's shutdown. > I raised questions about this and I still don't understand. Not much got > better comparing to previous version. Actually, I have no clue what > changed. Where is the changelog? > my reply at below link will help you https://lore.kernel.org/all/fe1a0e3b-3408-4a33-90e9-d4ffcfc7a99b@quicinc.com/ > > Best regards, > Krzysztof >
diff --git a/drivers/bluetooth/hci_qca.c b/drivers/bluetooth/hci_qca.c index 0934e74112a6..3f5173f1180b 100644 --- a/drivers/bluetooth/hci_qca.c +++ b/drivers/bluetooth/hci_qca.c @@ -2438,13 +2438,12 @@ static void qca_serdev_shutdown(struct device *dev) struct qca_serdev *qcadev = serdev_device_get_drvdata(serdev); struct hci_uart *hu = &qcadev->serdev_hu; struct hci_dev *hdev = hu->hdev; - struct qca_data *qca = hu->priv; const u8 ibs_wake_cmd[] = { 0xFD }; const u8 edl_reset_soc_cmd[] = { 0x01, 0x00, 0xFC, 0x01, 0x05 }; if (qcadev->btsoc_type == QCA_QCA6390) { - if (test_bit(QCA_BT_OFF, &qca->flags) || - !test_bit(HCI_RUNNING, &hdev->flags)) + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks) || + hci_dev_test_flag(hdev, HCI_SETUP)) return; serdev_device_write_flush(serdev);