Re: [PATCH v2 2/2] soc: qcom: add MDSS DSM memory driver

[Srinivas Kandagatla <srinivas.kandagatla@linaro.org>]

On 18/11/2022 08:53, Neil Armstrong wrote:
> Add a driver for the Qualcomm Modem Processing SubSystem DSM memory
> used to assign such regions of memory with remote MPSS processors.
> 
> This memory zone is shared between the APPS and the MPSS subsystem,
> and must be configured during the whole lifetime of the system.
> 
> 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 3507095a69f6..d4bb03292f45 100644
> --- a/drivers/of/platform.c
> +++ b/drivers/of/platform.c
> @@ -505,6 +505,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");

memory leak here?


> +		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);
> +

same here.

> +	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");
>