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.
In case of device tree, there is currently nothing to link the pdev
with its tty. So, we just match the first available pdev.
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]>
---
drivers/bluetooth/hci_intel.c | 275 +++++++++++++++++++++++++++++++++++++++---
1 file changed, 255 insertions(+), 20 deletions(-)
diff --git a/drivers/bluetooth/hci_intel.c b/drivers/bluetooth/hci_intel.c
index 275283c..b8e6322 100644
--- a/drivers/bluetooth/hci_intel.c
+++ b/drivers/bluetooth/hci_intel.c
@@ -26,6 +26,11 @@
#include <linux/skbuff.h>
#include <linux/firmware.h>
#include <linux/wait.h>
+#include <linux/tty.h>
+#include <linux/platform_device.h>
+#include <linux/gpio/consumer.h>
+#include <linux/of_platform.h>
+#include <linux/acpi.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -40,10 +45,20 @@
#define STATE_BOOTING 4
#define STATE_SPEED_CHANGING 5
+struct intel_device {
+ struct list_head list;
+ struct platform_device *pdev;
+ struct gpio_desc *reset;
+ struct hci_uart *hu;
+};
+LIST_HEAD(intel_device_list);
+DEFINE_SPINLOCK(intel_device_list_lock);
+
struct intel_data {
struct sk_buff *rx_skb;
struct sk_buff_head txq;
unsigned long flags;
+ struct intel_device *idev;
};
static u8 intel_convert_speed(unsigned int speed)
@@ -78,6 +93,120 @@ static u8 intel_convert_speed(unsigned int speed)
}
}
+static int intel_device_lock(struct intel_device *idev)
+{
+ struct list_head *p;
+
+ spin_lock(&intel_device_list_lock);
+
+ list_for_each(p, &intel_device_list) {
+ struct intel_device *dev = list_entry(p, struct intel_device,
+ list);
+ if (dev == idev)
+ return 0;
+ }
+
+ spin_unlock(&intel_device_list_lock);
+
+ return -ENODEV;
+}
+
+static void intel_device_unlock(struct intel_device *idev)
+{
+ spin_unlock(&intel_device_list_lock);
+}
+
+static int intel_set_power(struct hci_uart *hu, bool powered)
+{
+ struct intel_data *intel = hu->priv;
+ struct intel_device *idev = intel->idev;
+
+ if (intel_device_lock(idev))
+ return -ENODEV;
+
+ if (!idev->reset) {
+ intel_device_unlock(idev);
+ return -EINVAL;
+ }
+
+ BT_DBG("%s: set power %d", hu->hdev->name, powered);
+
+ gpiod_set_value(idev->reset, powered);
+
+ intel_device_unlock(idev);
+
+ return 0;
+}
+
+static struct intel_device *intel_get_idev(struct hci_uart *hu)
+{
+ struct list_head *p;
+ struct intel_device *idev_match = NULL;
+
+ spin_lock(&intel_device_list_lock);
+ list_for_each(p, &intel_device_list) {
+ struct intel_device *idev;
+
+ idev = list_entry(p, struct intel_device, list);
+
+ /* Return the first available pdev in case of device tree.
+ * There is currently nothing to match pdev with tty.
+ */
+ if (idev->pdev->dev.of_node) {
+ if (!idev->hu) {
+ idev_match = idev;
+ break;
+ }
+
+ /* tty device and pdev device should share the same parent
+ * which is the UART port.
+ */
+ } else if (hu->tty->dev->parent == idev->pdev->dev.parent) {
+ idev_match = idev;
+ break;
+ }
+ }
+
+ if (idev_match) {
+ idev_match->hu = hu;
+ BT_INFO("hu %p, Found compatible pm device (%s)", hu,
+ dev_name(&idev_match->pdev->dev));
+ }
+ spin_unlock(&intel_device_list_lock);
+
+ return idev_match;
+}
+
+static int intel_boot_wait(struct hci_uart *hu)
+{
+ struct intel_data *intel = hu->priv;
+ struct hci_dev *hdev = hu->hdev;
+ int err;
+
+ if (!test_bit(STATE_BOOTING, &intel->flags))
+ return 0;
+
+ 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);
+ clear_bit(STATE_BOOTING, &intel->flags);
+ return -EINTR;
+ }
+
+ if (err) {
+ BT_ERR("%s: Device boot timeout", hdev->name);
+ clear_bit(STATE_BOOTING, &intel->flags);
+ return -ETIMEDOUT;
+ }
+
+ return 0;
+}
+
static int intel_open(struct hci_uart *hu)
{
struct intel_data *intel;
@@ -91,6 +220,12 @@ static int intel_open(struct hci_uart *hu)
skb_queue_head_init(&intel->txq);
hu->priv = intel;
+
+ intel->idev = intel_get_idev(hu);
+
+ if (!intel_set_power(hu, true))
+ set_bit(STATE_BOOTING, &intel->flags);
+
return 0;
}
@@ -100,6 +235,13 @@ static int intel_close(struct hci_uart *hu)
BT_DBG("hu %p", hu);
+ intel_set_power(hu, false);
+
+ if (!intel_device_lock(intel->idev)) {
+ intel->idev->hu = NULL;
+ intel_device_unlock(intel->idev);
+ }
+
skb_queue_purge(&intel->txq);
kfree_skb(intel->rx_skb);
kfree(intel);
@@ -151,6 +293,8 @@ static int intel_set_baudrate(struct hci_uart *hu, unsigned int speed)
u8 intel_speed;
struct sk_buff *skb;
+ intel_boot_wait(hu);
+
BT_INFO("%s: Change controller speed to %d", hdev->name, speed);
/* Device will not accept speed change if Intel version has not been
@@ -229,6 +373,8 @@ static int intel_setup(struct hci_uart *hu)
if (oper_speed && init_speed && oper_speed != init_speed)
speed_change = 1;
+ intel_boot_wait(hu);
+
set_bit(STATE_BOOTLOADER, &intel->flags);
/* Read the Intel version information to determine if the device
@@ -511,10 +657,6 @@ done:
if (err < 0)
return err;
- calltime = ktime_get();
-
- set_bit(STATE_BOOTING, &intel->flags);
-
/* We need to restore the default speed before Intel reset */
if (speed_change) {
err = intel_set_baudrate(hu, init_speed);
@@ -523,6 +665,10 @@ done:
hci_uart_set_baudrate(hu, init_speed);
}
+ calltime = ktime_get();
+
+ set_bit(STATE_BOOTING, &intel->flags);
+
skb = __hci_cmd_sync(hdev, 0xfc01, sizeof(reset_param), reset_param,
HCI_INIT_TIMEOUT);
if (IS_ERR(skb))
@@ -537,21 +683,9 @@ done:
* 1 second. However if that happens, then just fail the setup
* since something went wrong.
*/
- 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;
- }
-
- if (err) {
- BT_ERR("%s: Device boot timeout", hdev->name);
- return -ETIMEDOUT;
- }
+ err = intel_boot_wait(hu);
+ if (err)
+ return err;
rettime = ktime_get();
delta = ktime_sub(rettime, calltime);
@@ -583,7 +717,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;
@@ -713,12 +848,112 @@ static const struct hci_uart_proto intel_proto = {
.dequeue = intel_dequeue,
};
+
+/*--- Platform Driver ---*/
+static const struct of_device_id intel_bt_of_match[] = {
+ { .compatible = "intel,bt-fmr",
+ .data = (void *)"intel,bt-fmr-reset", },
+ {},
+};
+
+#ifdef CONFIG_ACPI
+static const struct acpi_device_id intel_bt_acpi_match[] = {
+ { "INT33E1", (kernel_ulong_t)"reset" },
+ { },
+};
+#endif
+
+static int intel_probe(struct platform_device *pdev)
+{
+ struct intel_device *idev;
+ char *gpio_reset_name = NULL;
+
+ idev = devm_kzalloc(&pdev->dev, sizeof(*idev), GFP_KERNEL);
+ if (!idev)
+ return -ENOMEM;
+
+ idev->pdev = pdev;
+
+ if (ACPI_HANDLE(&pdev->dev)) {
+#ifdef CONFIG_ACPI
+ const struct acpi_device_id *id;
+
+ id = acpi_match_device(intel_bt_acpi_match, &pdev->dev);
+ if (!id)
+ return -ENODEV;
+
+ gpio_reset_name = (char *)id->driver_data;
+#endif
+ } else if (&pdev->dev.of_node) {
+ const struct of_device_id *match;
+
+ match = of_match_device(intel_bt_of_match, &pdev->dev);
+ if (!match)
+ return -ENODEV;
+
+ gpio_reset_name = (char *)match->data;
+ } else {
+ return -ENODEV;
+ }
+
+ if (!gpio_reset_name)
+ goto no_reset;
+
+ idev->reset = devm_gpiod_get_optional(&pdev->dev, gpio_reset_name,
+ GPIOD_OUT_LOW);
+ if (IS_ERR(idev->reset)) {
+ dev_err(&pdev->dev, "Unable to retrieve %s gpio\n",
+ gpio_reset_name);
+ return PTR_ERR(idev->reset);
+ }
+
+no_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",
+ .of_match_table = intel_bt_of_match,
+ .acpi_match_table = ACPI_PTR(intel_bt_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);
}
--
1.9.1
On 08/20/2015 02:42 PM, Ilya Faenson wrote:
> Hi Loic,
>
> -----Original Message-----
> From: Loic Poulain [mailto:[email protected]]
> Sent: Thursday, August 20, 2015 4:58 AM
> To: Ilya Faenson; [email protected]
> Cc: [email protected]
> Subject: Re: [PATCH] Bluetooth: hci_intel: Add platform driver
>
> Thanks Ilya,
>
>> Is there a patch with the DT documentation? It would be interesting to see what DT maintainers think of this approach.
> I don't have any documentation for now.
>
>> That allows you to run with a single device per platform only. Meanwhile, you could have something in the DT parameters identifying the UART. You would then be able to retrieve that parameter and match it against the tty in the BT protocol. Multiple devices per platform would then be supported.
> Actually It supports multiple chips but takes them in the registered order.
> But, I agree, it's not a correct way to do that.
> I'm not a DT expert but I think we can do this match in the same way as
> acpi.
> It just requires to make the Bluetooth entry a child of the serial port
> entry.
>
> for example:
>
> In dtsi you could have the usual uart description:
> uart1: serial@1 {
> compatible = "vendor,vendor-uart";
> reg = <0x4806a000 0x100>;
> interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
> clock-frequency = <48000000>;
> ...
> };
>
> And in the dts overlay, just add the device as a child node:
> uart1: serial@1 {
> bt-vendor {
> compatible = "vendor,bt-vendor";
> reset-gpio = <&pmx_gpio 77 0 >;
> interrupts = < 2 VENDOR_IRQ_TYPE_EDGE_FALLING >;
> ...
> }
> }
>
> I think it's a good way to link the tty with the pdev and there is no
> specific label to add and document.
>
> What do you think about this?
>
> I'm not specially against adding a parameter (vendor,tty = "ttys1").
> But it means that you need to be sure that your serial will be always
> named "ttys1", if someone remove/disable/add a serial port in a dts/dtsi, it
> can shift the tty number assigned by the driver.
>
> IF: All good ideas, Loic. I initially wanted BT DT configuration to be "like ACPI" too. The response from the DT maintainer was "DT's not ACPI". He essentially wanted us to describe the whole combo chip as a device with BT being one of the sub-devices. The point was that BT/WiFi/GPS/NFC/FM/whatever are dependent on each other and on some properties of the whole chip, say on power or clock. There is some truth to that but I believe the BT relationship to the UART is of more relevance to its driver than to the rest of the overall chip. In any case, we were not able to find common ground (no response to my last couple of messages) resulting in a customer not using bluetooth-next for that BT device integration. I guess DT has its own interests and seems to care little about BlueZ. I hope therefore that the second similar request from the major hardware vendor (you guys) will make DT more cooperative. All hardware vendors will then be able to benefit from whatever scheme gets ac
cepted. T
o
> make a long story short, I encourage you to start including DT in your patches. :-)
Hi Ilya,
Based on DT maintainer (Rob Herring?) I sent a couple of proposals after
which we ended up with the second option as shown above or at least that
was my understanding (see [1]).
Regards,
Arend
[1] http://article.gmane.org/gmane.linux.bluez.kernel/63216
> Regards,
> Loic
>
Hi Loic,
-----Original Message-----
From: Loic Poulain [mailto:[email protected]]=20
Sent: Thursday, August 20, 2015 4:58 AM
To: Ilya Faenson; [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Bluetooth: hci_intel: Add platform driver
Thanks Ilya,
> Is there a patch with the DT documentation? It would be interesting to se=
e what DT maintainers think of this approach.
I don't have any documentation for now.
> That allows you to run with a single device per platform only. Meanwhile,=
you could have something in the DT parameters identifying the UART. You wo=
uld then be able to retrieve that parameter and match it against the tty in=
the BT protocol. Multiple devices per platform would then be supported.
Actually It supports multiple chips but takes them in the registered order.
But, I agree, it's not a correct way to do that.
I'm not a DT expert but I think we can do this match in the same way as=20
acpi.
It just requires to make the Bluetooth entry a child of the serial port=20
entry.
for example:
In dtsi you could have the usual uart description:
uart1: serial@1 {
compatible =3D "vendor,vendor-uart";
reg =3D <0x4806a000 0x100>;
interrupts =3D <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency =3D <48000000>;
...
};
And in the dts overlay, just add the device as a child node:
uart1: serial@1 {
bt-vendor {
compatible =3D "vendor,bt-vendor";
reset-gpio =3D <&pmx_gpio 77 0 >;
interrupts =3D < 2 VENDOR_IRQ_TYPE_EDGE_FALLING >;
...
}
}
I think it's a good way to link the tty with the pdev and there is no
specific label to add and document.
What do you think about this?
I'm not specially against adding a parameter (vendor,tty =3D "ttys1").
But it means that you need to be sure that your serial will be always
named "ttys1", if someone remove/disable/add a serial port in a dts/dtsi, i=
t
can shift the tty number assigned by the driver.
IF: All good ideas, Loic. I initially wanted BT DT configuration to be "lik=
e ACPI" too. The response from the DT maintainer was "DT's not ACPI". He es=
sentially wanted us to describe the whole combo chip as a device with BT be=
ing one of the sub-devices. The point was that BT/WiFi/GPS/NFC/FM/whatever =
are dependent on each other and on some properties of the whole chip, say o=
n power or clock. There is some truth to that but I believe the BT relation=
ship to the UART is of more relevance to its driver than to the rest of the=
overall chip. In any case, we were not able to find common ground (no resp=
onse to my last couple of messages) resulting in a customer not using bluet=
ooth-next for that BT device integration. I guess DT has its own interests =
and seems to care little about BlueZ. I hope therefore that the second simi=
lar request from the major hardware vendor (you guys) will make DT more coo=
perative. All hardware vendors will then be able to benefit from whatever s=
cheme gets accepted. To make a long story short, I encourage you to start i=
ncluding DT in your patches. :-)
Regards,
Loic
--=20
Intel Open Source Technology Center
http://oss.intel.com/
Thanks Ilya,
> Is there a patch with the DT documentation? It would be interesting to see what DT maintainers think of this approach.
I don't have any documentation for now.
> That allows you to run with a single device per platform only. Meanwhile, you could have something in the DT parameters identifying the UART. You would then be able to retrieve that parameter and match it against the tty in the BT protocol. Multiple devices per platform would then be supported.
Actually It supports multiple chips but takes them in the registered order.
But, I agree, it's not a correct way to do that.
I'm not a DT expert but I think we can do this match in the same way as
acpi.
It just requires to make the Bluetooth entry a child of the serial port
entry.
for example:
In dtsi you could have the usual uart description:
uart1: serial@1 {
compatible = "vendor,vendor-uart";
reg = <0x4806a000 0x100>;
interrupts = <GIC_SPI 72 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <48000000>;
...
};
And in the dts overlay, just add the device as a child node:
uart1: serial@1 {
bt-vendor {
compatible = "vendor,bt-vendor";
reset-gpio = <&pmx_gpio 77 0 >;
interrupts = < 2 VENDOR_IRQ_TYPE_EDGE_FALLING >;
...
}
}
I think it's a good way to link the tty with the pdev and there is no
specific label to add and document.
What do you think about this?
I'm not specially against adding a parameter (vendor,tty = "ttys1").
But it means that you need to be sure that your serial will be always
named "ttys1", if someone remove/disable/add a serial port in a dts/dtsi, it
can shift the tty number assigned by the driver.
Regards,
Loic
--
Intel Open Source Technology Center
http://oss.intel.com/
Hi Loic,
-----Original Message-----
From: [email protected] [mailto:linux-bluetooth-owner@v=
ger.kernel.org] On Behalf Of Loic Poulain
Sent: Wednesday, August 19, 2015 1:14 PM
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH] Bluetooth: hci_intel: Add platform driver
On 19/08/2015 19:11, Loic Poulain wrote:
> 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.
Is there a patch with the DT documentation? It would be interesting to see =
what DT maintainers think of this approach.
>
> 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.
> In case of device tree, there is currently nothing to link the pdev
> with its tty. So, we just match the first available pdev.
That allows you to run with a single device per platform only. Meanwhile, y=
ou could have something in the DT parameters identifying the UART. You woul=
d then be able to retrieve that parameter and match it against the tty in t=
he BT protocol. Multiple devices per platform would then be supported.
Thanks,
-Ilya
>
> 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]>
> ---
> drivers/bluetooth/hci_intel.c | 275 +++++++++++++++++++++++++++++++++++=
++++---
> 1 file changed, 255 insertions(+), 20 deletions(-)
Apply over: Bluetooth: hci_uart: Add Intel baudrate configuration support
Regards,
Loic
--=20
Intel Open Source Technology Center
http://oss.intel.com/
On 19/08/2015 19:11, Loic Poulain wrote:
> 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.
> In case of device tree, there is currently nothing to link the pdev
> with its tty. So, we just match the first available pdev.
>
> 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]>
> ---
> drivers/bluetooth/hci_intel.c | 275 +++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 255 insertions(+), 20 deletions(-)
Apply over: Bluetooth: hci_uart: Add Intel baudrate configuration support
Regards,
Loic
--
Intel Open Source Technology Center
http://oss.intel.com/