linux-kernel - Re: [PATCH V7 1/2] dt-bindings: firmware: bootstats: Add the dtschema

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:   Wed, 5 Jul 2023 08:17:06 +0200
From:   Krzysztof Kozlowski <krzysztof.kozlowski@...aro.org>
To:     Souradeep Chowdhury <quic_schowdhu@...cinc.com>,
        Andy Gross <agross@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...ainline.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Rob Herring <robh+dt@...nel.org>, Arnd Bergmann <arnd@...db.de>
Cc:     linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        Sibi Sankar <quic_sibis@...cinc.com>,
        Rajendra Nayak <quic_rjendra@...cinc.com>
Subject: Re: [PATCH V7 1/2] dt-bindings: firmware: bootstats: Add the dtschema

On 05/07/2023 07:32, Souradeep Chowdhury wrote:
> Add the device tree bindings for boot stats. This has 2 properties
> pre-abl-time and abl-time which records the timestamps for boot
> stats.
> 
> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@...cinc.com>
> Link: https://lore.kernel.org/linux-arm-kernel/7d397e67-5d56-4975-98af-1ac9746c07f4@app.fastmail.com/
> ---
>  .../bindings/firmware/qcom,bootstats.yaml     | 38 +++++++++++++++++++
>  1 file changed, 38 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/firmware/qcom,bootstats.yaml
> 
> diff --git a/Documentation/devicetree/bindings/firmware/qcom,bootstats.yaml b/Documentation/devicetree/bindings/firmware/qcom,bootstats.yaml
> new file mode 100644
> index 000000000000..22e697524058
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/firmware/qcom,bootstats.yaml
> @@ -0,0 +1,38 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/firmware/qcom,bootstats.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Boot Stats

This explains nothing...

> +
> +maintainers:
> +  - Souradeep Chowdhury <quic_schowdhu@...cinc.com>
> +
> +description: |

Drop |

> +    Qualcomm's proprietary Android boot-loaders capture boot time

Wrong indentation.

> +    stats, like the time when the bootloader started execution and
> +    at what point the bootloader handed over control to the kernel.
> +    This is captured in the unit of ms in devicetree property.
> +
> +properties:
> +  pre-abl-time:
> +    description: The property to store the starting point of abl in ms.

String instead of ms? No, this is the craziest idea I saw recently. Use
-ms suffix.

> +    $ref: /schemas/types.yaml#/definitions/string-array
> +
> +  abl-time:
> +    description: The property to store the duration of abl in ms.
> +    $ref: /schemas/types.yaml#/definitions/string-array

I have no clue what this entire binding is about. Nothing can bind to
it, no usage explained. Properties are not used to "store the duration".
This does not look like suitable for DT, drop entire binding.

> +
> +required:
> +  - pre-abl-time
> +  - abl-time
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    bootstats {
> +        pre-abl-time="17627";
> +        abl-time="26748";

Missing spaces. Open existing DTS and look at existing coding style.

Best regards,
Krzysztof