Re: [PATCH v2 1/3] dt-bindings: reserved-memory: ramoops: Update the binding

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

 



Hi Krzysztof,

On 1/13/2023 5:34 PM, Krzysztof Kozlowski wrote:
Subject: drop second/last, redundant "binding". The "dt-bindings" prefix
is already stating that these are bindings.

Your subject says nothing. Everything is "update".


I will fix this.

On 13/01/2023 12:58, Mukesh Ojha wrote:
Update the ramoops region binding document with details
like region can also be reserved dynamically apart from
reserving it statically.
 > So what exactly can be here reserved dynamically? And what does it mean
'dynamically'? By whom? How is this property of hardware (not OS)?

Normally, for ramoops, already known physical address range from DDR are mentioned in Device tree which gets reserved from reserved-memory kernel framework(example 1 in this file).

By being dynamic, I meant, with this we are letting the framework to get the size from a range of allowable address (<0x0 0x00000000 0xffffffff 0xffffffff>).

Let me know, if you need more detail.


Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
---
Change in v2:
   - Added this patch as per changes going to be done in patch 3/3

  .../bindings/reserved-memory/ramoops.yaml          | 34 ++++++++++++++++++++--
  1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/reserved-memory/ramoops.yaml b/Documentation/devicetree/bindings/reserved-memory/ramoops.yaml
index 0391871..54e46e8 100644
--- a/Documentation/devicetree/bindings/reserved-memory/ramoops.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/ramoops.yaml
@@ -10,7 +10,8 @@ description: |
    ramoops provides persistent RAM storage for oops and panics, so they can be
    recovered after a reboot. This is a child-node of "/reserved-memory", and
    is named "ramoops" after the backend, rather than "pstore" which is the
-  subsystem.
+  subsystem. This region can be reserved both statically or dynamically by
+  using appropriate property in device tree.
Parts of this storage may be set aside for other persistent log buffers, such
    as kernel log messages, or for optional ECC error-correction data.  The total
@@ -112,7 +113,13 @@ unevaluatedProperties: false
required:
    - compatible
-  - reg
+
+oneOf:
+  - required:
+      - reg
+
+  - required:
+      - size

There is no such property. You cannot require it.

Forgot to mention this.

--- a/Documentation/devicetree/bindings/reserved-memory/ramoops.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/ramoops.yaml
@@ -37,6 +37,20 @@ properties:
   reg:
     description: region of memory that is preserved between reboots

+  size:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    minItems: 1
+    maxItems: 2
+    description: >
+      Length based on parent's \#size-cells. Size in bytes of memory to
+      reserve.
+
+  alloc-ranges:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: >
+      Address and Length pairs. Specifies regions of memory that are
+      acceptable to allocate from.
+
   ecc-size:
     $ref: /schemas/types.yaml#/definitions/uint32
description: enables ECC support and specifies ECC buffer size in bytes
@@ -119,6 +133,7 @@ oneOf:
       - reg

   - required:
+      - alloc-ranges
       - size

anyOf:
    - required: [record-size]
@@ -142,3 +149,26 @@ examples:
              };
          };
      };
+
+  - |
+    / {
+        compatible = "foo";
+        model = "foo";
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        reserved-memory {
+            #address-cells = <2>;
+            #size-cells = <2>;
+            ranges;
+
+            ramoops: ramoops_region {

Node names should be generic, no underscores in node names.
https://devicetree-specification.readthedocs.io/en/latest/chapter2-devicetree-basics.html#generic-names-recommendation


Thanks.

Any reason in naming it differently then existing one? You have there
example.

ramoops@bfdf0000 is not valid after dynamic allocation of memory.
Probably, will mention it as

ramoops_region: ramoops { ??

-Mukesh

+                compatible = "ramoops";
+                alloc-ranges = <0x0 0x00000000 0xffffffff 0xffffffff>;
+                size = <0x0 0x10000>;       /* 64kB */
+                console-size = <0x8000>;    /* 32kB */
+                record-size = <0x400>;      /*  1kB */
+                ecc-size = <16>;
+            };
+        };
+    };

Best regards,
Krzysztof




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux