2015-08-11 14:35:34

by Frederic Danis

[permalink] [raw]
Subject: [PATCH v5 0/4] Bluetooth: hci_bcm: Add ACPI and PM support

These patches move PM support from rfkill-gpio to hci_uart module.
On, off, suspend and resume functions are supported.
They also parse the ACPI entry to find initial UART baud rate.

v4->v5:
- Rename parameter and variable of bcm_suspend and bcm_resume

v3->v4:
- Fix commit message of rfkill-gpio patch

v2->v3:
- Add bcm_device existance check as driver can be unbound and reference in
bcm_data may be invalid
- Add explanation of retrieval of bcm_device corresponding to tty in use
- Rename variables : ptr -> p, device_list -> bcm_device_list,
device_list_lock -> bcm_device_list_lock
- Remove BCM2E39 in rfkill-gpio.c instead of making it conditional

v1->v2:
- Add BCM2E67 device in acpi_device_id table
- Add suspend/resume functions

Frederic Danis (4):
Bluetooth: hci_bcm: Add PM for BCM devices
net: rfkill: gpio: Remove BCM2E39 support
Bluetooth: hci_bcm: Retrieve UART speed from ACPI
Bluetooth: hci_bcm: Add suspend/resume PM functions

drivers/bluetooth/hci_bcm.c | 303 +++++++++++++++++++++++++++++++++++++++++++-
net/rfkill/rfkill-gpio.c | 1 -
2 files changed, 301 insertions(+), 3 deletions(-)

--
1.9.1



2015-08-11 20:01:24

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v5 0/4] Bluetooth: hci_bcm: Add ACPI and PM support

Hi Fred,

> These patches move PM support from rfkill-gpio to hci_uart module.
> On, off, suspend and resume functions are supported.
> They also parse the ACPI entry to find initial UART baud rate.
>
> v4->v5:
> - Rename parameter and variable of bcm_suspend and bcm_resume
>
> v3->v4:
> - Fix commit message of rfkill-gpio patch
>
> v2->v3:
> - Add bcm_device existance check as driver can be unbound and reference in
> bcm_data may be invalid
> - Add explanation of retrieval of bcm_device corresponding to tty in use
> - Rename variables : ptr -> p, device_list -> bcm_device_list,
> device_list_lock -> bcm_device_list_lock
> - Remove BCM2E39 in rfkill-gpio.c instead of making it conditional
>
> v1->v2:
> - Add BCM2E67 device in acpi_device_id table
> - Add suspend/resume functions
>
> Frederic Danis (4):
> Bluetooth: hci_bcm: Add PM for BCM devices
> net: rfkill: gpio: Remove BCM2E39 support
> Bluetooth: hci_bcm: Retrieve UART speed from ACPI
> Bluetooth: hci_bcm: Add suspend/resume PM functions
>
> drivers/bluetooth/hci_bcm.c | 303 +++++++++++++++++++++++++++++++++++++++++++-
> net/rfkill/rfkill-gpio.c | 1 -
> 2 files changed, 301 insertions(+), 3 deletions(-)

all 4 patches have been applied to bluetooth-next tree.

However I had to add missing CONFIG_PM_SLEEP #ifdef around PM code. Please keep an eye on that next time.

Regards

Marcel


2015-08-11 14:35:38

by Frederic Danis

[permalink] [raw]
Subject: [PATCH v5 4/4] Bluetooth: hci_bcm: Add suspend/resume PM functions

Add reference to hci_uart structure to bcm_device.
This allows suspend/resume callbacks to manage UART flow control.

Signed-off-by: Frederic Danis <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 61 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 60 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 12490eb..38c1204 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -51,6 +51,9 @@ struct bcm_device {
bool clk_enabled;

u32 init_speed;
+
+ struct hci_uart *hu;
+ bool is_suspended; /* suspend/resume flag */
};

