2015-08-27 05:21:51

by Marcel Holtmann

[permalink] [raw]
Subject: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

From: Loic Poulain <[email protected]>

A platform device can be used to provide some specific resources in
order to manage the controller. In this first patch we retrieve the
reset gpio which is used to power on/off the controller.

The main issue is to match the current tty with the correct pdev.
In case of ACPI, we can easily find the right tty/pdev pair because
they are both child of the same UART port.

If controller is powered-on from the driver, we need to wait for a
HCI boot event before being able to send any command.

Signed-off-by: Loic Poulain <[email protected]>
Signed-off-by: Marcel Holtmann <[email protected]>
---
drivers/bluetooth/hci_intel.c | 203 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 190 insertions(+), 13 deletions(-)

diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index da3192aee5e1..46426ff1c7c1 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -25,7 +25,12 @@
#include <linux/errno.h>
#include <linux/skbuff.h>
#include <linux/firmware.h>
+#include <linux/module.h>
#include <linux/wait.h>
+#include <linux/tty.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/acpi.h>

#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -39,6 +44,15 @@
#define STATE_FIRMWARE_FAILED 3
#define STATE_BOOTING 4

+struct intel_device {
+ struct list_head list;
+ struct platform_device *pdev;
+ struct gpio_desc *reset;
+};
+
+static LIST_HEAD(intel_device_list);
+static DEFINE_SPINLOCK(intel_device_list_lock);
+
struct intel_data {
struct sk_buff *rx_skb;
struct sk_buff_head txq;
@@ -77,6 +91,61 @@ static u8 intel_convert_speed(unsigned int speed)
}
}

+static int intel_wait_booting(struct hci_uart *hu)
+{
+ struct intel_data *intel = hu->priv;
+ int err;
+
+ err = wait_on_bit_timeout(&intel->flags, STATE_BOOTING,
+ TASK_INTERRUPTIBLE,
+ msecs_to_jiffies(1000));
+
+ if (err == 1) {
+ BT_ERR("%s: Device boot interrupted", hu->hdev->name);
+ return -EINTR;
+ }
+
+ if (err) {
+ BT_ERR("%s: Device boot timeout", hu->hdev->name);
+ return -ETIMEDOUT;
+ }
+
+ return err;
+}
+
+static int intel_set_power(struct hci_uart *hu, bool powered)
+{
+ struct list_head *p;
+ int err = -ENODEV;
+
+ spin_lock(&intel_device_list_lock);
+
+ list_for_each(p, &intel_device_list) {
+ struct intel_device *idev = list_entry(p, struct intel_device,
+ list);
+
+ /* tty device and pdev device should share the same parent
+ * which is the UART port.
+ */
+ if (hu->tty->dev->parent != idev->pdev->dev.parent)
+ continue;
+
+ if (!idev->reset) {
+ err = -ENOTSUPP;
+ break;
+ }
+
+ BT_INFO("hu %p, Switching compatible pm device (%s) to %u",
+ hu, dev_name(&idev->pdev->dev), powered);
+
+ gpiod_set_value(idev->reset, powered);
+ }
+
+ spin_unlock(&intel_device_list_lock);
+
+ return err;
+}
+
static int intel_open(struct hci_uart *hu)
{
struct intel_data *intel;
@@ -90,6 +159,10 @@ static int intel_open(struct hci_uart *hu)
skb_queue_head_init(&intel->txq);

hu->priv = intel;
+
+ if (!intel_set_power(hu, true))
+ set_bit(STATE_BOOTING, &intel->flags);
+
return 0;
}

@@ -99,6 +172,8 @@ static int intel_close(struct hci_uart *hu)

BT_DBG("hu %p", hu);

+ intel_set_power(hu, false);
+
skb_queue_purge(&intel->txq);
kfree_skb(intel->rx_skb);
kfree(intel);
@@ -149,6 +224,18 @@ static int intel_set_baudrate(struct hci_uart *hu, unsigned int speed)
struct hci_dev *hdev = hu->hdev;
u8 speed_cmd[] = { 0x06, 0xfc, 0x01, 0x00 };
struct sk_buff *skb;
+ int err;
+
+ /* This can be the first command sent to the chip, check
+ * that the controller is ready.
+ */
+ err = intel_wait_booting(hu);
+
+ clear_bit(STATE_BOOTING, &intel->flags);
+
+ /* In case of timeout, try to continue anyway */
+ if (err && err != ETIMEDOUT)
+ return err;

BT_INFO("%s: Change controller speed to %d", hdev->name, speed);

@@ -231,6 +318,15 @@ static int intel_setup(struct hci_uart *hu)
if (oper_speed && init_speed && oper_speed != init_speed)
speed_change = 1;

