[v1] Add support for Amlogic HCI UART

[PATCH 1/4] dt-bindings: net: bluetooth: Add support for Amlogic Bluetooth

Posted by Yang Li via B4 Relay 1 year, 7 months ago

From: Yang Li <yang.li@amlogic.com>

Add binding document for Amlogic Bluetooth chipsets attached over UART.

Signed-off-by: Yang Li <yang.li@amlogic.com>
---
 .../bindings/net/bluetooth/amlogic,w155s2-bt.yaml  | 62 ++++++++++++++++++++++
 1 file changed, 62 insertions(+)

diff --git a/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml
new file mode 100644
index 000000000000..d59e3206af62
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Copyright (C) 2024 Amlogic, Inc. All rights reserved
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/bluetooth/amlogic,w155s2-bt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Amlogic Bluetooth chips
+
+description:
+  This binding describes UART-attached Amlogic bluetooth chips.
+
+maintainers:
+  - Yang Li <yang.li@amlogic.com>
+
+properties:
+  compatible:
+    oneOf:
+      - const: amlogic,w155s2-bt
+      - items:
+          - enum:
+              - amlogic,w265s1-bt
+              - amlogic,w265p1-bt
+              - amlogic,w265s2-bt
+          - const: amlogic,w155s2-bt
+
+  amlogic,wcn-pwrseq:
+    default: 0
+    description: specify the power sequence used to power on Bluetooth
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  amlogic,firmware:
+    description: specify the path of firmware bin to load
+    $ref: /schemas/types.yaml#/definitions/string-array
+
+  amlogic,antenna-number:
+    default: 1
+    description: number of antenna
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+  amlogic,a2dp-sink-enable:
+    default: 0
+    description: enable sink mode with controller
+    $ref: /schemas/types.yaml#/definitions/uint32
+
+required:
+  - compatible
+  - amlogic,wcn-pwrseq
+  - amlogic,firmware
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/gpio/gpio.h>
+    serial {
+        bluetooth {
+            compatible = "amlogic,w155s2-bt";
+            amlogic,wcn-pwrseq = <1>;
+            amlogic,firmware = "amlogic/aml_w155s2_bt_uart.bin";
+        };
+    };

-- 
2.42.0

Re: [PATCH 1/4] dt-bindings: net: bluetooth: Add support for Amlogic Bluetooth

Posted by Krzysztof Kozlowski 1 year, 7 months ago

On 05/07/2024 13:20, Yang Li via B4 Relay wrote:
> From: Yang Li <yang.li@amlogic.com>
> 
> Add binding document for Amlogic Bluetooth chipsets attached over UART.
> 
> Signed-off-by: Yang Li <yang.li@amlogic.com>
> ---
>  .../bindings/net/bluetooth/amlogic,w155s2-bt.yaml  | 62 ++++++++++++++++++++++
>  1 file changed, 62 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml
> new file mode 100644
> index 000000000000..d59e3206af62
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# Copyright (C) 2024 Amlogic, Inc. All rights reserved
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/bluetooth/amlogic,w155s2-bt.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Amlogic Bluetooth chips
> +
> +description:
> +  This binding describes UART-attached Amlogic bluetooth chips.

Do not say that binding describes a binding. It is not helpful. Describe
the hardware instead.

> +
> +maintainers:
> +  - Yang Li <yang.li@amlogic.com>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - const: amlogic,w155s2-bt
> +      - items:
> +          - enum:
> +              - amlogic,w265s1-bt
> +              - amlogic,w265p1-bt
> +              - amlogic,w265s2-bt
> +          - const: amlogic,w155s2-bt
> +
> +  amlogic,wcn-pwrseq:

??

> +    default: 0
> +    description: specify the power sequence used to power on Bluetooth
> +    $ref: /schemas/types.yaml#/definitions/uint32

What? I have no clue what is this and have no clue what to say here. Drop.

> +
> +  amlogic,firmware:
> +    description: specify the path of firmware bin to load
> +    $ref: /schemas/types.yaml#/definitions/string-array

Srsly, it's third case in your bindings. Do not re-invent the wheel but
use existing, common properties. How to find them? Well, git grep or ask
your colleagues...




> +
> +  amlogic,antenna-number:
> +    default: 1
> +    description: number of antenna

Useless description. Do not repeat the property name but explain what is
it and how it is used.

