linux-kernel - Re: [PATCH v5 2/2] dt-bindings: net: mediatek,mt7925-bluetooth.yaml

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <25t3jzrqcdko5z5udbbcctaqldcrbycryazumw6mfj2c4qihmr@jcubfbuhikvp>
Date: Sat, 28 Jun 2025 14:03:53 +0200
From: Krzysztof Kozlowski <krzk@...nel.org>
To: Zhangchao Zhang <ot_zhangchao.zhang@...iatek.com>
Cc: Marcel Holtmann <marcel@...tmann.org>, 
	Matthias Brugger <matthias.bgg@...il.com>, Luiz Von Dentz <luiz.dentz@...il.com>, 
	AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>, Rob Herring <robh@...nel.org>, 
	Conor Dooley <conor+dt@...nel.org>, Sean Wang <sean.wang@...iatek.com>, 
	Jiande Lu <jiande.lu@...iatek.com>, Deren Wu <deren.Wu@...iatek.com>, 
	Chris Lu <chris.lu@...iatek.com>, Hao Qin <Hao.qin@...iatek.com>, 
	linux-bluetooth <linux-bluetooth@...r.kernel.org>, linux-kernel <linux-kernel@...r.kernel.org>, 
	linux-arm-kernel <linux-arm-kernel@...ts.infradead.org>, linux-mediatek <linux-mediatek@...ts.infradead.org>, 
	devicetree <devicetree@...r.kernel.org>
Subject: Re: [PATCH v5 2/2] dt-bindings: net: mediatek,mt7925-bluetooth.yaml


How did you manage to make mistake in my email is beyond my understand.
Tools should generate address list, not humans.

Where is the changelog?

On Fri, Jun 27, 2025 at 01:59:24PM +0800, Zhangchao Zhang wrote:
> Add hardware pins and compatible strings.

I don't see any pins here.

 
> As a binding file for the MTK Bluetooth driver code,
> it provides a set of compatible fields and hardware
> pins for the driver to use.

All this is not relevant. Describe the hardware here.

> 
> Signed-off-by: Zhangchao Zhang <ot_zhangchao.zhang@...iatek.com>
> ---


>  .../bluetooth/mediatek,mt7925-bluetooth.yaml  | 57 +++++++++++++++++++
>  MAINTAINERS                                   |  1 +
>  2 files changed, 58 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/net/bluetooth/mediatek,mt7925-bluetooth.yaml
> 
> diff --git a/Documentation/devicetree/bindings/net/bluetooth/mediatek,mt7925-bluetooth.yaml b/Documentation/devicetree/bindings/net/bluetooth/mediatek,mt7925-bluetooth.yaml
> new file mode 100644
> index 000000000000..230c24ada3b4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/bluetooth/mediatek,mt7925-bluetooth.yaml
> @@ -0,0 +1,57 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/net/bluetooth/mediatek,mt7925-bluetooth.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Bluetooth mediatek add gpio pin to reset bt

Not relevant. This is the title describing this specific hardware.

> +
> +maintainers:
> +  - Sean Wang <sean.wang@...iatek.com>
> +
> +description:
> +  7925 uses the USB bus to communicate with the host.

Waht is 7925?

> +  Two methods are used to reset Bluetooth.
> +  Provide hardware pin, when an exception occurs,
> +  resetting Bluetooth by hardware pin is more stable
> +  than resetting Bluetooth by software.
> +  If the corresponding pin is not found in dts,
> +  bluetooth can also be reset successfully.

Wrap this properly. The same commit msg.

> +
> +allOf:
> +  - $ref: bluetooth-controller.yaml#
> +
> +properties:
> +  compatible:
> +    enum:
> +      - mediatek,usb-bluetooth

NAK, you cannot have such compatible. I already said at v1 or v2 that it
will be rejected!

Why are you pushing this all the time, without reading the feedback?

You need SoC and device specific compatibles. There is no such hardware
as "usb-bluetooth". If there is, point me to datasheet for it.


> +
> +  gpio-controller:
> +    description:
> +      Marks the device node as s GPIO controller.

Irrelevant. Drop. See other schemas.

> +
> +  "#gpio-cells":
> +    const: 2
> +
> +  reset-gpios:
> +    maxItems: 1
> +

Drop blank line. Look how other files are doing this.


> +    description:
> +      An active-high reset pin for the Bluetooth core; on typical M.2

active-high? Really? I have doubts.

> +      key E modules this is the W_DISABLE2# pin.
> +
> +required:
> +  - compatible
> +  - "#gpio-cells"
> +  - reset-gpios
> +
> +unevaluatedProperties: false

This is random style. Why doing something completely different than
every other file? Missing blank line.

> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +      bluetooth {

Messed indentation.

To summarize:
Except writing something entirely different than every other binding
(why?), this does not represent hardware. Your commit msg explains ZERO
about the hardware. Binding description says nothing and is actually not
correct.

So again, if you send the same it will be rejected.

I expect answers UNDER EACH OF MY COMMENTS, because you keep ignoring
entire feedback given to you.

Best regards,
Krzysztof