| Message ID | 1673611126-13803-1-git-send-email-quic_mojha@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/3] dt-bindings: reserved-memory: ramoops: Update the binding | expand |
Hi, 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". > > 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)? > >> >> 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. I was thinking, since this size is part reserved-memory.yaml and we have allOf: - $ref: "reserved-memory.yaml" Is your comment still applies? -Mukesh > >> >> 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 > > Any reason in naming it differently then existing one? You have there > example. > >> + 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 >
On 27/01/2023 13:29, Mukesh Ojha wrote: >>> +oneOf: >>> + - required: >>> + - reg >>> + >>> + - required: >>> + - size >> >> There is no such property. You cannot require it. > > I was thinking, since this size is part reserved-memory.yaml and > we have > > allOf: > - $ref: "reserved-memory.yaml" > > Is your comment still applies? Ah, you are right. It's ok. Best regards, Krzysztof
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 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 { + compatible = "ramoops"; + alloc-ranges = <0x0 0x00000000 0xffffffff 0xffffffff>; + size = <0x0 0x10000>; /* 64kB */ + console-size = <0x8000>; /* 32kB */ + record-size = <0x400>; /* 1kB */ + ecc-size = <16>; + }; + }; + };
Update the ramoops region binding document with details like region can also be reserved dynamically apart from reserving it statically. 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(-)