Hi Krzysztof!
Thanks a lot for the fast review!
On 23.12.23 15:30, Krzysztof Kozlowski wrote:
On 22/12/2023 20:51, Alexander Graf wrote:
With ftrace in KHO, we are creating an ABI between old kernel and new
kernel about the state that they transfer. To ensure that we document
that state and catch any breaking change, let's add its schema to the
common devicetree bindings. This way, we can quickly reason about the
state that gets passed.
Please use scripts/get_maintainers.pl to get a list of necessary people
and lists to CC (and consider --no-git-fallback argument). It might
happen, that command when run on an older kernel, gives you outdated
entries. Therefore please be sure you base your patches on recent Linux
kernel.
Ah, this is about directly CC'ing maintainers? I was slightly picky on
CCs since the CC list is already a bit long for this patch set, so I
limited the CC list to mailing lists and people that I know were
directly interested. Happy to CC you next time.
Please use subject prefixes matching the subsystem. You can get them for
example with `git log --oneline -- DIRECTORY_OR_FILE` on the directory
your patch is touching.
A nit, subject: drop second/last, redundant "bindings". The
"dt-bindings" prefix is already stating that these are bindings.
Happy to fix up for v3 :)
Signed-off-by: Alexander Graf <graf@xxxxxxxxxx>
---
.../bindings/kho/ftrace/ftrace-array.yaml | 46 +++++++++++++++
.../bindings/kho/ftrace/ftrace-cpu.yaml | 56 +++++++++++++++++++
.../bindings/kho/ftrace/ftrace.yaml | 48 ++++++++++++++++
3 files changed, 150 insertions(+)
create mode 100644 Documentation/devicetree/bindings/kho/ftrace/ftrace-array.yaml
create mode 100644 Documentation/devicetree/bindings/kho/ftrace/ftrace-cpu.yaml
create mode 100644 Documentation/devicetree/bindings/kho/ftrace/ftrace.yaml
diff --git a/Documentation/devicetree/bindings/kho/ftrace/ftrace-array.yaml b/Documentation/devicetree/bindings/kho/ftrace/ftrace-array.yaml
new file mode 100644
index 000000000000..9960fefc292d
--- /dev/null
+++ b/Documentation/devicetree/bindings/kho/ftrace/ftrace-array.yaml
@@ -0,0 +1,46 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/kho/ftrace/ftrace-array.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Ftrace trace array
+
Missing description. Commit msg also does not tell me much. This must
stand on its own and must describe the hardware. Whatever you have in
cover letter, does not matter, especially that you did not Cc us on it.
Alrighty, I'll add descriptions and make the commit message stand on its
own.
For quick reference: KHO is a new mechanism this patch set introduces
which allows Linux to pass arbitrary memory and metadata between kernels
on kexec. I'm reusing FDTs to implement the hand over protocol, as
Linux-to-Linux boot communication holds very similar properties to
firmware-to-Linux boot communication. So this binding is not about
hardware; it's about preserving Linux subsystem state across kexec.
For more details, please refer to the KHO documentation which is part of
patch 7 of this patch set:
https://lore.kernel.org/lkml/20231222195144.24532-2-graf@xxxxxxxxxx/
+maintainers:
+ - Alexander Graf <graf@xxxxxxxxxx>
+
+properties:
+ compatible:
+ enum:
+ - ftrace,array-v1
+
+ trace_flags:
Underscores are not allowed. Does not look like generic property.
Let me make it "trace-flags" to not have underscores. Could you please
elaborate on what you mean by generic property?
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Bitmap of all the trace flags that were enabled in the trace array at the
+ point of serialization.
+
+# Subnodes will be of type "ftrace,cpu-v1", one each per CPU
+additionalProperties: true
No, this must be false. And it goes after required:
Ok, making it false and adding pattern matches instead for subnodes.
+
+required:
+ - compatible
+ - trace_flags
+
+examples:
+ - |
+ ftrace {
+ compatible = "ftrace-v1";
+ events = <1 1 2 2 3 3>;
+
+ global_trace {
Again, no underscores.
Ok :)
+ compatible = "ftrace,array-v1";
+ trace_flags = < 0x3354601 >;
+
+ cpu0 {
+ compatible = "ftrace,cpu-v1";
+ cpu = < 0x00 >;
Drop redundant spaces.
I don't understand what you're referring to as redundant spaces? Double
checking, I believe indentation is off for every line below "ftrace {".
Is that what you're referring to? Fixing :)
+ mem = < 0x101000000ULL 0x38ULL 0x101000100ULL 0x1000ULL 0x101000038ULL 0x38ULL 0x101002000ULL 0x1000ULL>;
? Do you see any of such syntax in DTS?
I was trying to make it easy to reason to readers about 64bit numbers -
and then potentially extend dtc to consume that new syntax. KHO DTs are
native/little endian, so dtc already has some difficulties interpreting
it which I'll need to fix up with patches to it eventually :). I'll
change it to something that looks more 32bit'y for now.
Alex
Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879
