[1/2] dt-bindings: reserved-memory: rmtfs: Allow dynamic allocation

Message ID 20230530193436.3833889-2-quic_bjorande@quicinc.com
State New
Headers
Series soc: qcom: rmtfs: Support dynamic allocation |

Commit Message

Bjorn Andersson May 30, 2023, 7:34 p.m. UTC
  Allow instances of the qcom,rmtfs-mem either be defined as a
reserved-memory regoin, or just standalone given just a size.

This relieve the DeviceTree source author the need to come up with a
static memory region for the region.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 .../reserved-memory/qcom,rmtfs-mem.yaml       | 23 ++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)
  

Comments

Konrad Dybcio May 30, 2023, 7:37 p.m. UTC | #1
On 30.05.2023 21:34, Bjorn Andersson wrote:
> Allow instances of the qcom,rmtfs-mem either be defined as a
> reserved-memory regoin, or just standalone given just a size.
> 
> This relieve the DeviceTree source author the need to come up with a
> static memory region for the region.
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  .../reserved-memory/qcom,rmtfs-mem.yaml       | 23 ++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> index bab982f00485..8b5de033f9ac 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> @@ -14,13 +14,16 @@ description: |
>  maintainers:
>    - Bjorn Andersson <bjorn.andersson@linaro.org>
>  
> -allOf:
> -  - $ref: reserved-memory.yaml
> -
>  properties:
>    compatible:
>      const: qcom,rmtfs-mem
>  
> +  qcom,alloc-size:
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    description:
> +      Requested size of the rmtfs memory allocation, when not defined as a
> +      reserved-memory region.
> +
>    qcom,client-id:
>      $ref: /schemas/types.yaml#/definitions/uint32
>      description: >
> @@ -36,6 +39,11 @@ properties:
>  required:
>    - qcom,client-id
>  
> +oneOf:
> +  - $ref: reserved-memory.yaml
> +  - required:
> +      - qcom,alloc-size
> +
>  unevaluatedProperties: false
>  
>  examples:
> @@ -53,3 +61,12 @@ examples:
>              qcom,client-id = <1>;
>          };
>      };
> +  - |
> +    rmtfs {
> +        compatible = "qcom,rmtfs-mem";
> +
> +        qcom,alloc-size = <(2*1024*1024)>;
2 nitty nits:

- Most uses of DT arithmetic put spaces between the operands
- You could add a comment explaining what this example brings to
  the table

Konrad

> +        qcom,client-id = <1>;
> +        qcom,vmid = <15>;
> +    };
> +...
  
Krzysztof Kozlowski May 31, 2023, 8:05 a.m. UTC | #2
On 30/05/2023 21:34, Bjorn Andersson wrote:
> Allow instances of the qcom,rmtfs-mem either be defined as a
> reserved-memory regoin, or just standalone given just a size.

typo: region

I am pretty sure I saw some patches from Qualcomm also making one of
reserved-memory users a bit more dynamic (some boot-thingy?). Would be
nice to unify...

> 
> This relieve the DeviceTree source author the need to come up with a
> static memory region for the region.

If you region does not have to be static, why bothering with the size in
DT? I assume this can be really dynamic and nothing is needed in DT. Not
even size.

Best regards,
Krzysztof
  
Bjorn Andersson May 31, 2023, 5:44 p.m. UTC | #3
On Wed, May 31, 2023 at 10:05:50AM +0200, Krzysztof Kozlowski wrote:
> On 30/05/2023 21:34, Bjorn Andersson wrote:
> > Allow instances of the qcom,rmtfs-mem either be defined as a
> > reserved-memory regoin, or just standalone given just a size.
> 
> typo: region
> 
> I am pretty sure I saw some patches from Qualcomm also making one of
> reserved-memory users a bit more dynamic (some boot-thingy?). Would be
> nice to unify...
> 
> > 
> > This relieve the DeviceTree source author the need to come up with a
> > static memory region for the region.
> 
> If you region does not have to be static, why bothering with the size in
> DT? I assume this can be really dynamic and nothing is needed in DT. Not
> even size.
> 

The size, client-id and vmid need to match the firmware and access
policy configuration, and are as such device-specific properties for the
region.

The desired size is available during the rmtfs handshake, so it's
plausible that one could come up with a mechanism of dynamic allocation.
But this would be complicated (as I'd prefer not to have the rmtfs
service in the kernel...), and we'd still end up with something in DT to
affect placement etc - often a reserved-memory region to specifically
define the placement.

We'd still need the client-id and vmid properties, and we still need
some mechanism to instantiate the rmtfs service and something that will
configure the access permissions for the region...

Regards,
Bjorn
  

Patch

diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
index bab982f00485..8b5de033f9ac 100644
--- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
@@ -14,13 +14,16 @@  description: |
 maintainers:
   - Bjorn Andersson <bjorn.andersson@linaro.org>
 
-allOf:
-  - $ref: reserved-memory.yaml
-
 properties:
   compatible:
     const: qcom,rmtfs-mem
 
+  qcom,alloc-size:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description:
+      Requested size of the rmtfs memory allocation, when not defined as a
+      reserved-memory region.
+
   qcom,client-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: >
@@ -36,6 +39,11 @@  properties:
 required:
   - qcom,client-id
 
+oneOf:
+  - $ref: reserved-memory.yaml
+  - required:
+      - qcom,alloc-size
+
 unevaluatedProperties: false
 
 examples:
@@ -53,3 +61,12 @@  examples:
             qcom,client-id = <1>;
         };
     };
+  - |
+    rmtfs {
+        compatible = "qcom,rmtfs-mem";
+
+        qcom,alloc-size = <(2*1024*1024)>;
+        qcom,client-id = <1>;
+        qcom,vmid = <15>;
+    };
+...