Messages in this thread Patch in this message | | | Date | Fri, 13 Jan 2023 18:52:31 +0530 | Subject | Re: [PATCH v2 1/3] dt-bindings: reserved-memory: ramoops: Update the binding | From | Mukesh Ojha <> |
| |
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@quicinc.com> >> --- >> 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 >
| |