[PATCH v2 1/2] dt-bindings: reserved-memory: rmtfs: Document qcom,assign-to-nav

[PATCH v2 1/2] dt-bindings: reserved-memory: rmtfs: Document qcom,assign-to-nav

[Konrad Dybcio]

Some SoCs mandate that the RMTFS is also assigned to the NAV VM, while
others really don't want that. Since it has to be conditional, add a
bool property to toggle this behavior.

Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
v1 -> v2:
- Rewrite the newly added description

 .../devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
index 2998f1c8f0db..4026788a4e40 100644
--- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
@@ -31,6 +31,12 @@ properties:
     description: >
       vmid of the remote processor, to set up memory protection
 
+  qcom,assign-to-nav:
+    type: boolean
+    description:
+      Whether to also assign the region to a third (NAV) VM, as opposed to
+      the usual 2.
+
 required:
   - qcom,client-id
 
-- 
2.39.0

[PATCH v2 2/2] soc: qcom: rmtfs: Optionally map RMTFS to NAV VM

[Konrad Dybcio]

From: Loic Poulain <loic.poulain@linaro.org>

Some SoCs require that RMTFS is also mapped to the NAV VM. Trying to
power on the modem without that results in the whole platform crashing
and forces a hard reboot within about 2 seconds.

Signed-off-by: Loic Poulain <loic.poulain@linaro.org>
[Konrad: reword, make conditional, add a define for NAV VMID]
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
v1 -> v2:
No changes

 drivers/soc/qcom/rmtfs_mem.c | 22 +++++++++++++++-------
 include/linux/qcom_scm.h     |  1 +
 2 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
index 0feaae357821..999f2c5b49b0 100644
--- a/drivers/soc/qcom/rmtfs_mem.c
+++ b/drivers/soc/qcom/rmtfs_mem.c
@@ -171,12 +171,13 @@ static void qcom_rmtfs_mem_release_device(struct device *dev)
 static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
 {
 	struct device_node *node = pdev->dev.of_node;
-	struct qcom_scm_vmperm perms[2];
+	struct qcom_scm_vmperm perms[3];
 	struct reserved_mem *rmem;
 	struct qcom_rmtfs_mem *rmtfs_mem;
+	bool assign_to_nav;
 	u32 client_id;
 	u32 vmid;
-	int ret;
+	int ret, cnt = 0;
 
 	rmem = of_reserved_mem_lookup(node);
 	if (!rmem) {
@@ -191,6 +192,8 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
 
 	}
 
+	assign_to_nav = of_property_read_bool(node, "qcom,assign-to-nav");
+
 	rmtfs_mem = kzalloc(sizeof(*rmtfs_mem), GFP_KERNEL);
 	if (!rmtfs_mem)
 		return -ENOMEM;
@@ -236,14 +239,19 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
 			goto remove_cdev;
 		}
 
