From 61c59e8b2a7f129a339608610069a26f4181c82a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ronald=20Tschal=C3=A4r?= Date: Sat, 23 Sep 2017 21:00:18 -0700 Subject: [PATCH 1/4] Bluetooth: hci_bcm: Fix logic errors calling apple acpi methods. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The methods are supposed to return 0 on success, and the flag to enable low-power operation was inverted. Signed-off-by: Ronald Tschalär --- drivers/bluetooth/hci_bcm.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c index 205e29596fdea0..5ef0f543d2eb2e 100644 --- a/drivers/bluetooth/hci_bcm.c +++ b/drivers/bluetooth/hci_bcm.c @@ -86,14 +86,14 @@ static LIST_HEAD(bcm_device_list); #ifdef CONFIG_ACPI static int bcm_apple_set_power(struct bcm_device *dev, bool enable) { - return ACPI_SUCCESS(acpi_evaluate_object(enable ? dev->btpu : dev->btpd, + return ACPI_FAILURE(acpi_evaluate_object(enable ? dev->btpu : dev->btpd, NULL, NULL, NULL)); } static int bcm_apple_set_device_wake(struct bcm_device *dev, bool enable) { - return ACPI_SUCCESS(acpi_execute_simple_method(dev->btlp, - NULL, enable)); + return ACPI_FAILURE(acpi_execute_simple_method(dev->btlp, + NULL, enable ? 0 : 1)); } static bool bcm_apple_probe(struct bcm_device *dev) From 1a59d8304a84b6c399ba9a2dd1766698f672e49e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ronald=20Tschal=C3=A4r?= Date: Sat, 23 Sep 2017 21:03:01 -0700 Subject: [PATCH 2/4] Bluetooth: hci_bcm: Set the init baudrate too on apple. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The device must be accessed via the configured baudrate for all operations, or the operations will time out. Note that currently bcm_set_baudrate() always fails (the commands always return -EBUSY) and hence the operating baudrate is never actually set. But if it were, it would need to be the same as the init baudrate, so we keep setting oper_speed anyway. Signed-off-by: Ronald Tschalär --- drivers/bluetooth/hci_bcm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c index 5ef0f543d2eb2e..84d75ee79e0bab 100644 --- a/drivers/bluetooth/hci_bcm.c +++ b/drivers/bluetooth/hci_bcm.c @@ -104,7 +104,8 @@ static bool bcm_apple_probe(struct bcm_device *dev) if (!acpi_dev_get_property(adev, "baud", ACPI_TYPE_BUFFER, &obj) && obj->buffer.length == 8) { - dev->oper_speed = *(u64 *)obj->buffer.pointer; + dev->init_speed = *(u64 *)obj->buffer.pointer; + dev->oper_speed = dev->init_speed; dev_info(&dev->pdev->dev, "oper_speed=%u\n", dev->oper_speed); } From 9fc6001d02b3e858f72c1a16109a3ffb1ee7f986 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ronald=20Tschal=C3=A4r?= Date: Sat, 23 Sep 2017 21:39:56 -0700 Subject: [PATCH 3/4] Bluetooth: hci_bcm: Handle intel-lpss based UART's too. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the UART is part of an intel-lpss based mfd, the resulting device tree has an extra level. Specifically we have the following layout (with the /sys/devices/ prefix removed for brevity): ACPI device node: LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:94/BCM2E7C:00 Associated physical device node: LNXSYSTM:00/LNXSYBUS:00/PNP0A08:00/device:94/BCM2E7C:00/physical_node -> pci0000:00/0000:00:1e.0/BCM2E7C:00/ UART device node: pci0000:00/0000:00:1e.0/dw-apb-uart.2 UART TTY device node: pci0000:00/0000:00:1e.0/dw-apb-uart.2/tty This driver was assuming that physical device's parent is the UART, but in this case the parent is the mfd device. We therefore check for both cases. Note that because this assumption appears in some user-space tools too, it might be better to put the physical device node under the UART instead of the mfd. Signed-off-by: Ronald Tschalär --- drivers/bluetooth/hci_bcm.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c index 84d75ee79e0bab..77ea89421611bc 100644 --- a/drivers/bluetooth/hci_bcm.c +++ b/drivers/bluetooth/hci_bcm.c @@ -359,7 +359,8 @@ static int bcm_open(struct hci_uart *hu) * platform device (saved during device probe) and * parent of tty device used by hci_uart */ - if (hu->tty->dev->parent == dev->pdev->dev.parent) { + if (hu->tty->dev->parent == dev->pdev->dev.parent || + hu->tty->dev->parent->parent == dev->pdev->dev.parent) { bcm->dev = dev; hu->init_speed = dev->init_speed; hu->oper_speed = dev->oper_speed; From 7dd4a90a92b97b017d9210899afcfbe985b10509 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ronald=20Tschal=C3=A4r?= Date: Sat, 23 Sep 2017 23:17:57 -0700 Subject: [PATCH 4/4] Bluetooth: hci_ldisc: Allow sleeping while proto locks are held. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit dec2c92880cc5435381d50e3045ef018a762a917 introduced locks around the proto functions, using rwlock's, which do not allow sleeping while the locks are held. However, the proto functions in hci_bcm use mutexes and hence need to be able to sleep. Therefore this replaces the rwlock's with rw_semaphore's. Because the writes are very rare compared to the reads the percpu variant is used. Lastly, the locks in the tx_wakeup callback needed to be removed because that is called from an IRQ context. But since it doesn't actually call any proto functions, instead just queueing work, and the HCI_UART_PROTO_READY flag is checked (again) in the worker, this doesn't cause any problems. Signed-off-by: Ronald Tschalär --- drivers/bluetooth/hci_ldisc.c | 31 +++++++++++++------------------ drivers/bluetooth/hci_uart.h | 2 +- 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/drivers/bluetooth/hci_ldisc.c b/drivers/bluetooth/hci_ldisc.c index 8397b716fa654e..76b6876f2762b0 100644 --- a/drivers/bluetooth/hci_ldisc.c +++ b/drivers/bluetooth/hci_ldisc.c @@ -114,12 +114,12 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu) struct sk_buff *skb = hu->tx_skb; if (!skb) { - read_lock(&hu->proto_lock); + percpu_down_read(&hu->proto_lock); if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) skb = hu->proto->dequeue(hu); - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); } else { hu->tx_skb = NULL; } @@ -129,8 +129,6 @@ static inline struct sk_buff *hci_uart_dequeue(struct hci_uart *hu) int hci_uart_tx_wakeup(struct hci_uart *hu) { - read_lock(&hu->proto_lock); - if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) goto no_schedule; @@ -144,8 +142,6 @@ int hci_uart_tx_wakeup(struct hci_uart *hu) schedule_work(&hu->write_work); no_schedule: - read_unlock(&hu->proto_lock); - return 0; } EXPORT_SYMBOL_GPL(hci_uart_tx_wakeup); @@ -246,12 +242,12 @@ static int hci_uart_flush(struct hci_dev *hdev) tty_ldisc_flush(tty); tty_driver_flush_buffer(tty); - read_lock(&hu->proto_lock); + percpu_down_read(&hu->proto_lock); if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) hu->proto->flush(hu); - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); return 0; } @@ -274,15 +270,15 @@ static int hci_uart_send_frame(struct hci_dev *hdev, struct sk_buff *skb) BT_DBG("%s: type %d len %d", hdev->name, hci_skb_pkt_type(skb), skb->len); - read_lock(&hu->proto_lock); + percpu_down_read(&hu->proto_lock); if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) { - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); return -EUNATCH; } hu->proto->enqueue(hu, skb); - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); hci_uart_tx_wakeup(hu); @@ -478,7 +474,7 @@ static int hci_uart_tty_open(struct tty_struct *tty) INIT_WORK(&hu->init_ready, hci_uart_init_work); INIT_WORK(&hu->write_work, hci_uart_write_work); - rwlock_init(&hu->proto_lock); + percpu_init_rwsem(&hu->proto_lock); /* Flush any pending characters in the driver */ tty_driver_flush_buffer(tty); @@ -495,7 +491,6 @@ static void hci_uart_tty_close(struct tty_struct *tty) { struct hci_uart *hu = tty->disc_data; struct hci_dev *hdev; - unsigned long flags; BT_DBG("tty %p", tty); @@ -512,9 +507,9 @@ static void hci_uart_tty_close(struct tty_struct *tty) cancel_work_sync(&hu->write_work); if (test_bit(HCI_UART_PROTO_READY, &hu->flags)) { - write_lock_irqsave(&hu->proto_lock, flags); + percpu_down_write(&hu->proto_lock); clear_bit(HCI_UART_PROTO_READY, &hu->flags); - write_unlock_irqrestore(&hu->proto_lock, flags); + percpu_up_write(&hu->proto_lock); if (hdev) { if (test_bit(HCI_UART_REGISTERED, &hu->flags)) @@ -574,10 +569,10 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data, if (!hu || tty != hu->tty) return; - read_lock(&hu->proto_lock); + percpu_down_read(&hu->proto_lock); if (!test_bit(HCI_UART_PROTO_READY, &hu->flags)) { - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); return; } @@ -585,7 +580,7 @@ static void hci_uart_tty_receive(struct tty_struct *tty, const u8 *data, * tty caller */ hu->proto->recv(hu, data, count); - read_unlock(&hu->proto_lock); + percpu_up_read(&hu->proto_lock); if (hu->hdev) hu->hdev->stat.byte_rx += count; diff --git a/drivers/bluetooth/hci_uart.h b/drivers/bluetooth/hci_uart.h index c6e9e1cf63f886..f1374a0ee75c31 100644 --- a/drivers/bluetooth/hci_uart.h +++ b/drivers/bluetooth/hci_uart.h @@ -87,7 +87,7 @@ struct hci_uart { struct work_struct write_work; const struct hci_uart_proto *proto; - rwlock_t proto_lock; /* Stop work for proto close */ + struct percpu_rw_semaphore proto_lock; /* Stop work for proto close */ void *priv; struct sk_buff *tx_skb;