[lkml]   [2018]   [Mar]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
Patch in this message
Subject[PATCH AUTOSEL for 4.15 099/124] Bluetooth: hci_bcm: Mandate presence of shutdown and device wake GPIO
From: Lukas Wunner <>

[ 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.

Cc: Frédéric Danis <>
Cc: Loic Poulain <>
Cc: Hans de Goede <>
Cc: Uwe Kleine-König <>
Cc: Linus Walleij <>
Reviewed-by: Andy Shevchenko <>
Signed-off-by: Lukas Wunner <>
Signed-off-by: Marcel Holtmann <>
Signed-off-by: Sasha Levin <>
drivers/bluetooth/hci_bcm.c | 24 +++++++++---------------
1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 707c2d1b84c7..b8eeac4e6f35 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -577,11 +577,9 @@ static int bcm_suspend_device(struct device *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 device *dev)

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_device *dev)

dev->clk = devm_clk_get(dev->dev, NULL);

- dev->device_wakeup = devm_gpiod_get_optional(dev->dev,
- "device-wakeup",
+ dev->device_wakeup = devm_gpiod_get(dev->dev, "device-wakeup",
if (IS_ERR(dev->device_wakeup))
return PTR_ERR(dev->device_wakeup);

- dev->shutdown = devm_gpiod_get_optional(dev->dev, "shutdown",
+ dev->shutdown = devm_gpiod_get(dev->dev, "shutdown", GPIOD_OUT_LOW);
if (IS_ERR(dev->shutdown))
return PTR_ERR(dev->shutdown);

 \ /
  Last update: 2018-03-19 23:19    [W:0.618 / U:0.064 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site