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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Thu, 25 Jan 2024 11:57:18 +0100
From: Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To: Oleksij Rempel <o.rempel@...gutronix.de>,
 Sebastian Reichel <sre@...nel.org>, Rob Herring <robh+dt@...nel.org>,
 Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
 Conor Dooley <conor+dt@...nel.org>,
 Srinivas Kandagatla <srinivas.kandagatla@...aro.org>
Cc: kernel@...gutronix.de, linux-kernel@...r.kernel.org,
 Liam Girdwood <lgirdwood@...il.com>, Mark Brown <broonie@...nel.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>,
 Daniel Lezcano <daniel.lezcano@...aro.org>, Zhang Rui <rui.zhang@...el.com>,
 Lukasz Luba <lukasz.luba@....com>, linux-pm@...r.kernel.org,
 devicetree@...r.kernel.org, Søren Andersen <san@...v.dk>
Subject: Re: [PATCH v2 4/8] dt-bindings: power: reset: add bindings for NVMEM
 hardware storing PSCR Data

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@...gutronix.de>
> ---
>  .../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@...gutronix.de>
> +
> +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