Re: [PATCH v2 6/8] dt-bindings: reserved-memory: Add secure CMA reserved memory range

From
Krzysztof Kozlowski <hidden>
Date
2023-11-11 12:48:58
Also in
dri-devel, linux-arm-kernel, linux-media, linux-mediatek, lkml
Reply to
Yong Wu, 2023-11-11
On 11/11/2023 12:15, Yong Wu wrote:
Add a binding for describing the secure CMA reserved memory range. The
memory range also will be defined in the TEE firmware. It means the TEE
will be configured with the same address/size that is being set in this
DT node.

Signed-off-by: Yong Wu <yong.wu@mediatek.com>
---
What was the outcome of previous discussion? I don't see any references
to the conclusion and your changelog "Reword the dt-binding description"
is way too generic.

You must explain what happened here.
quoted hunk ↗ jump to hunk
 .../reserved-memory/secure_cma_region.yaml    | 44 +++++++++++++++++++
 1 file changed, 44 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml
diff --git a/Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml b/Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml
new file mode 100644
index 000000000000..8ab559595fbe
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/secure_cma_region.yaml
@@ -0,0 +1,44 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/reserved-memory/secure_cma_region.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Secure Reserved CMA Region
+
+description:
+  This binding describes a CMA region that can dynamically transition
Describe the hardware or firmware, not the binding. Drop first four
words and rephrase it.
+between secure and non-secure states that a TEE can allocate memory
+from.
It does not look like you tested the bindings, at least after quick
look. Please run `make dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).
Maybe you need to update your dtschema and yamllint.

Do not send untested code.
quoted hunk ↗ jump to hunk
+
+maintainers:
+  - Yong Wu <yong.wu@mediatek.com>
+
+allOf:
+  - $ref: reserved-memory.yaml
+
+properties:
+  compatible:
+    const: secure_cma_region
Still wrong compatible. Look at other bindings - there is nowhere
underscore. Look at other reserved memory bindings especially.

Also, CMA is a Linux thingy, so either not suitable for bindings at all,
or you need Linux specific compatible. I don't quite get why do you even
put CMA there - adding Linux specific stuff will get obvious pushback...

quoted hunk ↗ jump to hunk
+
+required:
+  - compatible
+  - reg
+  - reusable
+
+unevaluatedProperties: false
+
+examples:
+  - |
+
Stray blank line.
quoted hunk ↗ jump to hunk
+    reserved-memory {
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges;
+
+        reserved-memory@80000000 {
+            compatible = "secure_cma_region";
+            reusable;
+            reg = <0x80000000 0x18000000>;
reg is second property. Open DTS and check how it is there.
+        };
+    };
Best regards,
Krzysztof
Thread (26 messages) 26 messages, 8 authors, last reply 2023-11-22
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help