Add LPM PM suspend/resume/host_wake LPM functions.
A LPM transaction is composed with a LPM request and ack/response.
Host can send a LPM suspend/resume request to the controller which
should respond with a LPM ack.
If resume is requested by the controller (irq), host has to send a LPM
ack once resumed.
Signed-off-by: Loic Poulain <[email protected]>
---
v2: Bracket coding style fixes.
Reorder lpm functions to avoid forward declaration.
drivers/bluetooth/hci_intel.c | 156 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 156 insertions(+)
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index 5d53185..eed1f72 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -46,12 +46,17 @@
#define STATE_BOOTING 4
#define STATE_LPM_ENABLED 5
#define STATE_TX_ACTIVE 6
+#define STATE_SUSPENDED 7
+#define STATE_LPM_TRANSACTION 8
+#define HCI_LPM_WAKE_PKT 0xf0
#define HCI_LPM_PKT 0xf1
#define HCI_LPM_MAX_SIZE 10
#define HCI_LPM_HDR_SIZE HCI_EVENT_HDR_SIZE
#define LPM_OP_TX_NOTIFY 0x00
+#define LPM_OP_SUSPEND_ACK 0x02
+#define LPM_OP_RESUME_ACK 0x03
struct hci_lpm_pkt {
__u8 opcode;
@@ -129,6 +134,141 @@ static int intel_wait_booting(struct hci_uart *hu)
return err;
}
+static int intel_wait_lpm_transaction(struct hci_uart *hu)
+{
+ struct intel_data *intel = hu->priv;
+ int err;
+
+ err = wait_on_bit_timeout(&intel->flags, STATE_LPM_TRANSACTION,
+ TASK_INTERRUPTIBLE,
+ msecs_to_jiffies(1000));
+
+ if (err == 1) {
+ bt_dev_err(hu->hdev, "LPM transaction interrupted");
+ return -EINTR;
+ }
+
+ if (err) {
+ bt_dev_err(hu->hdev, "LPM transaction timeout");
+ return -ETIMEDOUT;
+ }
+
+ return err;
+}
+
+static int intel_lpm_suspend(struct hci_uart *hu)
+{
+ static const u8 suspend[] = { 0x01, 0x01, 0x01 };
+ struct intel_data *intel = hu->priv;
+ struct sk_buff *skb;
+
+ if (!test_bit(STATE_LPM_ENABLED, &intel->flags) ||
+ test_bit(STATE_SUSPENDED, &intel->flags))
+ return 0;
+
+ if (test_bit(STATE_TX_ACTIVE, &intel->flags))
+ return -EAGAIN;
+
+ bt_dev_dbg(hu->hdev, "Suspending");
+
+ skb = bt_skb_alloc(sizeof(suspend), GFP_KERNEL);
+ if (!skb) {
+ bt_dev_err(hu->hdev, "Failed to alloc memory for LPM packet");
+ return -ENOMEM;
+ }
+
+ memcpy(skb_put(skb, sizeof(suspend)), suspend, sizeof(suspend));
+ bt_cb(skb)->pkt_type = HCI_LPM_PKT;
+
+ set_bit(STATE_LPM_TRANSACTION, &intel->flags);
+
+ skb_queue_tail(&intel->txq, skb);
+ hci_uart_tx_wakeup(hu);
+
+ intel_wait_lpm_transaction(hu);
+
+ clear_bit(STATE_LPM_TRANSACTION, &intel->flags);
+
+ if (!test_bit(STATE_SUSPENDED, &intel->flags)) {
+ bt_dev_err(hu->hdev, "Device suspend error");
+ return -EINVAL;
+ }
+
+ bt_dev_dbg(hu->hdev, "Suspended");
+
+ hci_uart_set_flow_control(hu, true);
+
+ return 0;
+}
+
+static int intel_lpm_resume(struct hci_uart *hu)
+{
+ struct intel_data *intel = hu->priv;
+ struct sk_buff *skb;
+
+ if (!test_bit(STATE_LPM_ENABLED, &intel->flags) ||
+ !test_bit(STATE_SUSPENDED, &intel->flags))
+ return 0;
+
+ bt_dev_dbg(hu->hdev, "Resuming");
+
+ hci_uart_set_flow_control(hu, false);
+
+ skb = bt_skb_alloc(0, GFP_KERNEL);
+ if (!skb) {
+ bt_dev_err(hu->hdev, "Failed to alloc memory for LPM packet");
+ return -ENOMEM;
+ }
+
+ bt_cb(skb)->pkt_type = HCI_LPM_WAKE_PKT;
+
+ set_bit(STATE_LPM_TRANSACTION, &intel->flags);
+
+ skb_queue_tail(&intel->txq, skb);
+ hci_uart_tx_wakeup(hu);
+
+ intel_wait_lpm_transaction(hu);
+
+ clear_bit(STATE_LPM_TRANSACTION, &intel->flags);
+
+ if (test_bit(STATE_SUSPENDED, &intel->flags)) {
+ bt_dev_err(hu->hdev, "Device resume error");
+ return -EINVAL;
+ }
+
+ bt_dev_dbg(hu->hdev, "Resumed");
+
+ return 0;
+}
+
+static int intel_lpm_host_wake(struct hci_uart *hu)
+{
+ static const u8 lpm_resume_ack[] = { LPM_OP_RESUME_ACK, 0x00 };
+ struct intel_data *intel = hu->priv;
+ struct sk_buff *skb;
+
+ hci_uart_set_flow_control(hu, false);
+
+ clear_bit(STATE_SUSPENDED, &intel->flags);
+
+ skb = bt_skb_alloc(sizeof(lpm_resume_ack), GFP_KERNEL);
+ if (!skb) {
+ bt_dev_err(hu->hdev, "Failed to alloc memory for LPM packet");
+ return -ENOMEM;
+ }
+
+ memcpy(skb_put(skb, sizeof(lpm_resume_ack)), lpm_resume_ack,
+ sizeof(lpm_resume_ack));
+ bt_cb(skb)->pkt_type = HCI_LPM_PKT;
+
+ skb_queue_tail(&intel->txq, skb);
+ hci_uart_tx_wakeup(hu);
+
+ bt_dev_dbg(hu->hdev, "Resumed by controller");
+
+ return 0;
+}
+
static irqreturn_t intel_irq(int irq, void *dev_id)
{
struct intel_device *idev = dev_id;
@@ -800,12 +940,28 @@ static void intel_recv_lpm_notify(struct hci_dev *hdev, int value)
static int intel_recv_lpm(struct hci_dev *hdev, struct sk_buff *skb)
{
struct hci_lpm_pkt *lpm = (void *)skb->data;
+ struct hci_uart *hu = hci_get_drvdata(hdev);
+ struct intel_data *intel = hu->priv;
switch (lpm->opcode) {
case LPM_OP_TX_NOTIFY:
if (lpm->dlen)
intel_recv_lpm_notify(hdev, lpm->data[0]);
break;
+ case LPM_OP_SUSPEND_ACK:
+ set_bit(STATE_SUSPENDED, &intel->flags);
+ if (test_and_clear_bit(STATE_LPM_TRANSACTION, &intel->flags)) {
+ smp_mb__after_atomic();
+ wake_up_bit(&intel->flags, STATE_LPM_TRANSACTION);
+ }
+ break;
+ case LPM_OP_RESUME_ACK:
+ clear_bit(STATE_SUSPENDED, &intel->flags);
+ if (test_and_clear_bit(STATE_LPM_TRANSACTION, &intel->flags)) {
+ smp_mb__after_atomic();
+ wake_up_bit(&intel->flags, STATE_LPM_TRANSACTION);
+ }
+ break;
default:
bt_dev_err(hdev, "Unknown LPM opcode (%02x)", lpm->opcode);
break;
--
1.9.1
Hi Loic,
> Implement runtime PM suspend/resume callbacks.
> If LPM supported, controller is put into supsend after a delay of
> inactivity (1s). Inactivity is based on LPM idle notification and
> host TX traffic.
>
> Signed-off-by: Loic Poulain <[email protected]>
> ---
> v2: Add pm_runtime comments.
> Use same suspend/resume callbacks for pm and runtime pm.
>
> drivers/bluetooth/hci_intel.c | 72 +++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 69 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
> index 5143554..8faf765 100644
> --- a/drivers/bluetooth/hci_intel.c
> +++ b/drivers/bluetooth/hci_intel.c
> @@ -32,6 +32,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/acpi.h>
> #include <linux/interrupt.h>
> +#include <linux/pm_runtime.h>
>
> #include <net/bluetooth/bluetooth.h>
> #include <net/bluetooth/hci_core.h>
> @@ -58,6 +59,8 @@
> #define LPM_OP_SUSPEND_ACK 0x02
> #define LPM_OP_RESUME_ACK 0x03
>
> +#define LPM_SUSPEND_DELAY_MS 1000
> +
> struct hci_lpm_pkt {
> __u8 opcode;
> __u8 dlen;
> @@ -79,6 +82,8 @@ static DEFINE_MUTEX(intel_device_list_lock);
> struct intel_data {
> struct sk_buff *rx_skb;
> struct sk_buff_head txq;
> + struct work_struct busy_work;
> + struct hci_uart *hu;
> unsigned long flags;
> };
>
> @@ -282,6 +287,11 @@ static irqreturn_t intel_irq(int irq, void *dev_id)
> intel_lpm_host_wake(idev->hu);
> mutex_unlock(&idev->hu_lock);
>
> + /* Host/Controller are now LPM resumed, trigger a new delayed suspend */
> + pm_runtime_get(&idev->pdev->dev);
> + pm_runtime_mark_last_busy(&idev->pdev->dev);
> + pm_runtime_put_autosuspend(&idev->pdev->dev);
> +
> return IRQ_HANDLED;
> }
>
> @@ -337,9 +347,17 @@ static int intel_set_power(struct hci_uart *hu, bool powered)
> }
>
> device_wakeup_enable(&idev->pdev->dev);
> +
> + pm_runtime_set_active(&idev->pdev->dev);
> + pm_runtime_use_autosuspend(&idev->pdev->dev);
> + pm_runtime_set_autosuspend_delay(&idev->pdev->dev,
> + LPM_SUSPEND_DELAY_MS);
> + pm_runtime_enable(&idev->pdev->dev);
> } else if (!powered && device_may_wakeup(&idev->pdev->dev)) {
> devm_free_irq(&idev->pdev->dev, idev->irq, idev);
> device_wakeup_disable(&idev->pdev->dev);
> +
> + pm_runtime_disable(&idev->pdev->dev);
> }
> }
>
> @@ -348,6 +366,28 @@ static int intel_set_power(struct hci_uart *hu, bool powered)
> return err;
> }
>
> +static void intel_busy_work(struct work_struct *work)
> +{
> + struct list_head *p;
> + struct intel_data *intel = container_of(work, struct intel_data,
> + busy_work);
> +
> + /* Link is busy, delay the suspend */
> + mutex_lock(&intel_device_list_lock);
> + list_for_each(p, &intel_device_list) {
> + struct intel_device *idev = list_entry(p, struct intel_device,
> + list);
Please double-check that list is indented correctly.
> +
> + if (intel->hu->tty->dev->parent == idev->pdev->dev.parent) {
> + pm_runtime_get(&idev->pdev->dev);
> + pm_runtime_mark_last_busy(&idev->pdev->dev);
> + pm_runtime_put_autosuspend(&idev->pdev->dev);
> + break;
> + }
> + }
> + mutex_unlock(&intel_device_list_lock);
> +}
> +
> static int intel_open(struct hci_uart *hu)
> {
> struct intel_data *intel;
> @@ -359,6 +399,9 @@ static int intel_open(struct hci_uart *hu)
> return -ENOMEM;
>
> skb_queue_head_init(&intel->txq);
> + INIT_WORK(&intel->busy_work, intel_busy_work);
> +
> + intel->hu = hu;
>
> hu->priv = intel;
>
> @@ -376,6 +419,8 @@ static int intel_close(struct hci_uart *hu)
>
> intel_set_power(hu, false);
>
> + cancel_work_sync(&intel->busy_work);
> +
Is this the right spot? Would you cancel the busy work first and then power down?
> skb_queue_purge(&intel->txq);
> kfree_skb(intel->rx_skb);
> kfree(intel);
> @@ -947,10 +992,12 @@ static void intel_recv_lpm_notify(struct hci_dev *hdev, int value)
>
> bt_dev_dbg(hdev, "TX idle notification (%d)", value);
>
> - if (value)
> + if (value) {
> set_bit(STATE_TX_ACTIVE, &intel->flags);
> - else
> + schedule_work(&intel->busy_work);
> + } else {
> clear_bit(STATE_TX_ACTIVE, &intel->flags);
> + }
> }
>
> static int intel_recv_lpm(struct hci_dev *hdev, struct sk_buff *skb)
> @@ -1025,9 +1072,27 @@ static int intel_recv(struct hci_uart *hu, const void *data, int count)
> static int intel_enqueue(struct hci_uart *hu, struct sk_buff *skb)
> {
> struct intel_data *intel = hu->priv;
> + struct list_head *p;
>
> BT_DBG("hu %p skb %p", hu, skb);
>
> + /* Be sure our controller is resumed and potential LPM transaction
> + * completed before enqueuing any packet.
> + */
> + mutex_lock(&intel_device_list_lock);
> + list_for_each(p, &intel_device_list) {
> + struct intel_device *idev = list_entry(p, struct intel_device,
> + list);
Same as above, but this might be just looking weird in a diff. So just double check.
> +
> + if (hu->tty->dev->parent == idev->pdev->dev.parent) {
> + pm_runtime_get_sync(&idev->pdev->dev);
> + pm_runtime_mark_last_busy(&idev->pdev->dev);
> + pm_runtime_put_autosuspend(&idev->pdev->dev);
> + break;
> + }
> + }
> + mutex_unlock(&intel_device_list_lock);
> +
> skb_queue_tail(&intel->txq, skb);
>
> return 0;
> @@ -1101,7 +1166,7 @@ static int intel_acpi_probe(struct intel_device *idev)
> }
> #endif
>
> -#ifdef CONFIG_PM_SLEEP
> +#ifdef CONFIG_PM
> static int intel_suspend(struct device *dev)
> {
> struct intel_device *idev = dev_get_drvdata(dev);
> @@ -1133,6 +1198,7 @@ static int intel_resume(struct device *dev)
>
> static const struct dev_pm_ops intel_pm_ops = {
> SET_SYSTEM_SLEEP_PM_OPS(intel_suspend, intel_resume)
> + SET_RUNTIME_PM_OPS(intel_suspend, intel_resume, NULL)
> };
Regards
Marcel
Hi Loic,
> Add LPM PM suspend/resume/host_wake LPM functions.
> A LPM transaction is composed with a LPM request and ack/response.
> Host can send a LPM suspend/resume request to the controller which
> should respond with a LPM ack.
> If resume is requested by the controller (irq), host has to send a LPM
> ack once resumed.
>
> Signed-off-by: Loic Poulain <[email protected]>
> ---
> v2: Bracket coding style fixes.
> Reorder lpm functions to avoid forward declaration.
>
> drivers/bluetooth/hci_intel.c | 156 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 156 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
> index 5d53185..eed1f72 100644
> --- a/drivers/bluetooth/hci_intel.c
> +++ b/drivers/bluetooth/hci_intel.c
> @@ -46,12 +46,17 @@
> #define STATE_BOOTING 4
> #define STATE_LPM_ENABLED 5
> #define STATE_TX_ACTIVE 6
> +#define STATE_SUSPENDED 7
> +#define STATE_LPM_TRANSACTION 8
>
> +#define HCI_LPM_WAKE_PKT 0xf0
> #define HCI_LPM_PKT 0xf1
> #define HCI_LPM_MAX_SIZE 10
> #define HCI_LPM_HDR_SIZE HCI_EVENT_HDR_SIZE
>
> #define LPM_OP_TX_NOTIFY 0x00
> +#define LPM_OP_SUSPEND_ACK 0x02
> +#define LPM_OP_RESUME_ACK 0x03
>
> struct hci_lpm_pkt {
> __u8 opcode;
> @@ -129,6 +134,141 @@ static int intel_wait_booting(struct hci_uart *hu)
> return err;
> }
>
> +static int intel_wait_lpm_transaction(struct hci_uart *hu)
> +{
> + struct intel_data *intel = hu->priv;
> + int err;
> +
> + err = wait_on_bit_timeout(&intel->flags, STATE_LPM_TRANSACTION,
> + TASK_INTERRUPTIBLE,
> + msecs_to_jiffies(1000));
> +
> + if (err == 1) {
> + bt_dev_err(hu->hdev, "LPM transaction interrupted");
> + return -EINTR;
> + }
> +
> + if (err) {
> + bt_dev_err(hu->hdev, "LPM transaction timeout");
> + return -ETIMEDOUT;
> + }
> +
> + return err;
> +}
> +
> +static int intel_lpm_suspend(struct hci_uart *hu)
> +{
> + static const u8 suspend[] = { 0x01, 0x01, 0x01 };
> + struct intel_data *intel = hu->priv;
> + struct sk_buff *skb;
> +
> + if (!test_bit(STATE_LPM_ENABLED, &intel->flags) ||
> + test_bit(STATE_SUSPENDED, &intel->flags))
> + return 0;
> +
> + if (test_bit(STATE_TX_ACTIVE, &intel->flags))
> + return -EAGAIN;
> +
> + bt_dev_dbg(hu->hdev, "Suspending");
> +
> + skb = bt_skb_alloc(sizeof(suspend), GFP_KERNEL);
> + if (!skb) {
> + bt_dev_err(hu->hdev, "Failed to alloc memory for LPM packet");
> + return -ENOMEM;
> + }
> +
> + memcpy(skb_put(skb, sizeof(suspend)), suspend, sizeof(suspend));
> + bt_cb(skb)->pkt_type = HCI_LPM_PKT;
> +
> + set_bit(STATE_LPM_TRANSACTION, &intel->flags);
> +
> + skb_queue_tail(&intel->txq, skb);
> + hci_uart_tx_wakeup(hu);
> +
> + intel_wait_lpm_transaction(hu);
is it okay to not check the return value here?
> +
> + clear_bit(STATE_LPM_TRANSACTION, &intel->flags);
> +
> + if (!test_bit(STATE_SUSPENDED, &intel->flags)) {
> + bt_dev_err(hu->hdev, "Device suspend error");
> + return -EINVAL;
> + }
> +
> + bt_dev_dbg(hu->hdev, "Suspended");
> +
> + hci_uart_set_flow_control(hu, true);
> +
> + return 0;
> +}
> +
> +static int intel_lpm_resume(struct hci_uart *hu)
> +{
> + struct intel_data *intel = hu->priv;
> + struct sk_buff *skb;
> +
> + if (!test_bit(STATE_LPM_ENABLED, &intel->flags) ||
> + !test_bit(STATE_SUSPENDED, &intel->flags))
> + return 0;
> +
> + bt_dev_dbg(hu->hdev, "Resuming");
> +
> + hci_uart_set_flow_control(hu, false);
> +
> + skb = bt_skb_alloc(0, GFP_KERNEL);
> + if (!skb) {
> + bt_dev_err(hu->hdev, "Failed to alloc memory for LPM packet");
> + return -ENOMEM;
> + }
> +
> + bt_cb(skb)->pkt_type = HCI_LPM_WAKE_PKT;
> +
> + set_bit(STATE_LPM_TRANSACTION, &intel->flags);
> +
> + skb_queue_tail(&intel->txq, skb);
> + hci_uart_tx_wakeup(hu);
> +
> + intel_wait_lpm_transaction(hu);
Same question as above. If this is fine for now, then maybe a quick comment that we continue even in case of failure would be a good idea.
> +
> + clear_bit(STATE_LPM_TRANSACTION, &intel->flags);
> +
> + if (test_bit(STATE_SUSPENDED, &intel->flags)) {
> + bt_dev_err(hu->hdev, "Device resume error");
> + return -EINVAL;
> + }
> +
> + bt_dev_dbg(hu->hdev, "Resumed");
> +
> + return 0;
> +}
> +
> +static int intel_lpm_host_wake(struct hci_uart *hu)
> +{
> + static const u8 lpm_resume_ack[] = { LPM_OP_RESUME_ACK, 0x00 };
> + struct intel_data *intel = hu->priv;
> + struct sk_buff *skb;
> +
> + hci_uart_set_flow_control(hu, false);
> +
> + clear_bit(STATE_SUSPENDED, &intel->flags);
> +
> + skb = bt_skb_alloc(sizeof(lpm_resume_ack), GFP_KERNEL);
> + if (!skb) {
> + bt_dev_err(hu->hdev, "Failed to alloc memory for LPM packet");
> + return -ENOMEM;
> + }
> +
> + memcpy(skb_put(skb, sizeof(lpm_resume_ack)), lpm_resume_ack,
> + sizeof(lpm_resume_ack));
> + bt_cb(skb)->pkt_type = HCI_LPM_PKT;
> +
> + skb_queue_tail(&intel->txq, skb);
> + hci_uart_tx_wakeup(hu);
> +
> + bt_dev_dbg(hu->hdev, "Resumed by controller");
> +
> + return 0;
> +}
> +
> static irqreturn_t intel_irq(int irq, void *dev_id)
> {
> struct intel_device *idev = dev_id;
> @@ -800,12 +940,28 @@ static void intel_recv_lpm_notify(struct hci_dev *hdev, int value)
> static int intel_recv_lpm(struct hci_dev *hdev, struct sk_buff *skb)
> {
> struct hci_lpm_pkt *lpm = (void *)skb->data;
> + struct hci_uart *hu = hci_get_drvdata(hdev);
> + struct intel_data *intel = hu->priv;
>
> switch (lpm->opcode) {
> case LPM_OP_TX_NOTIFY:
> if (lpm->dlen)
> intel_recv_lpm_notify(hdev, lpm->data[0]);
> break;
I forgot this earlier :(
case LPM_OP_TX_NOTIFY:
if (lpm->dlen < 1) {
bt_dev_err(hu->hdev, "Invalid LPM notification packet");
break;
}
intel_recv_lpm_notify(hdev, lpm->data[0]);
break;
Does it make sense to change it to this (obviously in a separate patch).
> + case LPM_OP_SUSPEND_ACK:
> + set_bit(STATE_SUSPENDED, &intel->flags);
> + if (test_and_clear_bit(STATE_LPM_TRANSACTION, &intel->flags)) {
> + smp_mb__after_atomic();
> + wake_up_bit(&intel->flags, STATE_LPM_TRANSACTION);
> + }
> + break;
> + case LPM_OP_RESUME_ACK:
> + clear_bit(STATE_SUSPENDED, &intel->flags);
> + if (test_and_clear_bit(STATE_LPM_TRANSACTION, &intel->flags)) {
> + smp_mb__after_atomic();
> + wake_up_bit(&intel->flags, STATE_LPM_TRANSACTION);
> + }
> + break;
> default:
> bt_dev_err(hdev, "Unknown LPM opcode (%02x)", lpm->opcode);
> break;
Regards
Marcel
Implement runtime PM suspend/resume callbacks.
If LPM supported, controller is put into supsend after a delay of
inactivity (1s). Inactivity is based on LPM idle notification and
host TX traffic.
Signed-off-by: Loic Poulain <[email protected]>
---
v2: Add pm_runtime comments.
Use same suspend/resume callbacks for pm and runtime pm.
drivers/bluetooth/hci_intel.c | 72 +++++++++++++++++++++++++++++++++++++++++--
1 file changed, 69 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index 5143554..8faf765 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -32,6 +32,7 @@
#include <linux/gpio/consumer.h>
#include <linux/acpi.h>
#include <linux/interrupt.h>
+#include <linux/pm_runtime.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -58,6 +59,8 @@
#define LPM_OP_SUSPEND_ACK 0x02
#define LPM_OP_RESUME_ACK 0x03
+#define LPM_SUSPEND_DELAY_MS 1000
+
struct hci_lpm_pkt {
__u8 opcode;
__u8 dlen;
@@ -79,6 +82,8 @@ static DEFINE_MUTEX(intel_device_list_lock);
struct intel_data {
struct sk_buff *rx_skb;
struct sk_buff_head txq;
+ struct work_struct busy_work;
+ struct hci_uart *hu;
unsigned long flags;
};
@@ -282,6 +287,11 @@ static irqreturn_t intel_irq(int irq, void *dev_id)
intel_lpm_host_wake(idev->hu);
mutex_unlock(&idev->hu_lock);
+ /* Host/Controller are now LPM resumed, trigger a new delayed suspend */
+ pm_runtime_get(&idev->pdev->dev);
+ pm_runtime_mark_last_busy(&idev->pdev->dev);
+ pm_runtime_put_autosuspend(&idev->pdev->dev);
+
return IRQ_HANDLED;
}
@@ -337,9 +347,17 @@ static int intel_set_power(struct hci_uart *hu, bool powered)
}
device_wakeup_enable(&idev->pdev->dev);
+
+ pm_runtime_set_active(&idev->pdev->dev);
+ pm_runtime_use_autosuspend(&idev->pdev->dev);
+ pm_runtime_set_autosuspend_delay(&idev->pdev->dev,
+ LPM_SUSPEND_DELAY_MS);
+ pm_runtime_enable(&idev->pdev->dev);
} else if (!powered && device_may_wakeup(&idev->pdev->dev)) {
devm_free_irq(&idev->pdev->dev, idev->irq, idev);
device_wakeup_disable(&idev->pdev->dev);
+
+ pm_runtime_disable(&idev->pdev->dev);
}
}
@@ -348,6 +366,28 @@ static int intel_set_power(struct hci_uart *hu, bool powered)
return err;
}
+static void intel_busy_work(struct work_struct *work)
+{
+ struct list_head *p;
+ struct intel_data *intel = container_of(work, struct intel_data,
+ busy_work);
+
+ /* Link is busy, delay the suspend */
+ mutex_lock(&intel_device_list_lock);
+ list_for_each(p, &intel_device_list) {
+ struct intel_device *idev = list_entry(p, struct intel_device,
+ list);
+
+ if (intel->hu->tty->dev->parent == idev->pdev->dev.parent) {
+ pm_runtime_get(&idev->pdev->dev);
+ pm_runtime_mark_last_busy(&idev->pdev->dev);
+ pm_runtime_put_autosuspend(&idev->pdev->dev);
+ break;
+ }
+ }
+ mutex_unlock(&intel_device_list_lock);
+}
+
static int intel_open(struct hci_uart *hu)
{
struct intel_data *intel;
@@ -359,6 +399,9 @@ static int intel_open(struct hci_uart *hu)
return -ENOMEM;
skb_queue_head_init(&intel->txq);
+ INIT_WORK(&intel->busy_work, intel_busy_work);
+
+ intel->hu = hu;
hu->priv = intel;
@@ -376,6 +419,8 @@ static int intel_close(struct hci_uart *hu)
intel_set_power(hu, false);
+ cancel_work_sync(&intel->busy_work);
+
skb_queue_purge(&intel->txq);
kfree_skb(intel->rx_skb);
kfree(intel);
@@ -947,10 +992,12 @@ static void intel_recv_lpm_notify(struct hci_dev *hdev, int value)
bt_dev_dbg(hdev, "TX idle notification (%d)", value);
- if (value)
+ if (value) {
set_bit(STATE_TX_ACTIVE, &intel->flags);
- else
+ schedule_work(&intel->busy_work);
+ } else {
clear_bit(STATE_TX_ACTIVE, &intel->flags);
+ }
}
static int intel_recv_lpm(struct hci_dev *hdev, struct sk_buff *skb)
@@ -1025,9 +1072,27 @@ static int intel_recv(struct hci_uart *hu, const void *data, int count)
static int intel_enqueue(struct hci_uart *hu, struct sk_buff *skb)
{
struct intel_data *intel = hu->priv;
+ struct list_head *p;
BT_DBG("hu %p skb %p", hu, skb);
+ /* Be sure our controller is resumed and potential LPM transaction
+ * completed before enqueuing any packet.
+ */
+ mutex_lock(&intel_device_list_lock);
+ list_for_each(p, &intel_device_list) {
+ struct intel_device *idev = list_entry(p, struct intel_device,
+ list);
+
+ if (hu->tty->dev->parent == idev->pdev->dev.parent) {
+ pm_runtime_get_sync(&idev->pdev->dev);
+ pm_runtime_mark_last_busy(&idev->pdev->dev);
+ pm_runtime_put_autosuspend(&idev->pdev->dev);
+ break;
+ }
+ }
+ mutex_unlock(&intel_device_list_lock);
+
skb_queue_tail(&intel->txq, skb);
return 0;
@@ -1101,7 +1166,7 @@ static int intel_acpi_probe(struct intel_device *idev)
}
#endif
-#ifdef CONFIG_PM_SLEEP
+#ifdef CONFIG_PM
static int intel_suspend(struct device *dev)
{
struct intel_device *idev = dev_get_drvdata(dev);
@@ -1133,6 +1198,7 @@ static int intel_resume(struct device *dev)
static const struct dev_pm_ops intel_pm_ops = {
SET_SYSTEM_SLEEP_PM_OPS(intel_suspend, intel_resume)
+ SET_RUNTIME_PM_OPS(intel_suspend, intel_resume, NULL)
};
static int intel_probe(struct platform_device *pdev)
--
1.9.1
Add PM suspend/resume callbacks which call lpm_suspend/resume.
Add LPM ack in threaded IRQ handler to notify the controller that
resume is complete.
Protect hci_uart against concurrent removing during suspend/resume.
Signed-off-by: Loic Poulain <[email protected]>
---
v2: Rebase on lpm functions reorder.
Add idev->hu comment.
drivers/bluetooth/hci_intel.c | 53 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 53 insertions(+)
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index eed1f72..5143554 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -68,6 +68,8 @@ struct intel_device {
struct list_head list;
struct platform_device *pdev;
struct gpio_desc *reset;
+ struct hci_uart *hu;
+ struct mutex hu_lock;
int irq;
};
@@ -275,6 +277,11 @@ static irqreturn_t intel_irq(int irq, void *dev_id)
dev_info(&idev->pdev->dev, "hci_intel irq\n");
+ mutex_lock(&idev->hu_lock);
+ if (idev->hu)
+ intel_lpm_host_wake(idev->hu);
+ mutex_unlock(&idev->hu_lock);
+
return IRQ_HANDLED;
}
@@ -305,6 +312,15 @@ static int intel_set_power(struct hci_uart *hu, bool powered)
gpiod_set_value(idev->reset, powered);
+ /* Provide to idev a hu reference which is used to run LPM
+ * transactions (lpm suspend/resume) from PM callbacks.
+ * hu needs to be protected against concurrent removing during
+ * these PM ops.
+ */
+ mutex_lock(&idev->hu_lock);
+ idev->hu = powered ? hu : NULL;
+ mutex_unlock(&idev->hu_lock);
+
if (idev->irq < 0)
break;
@@ -1085,6 +1101,40 @@ static int intel_acpi_probe(struct intel_device *idev)
}
#endif
+#ifdef CONFIG_PM_SLEEP
+static int intel_suspend(struct device *dev)
+{
+ struct intel_device *idev = dev_get_drvdata(dev);
+
+ dev_dbg(dev, "intel_suspend");
+
+ mutex_lock(&idev->hu_lock);
+ if (idev->hu)
+ intel_lpm_suspend(idev->hu);
+ mutex_unlock(&idev->hu_lock);
+
+ return 0;
+}
+
+static int intel_resume(struct device *dev)
+{
+ struct intel_device *idev = dev_get_drvdata(dev);
+
+ dev_dbg(dev, "intel_resume");
+
+ mutex_lock(&idev->hu_lock);
+ if (idev->hu)
+ intel_lpm_resume(idev->hu);
+ mutex_unlock(&idev->hu_lock);
+
+ return 0;
+}
+#endif
+
+static const struct dev_pm_ops intel_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(intel_suspend, intel_resume)
+};
+
static int intel_probe(struct platform_device *pdev)
{
struct intel_device *idev;
@@ -1093,6 +1143,8 @@ static int intel_probe(struct platform_device *pdev)
if (!idev)
return -ENOMEM;
+ mutex_init(&idev->hu_lock);
+
idev->pdev = pdev;
if (ACPI_HANDLE(&pdev->dev)) {
@@ -1169,6 +1221,7 @@ static struct platform_driver intel_driver = {
.driver = {
.name = "hci_intel",
.acpi_match_table = ACPI_PTR(intel_acpi_match),
+ .pm = &intel_pm_ops,
},
};
--
1.9.1