struct bcm_data {
@@ -170,6 +173,7 @@ static int bcm_open(struct hci_uart *hu)
if (hu->tty->dev->parent == dev->pdev->dev.parent) {
bcm->dev = dev;
hu->init_speed = dev->init_speed;
+ dev->hu = hu;
break;
}
}
@@ -190,8 +194,10 @@ static int bcm_close(struct hci_uart *hu)

/* Protect bcm->dev against removal of the device or driver */
spin_lock(&bcm_device_list_lock);
- if (bcm_device_exists(bcm->dev))
+ if (bcm_device_exists(bcm->dev)) {
bcm_gpio_set_power(bcm->dev, false);
+ bcm->dev->hu = NULL;
+ }
spin_unlock(&bcm_device_list_lock);

skb_queue_purge(&bcm->txq);
@@ -318,6 +324,53 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
return skb_dequeue(&bcm->txq);
}

+/* Platform suspend callback */
+static int bcm_suspend(struct device *dev)
+{
+ struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+
+ BT_DBG("suspend (%p): is_suspended %d", bdev, bdev->is_suspended);
+
+ if (!bdev->is_suspended) {
+ hci_uart_set_flow_control(bdev->hu, true);
+
+ /* Once this callback returns, driver suspends BT via GPIO */
+ bdev->is_suspended = true;
+ }
+
+ /* Suspend the device */
+ if (bdev->device_wakeup) {
+ gpiod_set_value(bdev->device_wakeup, false);
+ BT_DBG("suspend, delaying 15 ms");
+ mdelay(15);
+ }
+
+ return 0;
+}
+
+/* Platform resume callback */
+static int bcm_resume(struct device *dev)
+{
+ struct bcm_device *bdev = platform_get_drvdata(to_platform_device(dev));
+
+ BT_DBG("resume (%p): is_suspended %d", bdev, bdev->is_suspended);
+
+ if (bdev->device_wakeup) {
+ gpiod_set_value(bdev->device_wakeup, true);
+ BT_DBG("resume, delaying 15 ms");
+ mdelay(15);
+ }
+
+ /* When this callback executes, the device has woken up already */
+ if (bdev->is_suspended) {
+ bdev->is_suspended = false;
+
+ hci_uart_set_flow_control(bdev->hu, false);
+ }
+
+ return 0;
+}
+
static const struct acpi_gpio_params device_wakeup_gpios = { 0, 0, false };
static const struct acpi_gpio_params shutdown_gpios = { 1, 0, false };

@@ -474,12 +527,18 @@ static const struct acpi_device_id bcm_acpi_match[] = {
MODULE_DEVICE_TABLE(acpi, bcm_acpi_match);
#endif

+/* Platform suspend and resume callbacks */
+static const struct dev_pm_ops bcm_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(bcm_suspend, bcm_resume)
+};
+
static struct platform_driver bcm_driver = {
.probe = bcm_probe,
.remove = bcm_remove,
.driver = {
.name = "hci_bcm",
.acpi_match_table = ACPI_PTR(bcm_acpi_match),
+ .pm = &bcm_pm_ops,
},
};

--
1.9.1


2015-08-11 14:35:37

by Frederic Danis

[permalink] [raw]
Subject: [PATCH v5 3/4] Bluetooth: hci_bcm: Retrieve UART speed from ACPI

Parse platform_device's ACPI to retrieve UART init speed.
When BCM device is open, check if its TTY has same parent as one of the
platform devices saved. If yes, use platform_device's init speed.

Signed-off-by: Frederic Danis <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 28 ++++++++++++++++++++++++++++
1 file changed, 28 insertions(+)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 6ac6636..12490eb 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -49,6 +49,8 @@ struct bcm_device {

struct clk *clk;
bool clk_enabled;
+
+ u32 init_speed;
};

