diff mbox series

[v3,2/3] dt-bindings: net: bluetooth: Add NXP bluetooth support

Message ID 20230213145432.1192911-3-neeraj.sanjaykale@nxp.com
State New
Headers
Series Add support for NXP bluetooth chipsets |

Commit Message

Neeraj Sanjay Kale Feb. 13, 2023, 2:54 p.m. UTC 
  Add binding document for NXP bluetooth chipsets attached
over UART.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
v2: Resolved dt_binding_check errors. (Rob Herring)
v2: Modified description, added specific compatibility devices, corrected indentations. (Krzysztof Kozlowski)
v3: Modified description, renamed file (Krzysztof Kozlowski)
---
 .../bindings/net/bluetooth/nxp,w8xxx-bt.yaml  | 44 +++++++++++++++++++
 MAINTAINERS                                   |  7 +++
 2 files changed, 51 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml

Comments

Krzysztof Kozlowski Feb. 14, 2023, 8:33 a.m. UTC | #1 
On 13/02/2023 15:54, Neeraj Sanjay Kale wrote:
> Add binding document for NXP bluetooth chipsets attached
> over UART.
> 
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
> v2: Resolved dt_binding_check errors. (Rob Herring)
> v2: Modified description, added specific compatibility devices, corrected indentations. (Krzysztof Kozlowski)
> v3: Modified description, renamed file (Krzysztof Kozlowski)
> ---
>  .../bindings/net/bluetooth/nxp,w8xxx-bt.yaml  | 44 +++++++++++++++++++

I don't think I proposed such filename.

