Hi Zijun, On Thu, May 2, 2024 at 10:06 AM Zijun Hu <quic_zijuhu@xxxxxxxxxxx> wrote: > > 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 property enable-gpios is not configured within DT|ACPI for QCA6390. > > The commit is to fix a use-after-free issue within qca_serdev_shutdown() > during reboot, but also introduces this regression issue regarding above > steps since the VSC is not sent to reset controller during warm reboot. > > Fixed by sending the VSC to reset controller within qca_serdev_shutdown() > once BT was ever enabled, and the use-after-free issue is also be fixed > by this change since serdev is still opened when send to serdev. > > Fixes: 272970be3dab ("Bluetooth: hci_qca: Fix driver shutdown on closed serdev") > Cc: stable@xxxxxxxxxxxxxxx > Reported-by: Wren Turkal <wt@xxxxxxxxxxxxxxxx> > Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218726 > Signed-off-by: Zijun Hu <quic_zijuhu@xxxxxxxxxxx> > Tested-by: Wren Turkal <wt@xxxxxxxxxxxxxxxx> > --- > 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 0c9c9ee56592..8e35c9091486 100644 > --- a/drivers/bluetooth/hci_qca.c > +++ b/drivers/bluetooth/hci_qca.c > @@ -2450,13 +2450,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)) This probably deserves a comment on why you end up with HCI_QUIRK_NON_PERSISTENT_SETUP and HCI_SETUP flags here, also why you are removing the flags above since that was introduce to prevent use-after-free this sort of revert it so I do wonder how serdev can still be open if you haven't tested for QCA_BT_OFF for example? > + if (test_bit(HCI_QUIRK_NON_PERSISTENT_SETUP, &hdev->quirks) || > + hci_dev_test_flag(hdev, HCI_SETUP)) > return; > > serdev_device_write_flush(serdev); > -- > 2.7.4 > -- Luiz Augusto von Dentz