[PATCH 1/3] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth

[PATCH 1/3] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth

[Alistair Francis]

From: Vasily Khoruzhick <anarsoul@gmail.com>

Add binding document for bluetooth part of RTL8723BS/RTL8723CS

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 .../bindings/net/realtek,rtl8723bs-bt.yaml    | 56 +++++++++++++++++++
 1 file changed, 56 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/realtek,rtl8723bs-bt.yaml

diff --git a/Documentation/devicetree/bindings/net/realtek,rtl8723bs-bt.yaml b/Documentation/devicetree/bindings/net/realtek,rtl8723bs-bt.yaml
new file mode 100644
index 000000000000..9e212954f629
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/realtek,rtl8723bs-bt.yaml
@@ -0,0 +1,56 @@
+# SPDX-License-Identifier: GPL-2.0
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/realtek,rtl8723bs-bt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: RTL8723BS/RTL8723CS Bluetooth Device Tree Bindings
+
+maintainers:
+  - Vasily Khoruzhick <anarsoul@gmail.com>
+  - Alistair Francis <alistair@alistair23.me>
+
+description:
+  RTL8723CS/RTL8723CS is WiFi + BT chip. WiFi part is connected over SDIO, while
+  BT is connected over serial. It speaks H5 protocol with few extra commands
+  to upload firmware and change module speed.
+
+properties:
+  compatible:
+    oneOf:
+      - "realtek,rtl8723bs-bt"
+      - "realtek,rtl8723cs-bt"
+
+  device-wake-gpios:
+    description:
+      GPIO specifier, used to wakeup the BT module (active high)
+
+  enable-gpios:
+    description:
+      GPIO specifier, used to enable the BT module (active high)
+
+  host-wake-gpios:
+    desciption:
+      GPIO specifier, used to wakeup the host processor (active high)
+
+firmware-postfix: firmware postfix to be used for firmware config
+reset-gpios: GPIO specifier, used to reset the BT module (active high)
+
+required:
+  - compatible
+
+examples:
+  - |
+    &uart1 {
+        pinctrl-names = "default";
+        pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+        status = "okay";
+
+        bluetooth {
+            compatible = "realtek,rtl8723bs-bt";
+            reset-gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>; /* PL4 */
+            device-wake-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* PL5 */
+            host-wakeup-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+            firmware-postfix="pine64";
+        };
+    };
-- 
2.25.1

[PATCH 2/3] Bluetooth: hci_h5: Add support for binding RTL8723BS with device tree

[Alistair Francis]

From: Vasily Khoruzhick <anarsoul@gmail.com>

RTL8723BS is often used in ARM boards, so add ability to bind it
using device tree.

Signed-off-by: Vasily Khoruzhick <anarsoul@gmail.com>
Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 drivers/bluetooth/hci_h5.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 106c110efe56..6ea6cd73dff4 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -820,6 +820,8 @@ static int h5_serdev_probe(struct serdev_device *serdev)
 			return -ENODEV;
 
 		h5->vnd = (const struct h5_vnd *)data;
+		of_property_read_string(dev.of_node,
+					"firmware-postfix", &h5->id);
 	}
 
 
@@ -1019,6 +1021,8 @@ static const struct of_device_id rtl_bluetooth_of_match[] = {
 	{ .compatible = "realtek,rtl8822cs-bt",
 	  .data = (const void *)&rtl_vnd },
 #endif
+	{ .compatible = "realtek,rtl8822bs-bt",
+	  .data = (const void *)&rtl_vnd },
 	{ },
 };
 MODULE_DEVICE_TABLE(of, rtl_bluetooth_of_match);
-- 
2.25.1

[PATCH 3/3] arm64: allwinner: Enable Bluetooth and WiFi on sopine baseboard

[Alistair Francis]

The sopine board has an optional RTL8723BS WiFi + BT module that can be
connected to UART1. Add this to the device tree so that it will work
for users if connected.

Signed-off-by: Alistair Francis <alistair@alistair23.me>
---
 .../allwinner/sun50i-a64-sopine-baseboard.dts | 24 +++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
index 2f6ea9f3f6a2..5fd370b1e34e 100644
--- a/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
+++ b/arch/arm64/boot/dts/allwinner/sun50i-a64-sopine-baseboard.dts
@@ -103,6 +103,16 @@ ext_rgmii_phy: ethernet-phy@1 {
 	};
 };
 
+&mmc1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&mmc1_pins>;
+	vmmc-supply = <&reg_dldo4>;
+	vqmmc-supply = <&reg_eldo1>;
+	non-removable;
+	bus-width = <4>;
+	status = "okay";
+};
+
 &mmc2 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&mmc2_pins>;
@@ -174,6 +184,20 @@ &uart0 {
 	status = "okay";
 };
 