>  MAINTAINERS                                   |  7 +++
>  2 files changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml
> new file mode 100644
> index 000000000000..2685f6d5904f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/bluetooth/nxp-bluetooth.yaml#

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP Bluetooth chips
> +
> +description:
> +  This binding describes UART-attached NXP bluetooth chips.
> +  These chips are dual-radio chips supporting WiFi and Bluetooth,
> +  except for iw612, which is a tri-radio chip supporting 15.4
> +  as well.
> +  The bluetooth works on standard H4 protocol over 4-wire UART.
> +  The RTS and CTS lines are used during FW download.
> +  To enable power save mode, the host asserts break signal
> +  over UART-TX line to put the chip into power save state.
> +  De-asserting break wakes-up the BT chip.
> +
> +maintainers:
> +  - Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,88w8987-bt
> +      - nxp,88w8997-bt
> +      - nxp,88w9098-bt
> +      - nxp,iw416-bt
> +      - nxp,iw612-bt
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    uart2 {

This is a friendly reminder during the review process.

It seems my previous comments were not fully addressed. Maybe my
feedback got lost between the quotes, maybe you just forgot to apply it.
Please go back to the previous discussion and either implement all
requested changes or keep discussing them.

Thank you.

> +        uart-has-rtscts;
> +        bluetooth {
> +          compatible = "nxp,iw416-bt";

Wrong indentation. Use 4 spaces for example indentation.


Best regards,
Krzysztof
Rob Herring Feb. 14, 2023, 4:12 p.m. UTC | #2 
On Mon, 13 Feb 2023 20:24:31 +0530, Neeraj Sanjay Kale wrote:
> Add binding document for NXP bluetooth chipsets attached
> over UART.
> 
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
> v2: Resolved dt_binding_check errors. (Rob Herring)
> v2: Modified description, added specific compatibility devices, corrected indentations. (Krzysztof Kozlowski)
> v3: Modified description, renamed file (Krzysztof Kozlowski)
> ---
>  .../bindings/net/bluetooth/nxp,w8xxx-bt.yaml  | 44 +++++++++++++++++++
>  MAINTAINERS                                   |  7 +++
>  2 files changed, 51 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
./Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml: $id: relative path/filename doesn't match actual path or filename
	expected: http://devicetree.org/schemas/net/bluetooth/nxp,w8xxx-bt.yaml#

doc reference errors (make refcheckdocs):
MAINTAINERS: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230213145432.1192911-3-neeraj.sanjaykale@nxp.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.
Sherry Sun Feb. 16, 2023, 10:45 a.m. UTC | #3 
> -----Original Message-----
> From: Neeraj sanjay kale <neeraj.sanjaykale@nxp.com>
> Sent: 2023年2月13日 22:55
> To: davem@davemloft.net; edumazet@google.com; kuba@kernel.org;
> pabeni@redhat.com; robh+dt@kernel.org;
> krzysztof.kozlowski+dt@linaro.org; marcel@holtmann.org;
> johan.hedberg@gmail.com; luiz.dentz@gmail.com;
> gregkh@linuxfoundation.org; jirislaby@kernel.org; alok.a.tiwari@oracle.com;
> hdanton@sina.com; ilpo.jarvinen@linux.intel.com; leon@kernel.org
> Cc: netdev@vger.kernel.org; devicetree@vger.kernel.org; linux-
> kernel@vger.kernel.org; linux-bluetooth@vger.kernel.org; linux-
> serial@vger.kernel.org; Amitkumar Karwar <amitkumar.karwar@nxp.com>;
> Rohit Fule <rohit.fule@nxp.com>; Sherry Sun <sherry.sun@nxp.com>; Neeraj
> sanjay kale <neeraj.sanjaykale@nxp.com>
> Subject: [PATCH v3 2/3] dt-bindings: net: bluetooth: Add NXP bluetooth
> support
> 
> Add binding document for NXP bluetooth chipsets attached over UART.
> 
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
> v2: Resolved dt_binding_check errors. (Rob Herring)
> v2: Modified description, added specific compatibility devices, corrected
> indentations. (Krzysztof Kozlowski)
> v3: Modified description, renamed file (Krzysztof Kozlowski)
> ---
>  .../bindings/net/bluetooth/nxp,w8xxx-bt.yaml  | 44 +++++++++++++++++++
>  MAINTAINERS                                   |  7 +++
>  2 files changed, 51 insertions(+)
>  create mode 100644
> Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-
> bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-
> bt.yaml
> new file mode 100644
> index 000000000000..2685f6d5904f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-
> bt.yaml
> @@ -0,0 +1,44 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) %YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/bluetooth/nxp-bluetooth.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NXP Bluetooth chips
> +
> +description:
> +  This binding describes UART-attached NXP bluetooth chips.
> +  These chips are dual-radio chips supporting WiFi and Bluetooth,
> +  except for iw612, which is a tri-radio chip supporting 15.4
> +  as well.
> +  The bluetooth works on standard H4 protocol over 4-wire UART.
> +  The RTS and CTS lines are used during FW download.
> +  To enable power save mode, the host asserts break signal
> +  over UART-TX line to put the chip into power save state.
> +  De-asserting break wakes-up the BT chip.
> +
> +maintainers:
> +  - Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,88w8987-bt
> +      - nxp,88w8997-bt
> +      - nxp,88w9098-bt
> +      - nxp,iw416-bt
> +      - nxp,iw612-bt
Hi Neeraj,

No need to set one compatible for each NXP BT chip I think, otherwise the list will get longer and longer.
You can use one common compatible for all the new BT chips which support V3 bootloader, then you will get the chip ID from the bootloader in driver to distinguish the chips.

Best Regards
Sherry


> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    uart2 {
> +        uart-has-rtscts;
> +        bluetooth {
> +          compatible = "nxp,iw416-bt";
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 32dd41574930..211fc667c0ec 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -22835,6 +22835,13 @@ L:	linux-mm@kvack.org
>  S:	Maintained
>  F:	mm/zswap.c
> 
> +NXP BLUETOOTH WIRELESS DRIVERS
> +M:	Amitkumar Karwar <amitkumar.karwar@nxp.com>
> +M:	Neeraj Kale <neeraj.sanjaykale@nxp.com>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/net/bluetooth/nxp-
> bluetooth.yaml
> +F:	drivers/bluetooth/btnxpuart*
> +
>  THE REST
>  M:	Linus Torvalds <torvalds@linux-foundation.org>
>  L:	linux-kernel@vger.kernel.org
> --
> 2.34.1
Neeraj Sanjay Kale Feb. 21, 2023, 4:40 p.m. UTC | #4 
Hi Krzysztof,

Thank you for reviewing this patch. I have fixed all the review comments in this document.
Please let me know if you have any more comments or suggestions on the new v4 patch.

> >  .../bindings/net/bluetooth/nxp,w8xxx-bt.yaml  | 44
> > +++++++++++++++++++
> 
> I don't think I proposed such filename.
Renamed file to nxp,w8987-bt.yaml


> > +examples:
> > +  - |
> > +    uart2 {
> 
> This is a friendly reminder during the review process.
> 
> It seems my previous comments were not fully addressed. Maybe my
> feedback got lost between the quotes, maybe you just forgot to apply it.
> Please go back to the previous discussion and either implement all requested
> changes or keep discussing them.
> 
> Thank you.
> 
> > +        uart-has-rtscts;
> > +        bluetooth {
> > +          compatible = "nxp,iw416-bt";
> 
> Wrong indentation. Use 4 spaces for example indentation.
> 
Updated example indentation to use 4 spaces.

Thanks,
Neeraj
Krzysztof Kozlowski Feb. 21, 2023, 4:56 p.m. UTC | #5 
On 21/02/2023 17:40, Neeraj sanjay kale wrote:
> Hi Krzysztof,
> 
> Thank you for reviewing this patch. I have fixed all the review comments in this document.
> Please let me know if you have any more comments or suggestions on the new v4 patch.
> 
>>>  .../bindings/net/bluetooth/nxp,w8xxx-bt.yaml  | 44
>>> +++++++++++++++++++
>>
>> I don't think I proposed such filename.
> Renamed file to nxp,w8987-bt.yaml
> 
> 
>>> +examples:
>>> +  - |
>>> +    uart2 {
>>
>> This is a friendly reminder during the review process.
>>
>> It seems my previous comments were not fully addressed. Maybe my
>> feedback got lost between the quotes, maybe you just forgot to apply it.
>> Please go back to the previous discussion and either implement all requested
>> changes or keep discussing them.

And how did you fix this one?

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml
new file mode 100644
index 000000000000..2685f6d5904f
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/bluetooth/nxp,w8xxx-bt.yaml
@@ -0,0 +1,44 @@ 
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/net/bluetooth/nxp-bluetooth.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NXP Bluetooth chips
+
+description:
+  This binding describes UART-attached NXP bluetooth chips.
+  These chips are dual-radio chips supporting WiFi and Bluetooth,
+  except for iw612, which is a tri-radio chip supporting 15.4
+  as well.
+  The bluetooth works on standard H4 protocol over 4-wire UART.
+  The RTS and CTS lines are used during FW download.
+  To enable power save mode, the host asserts break signal
+  over UART-TX line to put the chip into power save state.
+  De-asserting break wakes-up the BT chip.
+
+maintainers:
+  - Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
+
+properties:
+  compatible:
+    enum:
+      - nxp,88w8987-bt
+      - nxp,88w8997-bt
+      - nxp,88w9098-bt
+      - nxp,iw416-bt
+      - nxp,iw612-bt
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    uart2 {
+        uart-has-rtscts;
+        bluetooth {
+          compatible = "nxp,iw416-bt";
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index 32dd41574930..211fc667c0ec 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -22835,6 +22835,13 @@  L:	linux-mm@kvack.org
 S:	Maintained
 F:	mm/zswap.c
 
+NXP BLUETOOTH WIRELESS DRIVERS
+M:	Amitkumar Karwar <amitkumar.karwar@nxp.com>
+M:	Neeraj Kale <neeraj.sanjaykale@nxp.com>
+S:	Maintained
+F:	Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml
+F:	drivers/bluetooth/btnxpuart*
+
 THE REST
 M:	Linus Torvalds <torvalds@linux-foundation.org>
 L:	linux-kernel@vger.kernel.org