+ /* Check that the controller is ready */
+ err = intel_wait_booting(hu);
+
+ clear_bit(STATE_BOOTING, &intel->flags);
+
+ /* In case of timeout, try to continue anyway */
+ if (err && err != ETIMEDOUT)
+ return err;
+
set_bit(STATE_BOOTLOADER, &intel->flags);

/* Read the Intel version information to determine if the device
@@ -540,19 +636,11 @@ done:
*/
BT_INFO("%s: Waiting for device to boot", hdev->name);

- err = wait_on_bit_timeout(&intel->flags, STATE_BOOTING,
- TASK_INTERRUPTIBLE,
- msecs_to_jiffies(1000));
-
- if (err == 1) {
- BT_ERR("%s: Device boot interrupted", hdev->name);
- return -EINTR;
- }
+ err = intel_wait_booting(hu);
+ if (err)
+ return err;

- if (err) {
- BT_ERR("%s: Device boot timeout", hdev->name);
- return -ETIMEDOUT;
- }
+ clear_bit(STATE_BOOTING, &intel->flags);

rettime = ktime_get();
delta = ktime_sub(rettime, calltime);
@@ -584,7 +672,8 @@ static int intel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
struct intel_data *intel = hu->priv;
struct hci_event_hdr *hdr;

- if (!test_bit(STATE_BOOTLOADER, &intel->flags))
+ if (!test_bit(STATE_BOOTLOADER, &intel->flags) &&
+ !test_bit(STATE_BOOTING, &intel->flags))
goto recv;

hdr = (void *)skb->data;
@@ -700,12 +789,100 @@ static const struct hci_uart_proto intel_proto = {
.dequeue = intel_dequeue,
};

+#ifdef CONFIG_ACPI
+static const struct acpi_device_id intel_acpi_match[] = {
+ { "INT33E1", 0 },
+ { },
+};
+MODULE_DEVICE_TABLE(acpi, intel_acpi_match);
+
+static int intel_acpi_probe(struct intel_device *idev)
+{
+ const struct acpi_device_id *id;
+
+ id = acpi_match_device(intel_acpi_match, &idev->pdev->dev);
+ if (!id)
+ return -ENODEV;
+
+ return 0;
+}
+#else
+static int intel_acpi_probe(struct intel_device *idev)
+{
+ return -ENODEV;
+}
+#endif
+
+static int intel_probe(struct platform_device *pdev)
+{
+ struct intel_device *idev;
+
+ idev = devm_kzalloc(&pdev->dev, sizeof(*idev), GFP_KERNEL);
+ if (!idev)
+ return -ENOMEM;
+
+ idev->pdev = pdev;
+
+ if (ACPI_HANDLE(&pdev->dev)) {
+ int err = intel_acpi_probe(idev);
+ if (err)
+ return err;
+ } else {
+ return -ENODEV;
+ }
+
+ idev->reset = devm_gpiod_get_optional(&pdev->dev, "reset",
+ GPIOD_OUT_LOW);
+ if (IS_ERR(idev->reset)) {
+ dev_err(&pdev->dev, "Unable to retrieve gpio\n");
+ return PTR_ERR(idev->reset);
+ }
+
+ platform_set_drvdata(pdev, idev);
+
+ /* Place this instance on the device list */
+ spin_lock(&intel_device_list_lock);
+ list_add_tail(&idev->list, &intel_device_list);
+ spin_unlock(&intel_device_list_lock);
+
+ dev_info(&pdev->dev, "registered.\n");
+
+ return 0;
+}
+
+static int intel_remove(struct platform_device *pdev)
+{
+ struct intel_device *idev = platform_get_drvdata(pdev);
+
+ spin_lock(&intel_device_list_lock);
+ list_del(&idev->list);
+ spin_unlock(&intel_device_list_lock);
+
+ dev_info(&pdev->dev, "unregistered.\n");
+
+ return 0;
+}
+
+static struct platform_driver intel_driver = {
+ .probe = intel_probe,
+ .remove = intel_remove,
+ .driver = {
+ .name = "hci_intel",
+ .acpi_match_table = ACPI_PTR(intel_acpi_match),
+ .owner = THIS_MODULE,
+ },
+};
+
int __init intel_init(void)
{
+ platform_driver_register(&intel_driver);
+
return hci_uart_register_proto(&intel_proto);
}

int __exit intel_deinit(void)
{
+ platform_driver_unregister(&intel_driver);
+
return hci_uart_unregister_proto(&intel_proto);
}
--
2.4.3



2015-08-27 19:14:39

by Ilya Faenson

[permalink] [raw]
Subject: RE: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Loic,

