Re: [PATCH v2 4/8] dt-bindings: power: reset: add bindings for NVMEM hardware storing PSCR Data

[Krzysztof Kozlowski]

On 24/01/2024 13:22, Oleksij Rempel wrote:
> Add device tree bindings that describe hardware implementations of
> Non-Volatile Memory (NVMEM) used for storing Power State Change Reasons
> (PSCR).

A nit, subject: drop second/last, redundant "bindings for". The
"dt-bindings" prefix is already stating that these are bindings.
See also:
https://elixir.bootlin.com/linux/v6.7-rc8/source/Documentation/devicetree/bindings/submitting-patches.rst#L18

> 
> Signed-off-by: Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> ---
>  .../bindings/power/reset/pscrr-nvmem.yaml     | 53 +++++++++++++++++++
>  1 file changed, 53 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/power/reset/pscrr-nvmem.yaml
> 
> diff --git a/Documentation/devicetree/bindings/power/reset/pscrr-nvmem.yaml b/Documentation/devicetree/bindings/power/reset/pscrr-nvmem.yaml
> new file mode 100644
> index 000000000000..779920dea283
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/power/reset/pscrr-nvmem.yaml
> @@ -0,0 +1,53 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/power/reset/pscrr-nvmem.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Generic NVMEM Power State Change Reason Recorder
> +
> +maintainers:
> +  - Oleksij Rempel <o.rempel@xxxxxxxxxxxxxx>
> +
> +description: This binding describes the Non-Volatile Memory (NVMEM) hardware

Same comment and also: describe the hardware, not the binding. s/This
binding describes/something useful/

> +  that stores Power State Change Reasons (PSCR).
> +
> +allOf:
> +  - $ref: pscrr.yaml#
> +
> +properties:
> +  compatible:
> +    const: pscrr-nvmem
> +

So that's a driver :/. Maybe Rob will like it, but it's a no from me.
Please come up with something really suiting DEVICES, not DRIVERS.

> +  nvmem-cells:
> +    description: |

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

> +      A phandle pointing to the nvmem-cells node where the power state change
> +      reasons are stored.
> +    maxItems: 1
> +


Best regards,
Krzysztof