All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth
@ 2017-08-07 10:39 ` Loic Poulain
  0 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-07 10:39 UTC (permalink / raw)
  To: marcel-kz+m5ild9QBg9hUCZPvPmw,
	johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, rjui-dY08KVG/lbpWk0Htik3J/w,
	sbranden-dY08KVG/lbpWk0Htik3J/w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, stefan.wahren-eS4NqCHxEME
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Loic Poulain

Add binding document for serial bluetooth chips using
Broadcom protocol.

Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 v2: dt-bindings as separate patch
     rebase on upcoming pi3 dts changes
 v3: changes in bcm serdev drivers:
     name refactoring and additional comments
     Add generic host_set_baudrate method
     Use agnostic device_property_read
 .../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt

diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
new file mode 100644
index 0000000..c51ea1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
@@ -0,0 +1,29 @@
+Broadcom Bluetooth Chips
+---------------------
+
+This documents the binding structure and common properties for serial
+attached Broadcom devices.
+
+Serial attached Broadcom devices shall be a child node of the host UART
+device the slave device is attached to.
+
+Required properties:
+
+ - compatible: should contain one of the following:
+   * "brcm,bcm43438-bt"
+
+Optional properties:
+
+ - max-speed: see Documentation/devicetree/bindings/serial/slave-device.txt
+
+Example:
+
+&uart2 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&uart2_pins>;
+
+       bluetooth {
+               compatible = "brcm,bcm43438-bt";
+               max-speed = <921600>;
+       };
+};
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH v3 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth
@ 2017-08-07 10:39 ` Loic Poulain
  0 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-07 10:39 UTC (permalink / raw)
  To: marcel, johan.hedberg, robh+dt, rjui, sbranden, f.fainelli,
	stefan.wahren
  Cc: devicetree, linux-bluetooth, linux-rpi-kernel, Loic Poulain

Add binding document for serial bluetooth chips using
Broadcom protocol.

Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
---
 v2: dt-bindings as separate patch
     rebase on upcoming pi3 dts changes
 v3: changes in bcm serdev drivers:
     name refactoring and additional comments
     Add generic host_set_baudrate method
     Use agnostic device_property_read
 .../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt

diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
new file mode 100644
index 0000000..c51ea1b
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
@@ -0,0 +1,29 @@
+Broadcom Bluetooth Chips
+---------------------
+
+This documents the binding structure and common properties for serial
+attached Broadcom devices.
+
+Serial attached Broadcom devices shall be a child node of the host UART
+device the slave device is attached to.
+
+Required properties:
+
+ - compatible: should contain one of the following:
+   * "brcm,bcm43438-bt"
+
+Optional properties:
+
+ - max-speed: see Documentation/devicetree/bindings/serial/slave-device.txt
+
+Example:
+
+&uart2 {
+       pinctrl-names = "default";
+       pinctrl-0 = <&uart2_pins>;
+
+       bluetooth {
+               compatible = "brcm,bcm43438-bt";
+               max-speed = <921600>;
+       };
+};
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-07 10:39 ` Loic Poulain
@ 2017-08-07 10:39     ` Loic Poulain
  -1 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-07 10:39 UTC (permalink / raw)
  To: marcel-kz+m5ild9QBg9hUCZPvPmw,
	johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, rjui-dY08KVG/lbpWk0Htik3J/w,
	sbranden-dY08KVG/lbpWk0Htik3J/w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, stefan.wahren-eS4NqCHxEME
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Loic Poulain

Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
This allows to automatically insert the bcm43438 to the bluetooth
subsystem instead of relying on userspace helpers (hciattach).

Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
of pl011/ttyAMA0.

Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 v2: dt-bindings as separate patch
     rebase on upcoming pi3 dts changes
 v3: changes in bcm serdev drivers:
     name refactoring and additional comments
     Add generic host_set_baudrate method
     Use agnostic device_property_read

 arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
index 20725ca..5abc1df 100644
--- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
+++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
@@ -8,6 +8,11 @@
 	compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
 	model = "Raspberry Pi 3 Model B";
 
+	chosen {
+		/* 8250 auxiliar UART instead of pl011 */
+		bootargs = "earlyprintk console=ttyS0,115200";
+	};
+
 	memory {
 		reg = <0 0x40000000>;
 	};
@@ -24,6 +29,11 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
 	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm43438-bt";
+		max-speed = <921600>;
+	};
 };
 
 /* uart1 is mapped to the pin header */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-07 10:39     ` Loic Poulain
  0 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-07 10:39 UTC (permalink / raw)
  To: marcel, johan.hedberg, robh+dt, rjui, sbranden, f.fainelli,
	stefan.wahren
  Cc: devicetree, linux-bluetooth, linux-rpi-kernel, Loic Poulain

Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
This allows to automatically insert the bcm43438 to the bluetooth
subsystem instead of relying on userspace helpers (hciattach).

Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
of pl011/ttyAMA0.

Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
---
 v2: dt-bindings as separate patch
     rebase on upcoming pi3 dts changes
 v3: changes in bcm serdev drivers:
     name refactoring and additional comments
     Add generic host_set_baudrate method
     Use agnostic device_property_read

 arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
index 20725ca..5abc1df 100644
--- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
+++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
@@ -8,6 +8,11 @@
 	compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
 	model = "Raspberry Pi 3 Model B";
 
+	chosen {
+		/* 8250 auxiliar UART instead of pl011 */
+		bootargs = "earlyprintk console=ttyS0,115200";
+	};
+
 	memory {
 		reg = <0 0x40000000>;
 	};
@@ -24,6 +29,11 @@
 	pinctrl-names = "default";
 	pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
 	status = "okay";
+
+	bluetooth {
+		compatible = "brcm,bcm43438-bt";
+		max-speed = <921600>;
+	};
 };
 
 /* uart1 is mapped to the pin header */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH v3 3/3] Bluetooth: hci_bcm: Add serdev support
  2017-08-07 10:39 ` Loic Poulain
@ 2017-08-07 10:39     ` Loic Poulain
  -1 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-07 10:39 UTC (permalink / raw)
  To: marcel-kz+m5ild9QBg9hUCZPvPmw,
	johan.hedberg-Re5JQEeQqe8AvxtiuMwx3w,
	robh+dt-DgEjT+Ai2ygdnm+yROfE0A, rjui-dY08KVG/lbpWk0Htik3J/w,
	sbranden-dY08KVG/lbpWk0Htik3J/w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, stefan.wahren-eS4NqCHxEME
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-bluetooth-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Loic Poulain

Add basic support for Broadcom serial slave devices.
Probe the serial device, retrieve its maximum speed and
register a new hci uart device.

Tested/compatible with bcm43438 (RPi3).

Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
 drivers/bluetooth/hci_bcm.c | 85 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 83 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 6b42372..086d73c 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -27,6 +27,8 @@
 #include <linux/firmware.h>
 #include <linux/module.h>
 #include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/gpio/consumer.h>
@@ -34,6 +36,7 @@
 #include <linux/interrupt.h>
 #include <linux/dmi.h>
 #include <linux/pm_runtime.h>
+#include <linux/serdev.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -46,6 +49,7 @@
 
 #define BCM_AUTOSUSPEND_DELAY	5000 /* default autosleep delay */
 
+/* platform device driver resources */
 struct bcm_device {
 	struct list_head	list;
 
@@ -68,6 +72,12 @@ struct bcm_device {
 #endif
 };
 
+/* serdev driver resources */
+struct bcm_serdev {
+	struct hci_uart hu;
+};
+
+/* generic bcm uart resources */
 struct bcm_data {
 	struct sk_buff		*rx_skb;
 	struct sk_buff_head	txq;
@@ -79,6 +89,14 @@ struct bcm_data {
 static DEFINE_MUTEX(bcm_device_lock);
 static LIST_HEAD(bcm_device_list);
 
+static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed)
+{
+	if (hu->serdev)
+		serdev_device_set_baudrate(hu->serdev, speed);
+	else
+		hci_uart_set_baudrate(hu, speed);
+}
+
 static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
 {
 	struct hci_dev *hdev = hu->hdev;
@@ -289,6 +307,14 @@ static int bcm_open(struct hci_uart *hu)
 
 	hu->priv = bcm;
 
+	/* If this is a serdev defined device, then only use
+	 * serdev open primitive and skip the rest.
+	 */
+	if (hu->serdev) {
+		serdev_device_open(hu->serdev);
+		goto out;
+	}
+
 	if (!hu->tty->dev)
 		goto out;
 
@@ -323,6 +349,12 @@ static int bcm_close(struct hci_uart *hu)
 
 	bt_dev_dbg(hu->hdev, "hu %p", hu);
 
+	/* If this is a serdev defined device, only use serdev
+	 * close primitive and then continue as usual.
+	 */
+	if (hu->serdev)
+		serdev_device_close(hu->serdev);
+
 	/* Protect bcm->dev against removal of the device or driver */
 	mutex_lock(&bcm_device_lock);
 	if (bcm_device_exists(bdev)) {
@@ -398,7 +430,7 @@ static int bcm_setup(struct hci_uart *hu)
 		speed = 0;
 
 	if (speed)
-		hci_uart_set_baudrate(hu, speed);
+		host_set_baudrate(hu, speed);
 
 	/* Operational speed if any */
 	if (hu->oper_speed)
@@ -411,7 +443,7 @@ static int bcm_setup(struct hci_uart *hu)
 	if (speed) {
 		err = bcm_set_baudrate(hu, speed);
 		if (!err)
-			hci_uart_set_baudrate(hu, speed);
+			host_set_baudrate(hu, speed);
 	}
 
 finalize:
@@ -903,9 +935,57 @@ static int bcm_remove(struct platform_device *pdev)
 	},
 };
 
+static int bcm_serdev_probe(struct serdev_device *serdev)
+{
+	struct bcm_serdev *bcmdev;
+	u32 speed;
+	int err;
+
+	bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
+	if (!bcmdev)
+		return -ENOMEM;
+
+	bcmdev->hu.serdev = serdev;
+	serdev_device_set_drvdata(serdev, bcmdev);
+
+	err = device_property_read_u32(&serdev->dev, "max-speed", &speed);
+	if (!err)
+		bcmdev->hu.oper_speed = speed;
+
+	return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
+}
+
+static void bcm_serdev_remove(struct serdev_device *serdev)
+{
+	struct bcm_serdev *bcmdev = serdev_device_get_drvdata(serdev);
+
+	hci_uart_unregister_device(&bcmdev->hu);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id bcm_bluetooth_of_match[] = {
+	{ .compatible = "brcm,bcm43438-bt" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
+#endif
+
+static struct serdev_device_driver bcm_serdev_driver = {
+	.probe = bcm_serdev_probe,
+	.remove = bcm_serdev_remove,
+	.driver = {
+		.name = "hci_uart_bcm",
+		.of_match_table = of_match_ptr(bcm_bluetooth_of_match),
+	},
+};
+
 int __init bcm_init(void)
 {
+	/* For now, we need to keep both platform device
+	 * driver (ACPI generated) and serdev driver (DT).
+	 */
 	platform_driver_register(&bcm_driver);
+	serdev_device_driver_register(&bcm_serdev_driver);
 
 	return hci_uart_register_proto(&bcm_proto);
 }
@@ -913,6 +993,7 @@ int __init bcm_init(void)
 int __exit bcm_deinit(void)
 {
 	platform_driver_unregister(&bcm_driver);
+	serdev_device_driver_unregister(&bcm_serdev_driver);
 
 	return hci_uart_unregister_proto(&bcm_proto);
 }
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* [PATCH v3 3/3] Bluetooth: hci_bcm: Add serdev support
@ 2017-08-07 10:39     ` Loic Poulain
  0 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-07 10:39 UTC (permalink / raw)
  To: marcel, johan.hedberg, robh+dt, rjui, sbranden, f.fainelli,
	stefan.wahren
  Cc: devicetree, linux-bluetooth, linux-rpi-kernel, Loic Poulain

Add basic support for Broadcom serial slave devices.
Probe the serial device, retrieve its maximum speed and
register a new hci uart device.

Tested/compatible with bcm43438 (RPi3).

Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
---
 drivers/bluetooth/hci_bcm.c | 85 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 83 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/hci_bcm.c b/drivers/bluetooth/hci_bcm.c
index 6b42372..086d73c 100644
--- a/drivers/bluetooth/hci_bcm.c
+++ b/drivers/bluetooth/hci_bcm.c
@@ -27,6 +27,8 @@
 #include <linux/firmware.h>
 #include <linux/module.h>
 #include <linux/acpi.h>
+#include <linux/of.h>
+#include <linux/property.h>
 #include <linux/platform_device.h>
 #include <linux/clk.h>
 #include <linux/gpio/consumer.h>
@@ -34,6 +36,7 @@
 #include <linux/interrupt.h>
 #include <linux/dmi.h>
 #include <linux/pm_runtime.h>
+#include <linux/serdev.h>
 
 #include <net/bluetooth/bluetooth.h>
 #include <net/bluetooth/hci_core.h>
@@ -46,6 +49,7 @@
 
 #define BCM_AUTOSUSPEND_DELAY	5000 /* default autosleep delay */
 
