On 30.05.2023 21:34, Bjorn Andersson wrote:
> In some configurations, the exact placement of the rmtfs shared memory
> region isn't so strict. In the current implementation the author of the
> DeviceTree source is forced to make up a memory region.
IIUC the test here would be... "works" / "doesn't", just as if one
misplaced the fixed region?
Does the downstream sharedmem-uio driver do any additional cryptic
magic or does it simply rely on the vendor's cma/dma pool settings?
Can we replicate its behavior to stop hardcoding rmtfs, period?
>
> Extend the rmtfs memory driver to relieve the author of this
> responsibility by introducing support for using dynamic allocation in
> the driver.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@xxxxxxxxxxx>
> ---
> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 10 ++++
> drivers/soc/qcom/rmtfs_mem.c | 66 +++++++++++++++++++------
> 2 files changed, 61 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> index d1440b790fa6..e6191b8ba4c6 100644
> --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts
> @@ -12,6 +12,8 @@
> #include "pm8998.dtsi"
> #include "pmi8998.dtsi"
>
> +/delete-node/ &rmtfs_mem;
> +
> / {
> model = "Qualcomm Technologies, Inc. SDM845 MTP";
> compatible = "qcom,sdm845-mtp", "qcom,sdm845";
> @@ -48,6 +50,14 @@ vreg_s4a_1p8: pm8998-smps4 {
> vin-supply = <&vph_pwr>;
> };
>
> + rmtfs {
> + compatible = "qcom,rmtfs-mem";
> +
> + qcom,alloc-size = <(2*1024*1024)>;
> + qcom,client-id = <1>;
> + qcom,vmid = <15>;
> + };
This should have been a separate patch.
> +
> thermal-zones {
> xo_thermal: xo-thermal {
> polling-delay-passive = <0>;
> diff --git a/drivers/soc/qcom/rmtfs_mem.c b/drivers/soc/qcom/rmtfs_mem.c
> index f83811f51175..5f56ded9f905 100644
> --- a/drivers/soc/qcom/rmtfs_mem.c
> +++ b/drivers/soc/qcom/rmtfs_mem.c
> @@ -3,6 +3,8 @@
> * Copyright (c) 2017 Linaro Ltd.
> */
>
> +#include "linux/gfp_types.h"
> +#include "linux/sizes.h"
<>?
> #include <linux/kernel.h>
> #include <linux/cdev.h>
> #include <linux/err.h>
> @@ -168,23 +170,63 @@ static void qcom_rmtfs_mem_release_device(struct device *dev)
> kfree(rmtfs_mem);
> }
>
> +static int qcom_rmtfs_acquire_mem(struct device *dev, struct qcom_rmtfs_mem *rmtfs_mem)
> +{
> + struct device_node *node = dev->of_node;
> + struct reserved_mem *rmem;
> + dma_addr_t dma_addr;
> + void *mem;
> + u32 size;
> + int ret;
> +
> + rmem = of_reserved_mem_lookup(node);
> + if (rmem) {
> + rmtfs_mem->addr = rmem->base;
> + rmtfs_mem->size = rmem->size;
> +
> + rmtfs_mem->base = devm_memremap(&rmtfs_mem->dev, rmtfs_mem->addr,
> + rmtfs_mem->size, MEMREMAP_WC);
> + if (IS_ERR(rmtfs_mem->base)) {
> + dev_err(dev, "failed to remap rmtfs_mem region\n");
> + return PTR_ERR(rmtfs_mem->base);
> + }
> +
> + return 0;
> + }
> +
> + ret = of_property_read_u32(node, "qcom,alloc-size", &size);
> + if (ret < 0) {
> + dev_err(dev, "rmtfs of unknown size\n");
> + return -EINVAL;
> + }
> +
> + /*
> + * Ensure that the protected region isn't adjacent to other protected
> + * regions by allocating an empty page on either side.
> + */
> + mem = dma_alloc_coherent(dev, size + 2 * SZ_4K, &dma_addr, GFP_KERNEL);
Should this be made pagesize-independent? Can we even run non-4K kernels on msm?
Konrad
> + if (mem) {
> + rmtfs_mem->base = mem + SZ_4K;
> + rmtfs_mem->addr = dma_addr + SZ_4K;
> + rmtfs_mem->size = size;
> +
> + return 0;
> + }
> +
> + dev_err(dev, "unable to allocate memory for rmtfs mem\n");
> + return -ENOMEM;
> +}
> +
> static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
> {
> struct device_node *node = pdev->dev.of_node;
> struct qcom_scm_vmperm perms[NUM_MAX_VMIDS + 1];
> - struct reserved_mem *rmem;
> struct qcom_rmtfs_mem *rmtfs_mem;
> u32 client_id;
> u32 vmid[NUM_MAX_VMIDS];
> int num_vmids;
> int ret, i;
>
> - rmem = of_reserved_mem_lookup(node);
> - if (!rmem) {
> - dev_err(&pdev->dev, "failed to acquire memory region\n");
> - return -EINVAL;
> - }
> -
> ret = of_property_read_u32(node, "qcom,client-id", &client_id);
> if (ret) {
> dev_err(&pdev->dev, "failed to parse \"qcom,client-id\"\n");
> @@ -196,22 +238,16 @@ static int qcom_rmtfs_mem_probe(struct platform_device *pdev)
> if (!rmtfs_mem)
> return -ENOMEM;
>
> - rmtfs_mem->addr = rmem->base;
> rmtfs_mem->client_id = client_id;
> - rmtfs_mem->size = rmem->size;
>
> device_initialize(&rmtfs_mem->dev);
> rmtfs_mem->dev.parent = &pdev->dev;
> rmtfs_mem->dev.groups = qcom_rmtfs_mem_groups;
> rmtfs_mem->dev.release = qcom_rmtfs_mem_release_device;
>
> - rmtfs_mem->base = devm_memremap(&rmtfs_mem->dev, rmtfs_mem->addr,
> - rmtfs_mem->size, MEMREMAP_WC);
> - if (IS_ERR(rmtfs_mem->base)) {
> - dev_err(&pdev->dev, "failed to remap rmtfs_mem region\n");
> - ret = PTR_ERR(rmtfs_mem->base);
> + ret = qcom_rmtfs_acquire_mem(&pdev->dev, rmtfs_mem);
> + if (ret < 0)
> goto put_device;
> - }
>
> cdev_init(&rmtfs_mem->cdev, &qcom_rmtfs_mem_fops);
> rmtfs_mem->cdev.owner = THIS_MODULE;
