Re: [PATCH 1/2] dt-bindings: input: add OnePlus pogo-pin keyboard binding

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] dt-bindings: incorrect use of additionalProperties: false with allOf $ref
- [Medium] dt-bindings: custom properties lack vendor prefix and explicit type reference
- [Low] dt-bindings: unnecessary #address-cells and #size-cells in example serial node
--

commit 9e61ffe1ac817966b2b3b703148b7060c7724704
Author: Yibo Wang <wyibo6109@gmail.com>

dt-bindings: input: add OnePlus pogo-pin keyboard binding

Adds device tree bindings for the Tinno keyboard MCU used by OnePlus and
OPPO tablets over a serial bus.

> diff --git a/Documentation/devicetree/bindings/input/oneplus,pogo-keyboard.yaml b/Documentation/devicetree/bindings/input/oneplus,pogo-keyboard.yaml
> new file mode 100644
> index 0000000000000..3086a2c4e2895
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/oneplus,pogo-keyboard.yaml
[ ... ]
> +allOf:
> +  - $ref: /schemas/serial/serial-peripheral-props.yaml#
> +
> +properties:
> +  compatible:
> +    const: oneplus,pogo-keyboard
[ ... ]
> +  wake-gpios:
> +    description: GPIO signalling attach/detach, also used as the wake line.
> +    maxItems: 1
> +
> +  touchpad-xy-max:
> +    description: Touchpad maximum X and Y coordinates in units.
> +    items:
> +      - description: maximum X
> +      - description: maximum Y
> +    minItems: 2
> +    maxItems: 2
> +
> +  touchpad-xy-resolution:
> +    description: Touchpad X and Y resolution in units per millimetre.
> +    items:
> +      - description: X resolution
> +      - description: Y resolution
> +    minItems: 2
> +    maxItems: 2

[Severity: Medium]
Do these custom properties need a vendor prefix and an explicit type
definition?

Device tree guidelines mandate that non-standard, device-specific properties
must include a vendor prefix to avoid namespace collisions. These should
probably be oneplus,touchpad-xy-max and oneplus,touchpad-xy-resolution.

Also, because they do not end in a standard recognized suffix, dt-schema
validation cannot infer their types. Should they include an explicit type
reference to /schemas/types.yaml#/definitions/uint32-array to prevent
dt_binding_check from emitting a missing type definition warning?

> +
> +required:
> +  - compatible
> +  - power-gpios
> +  - tx-enable-gpios
> +  - wake-gpios
> +
> +additionalProperties: false

[Severity: Medium]
Will this block properties inherited from the serial-peripheral-props.yaml
reference?

Since the schema references the serial properties via an allOf block earlier,
using additionalProperties: false strictly limits allowed properties to only
those defined in this file's properties block.

Should this be changed to unevaluatedProperties: false to allow standard
serial properties like max-speed?

> +
> +examples:
> +  - |
> +    #include <dt-bindings/gpio/gpio.h>
> +
> +    serial {
> +        #address-cells = <1>;
> +        #size-cells = <0>;

[Severity: Low]
Are these address size properties necessary for this example?

The child keyboard node does not contain a reg property, nor does its name
include a unit-address. Declaring address cells without any addressable
children triggers an avoid_unnecessary_addr_size warning from dtc when
evaluating the example.

> +
> +        keyboard {
> +            compatible = "oneplus,pogo-keyboard";
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009121143.550498-1-2024312277@stu.hit.edu.cn?part=1