[PATCH v2 0/3] Add Bluetooth support to Teres A64 I

[PATCH v2 0/3] Add Bluetooth support to Teres A64 I

[Hugo Grostabussiat]

The Teres A64 I laptop comes equipped with a serial-attached Realtek
rtl8723bs Bluetooth controller. This series creates the DT binding for
the the Realtek vendor extension of the HCI H5 driver, for which ACPI
bindings already exist, and enable support for the Teres A64 I board.

The first patch adds the DT binding documentation.
The second one implements such binding in the HCI H5 driver.
The last patch adds the appropriate device node to the Teres A64 I dts
file to enable Bluetooth.

Changes since v1:
- Adjusted driver code following advice from Marcel Holtmann (thanks for
  your time!)
- The DT bindings are documented for all serial attached Realtek controllers
  instead of juste the rtl8723bs
- Made the "config-name" property generic by removing the vendor prefix
- Kconfig change to make BT_HCIUART_RTL depend on either ACPI or OF

Hugo Grostabussiat (3):
  dt-bindings: net: bluetooth: add DT binding for Realtek controllers
  Bluetooth: hci_h5: Add DT support for rtl8723bs
  arm64: dts: allwinner: a64: Enable Bluetooth on Teres-I

 .../bindings/net/realtek-bluetooth.txt        | 25 ++++++++++++
 .../boot/dts/allwinner/sun50i-a64-teres-i.dts | 13 ++++++
 drivers/bluetooth/Kconfig                     |  2 +-
 drivers/bluetooth/hci_h5.c                    | 40 ++++++++++++++++---
 4 files changed, 74 insertions(+), 6 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt

-- 
2.23.0

[PATCH v2 1/3] dt-bindings: net: bluetooth: add DT binding for Realtek controllers

[Hugo Grostabussiat]

The rtl_bt driver already supports some Realtek controllers on ACPI
platforms.
This commit adds bindings for DT-only platforms.

Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
---
 .../bindings/net/realtek-bluetooth.txt        | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt

diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
new file mode 100644
index 000000000000..01d4ed146705
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
@@ -0,0 +1,25 @@
+Realtek Bluetooth controllers
+=============================
+
+This documents the binding structure and properties for the serial
+attached Bluetooth controllers from Realtek.
+
+Required properties:
+- compatible: currently, only "realtek,rt8723bs-bt" is supported
+
+Optional properties:
+- enable-gpio: gpio line controlling the power down (BT_DIS#) signal
+- device-wake: gpio line controlling the device wakeup (BT_WAKE) signal
+- config-name: postfix added to the name of the firmware file
+  containing the chip configuration
+
+Example:
+
+&uart1 {
+	bluetooth {
+		compatible = "realtek,rtl8723bs-bt";
+		enable-gpio = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
+		device-wake-gpio = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+		config-name = "teres_a64_i";
+	};
+};
-- 
2.23.0

Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: add DT binding for Realtek controllers

[Maxime Ripard]

Hi Hugo,

On Wed, Oct 30, 2019 at 11:43:31PM +0100, Hugo Grostabussiat wrote:
> The rtl_bt driver already supports some Realtek controllers on ACPI
> platforms.
> This commit adds bindings for DT-only platforms.
>
> Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
> ---
>  .../bindings/net/realtek-bluetooth.txt        | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt

You should write that binding using a YAML description. Free-form
device tree bindings are more or less deprecated now.

> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
> new file mode 100644
> index 000000000000..01d4ed146705
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
> @@ -0,0 +1,25 @@
> +Realtek Bluetooth controllers
> +=============================
> +
> +This documents the binding structure and properties for the serial
> +attached Bluetooth controllers from Realtek.
> +
> +Required properties:
> +- compatible: currently, only "realtek,rt8723bs-bt" is supported
> +
> +Optional properties:
> +- enable-gpio: gpio line controlling the power down (BT_DIS#) signal
> +- device-wake: gpio line controlling the device wakeup (BT_WAKE) signal
> +- config-name: postfix added to the name of the firmware file
> +  containing the chip configuration
> +
> +Example:
> +
> +&uart1 {
> +	bluetooth {
> +		compatible = "realtek,rtl8723bs-bt";
> +		enable-gpio = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
> +		device-wake-gpio = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
> +		config-name = "teres_a64_i";

IIRC, that has been discussed before and the standard "model" property
was to be preferred.

Maxime

Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: add DT binding for Realtek controllers

[Marcel Holtmann]

Hi Hugo,

>> The rtl_bt driver already supports some Realtek controllers on ACPI
>> platforms.
>> This commit adds bindings for DT-only platforms.
>> 
>> Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
>> ---
>> .../bindings/net/realtek-bluetooth.txt        | 25 +++++++++++++++++++
>> 1 file changed, 25 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt
> 
> You should write that binding using a YAML description. Free-form
> device tree bindings are more or less deprecated now.

unless we change all the Bluetooth descriptions, I prefer we keep it the “old” way.

> IIRC, that has been discussed before and the standard "model" property
> was to be preferred.

This one should really get an ACK from Rob.

Regards

Marcel

Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: add DT binding for Realtek controllers

[Maxime Ripard]

On Mon, Nov 04, 2019 at 03:20:59PM +0100, Marcel Holtmann wrote:
> >> The rtl_bt driver already supports some Realtek controllers on ACPI
> >> platforms.
> >> This commit adds bindings for DT-only platforms.
> >>
> >> Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
> >> ---
> >> .../bindings/net/realtek-bluetooth.txt        | 25 +++++++++++++++++++
> >> 1 file changed, 25 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt
> >
> > You should write that binding using a YAML description. Free-form
> > device tree bindings are more or less deprecated now.
>
> unless we change all the Bluetooth descriptions, I prefer we keep it the “old” way.

FWIW, this is a warning in checkpatch now.

Maxime

Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: add DT binding for Realtek controllers

[Rob Herring]

On Mon, Nov 04, 2019 at 03:20:59PM +0100, Marcel Holtmann wrote:
> Hi Hugo,
> 
> >> The rtl_bt driver already supports some Realtek controllers on ACPI
> >> platforms.
> >> This commit adds bindings for DT-only platforms.
> >> 
> >> Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
> >> ---
> >> .../bindings/net/realtek-bluetooth.txt        | 25 +++++++++++++++++++
> >> 1 file changed, 25 insertions(+)
> >> create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt
> > 
> > You should write that binding using a YAML description. Free-form
> > device tree bindings are more or less deprecated now.
> 
> unless we change all the Bluetooth descriptions, I prefer we keep it the “old” way.

Who's going to do that? Me? You? Randomly select some poor soul to do 
them all to get their device added? I only have 3500 to do. An all at 
once approach doesn't really work nor is it necessary.

I'd suggest new ones in schema and if you want to encourage conversions 
require any changes on existing ones to first be converted. Still up to 
you because it goes thru your tree.


> > IIRC, that has been discussed before and the standard "model" property
> > was to be preferred.
> 
> This one should really get an ACK from Rob.

Humm, maybe, but I don't see 'model' used in this context.

Rob

Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: add DT binding for Realtek controllers

[Rob Herring]

On Wed, Oct 30, 2019 at 11:43:31PM +0100, Hugo Grostabussiat wrote:
> The rtl_bt driver already supports some Realtek controllers on ACPI
> platforms.
> This commit adds bindings for DT-only platforms.
> 
> Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
> ---
>  .../bindings/net/realtek-bluetooth.txt        | 25 +++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt
> 
> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
> new file mode 100644
> index 000000000000..01d4ed146705
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth.txt
> @@ -0,0 +1,25 @@
> +Realtek Bluetooth controllers
> +=============================
> +
> +This documents the binding structure and properties for the serial
> +attached Bluetooth controllers from Realtek.
> +
> +Required properties:
> +- compatible: currently, only "realtek,rt8723bs-bt" is supported
> +
> +Optional properties:
> +- enable-gpio: gpio line controlling the power down (BT_DIS#) signal

enable-gpios

Though based on the pin name, powerdown-gpios may be more appropriate. 

> +- device-wake: gpio line controlling the device wakeup (BT_WAKE) signal

device-wake-gpios

> +- config-name: postfix added to the name of the firmware file
> +  containing the chip configuration

'firmware-name' is the common property for this. It's the full filename 
which is more flexible. 

What's the default name?

> +
> +Example:
> +
> +&uart1 {
> +	bluetooth {
> +		compatible = "realtek,rtl8723bs-bt";
> +		enable-gpio = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
> +		device-wake-gpio = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */

PL4 and PL6 aren't meaningful in this context.

> +		config-name = "teres_a64_i";
> +	};
> +};
> -- 
> 2.23.0
> 

Re: [PATCH v2 1/3] dt-bindings: net: bluetooth: add DT binding for Realtek controllers

[Hugo "Bonstra" Grostabussiat]

Le 05/11/2019 à 23:17, Rob Herring a écrit :
> On Wed, Oct 30, 2019 at 11:43:31PM +0100, Hugo Grostabussiat wrote:
>> The rtl_bt driver already supports some Realtek controllers on ACPI
>> platforms.
>> This commit adds bindings for DT-only platforms.
>>
>> Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
>> ---
>>  .../bindings/net/realtek-bluetooth.txt        | 25 +++++++++++++++++++
>>  1 file changed, 25 insertions(+)
>>  create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth.txt

>> +- config-name: postfix added to the name of the firmware file
>> +  containing the chip configuration
> 
> 'firmware-name' is the common property for this. It's the full filename 
> which is more flexible. 
> 
> What's the default name?
Depending on the chip, both a firmware file and a configuration file may
be required; the RTL8723BS is one of those chips.

For the firmware file, the default name is of the form:
rtl_bt/${CHIP_NAME}_fw.bin

For the config file:
rtl_bt/${CHIP_NAME}_config

[PATCH v2 2/3] Bluetooth: hci_h5: Add DT support for rtl8723bs

[Hugo Grostabussiat]

The hci_h5 already supports Realtek controllers discovered via ACPI. This
commit adds support for discovering via device tree for ACPI-less
platforms.

Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
---
 drivers/bluetooth/Kconfig  |  2 +-
 drivers/bluetooth/hci_h5.c | 40 +++++++++++++++++++++++++++++++++-----
 2 files changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
index aae665a3a254..48c9a004b033 100644
--- a/drivers/bluetooth/Kconfig
+++ b/drivers/bluetooth/Kconfig
@@ -211,7 +211,7 @@ config BT_HCIUART_RTL
 	depends on BT_HCIUART
 	depends on BT_HCIUART_SERDEV
 	depends on GPIOLIB
-	depends on ACPI
+	depends on (ACPI || OF)
 	select BT_HCIUART_3WIRE
 	select BT_RTL
 	help
diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index dacf297baf59..d4aceddaaab2 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -11,6 +11,7 @@
 #include <linux/gpio/consumer.h>
 #include <linux/kernel.h>
 #include <linux/mod_devicetable.h>
+#include <linux/of_device.h>
 #include <linux/serdev.h>
 #include <linux/skbuff.h>
 
@@ -782,7 +783,6 @@ static const struct hci_uart_proto h5p = {
 
 static int h5_serdev_probe(struct serdev_device *serdev)
 {
-	const struct acpi_device_id *match;
 	struct device *dev = &serdev->dev;
 	struct h5 *h5;
 
@@ -797,16 +797,32 @@ static int h5_serdev_probe(struct serdev_device *serdev)
 	serdev_device_set_drvdata(serdev, h5);
 
 	if (has_acpi_companion(dev)) {
-		match = acpi_match_device(dev->driver->acpi_match_table, dev);
-		if (!match)
+		const struct acpi_device_id *acpi_match;
+
+		acpi_match = acpi_match_device(
+				dev->driver->acpi_match_table, dev);
+		if (!acpi_match)
 			return -ENODEV;
 
-		h5->vnd = (const struct h5_vnd *)match->driver_data;
-		h5->id  = (char *)match->id;
+		h5->vnd = (const struct h5_vnd *)acpi_match->driver_data;
+		h5->id  = (char *)acpi_match->id;
 
 		if (h5->vnd->acpi_gpio_map)
 			devm_acpi_dev_add_driver_gpios(dev,
 						       h5->vnd->acpi_gpio_map);
+	} else if (dev->of_node) {
+		const struct of_device_id *of_match;
+		const char *cfgname = NULL;
+
+		of_match = of_match_device(dev->driver->of_match_table, dev);
+		if (!of_match)
+			return -ENODEV;
+
+		of_property_read_string(dev->of_node,
+					"config-name", &cfgname);
+
+		h5->vnd = (const struct h5_vnd *)of_match->data;
+		h5->id = cfgname;
 	}
 
 	h5->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
@@ -996,6 +1012,19 @@ static const struct acpi_device_id h5_acpi_match[] = {
 MODULE_DEVICE_TABLE(acpi, h5_acpi_match);
 #endif
 
+#ifdef CONFIG_OF
+static const struct of_device_id h5_of_match[] = {
+#ifdef CONFIG_BT_HCIUART_RTL
+	{
+		.compatible = "realtek,rtl8723bs-bt",
+		.data = &rtl_vnd
+	},
+#endif
+	{ },
+};
+MODULE_DEVICE_TABLE(of, h5_of_match);
+#endif
+
 static const struct dev_pm_ops h5_serdev_pm_ops = {
 	SET_SYSTEM_SLEEP_PM_OPS(h5_serdev_suspend, h5_serdev_resume)
 };
@@ -1006,6 +1035,7 @@ static struct serdev_device_driver h5_serdev_driver = {
 	.driver = {
 		.name = "hci_uart_h5",
 		.acpi_match_table = ACPI_PTR(h5_acpi_match),
+		.of_match_table = of_match_ptr(h5_of_match),
 		.pm = &h5_serdev_pm_ops,
 	},
 };
-- 
2.23.0

Re: [PATCH v2 2/3] Bluetooth: hci_h5: Add DT support for rtl8723bs

[Marcel Holtmann]

Hi Hugo,

> The hci_h5 already supports Realtek controllers discovered via ACPI. This
> commit adds support for discovering via device tree for ACPI-less
> platforms.
> 
> Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
> ---
> drivers/bluetooth/Kconfig  |  2 +-
> drivers/bluetooth/hci_h5.c | 40 +++++++++++++++++++++++++++++++++-----
> 2 files changed, 36 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/bluetooth/Kconfig b/drivers/bluetooth/Kconfig
> index aae665a3a254..48c9a004b033 100644
> --- a/drivers/bluetooth/Kconfig
> +++ b/drivers/bluetooth/Kconfig
> @@ -211,7 +211,7 @@ config BT_HCIUART_RTL
> 	depends on BT_HCIUART
> 	depends on BT_HCIUART_SERDEV
> 	depends on GPIOLIB
> -	depends on ACPI
> +	depends on (ACPI || OF)
> 	select BT_HCIUART_3WIRE
> 	select BT_RTL
> 	help
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index dacf297baf59..d4aceddaaab2 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -11,6 +11,7 @@
> #include <linux/gpio/consumer.h>
> #include <linux/kernel.h>
> #include <linux/mod_devicetable.h>
> +#include <linux/of_device.h>
> #include <linux/serdev.h>
> #include <linux/skbuff.h>
> 
> @@ -782,7 +783,6 @@ static const struct hci_uart_proto h5p = {
> 
> static int h5_serdev_probe(struct serdev_device *serdev)
> {
> -	const struct acpi_device_id *match;
> 	struct device *dev = &serdev->dev;
> 	struct h5 *h5;
> 
> @@ -797,16 +797,32 @@ static int h5_serdev_probe(struct serdev_device *serdev)
> 	serdev_device_set_drvdata(serdev, h5);
> 
> 	if (has_acpi_companion(dev)) {
> -		match = acpi_match_device(dev->driver->acpi_match_table, dev);
> -		if (!match)
> +		const struct acpi_device_id *acpi_match;
> +
> +		acpi_match = acpi_match_device(
> +				dev->driver->acpi_match_table, dev);

don’t bother with a different variable name here. Keep it match.

> +		if (!acpi_match)
> 			return -ENODEV;
> 
> -		h5->vnd = (const struct h5_vnd *)match->driver_data;
> -		h5->id  = (char *)match->id;
> +		h5->vnd = (const struct h5_vnd *)acpi_match->driver_data;
> +		h5->id  = (char *)acpi_match->id;
> 
> 		if (h5->vnd->acpi_gpio_map)
> 			devm_acpi_dev_add_driver_gpios(dev,
> 						       h5->vnd->acpi_gpio_map);
> +	} else if (dev->of_node) {
> +		const struct of_device_id *of_match;

Same here. Just keep the name as match.

> +		const char *cfgname = NULL;
> +
> +		of_match = of_match_device(dev->driver->of_match_table, dev);
> +		if (!of_match)
> +			return -ENODEV;
> +
> +		of_property_read_string(dev->of_node,
> +					"config-name", &cfgname);
> +
> +		h5->vnd = (const struct h5_vnd *)of_match->data;
> +		h5->id = cfgname;
> 	}
> 
> 	h5->enable_gpio = devm_gpiod_get_optional(dev, "enable", GPIOD_OUT_LOW);
> @@ -996,6 +1012,19 @@ static const struct acpi_device_id h5_acpi_match[] = {
> MODULE_DEVICE_TABLE(acpi, h5_acpi_match);
> #endif
> 
> +#ifdef CONFIG_OF
> +static const struct of_device_id h5_of_match[] = {
> +#ifdef CONFIG_BT_HCIUART_RTL
> +	{
> +		.compatible = "realtek,rtl8723bs-bt",
> +		.data = &rtl_vnd
> +	},
> +#endif
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(of, h5_of_match);
> +#endif
> +
> static const struct dev_pm_ops h5_serdev_pm_ops = {
> 	SET_SYSTEM_SLEEP_PM_OPS(h5_serdev_suspend, h5_serdev_resume)
> };
> @@ -1006,6 +1035,7 @@ static struct serdev_device_driver h5_serdev_driver = {
> 	.driver = {
> 		.name = "hci_uart_h5",
> 		.acpi_match_table = ACPI_PTR(h5_acpi_match),
> +		.of_match_table = of_match_ptr(h5_of_match),
> 		.pm = &h5_serdev_pm_ops,
> 	},
> };

Regards

Marcel

[PATCH v2 3/3] arm64: dts: allwinner: a64: Enable Bluetooth on Teres-I

[Hugo Grostabussiat]

The UART1 on the Teres-A64-I is connected to a rtl8723bs combo
WLAN/Bluetooth controller, with three GPIOs used for device reset,
host wake up and device wake up.

Currently, the host wake up feature is not supported by the HCI H5
driver.

Signed-off-by: Hugo Grostabussiat <bonstra@bonstra.fr.eu.org>
---
 .../arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
index 1069e7012c9c..038e4f0e07df 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-teres-i.dts
@@ -325,6 +325,19 @@
 	status = "okay";
 };
 
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+	status = "okay";
+
+	bluetooth {
+		compatible = "realtek,rtl8723bs-bt";
+		enable-gpio = <&r_pio 0 4 GPIO_ACTIVE_HIGH>; /* PL4 */
+		device-wake-gpio = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+		config-name = "teres_a64_i";
+	};
+};
+
 &usbphy {
 	usb1_vbus-supply = <&reg_usb1_vbus>;
 	status = "okay";
-- 
2.23.0