diff mbox series

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

Message ID 20230124174714.2775680-3-neeraj.sanjaykale@nxp.com (mailing list archive)
State Superseded
Headers show
Series Add support for NXP bluetooth chipsets | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch warning WARNING: added, moved or deleted file(s), does MAINTAINERS need updating? #112: new file mode 100644 total: 0 errors, 1 warnings, 67 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13114426.patch has style problems, please review. NOTE: Ignored message types: UNKNOWN_COMMIT_ID NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
tedd_an/GitLint fail WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 10: B1 Line exceeds max length (86>80): " create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml"
tedd_an/SubjectPrefix fail "Bluetooth: " prefix is not specified in the subject
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Neeraj Sanjay Kale Jan. 24, 2023, 5:47 p.m. UTC
Add binding document for generic and legacy NXP bluetooth
chipset.

Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
---
 .../bindings/net/bluetooth/nxp-bluetooth.yaml | 67 +++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml

Comments

Rob Herring (Arm) Jan. 24, 2023, 7:06 p.m. UTC | #1
On Tue, 24 Jan 2023 23:17:13 +0530, Neeraj Sanjay Kale wrote:
> Add binding document for generic and legacy NXP bluetooth
> chipset.
> 
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
>  .../bindings/net/bluetooth/nxp-bluetooth.yaml | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.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:
./Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml:67:1: [warning] too many blank lines (2 > 1) (empty-lines)

dtschema/dtc warnings/errors:
Error: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.example.dts:18.9-15 syntax error
FATAL ERROR: Unable to parse input tree
make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.example.dtb] Error 1
make[1]: *** Waiting for unfinished jobs....
make: *** [Makefile:1508: dt_binding_check] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230124174714.2775680-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.
Luiz Augusto von Dentz Jan. 24, 2023, 9:44 p.m. UTC | #2
Hi Rob, Tedd,

On Tue, Jan 24, 2023 at 11:06 AM Rob Herring <robh@kernel.org> wrote:
>
>
> On Tue, 24 Jan 2023 23:17:13 +0530, Neeraj Sanjay Kale wrote:
> > Add binding document for generic and legacy NXP bluetooth
> > chipset.
> >
> > Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> > ---
> >  .../bindings/net/bluetooth/nxp-bluetooth.yaml | 67 +++++++++++++++++++
> >  1 file changed, 67 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.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:
> ./Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml:67:1: [warning] too many blank lines (2 > 1) (empty-lines)
>
> dtschema/dtc warnings/errors:
> Error: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.example.dts:18.9-15 syntax error
> FATAL ERROR: Unable to parse input tree
> make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.example.dtb] Error 1
> make[1]: *** Waiting for unfinished jobs....
> make: *** [Makefile:1508: dt_binding_check] Error 2

I wonder if that is something that we could incorporate to our CI,
perhaps we can detect if the subject starts with dt-binding then we
attempt to make with DT_CHECKER_FLAGS, thoughts?

> doc reference errors (make refcheckdocs):
>
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230124174714.2775680-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.
>
Rob Herring (Arm) Jan. 24, 2023, 11:08 p.m. UTC | #3
On Tue, Jan 24, 2023 at 3:44 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Rob, Tedd,
>
> On Tue, Jan 24, 2023 at 11:06 AM Rob Herring <robh@kernel.org> wrote:
> >
> >
> > On Tue, 24 Jan 2023 23:17:13 +0530, Neeraj Sanjay Kale wrote:
> > > Add binding document for generic and legacy NXP bluetooth
> > > chipset.
> > >
> > > Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> > > ---
> > >  .../bindings/net/bluetooth/nxp-bluetooth.yaml | 67 +++++++++++++++++++
> > >  1 file changed, 67 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.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:
> > ./Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml:67:1: [warning] too many blank lines (2 > 1) (empty-lines)
> >
> > dtschema/dtc warnings/errors:
> > Error: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.example.dts:18.9-15 syntax error
> > FATAL ERROR: Unable to parse input tree
> > make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.example.dtb] Error 1
> > make[1]: *** Waiting for unfinished jobs....
> > make: *** [Makefile:1508: dt_binding_check] Error 2
>
> I wonder if that is something that we could incorporate to our CI,
> perhaps we can detect if the subject starts with dt-binding then we
> attempt to make with DT_CHECKER_FLAGS, thoughts?

What CI is that?

Better to look at the diffstat of the patch than subject. Lots of
subjects are wrong and I suspect there would be a fairly high
correlation of wrong subjects to schema errors.

Rob
Luiz Augusto von Dentz Jan. 25, 2023, 1:03 a.m. UTC | #4
Hi Rob,

On Tue, Jan 24, 2023 at 3:08 PM Rob Herring <robh@kernel.org> wrote:
>
> On Tue, Jan 24, 2023 at 3:44 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Rob, Tedd,
> >
> > On Tue, Jan 24, 2023 at 11:06 AM Rob Herring <robh@kernel.org> wrote:
> > >
> > >
> > > On Tue, 24 Jan 2023 23:17:13 +0530, Neeraj Sanjay Kale wrote:
> > > > Add binding document for generic and legacy NXP bluetooth
> > > > chipset.
> > > >
> > > > Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> > > > ---
> > > >  .../bindings/net/bluetooth/nxp-bluetooth.yaml | 67 +++++++++++++++++++
> > > >  1 file changed, 67 insertions(+)
> > > >  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.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:
> > > ./Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml:67:1: [warning] too many blank lines (2 > 1) (empty-lines)
> > >
> > > dtschema/dtc warnings/errors:
> > > Error: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.example.dts:18.9-15 syntax error
> > > FATAL ERROR: Unable to parse input tree
> > > make[1]: *** [scripts/Makefile.lib:434: Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.example.dtb] Error 1
> > > make[1]: *** Waiting for unfinished jobs....
> > > make: *** [Makefile:1508: dt_binding_check] Error 2
> >
> > I wonder if that is something that we could incorporate to our CI,
> > perhaps we can detect if the subject starts with dt-binding then we
> > attempt to make with DT_CHECKER_FLAGS, thoughts?
>
> What CI is that?

