Hi guys,
I've been fiddling with the BCM43430 driver a bit today, and have
managed to get it to work with device tree supplying the GPIO.
I've created a dt entry that looks like:
bt_brcm {
compatible = "brcm,b43430a1";
wakeup-gpios = <&gpio5 4 GPIO_ACTIVE_HIGH>;
ttydev = <&uart2>;
status = "ok";
};
};
This allows me to get the code to correlate the UART and the GPIOs,
thus: (forgive the very messy patch - this is my WIP, for illustration
only...)
The code below *works* - toggles the power GPIO, loads firmware, and
attaches fine.
However, if I uncomment the line containing:
err = bcm_setup_sleep(hu);
The driver dies, with timeouts, thus:
Bluetooth: hci0: BCM: chip id 94
Bluetooth: hci0: BCM43430A1 (001.002.009) build 0000
Bluetooth: hci0: BCM: failed to write clock (-56)
Bluetooth: hci0: BCM (001.002.009) build 0182
Bluetooth: hci0 command 0x0c56 tx timeout
Bluetooth: hci0 command 0x0c7a tx timeout
Bluetooth: hci0 command 0x0c6d tx timeout
Bluetooth: hci0 command 0x2008 tx timeout
Bluetooth: hci0 command 0x2009 tx timeout
The "Bluetooth: hci0: BCM: failed to write clock (-56)" is a red herring
- it works fine despite this - just the bcm_setup_sleep() function seems
to trip it up.
Any thoughts?
-Ian
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index f87bfdfee4ff..a164397ee2d7 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -34,6 +34,7 @@
#include <linux/interrupt.h>
#include <linux/dmi.h>
#include <linux/pm_runtime.h>
+#include <linux/of.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -50,6 +51,7 @@ struct bcm_device {
struct list_head list;
struct platform_device *pdev;
+ struct device_node *of_ttydev;
const char *name;
struct gpio_desc *device_wakeup;
@@ -145,6 +147,8 @@ static bool bcm_device_exists(struct bcm_device *device)
static int bcm_gpio_set_power(struct bcm_device *dev, bool powered)
{
+
+ printk("----------brcm: powered: %d\n", powered);
if (powered && !IS_ERR(dev->clk) && !dev->clk_enabled)
clk_prepare_enable(dev->clk);
@@ -203,7 +207,6 @@ static int bcm_request_irq(struct bcm_data *bcm)
unlock:
mutex_unlock(&bcm_device_lock);
-
return err;
}
@@ -279,6 +282,7 @@ static int bcm_open(struct hci_uart *hu)
bt_dev_dbg(hu->hdev, "hu %p", hu);
+printk("-----brcm open\n");
bcm = kzalloc(sizeof(*bcm), GFP_KERNEL);
if (!bcm)
return -ENOMEM;
@@ -298,7 +302,9 @@ 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->of_node == dev->of_ttydev) {
+
bcm->dev = dev;
hu->init_speed = dev->init_speed;
#ifdef CONFIG_PM
@@ -319,6 +325,7 @@ static int bcm_close(struct hci_uart *hu)
struct bcm_data *bcm = hu->priv;
struct bcm_device *bdev = bcm->dev;
+printk("--a--- %s\n", __func__);
bt_dev_dbg(hu->hdev, "hu %p", hu);
/* Protect bcm->dev against removal of the device or driver */
@@ -366,6 +373,7 @@ static int bcm_setup(struct hci_uart *hu)
unsigned int speed;
int err;
+printk("--a--- %s\n", __func__);
bt_dev_dbg(hu->hdev, "hu %p", hu);
hu->hdev->set_diag = bcm_set_diag;
@@ -420,10 +428,10 @@ static int bcm_setup(struct hci_uart *hu)
return err;
err = bcm_request_irq(bcm);
- if (!err)
- err = bcm_setup_sleep(hu);
+// if (!err)
+// err = bcm_setup_sleep(hu);
- return err;
+ return 0;
}
#define BCM_RECV_LM_DIAG \
@@ -705,17 +713,26 @@ static int bcm_resource(struct acpi_resource
*ares, void *data)
static int bcm_platform_probe(struct bcm_device *dev)
{
struct platform_device *pdev = dev->pdev;
+ const __be32 *np;
dev->name = dev_name(&pdev->dev);
dev->clk = devm_clk_get(&pdev->dev, NULL);
- dev->device_wakeup = devm_gpiod_get_optional(&pdev->dev,
- "device-wakeup",
- GPIOD_OUT_LOW);
+ dev->device_wakeup = gpiod_get(&pdev->dev, "wakeup",
GPIOD_OUT_HIGH);
+
+ np = of_get_property(dev_of_node(&pdev->dev), "ttydev", NULL);
+ dev->of_ttydev = of_find_node_by_phandle(be32_to_cpup(np));
+
+
+// dev->device_wakeup = devm_gpiod_get_optional(&pdev->dev,
+// "device-wakeup",
+// GPIOD_OUT_LOW);
if (IS_ERR(dev->device_wakeup))
return PTR_ERR(dev->device_wakeup);
+ printk("bcmbt------- %p\n", dev->device_wakeup);
+
dev->shutdown = devm_gpiod_get_optional(&pdev->dev, "shutdown",
GPIOD_OUT_LOW);
if (IS_ERR(dev->shutdown))
@@ -886,12 +903,22 @@ static const struct dev_pm_ops bcm_pm_ops = {
SET_RUNTIME_PM_OPS(bcm_suspend_device, bcm_resume_device, NULL)
};
+static struct of_device_id bcm_of_match[] = {
+ { .compatible = "brcm,b43430a1", },
+ { }
+};
+
static struct platform_driver bcm_driver = {
.probe = bcm_probe,
.remove = bcm_remove,
.driver = {
.name = "hci_bcm",
.acpi_match_table = ACPI_PTR(bcm_acpi_match),
+ .of_match_table = bcm_of_match,
.pm = &bcm_pm_ops,
},
};
13
Resent because of MUA error:
On 30/06/17 22:27, Marcel Holtmann wrote:
> Hi Ian,
>> It seems there is a fair bit of commonality among the h4 based
>> bluetooth drivers - probably better to abstract it out and make a
>> h4 core around which the other serdev and ldisc drivers could sit?
>
> don't try to overthink this. The code is already abstracted and
> optimized. Add the serdev support to hci_bcm.c and do not try to mess
> with the hci_nokia.c. That one is a total different driver and it is
> special in its own way.
It doesnt *look* terribly special - there are a few extra entries in
nokia_recv_pkts[] along with their helper functions - but its otherwise
very similar indeed...
I'm happy to add the code to hci_bcm, but it looks like theres a fair
bit of code duplication across at least these two drivers...
-Ian
On 30/06/17 22:27, Marcel Holtmann wrote:
> Hi Ian,
>
> don’t try to overthink this. The code is already abstracted and
> optimized. Add the serdev support to hci_bcm.c and do not try to mess
> with the hci_nokia.c. That one is a total different driver and it is
> special in its own way.
It doesnt *look* terribly special - there are a few extra entries in
nokia_recv_pkts[] along with their helper functions - but its otherwise
very similar indeed...
I'm happy to add the code to hci_bcm, but it looks like theres a fair
bit of code duplication across at least these two drivers...
-Ian
Hi Ian,
>>> If you have an embedded board with a proper DT definition, and the
>>> UART is hard-wired to the Bluetooth chip, then go for serdev. It
>>> means no btattach is needed. The kernel will enumerate this properly
>>> for you. That is the way forward. Someone needed to adapt the
>>> hci_bcm.c driver for this anyway. It just happens that you are the
>>> first one.
>>
>> Right - I'll follow that route and add serdev support to hci_bcm.c then!
>
> Hmm. ok, this gets to be a bitof a rabbit-hole.
>
> the hci-nokia driver is *extremely* similar in operation to the hci_bcm
> driver, other than being serdev based. even the firmware loading is
> "broadcom style", so I dont really see why it needs another driver,
> other than its been coded to one chip and a few small nokia-specific
> bits in it.
>
> It seems there is a fair bit of commonality among the h4 based bluetooth
> drivers - probably better to abstract it out and make a h4 core around
> which the other serdev and ldisc drivers could sit?
don’t try to overthink this. The code is already abstracted and optimized. Add the serdev support to hci_bcm.c and do not try to mess with the hci_nokia.c. That one is a total different driver and it is special in its own way.
Regards
Marcel
Just FYI, this is what I'd come up with earlier today, which works. But
its not serdev based:
Subject: [PATCH 1/5] bluetooth: Add DT support to hci_bcm
This patch adds minimal DT support to hci_bcm.
It takes the following parameters in its DT node:
ttydev - a phandle for the UART the device is attached to.
wakeup-gpios - a single GPIO used to control power to the chip.
init-speed - baud rate to initialise the chip at
oper-speed = baud rate to run the chip at after init.
bt_brcm {
compatible = "brcm,b43430a1";
ttydev = <&uart2>;
wakeup-gpios = <&gpio5 4 GPIO_ACTIVE_HIGH>;
init-speed = <115200>;
oper-speed = <115200>;
};
---
drivers/bluetooth/hci_bcm.c | 67
+++++++++++++++++++++++++++++++++++++++++++--
1 file changed, 64 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index df7fa8ff9e0e..cd6312462fb0 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -34,6 +34,7 @@
#include <linux/interrupt.h>
#include <linux/dmi.h>
#include <linux/pm_runtime.h>
+#include <linux/of.h>
#include <net/bluetooth/bluetooth.h>
#include <net/bluetooth/hci_core.h>
@@ -50,6 +51,7 @@ struct bcm_device {
struct list_head list;
struct platform_device *pdev;
+ struct device_node *of_tty_np;
const char *name;
struct gpio_desc *device_wakeup;
@@ -59,6 +61,7 @@ struct bcm_device {
bool clk_enabled;
u32 init_speed;
+ u32 oper_speed;
int irq;
u8 irq_polarity;
@@ -299,9 +302,13 @@ 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->of_node == dev->of_tty_np) {
+
bcm->dev = dev;
+
hu->init_speed = dev->init_speed;
+ hu->oper_speed = dev->oper_speed;
#ifdef CONFIG_PM
dev->hu = hu;
#endif
@@ -424,7 +431,7 @@ static int bcm_setup(struct hci_uart *hu)
if (!err)
err = bcm_setup_sleep(hu);
- return err;
+ return 0;
}
#define BCM_RECV_LM_DIAG \
@@ -703,6 +710,46 @@ static int bcm_resource(struct acpi_resource *ares,
void *data)
}
#endif /* CONFIG_ACPI */
+#ifdef CONFIG_OF
+static int bcm_dt_probe(struct bcm_device *dev)
+{
+ struct platform_device *pdev = dev->pdev;
+ const struct device_node *np = dev_of_node(&pdev->dev);
+
+ dev->name = dev_name(&pdev->dev);
+
+ /*
+ * If we know which ttydev btattach will use, we can match it
+ * at when the _open() call occurs, allowing us to control
+ * other functionality, such as wakeup GPIOs
+ *
+ */
+
+ dev->of_tty_np = of_parse_phandle(np, "ttydev", 0);
+ if(dev->of_tty_np) {
+ struct gpio_desc *g;
+
+ g = gpiod_get(&pdev->dev, "wakeup", GPIOD_OUT_LOW);
+ if(PTR_ERR(g))
+ dev->device_wakeup = g;
+
+ of_property_read_u32(np, "init-speed", &dev->init_speed);
+ of_property_read_u32(np, "oper-speed", &dev->oper_speed);
+ }
+
+ /*
+ * It is entirely possible for devices with no power control GPIOs
+ * to exist, so do not error on this case, however
+ *
+ */
+
+ return 0;
+}
+#else
+static int bcm_dt_probe(struct bcm_device *dev) { return 0; }
+#endif
+
+
static int bcm_platform_probe(struct bcm_device *dev)
{
struct platform_device *pdev = dev->pdev;
@@ -806,7 +853,9 @@ static int bcm_probe(struct platform_device *pdev)
dev->pdev = pdev;
- if (has_acpi_companion(&pdev->dev))
+ if(pdev->dev.of_node)
+ ret = bcm_dt_probe(dev);
+ else if (has_acpi_companion(&pdev->dev))
ret = bcm_acpi_probe(dev);
else
ret = bcm_platform_probe(dev);
@@ -887,12 +936,24 @@ static const struct dev_pm_ops bcm_pm_ops = {
SET_RUNTIME_PM_OPS(bcm_suspend_device, bcm_resume_device, NULL)
};
+#ifdef CONFIG_OF
+static struct of_device_id bcm_of_match[] = {
+ { .compatible = "brcm,b43430a1", },
+ { }
+};
+
+MODULE_DEVICE_TABLE(of, bcm_of_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),
+#ifdef CONFIG_OF
+ .of_match_table = bcm_of_match,
+#endif
.pm = &bcm_pm_ops,
},
};
--
2.11.0
On 30/06/17 18:59, Loic Poulain wrote:
> Yes, but there is no IRQ assigned to your bcm_device.
> Actually seems you find an other "issue", the bcm_request_irq() returns
> an error only on request_irq failure. However in your case, you never
> assigned the bcm->irq, so bcm_request_irq does not even try to request
> the irq and returns success.
Yes, thats what I was getting at.
I solved it by setting err to -EINVAL to begin with, rather than 0, as a
temporary hack, as I wasn't sure of the original functions intent.
Your solution is cleaner, so I've tested it, it works.
> You should try the following:
>
> diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
> index be53238..586f3d0 100644
> --- a/drivers/bluetooth/hci_bcm.c
> +++ b/drivers/bluetooth/hci_bcm.c
> @@ -185,22 +185,24 @@ static int bcm_request_irq(struct bcm_data *bcm)
> goto unlock;
> }
>
> - if (bdev->irq > 0) {
> - err = devm_request_irq(&bdev->pdev->dev, bdev->irq,
> - bcm_host_wake, IRQF_TRIGGER_RISING,
> - "host_wake", bdev);
> - if (err)
> - goto unlock;
> -
> - device_init_wakeup(&bdev->pdev->dev, true);
> -
> - pm_runtime_set_autosuspend_delay(&bdev->pdev->dev,
> - BCM_AUTOSUSPEND_DELAY);
> - pm_runtime_use_autosuspend(&bdev->pdev->dev);
> - pm_runtime_set_active(&bdev->pdev->dev);
> - pm_runtime_enable(&bdev->pdev->dev);
> + if (bdev->irq <= 0) {
> + err = -EOPNOTSUPP;
> + goto unlock;
> }
>
> + err = devm_request_irq(&bdev->pdev->dev, bdev->irq, bcm_host_wake,
> + IRQF_TRIGGER_RISING, "host_wake", bdev);
> + if (err)
> + goto unlock;
> +
> + device_init_wakeup(&bdev->pdev->dev, true);
> +
> + pm_runtime_set_autosuspend_delay(&bdev->pdev->dev,
> + BCM_AUTOSUSPEND_DELAY);
> + pm_runtime_use_autosuspend(&bdev->pdev->dev);
> + pm_runtime_set_active(&bdev->pdev->dev);
> + pm_runtime_enable(&bdev->pdev->dev);
> +
> unlock:
> mutex_unlock(&bcm_device_lock);
>
On 30/06/17 18:14, Ian Molton wrote:
> On 30/06/17 18:02, Marcel Holtmann wrote:
>> If you have an embedded board with a proper DT definition, and the
>> UART is hard-wired to the Bluetooth chip, then go for serdev. It
>> means no btattach is needed. The kernel will enumerate this properly
>> for you. That is the way forward. Someone needed to adapt the
>> hci_bcm.c driver for this anyway. It just happens that you are the
>> first one.
>
> Right - I'll follow that route and add serdev support to hci_bcm.c then!
Hmm. ok, this gets to be a bitof a rabbit-hole.
the hci-nokia driver is *extremely* similar in operation to the hci_bcm
driver, other than being serdev based. even the firmware loading is
"broadcom style", so I dont really see why it needs another driver,
other than its been coded to one chip and a few small nokia-specific
bits in it.
It seems there is a fair bit of commonality among the h4 based bluetooth
drivers - probably better to abstract it out and make a h4 core around
which the other serdev and ldisc drivers could sit?
I'll have a look at that on Monday.
-Ian
Hi Ian,
On 30/06/2017 18:35, Ian Molton wrote:
>> bcm_setup_sleep enable low power mode, so I think problem is
>> that the wakeup gpio is not correctly drove.
>> -> Are you sure that you don't toggle the power gpio instead of the
>> wakeup one.
>
> Possibly not - AFAIK, I only have one GPIO on this board, so I *assume*
> its the power one. I'll see if I can find out if wakeup/irq are connected.
>
> *HOWEVER* the code checks for success of bcm_request_irq() when deciding
> to execute bcm_setup_sleep(). This makes sense to me, as I'd expect the
> hardware to want to interrupt the host when it wakes up.
Yes, but there is no IRQ assigned to your bcm_device.
Actually seems you find an other "issue", the bcm_request_irq() returns
an error only on request_irq failure. However in your case, you never
assigned the bcm->irq, so bcm_request_irq does not even try to request
the irq and returns success.
You should try the following:
diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index be53238..586f3d0 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -185,22 +185,24 @@ static int bcm_request_irq(struct bcm_data *bcm)
goto unlock;
}
- if (bdev->irq > 0) {
- err = devm_request_irq(&bdev->pdev->dev, bdev->irq,
- bcm_host_wake, IRQF_TRIGGER_RISING,
- "host_wake", bdev);
- if (err)
- goto unlock;
-
- device_init_wakeup(&bdev->pdev->dev, true);
-
- pm_runtime_set_autosuspend_delay(&bdev->pdev->dev,
- BCM_AUTOSUSPEND_DELAY);
- pm_runtime_use_autosuspend(&bdev->pdev->dev);
- pm_runtime_set_active(&bdev->pdev->dev);
- pm_runtime_enable(&bdev->pdev->dev);
+ if (bdev->irq <= 0) {
+ err = -EOPNOTSUPP;
+ goto unlock;
}
+ err = devm_request_irq(&bdev->pdev->dev, bdev->irq, bcm_host_wake,
+ IRQF_TRIGGER_RISING, "host_wake", bdev);
+ if (err)
+ goto unlock;
+
+ device_init_wakeup(&bdev->pdev->dev, true);
+
+ pm_runtime_set_autosuspend_delay(&bdev->pdev->dev,
+ BCM_AUTOSUSPEND_DELAY);
+ pm_runtime_use_autosuspend(&bdev->pdev->dev);
+ pm_runtime_set_active(&bdev->pdev->dev);
+ pm_runtime_enable(&bdev->pdev->dev);
+
unlock:
mutex_unlock(&bcm_device_lock);
Regards,
Loic
On 30/06/17 18:02, Marcel Holtmann wrote:
> If you have an embedded board with a proper DT definition, and the
> UART is hard-wired to the Bluetooth chip, then go for serdev. It
> means no btattach is needed. The kernel will enumerate this properly
> for you. That is the way forward. Someone needed to adapt the
> hci_bcm.c driver for this anyway. It just happens that you are the
> first one.
Right - I'll follow that route and add serdev support to hci_bcm.c then!
Thanks,
-Ian
Hi Ian,
>>> However, if I uncomment the line containing:
>>>
>>> err = bcm_setup_sleep(hu);
>>>
>>> The driver dies, with timeouts, thus:
>>>
>>> Bluetooth: hci0: BCM: chip id 94
>>> Bluetooth: hci0: BCM43430A1 (001.002.009) build 0000
>>> Bluetooth: hci0: BCM: failed to write clock (-56)
>>> Bluetooth: hci0: BCM (001.002.009) build 0182
>>> Bluetooth: hci0 command 0x0c56 tx timeout
>>> Bluetooth: hci0 command 0x0c7a tx timeout
>>> Bluetooth: hci0 command 0x0c6d tx timeout
>>> Bluetooth: hci0 command 0x2008 tx timeout
>>> Bluetooth: hci0 command 0x2009 tx timeout
>>
>> bcm_setup_sleep enable low power mode, so I think problem is
>> that the wakeup gpio is not correctly drove.
>> -> Are you sure that you don't toggle the power gpio instead of the
>> wakeup one.
>
> Possibly not - AFAIK, I only have one GPIO on this board, so I *assume*
> its the power one. I'll see if I can find out if wakeup/irq are connected.
>
> *HOWEVER* the code checks for success of bcm_request_irq() when deciding
> to execute bcm_setup_sleep(). This makes sense to me, as I'd expect the
> hardware to want to interrupt the host when it wakes up.
>
>> -> Without host-wakeup gpio, you are not able to get controller events.
>>
>>> static struct platform_driver bcm_driver = {
>>> .probe = bcm_probe,
>>> .remove = bcm_remove,
>>> .driver = {
>>> .name = "hci_bcm",
>>> .acpi_match_table = ACPI_PTR(bcm_acpi_match),
>>> + .of_match_table = bcm_of_match,
>>> .pm = &bcm_pm_ops,
>>> },
>>> };
>>> 13
>>>
>>
>> I think you should look at the serial device driver solution
>> (e.g nokia.c), This is the preferable way now.
>
> damnit! thats very similar to my first idea on the matter, but I
> couldn't see how it was supposed to work.
>
> Whats the right path forward here? both the drivers you listed have some
> merit - the hci_bcm driver, I have working, and it seems to support
> loading different firmwares well.
>
> hci_nokia.c seems very similar, but only supports 2 firmwares, AFAICT.
the hci_ll.c for TI chips was changed to support ldisc and serdev. So clearly both can be supported from within the same driver.
If you have an embedded board with a proper DT definition, and the UART is hard-wired to the Bluetooth chip, then go for serdev. It means no btattach is needed. The kernel will enumerate this properly for you. That is the way forward. Someone needed to adapt the hci_bcm.c driver for this anyway. It just happens that you are the first one.
Regards
Marcel
Hi!
>>
>> However, if I uncomment the line containing:
>>
>> err = bcm_setup_sleep(hu);
>>
>> The driver dies, with timeouts, thus:
>>
>> Bluetooth: hci0: BCM: chip id 94
>> Bluetooth: hci0: BCM43430A1 (001.002.009) build 0000
>> Bluetooth: hci0: BCM: failed to write clock (-56)
>> Bluetooth: hci0: BCM (001.002.009) build 0182
>> Bluetooth: hci0 command 0x0c56 tx timeout
>> Bluetooth: hci0 command 0x0c7a tx timeout
>> Bluetooth: hci0 command 0x0c6d tx timeout
>> Bluetooth: hci0 command 0x2008 tx timeout
>> Bluetooth: hci0 command 0x2009 tx timeout
>
> bcm_setup_sleep enable low power mode, so I think problem is
> that the wakeup gpio is not correctly drove.
> -> Are you sure that you don't toggle the power gpio instead of the
> wakeup one.
Possibly not - AFAIK, I only have one GPIO on this board, so I *assume*
its the power one. I'll see if I can find out if wakeup/irq are connected.
*HOWEVER* the code checks for success of bcm_request_irq() when deciding
to execute bcm_setup_sleep(). This makes sense to me, as I'd expect the
hardware to want to interrupt the host when it wakes up.
> -> Without host-wakeup gpio, you are not able to get controller events.
>
>> static struct platform_driver bcm_driver = {
>> .probe = bcm_probe,
>> .remove = bcm_remove,
>> .driver = {
>> .name = "hci_bcm",
>> .acpi_match_table = ACPI_PTR(bcm_acpi_match),
>> + .of_match_table = bcm_of_match,
>> .pm = &bcm_pm_ops,
>> },
>> };
>> 13
>>
>
> I think you should look at the serial device driver solution
> (e.g nokia.c), This is the preferable way now.
damnit! thats very similar to my first idea on the matter, but I
couldn't see how it was supposed to work.
Whats the right path forward here? both the drivers you listed have some
merit - the hci_bcm driver, I have working, and it seems to support
loading different firmwares well.
hci_nokia.c seems very similar, but only supports 2 firmwares, AFAICT.
-Ian
Hi Ian,
On 29/06/2017 20:06, Ian Molton wrote:
> I've created a dt entry that looks like:
>
> bt_brcm {
> compatible = "brcm,b43430a1";
> wakeup-gpios = <&gpio5 4 GPIO_ACTIVE_HIGH>;
> ttydev = <&uart2>;
> status = "ok";
> };
> };
>
> This allows me to get the code to correlate the UART and the GPIOs,
> thus: (forgive the very messy patch - this is my WIP, for illustration
> only...)
>
> The code below *works* - toggles the power GPIO, loads firmware, and
> attaches fine.
Be careful here, you add only one gpio in our DT entry, the wakeup one.
Typically (bcm43xx) you need 2 gpios + 1 irq:
power (gpio): toggle the chip off/on
wakeup (gpio): wakeup the chip, allowing send/recv data over UART
host-wakeup (gpio): Request attention, data to send
In suspended mode, if you have wakeup gpio but no host-wakeup,
the controller will be unable to inform the host about data readiness,
So the host will not allow the controller to send its data (via wakeup).
The only 3 working conditions are:
1 - you don't have any resource, chip is powered on by default (hw
pull-up or firmware config), the driver operate without power mgmt.
2 - you have the power gpio, controller is power on/off on
attach/detach. No low-power mode except if condition 3.
3- you have the wakeup and host-wakeup resources, so the host is
able to manage the low-power mode (suspend/resume)
>
> However, if I uncomment the line containing:
>
> err = bcm_setup_sleep(hu);
>
> The driver dies, with timeouts, thus:
>
> Bluetooth: hci0: BCM: chip id 94
> Bluetooth: hci0: BCM43430A1 (001.002.009) build 0000
> Bluetooth: hci0: BCM: failed to write clock (-56)
> Bluetooth: hci0: BCM (001.002.009) build 0182
> Bluetooth: hci0 command 0x0c56 tx timeout
> Bluetooth: hci0 command 0x0c7a tx timeout
> Bluetooth: hci0 command 0x0c6d tx timeout
> Bluetooth: hci0 command 0x2008 tx timeout
> Bluetooth: hci0 command 0x2009 tx timeout
bcm_setup_sleep enable low power mode, so I think problem is
that the wakeup gpio is not correctly drove.
-> Are you sure that you don't toggle the power gpio instead of the
wakeup one.
-> Without host-wakeup gpio, you are not able to get controller events.
> static struct platform_driver bcm_driver = {
> .probe = bcm_probe,
> .remove = bcm_remove,
> .driver = {
> .name = "hci_bcm",
> .acpi_match_table = ACPI_PTR(bcm_acpi_match),
> + .of_match_table = bcm_of_match,
> .pm = &bcm_pm_ops,
> },
> };
> 13
>
I think you should look at the serial device driver solution
(e.g nokia.c), This is the preferable way now.
Regards,
Loic