-		perms[0].vmid = QCOM_SCM_VMID_HLOS;
-		perms[0].perm = QCOM_SCM_PERM_RW;
-		perms[1].vmid = vmid;
-		perms[1].perm = QCOM_SCM_PERM_RW;
+		perms[cnt].vmid = QCOM_SCM_VMID_HLOS;
+		perms[cnt++].perm = QCOM_SCM_PERM_RW;
+		perms[cnt].vmid = vmid;
+		perms[cnt++].perm = QCOM_SCM_PERM_RW;
+
+		if (assign_to_nav) {
+			perms[cnt].vmid = QCOM_SCM_VMID_NAV;
+			perms[cnt++].perm = QCOM_SCM_PERM_RW;
+		}
 
 		rmtfs_mem->perms = BIT(QCOM_SCM_VMID_HLOS);
 		ret = qcom_scm_assign_mem(rmtfs_mem->addr, rmtfs_mem->size,
-					  &rmtfs_mem->perms, perms, 2);
+					  &rmtfs_mem->perms, perms, cnt);
 		if (ret < 0) {
 			dev_err(&pdev->dev, "assign memory failed\n");
 			goto remove_cdev;
diff --git a/include/linux/qcom_scm.h b/include/linux/qcom_scm.h
index f8335644a01a..150b72edb879 100644
--- a/include/linux/qcom_scm.h
+++ b/include/linux/qcom_scm.h
@@ -55,6 +55,7 @@ enum qcom_scm_ice_cipher {
 #define QCOM_SCM_VMID_MSS_MSA    0xF
 #define QCOM_SCM_VMID_WLAN       0x18
 #define QCOM_SCM_VMID_WLAN_CE    0x19
+#define QCOM_SCM_VMID_NAV        0x2B
 #define QCOM_SCM_PERM_READ       0x4
 #define QCOM_SCM_PERM_WRITE      0x2
 #define QCOM_SCM_PERM_EXEC       0x1
-- 
2.39.0

Re: [PATCH v2 1/2] dt-bindings: reserved-memory: rmtfs: Document qcom,assign-to-nav

[Krzysztof Kozlowski]

On 02/01/2023 17:50, Konrad Dybcio wrote:
> Some SoCs mandate that the RMTFS is also assigned to the NAV VM, while
> others really don't want that. Since it has to be conditional, add a
> bool property to toggle this behavior.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> v1 -> v2:
> - Rewrite the newly added description


Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: [PATCH v2 1/2] dt-bindings: reserved-memory: rmtfs: Document qcom,assign-to-nav

[Bjorn Andersson]

On Mon, Jan 02, 2023 at 05:50:33PM +0100, Konrad Dybcio wrote:
> Some SoCs mandate that the RMTFS is also assigned to the NAV VM, while
> others really don't want that. Since it has to be conditional, add a
> bool property to toggle this behavior.
> 
> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> ---
> v1 -> v2:
> - Rewrite the newly added description
> 
>  .../devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> index 2998f1c8f0db..4026788a4e40 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> @@ -31,6 +31,12 @@ properties:
>      description: >
>        vmid of the remote processor, to set up memory protection
>  
> +  qcom,assign-to-nav:
> +    type: boolean
> +    description:
> +      Whether to also assign the region to a third (NAV) VM, as opposed to
> +      the usual 2.

For better or worse, the binding currently takes the vmid of the first
instance in qcom,vmid. Would it not be cleaner to turn qcom,vmid into an
array and pass the nav vmid as a second element in that array?

Regards,
Bjorn

> +
>  required:
>    - qcom,client-id
>  
> -- 
> 2.39.0
> 

Re: [PATCH v2 1/2] dt-bindings: reserved-memory: rmtfs: Document qcom,assign-to-nav

[Konrad Dybcio]

On 6.01.2023 18:55, Bjorn Andersson wrote:
> On Mon, Jan 02, 2023 at 05:50:33PM +0100, Konrad Dybcio wrote:
>> Some SoCs mandate that the RMTFS is also assigned to the NAV VM, while
>> others really don't want that. Since it has to be conditional, add a
>> bool property to toggle this behavior.
>>
>> Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
>> ---
>> v1 -> v2:
>> - Rewrite the newly added description
>>
>>  .../devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
>> index 2998f1c8f0db..4026788a4e40 100644
>> --- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
>> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
>> @@ -31,6 +31,12 @@ properties:
>>      description: >
>>        vmid of the remote processor, to set up memory protection
>>  
>> +  qcom,assign-to-nav:
>> +    type: boolean
>> +    description:
>> +      Whether to also assign the region to a third (NAV) VM, as opposed to
>> +      the usual 2.
> 
> For better or worse, the binding currently takes the vmid of the first
> instance in qcom,vmid. Would it not be cleaner to turn qcom,vmid into an
> array and pass the nav vmid as a second element in that array?
I suppose I could do that..

Konrad
> 
> Regards,
> Bjorn
> 
>> +
>>  required:
>>    - qcom,client-id
>>  
>> -- 
>> 2.39.0
>>