We have github actions that we run when a new patch appears on patchwork e.g:

https://patchwork.kernel.org/project/bluetooth/patch/20230124174714.2775680-3-neeraj.sanjaykale@nxp.com/

> Better to look at the diffstat of the patch than subject. Lots of
> subjects are wrong and I suspect there would be a fairly high
> correlation of wrong subjects to schema errors.

For now I'd keep it simple since otherwise we would have to probably
attempt to apply and make with DT_CHECKER_FLAGS every patch, well
perhaps that is ok if that doesn't produce too many false positives,
otherwise we have to filter the output like we do with the likes of
smatch and building with make W=1 C=1.

> Rob
Krzysztof Kozlowski Jan. 25, 2023, 7:21 a.m. UTC | #5
On 24/01/2023 18:47, Neeraj Sanjay Kale wrote:
> Add binding document for generic and legacy NXP bluetooth
> chipset.
> 
> Signed-off-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> ---
>  .../bindings/net/bluetooth/nxp-bluetooth.yaml | 67 +++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml
> new file mode 100644
> index 000000000000..d6226838ab1c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml

format is vendor,compatible

> @@ -0,0 +1,67 @@
> +# 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#"

Drop quotes from both

> +
> +title: NXP Bluetooth chips
> +
> +description:
> +  This documents the binding structure and common properties for serial
> +  attached NXP Bluetooth devices.

Drop "This documents the binding structure and common properties"... and
replace everything with proper hardware description.

> +
> +maintainers:
> +  - Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - nxp,nxp-generic-bt-chip
> +      - nxp,nxp-legacy-bt-chip

These are wrong on multiple levels. Duplicated vendor prefix. Not
specific compatible. Fake compatible (non-existing device).

> +
> +  firmware-name:
> +    description:
> +      Specify firmware file name. If this property is not
> +      specified, it is fetched from the user-space config
> +      file nxp/bt_mod_para.conf
> +
> +required:
> +  - compatible
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    &uart1 {
> +      pinctrl-names = "default";
> +      pinctrl-0 = <&pinctrl_uart1>;
> +      assigned-clocks = <&clk IMX8MM_CLK_UART1>;
> +      assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_80M>;
> +      fsl,uart-has-rtscts;

Drop everything above except serial node.

> +      status = "okay";

Drop

> +      bluetooth {
> +              compatible = "nxp,nxp-generic-bt-chip";

Wrong indentation. It's 4-space.

> +      };
> +    };
> +  - |
> +    &uart2 {
> +      bluetooth {
> +              compatible = "nxp,nxp-generic-bt-chip";
> +              firmware-name = "uartuart_n61x_v1.bin"
> +      };
> +    };
> +  - |
> +    &uart3 {
> +      bluetooth {
> +              compatible = "nxp,nxp-legacy-bt-chip";
> +      };
> +    };
> +  - |
> +    &uart4 {
> +      bluetooth {
> +              compatible = "nxp,nxp-legacy-bt-chip";
> +              firmware-name = "uartuart8987_bt.bin"
> +      };

Drop all these above - they do not bring anything.

> +    };
> +
> +

Best regards,
Krzysztof
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml
new file mode 100644
index 000000000000..d6226838ab1c
--- /dev/null
+++ b/Documentation/devicetree/bindings/net/bluetooth/nxp-bluetooth.yaml
@@ -0,0 +1,67 @@ 
+# 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 documents the binding structure and common properties for serial
+  attached NXP Bluetooth devices.
+
+maintainers:
+  - Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
+
+properties:
+  compatible:
+    enum:
+      - nxp,nxp-generic-bt-chip
+      - nxp,nxp-legacy-bt-chip
+
+  firmware-name:
+    description:
+      Specify firmware file name. If this property is not
+      specified, it is fetched from the user-space config
+      file nxp/bt_mod_para.conf
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    &uart1 {
+      pinctrl-names = "default";
+      pinctrl-0 = <&pinctrl_uart1>;
+      assigned-clocks = <&clk IMX8MM_CLK_UART1>;
+      assigned-clock-parents = <&clk IMX8MM_SYS_PLL1_80M>;
+      fsl,uart-has-rtscts;
+      status = "okay";
+      bluetooth {
+              compatible = "nxp,nxp-generic-bt-chip";
+      };
+    };
+  - |
+    &uart2 {
+      bluetooth {
+              compatible = "nxp,nxp-generic-bt-chip";
+              firmware-name = "uartuart_n61x_v1.bin"
+      };
+    };
+  - |
+    &uart3 {
+      bluetooth {
+              compatible = "nxp,nxp-legacy-bt-chip";
+      };
+    };
+  - |
+    &uart4 {
+      bluetooth {
+              compatible = "nxp,nxp-legacy-bt-chip";
+              firmware-name = "uartuart8987_bt.bin"
+      };
+    };
+
+