[PATCH v4 1/2] Bluetooth: hci_h5: Add the Realtek compatible flags for the device tree

[PATCH v4 1/2] Bluetooth: hci_h5: Add the Realtek compatible flags for the device tree

[David Summers]

With the changes requested by Marcel Holtmann and Rob Herring.

The patch is unchanged, but subject changed to the norm.

Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>
---
 drivers/bluetooth/hci_h5.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
index 8eede1197cd2..056ab2decccc 100644
--- a/drivers/bluetooth/hci_h5.c
+++ b/drivers/bluetooth/hci_h5.c
@@ -925,6 +925,22 @@ static struct h5_vnd rtl_vnd = {
 };
 #endif
 
+#ifdef CONFIG_OF
+static const struct of_device_id h5_of_match[] = {
+#ifdef CONFIG_BT_HCIUART_RTL
+       { .compatible = "realtek,rtl8723as-bt"},
+       { .compatible = "realtek,rtl8723bs-bt"},
+       { .compatible = "realtek,rtl8723ds-bt"},
+       { .compatible = "realtek,rtl8761atv"},
+       { .compatible = "realtek,rtl8821as-bt"},
+       { .compatible = "realtek,rtl8821cs-bt"},
+       { .compatible = "realtek,rtl8822bs-bt"},
+#endif
+       { }
+};
+MODULE_DEVICE_TABLE(of, h5_of_match);
+#endif
+
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id h5_acpi_match[] = {
 #ifdef CONFIG_BT_HCIUART_RTL
-- 
beagleboard@davidjohnsummers.uk

[PATCH v4 2/2] dt-bindings: Create the file for Realtek Bluetooth serial devices

[David Summers]

With the changes requested by Marcel Holtmann and Rob Herring.

Capitalisation as requested.

Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>
---
 .../bindings/net/realtek-bluetooth-serial.txt | 32 +++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt

diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
new file mode 100644
index 000000000000..2eddde1a0cf1
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
@@ -0,0 +1,32 @@
+Realtek Bluetooth devices connected via a UART.
+These devices typically also have a WI-FI connected via SDIO - the
+compatible described here is used just for referencing the Bluetooth.
+
+- compatible: should be "realtek,<name>-bt"
+  except for "realtek,trl8761atv" - which only has a serial Bluetooth connection
+       "realtek,rtl8723as-bt"
+       "realtek,rtl8723bs-bt"
+       "realtek,rtl8723ds-bt"
+       "realtek,rtl8761atv"
+       "realtek,rtl8821as-bt"
+       "realtek,rtl8821cs-bt"
+       "realtek,rtl8822bs-bt"
+
+Example:
+
+&uart0 {
+	status = "okay";
+	pinctrl-0 = <&uart0_xfer>, <&uart0_cts>;
+	bluetooth {
+		compatible = "realtek,rtl8723bs-bt";
+		uart_rts_gpios = <&gpio4 19 GPIO_ACTIVE_LOW>;
+		pinctrl-names = "default","rts_gpio";
+		pinctrl-0 = <&uart0_rts>;
+		pinctrl-1 = <&uart0_gpios>;
+		BT,reset_gpio    = <&gpio4 29 GPIO_ACTIVE_HIGH>;
+		BT,wake_gpio     = <&gpio4 26 GPIO_ACTIVE_HIGH>;
+		BT,wake_host_irq = <&gpio4 31 GPIO_ACTIVE_HIGH>;
+	};
+};
+
+this ensures that the Bluetooth device is tied to the correct uart.
-- 
beagleboard@davidjohnsummers.uk

Re: [PATCH v4 2/2] dt-bindings: Create the file for Realtek Bluetooth serial devices

[Chen-Yu Tsai]

Hi,

Interested party here. I'd like to get this working on the Pine64 which has
an RTL8723BS module. I had made some changes and had it working but didn't
get around to doing the patches yet.

On Tue, Jan 22, 2019 at 4:39 AM David Summers
<beagleboard@davidjohnsummers.uk> wrote:
>
> With the changes requested by Marcel Holtmann and Rob Herring.
>
> Capitalisation as requested.
>
> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>

The commit log is a bit lacking, as it doesn't provide any context or details
relevant to the changes in this patch, but also has details that should be
in the changelog, and not the commit log.

> ---
>  .../bindings/net/realtek-bluetooth-serial.txt | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>
> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
> new file mode 100644
> index 000000000000..2eddde1a0cf1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
> @@ -0,0 +1,32 @@
> +Realtek Bluetooth devices connected via a UART.
> +These devices typically also have a WI-FI connected via SDIO - the
> +compatible described here is used just for referencing the Bluetooth.
> +
> +- compatible: should be "realtek,<name>-bt"
> +  except for "realtek,trl8761atv" - which only has a serial Bluetooth connection
> +       "realtek,rtl8723as-bt"
> +       "realtek,rtl8723bs-bt"
> +       "realtek,rtl8723ds-bt"
> +       "realtek,rtl8761atv"
> +       "realtek,rtl8821as-bt"
> +       "realtek,rtl8821cs-bt"
> +       "realtek,rtl8822bs-bt"
> +
> +Example:
> +
> +&uart0 {
> +       status = "okay";
> +       pinctrl-0 = <&uart0_xfer>, <&uart0_cts>;
> +       bluetooth {
> +               compatible = "realtek,rtl8723bs-bt";
> +               uart_rts_gpios = <&gpio4 19 GPIO_ACTIVE_LOW>;
> +               pinctrl-names = "default","rts_gpio";
> +               pinctrl-0 = <&uart0_rts>;
> +               pinctrl-1 = <&uart0_gpios>;
> +               BT,reset_gpio    = <&gpio4 29 GPIO_ACTIVE_HIGH>;
> +               BT,wake_gpio     = <&gpio4 26 GPIO_ACTIVE_HIGH>;
> +               BT,wake_host_irq = <&gpio4 31 GPIO_ACTIVE_HIGH>;

You haven't listed these three properties in the bindings.

Second, for the last one, the property name says irq, but it takes
a GPIO phandle. This is slightly misleading.

Furthermore, ACPI already uses the names "device-wake-gpios",
"enable-gpios", and "host-wake-gpios". Would it be possible to
stick to those?

Last, you don't actually add support for them in the driver.

So why are they there?

Regards
ChenYu

> +       };
> +};
> +
> +this ensures that the Bluetooth device is tied to the correct uart.
> --
> beagleboard@davidjohnsummers.uk
>

Re: [PATCH v4 2/2] dt-bindings: Create the file for Realtek Bluetooth serial devices

[David Summers]

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

Hi Chen-Yu,

Thanks for the interest, and sorry for the slow reply - I've been away 
on business.

The latter was why my patches were so terse. I'd done v3 at the weekend:

https://www.spinics.net/lists/linux-bluetooth/msg78694.html

But Rob and Marcel wanted minor changes:

https://www.spinics.net/lists/linux-bluetooth/msg78706.html
https://www.spinics.net/lists/linux-bluetooth/msg78713.html

Now as I was going away, I quickly made the changes requested, and 
posted night before I left.

Your questions are good, but without a clear answer. What I will say is 
that

  * the device drive hci_h5 is a bluetooth hci 3 wire driver
  * I take this to be GND,RX,TX
  * I also assume this means BCSP
  * BCSP is either 3 wire or 5 wire
  * The device tree hooks I took from

    https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678

  * a patch direct from ASUS, and so I take it this is how the board is
    wired up.
  * If you look at the datasheet:
    http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf
  * You can see as well and RX and TX on pin 42,42 there is a UART_CTS
    on 44,  BT_WAKE on 6, BT_HOST_WAKE
  * So I'm guessing these are wired up as described in the asus device tree

Now my problem is I don't have a board, I'm trying to mainline this your 
Arm Arch users.

So really this asks the question as to if the hci_h5 is the correct 
driver for the RTL8723BS; I think hci_h5 is intended just for 3 wire - 
so probably can't get changes there. Does make you wonder if there 
should be a more general driver?

Anyway, I'm not expert here. Really just want to get this chip supported 
in the device tree. So I just did the minimal to load the module.

If you have any thoughts though, do let the thread know.

Regards,

David.




On 22/01/2019 02:40, Chen-Yu Tsai wrote:
> Hi,
>
> Interested party here. I'd like to get this working on the Pine64 which has
> an RTL8723BS module. I had made some changes and had it working but didn't
> get around to doing the patches yet.
>
> On Tue, Jan 22, 2019 at 4:39 AM David Summers
> <beagleboard@davidjohnsummers.uk> wrote:
>> With the changes requested by Marcel Holtmann and Rob Herring.
>>
>> Capitalisation as requested.
>>
>> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>
> The commit log is a bit lacking, as it doesn't provide any context or details
> relevant to the changes in this patch, but also has details that should be
> in the changelog, and not the commit log.
>
>> ---
>>   .../bindings/net/realtek-bluetooth-serial.txt | 32 +++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>> new file mode 100644
>> index 000000000000..2eddde1a0cf1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>> @@ -0,0 +1,32 @@
>> +Realtek Bluetooth devices connected via a UART.
>> +These devices typically also have a WI-FI connected via SDIO - the
>> +compatible described here is used just for referencing the Bluetooth.
>> +
>> +- compatible: should be "realtek,<name>-bt"
>> +  except for "realtek,trl8761atv" - which only has a serial Bluetooth connection
>> +       "realtek,rtl8723as-bt"
>> +       "realtek,rtl8723bs-bt"
>> +       "realtek,rtl8723ds-bt"
>> +       "realtek,rtl8761atv"
>> +       "realtek,rtl8821as-bt"
>> +       "realtek,rtl8821cs-bt"
>> +       "realtek,rtl8822bs-bt"
>> +
>> +Example:
>> +
>> +&uart0 {
>> +       status = "okay";
>> +       pinctrl-0 = <&uart0_xfer>, <&uart0_cts>;
>> +       bluetooth {
>> +               compatible = "realtek,rtl8723bs-bt";
>> +               uart_rts_gpios = <&gpio4 19 GPIO_ACTIVE_LOW>;
>> +               pinctrl-names = "default","rts_gpio";
>> +               pinctrl-0 = <&uart0_rts>;
>> +               pinctrl-1 = <&uart0_gpios>;
>> +               BT,reset_gpio    = <&gpio4 29 GPIO_ACTIVE_HIGH>;
>> +               BT,wake_gpio     = <&gpio4 26 GPIO_ACTIVE_HIGH>;
>> +               BT,wake_host_irq = <&gpio4 31 GPIO_ACTIVE_HIGH>;
> You haven't listed these three properties in the bindings.
>
> Second, for the last one, the property name says irq, but it takes
> a GPIO phandle. This is slightly misleading.
>
> Furthermore, ACPI already uses the names "device-wake-gpios",
> "enable-gpios", and "host-wake-gpios". Would it be possible to
> stick to those?
>
> Last, you don't actually add support for them in the driver.
>
> So why are they there?
>
> Regards
> ChenYu
>
>> +       };
>> +};
>> +
>> +this ensures that the Bluetooth device is tied to the correct uart.
>> --
>> beagleboard@davidjohnsummers.uk
>>


[-- Attachment #2: Type: text/html, Size: 7563 bytes --]

Re: [PATCH v4 2/2] dt-bindings: Create the file for Realtek Bluetooth serial devices

[David Summers]

Hi Chen-Yu,

Thanks for the interest, and sorry for the slow reply - I've been away 
on business.

The latter was why my patches were so terse. I'd done v3 at the weekend:

https://www.spinics.net/lists/linux-bluetooth/msg78694.html

But Rob and Marcel wanted minor changes:

https://www.spinics.net/lists/linux-bluetooth/msg78706.html
https://www.spinics.net/lists/linux-bluetooth/msg78713.html

Now as I was going away, I quickly made the changes requested, and 
posted night before I left.

Your questions are good, but without a clear answer. What I will say is 
that

  * the device drive hci_h5 is a bluetooth hci 3 wire driver
  * I take this to be GND,RX,TX
  * I also assume this means BCSP
  * BCSP is either 3 wire or 5 wire
  * The device tree hooks I took from

    https://github.com/TinkerBoard/debian_kernel/commit/6a3128ade33f758887048578ada61a4b7ab8e678

  * a patch direct from ASUS, and so I take it this is how the board is
    wired up.
  * If you look at the datasheet:
    http://files.pine64.org/doc/datasheet/pine64/RTL8723BS.pdf
  * You can see as well and RX and TX on pin 42,42 there is a UART_CTS
    on 44,  BT_WAKE on 6, BT_HOST_WAKE
  * So I'm guessing these are wired up as described in the asus device tree

Now my problem is I don't have a board, I'm trying to mainline this your 
Arm Arch users.

So really this asks the question as to if the hci_h5 is the correct 
driver for the RTL8723BS; I think hci_h5 is intended just for 3 wire - 
so probably can't get changes there. Does make you wonder if there 
should be a more general driver?

Anyway, I'm not expert here. Really just want to get this chip supported 
in the device tree. So I just did the minimal to load the module.

If you have any thoughts though, do let the thread know.

Regards,

David.


On 22/01/2019 02:40, Chen-Yu Tsai wrote:
> Hi,
>
> Interested party here. I'd like to get this working on the Pine64 which has
> an RTL8723BS module. I had made some changes and had it working but didn't
> get around to doing the patches yet.
>
> On Tue, Jan 22, 2019 at 4:39 AM David Summers
> <beagleboard@davidjohnsummers.uk> wrote:
>> With the changes requested by Marcel Holtmann and Rob Herring.
>>
>> Capitalisation as requested.
>>
>> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>
> The commit log is a bit lacking, as it doesn't provide any context or details
> relevant to the changes in this patch, but also has details that should be
> in the changelog, and not the commit log.
>
>> ---
>>   .../bindings/net/realtek-bluetooth-serial.txt | 32 +++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>> new file mode 100644
>> index 000000000000..2eddde1a0cf1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>> @@ -0,0 +1,32 @@
>> +Realtek Bluetooth devices connected via a UART.
>> +These devices typically also have a WI-FI connected via SDIO - the
>> +compatible described here is used just for referencing the Bluetooth.
>> +
>> +- compatible: should be "realtek,<name>-bt"
>> +  except for "realtek,trl8761atv" - which only has a serial Bluetooth connection
>> +       "realtek,rtl8723as-bt"
>> +       "realtek,rtl8723bs-bt"
>> +       "realtek,rtl8723ds-bt"
>> +       "realtek,rtl8761atv"
>> +       "realtek,rtl8821as-bt"
>> +       "realtek,rtl8821cs-bt"
>> +       "realtek,rtl8822bs-bt"
>> +
>> +Example:
>> +
>> +&uart0 {
>> +       status = "okay";
>> +       pinctrl-0 = <&uart0_xfer>, <&uart0_cts>;
>> +       bluetooth {
>> +               compatible = "realtek,rtl8723bs-bt";
>> +               uart_rts_gpios = <&gpio4 19 GPIO_ACTIVE_LOW>;
>> +               pinctrl-names = "default","rts_gpio";
>> +               pinctrl-0 = <&uart0_rts>;
>> +               pinctrl-1 = <&uart0_gpios>;
>> +               BT,reset_gpio    = <&gpio4 29 GPIO_ACTIVE_HIGH>;
>> +               BT,wake_gpio     = <&gpio4 26 GPIO_ACTIVE_HIGH>;
>> +               BT,wake_host_irq = <&gpio4 31 GPIO_ACTIVE_HIGH>;
> You haven't listed these three properties in the bindings.
>
> Second, for the last one, the property name says irq, but it takes
> a GPIO phandle. This is slightly misleading.
>
> Furthermore, ACPI already uses the names "device-wake-gpios",
> "enable-gpios", and "host-wake-gpios". Would it be possible to
> stick to those?
>
> Last, you don't actually add support for them in the driver.
>
> So why are they there?
>
> Regards
> ChenYu
>
>> +       };
>> +};
>> +
>> +this ensures that the Bluetooth device is tied to the correct uart.
>> --
>> beagleboard@davidjohnsummers.uk
>>

Re: [PATCH v4 2/2] dt-bindings: Create the file for Realtek Bluetooth serial devices

[Rob Herring]

On Mon, Jan 21, 2019 at 2:39 PM David Summers
<beagleboard@davidjohnsummers.uk> wrote:
>
> With the changes requested by Marcel Holtmann and Rob Herring.
>
> Capitalisation as requested.

Write something that makes sense for the git history. The above 2
lines are not useful information.

> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>
> ---
>  .../bindings/net/realtek-bluetooth-serial.txt | 32 +++++++++++++++++++
>  1 file changed, 32 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>
> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
> new file mode 100644
> index 000000000000..2eddde1a0cf1
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
> @@ -0,0 +1,32 @@
> +Realtek Bluetooth devices connected via a UART.
> +These devices typically also have a WI-FI connected via SDIO - the
> +compatible described here is used just for referencing the Bluetooth.
> +
> +- compatible: should be "realtek,<name>-bt"
> +  except for "realtek,trl8761atv" - which only has a serial Bluetooth connection
> +       "realtek,rtl8723as-bt"
> +       "realtek,rtl8723bs-bt"
> +       "realtek,rtl8723ds-bt"
> +       "realtek,rtl8761atv"
> +       "realtek,rtl8821as-bt"
> +       "realtek,rtl8821cs-bt"
> +       "realtek,rtl8822bs-bt"
> +
> +Example:
> +
> +&uart0 {
> +       status = "okay";

Don't show 'status' in examples.

> +       pinctrl-0 = <&uart0_xfer>, <&uart0_cts>;
> +       bluetooth {
> +               compatible = "realtek,rtl8723bs-bt";
> +               uart_rts_gpios = <&gpio4 19 GPIO_ACTIVE_LOW>;

We already have a standard property name and location for handling RTS
on a GPIO and this is not it.

> +               pinctrl-names = "default","rts_gpio";
> +               pinctrl-0 = <&uart0_rts>;

This belongs in the parent node.

> +               pinctrl-1 = <&uart0_gpios>;
> +               BT,reset_gpio    = <&gpio4 29 GPIO_ACTIVE_HIGH>;
> +               BT,wake_gpio     = <&gpio4 26 GPIO_ACTIVE_HIGH>;
> +               BT,wake_host_irq = <&gpio4 31 GPIO_ACTIVE_HIGH>;

All the above need to be documented. Putting in an example is not documented.

Drop the 'BT,' as the part preceding the comma should be a vendor name
if present and use '-' rather than '_' in property names.

'reset-gpios' is the standard name for a reset gpio. An irq should use
the 'interrupts' binding even if connected to a GPIO on the host.

Rob

Re: [PATCH v4 2/2] dt-bindings: Create the file for Realtek Bluetooth serial devices

[David Summers]

Dear All,

OK - spent some time last night and today reading 
/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt and 
/Documentation/devicetree/bindings/serial/serial.txt; reviewing the Asus 
Tinker Board device tree, and the comments. What is clear:

 1. I do not have the time available to learn in detail the preferred
    linux style of device tree
 2. The example I gave was for the ASUS Tinker Board, taken from the
    rk3288-miniarm.dts
 3. The current ASUS Tinker Board device tree is a patch done by ASUS
    for debian:
    https://github.com/TinkerBoard/debian_kernel/blob/develop/arch/arm/boot/dts/rk3288-miniarm.dts
 4. This device tree is non standard, and not suitable for inclusion in
    mainline
 5. This device tree is the only really device tree available for the
    ASUS Tinker Board (S)
 6. The schematic for the ASUS Tinker Board is available on:
    https://www.asus.com/uk/Single-Board-Computer/Tinker-Board/
 7. The schematic is not detailed, it for example does not show how
    Bluetooth and Wi-Fi are wired
 8. The only hints on wiring are in the ASUS developed patches to the
    debian device tree
 9. The documentation for realtek Bluetooth will only be accepted with
    an example that is complete:
    https://www.spinics.net/lists/linux-bluetooth/msg78545.html
10. Marcel has only had major problem with the first version of the
    patch. Since changes made to hci_h5.c his only problem has the
    naming of the compatible names, where he needs an Ack from Rob:
    https://www.spinics.net/lists/linux-bluetooth/msg78637.html
11. Marcel needs the device tree compatible hooks documented:
    https://www.spinics.net/lists/linux-bluetooth/msg78445.html
12. Now it is clear that I don't have time to do documentation, of the
    required standard, that is applicable to the ASUS Tinker Board (S)
13. So it is with regret that on reflection I have decided to withdraw
    this patchset

This to my mind is a pity, ASUS Tinker Board (S) uses currently don't 
have good mainline support, most use the debian/ASUS rk3288-miniarm.dts 
device tree. Without mainlining, it is very hard to support this board 
in distributions like Arm Arch. But its also clear that I am not the 
person to mainline the support. I neither have the knowledge of the 
device tree. Access to proper schematics for the device. Nor the time 
available to do this.

Hopefully someone else will take up mainlining these boards. I hope the 
bits I've done will at least give someone else ideas on how to proceed, 
and that they will be more successful than I. I'll probably keep 
learning the preferred device tree vocabulary and grammar - but at a 
slow pace as time allows.

Thank you all for your time and help,

David Summers

On 25/01/2019 19:46, Rob Herring wrote:
> On Mon, Jan 21, 2019 at 2:39 PM David Summers
> <beagleboard@davidjohnsummers.uk> wrote:
>> With the changes requested by Marcel Holtmann and Rob Herring.
>>
>> Capitalisation as requested.
> Write something that makes sense for the git history. The above 2
> lines are not useful information.
>
>> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>
>> ---
>>   .../bindings/net/realtek-bluetooth-serial.txt | 32 +++++++++++++++++++
>>   1 file changed, 32 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>>
>> diff --git a/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>> new file mode 100644
>> index 000000000000..2eddde1a0cf1
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/realtek-bluetooth-serial.txt
>> @@ -0,0 +1,32 @@
>> +Realtek Bluetooth devices connected via a UART.
>> +These devices typically also have a WI-FI connected via SDIO - the
>> +compatible described here is used just for referencing the Bluetooth.
>> +
>> +- compatible: should be "realtek,<name>-bt"
>> +  except for "realtek,trl8761atv" - which only has a serial Bluetooth connection
>> +       "realtek,rtl8723as-bt"
>> +       "realtek,rtl8723bs-bt"
>> +       "realtek,rtl8723ds-bt"
>> +       "realtek,rtl8761atv"
>> +       "realtek,rtl8821as-bt"
>> +       "realtek,rtl8821cs-bt"
>> +       "realtek,rtl8822bs-bt"
>> +
>> +Example:
>> +
>> +&uart0 {
>> +       status = "okay";
> Don't show 'status' in examples.
>
>> +       pinctrl-0 = <&uart0_xfer>, <&uart0_cts>;
>> +       bluetooth {
>> +               compatible = "realtek,rtl8723bs-bt";
>> +               uart_rts_gpios = <&gpio4 19 GPIO_ACTIVE_LOW>;
> We already have a standard property name and location for handling RTS
> on a GPIO and this is not it.
>
>> +               pinctrl-names = "default","rts_gpio";
>> +               pinctrl-0 = <&uart0_rts>;
> This belongs in the parent node.
>
>> +               pinctrl-1 = <&uart0_gpios>;
>> +               BT,reset_gpio    = <&gpio4 29 GPIO_ACTIVE_HIGH>;
>> +               BT,wake_gpio     = <&gpio4 26 GPIO_ACTIVE_HIGH>;
>> +               BT,wake_host_irq = <&gpio4 31 GPIO_ACTIVE_HIGH>;
> All the above need to be documented. Putting in an example is not documented.
>
> Drop the 'BT,' as the part preceding the comma should be a vendor name
> if present and use '-' rather than '_' in property names.
>
> 'reset-gpios' is the standard name for a reset gpio. An irq should use
> the 'interrupts' binding even if connected to a GPIO on the host.
>
> Rob

Re: [PATCH v4 1/2] Bluetooth: hci_h5: Add the Realtek compatible flags for the device tree

[Chen-Yu Tsai]

On Tue, Jan 22, 2019 at 4:39 AM David Summers
<beagleboard@davidjohnsummers.uk> wrote:
>
> With the changes requested by Marcel Holtmann and Rob Herring.
>
> The patch is unchanged, but subject changed to the norm.

Same stuff with the commit log.

> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>
> ---
>  drivers/bluetooth/hci_h5.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index 8eede1197cd2..056ab2decccc 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -925,6 +925,22 @@ static struct h5_vnd rtl_vnd = {
>  };
>  #endif
>
> +#ifdef CONFIG_OF
> +static const struct of_device_id h5_of_match[] = {
> +#ifdef CONFIG_BT_HCIUART_RTL
> +       { .compatible = "realtek,rtl8723as-bt"},
> +       { .compatible = "realtek,rtl8723bs-bt"},
> +       { .compatible = "realtek,rtl8723ds-bt"},
> +       { .compatible = "realtek,rtl8761atv"},
> +       { .compatible = "realtek,rtl8821as-bt"},
> +       { .compatible = "realtek,rtl8821cs-bt"},
> +       { .compatible = "realtek,rtl8822bs-bt"},
> +#endif
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, h5_of_match);
> +#endif
> +

You added the list of compatibles, but you aren't actually
referencing the list in the driver. Which means this patch
actually does nothing in terms of functionality.

ChenYu

>  #ifdef CONFIG_ACPI
>  static const struct acpi_device_id h5_acpi_match[] = {
>  #ifdef CONFIG_BT_HCIUART_RTL
> --
> beagleboard@davidjohnsummers.uk
>

Re: [PATCH v4 1/2] Bluetooth: hci_h5: Add the Realtek compatible flags for the device tree

[David Summers]

This patch has been withdrawn.

On 21/01/2019 20:39, David Summers wrote:
> With the changes requested by Marcel Holtmann and Rob Herring.
>
> The patch is unchanged, but subject changed to the norm.
>
> Signed-off-by: David Summers <beagleboard@davidjohnsummers.uk>
> ---
>   drivers/bluetooth/hci_h5.c | 16 ++++++++++++++++
>   1 file changed, 16 insertions(+)
>
> diff --git a/drivers/bluetooth/hci_h5.c b/drivers/bluetooth/hci_h5.c
> index 8eede1197cd2..056ab2decccc 100644
> --- a/drivers/bluetooth/hci_h5.c
> +++ b/drivers/bluetooth/hci_h5.c
> @@ -925,6 +925,22 @@ static struct h5_vnd rtl_vnd = {
>   };
>   #endif
>   
> +#ifdef CONFIG_OF
> +static const struct of_device_id h5_of_match[] = {
> +#ifdef CONFIG_BT_HCIUART_RTL
> +       { .compatible = "realtek,rtl8723as-bt"},
> +       { .compatible = "realtek,rtl8723bs-bt"},
> +       { .compatible = "realtek,rtl8723ds-bt"},
> +       { .compatible = "realtek,rtl8761atv"},
> +       { .compatible = "realtek,rtl8821as-bt"},
> +       { .compatible = "realtek,rtl8821cs-bt"},
> +       { .compatible = "realtek,rtl8822bs-bt"},
> +#endif
> +       { }
> +};
> +MODULE_DEVICE_TABLE(of, h5_of_match);
> +#endif
> +
>   #ifdef CONFIG_ACPI
>   static const struct acpi_device_id h5_acpi_match[] = {
>   #ifdef CONFIG_BT_HCIUART_RTL