Skip to content

Commit

Permalink
Bluetooth: hci_bcm: Mandate presence of shutdown and device wake GPIO
Browse files Browse the repository at this point in the history
Commit 0395ffc ("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 8a92056 ("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 62aaefa ("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 <frederic.danis.oss@gmail.com>
Cc: Loic Poulain <loic.poulain@linaro.org>
Cc: Hans de Goede <hdegoede@redhat.com>
Cc: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
Cc: Linus Walleij <linus.walleij@linaro.org>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
Signed-off-by: Marcel Holtmann <marcel@holtmann.org>
  • Loading branch information
Lukas Wunner authored and Marcel Holtmann committed Jan 10, 2018
1 parent 61f5ace commit 3e81a4c
Showing 1 changed file with 9 additions and 15 deletions.
24 changes: 9 additions & 15 deletions drivers/bluetooth/hci_bcm.c
Original file line number Diff line number Diff line change
Expand Up @@ -572,11 +572,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;
}
Expand All @@ -587,11 +585,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) {
Expand Down Expand Up @@ -774,14 +770,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",
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);

Expand Down

0 comments on commit 3e81a4c

Please sign in to comment.