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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 7/5/2023 11:47 AM, Krzysztof Kozlowski wrote:
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@xxxxxxxxxxx>
Link: https://lore.kernel.org/linux-arm-kernel/7d397e67-5d56-4975-98af-1ac9746c07f4@xxxxxxxxxxxxxxxx/
---
  .../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...

Ack


+
+maintainers:
+  - Souradeep Chowdhury <quic_schowdhu@xxxxxxxxxxx>
+
+description: |

Drop |

Ack


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

Wrong indentation.

Ack

+    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.

This binding was created as per the suggestion on version 6 of the patch by Arnd. The idea was that these 2 devicetree properties will be used to populate the bootstat values from the bootloader and exposed to the user via /sys/firmware/devicetree/ directly.

Details in the link below:-

https://lore.kernel.org/lkml/7d397e67-5d56-4975-98af-1ac9746c07f4@xxxxxxxxxxxxxxxx/T/#mbdc9ad95fcbb5ad7b56c6996a3933899b42d982c

Can you suggest any alternative way to represent this as a 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.

Ack


Best regards,
Krzysztof




[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux