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

[PATCH 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>
---
 .../devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml  | 5 +++++
 1 file changed, 5 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..1d8c4621178a 100644
--- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
+++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
@@ -31,6 +31,11 @@ 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 the NAV VM
+
 required:
   - qcom,client-id
 
-- 
2.39.0

[PATCH 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>
---
 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 1/2] dt-bindings: reserved-memory: rmtfs: Document qcom,assign-to-nav

[Krzysztof Kozlowski]

On 13/12/2022 18:03, 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>
> ---
>  .../devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml  | 5 +++++
>  1 file changed, 5 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..1d8c4621178a 100644
> --- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
> @@ -31,6 +31,11 @@ properties:
>      description: >
>        vmid of the remote processor, to set up memory protection
>  
> +  qcom,assign-to-nav:
> +    type: boolean
> +    description: >

No need for '>'

> +      whether to also assign the region to the NAV VM

Here and in property name you express desired Linux driver action, but
it is better to express the property of the hardware. What is
different/special in these SoCs or their configuration that additional
assignment is needed?


Best regards,
Krzysztof

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

[Konrad Dybcio]

On 14.12.2022 09:11, Krzysztof Kozlowski wrote:
> On 13/12/2022 18:03, 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>
>> ---
>>  .../devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml  | 5 +++++
>>  1 file changed, 5 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..1d8c4621178a 100644
>> --- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
>> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
>> @@ -31,6 +31,11 @@ properties:
>>      description: >
>>        vmid of the remote processor, to set up memory protection
>>  
>> +  qcom,assign-to-nav:
>> +    type: boolean
>> +    description: >
> 
> No need for '>'
> 
>> +      whether to also assign the region to the NAV VM
> 
> Here and in property name you express desired Linux driver action, but
> it is better to express the property of the hardware. What is
> different/special in these SoCs or their configuration that additional
> assignment is needed?
Honestly, I have no clue.. probably there's something more complex
than was there before connected to GPS..

Konrad
> 
> 
> Best regards,
> Krzysztof
> 

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

[Krzysztof Kozlowski]

On 14/12/2022 10:55, Konrad Dybcio wrote:
> 
> 
> On 14.12.2022 09:11, Krzysztof Kozlowski wrote:
>> On 13/12/2022 18:03, 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>
>>> ---
>>>  .../devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml  | 5 +++++
>>>  1 file changed, 5 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..1d8c4621178a 100644
>>> --- a/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
>>> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,rmtfs-mem.yaml
>>> @@ -31,6 +31,11 @@ properties:
>>>      description: >
>>>        vmid of the remote processor, to set up memory protection
>>>  
>>> +  qcom,assign-to-nav:
>>> +    type: boolean
>>> +    description: >
>>
>> No need for '>'
>>
>>> +      whether to also assign the region to the NAV VM
>>
>> Here and in property name you express desired Linux driver action, but
>> it is better to express the property of the hardware. What is
>> different/special in these SoCs or their configuration that additional
>> assignment is needed?
> Honestly, I have no clue.. probably there's something more complex
> than was there before connected to GPS..

One thing could be number of VMs - if it is even something known.

Best regards,
Krzysztof