linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] soc: qcom: Add support for Qualcomm Modem Processing SubSystem DSM memory
@ 2022-11-16 10:16 Neil Armstrong
  2022-11-16 10:16 ` [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS " Neil Armstrong
  2022-11-16 10:16 ` [PATCH 2/2] soc: qcom: add MDSS DSM memory driver Neil Armstrong
  0 siblings, 2 replies; 11+ messages in thread
From: Neil Armstrong @ 2022-11-16 10:16 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Krzysztof Kozlowski, Rob Herring,
	Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, Neil Armstrong, devicetree

The Qualcomm SM8550 SoC Modem Processing SubSystem requires that a memory
region named DSM should be shared with the Application Processor SubSystem.

This adds bindings for this MPSS DSM memory and driver implementation to
share this memory region with the Modem Processing SubSystem.

To: Andy Gross <agross@kernel.org>
To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konrad.dybcio@somainline.org>
To: Rob Herring <robh+dt@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Frank Rowand <frowand.list@gmail.com>
Cc: linux-arm-msm@vger.kernel.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

---
Neil Armstrong (2):
      dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
      soc: qcom: add MDSS DSM memory driver

 .../reserved-memory/qcom,mpss-dsm-mem.yaml         | 37 +++++++++
 drivers/of/platform.c                              |  1 +
 drivers/soc/qcom/Kconfig                           | 10 +++
 drivers/soc/qcom/Makefile                          |  1 +
 drivers/soc/qcom/mpss_dsm_mem.c                    | 95 ++++++++++++++++++++++
 5 files changed, 144 insertions(+)
---
base-commit: 3c1f24109dfc4fb1a3730ed237e50183c6bb26b3
change-id: 20221114-narmstrong-sm8550-upstream-mpss_dsm-21c438c65f9b

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-16 10:16 [PATCH 0/2] soc: qcom: Add support for Qualcomm Modem Processing SubSystem DSM memory Neil Armstrong
@ 2022-11-16 10:16 ` Neil Armstrong
  2022-11-16 12:17   ` Krzysztof Kozlowski
  2022-11-16 10:16 ` [PATCH 2/2] soc: qcom: add MDSS DSM memory driver Neil Armstrong
  1 sibling, 1 reply; 11+ messages in thread
From: Neil Armstrong @ 2022-11-16 10:16 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Krzysztof Kozlowski, Rob Herring,
	Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, Neil Armstrong, devicetree

This documents the Qualcomm Modem Processing SubSystem DSM shared memory.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../reserved-memory/qcom,mpss-dsm-mem.yaml         | 37 ++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml
new file mode 100644
index 000000000000..65f37e1356d4
--- /dev/null
+++ b/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/reserved-memory/qcom,mpss-dsm-mem.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: Qualcomm Modem Processing SubSystem DSM Memory
+
+description: |
+  This binding describes the Qualcomm Modem Processing SubSystem DSM, which serves the
+  purpose of describing the shared memory region used for MPSS remote processors.
+
+maintainers:
+  - Bjorn Andersson <bjorn.andersson@linaro.org>
+
+allOf:
+  - $ref: "reserved-memory.yaml"
+
+properties:
+  compatible:
+    const: qcom,mpss-dsm-mem
+
+unevaluatedProperties: false
+
+examples:
+  - |
+    reserved-memory {
+        #address-cells = <1>;
+        #size-cells = <1>;
+        ranges;
+
+        mpss-dsm@86700000 {
+            compatible = "qcom,mpss-dsm-mem";
+            reg = <0x86700000 0xe0000>;
+            no-map;
+        };
+    };

-- 
b4 0.10.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/2] soc: qcom: add MDSS DSM memory driver
  2022-11-16 10:16 [PATCH 0/2] soc: qcom: Add support for Qualcomm Modem Processing SubSystem DSM memory Neil Armstrong
  2022-11-16 10:16 ` [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS " Neil Armstrong
@ 2022-11-16 10:16 ` Neil Armstrong
  1 sibling, 0 replies; 11+ messages in thread
From: Neil Armstrong @ 2022-11-16 10:16 UTC (permalink / raw)
  To: Konrad Dybcio, Bjorn Andersson, Krzysztof Kozlowski, Rob Herring,
	Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, Neil Armstrong, devicetree

This adds a driver for the Qualcomm Modem Processing SubSystem DSM memory
used to assign such regions of memory with remote MPSS processors.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/of/platform.c           |  1 +
 drivers/soc/qcom/Kconfig        | 10 +++++
 drivers/soc/qcom/Makefile       |  1 +
 drivers/soc/qcom/mpss_dsm_mem.c | 95 +++++++++++++++++++++++++++++++++++++++++
 4 files changed, 107 insertions(+)

diff --git a/drivers/of/platform.c b/drivers/of/platform.c
index 81c8c227ab6b..d5a9962641f4 100644
--- a/drivers/of/platform.c
+++ b/drivers/of/platform.c
@@ -504,6 +504,7 @@ EXPORT_SYMBOL_GPL(of_platform_default_populate);
 
 static const struct of_device_id reserved_mem_matches[] = {
 	{ .compatible = "phram" },
+	{ .compatible = "qcom,mpss-dsm-mem" },
 	{ .compatible = "qcom,rmtfs-mem" },
 	{ .compatible = "qcom,cmd-db" },
 	{ .compatible = "qcom,smem" },
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 024e420f1bb7..7624a8af56bf 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -95,6 +95,16 @@ config QCOM_QMI_HELPERS
 	tristate
 	depends on NET
 
+config QCOM_MPSS_DSM_MEM
+	tristate "Qualcomm Modem Processing SubSystem DSM memory driver"
+	depends on ARCH_QCOM
+	select QCOM_SCM
+	help
+	  The Qualcomm Modem Processing SubSystem DSM memory driver is used to
+	  assign regions of DSM memory with remote MPSS processors.
+
+	  Say y here if you intend to boot the modem remoteproc.
+
 config QCOM_RMTFS_MEM
 	tristate "Qualcomm Remote Filesystem memory driver"
 	depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index d66604aff2b0..0d0e850f1a18 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -16,6 +16,7 @@ qcom_rpmh-y			+= rpmh-rsc.o
 qcom_rpmh-y			+= rpmh.o
 obj-$(CONFIG_QCOM_SMD_RPM)	+= smd-rpm.o
 obj-$(CONFIG_QCOM_SMEM) +=	smem.o
+obj-$(CONFIG_QCOM_MPSS_DSM_MEM) += mpss_dsm_mem.o
 obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
 obj-$(CONFIG_QCOM_SMP2P)	+= smp2p.o
 obj-$(CONFIG_QCOM_SMSM)	+= smsm.o
diff --git a/drivers/soc/qcom/mpss_dsm_mem.c b/drivers/soc/qcom/mpss_dsm_mem.c
new file mode 100644
index 000000000000..5584bd101aee
--- /dev/null
+++ b/drivers/soc/qcom/mpss_dsm_mem.c
@@ -0,0 +1,95 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2022 Linaro Ltd.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/of.h>
+#include <linux/of_reserved_mem.h>
+#include <linux/slab.h>
+#include <linux/qcom_scm.h>
+
+struct qcom_mpss_dsm_mem {
+	phys_addr_t addr;
+	phys_addr_t size;
+
+	unsigned int perms;
+};
+
+static int qcom_mpss_dsm_mem_probe(struct platform_device *pdev)
+{
+	struct device_node *node = pdev->dev.of_node;
+	struct qcom_scm_vmperm perm;
+	struct reserved_mem *rmem;
+	struct qcom_mpss_dsm_mem *mpss_dsm_mem;
+	int ret;
+
+	if (!qcom_scm_is_available())
+		return -EPROBE_DEFER;
+
+	rmem = of_reserved_mem_lookup(node);
+	if (!rmem) {
+		dev_err(&pdev->dev, "failed to acquire memory region\n");
+		return -EINVAL;
+	}
+
+	mpss_dsm_mem = kzalloc(sizeof(*mpss_dsm_mem), GFP_KERNEL);
+	if (!mpss_dsm_mem)
+		return -ENOMEM;
+
+	mpss_dsm_mem->addr = rmem->base;
+	mpss_dsm_mem->size = rmem->size;
+
+	perm.vmid = QCOM_SCM_VMID_MSS_MSA;
+	perm.perm = QCOM_SCM_PERM_RW;
+
+	mpss_dsm_mem->perms = BIT(QCOM_SCM_VMID_HLOS);
+	ret = qcom_scm_assign_mem(mpss_dsm_mem->addr, mpss_dsm_mem->size,
+				  &mpss_dsm_mem->perms, &perm, 1);
+	if (ret < 0) {
+		dev_err(&pdev->dev, "assign memory failed\n");
+		return ret;
+	}
+
+	dev_set_drvdata(&pdev->dev, mpss_dsm_mem);
+
+	return 0;
+}
+
+static int qcom_mpss_dsm_mem_remove(struct platform_device *pdev)
+{
+	struct qcom_mpss_dsm_mem *mpss_dsm_mem = dev_get_drvdata(&pdev->dev);
+	struct qcom_scm_vmperm perm;
+
+	perm.vmid = QCOM_SCM_VMID_HLOS;
+	perm.perm = QCOM_SCM_PERM_RW;
+
+	qcom_scm_assign_mem(mpss_dsm_mem->addr, mpss_dsm_mem->size,
+			    &mpss_dsm_mem->perms, &perm, 1);
+
+	return 0;
+}
+
+static const struct of_device_id qcom_mpss_dsm_mem_of_match[] = {
+	{ .compatible = "qcom,mpss-dsm-mem" },
+	{}
+};
+MODULE_DEVICE_TABLE(of, qcom_mpss_dsm_mem_of_match);
+
+static struct platform_driver qcom_mpss_dsm_mem_driver = {
+	.probe = qcom_mpss_dsm_mem_probe,
+	.remove = qcom_mpss_dsm_mem_remove,
+	.driver  = {
+		.name  = "qcom_mpss_dsm_mem",
+		.of_match_table = qcom_mpss_dsm_mem_of_match,
+	},
+};
+
+module_platform_driver(qcom_mpss_dsm_mem_driver);
+
+MODULE_AUTHOR("Linaro Ltd");
+MODULE_DESCRIPTION("Qualcomm DSM memory driver");
+MODULE_LICENSE("GPL");

-- 
b4 0.10.1

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-16 10:16 ` [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS " Neil Armstrong
@ 2022-11-16 12:17   ` Krzysztof Kozlowski
  2022-11-17  9:47     ` Neil Armstrong
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-16 12:17 UTC (permalink / raw)
  To: Neil Armstrong, Konrad Dybcio, Bjorn Andersson,
	Krzysztof Kozlowski, Rob Herring, Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, devicetree

On 16/11/2022 11:16, Neil Armstrong wrote:
> This documents the Qualcomm Modem Processing SubSystem DSM shared memory.

Do not use "This commit/patch".
https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95

> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  .../reserved-memory/qcom,mpss-dsm-mem.yaml         | 37 ++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml
> new file mode 100644
> index 000000000000..65f37e1356d4
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml
> @@ -0,0 +1,37 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/reserved-memory/qcom,mpss-dsm-mem.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"

Drop quotes from above.

I know that this and few further pieces came from existing files...

> +
> +title: Qualcomm Modem Processing SubSystem DSM Memory
> +
> +description: |
> +  This binding describes the Qualcomm Modem Processing SubSystem DSM, which serves the

Drop "This binding describes"

> +  purpose of describing the shared memory region used for MPSS remote processors.

Entire description seems like not wrapped at 80.

> +
> +maintainers:
> +  - Bjorn Andersson <bjorn.andersson@linaro.org>

Need to update the address.

> +
> +allOf:
> +  - $ref: "reserved-memory.yaml"

Drop quotes.

> +
> +properties:
> +  compatible:
> +    const: qcom,mpss-dsm-mem

Why do we need dedicated binding and compatible for it instead of using
memory-region phandle in the device?

> +
> +unevaluatedProperties: false


Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-16 12:17   ` Krzysztof Kozlowski
@ 2022-11-17  9:47     ` Neil Armstrong
  2022-11-18 10:45       ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: Neil Armstrong @ 2022-11-17  9:47 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Konrad Dybcio, Krzysztof Kozlowski,
	Rob Herring, Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, devicetree

On 16/11/2022 13:17, Krzysztof Kozlowski wrote:
> On 16/11/2022 11:16, Neil Armstrong wrote:
>> This documents the Qualcomm Modem Processing SubSystem DSM shared memory.
> 
> Do not use "This commit/patch".
> https://elixir.bootlin.com/linux/v5.17.1/source/Documentation/process/submitting-patches.rst#L95
> 
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   .../reserved-memory/qcom,mpss-dsm-mem.yaml         | 37 ++++++++++++++++++++++
>>   1 file changed, 37 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml b/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml
>> new file mode 100644
>> index 000000000000..65f37e1356d4
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reserved-memory/qcom,mpss-dsm-mem.yaml
>> @@ -0,0 +1,37 @@
>> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
>> +%YAML 1.2
>> +---
>> +$id: "http://devicetree.org/schemas/reserved-memory/qcom,mpss-dsm-mem.yaml#"
>> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> 
> Drop quotes from above.
> 
> I know that this and few further pieces came from existing files...

Yep sorry, I'll clean it up.

> 
>> +
>> +title: Qualcomm Modem Processing SubSystem DSM Memory
>> +
>> +description: |
>> +  This binding describes the Qualcomm Modem Processing SubSystem DSM, which serves the
> 
> Drop "This binding describes"
> 
>> +  purpose of describing the shared memory region used for MPSS remote processors.
> 
> Entire description seems like not wrapped at 80.
> 
>> +
>> +maintainers:
>> +  - Bjorn Andersson <bjorn.andersson@linaro.org>
> 
> Need to update the address.

Argh

> 
>> +
>> +allOf:
>> +  - $ref: "reserved-memory.yaml"
> 
> Drop quotes.
> 
>> +
>> +properties:
>> +  compatible:
>> +    const: qcom,mpss-dsm-mem
> 
> Why do we need dedicated binding and compatible for it instead of using
> memory-region phandle in the device?

So like rmtfs, this memory zone is shared between APPS and the MPSS subsystem.

Like rmtfs it makes no sense to link it to the MPSS PAS, since it's only a launcher,
it doesn't represent the MPSS subsystem.

In the PAS startup process, the resources are released from APPS once the MPSS subsystem
is running, which is not the case with the MPSS DSM where it must be shared during the whole
lifetime of the system.

Neil

> 
>> +
>> +unevaluatedProperties: false
> 
> 
> Best regards,
> Krzysztof
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-17  9:47     ` Neil Armstrong
@ 2022-11-18 10:45       ` Krzysztof Kozlowski
  2022-11-18 13:30         ` neil.armstrong
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-18 10:45 UTC (permalink / raw)
  To: neil.armstrong, Bjorn Andersson, Konrad Dybcio,
	Krzysztof Kozlowski, Rob Herring, Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, devicetree

On 17/11/2022 10:47, Neil Armstrong wrote:
>>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    const: qcom,mpss-dsm-mem
>>
>> Why do we need dedicated binding and compatible for it instead of using
>> memory-region phandle in the device?
> 
> So like rmtfs, this memory zone is shared between APPS and the MPSS subsystem.
> 
> Like rmtfs it makes no sense to link it to the MPSS PAS, since it's only a launcher,
> it doesn't represent the MPSS subsystem.

This also does not represent a device. Memory region is not a device, so
this is as well not correct representation of hardware.

> 
> In the PAS startup process, the resources are released from APPS once the MPSS subsystem
> is running, which is not the case with the MPSS DSM where it must be shared during the whole
> lifetime of the system.

I don't think that PAS releases the region. I checked the
qcom_q6v5_pas.c and there is only ioremap. The device stays loaded thus
the memory stays mapped.

We have already three of such "memory region devices" and we keep
growing it. It's not scalable.

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-18 10:45       ` Krzysztof Kozlowski
@ 2022-11-18 13:30         ` neil.armstrong
  2022-11-18 14:03           ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: neil.armstrong @ 2022-11-18 13:30 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio,
	Krzysztof Kozlowski, Rob Herring, Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, devicetree

On 18/11/2022 11:45, Krzysztof Kozlowski wrote:
> On 17/11/2022 10:47, Neil Armstrong wrote:
>>>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    const: qcom,mpss-dsm-mem
>>>
>>> Why do we need dedicated binding and compatible for it instead of using
>>> memory-region phandle in the device?
>>
>> So like rmtfs, this memory zone is shared between APPS and the MPSS subsystem.
>>
>> Like rmtfs it makes no sense to link it to the MPSS PAS, since it's only a launcher,
>> it doesn't represent the MPSS subsystem.
> 
> This also does not represent a device. Memory region is not a device, so
> this is as well not correct representation of hardware.

I never used the term device so far, but a shared memory region with a platform
specific process to share the region between subsystems.

> 
>>
>> In the PAS startup process, the resources are released from APPS once the MPSS subsystem
>> is running, which is not the case with the MPSS DSM where it must be shared during the whole
>> lifetime of the system.
> 
> I don't think that PAS releases the region. I checked the
> qcom_q6v5_pas.c and there is only ioremap. The device stays loaded thus
> the memory stays mapped.
Yes PAS does release the firmware region when the firmware is started,
qcom_scm_pas_metadata_release() does that.

> 
> We have already three of such "memory region devices" and we keep
> growing it. It's not scalable.

If we want to properly describe this, we must then represent the MPSS subsystem
and associate this memory region.

> 
> Best regards,
> Krzysztof

Thanks,
Neil

> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-18 13:30         ` neil.armstrong
@ 2022-11-18 14:03           ` Krzysztof Kozlowski
  2022-11-23 10:19             ` neil.armstrong
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-18 14:03 UTC (permalink / raw)
  To: neil.armstrong, Bjorn Andersson, Konrad Dybcio,
	Krzysztof Kozlowski, Rob Herring, Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, devicetree

On 18/11/2022 14:30, neil.armstrong@linaro.org wrote:
> On 18/11/2022 11:45, Krzysztof Kozlowski wrote:
>> On 17/11/2022 10:47, Neil Armstrong wrote:
>>>>
>>>>> +
>>>>> +properties:
>>>>> +  compatible:
>>>>> +    const: qcom,mpss-dsm-mem
>>>>
>>>> Why do we need dedicated binding and compatible for it instead of using
>>>> memory-region phandle in the device?
>>>
>>> So like rmtfs, this memory zone is shared between APPS and the MPSS subsystem.
>>>
>>> Like rmtfs it makes no sense to link it to the MPSS PAS, since it's only a launcher,
>>> it doesn't represent the MPSS subsystem.
>>
>> This also does not represent a device. Memory region is not a device, so
>> this is as well not correct representation of hardware.
> 
> I never used the term device so far, but a shared memory region with a platform
> specific process to share the region between subsystems.

Yes, but you create a device in patch #2 for this binding.

> 
>>
>>>
>>> In the PAS startup process, the resources are released from APPS once the MPSS subsystem
>>> is running, which is not the case with the MPSS DSM where it must be shared during the whole
>>> lifetime of the system.
>>
>> I don't think that PAS releases the region. I checked the
>> qcom_q6v5_pas.c and there is only ioremap. The device stays loaded thus
>> the memory stays mapped.
> Yes PAS does release the firmware region when the firmware is started,
> qcom_scm_pas_metadata_release() does that.

Indeed, I see it now.

> 
>>
>> We have already three of such "memory region devices" and we keep
>> growing it. It's not scalable.
> 
> If we want to properly describe this, we must then represent the MPSS subsystem
> and associate this memory region.

I don't see why. None of devices in your DTS reference this memory
region, so it is purely to keep it mapped for Modem, right? In such case
I still do not get why PAS/PIL, who starts and stops the remote
processor, could not prepare the memory and share it with modem.

The point is that this memory region is nothing special and does not
deserve its own compatible. We keep adding here compatibles to fulfill
some Linux implementation specifics, don't we?

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-18 14:03           ` Krzysztof Kozlowski
@ 2022-11-23 10:19             ` neil.armstrong
  2022-11-24 13:57               ` Krzysztof Kozlowski
  0 siblings, 1 reply; 11+ messages in thread
From: neil.armstrong @ 2022-11-23 10:19 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio,
	Krzysztof Kozlowski, Rob Herring, Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, devicetree

On 18/11/2022 15:03, Krzysztof Kozlowski wrote:
> On 18/11/2022 14:30, neil.armstrong@linaro.org wrote:
>> On 18/11/2022 11:45, Krzysztof Kozlowski wrote:
>>> On 17/11/2022 10:47, Neil Armstrong wrote:
>>>>>
>>>>>> +
>>>>>> +properties:
>>>>>> +  compatible:
>>>>>> +    const: qcom,mpss-dsm-mem
>>>>>
>>>>> Why do we need dedicated binding and compatible for it instead of using
>>>>> memory-region phandle in the device?
>>>>
>>>> So like rmtfs, this memory zone is shared between APPS and the MPSS subsystem.
>>>>
>>>> Like rmtfs it makes no sense to link it to the MPSS PAS, since it's only a launcher,
>>>> it doesn't represent the MPSS subsystem.
>>>
>>> This also does not represent a device. Memory region is not a device, so
>>> this is as well not correct representation of hardware.
>>
>> I never used the term device so far, but a shared memory region with a platform
>> specific process to share the region between subsystems.
> 
> Yes, but you create a device in patch #2 for this binding.

Implementation details are out of scope here.

> 
>>
>>>
>>>>
>>>> In the PAS startup process, the resources are released from APPS once the MPSS subsystem
>>>> is running, which is not the case with the MPSS DSM where it must be shared during the whole
>>>> lifetime of the system.
>>>
>>> I don't think that PAS releases the region. I checked the
>>> qcom_q6v5_pas.c and there is only ioremap. The device stays loaded thus
>>> the memory stays mapped.
>> Yes PAS does release the firmware region when the firmware is started,
>> qcom_scm_pas_metadata_release() does that.
> 
> Indeed, I see it now.
> 
>>
>>>
>>> We have already three of such "memory region devices" and we keep
>>> growing it. It's not scalable.
>>
>> If we want to properly describe this, we must then represent the MPSS subsystem
>> and associate this memory region.
> 
> I don't see why. None of devices in your DTS reference this memory
> region, so it is purely to keep it mapped for Modem, right? In such case
> I still do not get why PAS/PIL, who starts and stops the remote
> processor, could not prepare the memory and share it with modem.

OK you've got a point, but this is still an implementation detail.

I got some more background about why this memory zone appeared, before it
was including in the modem reserved-memories, but for flexibility reasons
this is now in the hands of the APPS runtime (linux) to setup this memory
zone and share it to the MPSS subsystem.

So the only requirement for the PAS is to make sure this zone is shared
before starting the startup process, not to actually do the share setup.

The zone will need to be shared whatever the PAS state is (probe, startup, remove, ...).

> 
> The point is that this memory region is nothing special and does not
> deserve its own compatible. We keep adding here compatibles to fulfill
> some Linux implementation specifics, don't we?

Yes this memory region is now special since it has to be shared explicitly.

> 
> Best regards,
> Krzysztof
> 

Thanks,
Neil

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-23 10:19             ` neil.armstrong
@ 2022-11-24 13:57               ` Krzysztof Kozlowski
  2022-11-29  8:27                 ` neil.armstrong
  0 siblings, 1 reply; 11+ messages in thread
From: Krzysztof Kozlowski @ 2022-11-24 13:57 UTC (permalink / raw)
  To: neil.armstrong, Bjorn Andersson, Konrad Dybcio,
	Krzysztof Kozlowski, Rob Herring, Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, devicetree

On 23/11/2022 11:19, neil.armstrong@linaro.org wrote:
>>>> We have already three of such "memory region devices" and we keep
>>>> growing it. It's not scalable.
>>>
>>> If we want to properly describe this, we must then represent the MPSS subsystem
>>> and associate this memory region.
>>
>> I don't see why. None of devices in your DTS reference this memory
>> region, so it is purely to keep it mapped for Modem, right? In such case
>> I still do not get why PAS/PIL, who starts and stops the remote
>> processor, could not prepare the memory and share it with modem.
> 
> OK you've got a point, but this is still an implementation detail.
> 
> I got some more background about why this memory zone appeared, before it
> was including in the modem reserved-memories, but for flexibility reasons
> this is now in the hands of the APPS runtime (linux) to setup this memory
> zone and share it to the MPSS subsystem.
> 
> So the only requirement for the PAS is to make sure this zone is shared
> before starting the startup process, not to actually do the share setup.
> 
> The zone will need to be shared whatever the PAS state is (probe, startup, remove, ...).

I don't understand here. The memory region should be shared before
remote processor start or before the PAS probe? If the second, why? What
if your driver probes a bit later - after PAS probe? Then all is lost...

Best regards,
Krzysztof


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS DSM memory
  2022-11-24 13:57               ` Krzysztof Kozlowski
@ 2022-11-29  8:27                 ` neil.armstrong
  0 siblings, 0 replies; 11+ messages in thread
From: neil.armstrong @ 2022-11-29  8:27 UTC (permalink / raw)
  To: Krzysztof Kozlowski, Bjorn Andersson, Konrad Dybcio,
	Krzysztof Kozlowski, Rob Herring, Andy Gross, Frank Rowand
  Cc: linux-kernel, linux-arm-msm, devicetree

On 24/11/2022 14:57, Krzysztof Kozlowski wrote:
> On 23/11/2022 11:19, neil.armstrong@linaro.org wrote:
>>>>> We have already three of such "memory region devices" and we keep
>>>>> growing it. It's not scalable.
>>>>
>>>> If we want to properly describe this, we must then represent the MPSS subsystem
>>>> and associate this memory region.
>>>
>>> I don't see why. None of devices in your DTS reference this memory
>>> region, so it is purely to keep it mapped for Modem, right? In such case
>>> I still do not get why PAS/PIL, who starts and stops the remote
>>> processor, could not prepare the memory and share it with modem.
>>
>> OK you've got a point, but this is still an implementation detail.
>>
>> I got some more background about why this memory zone appeared, before it
>> was including in the modem reserved-memories, but for flexibility reasons
>> this is now in the hands of the APPS runtime (linux) to setup this memory
>> zone and share it to the MPSS subsystem.
>>
>> So the only requirement for the PAS is to make sure this zone is shared
>> before starting the startup process, not to actually do the share setup.
>>
>> The zone will need to be shared whatever the PAS state is (probe, startup, remove, ...).
> 
> I don't understand here. The memory region should be shared before
> remote processor start or before the PAS probe? If the second, why? What
> if your driver probes a bit later - after PAS probe? Then all is lost...

After offline discussions, I'll associate the DSM memory zone to the PAS MPSS
loader instead to make sure the zone is shared before startup.

Thanks,
Neil

> 
> Best regards,
> Krzysztof
> 


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2022-11-29  8:27 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-16 10:16 [PATCH 0/2] soc: qcom: Add support for Qualcomm Modem Processing SubSystem DSM memory Neil Armstrong
2022-11-16 10:16 ` [PATCH 1/2] dt-bindings: reserved-memory: document Qualcomm MPSS " Neil Armstrong
2022-11-16 12:17   ` Krzysztof Kozlowski
2022-11-17  9:47     ` Neil Armstrong
2022-11-18 10:45       ` Krzysztof Kozlowski
2022-11-18 13:30         ` neil.armstrong
2022-11-18 14:03           ` Krzysztof Kozlowski
2022-11-23 10:19             ` neil.armstrong
2022-11-24 13:57               ` Krzysztof Kozlowski
2022-11-29  8:27                 ` neil.armstrong
2022-11-16 10:16 ` [PATCH 2/2] soc: qcom: add MDSS DSM memory driver Neil Armstrong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).