+&uart1 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
+	status = "okay";
+
+	bluetooth {
+		compatible = "realtek,rtl8723bs-bt";
+		reset-gpios = <&r_pio 0 4 GPIO_ACTIVE_LOW>; /* PL4 */
+		device-wake-gpios = <&r_pio 0 5 GPIO_ACTIVE_HIGH>; /* PL5 */
+		host-wake-gpios = <&r_pio 0 6 GPIO_ACTIVE_HIGH>; /* PL6 */
+		firmware-postfix = "pine64";
+	};
+};
+
 /* On Pi-2 connector */
 &uart2 {
 	pinctrl-names = "default";
-- 
2.25.1

RE: [PATCH 1/3] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth

[Martin Blumenstingl]

Hi Alistair,

+Cc Max Chou, he may be interested in this also

[...]
> @@ -0,0 +1,56 @@
> +# SPDX-License-Identifier: GPL-2.0
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/realtek,rtl8723bs-bt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: RTL8723BS/RTL8723CS Bluetooth Device Tree Bindings
I suggest you also add RTL8822C here (as well as to the compatible enum
and the description below). commit 848fc6164158d6 ("Bluetooth: hci_h5:
btrtl: Add support for RTL8822C") adde support for that chip but didn't
add the dt-binding documentation.

[...]
> +  device-wake-gpios:
> +    description:
> +      GPIO specifier, used to wakeup the BT module (active high)
> +
> +  enable-gpios:
> +    description:
> +      GPIO specifier, used to enable the BT module (active high)
> +
> +  host-wake-gpios:
> +    desciption:
> +      GPIO specifier, used to wakeup the host processor (active high)
regarding all GPIOs here: it entirely depends on the board whether these
are active HIGH or LOW. even though the actual Bluetooth part may
require a specific polarity there can be (for example) a transistor on
the board which could be used to invert the polarity (from the SoC's
view).

also "make dt_binding_check" reports:
  properties:host-wake-gpios: 'maxItems' is a required property
I assume that it'll be the same for the other properties

> +firmware-postfix: firmware postfix to be used for firmware config
there's no other dt-binding that uses "firmware-postfix" yet. However,
there are a few that use "firmware-name". My opinion hasn't changed
since Vasily has posted this series initially: I would not add that
property for now because there seems to be a "standard" config blob
(which works for "all" boards), see Hans' analysis result of the ACPI
config blobs for RTL8723BS: [0].
Getting that "standard" config blob into linux-firmware would be
awesome (I assume licensing is not an issue here, Hans can probably give
more details here). I'm not sure about the licenses of "board specific"
config blobs and whether these can be added to linux-firmware.

also indentation seems wrong here

> +reset-gpios: GPIO specifier, used to reset the BT module (active high)
indentation seems wrong here too

also please note that there is currently no support for this property
inside the hci_h5 driver and you don't seem to add support for it within
this series either. so please double check that the reset GPIO is really
wired up on your sopine board.

> +required:
> +  - compatible
> +
> +examples:
> +  - |
> +    &uart1 {
> +        pinctrl-names = "default";
> +        pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
> +        status = "okay";
AFAIK the "status" property should be omitted from examples

also please add a "uart-has-rtscts" propery, see
Documentation/devicetree/bindings/serial/serial.yaml
Also please update patch #3.


Martin


[0] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/rtl_bt/rtl8723bs_config-OBDA8723.bin?id=e6b9001e91110c654573b8f8e2db6155d10d3b57

Re: [PATCH 1/3] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth

[Alistair Francis]

On 4/04/2020 3:42 pm, Martin Blumenstingl wrote:
> Hi Alistair,
>
> +Cc Max Chou, he may be interested in this also
>
> [...]
>> @@ -0,0 +1,56 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/realtek,rtl8723bs-bt.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: RTL8723BS/RTL8723CS Bluetooth Device Tree Bindings
> I suggest you also add RTL8822C here (as well as to the compatible enum
> and the description below). commit 848fc6164158d6 ("Bluetooth: hci_h5:
> btrtl: Add support for RTL8822C") adde support for that chip but didn't
> add the dt-binding documentation.
>
> [...]
>> +  device-wake-gpios:
>> +    description:
>> +      GPIO specifier, used to wakeup the BT module (active high)
>> +
>> +  enable-gpios:
>> +    description:
>> +      GPIO specifier, used to enable the BT module (active high)
>> +
>> +  host-wake-gpios:
>> +    desciption:
>> +      GPIO specifier, used to wakeup the host processor (active high)
> regarding all GPIOs here: it entirely depends on the board whether these
> are active HIGH or LOW. even though the actual Bluetooth part may
> require a specific polarity there can be (for example) a transistor on
> the board which could be used to invert the polarity (from the SoC's
> view).
>
> also "make dt_binding_check" reports:
>    properties:host-wake-gpios: 'maxItems' is a required property
> I assume that it'll be the same for the other properties
>
>> +firmware-postfix: firmware postfix to be used for firmware config
> there's no other dt-binding that uses "firmware-postfix" yet. However,
> there are a few that use "firmware-name". My opinion hasn't changed
> since Vasily has posted this series initially: I would not add that
> property for now because there seems to be a "standard" config blob
> (which works for "all" boards), see Hans' analysis result of the ACPI
> config blobs for RTL8723BS: [0].
> Getting that "standard" config blob into linux-firmware would be
> awesome (I assume licensing is not an issue here, Hans can probably give
> more details here). I'm not sure about the licenses of "board specific"
> config blobs and whether these can be added to linux-firmware.
>
> also indentation seems wrong here
>
>> +reset-gpios: GPIO specifier, used to reset the BT module (active high)
> indentation seems wrong here too
>
> also please note that there is currently no support for this property
> inside the hci_h5 driver and you don't seem to add support for it within
> this series either. so please double check that the reset GPIO is really
> wired up on your sopine board.
>
>> +required:
>> +  - compatible
>> +
>> +examples:
>> +  - |
>> +    &uart1 {
>> +        pinctrl-names = "default";
>> +        pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
>> +        status = "okay";
> AFAIK the "status" property should be omitted from examples
>
> also please add a "uart-has-rtscts" propery, see
> Documentation/devicetree/bindings/serial/serial.yaml
> Also please update patch #3.
>
>
> Martin
>
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/rtl_bt/rtl8723bs_config-OBDA8723.bin?id=e6b9001e91110c654573b8f8e2db6155d10d3b57

Re: [PATCH 1/3] dt-bindings: net: bluetooth: Add rtl8723bs-bluetooth

[Alistair Francis]

On 4/04/2020 3:42 pm, Martin Blumenstingl wrote:
> Hi Alistair,
>
> +Cc Max Chou, he may be interested in this also
>
> [...]
>> @@ -0,0 +1,56 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/realtek,rtl8723bs-bt.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: RTL8723BS/RTL8723CS Bluetooth Device Tree Bindings
> I suggest you also add RTL8822C here (as well as to the compatible enum
> and the description below). commit 848fc6164158d6 ("Bluetooth: hci_h5:
> btrtl: Add support for RTL8822C") adde support for that chip but didn't
> add the dt-binding documentation.


Done!


>
> [...]
>> +  device-wake-gpios:
>> +    description:
>> +      GPIO specifier, used to wakeup the BT module (active high)
>> +
>> +  enable-gpios:
>> +    description:
>> +      GPIO specifier, used to enable the BT module (active high)
>> +
>> +  host-wake-gpios:
>> +    desciption:
>> +      GPIO specifier, used to wakeup the host processor (active high)
> regarding all GPIOs here: it entirely depends on the board whether these
> are active HIGH or LOW. even though the actual Bluetooth part may
> require a specific polarity there can be (for example) a transistor on
> the board which could be used to invert the polarity (from the SoC's
> view).


I have removed the "(active..." part from the GPIOs.


>
> also "make dt_binding_check" reports:
>    properties:host-wake-gpios: 'maxItems' is a required property
> I assume that it'll be the same for the other properties


Added.


>
>> +firmware-postfix: firmware postfix to be used for firmware config
> there's no other dt-binding that uses "firmware-postfix" yet. However,
> there are a few that use "firmware-name". My opinion hasn't changed
> since Vasily has posted this series initially: I would not add that
> property for now because there seems to be a "standard" config blob
> (which works for "all" boards), see Hans' analysis result of the ACPI
> config blobs for RTL8723BS: [0].


I have removed the 'firmware-postfix" part from this series.


> Getting that "standard" config blob into linux-firmware would be
> awesome (I assume licensing is not an issue here, Hans can probably give
> more details here). I'm not sure about the licenses of "board specific"
> config blobs and whether these can be added to linux-firmware.
>
> also indentation seems wrong here
>
>> +reset-gpios: GPIO specifier, used to reset the BT module (active high)
> indentation seems wrong here too


Fixed.


>
> also please note that there is currently no support for this property
> inside the hci_h5 driver and you don't seem to add support for it within
> this series either. so please double check that the reset GPIO is really
> wired up on your sopine board.


Removed.


>
>> +required:
>> +  - compatible
>> +
>> +examples:
>> +  - |
>> +    &uart1 {
>> +        pinctrl-names = "default";
>> +        pinctrl-0 = <&uart1_pins>, <&uart1_rts_cts_pins>;
>> +        status = "okay";
> AFAIK the "status" property should be omitted from examples


Removed.


> Z
> also please add a "uart-has-rtscts" propery, see
> Documentation/devicetree/bindings/serial/serial.yaml
> Also please update patch #3.


Added.


Thanks for the review.


Alistair


>
>
> Martin
>
>
> [0] https://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git/commit/rtl_bt/rtl8723bs_config-OBDA8723.bin?id=e6b9001e91110c654573b8f8e2db6155d10d3b57