Bluetooth: hci_bcm: Mandate presence of shutdown and device wake GPIO
Commit0395ffc1ee
("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. Commit8a92056837
("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 commit62aaefa7d0
("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>
This commit is contained in:
parent
61f5acea87
commit
3e81a4ca51
|
@ -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);
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
@ -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);
|
||||
}
|
||||
|
||||
/* When this executes, the device has woken up already */
|
||||
if (bdev->is_suspended && bdev->hu) {
|
||||
|
@ -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",
|
||||
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);
|
||||
|
||||
|
|
Loading…
Reference in New Issue