Skip to contentSkip to conversation

Patch "Bluetooth: hci_bcm: Mandate presence of shutdown and device wake GPIO" has been added to the 4.15-stable tree

PATCH [email protected] 2 participants 3 messages Apr 9, 2018

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

Bluetooth: hci_bcm: Mandate presence of shutdown and device wake GPIO

to the 4.15-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
bluetooth-hci_bcm-mandate-presence-of-shutdown-and-device-wake-gpio.patch
and it can be found in the queue-4.15 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From foo@baz Mon Apr 9 10:16:32 CEST 2018

From: Lukas Wunner <[email protected]>
Date: Wed, 10 Jan 2018 16:32:10 +0100
Subject: Bluetooth: hci_bcm: Mandate presence of shutdown and device wake GPIO

From: Lukas Wunner <[email protected]>

[ Upstream commit 3e81a4ca51a1172253078ca7abd6a91040b8fcf4 ]

Commit 0395ffc1ee05 ("Bluetooth: hci_bcm: Add PM for BCM devices") amended this driver to request a shutdown and device wake GPIO on probe, but mandated that only one of them need to be present:

	/* Make sure at-least one of the GPIO is defined and that
	 * a name is specified for this instance
	 */
	if ((!dev->device_wakeup && !dev->shutdown) || !dev->name) {
		dev_err(&pdev->dev, "invalid platform data\n");
		return -EINVAL;
	}

However the same commit added a call to bcm_gpio_set_power() to the ->probe hook, which unconditionally accesses *both* GPIOs. Luckily, the resulting NULL pointer deref was never reported, suggesting there's no machine where either GPIO is missing.

Commit 8a92056837fd ("Bluetooth: hci_bcm: Add (runtime)pm support to the serdev driver") removed the check whether at least one of the GPIOs is present without specifying a reason.

Because commit 62aaefa7d038 ("Bluetooth: hci_bcm: improve use of gpios API") refactored the driver to use devm_gpiod_get_optional() instead of devm_gpiod_get(), one is now tempted to believe that the driver doesn't require *any* of the two GPIOs.

Which is wrong, the driver still requires both GPIOs to avoid a NULL pointer deref. To this end, establish the status quo ante and request the GPIOs with devm_gpiod_get() again. Bail out of ->probe if either of them is missing.

Oddly enough, whereas bcm_gpio_set_power() accesses the device wake pin unconditionally, bcm_suspend_device() and bcm_resume_device() do check for its presence before accessing it. Those checks are superfluous, so remove them.

---
 drivers/bluetooth/hci_bcm.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)
drivers/bluetooth/hci_bcm.c +9 -15
@@ -577,11 +577,9 @@ static int bcm_suspend_device(struct dev
 	}
 
 	/* Suspend the device */
-	if (bdev->device_wakeup) {
-		gpiod_set_value(bdev->device_wakeup, false);
-		bt_dev_dbg(bdev, "suspend, delaying 15 ms");
-		mdelay(15);
-	}
+	gpiod_set_value(bdev->device_wakeup, false);
+	bt_dev_dbg(bdev, "suspend, delaying 15 ms");
+	mdelay(15);
 
 	return 0;
 }
@@ -592,11 +590,9 @@ static int bcm_resume_device(struct devi
 
 	bt_dev_dbg(bdev, "");
 
-	if (bdev->device_wakeup) {
-		gpiod_set_value(bdev->device_wakeup, true);
-		bt_dev_dbg(bdev, "resume, delaying 15 ms");
-		mdelay(15);
-	}
+	gpiod_set_value(bdev->device_wakeup, true);
+	bt_dev_dbg(bdev, "resume, delaying 15 ms");
+	mdelay(15);
 
 	/* When this executes, the device has woken up already */
 	if (bdev->is_suspended && bdev->hu) {
@@ -779,14 +775,12 @@ static int bcm_get_resources(struct bcm_
 
 	dev->clk = devm_clk_get(dev->dev, NULL);
 
-	dev->device_wakeup = devm_gpiod_get_optional(dev->dev,
-						     "device-wakeup",
-						     GPIOD_OUT_LOW);
+	dev->device_wakeup = devm_gpiod_get(dev->dev, "device-wakeup",
+					    GPIOD_OUT_LOW);
 	if (IS_ERR(dev->device_wakeup))
 		return PTR_ERR(dev->device_wakeup);
 
-	dev->shutdown = devm_gpiod_get_optional(dev->dev, "shutdown",
-						GPIOD_OUT_LOW);
+	dev->shutdown = devm_gpiod_get(dev->dev, "shutdown", GPIOD_OUT_LOW);
 	if (IS_ERR(dev->shutdown))
 		return PTR_ERR(dev->shutdown);
 

Patches currently in stable-queue which might be from [email protected] are

queue-4.15/bluetooth-hci_bcm-mandate-presence-of-shutdown-and-device-wake-gpio.patch queue-4.15/bluetooth-hci_bcm-validate-irq-before-using-it.patch

#1 lore raw
Quoted 6 lines · On Mon, Apr 09, 2018 at 11:36:08AM +0200, [email protected] wrote:

This is a note to let you know that I've just added the patch titled

Bluetooth: hci_bcm: Mandate presence of shutdown and device wake GPIO

to the 4.15-stable tree

This commit unfortunately had to be fixed up by Stefan Wahren's upstream commit ab2f336cb7e6 ("Bluetooth: hci_bcm: Make shutdown and device wake GPIO optional").

Please be sure to apply that one on top. Thanks a lot.

Lukas

author maintainer
#2 lore raw
Quoted 6 lines · Lukas Wunner
Quoted 5 lines · On Mon, Apr 09, 2018 at 11:36:08AM +0200, [email protected] wrote:

This is a note to let you know that I've just added the patch titled

Bluetooth: hci_bcm: Mandate presence of shutdown and device wake GPIO

Quoted 7 lines
Quoted 1 line

to the 4.15-stable tree

This commit unfortunately had to be fixed up by Stefan Wahren's upstream commit ab2f336cb7e6 ("Bluetooth: hci_bcm: Make shutdown and device wake GPIO optional").

Please be sure to apply that one on top. Thanks a lot.

Now queued up, thanks!

greg k-h