struct bcm_data {
@@ -167,6 +169,7 @@ static int bcm_open(struct hci_uart *hu)
*/
if (hu->tty->dev->parent == dev->pdev->dev.parent) {
bcm->dev = dev;
+ hu->init_speed = dev->init_speed;
break;
}
}
@@ -324,11 +327,29 @@ static const struct acpi_gpio_mapping acpi_bcm_default_gpios[] = {
{ },
};

+static int bcm_resource(struct acpi_resource *ares, void *data)
+{
+ struct bcm_device *dev = data;
+
+ if (ares->type == ACPI_RESOURCE_TYPE_SERIAL_BUS) {
+ struct acpi_resource_uart_serialbus *sb;
+
+ sb = &ares->data.uart_serial_bus;
+ if (sb->type == ACPI_RESOURCE_SERIAL_TYPE_UART)
+ dev->init_speed = sb->default_baud_rate;
+ }
+
+ /* Always tell the ACPI core to skip this resource */
+ return 1;
+}
+
static int bcm_acpi_probe(struct bcm_device *dev)
{
struct platform_device *pdev = dev->pdev;
const struct acpi_device_id *id;
struct gpio_desc *gpio;
+ struct acpi_device *adev;
+ LIST_HEAD(resources);
int ret;

id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
@@ -368,6 +389,13 @@ static int bcm_acpi_probe(struct bcm_device *dev)
return -EINVAL;
}

+ /* Retrieve UART ACPI info */
+ adev = ACPI_COMPANION(&dev->pdev->dev);
+ if (!adev)
+ return 0;
+
+ acpi_dev_get_resources(adev, &resources, bcm_resource, dev);
+
return 0;
}

--
1.9.1


2015-08-11 14:35:36

by Frederic Danis

[permalink] [raw]
Subject: [PATCH v5 2/4] net: rfkill: gpio: Remove BCM2E39 support

Power management support for BCM2E39 is now performed in Bluetooth
BCM UART driver.

