From: Sean Wang <[email protected]>
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 <[email protected]>
---
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);
+
return err;
}
--
2.25.1
Il 13/09/22 00:18, [email protected] ha scritto:
> From: Sean Wang <[email protected]>
>
> 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 <[email protected]>
This looks like a fix, so you probably want a Fixes tag for backport.
Regards,
Angelo
> ---
> 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);
> +
> return err;
> }
>
Hi, Angelo
On Tue, Sep 13, 2022 at 1:23 AM AngeloGioacchino Del Regno
<[email protected]> wrote:
>
> Il 13/09/22 00:18, [email protected] ha scritto:
> > From: Sean Wang <[email protected]>
> >
> > 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 <[email protected]>
>
> This looks like a fix, so you probably want a Fixes tag for backport.
I didn't add the fix tag because there is not a previous patch that
had issues the patch needs to fix.
It would be looking more like an enhancement patch for me to fix up
the potential issue happening in the firmware where the existing
driver cannot detect and recover in time with .cmd_timeout callback
but actually, the kind of potential issue in firmware I was worried
about in the firmware didn't happen or being reported so far.
Sean
>
> Regards,
> Angelo
>
> > ---
> > 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);
> > +
> > return err;
> > }
> >
>
Hi Sean,
On Mon, Sep 12, 2022 at 3:18 PM <[email protected]> wrote:
>
> From: Sean Wang <[email protected]>
>
> 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 <[email protected]>
> ---
> 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?
> 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
would probably have been better to have its own file for vendor
specific commands like this and use btmtk_ prefix as well.
> }
>
> --
> 2.25.1
>
--
Luiz Augusto von Dentz
Hi Luiz,
On Wed, Sep 14, 2022 at 3:46 PM Luiz Augusto von Dentz
<[email protected]> wrote:
>
> Hi Sean,
>
> On Mon, Sep 12, 2022 at 3:18 PM <[email protected]> wrote:
> >
> > From: Sean Wang <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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