netdev - Re: [PATCH v3 2/3] dt-bindings: net: bluetooth: Add NXP bluetooth support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <60928656-c565-773d-52e6-2142e997eee4@linaro.org>
Date:   Tue, 14 Feb 2023 09:33:41 +0100
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Neeraj Sanjay Kale <neeraj.sanjaykale@....com>,
        davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, robh+dt@...nel.org,
        krzysztof.kozlowski+dt@...aro.org, marcel@...tmann.org,
        johan.hedberg@...il.com, luiz.dentz@...il.com,
        gregkh@...uxfoundation.org, jirislaby@...nel.org,
        alok.a.tiwari@...cle.com, hdanton@...a.com,
        ilpo.jarvinen@...ux.intel.com, leon@...nel.org
Cc:     netdev@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-bluetooth@...r.kernel.org,
        linux-serial@...r.kernel.org, amitkumar.karwar@....com,
        rohit.fule@....com, sherry.sun@....com
Subject: Re: [PATCH v3 2/3] dt-bindings: net: bluetooth: Add NXP bluetooth
 support

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@....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@....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