Message-ID: <fcf66269-459c-c7fc-82b7-73303bd67950@linaro.org>
Date:   Wed, 25 Jan 2023 08:21:42 +0100
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.7.0
Subject: Re: [PATCH v1 2/3] dt-bindings: net: bluetooth: Add NXP bluetooth
 support
Content-Language: en-US
To:     Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>,
        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
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@nxp.com,
        rohit.fule@nxp.com, sherry.sun@nxp.com
References: <20230124174714.2775680-1-neeraj.sanjaykale@nxp.com>
 <20230124174714.2775680-3-neeraj.sanjaykale@nxp.com>
From:   Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
In-Reply-To: <20230124174714.2775680-3-neeraj.sanjaykale@nxp.com>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

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