Signed-off-by: Frederic Danis <[email protected]>
---
net/rfkill/rfkill-gpio.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/net/rfkill/rfkill-gpio.c b/net/rfkill/rfkill-gpio.c
index d5d58d9..9312722 100644
--- a/net/rfkill/rfkill-gpio.c
+++ b/net/rfkill/rfkill-gpio.c
@@ -164,7 +164,6 @@ static int rfkill_gpio_remove(struct platform_device *pdev)
#ifdef CONFIG_ACPI
static const struct acpi_device_id rfkill_acpi_match[] = {
{ "BCM2E1A", RFKILL_TYPE_BLUETOOTH },
- { "BCM2E39", RFKILL_TYPE_BLUETOOTH },
{ "BCM2E3D", RFKILL_TYPE_BLUETOOTH },
{ "BCM2E40", RFKILL_TYPE_BLUETOOTH },
{ "BCM2E64", RFKILL_TYPE_BLUETOOTH },
--
1.9.1


2015-08-11 14:35:35

by Frederic Danis

[permalink] [raw]
Subject: [PATCH v5 1/4] Bluetooth: hci_bcm: Add PM for BCM devices

Retrieve "shutdown" and "device_wakeup" GPIOs from ACPI.
Set device off during platform device enumeration.
Set device on only when attached.
As driver can be unbound we need to check if the bcm_device still exists
before calling GPIO's functions, this is protected using device_list_lock.

Signed-off-by: Frederic Danis <[email protected]>
---
drivers/bluetooth/hci_bcm.c | 216 +++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 214 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 23523e1..6ac6636 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -25,6 +25,12 @@
#include <linux/errno.h>
#include <linux/skbuff.h>
#include <linux/firmware.h>
+#include <linux/module.h>
+#include <linux/acpi.h>
+#include <linux/platform_device.h>
+#include <linux/clk.h>
+#include <linux/gpio/consumer.h>
+#include <linux/tty.h>

#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -32,11 +38,30 @@
#include "btbcm.h"
#include "hci_uart.h"

+struct bcm_device {
+ struct list_head list;
+
+ struct platform_device *pdev;
+
+ const char *name;
+ struct gpio_desc *device_wakeup;
+ struct gpio_desc *shutdown;
+
+ struct clk *clk;
+ bool clk_enabled;
+};
+
struct bcm_data {
- struct sk_buff *rx_skb;
- struct sk_buff_head txq;
+ struct sk_buff *rx_skb;
+ struct sk_buff_head txq;
+
+ struct bcm_device *dev;
};

+/* List of BCM BT UART devices */
+static DEFINE_SPINLOCK(bcm_device_list_lock);
+static LIST_HEAD(bcm_device_list);
+
static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
{
struct hci_dev *hdev = hu->hdev;
@@ -86,9 +111,41 @@ static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
return 0;
}

+/* bcm_device_exists should be protected by bcm_device_list_lock */
+static bool bcm_device_exists(struct bcm_device *device)
+{
+ struct list_head *p;
+
+ list_for_each(p, &bcm_device_list) {
+ struct bcm_device *dev = list_entry(p, struct bcm_device, list);
+
+ if (device == dev)
+ return true;
+ }
+
+ return false;
+}
+
+static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
+{
+ if (powered && !IS_ERR(dev->clk) && !dev->clk_enabled)
+ clk_enable(dev->clk);
+
+ gpiod_set_value_cansleep(dev->shutdown, powered);
+ gpiod_set_value_cansleep(dev->device_wakeup, powered);
+
+ if (!powered && !IS_ERR(dev->clk) && dev->clk_enabled)
+ clk_disable(dev->clk);
+
+ dev->clk_enabled = powered;
+
+ return 0;
+}
+
static int bcm_open(struct hci_uart *hu)
{
struct bcm_data *bcm;
+ struct list_head *p;

BT_DBG("hu %p", hu);

@@ -99,6 +156,26 @@ static int bcm_open(struct hci_uart *hu)
skb_queue_head_init(&bcm->txq);

hu->priv = bcm;
+
+ spin_lock(&bcm_device_list_lock);
+ list_for_each(p, &bcm_device_list) {
+ struct bcm_device *dev = list_entry(p, struct bcm_device, list);
+
+ /* Retrieve saved bcm_device based on parent of the
+ * platform device (saved during device probe) and
+ * parent of tty device used by hci_uart
+ */
+ if (hu->tty->dev->parent == dev->pdev->dev.parent) {
+ bcm->dev = dev;
+ break;
+ }
+ }
+
+ if (bcm->dev)
+ bcm_gpio_set_power(bcm->dev, true);
+
+ spin_unlock(&bcm_device_list_lock);
+
return 0;
}

@@ -108,6 +185,12 @@ static int bcm_close(struct hci_uart *hu)

BT_DBG("hu %p", hu);

+ /* Protect bcm->dev against removal of the device or driver */
+ spin_lock(&bcm_device_list_lock);
+ if (bcm_device_exists(bcm->dev))
+ bcm_gpio_set_power(bcm->dev, false);
+ spin_unlock(&bcm_device_list_lock);
+
skb_queue_purge(&bcm->txq);
kfree_skb(bcm->rx_skb);
kfree(bcm);
@@ -232,6 +315,113 @@ static struct sk_buff *bcm_dequeue(struct hci_uart *hu)
return skb_dequeue(&bcm->txq);
}