-----Original Message-----
From: Loic Poulain [mailto:[email protected]]=20
Sent: Thursday, August 27, 2015 2:59 PM
To: Ilya Faenson; Marcel Holtmann
Cc: [email protected]
Subject: Re: [PATCH v2] Bluetooth: hci_intel: Add support for platform driv=
er

Hi Ilya,

> IF: Appreciate the clarifications. Last time I looked, Loic's code has su=
pported both ACPI and DT though. So just the ACPI part has been applied to =
the bluetooth-next?

Yes, I removed the DT part since our last thread (I keep it locally for=20
now).

IF: Oh, clear enough, too bad. I hoped you would be the first. :-)

--=20
Intel Open Source Technology Center
http://oss.intel.com/

2015-08-27 18:59:12

by Loic Poulain

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Ilya,

> IF: Appreciate the clarifications. Last time I looked, Loic's code has supported both ACPI and DT though. So just the ACPI part has been applied to the bluetooth-next?

Yes, I removed the DT part since our last thread (I keep it locally for
now).

--
Intel Open Source Technology Center
http://oss.intel.com/

2015-08-27 18:56:46

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Ilya,

>>>> A platform device can be used to provide some specific resources in
>>>> order to manage the controller. In this first patch we retrieve the
>>>> reset gpio which is used to power on/off the controller.
>>>>
>>>> The main issue is to match the current tty with the correct pdev.
>>>> In case of ACPI, we can easily find the right tty/pdev pair because
>>>> they are both child of the same UART port.
>>>>
>>>> If controller is powered-on from the driver, we need to wait for a
>>>> HCI boot event before being able to send any command.
>>>>
>>>> Signed-off-by: Loic Poulain <[email protected]>
>>>> Signed-off-by: Marcel Holtmann <[email protected]>
>>>> ---
>>>> drivers/bluetooth/hci_intel.c | 203 +++++++++++++++++++++++++++++++++++++++---
>>>> 1 file changed, 190 insertions(+), 13 deletions(-)
>>>
>>> ACK, It Works on my hardware.
>>
>> okay then, this patch has been applied to bluetooth-next now.
>>
>> IF: Just out of curiosity... How come you've accepted the Intel platform device implementation yourself while you've previously referred the similar Broadcom implementation to the Device Tree folks?
>
> these are ACPI devices where we have a clear tree of parent devices. I accepted also the Broadcom version for ACPI. Within ACPI the relationship between TTY and its GPIO etc. is expressed.
>
> That is the part that is missing within DT. And the DT folks should be signing off on that. In that case DT is defining the standard on how to express these relations. For ACPI based devices this is already expressed in shipping devices.
>
> IF: Appreciate the clarifications. Last time I looked, Loic's code has supported both ACPI and DT though. So just the ACPI part has been applied to the bluetooth-next?

so far I only applied ACPI module matching tables. I have not seen any DT ones.

Regards

Marcel


2015-08-27 18:35:07

by Ilya Faenson

[permalink] [raw]
Subject: RE: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Marcel,

-----Original Message-----
From: Marcel Holtmann [mailto:[email protected]]
Sent: Thursday, August 27, 2015 2:28 PM
To: Ilya Faenson
Cc: Loic Poulain; [email protected]
Subject: Re: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Ilya,

>>> A platform device can be used to provide some specific resources in
>>> order to manage the controller. In this first patch we retrieve the
>>> reset gpio which is used to power on/off the controller.
>>>
>>> The main issue is to match the current tty with the correct pdev.
>>> In case of ACPI, we can easily find the right tty/pdev pair because
>>> they are both child of the same UART port.
>>>
>>> If controller is powered-on from the driver, we need to wait for a
>>> HCI boot event before being able to send any command.
>>>
>>> Signed-off-by: Loic Poulain <[email protected]>
>>> Signed-off-by: Marcel Holtmann <[email protected]>
>>> ---
>>> drivers/bluetooth/hci_intel.c | 203 +++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 190 insertions(+), 13 deletions(-)
>>
>> ACK, It Works on my hardware.
>
> okay then, this patch has been applied to bluetooth-next now.
>
> IF: Just out of curiosity... How come you've accepted the Intel platform device implementation yourself while you've previously referred the similar Broadcom implementation to the Device Tree folks?

these are ACPI devices where we have a clear tree of parent devices. I accepted also the Broadcom version for ACPI. Within ACPI the relationship between TTY and its GPIO etc. is expressed.

That is the part that is missing within DT. And the DT folks should be signing off on that. In that case DT is defining the standard on how to express these relations. For ACPI based devices this is already expressed in shipping devices.

IF: Appreciate the clarifications. Last time I looked, Loic's code has supported both ACPI and DT though. So just the ACPI part has been applied to the bluetooth-next?

Regards

Marcel


2015-08-27 18:28:05

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Ilya,

