Hi Luiz,
On Fri, 2024-09-20 at 10:54 -0400, Luiz Augusto von Dentz wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hi Chris,
>
> On Thu, Sep 19, 2024 at 3:49 AM Chris Lu <chris.lu@xxxxxxxxxxxx>
> wrote:
> >
> > MediaTek claim an special usb intr interface for ISO data
> transmission.
> > The interface need to be released before unregistering hci device
> when
> > usb disconnect. Removing BT usb dongle without properly releasing
> the
> > interface may cause Kernel panic while unregister hci device.
> >
> > Signed-off-by: Chris Lu <chris.lu@xxxxxxxxxxxx>
> > ---
> > drivers/bluetooth/btusb.c | 12 +++++++-----
> > 1 file changed, 7 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
> > index dfc42bdc8aaf..37e67b451b34 100644
> > --- a/drivers/bluetooth/btusb.c
> > +++ b/drivers/bluetooth/btusb.c
> > @@ -2614,9 +2614,9 @@ static void btusb_mtk_claim_iso_intf(struct
> btusb_data *data)
> > set_bit(BTMTK_ISOPKT_OVER_INTR, &btmtk_data->flags);
> > }
> >
> > -static void btusb_mtk_release_iso_intf(struct btusb_data *data)
> > +static int btusb_mtk_release_iso_intf(struct hci_dev *hdev)
> > {
> > - struct btmtk_data *btmtk_data = hci_get_priv(data->hdev);
> > + struct btmtk_data *btmtk_data = hci_get_priv(hdev);
> >
> > if (btmtk_data->isopkt_intf) {
> > usb_kill_anchored_urbs(&btmtk_data->isopkt_anchor);
> > @@ -2630,6 +2630,8 @@ static void btusb_mtk_release_iso_intf(struct
> btusb_data *data)
> > }
> >
> > clear_bit(BTMTK_ISOPKT_OVER_INTR, &btmtk_data->flags);
> > +
> > + return 0;
> > }
> >
> > static int btusb_mtk_reset(struct hci_dev *hdev, void *rst_data)
> > @@ -2649,7 +2651,7 @@ static int btusb_mtk_reset(struct hci_dev
> *hdev, void *rst_data)
> > return err;
> >
> > if (test_bit(BTMTK_ISOPKT_RUNNING, &btmtk_data->flags))
> > - btusb_mtk_release_iso_intf(data);
> > + btusb_mtk_release_iso_intf(hdev);
>
> We can probably move the check for BTMTK_ISOPKT_RUNNING into
> btusb_mtk_release_iso_intf to avoid having to duplicate it whenever
> calling btusb_mtk_release_iso_intf.
>
Yes, condition check here is redundant, Keep the check in
btusb_mtk_release_iso_intf is better.
> >
> > btusb_stop_traffic(data);
> > usb_kill_anchored_urbs(&data->tx_anchor);
> > @@ -2703,14 +2705,13 @@ static int btusb_mtk_setup(struct hci_dev
> *hdev)
> >
> > static int btusb_mtk_shutdown(struct hci_dev *hdev)
> > {
> > - struct btusb_data *data = hci_get_drvdata(hdev);
> > struct btmtk_data *btmtk_data = hci_get_priv(hdev);
> > int ret;
> >
> > ret = btmtk_usb_shutdown(hdev);
> >
> > if (test_bit(BTMTK_ISOPKT_RUNNING, &btmtk_data->flags))
> > - btusb_mtk_release_iso_intf(data);
> > + btusb_mtk_release_iso_intf(hdev);
>
> Ditto.
>
remove redundant check.
> > return ret;
> > }
> > @@ -3824,6 +3825,7 @@ static int btusb_probe(struct usb_interface
> *intf,
> > data->recv_acl = btmtk_usb_recv_acl;
> > data->suspend = btmtk_usb_suspend;
> > data->resume = btmtk_usb_resume;
> > + data->disconnect = btusb_mtk_release_iso_intf;
>
> I'd wrap (e.g. btmtk_usb_disconnect) the call to
> btusb_mtk_release_iso_intf since that is only meant to release the
> ISO
> endpoint.
>
Add a new function "btusb_mtk_disconnect". Since this function will use
btusb_data which is defined in btusb.c only, I'll also define this
function in btusb.c.
Thanks for your advice. I'll submit v3 later.
Best regards,
Chris Lu
> > }
> >
> > if (id->driver_info & BTUSB_SWAVE) {
> > --
> > 2.18.0
> >
>
>
> --
> Luiz Augusto von Dentz
