Re: [PATCH 1/2] dt-bindings: input: add Semtech SX951x binding

[Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>]

On 05/05/2025 22:38, David Bauer wrote:
> Add device-tree binding for the Semtech SX9512/SX9513 family of touch
> controllers with integrated LED driver.
> 
> Signed-off-by: David Bauer <mail@david-bauer.net>

You CC-ed an address, which suggests you do not work on mainline kernel
or you do not use get_maintainers.pl/b4/patman. Please rebase and always
work on mainline or start using mentioned tools, so correct addresses
will be used.

Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument, so you will
not CC people just because they made one commit years ago). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.

Tools like b4 or scripts/get_maintainer.pl provide you proper list of
people, so fix your workflow. Tools might also fail if you work on some
ancient tree (don't, instead use mainline) or work on fork of kernel
(don't, instead use mainline). Just use b4 and everything should be
fine, although remember about `b4 prep --auto-to-cc` if you added new
patches to the patchset.


> ---
>  .../bindings/input/semtech,sx951x.yaml        | 180 ++++++++++++++++++
>  1 file changed, 180 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> 
> diff --git a/Documentation/devicetree/bindings/input/semtech,sx951x.yaml b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> new file mode 100644
> index 000000000000..e4f938decd86
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/input/semtech,sx951x.yaml
> @@ -0,0 +1,180 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/input/semtech,sx951x.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Semtech SX9512/SX9513 based capacitive touch sensors
> +
> +description: |

Do not need '|' unless you need to preserve formatting.

> +  The Semtech SX9512/SX9513 Family of capacitive touch controllers
> +  with integrated LED drivers. The device communication is using I2C only.
> +
> +maintainers:
> +  - David Bauer <mail@david-bauer.net>
> +
> +properties:
> +  compatible:
> +    enum:
> +      - semtech,sx9512
> +      - semtech,sx9513

Devices are not compatible? What are the differences?

> +
> +  reg:
> +    maxItems: 1
> +
> +  '#address-cells':
> +    const: 1
> +
> +  '#size-cells':
> +    const: 0
> +
> +  poll-interval:
> +    default: 100
> +    description: |

Do not need '|' unless you need to preserve formatting. Same comment
everywhere.

> +      The polling interval for touch events in milliseconds.

Missing -ms property unit suffix... unless you are using existing
property from common schema, but I do not see any reference (and thus
unevaluatedProperties at the end).

> +
> +patternProperties:
> +  "^channel@[0-7]$":
> +    $ref: input.yaml#
> +    type: object
> +    description: |
> +      Each node represents a channel of the touch controller.
> +      Each channel provides a capacitive touch sensor input and
> +      an LED driver output.
> +
> +    properties:
> +      reg:
> +        enum: [0, 1, 2, 3, 4, 5, 6, 7]
> +
> +      linux,keycodes:
> +        maxItems: 1
> +        description: |
> +          Specifies an array of numeric keycode values to
> +          be used for the channels. If this property is
> +          omitted, the channel is not used as a key.
> +
> +      semtech,cin-delta:

Use proper unit suffix and express it in pF.

> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 3
> +        default: 3
> +        description: |
> +          The capacitance delta which is used to detect a touch
> +          or release event. The property value is mapped to a
> +          farad range between 7pF and 2.3pF internally. The delta
> +          becomes smaller the higher the value is.
> +
> +      semtech,sense-threshold:
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 255
> +        default: 4
> +        description: |
> +          The threshold value after which the channel detects a touch.
> +          Refer to the datasheet for the internal calculation of the
> +          resulting touch sensitivity.
> +
> +      led:

I think subnode is here not needed. This should be part of the channel,
probably.

> +        $ref: /schemas/leds/common.yaml#
> +        type: object
> +        unevaluatedProperties: false
> +        description: |
> +          Presence of this property indicates the channel
> +          is used as an LED driver.
> +
> +    required:
> +      - reg
> +
> +    additionalProperties: false

unevaluatedProperties instead

> +
> +
> +required:
> +  - compatible
> +  - reg
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/input/input.h>
> +    #include <dt-bindings/leds/common.h>
> +    i2c {
> +      #address-cells = <1>;
> +      #size-cells = <0>;
> +
> +      touch@2b {
> +        compatible = "semtech,sx9512";
> +

Drop blank line

> +        reg = <0x2b>;
> +


Best regards,
Krzysztof