Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Ismael Ferreras Morezuelas <swyterzone@gmail.com>
To:     marcel@holtmann.org, johan.hedberg@gmail.com, luiz.dentz@gmail.com,
        davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
        luiz.von.dentz@intel.com, quic_zijuhu@quicinc.com,
        hdegoede@redhat.com, swyterzone@gmail.com
Cc:     linux-kernel@vger.kernel.org, linux-bluetooth@vger.kernel.org,
        netdev@vger.kernel.org
Subject: [PATCH 3/3] Bluetooth: btusb: Add a parameter to let users disable the fake CSR force-suspend hack
Date:   Sat, 29 Oct 2022 22:24:54 +0200
Message-Id: <20221029202454.25651-3-swyterzone@gmail.com>
In-Reply-To: <20221029202454.25651-1-swyterzone@gmail.com>
References: <20221029202454.25651-1-swyterzone@gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

A few users have reported that their cloned Chinese dongle doesn't
work well with the hack Hans de Goede added, that tries this
off-on mechanism as a way to unfreeze them.

It's still more than worthwhile to have it, as in the vast majority
of cases it either completely brings dongles to life or just resets
them harmlessly as it already happens during normal USB operation.

This is nothing new and the controllers are expected to behave
correctly. But yeah, go figure. :)

For that unhappy minority we can easily handle this edge case by letting
users disable it via our «btusb.disable_fake_csr_forcesuspend_hack=1» kernel option.

I believe this is the most generic way of doing it, given the constraints
and by still having a good out-of-the-box experience.

No clone left behind.

Cc: stable@vger.kernel.org
Cc: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Ismael Ferreras Morezuelas <swyterzone@gmail.com>
---
 drivers/bluetooth/btusb.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

diff --git a/drivers/bluetooth/btusb.c b/drivers/bluetooth/btusb.c
index 8f34bf195bae..d31d4f925463 100644
--- a/drivers/bluetooth/btusb.c
+++ b/drivers/bluetooth/btusb.c
@@ -34,6 +34,7 @@ static bool force_scofix;
 static bool enable_autosuspend = IS_ENABLED(CONFIG_BT_HCIBTUSB_AUTOSUSPEND);
 static bool enable_poll_sync = IS_ENABLED(CONFIG_BT_HCIBTUSB_POLL_SYNC);
 static bool reset = true;
+static bool disable_fake_csr_forcesuspend_hack;
 
 static struct usb_driver btusb_driver;
 
@@ -2171,7 +2172,7 @@ static int btusb_setup_csr(struct hci_dev *hdev)
 		is_fake = true;
 
 	if (is_fake) {
-		bt_dev_warn(hdev, "CSR: Unbranded CSR clone detected; adding workarounds and force-suspending once...");
+		bt_dev_warn(hdev, "CSR: Unbranded CSR clone detected; adding workarounds...");
 
 		/* Generally these clones have big discrepancies between
 		 * advertised features and what's actually supported.
@@ -2215,21 +2216,24 @@ static int btusb_setup_csr(struct hci_dev *hdev)
 		 * apply this initialization quirk to every controller that gets here,
 		 * it should be harmless. The alternative is to not work at all.
 		 */
-		pm_runtime_allow(&data->udev->dev);
+		if (!disable_fake_csr_forcesuspend_hack) {
+			bt_dev_warn(hdev, "CSR: Unbranded CSR clone detected; force-suspending once...");
+			pm_runtime_allow(&data->udev->dev);
 
-		ret = pm_runtime_suspend(&data->udev->dev);
-		if (ret >= 0)
-			msleep(200);
-		else
-			bt_dev_warn(hdev, "CSR: Couldn't suspend the device for our Barrot 8041a02 receive-issue workaround");
+			ret = pm_runtime_suspend(&data->udev->dev);
+			if (ret >= 0)
+				msleep(200);
+			else
+				bt_dev_warn(hdev, "CSR: Couldn't suspend the device for our Barrot 8041a02 receive-issue workaround");
 
-		pm_runtime_forbid(&data->udev->dev);
+			pm_runtime_forbid(&data->udev->dev);
 
-		device_set_wakeup_capable(&data->udev->dev, false);
+			device_set_wakeup_capable(&data->udev->dev, false);
 
-		/* Re-enable autosuspend if this was requested */
-		if (enable_autosuspend)
-			usb_enable_autosuspend(data->udev);
+			/* Re-enable autosuspend if this was requested */
+			if (enable_autosuspend)
+				usb_enable_autosuspend(data->udev);
+		}
 	}
 
 	kfree_skb(skb);
@@ -4312,6 +4316,9 @@ MODULE_PARM_DESC(enable_autosuspend, "Enable USB autosuspend by default");
 module_param(reset, bool, 0644);
 MODULE_PARM_DESC(reset, "Send HCI reset command on initialization");
 
+module_param(disable_fake_csr_forcesuspend_hack, bool, 0644);
+MODULE_PARM_DESC(disable_fake_csr_forcesuspend_hack, "Don't indiscriminately force-suspend Chinese-cloned CSR dongles trying to unfreeze them");
+
 MODULE_AUTHOR("Marcel Holtmann <marcel@holtmann.org>");
 MODULE_DESCRIPTION("Generic Bluetooth USB driver ver " VERSION);
 MODULE_VERSION(VERSION);
-- 
2.38.1