>>> A platform device can be used to provide some specific resources in
>>> order to manage the controller. In this first patch we retrieve the
>>> reset gpio which is used to power on/off the controller.
>>>
>>> The main issue is to match the current tty with the correct pdev.
>>> In case of ACPI, we can easily find the right tty/pdev pair because
>>> they are both child of the same UART port.
>>>
>>> If controller is powered-on from the driver, we need to wait for a
>>> HCI boot event before being able to send any command.
>>>
>>> Signed-off-by: Loic Poulain <[email protected]>
>>> Signed-off-by: Marcel Holtmann <[email protected]>
>>> ---
>>> drivers/bluetooth/hci_intel.c | 203 +++++++++++++++++++++++++++++++++++++++---
>>> 1 file changed, 190 insertions(+), 13 deletions(-)
>>
>> ACK, It Works on my hardware.
>
> okay then, this patch has been applied to bluetooth-next now.
>
> IF: Just out of curiosity... How come you've accepted the Intel platform device implementation yourself while you've previously referred the similar Broadcom implementation to the Device Tree folks?

these are ACPI devices where we have a clear tree of parent devices. I accepted also the Broadcom version for ACPI. Within ACPI the relationship between TTY and its GPIO etc. is expressed.

That is the part that is missing within DT. And the DT folks should be signing off on that. In that case DT is defining the standard on how to express these relations. For ACPI based devices this is already expressed in shipping devices.

Regards

Marcel


2015-08-27 18:09:54

by Ilya Faenson

[permalink] [raw]
Subject: RE: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Marcel,

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Marcel Holtmann
Sent: Thursday, August 27, 2015 11:20 AM
To: Loic Poulain
Cc: [email protected]
Subject: Re: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Loic,

>> A platform device can be used to provide some specific resources in
>> order to manage the controller. In this first patch we retrieve the
>> reset gpio which is used to power on/off the controller.
>>
>> The main issue is to match the current tty with the correct pdev.
>> In case of ACPI, we can easily find the right tty/pdev pair because
>> they are both child of the same UART port.
>>
>> If controller is powered-on from the driver, we need to wait for a
>> HCI boot event before being able to send any command.
>>
>> Signed-off-by: Loic Poulain <[email protected]>
>> Signed-off-by: Marcel Holtmann <[email protected]>
>> ---
>> drivers/bluetooth/hci_intel.c | 203 +++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 190 insertions(+), 13 deletions(-)
>
> ACK, It Works on my hardware.

okay then, this patch has been applied to bluetooth-next now.

IF: Just out of curiosity... How come you've accepted the Intel platform device implementation yourself while you've previously referred the similar Broadcom implementation to the Device Tree folks?

Regards

Marcel

2015-08-27 15:19:57

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Loic,

>> A platform device can be used to provide some specific resources in
>> order to manage the controller. In this first patch we retrieve the
>> reset gpio which is used to power on/off the controller.
>>
>> The main issue is to match the current tty with the correct pdev.
>> In case of ACPI, we can easily find the right tty/pdev pair because
>> they are both child of the same UART port.
>>
>> If controller is powered-on from the driver, we need to wait for a
>> HCI boot event before being able to send any command.
>>
>> Signed-off-by: Loic Poulain <[email protected]>
>> Signed-off-by: Marcel Holtmann <[email protected]>
>> ---
>> drivers/bluetooth/hci_intel.c | 203 +++++++++++++++++++++++++++++++++++++++---
>> 1 file changed, 190 insertions(+), 13 deletions(-)
>
> ACK, It Works on my hardware.

okay then, this patch has been applied to bluetooth-next now.

Regards

Marcel


2015-08-27 08:51:42

by Loic Poulain

[permalink] [raw]
Subject: Re: [PATCH v2] Bluetooth: hci_intel: Add support for platform driver

Hi Marcel,

On 27/08/2015 07:21, Marcel Holtmann wrote:
> From: Loic Poulain <[email protected]>
>
> A platform device can be used to provide some specific resources in
> order to manage the controller. In this first patch we retrieve the
> reset gpio which is used to power on/off the controller.
>
> The main issue is to match the current tty with the correct pdev.
> In case of ACPI, we can easily find the right tty/pdev pair because
> they are both child of the same UART port.
>
> If controller is powered-on from the driver, we need to wait for a
> HCI boot event before being able to send any command.
>
> Signed-off-by: Loic Poulain <[email protected]>
> Signed-off-by: Marcel Holtmann <[email protected]>
> ---
> drivers/bluetooth/hci_intel.c | 203 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 190 insertions(+), 13 deletions(-)

ACK, It Works on my hardware.

Regards,
Loic
--
Intel Open Source Technology Center
http://oss.intel.com/