Re: [PATCH 3/4] Bluetooth: btusb: mediatek: reset the device as WMT failed

[Sean Wang]

Hi Luiz,

On Wed, Sep 14, 2022 at 3:46 PM Luiz Augusto von Dentz
<luiz.dentz@xxxxxxxxx> wrote:
>
> Hi Sean,
>
> On Mon, Sep 12, 2022 at 3:18 PM <sean.wang@xxxxxxxxxxxx> wrote:
> >
> > From: Sean Wang <sean.wang@xxxxxxxxxxxx>
> >
> > Reset the BT device whenever the driver detected any WMT failure happened
> > to recover such kind of system-level error as soon as possible.
> >
> > Signed-off-by: Sean Wang <sean.wang@xxxxxxxxxxxx>
> > ---
> >  drivers/bluetooth/btusb.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index 653f57a98233..dc86726c8271 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -2576,6 +2576,10 @@ static int btusb_mtk_hci_wmt_sync(struct hci_dev *hdev,
> >         data->evt_skb = NULL;
> >  err_free_wc:
> >         kfree(wc);
> > +
> > +       if (err < 0)
> > +               btmtk_reset_sync(hdev);
>
> Doesn't reset itself can fail?

The reset is supposed not to fail so there is no return value is
designated in the function

>
> >         return err;
>
> It would probably be better to reset on error at the caller IMO, also
> in case it fails during firmware upload does reset even work? Also it

The reset is supposed to work even without the firmware uploaded but I
need to have further confirmation with fw folks to ensure this point.
Anyway, I will try to move the reset on the error at the caller or
based on the context in the next version because I thought again that
will also help
working out a patch to recover any error present at firmware
initialization that the driver currently cannot handle and the patch
cannot cover.

> would probably have been better to have its own file for vendor
> specific commands like this and use btmtk_ prefix as well.

I had tried to move btusb_mtk_hci_wmt_sync to btmtk.c to allow it to
be reused by all mtk bluetooth drivers but some reason stopped me from
doing that.
that is btusb_mtk_hci_wmt_sync has the reference to the data bundled
with btusb.c and it seemed a bit harder for me to split out from
btusb.c for the moment,
such as btusb data->flag the function will refer to and is shared by
all vendors, so I still temporarily leave the vendor-specific commands
there.
I think that would be easy to do if btusb.c can support a pointer in
struct btusb_data pointed to the vendor-specific data area where I can
put the flag and other
vendor-specific stuff the btmtk.c needed there.

             Sean
>
> >  }
> >
> > --
> > 2.25.1
> >
>
>
> --
> Luiz Augusto von Dentz