On Wed, Jun 28, 2023 at 05:17:13PM -0600, Rob Herring wrote:

On Wed, Jun 28, 2023 at 8:11 AM Pavan Kondeti [off-list ref] wrote:

quoted

On Wed, Jun 28, 2023 at 06:04:35PM +0530, Mukesh Ojha wrote:

quoted

Qualcomm ramoops minidump logger provide a means of storing
the ramoops data to some dynamically reserved memory instead
of traditionally implemented ramoops where the region should
be statically fixed ram region. Its device tree binding
would be exactly same as ramoops device tree binding and is
going to contain traditional ramoops frontend data and this
content will be collected via Qualcomm minidump infrastructure
provided from the boot firmware.

Signed-off-by: Mukesh Ojha <redacted>
---
 .../devicetree/bindings/soc/qcom/qcom,ramoops.yaml | 126 +++++++++++++++++++++
 1 file changed, 126 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,ramoops.yaml

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,ramoops.yaml b/Documentation/devicetree/bindings/soc/qcom/qcom,ramoops.yaml
new file mode 100644
index 000000000000..b1fdcf3f8ad4
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,ramoops.yaml

@@ -0,0 +1,126 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/soc/qcom/qcom,ramoops.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm Ramoops minidump logger
+
+description: |
+  Qualcomm ramoops minidump logger provide a means of storing the ramoops
+  data to some dynamically reserved memory instead of traditionally
+  implemented ramoops where the region should be statically fixed ram
+  region. Because of its similarity with ramoops it will also have same
+  set of property what ramoops have it in its schema and is going to
+  contain traditional ramoops frontend data and this region will be
+  collected via Qualcomm minidump infrastructure provided from the
+  boot firmware.
+
+maintainers:
+  - Mukesh Ojha <quic_mojha@quicinc.com>
+
+properties:
+  compatible:
+    items:
+      - enum:
+          - qcom,sm8450-ramoops
+      - const: qcom,ramoops
+
+  memory-region:
+    maxItems: 1
+    description: handle to memory reservation for qcom,ramoops region.
+
+  ecc-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: enables ECC support and specifies ECC buffer size in bytes
+    default: 0 # no ECC
+
+  record-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: maximum size in bytes of each kmsg dump
+    default: 0
+
+  console-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: size in bytes of log buffer reserved for kernel messages
+    default: 0
+
+  ftrace-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: size in bytes of log buffer reserved for function tracing and profiling
+    default: 0
+
+  pmsg-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: size in bytes of log buffer reserved for userspace messages
+    default: 0
+
+  mem-type:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: if present, sets the type of mapping is to be used to map the reserved region.
+    default: 0
+    oneOf:
+      - const: 0
+        description: write-combined
+      - const: 1
+        description: unbuffered
+      - const: 2
+        description: cached
+
+  max-reason:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 2 # log oopses and panics
+    maximum: 0x7fffffff
+    description: |
+      If present, sets maximum type of kmsg dump reasons to store.
+      This can be set to INT_MAX to store all kmsg dumps.
+      See include/linux/kmsg_dump.h KMSG_DUMP_* for other kmsg dump reason values.
+      Setting this to 0 (KMSG_DUMP_UNDEF), means the reason filtering will be
+      controlled by the printk.always_kmsg_dump boot param.
+      If unset, it will be 2 (KMSG_DUMP_OOPS), otherwise 5 (KMSG_DUMP_MAX).
+
+  flags:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    default: 0
+    description: |
+      If present, pass ramoops behavioral flags
+      (see include/linux/pstore_ram.h RAMOOPS_FLAG_* for flag values).
+
+  no-dump-oops:
+    deprecated: true
+    type: boolean
+    description: |
+      Use max_reason instead. If present, and max_reason is not specified,
+      it is equivalent to max_reason = 1 (KMSG_DUMP_PANIC).
+
+  unbuffered:
+    deprecated: true
+    type: boolean
+    description: |
+      Use mem_type instead. If present, and mem_type is not specified,
+      it is equivalent to mem_type = 1 and uses unbuffered mappings to map
+      the reserved region (defaults to buffered mappings mem_type = 0).
+      If both are specified -- "mem_type" overrides "unbuffered".
+

Most of the properties you added here are already documented at
Documentation/devicetree/bindings/reserved-memory/ramoops.yaml

That is certainly a problem. Don't define the same property more than
once. Not yet checked and enforced by the tools, but it will be.

quoted

Can't we just reference them here? would something like work?

max-reason:
  $ref: "../../reserved-memory/ramoops.yaml#/properties/max-reason

Can work, but no. Common properties need to go into a schema of common
properties which the device specific schemas reference.

Thanks for the clarification. We need to define the common properties
and make it available under /schemas/<>.yaml and add a reference to it
in these bindings. Is my understanding correct?

quoted

quoted

+unevaluatedProperties: false
+

will there be any additional properties be added dynamically? if not,
should not we use "additionalProperties: false" here?

I don't know what you mean by dynamically, but that's not the criteria
for which to use.

ok, I was wrong in saying dynamically. I should say "are there any other
properties that will be included that are not documented here".

is below my understanding correct?

additionalProperties needs to be set to false if at all this binding
defines all the possible properties (including child nodes) and not
referring to any other schemas.

If that is not the case and this binding references to other schemas,
then unevaluatedProperties could be used since additionalProperties
can't support combining schemas.

Thanks,
Pavan

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel