Re: [PATCH 12/13] Bluetooth: btqca: Fix undetected error HCI status in qca_send_reset()

[Luiz Augusto von Dentz]

Hi Zijun,

On Mon, Jun 22, 2026 at 10:52 AM Zijun Hu <zijun.hu@xxxxxxxxxxxxxxxx> wrote:
>
> qca_send_reset() uses __hci_cmd_sync() which returns an skb but never
> reads the HCI status byte from skb->data[0], so a non-zero error status
> returned by the controller is silently ignored.
>
> Fix by replacing qca_send_reset() with __hci_reset_sync() which
> properly extracts and converts the HCI status byte to a negative errno.
>
> Fixes: 83e81961ff7e ("Bluetooth: btqca: Introduce generic QCA ROME support")
> Signed-off-by: Zijun Hu <zijun.hu@xxxxxxxxxxxxxxxx>
> ---
>  drivers/bluetooth/btqca.c | 22 ++--------------------
>  1 file changed, 2 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index 04ebe290bc78..875216e15603 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -190,25 +190,6 @@ static int qca_send_patch_config_cmd(struct hci_dev *hdev)
>         return err;
>  }
>
> -static int qca_send_reset(struct hci_dev *hdev)
> -{
> -       struct sk_buff *skb;
> -       int err;
> -
> -       bt_dev_dbg(hdev, "QCA HCI_RESET");
> -
> -       skb = __hci_cmd_sync(hdev, HCI_OP_RESET, 0, NULL, HCI_INIT_TIMEOUT);
> -       if (IS_ERR(skb)) {
> -               err = PTR_ERR(skb);
> -               bt_dev_err(hdev, "QCA Reset failed (%d)", err);
> -               return err;
> -       }
> -
> -       kfree_skb(skb);
> -
> -       return 0;
> -}
> -
>  static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
>  {
>         u8 cmd;
> @@ -990,7 +971,8 @@ int qca_uart_setup(struct hci_dev *hdev, uint8_t baudrate,
>         }
>
>         /* Perform HCI reset */
> -       err = qca_send_reset(hdev);
> +       bt_dev_dbg(hdev, "QCA HCI_RESET");
> +       err = __hci_reset_sync(hdev);

So All the fuss about the reset is just to use it here? Actually the
distinction between -err and status is rather important. The first
means the command didn't run (timeout, cancel, etc), or it did run but
returned a status != 0. If you want to capture both then you must use
if (err). And yes there are parts of the code that test for < 0, but
that is either a bug or they are intentionally ignoring the reset
status as it is probably a non-recoverable error at that point.

>         if (err < 0) {
>                 bt_dev_err(hdev, "QCA Failed to run HCI_RESET (%d)", err);
>                 return err;
>
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz