Re: [RFC PATCH v2 2/3] dt-bindings: net: bluetooth: Add AIC8800D80

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] dt-bindings: net: bluetooth: Incorrect max-speed property constraint
- [Low] dt-bindings: net: bluetooth: Unidiomatic reg property constraint syntax
--

commit 8cfd6d94bf45715d77abf03541df961e63f18ae5
Author: Yanli Yang <yanli.yang@bedmex.com>

dt-bindings: net: bluetooth: Add AIC8800D80

This commit adds a devicetree binding for the AIC8800D80 Bluetooth chip.
It describes the SDIO firmware-loading function and the UART HCI interface
which uses the H4 protocol with hardware flow control.

> diff --git a/Documentation/devicetree/bindings/net/bluetooth/aic,aic8800d80-bt.yaml b/Documentation/devicetree/bindings/net/bluetooth/aic,aic8800d80-bt.yaml
> new file mode 100644
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/bluetooth/aic,aic8800d80-bt.yaml

[ ... ]

> +allOf:
> +  - $ref: bluetooth-controller.yaml#
> +  - if:
> +      properties:
> +        compatible:
> +          const: aic,aic8800d80-bt-sdio
> +    then:
> +      properties:
> +        reg:
> +          const: [[1]]

[Severity: Low]
Is this the correct syntax for constraining the reg property?

In devicetree YAML bindings, reg constraints are typically expressed using
standard json-schema properties like minimum/maximum or an items list
definition. Does using a 2D array [[1]] expose the internal parsing
representation of dt-schema, making it unidiomatic? Could this fail
validation in future tooling updates or confuse other developers?

> +        max-speed: false
> +        aic,firmware-sdio: false
> +      required:
> +        - reg
> +    else:
> +      properties:
> +        reg: false
> +        max-speed:
> +          const: 1500000
> +      required:
> +        - max-speed

[Severity: Medium]
Does this incorrectly mandate a fixed max-speed?

According to standard devicetree principles and the core
serial-peripheral-props.yaml specification, max-speed represents a
board-specific hardware limitation and should only be present if the maximum
is less than the slave device can support. By requiring it and forcing it to
const: 1500000, does this force redundant, fixed configuration into every
device tree using this compatible string?

Would this also prevent developers from specifying a lower speed if their
particular board design has signal quality issues? If the device always
operates at 1.5 Mbaud by default, should the kernel driver handle that based
on the compatible string rather than hardcoding a required static property in
the DT?

> +        - aic,firmware-sdio
> +      allOf:
> +        - $ref: /schemas/serial/serial-peripheral-props.yaml#

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1789897503.git.yanli.yang@bedmex.com?part=2