+/* platform device driver resources */
 struct bcm_device {
 	struct list_head	list;
 
@@ -68,6 +72,12 @@ struct bcm_device {
 #endif
 };
 
+/* serdev driver resources */
+struct bcm_serdev {
+	struct hci_uart hu;
+};
+
+/* generic bcm uart resources */
 struct bcm_data {
 	struct sk_buff		*rx_skb;
 	struct sk_buff_head	txq;
@@ -79,6 +89,14 @@ struct bcm_data {
 static DEFINE_MUTEX(bcm_device_lock);
 static LIST_HEAD(bcm_device_list);
 
+static inline void host_set_baudrate(struct hci_uart *hu, unsigned int speed)
+{
+	if (hu->serdev)
+		serdev_device_set_baudrate(hu->serdev, speed);
+	else
+		hci_uart_set_baudrate(hu, speed);
+}
+
 static int bcm_set_baudrate(struct hci_uart *hu, unsigned int speed)
 {
 	struct hci_dev *hdev = hu->hdev;
@@ -289,6 +307,14 @@ static int bcm_open(struct hci_uart *hu)
 
 	hu->priv = bcm;
 
+	/* If this is a serdev defined device, then only use
+	 * serdev open primitive and skip the rest.
+	 */
+	if (hu->serdev) {
+		serdev_device_open(hu->serdev);
+		goto out;
+	}
+
 	if (!hu->tty->dev)
 		goto out;
 
@@ -323,6 +349,12 @@ static int bcm_close(struct hci_uart *hu)
 
 	bt_dev_dbg(hu->hdev, "hu %p", hu);
 
+	/* If this is a serdev defined device, only use serdev
+	 * close primitive and then continue as usual.
+	 */
+	if (hu->serdev)
+		serdev_device_close(hu->serdev);
+
 	/* Protect bcm->dev against removal of the device or driver */
 	mutex_lock(&bcm_device_lock);
 	if (bcm_device_exists(bdev)) {
@@ -398,7 +430,7 @@ static int bcm_setup(struct hci_uart *hu)
 		speed = 0;
 
 	if (speed)
-		hci_uart_set_baudrate(hu, speed);
+		host_set_baudrate(hu, speed);
 
 	/* Operational speed if any */
 	if (hu->oper_speed)
@@ -411,7 +443,7 @@ static int bcm_setup(struct hci_uart *hu)
 	if (speed) {
 		err = bcm_set_baudrate(hu, speed);
 		if (!err)
-			hci_uart_set_baudrate(hu, speed);
+			host_set_baudrate(hu, speed);
 	}
 
 finalize:
@@ -903,9 +935,57 @@ static int bcm_remove(struct platform_device *pdev)
 	},
 };
 
+static int bcm_serdev_probe(struct serdev_device *serdev)
+{
+	struct bcm_serdev *bcmdev;
+	u32 speed;
+	int err;
+
+	bcmdev = devm_kzalloc(&serdev->dev, sizeof(*bcmdev), GFP_KERNEL);
+	if (!bcmdev)
+		return -ENOMEM;
+
+	bcmdev->hu.serdev = serdev;
+	serdev_device_set_drvdata(serdev, bcmdev);
+
+	err = device_property_read_u32(&serdev->dev, "max-speed", &speed);
+	if (!err)
+		bcmdev->hu.oper_speed = speed;
+
+	return hci_uart_register_device(&bcmdev->hu, &bcm_proto);
+}
+
+static void bcm_serdev_remove(struct serdev_device *serdev)
+{
+	struct bcm_serdev *bcmdev = serdev_device_get_drvdata(serdev);
+
+	hci_uart_unregister_device(&bcmdev->hu);
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id bcm_bluetooth_of_match[] = {
+	{ .compatible = "brcm,bcm43438-bt" },
+	{ },
+};
+MODULE_DEVICE_TABLE(of, bcm_bluetooth_of_match);
+#endif
+
+static struct serdev_device_driver bcm_serdev_driver = {
+	.probe = bcm_serdev_probe,
+	.remove = bcm_serdev_remove,
+	.driver = {
+		.name = "hci_uart_bcm",
+		.of_match_table = of_match_ptr(bcm_bluetooth_of_match),
+	},
+};
+
 int __init bcm_init(void)
 {
+	/* For now, we need to keep both platform device
+	 * driver (ACPI generated) and serdev driver (DT).
+	 */
 	platform_driver_register(&bcm_driver);
+	serdev_device_driver_register(&bcm_serdev_driver);
 
 	return hci_uart_register_proto(&bcm_proto);
 }
@@ -913,6 +993,7 @@ int __init bcm_init(void)
 int __exit bcm_deinit(void)
 {
 	platform_driver_unregister(&bcm_driver);
+	serdev_device_driver_unregister(&bcm_serdev_driver);
 
 	return hci_uart_unregister_proto(&bcm_proto);
 }
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-07 10:39     ` Loic Poulain
@ 2017-08-09 23:10         ` Rob Herring
  -1 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2017-08-09 23:10 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Marcel Holtmann, Johan Hedberg, Ray Jui, Scott Branden,
	Florian Fainelli, Stefan Wahren,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, Aug 7, 2017 at 5:39 AM, Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
> This allows to automatically insert the bcm43438 to the bluetooth
> subsystem instead of relying on userspace helpers (hciattach).
>
> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
> of pl011/ttyAMA0.
>
> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  v2: dt-bindings as separate patch
>      rebase on upcoming pi3 dts changes
>  v3: changes in bcm serdev drivers:
>      name refactoring and additional comments
>      Add generic host_set_baudrate method
>      Use agnostic device_property_read
>
>  arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> index 20725ca..5abc1df 100644
> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> @@ -8,6 +8,11 @@
>         compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>         model = "Raspberry Pi 3 Model B";
>
> +       chosen {
> +               /* 8250 auxiliar UART instead of pl011 */
> +               bootargs = "earlyprintk console=ttyS0,115200";

This is an unrelated change. "earlyprintk" is arm32 specific and only
works with a kernel built with a specific uart type and address. Also,
stdout-path property is preferred to set the default over "console".

> +       };
> +
>         memory {
>                 reg = <0 0x40000000>;
>         };
> @@ -24,6 +29,11 @@
>         pinctrl-names = "default";
>         pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
>         status = "okay";
> +
> +       bluetooth {
> +               compatible = "brcm,bcm43438-bt";
> +               max-speed = <921600>;
> +       };
>  };
>
>  /* uart1 is mapped to the pin header */
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-09 23:10         ` Rob Herring
  0 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2017-08-09 23:10 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Marcel Holtmann, Johan Hedberg, Ray Jui, Scott Branden,
	Florian Fainelli, Stefan Wahren, devicetree,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel

On Mon, Aug 7, 2017 at 5:39 AM, Loic Poulain <loic.poulain@gmail.com> wrote:
> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
> This allows to automatically insert the bcm43438 to the bluetooth
> subsystem instead of relying on userspace helpers (hciattach).
>
> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
> of pl011/ttyAMA0.
>
> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
> ---
>  v2: dt-bindings as separate patch
>      rebase on upcoming pi3 dts changes
>  v3: changes in bcm serdev drivers:
>      name refactoring and additional comments
>      Add generic host_set_baudrate method
>      Use agnostic device_property_read
>
>  arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> index 20725ca..5abc1df 100644
> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> @@ -8,6 +8,11 @@
>         compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>         model = "Raspberry Pi 3 Model B";
>
> +       chosen {
> +               /* 8250 auxiliar UART instead of pl011 */
> +               bootargs = "earlyprintk console=ttyS0,115200";

This is an unrelated change. "earlyprintk" is arm32 specific and only
works with a kernel built with a specific uart type and address. Also,
stdout-path property is preferred to set the default over "console".

> +       };
> +
>         memory {
>                 reg = <0 0x40000000>;
>         };
> @@ -24,6 +29,11 @@
>         pinctrl-names = "default";
>         pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
>         status = "okay";
> +
> +       bluetooth {
> +               compatible = "brcm,bcm43438-bt";
> +               max-speed = <921600>;
> +       };
>  };
>
>  /* uart1 is mapped to the pin header */
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth
  2017-08-07 10:39 ` Loic Poulain
@ 2017-08-09 23:16     ` Rob Herring
  -1 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2017-08-09 23:16 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Marcel Holtmann, Johan Hedberg, Ray Jui, Scott Branden,
	Florian Fainelli, Stefan Wahren,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Mon, Aug 7, 2017 at 5:39 AM, Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Add binding document for serial bluetooth chips using
> Broadcom protocol.
>
> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
>  v2: dt-bindings as separate patch
>      rebase on upcoming pi3 dts changes
>  v3: changes in bcm serdev drivers:
>      name refactoring and additional comments
>      Add generic host_set_baudrate method
>      Use agnostic device_property_read
>  .../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> new file mode 100644
> index 0000000..c51ea1b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> @@ -0,0 +1,29 @@
> +Broadcom Bluetooth Chips
> +---------------------
> +
> +This documents the binding structure and common properties for serial
> +attached Broadcom devices.
> +
> +Serial attached Broadcom devices shall be a child node of the host UART
> +device the slave device is attached to.
> +
> +Required properties:
> +
> + - compatible: should contain one of the following:
> +   * "brcm,bcm43438-bt"
> +
> +Optional properties:

Most Broadcom devices have a couple of GPIOs needing control. Maybe
they are tied off active on RPi3, but you should document them here
even if the driver doesn't yet need them. I think they are the same as
the Nokia BT IIRC. Same goes for any input clocks.

> +
> + - max-speed: see Documentation/devicetree/bindings/serial/slave-device.txt
> +
> +Example:
> +
> +&uart2 {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&uart2_pins>;
> +
> +       bluetooth {
> +               compatible = "brcm,bcm43438-bt";
> +               max-speed = <921600>;
> +       };
> +};
> --
> 1.9.1
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth
@ 2017-08-09 23:16     ` Rob Herring
  0 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2017-08-09 23:16 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Marcel Holtmann, Johan Hedberg, Ray Jui, Scott Branden,
	Florian Fainelli, Stefan Wahren, devicetree,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel

On Mon, Aug 7, 2017 at 5:39 AM, Loic Poulain <loic.poulain@gmail.com> wrote:
> Add binding document for serial bluetooth chips using
> Broadcom protocol.
>
> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
> ---
>  v2: dt-bindings as separate patch
>      rebase on upcoming pi3 dts changes
>  v3: changes in bcm serdev drivers:
>      name refactoring and additional comments
>      Add generic host_set_baudrate method
>      Use agnostic device_property_read
>  .../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>
> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> new file mode 100644
> index 0000000..c51ea1b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
> @@ -0,0 +1,29 @@
> +Broadcom Bluetooth Chips
> +---------------------
> +
> +This documents the binding structure and common properties for serial
> +attached Broadcom devices.
> +
> +Serial attached Broadcom devices shall be a child node of the host UART
> +device the slave device is attached to.
> +
> +Required properties:
> +
> + - compatible: should contain one of the following:
> +   * "brcm,bcm43438-bt"
> +
> +Optional properties:

Most Broadcom devices have a couple of GPIOs needing control. Maybe
they are tied off active on RPi3, but you should document them here
even if the driver doesn't yet need them. I think they are the same as
the Nokia BT IIRC. Same goes for any input clocks.

> +
> + - max-speed: see Documentation/devicetree/bindings/serial/slave-device.txt
> +
> +Example:
> +
> +&uart2 {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&uart2_pins>;
> +
> +       bluetooth {
> +               compatible = "brcm,bcm43438-bt";
> +               max-speed = <921600>;
> +       };
> +};
> --
> 1.9.1
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-09 23:10         ` Rob Herring
@ 2017-08-10 16:15             ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-10 16:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: Loic Poulain, Johan Hedberg, Ray Jui, Scott Branden,
	Florian Fainelli, Stefan Wahren,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Rob,

>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>> This allows to automatically insert the bcm43438 to the bluetooth
>> subsystem instead of relying on userspace helpers (hciattach).
>> 
>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>> of pl011/ttyAMA0.
>> 
>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> v2: dt-bindings as separate patch
>>     rebase on upcoming pi3 dts changes
>> v3: changes in bcm serdev drivers:
>>     name refactoring and additional comments
>>     Add generic host_set_baudrate method
>>     Use agnostic device_property_read
>> 
>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> index 20725ca..5abc1df 100644
>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> @@ -8,6 +8,11 @@
>>        compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>        model = "Raspberry Pi 3 Model B";
>> 
>> +       chosen {
>> +               /* 8250 auxiliar UART instead of pl011 */
>> +               bootargs = "earlyprintk console=ttyS0,115200";
> 
> This is an unrelated change. "earlyprintk" is arm32 specific and only
> works with a kernel built with a specific uart type and address. Also,
> stdout-path property is preferred to set the default over "console”.

the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.

Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.

Regards

Marcel

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-10 16:15             ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-10 16:15 UTC (permalink / raw)
  To: Rob Herring
  Cc: Loic Poulain, Johan Hedberg, Ray Jui, Scott Branden,
	Florian Fainelli, Stefan Wahren, devicetree,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel

Hi Rob,

>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>> This allows to automatically insert the bcm43438 to the bluetooth
>> subsystem instead of relying on userspace helpers (hciattach).
>> 
>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>> of pl011/ttyAMA0.
>> 
>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>> ---
>> v2: dt-bindings as separate patch
>>     rebase on upcoming pi3 dts changes
>> v3: changes in bcm serdev drivers:
>>     name refactoring and additional comments
>>     Add generic host_set_baudrate method
>>     Use agnostic device_property_read
>> 
>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>> 
>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> index 20725ca..5abc1df 100644
>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> @@ -8,6 +8,11 @@
>>        compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>        model = "Raspberry Pi 3 Model B";
>> 
>> +       chosen {
>> +               /* 8250 auxiliar UART instead of pl011 */
>> +               bootargs = "earlyprintk console=ttyS0,115200";
> 
> This is an unrelated change. "earlyprintk" is arm32 specific and only
> works with a kernel built with a specific uart type and address. Also,
> stdout-path property is preferred to set the default over "console”.

the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.

Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-10 16:15             ` Marcel Holtmann
@ 2017-08-14 13:56                 ` Loic Poulain
  -1 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-14 13:56 UTC (permalink / raw)
  To: Marcel Holtmann, Rob Herring, Stefan Wahren
  Cc: Johan Hedberg, Ray Jui, Scott Branden, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Rob, Marcel,


> Hi Rob,
>
>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>> This allows to automatically insert the bcm43438 to the bluetooth
>>> subsystem instead of relying on userspace helpers (hciattach).
>>>
>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>> of pl011/ttyAMA0.
>>>
>>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>> ---
>>> v2: dt-bindings as separate patch
>>>      rebase on upcoming pi3 dts changes
>>> v3: changes in bcm serdev drivers:
>>>      name refactoring and additional comments
>>>      Add generic host_set_baudrate method
>>>      Use agnostic device_property_read
>>>
>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>> index 20725ca..5abc1df 100644
>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>> @@ -8,6 +8,11 @@
>>>         compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>         model = "Raspberry Pi 3 Model B";
>>>
>>> +       chosen {
>>> +               /* 8250 auxiliar UART instead of pl011 */
>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>> works with a kernel built with a specific uart type and address. Also,
>> stdout-path property is preferred to set the default over "console”.
I'm going to use stdout-path according to your comment.
However one of the goals was to overwrite the bcm283x.dtsi bootargs,
I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
which should require update too.

> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>
> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>

It is also possible to mux the auxiliary mini UART to drive the Bluetooth
controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
however SoC spec says that this mini UART is intented to be used mainly as
a console due to shallow FIFOs and no DMA.

Regards,
Loic

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-14 13:56                 ` Loic Poulain
  0 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-14 13:56 UTC (permalink / raw)
  To: Marcel Holtmann, Rob Herring, Stefan Wahren
  Cc: Johan Hedberg, Ray Jui, Scott Branden, Florian Fainelli,
	devicetree, open list:BLUETOOTH DRIVERS, linux-rpi-kernel

Hi Rob, Marcel,


> Hi Rob,
>
>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>> This allows to automatically insert the bcm43438 to the bluetooth
>>> subsystem instead of relying on userspace helpers (hciattach).
>>>
>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>> of pl011/ttyAMA0.
>>>
>>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>>> ---
>>> v2: dt-bindings as separate patch
>>>      rebase on upcoming pi3 dts changes
>>> v3: changes in bcm serdev drivers:
>>>      name refactoring and additional comments
>>>      Add generic host_set_baudrate method
>>>      Use agnostic device_property_read
>>>
>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>> 1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>> index 20725ca..5abc1df 100644
>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>> @@ -8,6 +8,11 @@
>>>         compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>         model = "Raspberry Pi 3 Model B";
>>>
>>> +       chosen {
>>> +               /* 8250 auxiliar UART instead of pl011 */
>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>> works with a kernel built with a specific uart type and address. Also,
>> stdout-path property is preferred to set the default over "console”.
I'm going to use stdout-path according to your comment.
However one of the goals was to overwrite the bcm283x.dtsi bootargs,
I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
which should require update too.

> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>
> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>

It is also possible to mux the auxiliary mini UART to drive the Bluetooth
controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
however SoC spec says that this mini UART is intented to be used mainly as
a console due to shallow FIFOs and no DMA.

Regards,
Loic

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-14 13:56                 ` Loic Poulain
@ 2017-08-14 16:02                     ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-14 16:02 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Loic,

>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>> 
>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>> of pl011/ttyAMA0.
>>>> 
>>>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>> ---
>>>> v2: dt-bindings as separate patch
>>>>     rebase on upcoming pi3 dts changes
>>>> v3: changes in bcm serdev drivers:
>>>>     name refactoring and additional comments
>>>>     Add generic host_set_baudrate method
>>>>     Use agnostic device_property_read
>>>> 
>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>> 1 file changed, 10 insertions(+)
>>>> 
>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>> index 20725ca..5abc1df 100644
>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>> @@ -8,6 +8,11 @@
>>>>        compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>        model = "Raspberry Pi 3 Model B";
>>>> 
>>>> +       chosen {
>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>> works with a kernel built with a specific uart type and address. Also,
>>> stdout-path property is preferred to set the default over "console”.
> I'm going to use stdout-path according to your comment.
> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
> which should require update too.
> 
>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>> 
>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>> 
> 
> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
> however SoC spec says that this mini UART is intented to be used mainly as
> a console due to shallow FIFOs and no DMA.

what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-14 16:02                     ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-14 16:02 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli, devicetree,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel

Hi Loic,

>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>> 
>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>> of pl011/ttyAMA0.
>>>> 
>>>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>>>> ---
>>>> v2: dt-bindings as separate patch
>>>>     rebase on upcoming pi3 dts changes
>>>> v3: changes in bcm serdev drivers:
>>>>     name refactoring and additional comments
>>>>     Add generic host_set_baudrate method
>>>>     Use agnostic device_property_read
>>>> 
>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>> 1 file changed, 10 insertions(+)
>>>> 
>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>> index 20725ca..5abc1df 100644
>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>> @@ -8,6 +8,11 @@
>>>>        compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>        model = "Raspberry Pi 3 Model B";
>>>> 
>>>> +       chosen {
>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>> works with a kernel built with a specific uart type and address. Also,
>>> stdout-path property is preferred to set the default over "console”.
> I'm going to use stdout-path according to your comment.
> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
> which should require update too.
> 
>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>> 
>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>> 
> 
> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
> however SoC spec says that this mini UART is intented to be used mainly as
> a console due to shallow FIFOs and no DMA.

what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-14 16:02                     ` Marcel Holtmann
@ 2017-08-14 17:32                         ` Loic Poulain
  -1 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-14 17:32 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r



On 14/08/2017 18:02, Marcel Holtmann wrote:
> Hi Loic,
>
>>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>>>
>>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>>> of pl011/ttyAMA0.
>>>>>
>>>>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>> ---
>>>>> v2: dt-bindings as separate patch
>>>>>      rebase on upcoming pi3 dts changes
>>>>> v3: changes in bcm serdev drivers:
>>>>>      name refactoring and additional comments
>>>>>      Add generic host_set_baudrate method
>>>>>      Use agnostic device_property_read
>>>>>
>>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>>> 1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>> index 20725ca..5abc1df 100644
>>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>> @@ -8,6 +8,11 @@
>>>>>         compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>>         model = "Raspberry Pi 3 Model B";
>>>>>
>>>>> +       chosen {
>>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>>> works with a kernel built with a specific uart type and address. Also,
>>>> stdout-path property is preferred to set the default over "console”.
>> I'm going to use stdout-path according to your comment.
>> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
>> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
>> which should require update too.
>>
>>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>>>
>>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>>>
>> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
>> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
>> however SoC spec says that this mini UART is intented to be used mainly as
>> a console due to shallow FIFOs and no DMA.
> what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
>
> Regards
>
> Marcel
>

I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 
4.13-rc3).
Using 32-bit multi_v7_defconfig + some additional confs to enable 
miniuart (BCM2835AUX...) for console.
I push this on a raspbian image.

my boot/config.txt is very simple (no overlay):

kernel=zImage
device_tree=bcm2837-rpi-3-b.dtb
# hack for miniuart serial clock
core_freq=250
dtparam=audio=on

Regards,
Loic



--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-14 17:32                         ` Loic Poulain
  0 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-14 17:32 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli, devicetree,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel



On 14/08/2017 18:02, Marcel Holtmann wrote:
> Hi Loic,
>
>>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>>>
>>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>>> of pl011/ttyAMA0.
>>>>>
>>>>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>>>>> ---
>>>>> v2: dt-bindings as separate patch
>>>>>      rebase on upcoming pi3 dts changes
>>>>> v3: changes in bcm serdev drivers:
>>>>>      name refactoring and additional comments
>>>>>      Add generic host_set_baudrate method
>>>>>      Use agnostic device_property_read
>>>>>
>>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>>> 1 file changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>> index 20725ca..5abc1df 100644
>>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>> @@ -8,6 +8,11 @@
>>>>>         compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>>         model = "Raspberry Pi 3 Model B";
>>>>>
>>>>> +       chosen {
>>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>>> works with a kernel built with a specific uart type and address. Also,
>>>> stdout-path property is preferred to set the default over "console”.
>> I'm going to use stdout-path according to your comment.
>> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
>> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
>> which should require update too.
>>
>>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>>>
>>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>>>
>> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
>> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
>> however SoC spec says that this mini UART is intented to be used mainly as
>> a console due to shallow FIFOs and no DMA.
> what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
>
> Regards
>
> Marcel
>

I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 
4.13-rc3).
Using 32-bit multi_v7_defconfig + some additional confs to enable 
miniuart (BCM2835AUX...) for console.
I push this on a raspbian image.

my boot/config.txt is very simple (no overlay):

kernel=zImage
device_tree=bcm2837-rpi-3-b.dtb
# hack for miniuart serial clock
core_freq=250
dtparam=audio=on

Regards,
Loic

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-14 17:32                         ` Loic Poulain
@ 2017-08-14 17:39                             ` Stefan Wahren
  -1 siblings, 0 replies; 50+ messages in thread
From: Stefan Wahren @ 2017-08-14 17:39 UTC (permalink / raw)
  To: Marcel Holtmann, Loic Poulain
  Cc: Ray Jui, Scott Branden, Rob Herring, Florian Fainelli,
	open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Johan Hedberg,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Loic,

> Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> hat am 14. August 2017 um 19:32 geschrieben:
> 
> 
> 
> 
> On 14/08/2017 18:02, Marcel Holtmann wrote:
> > Hi Loic,
> >
> >>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
> >>>>> This allows to automatically insert the bcm43438 to the bluetooth
> >>>>> subsystem instead of relying on userspace helpers (hciattach).
> >>>>>
> >>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
> >>>>> of pl011/ttyAMA0.
> >>>>>
> >>>>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> >>>>> ---
> >>>>> v2: dt-bindings as separate patch
> >>>>>      rebase on upcoming pi3 dts changes
> >>>>> v3: changes in bcm serdev drivers:
> >>>>>      name refactoring and additional comments
> >>>>>      Add generic host_set_baudrate method
> >>>>>      Use agnostic device_property_read
> >>>>>
> >>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
> >>>>> 1 file changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> >>>>> index 20725ca..5abc1df 100644
> >>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> >>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> >>>>> @@ -8,6 +8,11 @@
> >>>>>         compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
> >>>>>         model = "Raspberry Pi 3 Model B";
> >>>>>
> >>>>> +       chosen {
> >>>>> +               /* 8250 auxiliar UART instead of pl011 */
> >>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
> >>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
> >>>> works with a kernel built with a specific uart type and address. Also,
> >>>> stdout-path property is preferred to set the default over "console”.
> >> I'm going to use stdout-path according to your comment.
> >> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
> >> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
> >> which should require update too.
> >>
> >>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
> >>>
> >>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
> >>>
> >> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
> >> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
> >> however SoC spec says that this mini UART is intented to be used mainly as
> >> a console due to shallow FIFOs and no DMA.
> > what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
> >
> > Regards
> >
> > Marcel
> >
> 
> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 
> 4.13-rc3).
> Using 32-bit multi_v7_defconfig + some additional confs to enable 
> miniuart (BCM2835AUX...) for console.
> I push this on a raspbian image.
> 
> my boot/config.txt is very simple (no overlay):
> 
> kernel=zImage
> device_tree=bcm2837-rpi-3-b.dtb
> # hack for miniuart serial clock
> core_freq=250
> dtparam=audio=on
> 

since the BCM2837 is a 64 bit platform, could you please test also this case?

Thanks
Stefan

> Regards,
> Loic
> 
> 
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-14 17:39                             ` Stefan Wahren
  0 siblings, 0 replies; 50+ messages in thread
From: Stefan Wahren @ 2017-08-14 17:39 UTC (permalink / raw)
  To: Marcel Holtmann, Loic Poulain
  Cc: Ray Jui, Scott Branden, Rob Herring, Florian Fainelli,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel, Johan Hedberg,
	devicetree

Hi Loic,

> Loic Poulain <loic.poulain@gmail.com> hat am 14. August 2017 um 19:32 ges=
chrieben:
>=20
>=20
>=20
>=20
> On 14/08/2017 18:02, Marcel Holtmann wrote:
> > Hi Loic,
> >
> >>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
> >>>>> This allows to automatically insert the bcm43438 to the bluetooth
> >>>>> subsystem instead of relying on userspace helpers (hciattach).
> >>>>>
> >>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
> >>>>> of pl011/ttyAMA0.
> >>>>>
> >>>>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
> >>>>> ---
> >>>>> v2: dt-bindings as separate patch
> >>>>>      rebase on upcoming pi3 dts changes
> >>>>> v3: changes in bcm serdev drivers:
> >>>>>      name refactoring and additional comments
> >>>>>      Add generic host_set_baudrate method
> >>>>>      Use agnostic device_property_read
> >>>>>
> >>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
> >>>>> 1 file changed, 10 insertions(+)
> >>>>>
> >>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/=
dts/bcm2837-rpi-3-b.dts
> >>>>> index 20725ca..5abc1df 100644
> >>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> >>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> >>>>> @@ -8,6 +8,11 @@
> >>>>>         compatible =3D "raspberrypi,3-model-b", "brcm,bcm2837";
> >>>>>         model =3D "Raspberry Pi 3 Model B";
> >>>>>
> >>>>> +       chosen {
> >>>>> +               /* 8250 auxiliar UART instead of pl011 */
> >>>>> +               bootargs =3D "earlyprintk console=3DttyS0,115200";
> >>>> This is an unrelated change. "earlyprintk" is arm32 specific and onl=
y
> >>>> works with a kernel built with a specific uart type and address. Als=
o,
> >>>> stdout-path property is preferred to set the default over "console=
=E2=80=9D.
> >> I'm going to use stdout-path according to your comment.
> >> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
> >> I assume the same comment apply on the dtsi (stdpath usage /earlyprint=
k)
> >> which should require update too.
> >>
> >>> the whole fun with the serial console on the rPI3 and the bt-miniuart=
 overlay is something we should solve now. What is the upstream story on th=
is since the config.txt and everything around it is confusing and also misl=
eading since it relies on a Raspbian userspace.
> >>>
> >>> Do we need to the kernel and init to stay away from the ttyAMA0 to av=
oid confusing the BT chip? As I mentioned earlier, I can not even get a Fed=
ora 26 with hciattach or btattach to work.
> >>>
> >> It is also possible to mux the auxiliary mini UART to drive the Blueto=
oth
> >> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/conso=
le,
> >> however SoC spec says that this mini UART is intented to be used mainl=
y as
> >> a console due to shallow FIFOs and no DMA.
> > what upstream kernel tree did you test this with? On a Fedora 26 kernel=
 (now 4.12 based), I still do not have the Bluetooth hardware working.
> >
> > Regards
> >
> > Marcel
> >
>=20
> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and=20
> 4.13-rc3).
> Using 32-bit multi_v7_defconfig + some additional confs to enable=20
> miniuart (BCM2835AUX...) for console.
> I push this on a raspbian image.
>=20
> my boot/config.txt is very simple (no overlay):
>=20
> kernel=3DzImage
> device_tree=3Dbcm2837-rpi-3-b.dtb
> # hack for miniuart serial clock
> core_freq=3D250
> dtparam=3Daudio=3Don
>=20

since the BCM2837 is a 64 bit platform, could you please test also this cas=
e?

Thanks
Stefan

> Regards,
> Loic
>=20
>=20
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-14 17:32                         ` Loic Poulain
@ 2017-08-14 18:36                             ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-14 18:36 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Loic,

>>>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>>>> 
>>>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>>>> of pl011/ttyAMA0.
>>>>>> 
>>>>>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> ---
>>>>>> v2: dt-bindings as separate patch
>>>>>>     rebase on upcoming pi3 dts changes
>>>>>> v3: changes in bcm serdev drivers:
>>>>>>     name refactoring and additional comments
>>>>>>     Add generic host_set_baudrate method
>>>>>>     Use agnostic device_property_read
>>>>>> 
>>>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>>>> 1 file changed, 10 insertions(+)
>>>>>> 
>>>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> index 20725ca..5abc1df 100644
>>>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> @@ -8,6 +8,11 @@
>>>>>>        compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>>>        model = "Raspberry Pi 3 Model B";
>>>>>> 
>>>>>> +       chosen {
>>>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>>>> works with a kernel built with a specific uart type and address. Also,
>>>>> stdout-path property is preferred to set the default over "console”.
>>> I'm going to use stdout-path according to your comment.
>>> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
>>> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
>>> which should require update too.
>>> 
>>>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>>>> 
>>>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>>>> 
>>> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
>>> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
>>> however SoC spec says that this mini UART is intented to be used mainly as
>>> a console due to shallow FIFOs and no DMA.
>> what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
>> 
>> Regards
>> 
>> Marcel
>> 
> 
> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
> I push this on a raspbian image.
> 
> my boot/config.txt is very simple (no overlay):
> 
> kernel=zImage
> device_tree=bcm2837-rpi-3-b.dtb
> # hack for miniuart serial clock
> core_freq=250
> dtparam=audio=on

I really wonder if the Fedora 26 (which actually uses u-boot as kernel) messes up the DT somehow.

Regards

Marcel

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-14 18:36                             ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-14 18:36 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli, devicetree,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel

Hi Loic,

>>>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>>>> 
>>>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>>>> of pl011/ttyAMA0.
>>>>>> 
>>>>>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>>>>>> ---
>>>>>> v2: dt-bindings as separate patch
>>>>>>     rebase on upcoming pi3 dts changes
>>>>>> v3: changes in bcm serdev drivers:
>>>>>>     name refactoring and additional comments
>>>>>>     Add generic host_set_baudrate method
>>>>>>     Use agnostic device_property_read
>>>>>> 
>>>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>>>> 1 file changed, 10 insertions(+)
>>>>>> 
>>>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> index 20725ca..5abc1df 100644
>>>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> @@ -8,6 +8,11 @@
>>>>>>        compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>>>        model = "Raspberry Pi 3 Model B";
>>>>>> 
>>>>>> +       chosen {
>>>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>>>> works with a kernel built with a specific uart type and address. Also,
>>>>> stdout-path property is preferred to set the default over "console”.
>>> I'm going to use stdout-path according to your comment.
>>> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
>>> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
>>> which should require update too.
>>> 
>>>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>>>> 
>>>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>>>> 
>>> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
>>> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
>>> however SoC spec says that this mini UART is intented to be used mainly as
>>> a console due to shallow FIFOs and no DMA.
>> what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
>> 
>> Regards
>> 
>> Marcel
>> 
> 
> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
> I push this on a raspbian image.
> 
> my boot/config.txt is very simple (no overlay):
> 
> kernel=zImage
> device_tree=bcm2837-rpi-3-b.dtb
> # hack for miniuart serial clock
> core_freq=250
> dtparam=audio=on

I really wonder if the Fedora 26 (which actually uses u-boot as kernel) messes up the DT somehow.

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-14 17:32                         ` Loic Poulain
@ 2017-08-14 22:16                             ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-14 22:16 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Loic,

>>>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>>>> 
>>>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>>>> of pl011/ttyAMA0.
>>>>>> 
>>>>>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>> ---
>>>>>> v2: dt-bindings as separate patch
>>>>>>     rebase on upcoming pi3 dts changes
>>>>>> v3: changes in bcm serdev drivers:
>>>>>>     name refactoring and additional comments
>>>>>>     Add generic host_set_baudrate method
>>>>>>     Use agnostic device_property_read
>>>>>> 
>>>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>>>> 1 file changed, 10 insertions(+)
>>>>>> 
>>>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> index 20725ca..5abc1df 100644
>>>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> @@ -8,6 +8,11 @@
>>>>>>        compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>>>        model = "Raspberry Pi 3 Model B";
>>>>>> 
>>>>>> +       chosen {
>>>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>>>> works with a kernel built with a specific uart type and address. Also,
>>>>> stdout-path property is preferred to set the default over "console”.
>>> I'm going to use stdout-path according to your comment.
>>> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
>>> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
>>> which should require update too.
>>> 
>>>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>>>> 
>>>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>>>> 
>>> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
>>> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
>>> however SoC spec says that this mini UART is intented to be used mainly as
>>> a console due to shallow FIFOs and no DMA.
>> what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
>> 
>> Regards
>> 
>> Marcel
>> 
> 
> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
> I push this on a raspbian image.
> 
> my boot/config.txt is very simple (no overlay):
> 
> kernel=zImage
> device_tree=bcm2837-rpi-3-b.dtb
> # hack for miniuart serial clock
> core_freq=250
> dtparam=audio=on

so the Fedora 26 kernel that is based on 4.12 is missing uart0 configuration in DT. Adding it to bcm2837-rpi-3-b.dts will allow for btattach to actually work.

&uart0 {
	pinctrl-names = "default";
	pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
	status = "okay”;
};

It kinda works, but not all of it. This command confuses me.

< HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
        01                                               .               
> HCI Event: Command Complete (0x0e) plen 4
      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
        Status: Unknown HCI Command (0x01)

And I am seeing fun stuff like failed frame assembly.

[  888.687594] Bluetooth: hci0: BCM: chip id 94
[  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
[  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
[  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
[  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182

Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.

Regards

Marcel

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-14 22:16                             ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-14 22:16 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli, devicetree,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel

Hi Loic,

>>>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>>>> 
>>>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>>>> of pl011/ttyAMA0.
>>>>>> 
>>>>>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>>>>>> ---
>>>>>> v2: dt-bindings as separate patch
>>>>>>     rebase on upcoming pi3 dts changes
>>>>>> v3: changes in bcm serdev drivers:
>>>>>>     name refactoring and additional comments
>>>>>>     Add generic host_set_baudrate method
>>>>>>     Use agnostic device_property_read
>>>>>> 
>>>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>>>> 1 file changed, 10 insertions(+)
>>>>>> 
>>>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> index 20725ca..5abc1df 100644
>>>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>> @@ -8,6 +8,11 @@
>>>>>>        compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>>>        model = "Raspberry Pi 3 Model B";
>>>>>> 
>>>>>> +       chosen {
>>>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>>>> works with a kernel built with a specific uart type and address. Also,
>>>>> stdout-path property is preferred to set the default over "console”.
>>> I'm going to use stdout-path according to your comment.
>>> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
>>> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
>>> which should require update too.
>>> 
>>>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>>>> 
>>>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>>>> 
>>> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
>>> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
>>> however SoC spec says that this mini UART is intented to be used mainly as
>>> a console due to shallow FIFOs and no DMA.
>> what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
>> 
>> Regards
>> 
>> Marcel
>> 
> 
> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
> I push this on a raspbian image.
> 
> my boot/config.txt is very simple (no overlay):
> 
> kernel=zImage
> device_tree=bcm2837-rpi-3-b.dtb
> # hack for miniuart serial clock
> core_freq=250
> dtparam=audio=on

so the Fedora 26 kernel that is based on 4.12 is missing uart0 configuration in DT. Adding it to bcm2837-rpi-3-b.dts will allow for btattach to actually work.

&uart0 {
	pinctrl-names = "default";
	pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
	status = "okay”;
};

It kinda works, but not all of it. This command confuses me.

< HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
        01                                               .               
> HCI Event: Command Complete (0x0e) plen 4
      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
        Status: Unknown HCI Command (0x01)

And I am seeing fun stuff like failed frame assembly.

[  888.687594] Bluetooth: hci0: BCM: chip id 94
[  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
[  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
[  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
[  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182

Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-14 22:16                             ` Marcel Holtmann
@ 2017-08-15  4:39                                 ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-15  4:39 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli,
	devicetree-u79uwXL29TY76Z2rM5mHXA, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Loic,

>>>>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>>>>> 
>>>>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>>>>> of pl011/ttyAMA0.
>>>>>>> 
>>>>>>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>>>>>>> ---
>>>>>>> v2: dt-bindings as separate patch
>>>>>>>    rebase on upcoming pi3 dts changes
>>>>>>> v3: changes in bcm serdev drivers:
>>>>>>>    name refactoring and additional comments
>>>>>>>    Add generic host_set_baudrate method
>>>>>>>    Use agnostic device_property_read
>>>>>>> 
>>>>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>>>>> 1 file changed, 10 insertions(+)
>>>>>>> 
>>>>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>>> index 20725ca..5abc1df 100644
>>>>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>>> @@ -8,6 +8,11 @@
>>>>>>>       compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>>>>       model = "Raspberry Pi 3 Model B";
>>>>>>> 
>>>>>>> +       chosen {
>>>>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>>>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>>>>> works with a kernel built with a specific uart type and address. Also,
>>>>>> stdout-path property is preferred to set the default over "console”.
>>>> I'm going to use stdout-path according to your comment.
>>>> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
>>>> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
>>>> which should require update too.
>>>> 
>>>>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>>>>> 
>>>>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>>>>> 
>>>> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
>>>> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
>>>> however SoC spec says that this mini UART is intented to be used mainly as
>>>> a console due to shallow FIFOs and no DMA.
>>> what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
>>> 
>>> Regards
>>> 
>>> Marcel
>>> 
>> 
>> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
>> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
>> I push this on a raspbian image.
>> 
>> my boot/config.txt is very simple (no overlay):
>> 
>> kernel=zImage
>> device_tree=bcm2837-rpi-3-b.dtb
>> # hack for miniuart serial clock
>> core_freq=250
>> dtparam=audio=on
> 
> so the Fedora 26 kernel that is based on 4.12 is missing uart0 configuration in DT. Adding it to bcm2837-rpi-3-b.dts will allow for btattach to actually work.
> 
> &uart0 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
> 	status = "okay”;
> };
> 
> It kinda works, but not all of it. This command confuses me.
> 
> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>        01                                               .               
>> HCI Event: Command Complete (0x0e) plen 4
>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>        Status: Unknown HCI Command (0x01)
> 
> And I am seeing fun stuff like failed frame assembly.
> 
> [  888.687594] Bluetooth: hci0: BCM: chip id 94
> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
> 
> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.

so here is my suspicion. By default the hci_bcm driver tries to enable an UART operational speed of 4000000. However this hardware does not support it and it does not even support the required command to change the clock. That is why things do not work properly with the hci_bcm driver and btattach. I assume that is why you added max-speed 921600 for the serdev support. What I can figure out that a speed up-to 3000000 (as in some of the hciattach examples) will actually work since that doesn’t use the UART Clock setting to change the clock type.

So I wonder if using an oper_speed = 4000000 as default is actually a good idea. Maybe we should only do that if we have found a matching ACPI entry. Or if we get a max-speed entry from DT. Otherwise actually default to 115200.

We also really need to convert the ACPI support to use serdev as well. At least the standard tables list them as BTH0 which we could use to identify them as Bluetooth devices. Or we just need to do it based on the _HID string.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-15  4:39                                 ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-15  4:39 UTC (permalink / raw)
  To: Loic Poulain
  Cc: Rob Herring, Stefan Wahren, Johan Hedberg, Ray Jui,
	Scott Branden, Florian Fainelli, devicetree,
	open list:BLUETOOTH DRIVERS, linux-rpi-kernel

Hi Loic,

>>>>>>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>>>>>>> This allows to automatically insert the bcm43438 to the bluetooth
>>>>>>> subsystem instead of relying on userspace helpers (hciattach).
>>>>>>> 
>>>>>>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>>>>>>> of pl011/ttyAMA0.
>>>>>>> 
>>>>>>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>>>>>>> ---
>>>>>>> v2: dt-bindings as separate patch
>>>>>>>    rebase on upcoming pi3 dts changes
>>>>>>> v3: changes in bcm serdev drivers:
>>>>>>>    name refactoring and additional comments
>>>>>>>    Add generic host_set_baudrate method
>>>>>>>    Use agnostic device_property_read
>>>>>>> 
>>>>>>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>>>>>>> 1 file changed, 10 insertions(+)
>>>>>>> 
>>>>>>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>>> index 20725ca..5abc1df 100644
>>>>>>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>>>>>>> @@ -8,6 +8,11 @@
>>>>>>>       compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>>>>>>       model = "Raspberry Pi 3 Model B";
>>>>>>> 
>>>>>>> +       chosen {
>>>>>>> +               /* 8250 auxiliar UART instead of pl011 */
>>>>>>> +               bootargs = "earlyprintk console=ttyS0,115200";
>>>>>> This is an unrelated change. "earlyprintk" is arm32 specific and only
>>>>>> works with a kernel built with a specific uart type and address. Also,
>>>>>> stdout-path property is preferred to set the default over "console”.
>>>> I'm going to use stdout-path according to your comment.
>>>> However one of the goals was to overwrite the bcm283x.dtsi bootargs,
>>>> I assume the same comment apply on the dtsi (stdpath usage /earlyprintk)
>>>> which should require update too.
>>>> 
>>>>> the whole fun with the serial console on the rPI3 and the bt-miniuart overlay is something we should solve now. What is the upstream story on this since the config.txt and everything around it is confusing and also misleading since it relies on a Raspbian userspace.
>>>>> 
>>>>> Do we need to the kernel and init to stay away from the ttyAMA0 to avoid confusing the BT chip? As I mentioned earlier, I can not even get a Fedora 26 with hciattach or btattach to work.
>>>>> 
>>>> It is also possible to mux the auxiliary mini UART to drive the Bluetooth
>>>> controller in order to keep ttyAMA0 as the "legacy" RPi ext-uart/console,
>>>> however SoC spec says that this mini UART is intented to be used mainly as
>>>> a console due to shallow FIFOs and no DMA.
>>> what upstream kernel tree did you test this with? On a Fedora 26 kernel (now 4.12 based), I still do not have the Bluetooth hardware working.
>>> 
>>> Regards
>>> 
>>> Marcel
>>> 
>> 
>> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
>> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
>> I push this on a raspbian image.
>> 
>> my boot/config.txt is very simple (no overlay):
>> 
>> kernel=zImage
>> device_tree=bcm2837-rpi-3-b.dtb
>> # hack for miniuart serial clock
>> core_freq=250
>> dtparam=audio=on
> 
> so the Fedora 26 kernel that is based on 4.12 is missing uart0 configuration in DT. Adding it to bcm2837-rpi-3-b.dts will allow for btattach to actually work.
> 
> &uart0 {
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
> 	status = "okay”;
> };
> 
> It kinda works, but not all of it. This command confuses me.
> 
> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>        01                                               .               
>> HCI Event: Command Complete (0x0e) plen 4
>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>        Status: Unknown HCI Command (0x01)
> 
> And I am seeing fun stuff like failed frame assembly.
> 
> [  888.687594] Bluetooth: hci0: BCM: chip id 94
> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
> 
> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.

so here is my suspicion. By default the hci_bcm driver tries to enable an UART operational speed of 4000000. However this hardware does not support it and it does not even support the required command to change the clock. That is why things do not work properly with the hci_bcm driver and btattach. I assume that is why you added max-speed 921600 for the serdev support. What I can figure out that a speed up-to 3000000 (as in some of the hciattach examples) will actually work since that doesn’t use the UART Clock setting to change the clock type.

So I wonder if using an oper_speed = 4000000 as default is actually a good idea. Maybe we should only do that if we have found a matching ACPI entry. Or if we get a max-speed entry from DT. Otherwise actually default to 115200.

We also really need to convert the ACPI support to use serdev as well. At least the standard tables list them as BTH0 which we could use to identify them as Bluetooth devices. Or we just need to do it based on the _HID string.

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-07 10:39     ` Loic Poulain
@ 2017-08-15 13:06         ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-15 13:06 UTC (permalink / raw)
  To: Loic Poulain, Rob Herring
  Cc: Johan Hedberg, rjui-dY08KVG/lbpWk0Htik3J/w,
	sbranden-dY08KVG/lbpWk0Htik3J/w,
	f.fainelli-Re5JQEeQqe8AvxtiuMwx3w, Stefan Wahren, devicetree,
	Bluez mailing list,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Rob,

> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
> This allows to automatically insert the bcm43438 to the bluetooth
> subsystem instead of relying on userspace helpers (hciattach).
> 
> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
> of pl011/ttyAMA0.
> 
> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> v2: dt-bindings as separate patch
>     rebase on upcoming pi3 dts changes
> v3: changes in bcm serdev drivers:
>     name refactoring and additional comments
>     Add generic host_set_baudrate method
>     Use agnostic device_property_read
> 
> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> index 20725ca..5abc1df 100644
> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> @@ -8,6 +8,11 @@
> 	compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
> 	model = "Raspberry Pi 3 Model B";
> 
> +	chosen {
> +		/* 8250 auxiliar UART instead of pl011 */
> +		bootargs = "earlyprintk console=ttyS0,115200";
> +	};
> +
> 	memory {
> 		reg = <0 0x40000000>;
> 	};
> @@ -24,6 +29,11 @@
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
> 	status = "okay";
> +
> +	bluetooth {
> +		compatible = "brcm,bcm43438-bt";
> +		max-speed = <921600>;
> +	};
> };

I know that hci_ll.c DT entry also uses max-speed for the naming. Is this something common we should be doing? Essentially it is not really max-speed. It is the operational speed that we are configuring. There is no down negotiation ongoing. Does it make sense to use oper-speed instead?

Are we otherwise fine with compatible string naming?

Loic, the hciattach examples list 3000000 as operational speed. Any reason why we should limit it?

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-15 13:06         ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-15 13:06 UTC (permalink / raw)
  To: Loic Poulain, Rob Herring
  Cc: Johan Hedberg, rjui, sbranden, f.fainelli, Stefan Wahren,
	devicetree, Bluez mailing list, linux-rpi-kernel

Hi Rob,

> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
> This allows to automatically insert the bcm43438 to the bluetooth
> subsystem instead of relying on userspace helpers (hciattach).
> 
> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
> of pl011/ttyAMA0.
> 
> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
> ---
> v2: dt-bindings as separate patch
>     rebase on upcoming pi3 dts changes
> v3: changes in bcm serdev drivers:
>     name refactoring and additional comments
>     Add generic host_set_baudrate method
>     Use agnostic device_property_read
> 
> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
> 1 file changed, 10 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> index 20725ca..5abc1df 100644
> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
> @@ -8,6 +8,11 @@
> 	compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
> 	model = "Raspberry Pi 3 Model B";
> 
> +	chosen {
> +		/* 8250 auxiliar UART instead of pl011 */
> +		bootargs = "earlyprintk console=ttyS0,115200";
> +	};
> +
> 	memory {
> 		reg = <0 0x40000000>;
> 	};
> @@ -24,6 +29,11 @@
> 	pinctrl-names = "default";
> 	pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
> 	status = "okay";
> +
> +	bluetooth {
> +		compatible = "brcm,bcm43438-bt";
> +		max-speed = <921600>;
> +	};
> };

I know that hci_ll.c DT entry also uses max-speed for the naming. Is this something common we should be doing? Essentially it is not really max-speed. It is the operational speed that we are configuring. There is no down negotiation ongoing. Does it make sense to use oper-speed instead?

Are we otherwise fine with compatible string naming?

Loic, the hciattach examples list 3000000 as operational speed. Any reason why we should limit it?

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth
  2017-08-09 23:16     ` Rob Herring
@ 2017-08-15 17:42         ` Eric Anholt
  -1 siblings, 0 replies; 50+ messages in thread
From: Eric Anholt @ 2017-08-15 17:42 UTC (permalink / raw)
  To: Rob Herring, Loic Poulain
  Cc: devicetree@vger.kernel.org, Johan Hedberg, Scott Branden,
	Ray Jui, Marcel Holtmann, open list:BLUETOOTH DRIVERS,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 2256 bytes --]

Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> writes:

> On Mon, Aug 7, 2017 at 5:39 AM, Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> Add binding document for serial bluetooth chips using
>> Broadcom protocol.
>>
>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>>  v2: dt-bindings as separate patch
>>      rebase on upcoming pi3 dts changes
>>  v3: changes in bcm serdev drivers:
>>      name refactoring and additional comments
>>      Add generic host_set_baudrate method
>>      Use agnostic device_property_read
>>  .../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>> new file mode 100644
>> index 0000000..c51ea1b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>> @@ -0,0 +1,29 @@
>> +Broadcom Bluetooth Chips
>> +---------------------
>> +
>> +This documents the binding structure and common properties for serial
>> +attached Broadcom devices.
>> +
>> +Serial attached Broadcom devices shall be a child node of the host UART
>> +device the slave device is attached to.
>> +
>> +Required properties:
>> +
>> + - compatible: should contain one of the following:
>> +   * "brcm,bcm43438-bt"
>> +
>> +Optional properties:
>
> Most Broadcom devices have a couple of GPIOs needing control. Maybe
> they are tied off active on RPi3, but you should document them here
> even if the driver doesn't yet need them. I think they are the same as
> the Nokia BT IIRC. Same goes for any input clocks.

As far as the clock goes, I think it would be a reference to <&cprman
BCM2835_CLOCK_GP2>, and the clock should be at 32Khz.  (This is already
set up at boot by the firmware).

In the schematics I have, there are two GPIO lines going to the chip:
BT_ON (expander gpio 0) and WL_ON (expander gpio 1).  We don't have
support for the expander yet, and they are already enabled at boot time,
so hopefully they can be optional properties.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth
@ 2017-08-15 17:42         ` Eric Anholt
  0 siblings, 0 replies; 50+ messages in thread
From: Eric Anholt @ 2017-08-15 17:42 UTC (permalink / raw)
  To: Rob Herring, Loic Poulain
  Cc: devicetree, Johan Hedberg, Scott Branden, Ray Jui,
	Marcel Holtmann, open list:BLUETOOTH DRIVERS, linux-rpi-kernel

[-- Attachment #1: Type: text/plain, Size: 2167 bytes --]

Rob Herring <robh+dt@kernel.org> writes:

> On Mon, Aug 7, 2017 at 5:39 AM, Loic Poulain <loic.poulain@gmail.com> wrote:
>> Add binding document for serial bluetooth chips using
>> Broadcom protocol.
>>
>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>> ---
>>  v2: dt-bindings as separate patch
>>      rebase on upcoming pi3 dts changes
>>  v3: changes in bcm serdev drivers:
>>      name refactoring and additional comments
>>      Add generic host_set_baudrate method
>>      Use agnostic device_property_read
>>  .../devicetree/bindings/net/broadcom-bluetooth.txt | 29 ++++++++++++++++++++++
>>  1 file changed, 29 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>> new file mode 100644
>> index 0000000..c51ea1b
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/broadcom-bluetooth.txt
>> @@ -0,0 +1,29 @@
>> +Broadcom Bluetooth Chips
>> +---------------------
>> +
>> +This documents the binding structure and common properties for serial
>> +attached Broadcom devices.
>> +
>> +Serial attached Broadcom devices shall be a child node of the host UART
>> +device the slave device is attached to.
>> +
>> +Required properties:
>> +
>> + - compatible: should contain one of the following:
>> +   * "brcm,bcm43438-bt"
>> +
>> +Optional properties:
>
> Most Broadcom devices have a couple of GPIOs needing control. Maybe
> they are tied off active on RPi3, but you should document them here
> even if the driver doesn't yet need them. I think they are the same as
> the Nokia BT IIRC. Same goes for any input clocks.

As far as the clock goes, I think it would be a reference to <&cprman
BCM2835_CLOCK_GP2>, and the clock should be at 32Khz.  (This is already
set up at boot by the firmware).

In the schematics I have, there are two GPIO lines going to the chip:
BT_ON (expander gpio 0) and WL_ON (expander gpio 1).  We don't have
support for the expander yet, and they are already enabled at boot time,
so hopefully they can be optional properties.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-15 13:06         ` Marcel Holtmann
@ 2017-08-15 19:12             ` Rob Herring
  -1 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2017-08-15 19:12 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Loic Poulain, Johan Hedberg, Ray Jui, Scott Branden,
	Florian Fainelli, Stefan Wahren, devicetree, Bluez mailing list,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On Tue, Aug 15, 2017 at 8:06 AM, Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> wrote:
> Hi Rob,
>
>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>> This allows to automatically insert the bcm43438 to the bluetooth
>> subsystem instead of relying on userspace helpers (hciattach).
>>
>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>> of pl011/ttyAMA0.
>>
>> Signed-off-by: Loic Poulain <loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
>> ---
>> v2: dt-bindings as separate patch
>>     rebase on upcoming pi3 dts changes
>> v3: changes in bcm serdev drivers:
>>     name refactoring and additional comments
>>     Add generic host_set_baudrate method
>>     Use agnostic device_property_read
>>
>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> index 20725ca..5abc1df 100644
>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> @@ -8,6 +8,11 @@
>>       compatible = "raspberrypi,3-model-b", "brcm,bcm2837";
>>       model = "Raspberry Pi 3 Model B";
>>
>> +     chosen {
>> +             /* 8250 auxiliar UART instead of pl011 */
>> +             bootargs = "earlyprintk console=ttyS0,115200";
>> +     };
>> +
>>       memory {
>>               reg = <0 0x40000000>;
>>       };
>> @@ -24,6 +29,11 @@
>>       pinctrl-names = "default";
>>       pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
>>       status = "okay";
>> +
>> +     bluetooth {
>> +             compatible = "brcm,bcm43438-bt";
>> +             max-speed = <921600>;
>> +     };
>> };
>
> I know that hci_ll.c DT entry also uses max-speed for the naming. Is this something common we should be doing? Essentially it is not really max-speed. It is the operational speed that we are configuring. There is no down negotiation ongoing. Does it make sense to use oper-speed instead?

max-speed is correct to use. By default, a driver should know the max
speed of a device (implied by the compatible). The DT property is only
for when there is some host or board limitation as this case seems to
be.

> Are we otherwise fine with compatible string naming?

Yes.

Rob

>
> Loic, the hciattach examples list 3000000 as operational speed. Any reason why we should limit it?
>
> Regards
>
> Marcel
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-15 19:12             ` Rob Herring
  0 siblings, 0 replies; 50+ messages in thread
From: Rob Herring @ 2017-08-15 19:12 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Loic Poulain, Johan Hedberg, Ray Jui, Scott Branden,
	Florian Fainelli, Stefan Wahren, devicetree, Bluez mailing list,
	linux-rpi-kernel

On Tue, Aug 15, 2017 at 8:06 AM, Marcel Holtmann <marcel@holtmann.org> wrot=
e:
> Hi Rob,
>
>> Add BCM43438 as a slave device of uart0 (pl011/ttyAMA0).
>> This allows to automatically insert the bcm43438 to the bluetooth
>> subsystem instead of relying on userspace helpers (hciattach).
>>
>> Overwrite bootargs to use 8250 aux uart (ttyS0) as console instead
>> of pl011/ttyAMA0.
>>
>> Signed-off-by: Loic Poulain <loic.poulain@gmail.com>
>> ---
>> v2: dt-bindings as separate patch
>>     rebase on upcoming pi3 dts changes
>> v3: changes in bcm serdev drivers:
>>     name refactoring and additional comments
>>     Add generic host_set_baudrate method
>>     Use agnostic device_property_read
>>
>> arch/arm/boot/dts/bcm2837-rpi-3-b.dts | 10 ++++++++++
>> 1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts b/arch/arm/boot/dts/b=
cm2837-rpi-3-b.dts
>> index 20725ca..5abc1df 100644
>> --- a/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> +++ b/arch/arm/boot/dts/bcm2837-rpi-3-b.dts
>> @@ -8,6 +8,11 @@
>>       compatible =3D "raspberrypi,3-model-b", "brcm,bcm2837";
>>       model =3D "Raspberry Pi 3 Model B";
>>
>> +     chosen {
>> +             /* 8250 auxiliar UART instead of pl011 */
>> +             bootargs =3D "earlyprintk console=3DttyS0,115200";
>> +     };
>> +
>>       memory {
>>               reg =3D <0 0x40000000>;
>>       };
>> @@ -24,6 +29,11 @@
>>       pinctrl-names =3D "default";
>>       pinctrl-0 =3D <&uart0_gpio32 &gpclk2_gpio43>;
>>       status =3D "okay";
>> +
>> +     bluetooth {
>> +             compatible =3D "brcm,bcm43438-bt";
>> +             max-speed =3D <921600>;
>> +     };
>> };
>
> I know that hci_ll.c DT entry also uses max-speed for the naming. Is this=
 something common we should be doing? Essentially it is not really max-spee=
d. It is the operational speed that we are configuring. There is no down ne=
gotiation ongoing. Does it make sense to use oper-speed instead?

max-speed is correct to use. By default, a driver should know the max
speed of a device (implied by the compatible). The DT property is only
for when there is some host or board limitation as this case seems to
be.

> Are we otherwise fine with compatible string naming?

Yes.

Rob

>
> Loic, the hciattach examples list 3000000 as operational speed. Any reaso=
n why we should limit it?
>
> Regards
>
> Marcel
>

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-14 22:16                             ` Marcel Holtmann
@ 2017-08-16 12:37                                 ` Peter Robinson
  -1 siblings, 0 replies; 50+ messages in thread
From: Peter Robinson @ 2017-08-16 12:37 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Loic Poulain, devicetree-u79uwXL29TY76Z2rM5mHXA, Johan Hedberg,
	Scott Branden, Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

>> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
>> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
>> I push this on a raspbian image.
>>
>> my boot/config.txt is very simple (no overlay):
>>
>> kernel=zImage
>> device_tree=bcm2837-rpi-3-b.dtb
>> # hack for miniuart serial clock
>> core_freq=250
>> dtparam=audio=on
>
> so the Fedora 26 kernel that is based on 4.12 is missing uart0 configuration in DT. Adding it to bcm2837-rpi-3-b.dts will allow for btattach to actually work.

I'm the Fedora maintainer for the Raspberry Pi (and a lot of ARM on
Fedora in general).

> &uart0 {
>         pinctrl-names = "default";
>         pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
>         status = "okay”;
> };
>
> It kinda works, but not all of it. This command confuses me.

With the upstream kernel, some DT bits headed upstream (ie the Fedora
kernel) plus these patches I get to the "kinda works" too in that it
sees a BT adapter but doesn't give it a mac address.

> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>         01                                               .
>> HCI Event: Command Complete (0x0e) plen 4
>       Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>         Status: Unknown HCI Command (0x01)
>
> And I am seeing fun stuff like failed frame assembly.
>
> [  888.687594] Bluetooth: hci0: BCM: chip id 94
> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>
> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.

I've tried on and off to get the BT working, there seems to lots of
options and bits needed including some patches to the bluez [1] stuff
but between not quite upstream kernel bits and numerous distros all
doing it slightly differently I've never got it to work well.

The yocto [1] bits seem fairly representative of some the patches
flying around "to get it working" although I'm not sure how many of
these are actually required and how many are superfluous with this
patch set. There seems to be a firmware required that's not
distributed with linux-firmware which would also be nice to resolve.

Peter

[1] https://git.yoctoproject.org/cgit/cgit.cgi/meta-raspberrypi/tree/recipes-connectivity/bluez5/bluez5
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-16 12:37                                 ` Peter Robinson
  0 siblings, 0 replies; 50+ messages in thread
From: Peter Robinson @ 2017-08-16 12:37 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Loic Poulain, devicetree, Johan Hedberg, Scott Branden, Ray Jui,
	open list:BLUETOOTH DRIVERS, Rob Herring, linux-rpi-kernel

>> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.1=
3-rc3).
>> Using 32-bit multi_v7_defconfig + some additional confs to enable miniua=
rt (BCM2835AUX...) for console.
>> I push this on a raspbian image.
>>
>> my boot/config.txt is very simple (no overlay):
>>
>> kernel=3DzImage
>> device_tree=3Dbcm2837-rpi-3-b.dtb
>> # hack for miniuart serial clock
>> core_freq=3D250
>> dtparam=3Daudio=3Don
>
> so the Fedora 26 kernel that is based on 4.12 is missing uart0 configurat=
ion in DT. Adding it to bcm2837-rpi-3-b.dts will allow for btattach to actu=
ally work.

I'm the Fedora maintainer for the Raspberry Pi (and a lot of ARM on
Fedora in general).

> &uart0 {
>         pinctrl-names =3D "default";
>         pinctrl-0 =3D <&uart0_gpio32 &gpclk2_gpio43>;
>         status =3D "okay=E2=80=9D;
> };
>
> It kinda works, but not all of it. This command confuses me.

With the upstream kernel, some DT bits headed upstream (ie the Fedora
kernel) plus these patches I get to the "kinda works" too in that it
sees a BT adapter but doesn't give it a mac address.

> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>         01                                               .
>> HCI Event: Command Complete (0x0e) plen 4
>       Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>         Status: Unknown HCI Command (0x01)
>
> And I am seeing fun stuff like failed frame assembly.
>
> [  888.687594] Bluetooth: hci0: BCM: chip id 94
> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>
> Actually not providing the firmware makes the controller work. It however=
 is stuck ad AA:AA:.. default address. Providing the firmware turns the add=
ress active. However then it never completes.

I've tried on and off to get the BT working, there seems to lots of
options and bits needed including some patches to the bluez [1] stuff
but between not quite upstream kernel bits and numerous distros all
doing it slightly differently I've never got it to work well.

The yocto [1] bits seem fairly representative of some the patches
flying around "to get it working" although I'm not sure how many of
these are actually required and how many are superfluous with this
patch set. There seems to be a firmware required that's not
distributed with linux-firmware which would also be nice to resolve.

Peter

[1] https://git.yoctoproject.org/cgit/cgit.cgi/meta-raspberrypi/tree/recipe=
s-connectivity/bluez5/bluez5

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-16 12:37                                 ` Peter Robinson
@ 2017-08-16 14:52                                     ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-16 14:52 UTC (permalink / raw)
  To: Peter Robinson
  Cc: Loic Poulain, devicetree-u79uwXL29TY76Z2rM5mHXA, Johan Hedberg,
	Scott Branden, Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Peter,

>>> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
>>> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
>>> I push this on a raspbian image.
>>> 
>>> my boot/config.txt is very simple (no overlay):
>>> 
>>> kernel=zImage
>>> device_tree=bcm2837-rpi-3-b.dtb
>>> # hack for miniuart serial clock
>>> core_freq=250
>>> dtparam=audio=on
>> 
>> so the Fedora 26 kernel that is based on 4.12 is missing uart0 configuration in DT. Adding it to bcm2837-rpi-3-b.dts will allow for btattach to actually work.
> 
> I'm the Fedora maintainer for the Raspberry Pi (and a lot of ARM on
> Fedora in general).
> 
>> &uart0 {
>>        pinctrl-names = "default";
>>        pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
>>        status = "okay”;
>> };
>> 
>> It kinda works, but not all of it. This command confuses me.
> 
> With the upstream kernel, some DT bits headed upstream (ie the Fedora
> kernel) plus these patches I get to the "kinda works" too in that it
> sees a BT adapter but doesn't give it a mac address.

we need to blacklist the AA:AA:.. address in the hci_bcm.c driver. Once you load the firmware, then it has a proper address. I need to confirm this with a second rPI3, but it seems loading the firmware is actually important for this card.

>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>        01                                               .
>>> HCI Event: Command Complete (0x0e) plen 4
>>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>        Status: Unknown HCI Command (0x01)
>> 
>> And I am seeing fun stuff like failed frame assembly.
>> 
>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>> 
>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
> 
> I've tried on and off to get the BT working, there seems to lots of
> options and bits needed including some patches to the bluez [1] stuff
> but between not quite upstream kernel bits and numerous distros all
> doing it slightly differently I've never got it to work well.
> 
> The yocto [1] bits seem fairly representative of some the patches
> flying around "to get it working" although I'm not sure how many of
> these are actually required and how many are superfluous with this
> patch set. There seems to be a firmware required that's not
> distributed with linux-firmware which would also be nice to resolve.

Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.

With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.

Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.

And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.

That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-16 14:52                                     ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-16 14:52 UTC (permalink / raw)
  To: Peter Robinson
  Cc: Loic Poulain, devicetree, Johan Hedberg, Scott Branden, Ray Jui,
	open list:BLUETOOTH DRIVERS, Rob Herring, linux-rpi-kernel

Hi Peter,

>>> I built kernel, modules and dtb from a bluetooth-next tree (4.12 and 4.13-rc3).
>>> Using 32-bit multi_v7_defconfig + some additional confs to enable miniuart (BCM2835AUX...) for console.
>>> I push this on a raspbian image.
>>> 
>>> my boot/config.txt is very simple (no overlay):
>>> 
>>> kernel=zImage
>>> device_tree=bcm2837-rpi-3-b.dtb
>>> # hack for miniuart serial clock
>>> core_freq=250
>>> dtparam=audio=on
>> 
>> so the Fedora 26 kernel that is based on 4.12 is missing uart0 configuration in DT. Adding it to bcm2837-rpi-3-b.dts will allow for btattach to actually work.
> 
> I'm the Fedora maintainer for the Raspberry Pi (and a lot of ARM on
> Fedora in general).
> 
>> &uart0 {
>>        pinctrl-names = "default";
>>        pinctrl-0 = <&uart0_gpio32 &gpclk2_gpio43>;
>>        status = "okay”;
>> };
>> 
>> It kinda works, but not all of it. This command confuses me.
> 
> With the upstream kernel, some DT bits headed upstream (ie the Fedora
> kernel) plus these patches I get to the "kinda works" too in that it
> sees a BT adapter but doesn't give it a mac address.

we need to blacklist the AA:AA:.. address in the hci_bcm.c driver. Once you load the firmware, then it has a proper address. I need to confirm this with a second rPI3, but it seems loading the firmware is actually important for this card.

>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>        01                                               .
>>> HCI Event: Command Complete (0x0e) plen 4
>>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>        Status: Unknown HCI Command (0x01)
>> 
>> And I am seeing fun stuff like failed frame assembly.
>> 
>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>> 
>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
> 
> I've tried on and off to get the BT working, there seems to lots of
> options and bits needed including some patches to the bluez [1] stuff
> but between not quite upstream kernel bits and numerous distros all
> doing it slightly differently I've never got it to work well.
> 
> The yocto [1] bits seem fairly representative of some the patches
> flying around "to get it working" although I'm not sure how many of
> these are actually required and how many are superfluous with this
> patch set. There seems to be a firmware required that's not
> distributed with linux-firmware which would also be nice to resolve.

Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.

With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.

Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.

And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.

That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-16 14:52                                     ` Marcel Holtmann
@ 2017-08-17 10:07                                         ` Loic Poulain
  -1 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-17 10:07 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Peter Robinson, devicetree-u79uwXL29TY76Z2rM5mHXA, Johan Hedberg,
	Scott Branden, Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Marcel,


>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>         01                                               .
>>>> HCI Event: Command Complete (0x0e) plen 4
>>>       Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>         Status: Unknown HCI Command (0x01)
>>>
>>> And I am seeing fun stuff like failed frame assembly.
>>>
>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>
>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>> I've tried on and off to get the BT working, there seems to lots of
>> options and bits needed including some patches to the bluez [1] stuff
>> but between not quite upstream kernel bits and numerous distros all
>> doing it slightly differently I've never got it to work well.
>>
>> The yocto [1] bits seem fairly representative of some the patches
>> flying around "to get it working" although I'm not sure how many of
>> these are actually required and how many are superfluous with this
>> patch set. There seems to be a firmware required that's not
>> distributed with linux-firmware which would also be nice to resolve.
> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>
> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>
> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>
> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>
> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>

I also meet this Frame reassembly failure, Seems we receive a 0x00 byte 
from the controller (unknown pkt type).

Regarding the speed, I'm unable to reach 3Mbps, I selected 921600 
because this is the baudrate used by raspbian bt script.

Maybe they had some issues at higher speed (empirical value ?), However 
2Mbps seems ok on my side (need to double check/adjust).

In a second step, need to check If we can use hardware flow control, I 
heard that pin 16&17 are routed to the bcm RTS/CTS.
Since there is no DMA usage, it could help.

Regards,
Loic

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-17 10:07                                         ` Loic Poulain
  0 siblings, 0 replies; 50+ messages in thread
From: Loic Poulain @ 2017-08-17 10:07 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Peter Robinson, devicetree, Johan Hedberg, Scott Branden,
	Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel

Hi Marcel,


>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>         01                                               .
>>>> HCI Event: Command Complete (0x0e) plen 4
>>>       Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>         Status: Unknown HCI Command (0x01)
>>>
>>> And I am seeing fun stuff like failed frame assembly.
>>>
>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>
>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>> I've tried on and off to get the BT working, there seems to lots of
>> options and bits needed including some patches to the bluez [1] stuff
>> but between not quite upstream kernel bits and numerous distros all
>> doing it slightly differently I've never got it to work well.
>>
>> The yocto [1] bits seem fairly representative of some the patches
>> flying around "to get it working" although I'm not sure how many of
>> these are actually required and how many are superfluous with this
>> patch set. There seems to be a firmware required that's not
>> distributed with linux-firmware which would also be nice to resolve.
> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>
> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>
> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>
> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>
> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>

I also meet this Frame reassembly failure, Seems we receive a 0x00 byte 
from the controller (unknown pkt type).

Regarding the speed, I'm unable to reach 3Mbps, I selected 921600 
because this is the baudrate used by raspbian bt script.

Maybe they had some issues at higher speed (empirical value ?), However 
2Mbps seems ok on my side (need to double check/adjust).

In a second step, need to check If we can use hardware flow control, I 
heard that pin 16&17 are routed to the bcm RTS/CTS.
Since there is no DMA usage, it could help.

Regards,
Loic

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-17 10:07                                         ` Loic Poulain
@ 2017-08-17 10:35                                             ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-17 10:35 UTC (permalink / raw)
  To: Loic Poulain, Eric Anholt
  Cc: Peter Robinson, devicetree-u79uwXL29TY76Z2rM5mHXA, Johan Hedberg,
	Scott Branden, Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Loic,

>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>        01                                               .
>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>        Status: Unknown HCI Command (0x01)
>>>> 
>>>> And I am seeing fun stuff like failed frame assembly.
>>>> 
>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>> 
>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>> I've tried on and off to get the BT working, there seems to lots of
>>> options and bits needed including some patches to the bluez [1] stuff
>>> but between not quite upstream kernel bits and numerous distros all
>>> doing it slightly differently I've never got it to work well.
>>> 
>>> The yocto [1] bits seem fairly representative of some the patches
>>> flying around "to get it working" although I'm not sure how many of
>>> these are actually required and how many are superfluous with this
>>> patch set. There seems to be a firmware required that's not
>>> distributed with linux-firmware which would also be nice to resolve.
>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>> 
>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>> 
>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>> 
>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>> 
>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>> 
> 
> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).

that means adding something like this will silence it.

+#define BCM_RECV_NULL \
+       .type = 0x00, \
+       .hlen = 0, \
+       .loff = 0, \
+       .lsize = 0, \
+       .maxlen = 0
+
+static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
+{
+       kfree_skb(skb);
+       return 0;
+}
+
 static const struct h4_recv_pkt bcm_recv_pkts[] = {
        { H4_RECV_ACL,      .recv = hci_recv_frame },
        { H4_RECV_SCO,      .recv = hci_recv_frame },
        { H4_RECV_EVENT,    .recv = hci_recv_frame },
        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
+       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
 };

It does silence it indeed. Maybe this is some of their markers to ensure that the baud rate change worked. Or it is the indication that the reset completed. Do you happen to know between which commands we receive it?

> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600 because this is the baudrate used by raspbian bt script.

That is the same I experienced. The 921600 works fine, but trying 3Mbps doesn’t. However it seems with hciattach you can crank it up to 3Mpbs somehow. Maybe the delay is just to short for the UART switching in that case.

> Maybe they had some issues at higher speed (empirical value ?), However 2Mbps seems ok on my side (need to double check/adjust).

We have to make it a DT max-speed param anyway. So I am not that worried.

> In a second step, need to check If we can use hardware flow control, I heard that pin 16&17 are routed to the bcm RTS/CTS.
> Since there is no DMA usage, it could help.

Eric, any chance you can dig out the recommendation for the Bluetooth controller usage? I would also like to see the recommended PCM settings for this hardware. It bet it is somehow wired up to the sound controller.

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-17 10:35                                             ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-17 10:35 UTC (permalink / raw)
  To: Loic Poulain, Eric Anholt
  Cc: Peter Robinson, devicetree, Johan Hedberg, Scott Branden,
	Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel

Hi Loic,

>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>        01                                               .
>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>        Status: Unknown HCI Command (0x01)
>>>> 
>>>> And I am seeing fun stuff like failed frame assembly.
>>>> 
>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>> 
>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>> I've tried on and off to get the BT working, there seems to lots of
>>> options and bits needed including some patches to the bluez [1] stuff
>>> but between not quite upstream kernel bits and numerous distros all
>>> doing it slightly differently I've never got it to work well.
>>> 
>>> The yocto [1] bits seem fairly representative of some the patches
>>> flying around "to get it working" although I'm not sure how many of
>>> these are actually required and how many are superfluous with this
>>> patch set. There seems to be a firmware required that's not
>>> distributed with linux-firmware which would also be nice to resolve.
>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>> 
>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>> 
>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>> 
>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>> 
>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>> 
> 
> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).

that means adding something like this will silence it.

+#define BCM_RECV_NULL \
+       .type = 0x00, \
+       .hlen = 0, \
+       .loff = 0, \
+       .lsize = 0, \
+       .maxlen = 0
+
+static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
+{
+       kfree_skb(skb);
+       return 0;
+}
+
 static const struct h4_recv_pkt bcm_recv_pkts[] = {
        { H4_RECV_ACL,      .recv = hci_recv_frame },
        { H4_RECV_SCO,      .recv = hci_recv_frame },
        { H4_RECV_EVENT,    .recv = hci_recv_frame },
        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
+       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
 };

It does silence it indeed. Maybe this is some of their markers to ensure that the baud rate change worked. Or it is the indication that the reset completed. Do you happen to know between which commands we receive it?

> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600 because this is the baudrate used by raspbian bt script.

That is the same I experienced. The 921600 works fine, but trying 3Mbps doesn’t. However it seems with hciattach you can crank it up to 3Mpbs somehow. Maybe the delay is just to short for the UART switching in that case.

> Maybe they had some issues at higher speed (empirical value ?), However 2Mbps seems ok on my side (need to double check/adjust).

We have to make it a DT max-speed param anyway. So I am not that worried.

> In a second step, need to check If we can use hardware flow control, I heard that pin 16&17 are routed to the bcm RTS/CTS.
> Since there is no DMA usage, it could help.

Eric, any chance you can dig out the recommendation for the Bluetooth controller usage? I would also like to see the recommended PCM settings for this hardware. It bet it is somehow wired up to the sound controller.

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-17 10:35                                             ` Marcel Holtmann
@ 2017-08-17 10:47                                                 ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-17 10:47 UTC (permalink / raw)
  To: Loic Poulain, Eric Anholt
  Cc: Peter Robinson, devicetree-u79uwXL29TY76Z2rM5mHXA, Johan Hedberg,
	Scott Branden, Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Loic,

>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>       01                                               .
>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>     Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>       Status: Unknown HCI Command (0x01)
>>>>> 
>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>> 
>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>> 
>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>> I've tried on and off to get the BT working, there seems to lots of
>>>> options and bits needed including some patches to the bluez [1] stuff
>>>> but between not quite upstream kernel bits and numerous distros all
>>>> doing it slightly differently I've never got it to work well.
>>>> 
>>>> The yocto [1] bits seem fairly representative of some the patches
>>>> flying around "to get it working" although I'm not sure how many of
>>>> these are actually required and how many are superfluous with this
>>>> patch set. There seems to be a firmware required that's not
>>>> distributed with linux-firmware which would also be nice to resolve.
>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>> 
>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>> 
>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>> 
>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>> 
>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>> 
>> 
>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
> 
> that means adding something like this will silence it.
> 
> +#define BCM_RECV_NULL \
> +       .type = 0x00, \
> +       .hlen = 0, \
> +       .loff = 0, \
> +       .lsize = 0, \
> +       .maxlen = 0
> +
> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       kfree_skb(skb);
> +       return 0;
> +}
> +
> static const struct h4_recv_pkt bcm_recv_pkts[] = {
>        { H4_RECV_ACL,      .recv = hci_recv_frame },
>        { H4_RECV_SCO,      .recv = hci_recv_frame },
>        { H4_RECV_EVENT,    .recv = hci_recv_frame },
>        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
> };
> 
> It does silence it indeed. Maybe this is some of their markers to ensure that the baud rate change worked. Or it is the indication that the reset completed. Do you happen to know between which commands we receive it?

I converted it into being a vendor diagnostic packet.

< HCI Command: Broadcom Launch RAM (0x3f|0x004e) plen 4
        Address: 0xffffffff
> HCI Event: Command Complete (0x0e) plen 4
      Broadcom Launch RAM (0x3f|0x004e) ncmd 1
        Status: Success (0x00)
= Vendor Diagnostic (len 0)
< HCI Command: Broadcom Update UART Baud Rate (0x3f|0x0018) plen 6
        00 00 00 10 0e 00                                ......          
> HCI Event: Command Complete (0x0e) plen 4
      Broadcom Update UART Baud Rate (0x3f|0x0018) ncmd 1
        Status: Success (0x00)
< HCI Command: Reset (0x03|0x0003) plen 0
> HCI Event: Command Complete (0x0e) plen 4
      Reset (0x03|0x0003) ncmd 1
        Status: Success (0x00)

So it comes right after the firmware launch command. Which means this is just an indication there. My assumption it safe to just silence this empty packet. Or maybe actually leaving it as empty vendor diagnostic packet is just fine as well. Then we would actually know how often it happens.

+#define BCM_RECV_NULL \
+       .type = BCM_NULL_PKT, \
+       .hlen = BCM_NULL_SIZE, \
+       .loff = 0, \
+       .lsize = 0, \
+       .maxlen = BCM_NULL_SIZE
+
 static const struct h4_recv_pkt bcm_recv_pkts[] = {
        { H4_RECV_ACL,      .recv = hci_recv_frame },
        { H4_RECV_SCO,      .recv = hci_recv_frame },
        { H4_RECV_EVENT,    .recv = hci_recv_frame },
        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
+       { BCM_RECV_NULL,    .recv = hci_recv_diag  },
 };

Regards

Marcel

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-17 10:47                                                 ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-17 10:47 UTC (permalink / raw)
  To: Loic Poulain, Eric Anholt
  Cc: Peter Robinson, devicetree, Johan Hedberg, Scott Branden,
	Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel

Hi Loic,

>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>       01                                               .
>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>     Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>       Status: Unknown HCI Command (0x01)
>>>>> 
>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>> 
>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>> 
>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>> I've tried on and off to get the BT working, there seems to lots of
>>>> options and bits needed including some patches to the bluez [1] stuff
>>>> but between not quite upstream kernel bits and numerous distros all
>>>> doing it slightly differently I've never got it to work well.
>>>> 
>>>> The yocto [1] bits seem fairly representative of some the patches
>>>> flying around "to get it working" although I'm not sure how many of
>>>> these are actually required and how many are superfluous with this
>>>> patch set. There seems to be a firmware required that's not
>>>> distributed with linux-firmware which would also be nice to resolve.
>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>> 
>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>> 
>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>> 
>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>> 
>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>> 
>> 
>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
> 
> that means adding something like this will silence it.
> 
> +#define BCM_RECV_NULL \
> +       .type = 0x00, \
> +       .hlen = 0, \
> +       .loff = 0, \
> +       .lsize = 0, \
> +       .maxlen = 0
> +
> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       kfree_skb(skb);
> +       return 0;
> +}
> +
> static const struct h4_recv_pkt bcm_recv_pkts[] = {
>        { H4_RECV_ACL,      .recv = hci_recv_frame },
>        { H4_RECV_SCO,      .recv = hci_recv_frame },
>        { H4_RECV_EVENT,    .recv = hci_recv_frame },
>        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
> };
> 
> It does silence it indeed. Maybe this is some of their markers to ensure that the baud rate change worked. Or it is the indication that the reset completed. Do you happen to know between which commands we receive it?

I converted it into being a vendor diagnostic packet.

< HCI Command: Broadcom Launch RAM (0x3f|0x004e) plen 4
        Address: 0xffffffff
> HCI Event: Command Complete (0x0e) plen 4
      Broadcom Launch RAM (0x3f|0x004e) ncmd 1
        Status: Success (0x00)
= Vendor Diagnostic (len 0)
< HCI Command: Broadcom Update UART Baud Rate (0x3f|0x0018) plen 6
        00 00 00 10 0e 00                                ......          
> HCI Event: Command Complete (0x0e) plen 4
      Broadcom Update UART Baud Rate (0x3f|0x0018) ncmd 1
        Status: Success (0x00)
< HCI Command: Reset (0x03|0x0003) plen 0
> HCI Event: Command Complete (0x0e) plen 4
      Reset (0x03|0x0003) ncmd 1
        Status: Success (0x00)

So it comes right after the firmware launch command. Which means this is just an indication there. My assumption it safe to just silence this empty packet. Or maybe actually leaving it as empty vendor diagnostic packet is just fine as well. Then we would actually know how often it happens.

+#define BCM_RECV_NULL \
+       .type = BCM_NULL_PKT, \
+       .hlen = BCM_NULL_SIZE, \
+       .loff = 0, \
+       .lsize = 0, \
+       .maxlen = BCM_NULL_SIZE
+
 static const struct h4_recv_pkt bcm_recv_pkts[] = {
        { H4_RECV_ACL,      .recv = hci_recv_frame },
        { H4_RECV_SCO,      .recv = hci_recv_frame },
        { H4_RECV_EVENT,    .recv = hci_recv_frame },
        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
+       { BCM_RECV_NULL,    .recv = hci_recv_diag  },
 };

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-17 10:35                                             ` Marcel Holtmann
@ 2017-08-17 16:21                                                 ` Eric Anholt
  -1 siblings, 0 replies; 50+ messages in thread
From: Eric Anholt @ 2017-08-17 16:21 UTC (permalink / raw)
  To: Marcel Holtmann, Loic Poulain
  Cc: Peter Robinson, devicetree@vger.kernel.org, Johan Hedberg,
	Scott Branden, Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 5391 bytes --]

Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> writes:

> Hi Loic,
>
>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>        01                                               .
>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>        Status: Unknown HCI Command (0x01)
>>>>> 
>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>> 
>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>> 
>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>> I've tried on and off to get the BT working, there seems to lots of
>>>> options and bits needed including some patches to the bluez [1] stuff
>>>> but between not quite upstream kernel bits and numerous distros all
>>>> doing it slightly differently I've never got it to work well.
>>>> 
>>>> The yocto [1] bits seem fairly representative of some the patches
>>>> flying around "to get it working" although I'm not sure how many of
>>>> these are actually required and how many are superfluous with this
>>>> patch set. There seems to be a firmware required that's not
>>>> distributed with linux-firmware which would also be nice to resolve.
>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>> 
>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>> 
>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>> 
>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>> 
>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>> 
>> 
>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
>
> that means adding something like this will silence it.
>
> +#define BCM_RECV_NULL \
> +       .type = 0x00, \
> +       .hlen = 0, \
> +       .loff = 0, \
> +       .lsize = 0, \
> +       .maxlen = 0
> +
> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       kfree_skb(skb);
> +       return 0;
> +}
> +
>  static const struct h4_recv_pkt bcm_recv_pkts[] = {
>         { H4_RECV_ACL,      .recv = hci_recv_frame },
>         { H4_RECV_SCO,      .recv = hci_recv_frame },
>         { H4_RECV_EVENT,    .recv = hci_recv_frame },
>         { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
>  };
>
> It does silence it indeed. Maybe this is some of their markers to
> ensure that the baud rate change worked. Or it is the indication that
> the reset completed. Do you happen to know between which commands we
> receive it?
>
>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>> because this is the baudrate used by raspbian bt script.
>
> That is the same I experienced. The 921600 works fine, but trying
> 3Mbps doesn’t. However it seems with hciattach you can crank it up to
> 3Mpbs somehow. Maybe the delay is just to short for the UART switching
> in that case.
>
>> Maybe they had some issues at higher speed (empirical value ?),
>> However 2Mbps seems ok on my side (need to double check/adjust).
>
> We have to make it a DT max-speed param anyway. So I am not that
> worried.
>
>> In a second step, need to check If we can use hardware flow control,
>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is
>> no DMA usage, it could help.

The BT's CTS/RTS are tied to 3v3/ground.

> Eric, any chance you can dig out the recommendation for the Bluetooth
> controller usage? I would also like to see the recommended PCM
> settings for this hardware. It bet it is somehow wired up to the sound
> controller.

BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to
the i2s controller (pcm_gpio28 pin group).

I'm not clear on what you're asking for, or where I would go to find
anything else.  However, for integration stuff on the board, Phil, Dom,
and Gordon at RPi are likely to know more.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-17 16:21                                                 ` Eric Anholt
  0 siblings, 0 replies; 50+ messages in thread
From: Eric Anholt @ 2017-08-17 16:21 UTC (permalink / raw)
  To: Marcel Holtmann, Loic Poulain
  Cc: Peter Robinson, devicetree, Johan Hedberg, Scott Branden,
	Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel

[-- Attachment #1: Type: text/plain, Size: 5364 bytes --]

Marcel Holtmann <marcel@holtmann.org> writes:

> Hi Loic,
>
>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>        01                                               .
>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>        Status: Unknown HCI Command (0x01)
>>>>> 
>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>> 
>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>> 
>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>> I've tried on and off to get the BT working, there seems to lots of
>>>> options and bits needed including some patches to the bluez [1] stuff
>>>> but between not quite upstream kernel bits and numerous distros all
>>>> doing it slightly differently I've never got it to work well.
>>>> 
>>>> The yocto [1] bits seem fairly representative of some the patches
>>>> flying around "to get it working" although I'm not sure how many of
>>>> these are actually required and how many are superfluous with this
>>>> patch set. There seems to be a firmware required that's not
>>>> distributed with linux-firmware which would also be nice to resolve.
>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>> 
>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>> 
>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>> 
>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>> 
>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>> 
>> 
>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
>
> that means adding something like this will silence it.
>
> +#define BCM_RECV_NULL \
> +       .type = 0x00, \
> +       .hlen = 0, \
> +       .loff = 0, \
> +       .lsize = 0, \
> +       .maxlen = 0
> +
> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
> +{
> +       kfree_skb(skb);
> +       return 0;
> +}
> +
>  static const struct h4_recv_pkt bcm_recv_pkts[] = {
>         { H4_RECV_ACL,      .recv = hci_recv_frame },
>         { H4_RECV_SCO,      .recv = hci_recv_frame },
>         { H4_RECV_EVENT,    .recv = hci_recv_frame },
>         { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
>  };
>
> It does silence it indeed. Maybe this is some of their markers to
> ensure that the baud rate change worked. Or it is the indication that
> the reset completed. Do you happen to know between which commands we
> receive it?
>
>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>> because this is the baudrate used by raspbian bt script.
>
> That is the same I experienced. The 921600 works fine, but trying
> 3Mbps doesn’t. However it seems with hciattach you can crank it up to
> 3Mpbs somehow. Maybe the delay is just to short for the UART switching
> in that case.
>
>> Maybe they had some issues at higher speed (empirical value ?),
>> However 2Mbps seems ok on my side (need to double check/adjust).
>
> We have to make it a DT max-speed param anyway. So I am not that
> worried.
>
>> In a second step, need to check If we can use hardware flow control,
>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is
>> no DMA usage, it could help.

The BT's CTS/RTS are tied to 3v3/ground.

> Eric, any chance you can dig out the recommendation for the Bluetooth
> controller usage? I would also like to see the recommended PCM
> settings for this hardware. It bet it is somehow wired up to the sound
> controller.

BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to
the i2s controller (pcm_gpio28 pin group).

I'm not clear on what you're asking for, or where I would go to find
anything else.  However, for integration stuff on the board, Phil, Dom,
and Gordon at RPi are likely to know more.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-17 16:21                                                 ` Eric Anholt
@ 2017-08-17 17:34                                                     ` Marcel Holtmann
  -1 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-17 17:34 UTC (permalink / raw)
  To: Eric Anholt
  Cc: Loic Poulain, Peter Robinson, devicetree-u79uwXL29TY76Z2rM5mHXA,
	Johan Hedberg, Scott Branden, Ray Jui,
	open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

Hi Eric,

>>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>>       01                                               .
>>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>>     Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>>       Status: Unknown HCI Command (0x01)
>>>>>> 
>>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>>> 
>>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>>> 
>>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>>> I've tried on and off to get the BT working, there seems to lots of
>>>>> options and bits needed including some patches to the bluez [1] stuff
>>>>> but between not quite upstream kernel bits and numerous distros all
>>>>> doing it slightly differently I've never got it to work well.
>>>>> 
>>>>> The yocto [1] bits seem fairly representative of some the patches
>>>>> flying around "to get it working" although I'm not sure how many of
>>>>> these are actually required and how many are superfluous with this
>>>>> patch set. There seems to be a firmware required that's not
>>>>> distributed with linux-firmware which would also be nice to resolve.
>>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>>> 
>>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>>> 
>>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>>> 
>>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>>> 
>>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>>> 
>>> 
>>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
>> 
>> that means adding something like this will silence it.
>> 
>> +#define BCM_RECV_NULL \
>> +       .type = 0x00, \
>> +       .hlen = 0, \
>> +       .loff = 0, \
>> +       .lsize = 0, \
>> +       .maxlen = 0
>> +
>> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
>> +{
>> +       kfree_skb(skb);
>> +       return 0;
>> +}
>> +
>> static const struct h4_recv_pkt bcm_recv_pkts[] = {
>>        { H4_RECV_ACL,      .recv = hci_recv_frame },
>>        { H4_RECV_SCO,      .recv = hci_recv_frame },
>>        { H4_RECV_EVENT,    .recv = hci_recv_frame },
>>        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
>> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
>> };
>> 
>> It does silence it indeed. Maybe this is some of their markers to
>> ensure that the baud rate change worked. Or it is the indication that
>> the reset completed. Do you happen to know between which commands we
>> receive it?
>> 
>>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>>> because this is the baudrate used by raspbian bt script.
>> 
>> That is the same I experienced. The 921600 works fine, but trying
>> 3Mbps doesn’t. However it seems with hciattach you can crank it up to
>> 3Mpbs somehow. Maybe the delay is just to short for the UART switching
>> in that case.
>> 
>>> Maybe they had some issues at higher speed (empirical value ?),
>>> However 2Mbps seems ok on my side (need to double check/adjust).
>> 
>> We have to make it a DT max-speed param anyway. So I am not that
>> worried.
>> 
>>> In a second step, need to check If we can use hardware flow control,
>>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is
>>> no DMA usage, it could help.
> 
> The BT's CTS/RTS are tied to 3v3/ground.
> 
>> Eric, any chance you can dig out the recommendation for the Bluetooth
>> controller usage? I would also like to see the recommended PCM
>> settings for this hardware. It bet it is somehow wired up to the sound
>> controller.
> 
> BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to
> the i2s controller (pcm_gpio28 pin group).
> 
> I'm not clear on what you're asking for, or where I would go to find
> anything else.  However, for integration stuff on the board, Phil, Dom,
> and Gordon at RPi are likely to know more.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/bluetooth/btbcm.h

We have to send Set Sleepmode, Set PCM Int Params and Set PCM Format Params commands via Bluetooth HCI that will configure the sleep behavior and PCM parameters. So even if you route it via GPIO 28-31, you still need to tell the controller what parameters are used there.

For the sleep mode parameters we most likely need to have the GPIO Expander working first. However that is something we should set correctly as well to allow the controller to go to sleep.

Regards

Marcel

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-17 17:34                                                     ` Marcel Holtmann
  0 siblings, 0 replies; 50+ messages in thread
From: Marcel Holtmann @ 2017-08-17 17:34 UTC (permalink / raw)
  To: Eric Anholt
  Cc: Loic Poulain, Peter Robinson, devicetree, Johan Hedberg,
	Scott Branden, Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel

Hi Eric,

>>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>>       01                                               .
>>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>>     Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>>       Status: Unknown HCI Command (0x01)
>>>>>> 
>>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>>> 
>>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>>> 
>>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>>> I've tried on and off to get the BT working, there seems to lots of
>>>>> options and bits needed including some patches to the bluez [1] stuff
>>>>> but between not quite upstream kernel bits and numerous distros all
>>>>> doing it slightly differently I've never got it to work well.
>>>>> 
>>>>> The yocto [1] bits seem fairly representative of some the patches
>>>>> flying around "to get it working" although I'm not sure how many of
>>>>> these are actually required and how many are superfluous with this
>>>>> patch set. There seems to be a firmware required that's not
>>>>> distributed with linux-firmware which would also be nice to resolve.
>>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>>> 
>>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>>> 
>>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>>> 
>>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>>> 
>>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>>> 
>>> 
>>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
>> 
>> that means adding something like this will silence it.
>> 
>> +#define BCM_RECV_NULL \
>> +       .type = 0x00, \
>> +       .hlen = 0, \
>> +       .loff = 0, \
>> +       .lsize = 0, \
>> +       .maxlen = 0
>> +
>> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
>> +{
>> +       kfree_skb(skb);
>> +       return 0;
>> +}
>> +
>> static const struct h4_recv_pkt bcm_recv_pkts[] = {
>>        { H4_RECV_ACL,      .recv = hci_recv_frame },
>>        { H4_RECV_SCO,      .recv = hci_recv_frame },
>>        { H4_RECV_EVENT,    .recv = hci_recv_frame },
>>        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
>> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
>> };
>> 
>> It does silence it indeed. Maybe this is some of their markers to
>> ensure that the baud rate change worked. Or it is the indication that
>> the reset completed. Do you happen to know between which commands we
>> receive it?
>> 
>>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>>> because this is the baudrate used by raspbian bt script.
>> 
>> That is the same I experienced. The 921600 works fine, but trying
>> 3Mbps doesn’t. However it seems with hciattach you can crank it up to
>> 3Mpbs somehow. Maybe the delay is just to short for the UART switching
>> in that case.
>> 
>>> Maybe they had some issues at higher speed (empirical value ?),
>>> However 2Mbps seems ok on my side (need to double check/adjust).
>> 
>> We have to make it a DT max-speed param anyway. So I am not that
>> worried.
>> 
>>> In a second step, need to check If we can use hardware flow control,
>>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is
>>> no DMA usage, it could help.
> 
> The BT's CTS/RTS are tied to 3v3/ground.
> 
>> Eric, any chance you can dig out the recommendation for the Bluetooth
>> controller usage? I would also like to see the recommended PCM
>> settings for this hardware. It bet it is somehow wired up to the sound
>> controller.
> 
> BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to
> the i2s controller (pcm_gpio28 pin group).
> 
> I'm not clear on what you're asking for, or where I would go to find
> anything else.  However, for integration stuff on the board, Phil, Dom,
> and Gordon at RPi are likely to know more.

https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/bluetooth/btbcm.h

We have to send Set Sleepmode, Set PCM Int Params and Set PCM Format Params commands via Bluetooth HCI that will configure the sleep behavior and PCM parameters. So even if you route it via GPIO 28-31, you still need to tell the controller what parameters are used there.

For the sleep mode parameters we most likely need to have the GPIO Expander working first. However that is something we should set correctly as well to allow the controller to go to sleep.

Regards

Marcel


^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-17 17:34                                                     ` Marcel Holtmann
@ 2017-08-17 19:38                                                         ` Eric Anholt
  -1 siblings, 0 replies; 50+ messages in thread
From: Eric Anholt @ 2017-08-17 19:38 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Loic Poulain, Peter Robinson, devicetree@vger.kernel.org,
	Johan Hedberg, Scott Branden, Ray Jui,
	open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

[-- Attachment #1: Type: text/plain, Size: 6015 bytes --]

Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> writes:

> Hi Eric,
>
>>>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>>>       01                                               .
>>>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>>>     Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>>>       Status: Unknown HCI Command (0x01)
>>>>>>> 
>>>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>>>> 
>>>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>>>> 
>>>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>>>> I've tried on and off to get the BT working, there seems to lots of
>>>>>> options and bits needed including some patches to the bluez [1] stuff
>>>>>> but between not quite upstream kernel bits and numerous distros all
>>>>>> doing it slightly differently I've never got it to work well.
>>>>>> 
>>>>>> The yocto [1] bits seem fairly representative of some the patches
>>>>>> flying around "to get it working" although I'm not sure how many of
>>>>>> these are actually required and how many are superfluous with this
>>>>>> patch set. There seems to be a firmware required that's not
>>>>>> distributed with linux-firmware which would also be nice to resolve.
>>>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>>>> 
>>>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>>>> 
>>>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>>>> 
>>>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>>>> 
>>>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>>>> 
>>>> 
>>>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
>>> 
>>> that means adding something like this will silence it.
>>> 
>>> +#define BCM_RECV_NULL \
>>> +       .type = 0x00, \
>>> +       .hlen = 0, \
>>> +       .loff = 0, \
>>> +       .lsize = 0, \
>>> +       .maxlen = 0
>>> +
>>> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
>>> +{
>>> +       kfree_skb(skb);
>>> +       return 0;
>>> +}
>>> +
>>> static const struct h4_recv_pkt bcm_recv_pkts[] = {
>>>        { H4_RECV_ACL,      .recv = hci_recv_frame },
>>>        { H4_RECV_SCO,      .recv = hci_recv_frame },
>>>        { H4_RECV_EVENT,    .recv = hci_recv_frame },
>>>        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
>>> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
>>> };
>>> 
>>> It does silence it indeed. Maybe this is some of their markers to
>>> ensure that the baud rate change worked. Or it is the indication that
>>> the reset completed. Do you happen to know between which commands we
>>> receive it?
>>> 
>>>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>>>> because this is the baudrate used by raspbian bt script.
>>> 
>>> That is the same I experienced. The 921600 works fine, but trying
>>> 3Mbps doesn’t. However it seems with hciattach you can crank it up to
>>> 3Mpbs somehow. Maybe the delay is just to short for the UART switching
>>> in that case.
>>> 
>>>> Maybe they had some issues at higher speed (empirical value ?),
>>>> However 2Mbps seems ok on my side (need to double check/adjust).
>>> 
>>> We have to make it a DT max-speed param anyway. So I am not that
>>> worried.
>>> 
>>>> In a second step, need to check If we can use hardware flow control,
>>>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is
>>>> no DMA usage, it could help.
>> 
>> The BT's CTS/RTS are tied to 3v3/ground.
>> 
>>> Eric, any chance you can dig out the recommendation for the Bluetooth
>>> controller usage? I would also like to see the recommended PCM
>>> settings for this hardware. It bet it is somehow wired up to the sound
>>> controller.
>> 
>> BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to
>> the i2s controller (pcm_gpio28 pin group).
>> 
>> I'm not clear on what you're asking for, or where I would go to find
>> anything else.  However, for integration stuff on the board, Phil, Dom,
>> and Gordon at RPi are likely to know more.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/bluetooth/btbcm.h
>
> We have to send Set Sleepmode, Set PCM Int Params and Set PCM Format
> Params commands via Bluetooth HCI that will configure the sleep
> behavior and PCM parameters. So even if you route it via GPIO 28-31,
> you still need to tell the controller what parameters are used there.

Yeah, no clue about those.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-17 19:38                                                         ` Eric Anholt
  0 siblings, 0 replies; 50+ messages in thread
From: Eric Anholt @ 2017-08-17 19:38 UTC (permalink / raw)
  To: Marcel Holtmann
  Cc: Loic Poulain, Peter Robinson, devicetree, Johan Hedberg,
	Scott Branden, Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel

[-- Attachment #1: Type: text/plain, Size: 5988 bytes --]

Marcel Holtmann <marcel@holtmann.org> writes:

> Hi Eric,
>
>>>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>>>       01                                               .
>>>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>>>     Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>>>       Status: Unknown HCI Command (0x01)
>>>>>>> 
>>>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>>>> 
>>>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>>>> 
>>>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>>>> I've tried on and off to get the BT working, there seems to lots of
>>>>>> options and bits needed including some patches to the bluez [1] stuff
>>>>>> but between not quite upstream kernel bits and numerous distros all
>>>>>> doing it slightly differently I've never got it to work well.
>>>>>> 
>>>>>> The yocto [1] bits seem fairly representative of some the patches
>>>>>> flying around "to get it working" although I'm not sure how many of
>>>>>> these are actually required and how many are superfluous with this
>>>>>> patch set. There seems to be a firmware required that's not
>>>>>> distributed with linux-firmware which would also be nice to resolve.
>>>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>>>> 
>>>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>>>> 
>>>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>>>> 
>>>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>>>> 
>>>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>>>> 
>>>> 
>>>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
>>> 
>>> that means adding something like this will silence it.
>>> 
>>> +#define BCM_RECV_NULL \
>>> +       .type = 0x00, \
>>> +       .hlen = 0, \
>>> +       .loff = 0, \
>>> +       .lsize = 0, \
>>> +       .maxlen = 0
>>> +
>>> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
>>> +{
>>> +       kfree_skb(skb);
>>> +       return 0;
>>> +}
>>> +
>>> static const struct h4_recv_pkt bcm_recv_pkts[] = {
>>>        { H4_RECV_ACL,      .recv = hci_recv_frame },
>>>        { H4_RECV_SCO,      .recv = hci_recv_frame },
>>>        { H4_RECV_EVENT,    .recv = hci_recv_frame },
>>>        { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
>>> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
>>> };
>>> 
>>> It does silence it indeed. Maybe this is some of their markers to
>>> ensure that the baud rate change worked. Or it is the indication that
>>> the reset completed. Do you happen to know between which commands we
>>> receive it?
>>> 
>>>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>>>> because this is the baudrate used by raspbian bt script.
>>> 
>>> That is the same I experienced. The 921600 works fine, but trying
>>> 3Mbps doesn’t. However it seems with hciattach you can crank it up to
>>> 3Mpbs somehow. Maybe the delay is just to short for the UART switching
>>> in that case.
>>> 
>>>> Maybe they had some issues at higher speed (empirical value ?),
>>>> However 2Mbps seems ok on my side (need to double check/adjust).
>>> 
>>> We have to make it a DT max-speed param anyway. So I am not that
>>> worried.
>>> 
>>>> In a second step, need to check If we can use hardware flow control,
>>>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is
>>>> no DMA usage, it could help.
>> 
>> The BT's CTS/RTS are tied to 3v3/ground.
>> 
>>> Eric, any chance you can dig out the recommendation for the Bluetooth
>>> controller usage? I would also like to see the recommended PCM
>>> settings for this hardware. It bet it is somehow wired up to the sound
>>> controller.
>> 
>> BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to
>> the i2s controller (pcm_gpio28 pin group).
>> 
>> I'm not clear on what you're asking for, or where I would go to find
>> anything else.  However, for integration stuff on the board, Phil, Dom,
>> and Gordon at RPi are likely to know more.
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/bluetooth/btbcm.h
>
> We have to send Set Sleepmode, Set PCM Int Params and Set PCM Format
> Params commands via Bluetooth HCI that will configure the sleep
> behavior and PCM parameters. So even if you route it via GPIO 28-31,
> you still need to tell the controller what parameters are used there.

Yeah, no clue about those.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
  2017-08-17 16:21                                                 ` Eric Anholt
@ 2017-08-17 20:08                                                     ` Phil Elwell
  -1 siblings, 0 replies; 50+ messages in thread
From: Phil Elwell @ 2017-08-17 20:08 UTC (permalink / raw)
  To: Eric Anholt, Marcel Holtmann, Loic Poulain
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Johan Hedberg, Scott Branden,
	Ray Jui, open list:BLUETOOTH DRIVERS, Rob Herring,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 17/08/2017 17:21, Eric Anholt wrote:
> Marcel Holtmann <marcel-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org> writes:
>
>> Hi Loic,
>>
>>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) plen 1
>>>>>>        01                                               .
>>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>>        Status: Unknown HCI Command (0x01)
>>>>>>
>>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>>>
>>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 0182
>>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>>>
>>>>>> Actually not providing the firmware makes the controller work. It however is stuck ad AA:AA:.. default address. Providing the firmware turns the address active. However then it never completes.
>>>>> I've tried on and off to get the BT working, there seems to lots of
>>>>> options and bits needed including some patches to the bluez [1] stuff
>>>>> but between not quite upstream kernel bits and numerous distros all
>>>>> doing it slightly differently I've never got it to work well.
>>>>>
>>>>> The yocto [1] bits seem fairly representative of some the patches
>>>>> flying around "to get it working" although I'm not sure how many of
>>>>> these are actually required and how many are superfluous with this
>>>>> patch set. There seems to be a firmware required that's not
>>>>> distributed with linux-firmware which would also be nice to resolve.
>>>> Non of these Yocto patches are actually needed. The culprit is the .oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will start to work smoothly. I sent a patch that takes the .oper_speed out completely and only applies it for the ACPI based devices where we know that it works.
>>>>
>>>> With my patch and the right DT entries for uart0 it actually works with “btattach -B /dev/ttyAMA0 -P bcm”. It will load the firmware, configure it and head towards the right path.
>>>>
>>>> Obviously btattach is only an interim step here. Loic’s patches for serdev integration and changing the DT to expose uart0 as serial-slave for Bluetooth is the right approach. Once Loic’s resends the patches we can get them into bluetooth-next and start merging these towards upstream. After that, Bluetooth should work just out of the box like with any USB dongle.
>>>>
>>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire) instead of H:4 is possible, we could consider it, but as long as the UART wiring doesn’t cause any bit errors, it is not worth it.
>>>>
>>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84)” error. I need to figure out where that is. Frankly we really need to hexdump the packet when this happens.
>>>>
>>>
>>> I also meet this Frame reassembly failure, Seems we receive a 0x00 byte from the controller (unknown pkt type).
>>
>> that means adding something like this will silence it.
>>
>> +#define BCM_RECV_NULL \
>> +       .type = 0x00, \
>> +       .hlen = 0, \
>> +       .loff = 0, \
>> +       .lsize = 0, \
>> +       .maxlen = 0
>> +
>> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
>> +{
>> +       kfree_skb(skb);
>> +       return 0;
>> +}
>> +
>>  static const struct h4_recv_pkt bcm_recv_pkts[] = {
>>         { H4_RECV_ACL,      .recv = hci_recv_frame },
>>         { H4_RECV_SCO,      .recv = hci_recv_frame },
>>         { H4_RECV_EVENT,    .recv = hci_recv_frame },
>>         { BCM_RECV_LM_DIAG, .recv = hci_recv_diag  },
>> +       { BCM_RECV_NULL,    .recv = bcm_recv_null  },
>>  };
>>
>> It does silence it indeed. Maybe this is some of their markers to
>> ensure that the baud rate change worked. Or it is the indication that
>> the reset completed. Do you happen to know between which commands we
>> receive it?
>>
>>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>>> because this is the baudrate used by raspbian bt script.
>>
>> That is the same I experienced. The 921600 works fine, but trying
>> 3Mbps doesn’t. However it seems with hciattach you can crank it up to
>> 3Mpbs somehow. Maybe the delay is just to short for the UART switching
>> in that case.
>>
>>> Maybe they had some issues at higher speed (empirical value ?),
>>> However 2Mbps seems ok on my side (need to double check/adjust).
>>
>> We have to make it a DT max-speed param anyway. So I am not that
>> worried.
>>
>>> In a second step, need to check If we can use hardware flow control,
>>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is
>>> no DMA usage, it could help.
>
> The BT's CTS/RTS are tied to 3v3/ground.
>
>> Eric, any chance you can dig out the recommendation for the Bluetooth
>> controller usage? I would also like to see the recommended PCM
>> settings for this hardware. It bet it is somehow wired up to the sound
>> controller.
>
> BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to
> the i2s controller (pcm_gpio28 pin group).
>
> I'm not clear on what you're asking for, or where I would go to find
> anything else.  However, for integration stuff on the board, Phil, Dom,
> and Gordon at RPi are likely to know more.

Eric is correct - GPIOs 28-31 on the Pi 3B are connected to the PCM function on
the 43438. We haven't made use of those pins, and we are unlikely to. On the
Zero W, 30 and 31 are connected to the BT RTS and CTS, but on Pi 3B the flow control
pins are not connected, hence the baud rate restriction due to the risk of data
loss - particularly likely and fatal during firmware uploaded since at boot time
systemd is "aggresively parallel". This is the reason for the BlueZ patch that
defers switching to the higher baudrate until after firmware loading is complete.

Regards,

Phil

^ permalink raw reply	[flat|nested] 50+ messages in thread

* Re: [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave
@ 2017-08-17 20:08                                                     ` Phil Elwell
  0 siblings, 0 replies; 50+ messages in thread
From: Phil Elwell @ 2017-08-17 20:08 UTC (permalink / raw)
  To: Eric Anholt, Marcel Holtmann, Loic Poulain
  Cc: devicetree, Johan Hedberg, Scott Branden, Ray Jui,
	open list:BLUETOOTH DRIVERS, Rob Herring, linux-rpi-kernel

On 17/08/2017 17:21, Eric Anholt wrote:
> Marcel Holtmann <marcel@holtmann.org> writes:
>
>> Hi Loic,
>>
>>>>>> < HCI Command: Broadcom Write UART Clock Setting (0x3f|0x0045) ple=
n 1
>>>>>>        01                                               .
>>>>>>> HCI Event: Command Complete (0x0e) plen 4
>>>>>>      Broadcom Write UART Clock Setting (0x3f|0x0045) ncmd 1
>>>>>>        Status: Unknown HCI Command (0x01)
>>>>>>
>>>>>> And I am seeing fun stuff like failed frame assembly.
>>>>>>
>>>>>> [  888.687594] Bluetooth: hci0: BCM: chip id 94
>>>>>> [  888.687821] Bluetooth: hci0: BCM43430A1 (001.002.009) build 018=
2
>>>>>> [  892.059023] Bluetooth: hci0: Frame reassembly failed (-84)
>>>>>> [  892.316936] Bluetooth: hci0: BCM: failed to write clock (-56)
>>>>>> [  892.429478] Bluetooth: hci0: BCM (001.002.009) build 0182
>>>>>>
>>>>>> Actually not providing the firmware makes the controller work. It =
however is stuck ad AA:AA:.. default address. Providing the firmware turn=
s the address active. However then it never completes.
>>>>> I've tried on and off to get the BT working, there seems to lots of=

>>>>> options and bits needed including some patches to the bluez [1] stu=
ff
>>>>> but between not quite upstream kernel bits and numerous distros all=

>>>>> doing it slightly differently I've never got it to work well.
>>>>>
>>>>> The yocto [1] bits seem fairly representative of some the patches
>>>>> flying around "to get it working" although I'm not sure how many of=

>>>>> these are actually required and how many are superfluous with this
>>>>> patch set. There seems to be a firmware required that's not
>>>>> distributed with linux-firmware which would also be nice to resolve=
=2E
>>>> Non of these Yocto patches are actually needed. The culprit is the .=
oper_speed setting to be 4Mbps. Once you reduce that to 921600 thing will=
 start to work smoothly. I sent a patch that takes the .oper_speed out co=
mpletely and only applies it for the ACPI based devices where we know tha=
t it works.
>>>>
>>>> With my patch and the right DT entries for uart0 it actually works w=
ith =93btattach -B /dev/ttyAMA0 -P bcm=94. It will load the firmware, con=
figure it and head towards the right path.
>>>>
>>>> Obviously btattach is only an interim step here. Loic=92s patches fo=
r serdev integration and changing the DT to expose uart0 as serial-slave =
for Bluetooth is the right approach. Once Loic=92s resends the patches we=
 can get them into bluetooth-next and start merging these towards upstrea=
m. After that, Bluetooth should work just out of the box like with any US=
B dongle.
>>>>
>>>> And the Yocto patches should be abandoned. If using H:5 (aka 3-Wire)=
 instead of H:4 is possible, we could consider it, but as long as the UAR=
T wiring doesn=92t cause any bit errors, it is not worth it.
>>>>
>>>> That said, I do see a "Bluetooth: hci0: Frame reassembly failed (-84=
)=94 error. I need to figure out where that is. Frankly we really need to=
 hexdump the packet when this happens.
>>>>
>>>
>>> I also meet this Frame reassembly failure, Seems we receive a 0x00 by=
te from the controller (unknown pkt type).
>>
>> that means adding something like this will silence it.
>>
>> +#define BCM_RECV_NULL \
>> +       .type =3D 0x00, \
>> +       .hlen =3D 0, \
>> +       .loff =3D 0, \
>> +       .lsize =3D 0, \
>> +       .maxlen =3D 0
>> +
>> +static int bcm_recv_null(struct hci_dev *hdev, struct sk_buff *skb)
>> +{
>> +       kfree_skb(skb);
>> +       return 0;
>> +}
>> +
>>  static const struct h4_recv_pkt bcm_recv_pkts[] =3D {
>>         { H4_RECV_ACL,      .recv =3D hci_recv_frame },
>>         { H4_RECV_SCO,      .recv =3D hci_recv_frame },
>>         { H4_RECV_EVENT,    .recv =3D hci_recv_frame },
>>         { BCM_RECV_LM_DIAG, .recv =3D hci_recv_diag  },
>> +       { BCM_RECV_NULL,    .recv =3D bcm_recv_null  },
>>  };
>>
>> It does silence it indeed. Maybe this is some of their markers to
>> ensure that the baud rate change worked. Or it is the indication that
>> the reset completed. Do you happen to know between which commands we
>> receive it?
>>
>>> Regarding the speed, I'm unable to reach 3Mbps, I selected 921600
>>> because this is the baudrate used by raspbian bt script.
>>
>> That is the same I experienced. The 921600 works fine, but trying
>> 3Mbps doesn=92t. However it seems with hciattach you can crank it up t=
o
>> 3Mpbs somehow. Maybe the delay is just to short for the UART switching=

>> in that case.
>>
>>> Maybe they had some issues at higher speed (empirical value ?),
>>> However 2Mbps seems ok on my side (need to double check/adjust).
>>
>> We have to make it a DT max-speed param anyway. So I am not that
>> worried.
>>
>>> In a second step, need to check If we can use hardware flow control,
>>> I heard that pin 16&17 are routed to the bcm RTS/CTS.  Since there is=

>>> no DMA usage, it could help.
>
> The BT's CTS/RTS are tied to 3v3/ground.
>
>> Eric, any chance you can dig out the recommendation for the Bluetooth
>> controller usage? I would also like to see the recommended PCM
>> settings for this hardware. It bet it is somehow wired up to the sound=

>> controller.
>
> BT's PCM is connected to gpio 28-31 on the pi3, which you can pinmux to=

> the i2s controller (pcm_gpio28 pin group).
>
> I'm not clear on what you're asking for, or where I would go to find
> anything else.  However, for integration stuff on the board, Phil, Dom,=

> and Gordon at RPi are likely to know more.

Eric is correct - GPIOs 28-31 on the Pi 3B are connected to the PCM funct=
ion on
the 43438. We haven't made use of those pins, and we are unlikely to. On =
the
Zero W, 30 and 31 are connected to the BT RTS and CTS, but on Pi 3B the f=
low control
pins are not connected, hence the baud rate restriction due to the risk o=
f data
loss - particularly likely and fatal during firmware uploaded since at bo=
ot time
systemd is "aggresively parallel". This is the reason for the BlueZ patch=
 that
defers switching to the higher baudrate until after firmware loading is c=
omplete.

Regards,

Phil

^ permalink raw reply	[flat|nested] 50+ messages in thread

end of thread, other threads:[~2017-08-17 20:08 UTC | newest]

Thread overview: 50+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 10:39 [PATCH v3 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth Loic Poulain
2017-08-07 10:39 ` Loic Poulain
     [not found] ` <1502102366-2760-1-git-send-email-loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-08-07 10:39   ` [PATCH v3 2/3] ARM: dts: bcm2837-rpi-3-b: Add bcm43438 as serial slave Loic Poulain
2017-08-07 10:39     ` Loic Poulain
     [not found]     ` <1502102366-2760-2-git-send-email-loic.poulain-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-08-09 23:10       ` Rob Herring
2017-08-09 23:10         ` Rob Herring
     [not found]         ` <CAL_JsqKxA2pgtjmG-cxy5XPWuRWUiia-weRZDSh4++Vn=QZmqA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-10 16:15           ` Marcel Holtmann
2017-08-10 16:15             ` Marcel Holtmann
     [not found]             ` <8186E2A3-D0AC-498F-8229-78862D363F78-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
2017-08-14 13:56               ` Loic Poulain
2017-08-14 13:56                 ` Loic Poulain
     [not found]                 ` <392e6d51-33d6-1585-6582-1397a0c64852-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-08-14 16:02                   ` Marcel Holtmann
2017-08-14 16:02                     ` Marcel Holtmann
     [not found]                     ` <161632C7-EBBF-47F5-86DC-453E10395B54-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
2017-08-14 17:32                       ` Loic Poulain
2017-08-14 17:32                         ` Loic Poulain
     [not found]                         ` <e001e055-10a5-facd-a8e1-57c1bdb78172-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-08-14 17:39                           ` Stefan Wahren
2017-08-14 17:39                             ` Stefan Wahren
2017-08-14 18:36                           ` Marcel Holtmann
2017-08-14 18:36                             ` Marcel Holtmann
2017-08-14 22:16                           ` Marcel Holtmann
2017-08-14 22:16                             ` Marcel Holtmann
     [not found]                             ` <FCD47ED6-F01F-4921-97A3-BBC1D0263747-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
2017-08-15  4:39                               ` Marcel Holtmann
2017-08-15  4:39                                 ` Marcel Holtmann
2017-08-16 12:37                               ` Peter Robinson
2017-08-16 12:37                                 ` Peter Robinson
     [not found]                                 ` <CALeDE9MeMHtCD8esCcznHjS+z6o7Ezh9dP1OZq=77DuYNS2=2A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-16 14:52                                   ` Marcel Holtmann
2017-08-16 14:52                                     ` Marcel Holtmann
     [not found]                                     ` <0C198B33-8CB6-4D6A-B89F-35D3CD771135-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
2017-08-17 10:07                                       ` Loic Poulain
2017-08-17 10:07                                         ` Loic Poulain
     [not found]                                         ` <92579650-5eb0-cdf2-405a-7c5be9c54770-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2017-08-17 10:35                                           ` Marcel Holtmann
2017-08-17 10:35                                             ` Marcel Holtmann
     [not found]                                             ` <8CF7E37E-BF1C-4E32-AF3A-E44BA2E5A2B9-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
2017-08-17 10:47                                               ` Marcel Holtmann
2017-08-17 10:47                                                 ` Marcel Holtmann
2017-08-17 16:21                                               ` Eric Anholt
2017-08-17 16:21                                                 ` Eric Anholt
     [not found]                                                 ` <87shgqgo7b.fsf-omZaPlIz5HhaEpDpdNBo/KxOck334EZe@public.gmane.org>
2017-08-17 17:34                                                   ` Marcel Holtmann
2017-08-17 17:34                                                     ` Marcel Holtmann
     [not found]                                                     ` <E1FFE8C1-97C9-4640-A12A-3AE37F9B655F-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
2017-08-17 19:38                                                       ` Eric Anholt
2017-08-17 19:38                                                         ` Eric Anholt
2017-08-17 20:08                                                   ` Phil Elwell
2017-08-17 20:08                                                     ` Phil Elwell
2017-08-15 13:06       ` Marcel Holtmann
2017-08-15 13:06         ` Marcel Holtmann
     [not found]         ` <43820E05-9D40-4470-AE6C-B7B6C705E0A5-kz+m5ild9QBg9hUCZPvPmw@public.gmane.org>
2017-08-15 19:12           ` Rob Herring
2017-08-15 19:12             ` Rob Herring
2017-08-07 10:39   ` [PATCH v3 3/3] Bluetooth: hci_bcm: Add serdev support Loic Poulain
2017-08-07 10:39     ` Loic Poulain
2017-08-09 23:16   ` [PATCH v3 1/3] dt-bindings: net: bluetooth: Add broadcom-bluetooth Rob Herring
2017-08-09 23:16     ` Rob Herring
     [not found]     ` <CAL_Jsq+akZpqXMPnSh8KRVchWJpFTrzFi6LpOpJ9FYDCrY1hPQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-08-15 17:42       ` Eric Anholt
2017-08-15 17:42         ` Eric Anholt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.