From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.8 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 51042C4338F for ; Fri, 6 Aug 2021 08:51:20 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 28F6860F70 for ; Fri, 6 Aug 2021 08:51:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S243591AbhHFIve (ORCPT ); Fri, 6 Aug 2021 04:51:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36584 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S241956AbhHFIve (ORCPT ); Fri, 6 Aug 2021 04:51:34 -0400 Received: from mail-wr1-x435.google.com (mail-wr1-x435.google.com [IPv6:2a00:1450:4864:20::435]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AE73EC061799 for ; Fri, 6 Aug 2021 01:51:17 -0700 (PDT) Received: by mail-wr1-x435.google.com with SMTP id c9so10063443wri.8 for ; Fri, 06 Aug 2021 01:51:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=baylibre-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:in-reply-to:references:date:message-id :mime-version; bh=i3HIreYA7e+PTu1uya0LcVYjOhfUuTdBKhb3eZlbvSg=; b=zoURBE/zCK6Jg4cUCzmyQJIGyj71YgQhER3ymo6aSPDUoFev60BJfnaw+mAOCrN5fD bw7oIJp5qEA05l44xiD7+bns6dI9WKYIgmzZRYxrklC3YhMomgIaR+ItVJmVxMTj+S4K rLECMHDLQ25XJJxI4jk34iFtXQhsPtJI6xibQUhZdsngzxYdkzlu4U/MAy5AQZ+QgGlf Motx9cIWsbXeG0wdyDbvwblZJCf9Js3fv8AwNfyHt7j1T9UHo3LXWzvayiJ1MBZL/y4y py5MFPKewTyv716au90cxNhcJapwOhbBoBMKI5Hbr6MZ6mX7WUs6vfoEF27onHlIx4Nf 5q+g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:in-reply-to:references:date :message-id:mime-version; bh=i3HIreYA7e+PTu1uya0LcVYjOhfUuTdBKhb3eZlbvSg=; b=gZK200kD5oNJ2pnxRVJvMDEybJBxdhbHGuChbDDOC4ldJpRCsE0Tqpiqy/dUQJUpsf Gzuc25Qyn9dKfni8Zur6Xcv0mPA/f5UOeUqOd+iAvCVbyOzeklz+trIegnb8krsbPwds jHCEPFiEDYVb3Qlui6oRR88ueKdrIhh20vGzRFQ6Z4xBvUuNDXV+6PdOPbOtUUJe0HTI dsCeUcKz+i++zIzIoa0aUNLgdjatOstrrKQ57mS1x0gXucfaxP3lvjbQxD6mDpho+6Vv 1urR9bCHTm3TwFUtZHae/4TnXqlCsasiKWcegDf15dlaSayu6kZhmTyeEy94mMlSGiYB FI0g== X-Gm-Message-State: AOAM532ik0thUGMrFAdjgOO/1qXOkBWT8juxi1vh30o6CDbNqgX5zj5a E7duwxTU+L8dJ/I/ZXW3bK7T9g== X-Google-Smtp-Source: ABdhPJwJeV37G5icUbCsLwbm3RQZzW8r3qxYqn1uYTQ3qULpCThIsIRQqL6WTfH8r6I6ljU3VE1sYg== X-Received: by 2002:a5d:6912:: with SMTP id t18mr9577971wru.234.1628239876154; Fri, 06 Aug 2021 01:51:16 -0700 (PDT) Received: from localhost ([2a01:cb19:826e:8e00:92f3:12fa:e0ef:50e9]) by smtp.gmail.com with ESMTPSA id q14sm8988749wrm.66.2021.08.06.01.51.15 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 06 Aug 2021 01:51:15 -0700 (PDT) From: Mattijs Korpershoek To: Kai-Heng Feng Cc: Marcel Holtmann , Johan Hedberg , Luiz Augusto von Dentz , "David S. Miller" , Jakub Kicinski , Fabien Parent , Sean Wang , "open list:BLUETOOTH SUBSYSTEM" , "open list:NETWORKING [GENERAL]" , open list Subject: Re: [PATCH v2] Bluetooth: Shutdown controller after workqueues are flushed or cancelled In-Reply-To: References: <20210514071452.25220-1-kai.heng.feng@canonical.com> <576B26FD-81F8-4632-82F6-57C4A7C096C4@holtmann.org> <8735ryk0o7.fsf@baylibre.com> <87y29o58su.fsf@baylibre.com> <87a6lzx7jf.fsf@baylibre.com> <87bl6cnzy2.fsf@baylibre.com> Date: Fri, 06 Aug 2021 10:51:14 +0200 Message-ID: <87tuk3j6rh.fsf@baylibre.com> MIME-Version: 1.0 Content-Type: text/plain Precedence: bulk List-ID: X-Mailing-List: linux-bluetooth@vger.kernel.org Hi Kai-Heng, Kai-Heng Feng writes: > Hi Mattijs, > > On Thu, Aug 5, 2021 at 2:55 PM Mattijs Korpershoek > wrote: >> >> Hi Kai-Heng, >> >> Thanks for your patch, >> >> Kai-Heng Feng writes: >> > > [snipped] > >> I confirm this diff works for me: >> >> root@i500-pumpkin:~# hciconfig hci0 up >> root@i500-pumpkin:~# hciconfig hci0 down >> root@i500-pumpkin:~# hciconfig hci0 up >> root@i500-pumpkin:~# hciconfig hci0 >> hci0: Type: Primary Bus: SDIO >> BD Address: 00:0C:E7:55:FF:12 ACL MTU: 1021:8 SCO MTU: 244:4 >> UP RUNNING >> RX bytes:11268 acl:0 sco:0 events:829 errors:0 >> TX bytes:182569 acl:0 sco:0 commands:829 errors:0 >> >> root@i500-pumpkin:~# hcitool scan >> Scanning ... >> Pixel 3 XL >> >> Tested-by: Mattijs Korpershoek > > I found that btmtksdio_flush() only cancels the work instead of doing > flush_work(). That probably explains why putting ->shutdown right > before ->flush doesn't work. > So can you please test the following again: > diff --git a/drivers/bluetooth/btmtksdio.c b/drivers/bluetooth/btmtksdio.c > index 9872ef18f9fea..b33c05ad2150b 100644 > --- a/drivers/bluetooth/btmtksdio.c > +++ b/drivers/bluetooth/btmtksdio.c > @@ -649,9 +649,9 @@ static int btmtksdio_flush(struct hci_dev *hdev) > { > struct btmtksdio_dev *bdev = hci_get_drvdata(hdev); > > - skb_queue_purge(&bdev->txq); > + flush_work(&bdev->tx_work); > > - cancel_work_sync(&bdev->tx_work); > + skb_queue_purge(&bdev->txq); > > return 0; > } > diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c > index 2560ed2f144d4..a61e610a400cb 100644 > > --- a/net/bluetooth/hci_core.c > +++ b/net/bluetooth/hci_core.c > @@ -1785,6 +1785,14 @@ int hci_dev_do_close(struct hci_dev *hdev) > aosp_do_close(hdev); > msft_do_close(hdev); > > + if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) && > + !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && > + test_bit(HCI_UP, &hdev->flags)) { > + /* Execute vendor specific shutdown routine */ > + if (hdev->shutdown) > + hdev->shutdown(hdev); > + } > + > if (hdev->flush) > hdev->flush(hdev); > > @@ -1798,14 +1806,6 @@ int hci_dev_do_close(struct hci_dev *hdev) > clear_bit(HCI_INIT, &hdev->flags); > } > > - if (!hci_dev_test_flag(hdev, HCI_UNREGISTER) && > - !hci_dev_test_flag(hdev, HCI_USER_CHANNEL) && > - test_bit(HCI_UP, &hdev->flags)) { > - /* Execute vendor specific shutdown routine */ > - if (hdev->shutdown) > - hdev->shutdown(hdev); > - } > - > /* flush cmd work */ > flush_work(&hdev->cmd_work); I've tried this but I have the same (broken) symptoms as before. Here are some logs of v3: dmesg: https://pastebin.com/1x4UHkzy ftrace: https://pastebin.com/Lm1d6AWy Mattijs > > Kai-Heng