Re: [PATCH v4 6/7] dt-bindings: rtc-rv8803: add tamper detection property node

[Krzysztof Kozlowski <krzk@kernel.org>]

On Wed, May 21, 2025 at 11:05:51AM GMT, Markus Burri wrote:
> The RV8901 RTC chip provides a function to store timestamp events.
> There are three input pins (EVIN1-3) available for triggering.
> The input pins can be configured and adapted according to the connected
> hardware.
> Add document of tamper detection properties for epson,rx8901 rtc chip.
> 
> Signed-off-by: Markus Burri <markus.burri@mt.com>
> ---
>  .../devicetree/bindings/rtc/epson,rx8900.yaml | 37 +++++++++++++++++--
>  1 file changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/rtc/epson,rx8900.yaml b/Documentation/devicetree/bindings/rtc/epson,rx8900.yaml
> index 03af81754482..2682cbb9097d 100644
> --- a/Documentation/devicetree/bindings/rtc/epson,rx8900.yaml
> +++ b/Documentation/devicetree/bindings/rtc/epson,rx8900.yaml
> @@ -9,9 +9,6 @@ title: EPSON RX8900 / Microcrystal RV8803 Real-Time Clock
>  maintainers:
>    - Marek Vasut <marex@denx.de>
>  
> -allOf:
> -  - $ref: rtc.yaml#
> -
>  properties:
>    compatible:
>      enum:
> @@ -33,6 +30,40 @@ properties:
>  
>    wakeup-source: true
>  
> +  tamper:
> +    description: Subnode for tamper configuration.
> +      This subnode is only available for epson,rx8901.

Drop last sentence. Don't repeat constraints in free form text.

> +    type: object
> +    additionalProperties: false
> +
> +    properties:
> +      buffer-overwrite:

Missing vendor prefix or where is this property defined?

> +        type: boolean
> +        description: Set the buffer mode to overwrite. Default is inhibit.

Why woould this be a board configuration? You described the desired
Linux feature or behavior, not the actual hardware. The bindings are
about the latter, so instead you need to rephrase the property and its
description to match actual hardware capabilities/features/configuration
etc.

> +
> +    patternProperties:
> +      "^evin-[0-3]$":

Commit says three, schema says four.

Where is this property defined? Which common schema? Or is it vendor
property?

> +        $ref: /schemas/types.yaml#/definitions/uint32-array
> +        minItems: 3
> +        maxItems: 3
> +        description: External event input pin configuration.
> +          The configuration is an array of tree values and contains
> +          "pull-resistor", "trigger" and "filter".

What are the values here? Missing constraints, missing defaults.

Best regards,
Krzysztof