+static const struct acpi_gpio_params device_wakeup_gpios = { 0, 0, false };
+static const struct acpi_gpio_params shutdown_gpios = { 1, 0, false };
+
+static const struct acpi_gpio_mapping acpi_bcm_default_gpios[] = {
+ { "device-wakeup-gpios", &device_wakeup_gpios, 1 },
+ { "shutdown-gpios", &shutdown_gpios, 1 },
+ { },
+};
+
+static int bcm_acpi_probe(struct bcm_device *dev)
+{
+ struct platform_device *pdev = dev->pdev;
+ const struct acpi_device_id *id;
+ struct gpio_desc *gpio;
+ int ret;
+
+ id = acpi_match_device(pdev->dev.driver->acpi_match_table, &pdev->dev);
+ if (!id)
+ return -ENODEV;
+
+ /* Retrieve GPIO data */
+ dev->name = dev_name(&pdev->dev);
+ ret = acpi_dev_add_driver_gpios(ACPI_COMPANION(&pdev->dev),
+ acpi_bcm_default_gpios);
+ if (ret)
+ return ret;
+
+ dev->clk = devm_clk_get(&pdev->dev, NULL);
+
+ gpio = devm_gpiod_get(&pdev->dev, "device-wakeup");
+ if (!IS_ERR(gpio)) {
+ ret = gpiod_direction_output(gpio, 0);
+ if (ret)
+ return ret;
+ dev->device_wakeup = gpio;
+ }
+
+ gpio = devm_gpiod_get(&pdev->dev, "shutdown");
+ if (!IS_ERR(gpio)) {
+ ret = gpiod_direction_output(gpio, 0);
+ if (ret)
+ return ret;
+ dev->shutdown = gpio;
+ }
+
+ /* 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;
+ }
+
+ return 0;
+}
+
+static int bcm_probe(struct platform_device *pdev)
+{
+ struct bcm_device *dev;
+ struct acpi_device_id *pdata = pdev->dev.platform_data;
+ int ret;
+
+ dev = devm_kzalloc(&pdev->dev, sizeof(*dev), GFP_KERNEL);
+ if (!dev)
+ return -ENOMEM;
+
+ dev->pdev = pdev;
+
+ if (ACPI_HANDLE(&pdev->dev)) {
+ ret = bcm_acpi_probe(dev);
+ if (ret)
+ return ret;
+ } else if (pdata) {
+ dev->name = pdata->id;
+ } else {
+ return -ENODEV;
+ }
+
+ platform_set_drvdata(pdev, dev);
+
+ dev_info(&pdev->dev, "%s device registered.\n", dev->name);
+
+ /* Place this instance on the device list */
+ spin_lock(&bcm_device_list_lock);
+ list_add_tail(&dev->list, &bcm_device_list);
+ spin_unlock(&bcm_device_list_lock);
+
+ bcm_gpio_set_power(dev, false);
+
+ return 0;
+}
+
+static int bcm_remove(struct platform_device *pdev)
+{
+ struct bcm_device *dev = platform_get_drvdata(pdev);
+
+ spin_lock(&bcm_device_list_lock);
+ list_del(&dev->list);
+ spin_unlock(&bcm_device_list_lock);
+
+ acpi_dev_remove_driver_gpios(ACPI_COMPANION(&pdev->dev));
+
+ dev_info(&pdev->dev, "%s device unregistered.\n", dev->name);
+
+ return 0;
+}
+
static const struct hci_uart_proto bcm_proto = {
.id = HCI_UART_BCM,
.name = "BCM",
@@ -247,12 +437,34 @@ static const struct hci_uart_proto bcm_proto = {
.dequeue = bcm_dequeue,
};

+#ifdef CONFIG_ACPI
+static const struct acpi_device_id bcm_acpi_match[] = {
+ { "BCM2E39", 0 },
+ { "BCM2E67", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, bcm_acpi_match);
+#endif
+
+static struct platform_driver bcm_driver = {
+ .probe = bcm_probe,
+ .remove = bcm_remove,
+ .driver = {
+ .name = "hci_bcm",
+ .acpi_match_table = ACPI_PTR(bcm_acpi_match),
+ },
+};
+
int __init bcm_init(void)
{
+ platform_driver_register(&bcm_driver);
+
return hci_uart_register_proto(&bcm_proto);
}

int __exit bcm_deinit(void)
{
+ platform_driver_unregister(&bcm_driver);
+
return hci_uart_unregister_proto(&bcm_proto);
}
--
1.9.1