Or drop the property.

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +  amlogic,a2dp-sink-enable:
> +    default: 0
> +    description: enable sink mode with controller

Again, not possible to figure out.

NAK

> +    $ref: /schemas/types.yaml#/definitions/uint32
> +
> +required:
> +  - compatible
> +  - amlogic,wcn-pwrseq
> +  - amlogic,firmware
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +    serial {

Drop

> +        bluetooth {
> +            compatible = "amlogic,w155s2-bt";
> +            amlogic,wcn-pwrseq = <1>;

Joking, right?



Best regards,
Krzysztof

Re: [PATCH 1/4] dt-bindings: net: bluetooth: Add support for Amlogic Bluetooth

Posted by Yang Li 1 year, 7 months ago

On 2024/7/7 21:08, Krzysztof Kozlowski wrote:
> [你通常不会收到来自 krzk@kernel.org 的电子邮件。请访问 https://aka.ms/LearnAboutSenderIdentification，以了解这一点为什么很重要]
>
> [ EXTERNAL EMAIL ]
>
> On 05/07/2024 13:20, Yang Li via B4 Relay wrote:
>> From: Yang Li <yang.li@amlogic.com>
>>
>> Add binding document for Amlogic Bluetooth chipsets attached over UART.
>>
>> Signed-off-by: Yang Li <yang.li@amlogic.com>
>> ---
>>   .../bindings/net/bluetooth/amlogic,w155s2-bt.yaml  | 62 ++++++++++++++++++++++
>>   1 file changed, 62 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml
>> new file mode 100644
>> index 000000000000..d59e3206af62
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/net/bluetooth/amlogic,w155s2-bt.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +# Copyright (C) 2024 Amlogic, Inc. All rights reserved
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/net/bluetooth/amlogic,w155s2-bt.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: Amlogic Bluetooth chips
>> +
>> +description:
>> +  This binding describes UART-attached Amlogic bluetooth chips.
> Do not say that binding describes a binding. It is not helpful. Describe
> the hardware instead.
Well do.
>
>> +
>> +maintainers:
>> +  - Yang Li <yang.li@amlogic.com>
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - const: amlogic,w155s2-bt
>> +      - items:
>> +          - enum:
>> +              - amlogic,w265s1-bt
>> +              - amlogic,w265p1-bt
>> +              - amlogic,w265s2-bt
>> +          - const: amlogic,w155s2-bt
>> +
>> +  amlogic,wcn-pwrseq:
> ??
I will remove this line, add regolator.
>
>> +    default: 0
>> +    description: specify the power sequence used to power on Bluetooth
>> +    $ref: /schemas/types.yaml#/definitions/uint32
> What? I have no clue what is this and have no clue what to say here. Drop.
will do.
>> +
>> +  amlogic,firmware:
>> +    description: specify the path of firmware bin to load
>> +    $ref: /schemas/types.yaml#/definitions/string-array
> Srsly, it's third case in your bindings. Do not re-invent the wheel but
> use existing, common properties. How to find them? Well, git grep or ask
> your colleagues...
I will change "amlogic,firmware" to " firmware-name".
>> +
>> +  amlogic,antenna-number:
>> +    default: 1
>> +    description: number of antenna
> Useless description. Do not repeat the property name but explain what is
> it and how it is used.
>
> Or drop the property.
I will change the description.
>
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +  amlogic,a2dp-sink-enable:
>> +    default: 0
>> +    description: enable sink mode with controller
> Again, not possible to figure out.
>
> NAK
Well do.
>
>> +    $ref: /schemas/types.yaml#/definitions/uint32
>> +
>> +required:
>> +  - compatible
>> +  - amlogic,wcn-pwrseq
>> +  - amlogic,firmware
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    #include <dt-bindings/gpio/gpio.h>
>> +    serial {
> Drop
Will do.
>
>> +        bluetooth {
>> +            compatible = "amlogic,w155s2-bt";
>> +            amlogic,wcn-pwrseq = <1>;
> Joking, right?
Well, I will remove it.
>
>
>
> Best regards,
> Krzysztof
>

[PATCH 1/4] dt-bindings: net: bluetooth: Add support for Amlogic Bluetooth
[PATCH 2/4] Bluetooth: hci_uart: Add support for Amlogic HCI UART
[PATCH 3/4] arm64: defconfig: Enable hci_uart for Amlogic Bluetooth
[PATCH 4/4] MAINTAINERS: Add an entry for Amlogic HCI UART