2017-04-22 17:35:27

by Bjorn Andersson

[permalink] [raw]
Subject: [RFC 1/3] dt-binding: soc: qcom: Add binding for RFSA

This adds the binding for describing shared memory buffers for
implementing the remote filesystem protocol.

Signed-off-by: Bjorn Andersson <[email protected]>
---

My initial attempt was to mimic the ramoops of just adding the compatible to
the reserved-memory node, but I have not been able to figure out a sane way of
getting hold of the base address in the case that the memory region is
described my a "size" only (done on some platforms).

The problem is that we create the reserved_mem objects (and remove the
memblocks) while we're still operating on the flattened representation, so
without a phandle it doesn't seem like we have anything to perform the
comparison with later on.

.../devicetree/bindings/soc/qcom/qcom,rfsa.txt | 43 ++++++++++++++++++++++
1 file changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt

diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt
new file mode 100644
index 000000000000..b4de0de74e46
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt
@@ -0,0 +1,43 @@
+Qualcomm Remote File System Access binding
+
+This binding describes the Qualcomm RFSA, which serves the purpose of managing
+the shared memory region used for remote processors to access block device data
+using the Remote Filesystem protocol.
+
+- compatible:
+ Usage: required
+ Value type: <stringlist>
+ Definition: must be:
+ "qcom,rfsa"
+
+- memory-region:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: handle to memory reservation the associated rfsa region.
+
+- qcom,client-id:
+ Usage: required
+ Value type: <u32>
+ Definition: identifier of the client to use this region for buffers.
+
+= EXAMPLE
+The following example shows the RFSA setup for APQ8016, with the RFSA region
+for the Hexagon DSP (id #1) located at 0x86700000.
+
+ reserved-memory {
+ #address-cells = <2>;
+ #size-cells = <2>;
+ ranges;
+
+ rmtfs: rmtfs@86700000 {
+ reg = <0x0 0x86700000 0x0 0xe0000>;
+ no-map;
+ };
+ };
+
+ hexagon-rfsa {
+ compatible = "qcom,rfsa";
+ memory-region = <&rmtfs>;
+
+ qcom,client-id = <1>;
+ };
--
2.12.0


2017-04-22 17:35:35

by Bjorn Andersson

[permalink] [raw]
Subject: [RFC 3/3] soc: qcom: rfsa driver

The rfsa driver is used for allocating and exposing regions of shared
memory with remote processors for the purpose of exchanging sector-data
between the remote filesystem service and its clients.

It provides accessors for the properties needed by the user space remote
filesystem implementation through sysfs and a character device that can be used
to read and write the requested chunks of data.

Signed-off-by: Bjorn Andersson <[email protected]>
---
drivers/soc/qcom/Kconfig | 8 ++
drivers/soc/qcom/Makefile | 1 +
drivers/soc/qcom/rfsa.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 270 insertions(+)
create mode 100644 drivers/soc/qcom/rfsa.c

diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index 9fca977ef18d..788a63cd430e 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -24,6 +24,14 @@ config QCOM_PM
modes. It interface with various system drivers to put the cores in
low power modes.

+config QCOM_RFSA
+ tristate "Qualcomm Remote Filesystem Access driver"
+ help
+ The Qualcomm remote filesystem access driver is used for allocating
+ and exposing regions of shared memory with remote processors for the
+ purpose of exchanging sector-data between the remote filesystem
+ service and its clients.
+
config QCOM_SMEM
tristate "Qualcomm Shared Memory Manager (SMEM)"
depends on ARCH_QCOM
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 414f0de274fa..d1bbc791ddc0 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -1,6 +1,7 @@
obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
obj-$(CONFIG_QCOM_PM) += spm.o
+obj-$(CONFIG_QCOM_RFSA) += rfsa.o
obj-$(CONFIG_QCOM_SMD_RPM) += smd-rpm.o
obj-$(CONFIG_QCOM_SMEM) += smem.o
obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
diff --git a/drivers/soc/qcom/rfsa.c b/drivers/soc/qcom/rfsa.c
new file mode 100644
index 000000000000..1b79976dad9d
--- /dev/null
+++ b/drivers/soc/qcom/rfsa.c
@@ -0,0 +1,261 @@
+/*
+ * Copyright (c) 2017 Linaro Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/cdev.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/of_fdt.h>
+#include <linux/dma-mapping.h>
+#include <linux/slab.h>
+#include <linux/uaccess.h>
+#include <linux/io.h>
+
+#define QCOM_RFSA_DEV_MAX (MINORMASK + 1)
+
+static dev_t qcom_rfsa_major;
+
+struct qcom_rfsa {
+ struct device dev;
+ struct cdev cdev;
+
+ void *base;
+ phys_addr_t addr;
+ phys_addr_t size;
+
+ unsigned int client_id;
+};
+
+static ssize_t qcom_rfsa_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf);
+
+static DEVICE_ATTR(phys_addr, 0400, qcom_rfsa_show, NULL);
+static DEVICE_ATTR(size, 0400, qcom_rfsa_show, NULL);
+static DEVICE_ATTR(client_id, 0400, qcom_rfsa_show, NULL);
+
+static ssize_t qcom_rfsa_show(struct device *dev,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct qcom_rfsa *rfsa = container_of(dev, struct qcom_rfsa, dev);
+
+ if (attr == &dev_attr_phys_addr)
+ return sprintf(buf, "%pa\n", &rfsa->addr);
+ if (attr == &dev_attr_size)
+ return sprintf(buf, "%pa\n", &rfsa->size);
+ if (attr == &dev_attr_client_id)
+ return sprintf(buf, "%d\n", rfsa->client_id);
+
+ return -EINVAL;
+}
+
+static struct attribute *qcom_rfsa_attrs[] = {
+ &dev_attr_phys_addr.attr,
+ &dev_attr_size.attr,
+ &dev_attr_client_id.attr,
+ NULL
+};
+ATTRIBUTE_GROUPS(qcom_rfsa);
+
+static int qcom_rfsa_open(struct inode *inode, struct file *filp)
+{
+ struct qcom_rfsa *rfsa = container_of(inode->i_cdev, struct qcom_rfsa, cdev);
+
+ get_device(&rfsa->dev);
+ filp->private_data = rfsa;
+
+ return 0;
+}
+static ssize_t qcom_rfsa_read(struct file *filp,
+ char __user *buf, size_t count, loff_t *f_pos)
+{
+ struct qcom_rfsa *rfsa = filp->private_data;
+
+ if (*f_pos >= rfsa->size)
+ return 0;
+
+ if (*f_pos + count >= rfsa->size)
+ count = rfsa->size - *f_pos;
+
+ if (copy_to_user(buf, rfsa->base + *f_pos, count))
+ return -EFAULT;
+
+ *f_pos += count;
+ return count;
+}
+
+static ssize_t qcom_rfsa_write(struct file *filp,
+ const char __user *buf, size_t count,
+ loff_t *f_pos)
+{
+ struct qcom_rfsa *rfsa = filp->private_data;
+
+ if (*f_pos >= rfsa->size)
+ return 0;
+
+ if (*f_pos + count >= rfsa->size)
+ count = rfsa->size - *f_pos;
+
+ if (copy_from_user(rfsa->base + *f_pos, buf, count))
+ return -EFAULT;
+
+ *f_pos += count;
+ return count;
+}
+
+static int qcom_rfsa_release(struct inode *inode, struct file *filp)
+{
+ struct qcom_rfsa *rfsa = filp->private_data;
+
+ put_device(&rfsa->dev);
+
+ return 0;
+}
+
+static const struct file_operations qcom_rfsa_fops = {
+ .owner = THIS_MODULE,
+ .open = qcom_rfsa_open,
+ .read = qcom_rfsa_read,
+ .write = qcom_rfsa_write,
+ .release = qcom_rfsa_release,
+ .llseek = default_llseek,
+};
+
+static void qcom_rfsa_release_device(struct device *dev)
+{
+ struct qcom_rfsa *rfsa = container_of(dev, struct qcom_rfsa, dev);
+
+ kfree(rfsa);
+}
+
+static int qcom_rfsa_probe(struct platform_device *pdev)
+{
+ struct device_node *node = pdev->dev.of_node;
+ struct reserved_mem *rmem;
+ struct qcom_rfsa *rfsa;
+ u32 client_id;
+ int ret;
+
+ rmem = of_get_reserved_mem_by_idx(node, 0);
+ 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");
+ return ret;
+
+ }
+
+ rfsa = kzalloc(sizeof(*rfsa), GFP_KERNEL);
+ if (!rfsa)
+ return -ENOMEM;
+
+ rfsa->addr = rmem->base;
+ rfsa->client_id = client_id;
+ rfsa->size = rmem->size;
+
+ device_initialize(&rfsa->dev);
+ rfsa->dev.parent = &pdev->dev;
+ rfsa->dev.groups = qcom_rfsa_groups;
+
+ cdev_init(&rfsa->cdev, &qcom_rfsa_fops);
+ rfsa->cdev.owner = THIS_MODULE;
+
+ dev_set_name(&rfsa->dev, "qcom_rfsa%d", client_id);
+ rfsa->dev.id = client_id;
+ rfsa->dev.devt = MKDEV(MAJOR(qcom_rfsa_major), client_id);
+
+ ret = cdev_device_add(&rfsa->cdev, &rfsa->dev);
+ if (ret) {
+ dev_err(&pdev->dev, "failed to add cdev: %d\n", ret);
+ put_device(&rfsa->dev);
+ return ret;
+ }
+
+ rfsa->dev.release = qcom_rfsa_release_device;
+
+ rfsa->base = devm_memremap(&rfsa->dev, rfsa->addr, rfsa->size, MEMREMAP_WC);
+ if (IS_ERR(rfsa->base)) {
+ dev_err(&pdev->dev, "failed to remap rfsa region\n");
+
+ device_del(&rfsa->dev);
+ put_device(&rfsa->dev);
+
+ return PTR_ERR(rfsa->base);
+ }
+
+ dev_set_drvdata(&pdev->dev, rfsa);
+
+ return 0;
+}
+
+static int qcom_rfsa_remove(struct platform_device *pdev)
+{
+ struct qcom_rfsa *rfsa = dev_get_drvdata(&pdev->dev);
+
+ cdev_del(&rfsa->cdev);
+ device_del(&rfsa->dev);
+ put_device(&rfsa->dev);
+
+ return 0;
+}
+
+static const struct of_device_id qcom_rfsa_of_match[] = {
+ { .compatible = "qcom,rfsa" },
+ {}
+};
+MODULE_DEVICE_TABLE(of, qcom_rfsa_of_match);
+
+static struct platform_driver qcom_rfsa_driver = {
+ .probe = qcom_rfsa_probe,
+ .remove = qcom_rfsa_remove,
+ .driver = {
+ .name = "qcom_rfsa",
+ .of_match_table = qcom_rfsa_of_match,
+ },
+};
+
+static int qcom_rfsa_init(void)
+{
+ int ret;
+
+ ret = alloc_chrdev_region(&qcom_rfsa_major, 0, QCOM_RFSA_DEV_MAX,
+ "qcom_rfsa");
+ if (ret < 0) {
+ pr_err("qcom_rfsa: failed to allocate char dev region\n");
+ return ret;
+ }
+
+ ret = platform_driver_register(&qcom_rfsa_driver);
+ if (ret < 0) {
+ pr_err("qcom_rfsa: failed to register rfsa driver\n");
+ unregister_chrdev_region(qcom_rfsa_major, QCOM_RFSA_DEV_MAX);
+ }
+
+ return ret;
+}
+module_init(qcom_rfsa_init);
+
+static void qcom_rfsa_exit(void)
+{
+ platform_driver_unregister(&qcom_rfsa_driver);
+ unregister_chrdev_region(qcom_rfsa_major, QCOM_RFSA_DEV_MAX);
+}
+module_exit(qcom_rfsa_exit);
--
2.12.0

2017-04-22 17:35:56

by Bjorn Andersson

[permalink] [raw]
Subject: [RFC 2/3] of: reserved_mem: Accessor for acquiring reserved_mem

In some cases drivers referencing a reserved-memory region might want to
remap the entire region, but when defining the reserved-memory by "size"
the client driver has no means to know the associated base address of
the reserved memory region.

This patch adds an accessor for such drivers to acquire a handle to
their associated reserved-memory for this purpose.

Signed-off-by: Bjorn Andersson <[email protected]>
---

I would have preferred if we could provide a mechanism for drivers to find the
reserved_mem of their own device_node, but without a phandle I have not been
able to figure out a sane way to make the match.

Suggestions are very welcome.

drivers/of/of_reserved_mem.c | 26 ++++++++++++++++++++++++++
include/linux/of_reserved_mem.h | 8 ++++++++
2 files changed, 34 insertions(+)

diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
index d507c3569a88..aa69c9590a5c 100644
--- a/drivers/of/of_reserved_mem.c
+++ b/drivers/of/of_reserved_mem.c
@@ -397,3 +397,29 @@ void of_reserved_mem_device_release(struct device *dev)
rmem->ops->device_release(rmem, dev);
}
EXPORT_SYMBOL_GPL(of_reserved_mem_device_release);
+
+/**
+ * of_get_reserved_mem_by_idx() - acquire reserved_mem from memory-region
+ * @np: node pointer containing the "memory-region"
+ * @idx: index within memory-region
+ *
+ * This function allows drivers to acquire a reference to the reserved_mem
+ * struct which is referenced by their memory-region.
+ *
+ * Returns a reserved_mem reference, or NULL on error.
+ */
+struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np, int idx)
+{
+ struct device_node *target;
+ struct reserved_mem *rmem;
+
+ target = of_parse_phandle(np, "memory-region", idx);
+ if (!target)
+ return NULL;
+
+ rmem = __find_rmem(target);
+ of_node_put(target);
+
+ return rmem;
+}
+EXPORT_SYMBOL_GPL(of_get_reserved_mem_by_idx);
diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
index f8e1992d6423..a9abbe7dd3de 100644
--- a/include/linux/of_reserved_mem.h
+++ b/include/linux/of_reserved_mem.h
@@ -34,6 +34,8 @@ int of_reserved_mem_device_init_by_idx(struct device *dev,
struct device_node *np, int idx);
void of_reserved_mem_device_release(struct device *dev);

+struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np, int idx);
+
int early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
phys_addr_t align,
phys_addr_t start,
@@ -52,6 +54,12 @@ static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
}
static inline void of_reserved_mem_device_release(struct device *pdev) { }

+static inline struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np,
+ int idx);
+{
+ return NULL;
+}
+
static inline void fdt_init_reserved_mem(void) { }
static inline void fdt_reserved_mem_save_node(unsigned long node,
const char *uname, phys_addr_t base, phys_addr_t size) { }
--
2.12.0

2017-04-24 17:27:48

by Frank Rowand

[permalink] [raw]
Subject: Re: [RFC 2/3] of: reserved_mem: Accessor for acquiring reserved_mem

On 04/22/17 10:35, Bjorn Andersson wrote:
> In some cases drivers referencing a reserved-memory region might want to
> remap the entire region, but when defining the reserved-memory by "size"
> the client driver has no means to know the associated base address of
> the reserved memory region.
>
> This patch adds an accessor for such drivers to acquire a handle to
> their associated reserved-memory for this purpose.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>
> I would have preferred if we could provide a mechanism for drivers to find the
> reserved_mem of their own device_node, but without a phandle I have not been
> able to figure out a sane way to make the match.
>
> Suggestions are very welcome.
>
> drivers/of/of_reserved_mem.c | 26 ++++++++++++++++++++++++++
> include/linux/of_reserved_mem.h | 8 ++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index d507c3569a88..aa69c9590a5c 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -397,3 +397,29 @@ void of_reserved_mem_device_release(struct device *dev)
> rmem->ops->device_release(rmem, dev);
> }
> EXPORT_SYMBOL_GPL(of_reserved_mem_device_release);
> +
> +/**
> + * of_get_reserved_mem_by_idx() - acquire reserved_mem from memory-region
> + * @np: node pointer containing the "memory-region"
> + * @idx: index within memory-region
> + *
> + * This function allows drivers to acquire a reference to the reserved_mem
> + * struct which is referenced by their memory-region.
> + *
> + * Returns a reserved_mem reference, or NULL on error.
> + */
> +struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np, int idx)
> +{
> + struct device_node *target;
> + struct reserved_mem *rmem;
> +
> + target = of_parse_phandle(np, "memory-region", idx);
> + if (!target)
> + return NULL;
> +
> + rmem = __find_rmem(target);
> + of_node_put(target);
> +
> + return rmem;
> +}
> +EXPORT_SYMBOL_GPL(of_get_reserved_mem_by_idx);
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index f8e1992d6423..a9abbe7dd3de 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -34,6 +34,8 @@ int of_reserved_mem_device_init_by_idx(struct device *dev,
> struct device_node *np, int idx);
> void of_reserved_mem_device_release(struct device *dev);
>
> +struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np, int idx);
> +
> int early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> phys_addr_t align,
> phys_addr_t start,
> @@ -52,6 +54,12 @@ static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
> }
> static inline void of_reserved_mem_device_release(struct device *pdev) { }
>
> +static inline struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np,
> + int idx);
> +{
> + return NULL;
> +}
> +
> static inline void fdt_init_reserved_mem(void) { }
> static inline void fdt_reserved_mem_save_node(unsigned long node,
> const char *uname, phys_addr_t base, phys_addr_t size) { }
>

Reviewed-by: Frank Rowand <[email protected]>

2017-04-25 13:21:49

by Sricharan R

[permalink] [raw]
Subject: Re: [RFC 3/3] soc: qcom: rfsa driver

Hi Bjorn,

On 4/22/2017 11:05 PM, Bjorn Andersson wrote:
> The rfsa driver is used for allocating and exposing regions of shared
> memory with remote processors for the purpose of exchanging sector-data
> between the remote filesystem service and its clients.
>
> It provides accessors for the properties needed by the user space remote
> filesystem implementation through sysfs and a character device that can be used
> to read and write the requested chunks of data.
>

couple of minor nits.

> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
> drivers/soc/qcom/Kconfig | 8 ++
> drivers/soc/qcom/Makefile | 1 +
> drivers/soc/qcom/rfsa.c | 261 ++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 270 insertions(+)
> create mode 100644 drivers/soc/qcom/rfsa.c
>
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index 9fca977ef18d..788a63cd430e 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -24,6 +24,14 @@ config QCOM_PM
> modes. It interface with various system drivers to put the cores in
> low power modes.
>
> +config QCOM_RFSA
> + tristate "Qualcomm Remote Filesystem Access driver"

depends on ARCH_QCOM ?

> + help
> + The Qualcomm remote filesystem access driver is used for allocating
> + and exposing regions of shared memory with remote processors for the
> + purpose of exchanging sector-data between the remote filesystem
> + service and its clients.
> +
> config QCOM_SMEM
> tristate "Qualcomm Shared Memory Manager (SMEM)"
> depends on ARCH_QCOM
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 414f0de274fa..d1bbc791ddc0 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -1,6 +1,7 @@
> obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
> obj-$(CONFIG_QCOM_PM) += spm.o
> +obj-$(CONFIG_QCOM_RFSA) += rfsa.o
> obj-$(CONFIG_QCOM_SMD_RPM) += smd-rpm.o
> obj-$(CONFIG_QCOM_SMEM) += smem.o
> obj-$(CONFIG_QCOM_SMEM_STATE) += smem_state.o
> diff --git a/drivers/soc/qcom/rfsa.c b/drivers/soc/qcom/rfsa.c
> new file mode 100644
> index 000000000000..1b79976dad9d
> --- /dev/null
> +++ b/drivers/soc/qcom/rfsa.c
> @@ -0,0 +1,261 @@
> +/*
> + * Copyright (c) 2017 Linaro Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 and
> + * only version 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/cdev.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/of_fdt.h>
> +#include <linux/dma-mapping.h>
> +#include <linux/slab.h>
> +#include <linux/uaccess.h>
> +#include <linux/io.h>
> +
> +#define QCOM_RFSA_DEV_MAX (MINORMASK + 1)
> +
> +static dev_t qcom_rfsa_major;
> +
> +struct qcom_rfsa {
> + struct device dev;
> + struct cdev cdev;
> +
> + void *base;
> + phys_addr_t addr;
> + phys_addr_t size;
> +
> + unsigned int client_id;
> +};
> +
> +static ssize_t qcom_rfsa_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf);
> +
> +static DEVICE_ATTR(phys_addr, 0400, qcom_rfsa_show, NULL);
> +static DEVICE_ATTR(size, 0400, qcom_rfsa_show, NULL);
> +static DEVICE_ATTR(client_id, 0400, qcom_rfsa_show, NULL);
> +
> +static ssize_t qcom_rfsa_show(struct device *dev,
> + struct device_attribute *attr,
> + char *buf)
> +{
> + struct qcom_rfsa *rfsa = container_of(dev, struct qcom_rfsa, dev);
> +
> + if (attr == &dev_attr_phys_addr)
> + return sprintf(buf, "%pa\n", &rfsa->addr);
> + if (attr == &dev_attr_size)
> + return sprintf(buf, "%pa\n", &rfsa->size);
> + if (attr == &dev_attr_client_id)
> + return sprintf(buf, "%d\n", rfsa->client_id);
> +
> + return -EINVAL;
> +}
> +
> +static struct attribute *qcom_rfsa_attrs[] = {
> + &dev_attr_phys_addr.attr,
> + &dev_attr_size.attr,
> + &dev_attr_client_id.attr,
> + NULL
> +};
> +ATTRIBUTE_GROUPS(qcom_rfsa);
> +
> +static int qcom_rfsa_open(struct inode *inode, struct file *filp)
> +{
> + struct qcom_rfsa *rfsa = container_of(inode->i_cdev, struct qcom_rfsa, cdev);
> +
> + get_device(&rfsa->dev);
> + filp->private_data = rfsa;
> +
> + return 0;
> +}
> +static ssize_t qcom_rfsa_read(struct file *filp,
> + char __user *buf, size_t count, loff_t *f_pos)
> +{
> + struct qcom_rfsa *rfsa = filp->private_data;
> +
> + if (*f_pos >= rfsa->size)
> + return 0;
> +
> + if (*f_pos + count >= rfsa->size)
> + count = rfsa->size - *f_pos;
> +
> + if (copy_to_user(buf, rfsa->base + *f_pos, count))
> + return -EFAULT;
> +
> + *f_pos += count;
> + return count;
> +}
> +
> +static ssize_t qcom_rfsa_write(struct file *filp,
> + const char __user *buf, size_t count,
> + loff_t *f_pos)
> +{
> + struct qcom_rfsa *rfsa = filp->private_data;
> +
> + if (*f_pos >= rfsa->size)
> + return 0;
> +
> + if (*f_pos + count >= rfsa->size)
> + count = rfsa->size - *f_pos;
> +
> + if (copy_from_user(rfsa->base + *f_pos, buf, count))
> + return -EFAULT;
> +
> + *f_pos += count;
> + return count;
> +}
> +
> +static int qcom_rfsa_release(struct inode *inode, struct file *filp)
> +{
> + struct qcom_rfsa *rfsa = filp->private_data;
> +
> + put_device(&rfsa->dev);
> +
> + return 0;
> +}
> +
> +static const struct file_operations qcom_rfsa_fops = {
> + .owner = THIS_MODULE,
> + .open = qcom_rfsa_open,
> + .read = qcom_rfsa_read,
> + .write = qcom_rfsa_write,
> + .release = qcom_rfsa_release,
> + .llseek = default_llseek,
> +};
> +
> +static void qcom_rfsa_release_device(struct device *dev)
> +{
> + struct qcom_rfsa *rfsa = container_of(dev, struct qcom_rfsa, dev);
> +
> + kfree(rfsa);
> +}
> +
> +static int qcom_rfsa_probe(struct platform_device *pdev)
> +{
> + struct device_node *node = pdev->dev.of_node;
> + struct reserved_mem *rmem;
> + struct qcom_rfsa *rfsa;
> + u32 client_id;
> + int ret;
> +
> + rmem = of_get_reserved_mem_by_idx(node, 0);
> + 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");
> + return ret;
> +
> + }
> +
> + rfsa = kzalloc(sizeof(*rfsa), GFP_KERNEL);
> + if (!rfsa)
> + return -ENOMEM;
> +
> + rfsa->addr = rmem->base;
> + rfsa->client_id = client_id;
> + rfsa->size = rmem->size;
> +
> + device_initialize(&rfsa->dev);
> + rfsa->dev.parent = &pdev->dev;
> + rfsa->dev.groups = qcom_rfsa_groups;
> +
> + cdev_init(&rfsa->cdev, &qcom_rfsa_fops);
> + rfsa->cdev.owner = THIS_MODULE;
> +
> + dev_set_name(&rfsa->dev, "qcom_rfsa%d", client_id);
> + rfsa->dev.id = client_id;
> + rfsa->dev.devt = MKDEV(MAJOR(qcom_rfsa_major), client_id);
> +
> + ret = cdev_device_add(&rfsa->cdev, &rfsa->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "failed to add cdev: %d\n", ret);
> + put_device(&rfsa->dev);
> + return ret;
> + }
> +
> + rfsa->dev.release = qcom_rfsa_release_device;
> +
> + rfsa->base = devm_memremap(&rfsa->dev, rfsa->addr, rfsa->size, MEMREMAP_WC);
> + if (IS_ERR(rfsa->base)) {
> + dev_err(&pdev->dev, "failed to remap rfsa region\n");
> +
> + device_del(&rfsa->dev);
> + put_device(&rfsa->dev);
> +
> + return PTR_ERR(rfsa->base);
> + }
> +
> + dev_set_drvdata(&pdev->dev, rfsa);
> +
> + return 0;
> +}
> +
> +static int qcom_rfsa_remove(struct platform_device *pdev)
> +{
> + struct qcom_rfsa *rfsa = dev_get_drvdata(&pdev->dev);
> +
> + cdev_del(&rfsa->cdev);
> + device_del(&rfsa->dev);

cdev_device_del instead of the above two ?

Regards,
Sricharan


--
"QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation

2017-04-28 17:42:49

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [RFC 1/3] dt-binding: soc: qcom: Add binding for RFSA

On Sat, Apr 22, 2017 at 10:35:17AM -0700, Bjorn Andersson wrote:
> This adds the binding for describing shared memory buffers for
> implementing the remote filesystem protocol.
>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
>
> My initial attempt was to mimic the ramoops of just adding the compatible to
> the reserved-memory node, but I have not been able to figure out a sane way of
> getting hold of the base address in the case that the memory region is
> described my a "size" only (done on some platforms).

I prefer the ramoops way.

> The problem is that we create the reserved_mem objects (and remove the
> memblocks) while we're still operating on the flattened representation, so
> without a phandle it doesn't seem like we have anything to perform the
> comparison with later on.

I'm not sure I follow.

>
> .../devicetree/bindings/soc/qcom/qcom,rfsa.txt | 43 ++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt
>
> diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt
> new file mode 100644
> index 000000000000..b4de0de74e46
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt
> @@ -0,0 +1,43 @@
> +Qualcomm Remote File System Access binding
> +
> +This binding describes the Qualcomm RFSA, which serves the purpose of managing
> +the shared memory region used for remote processors to access block device data
> +using the Remote Filesystem protocol.
> +
> +- compatible:
> + Usage: required
> + Value type: <stringlist>
> + Definition: must be:
> + "qcom,rfsa"

No versioning?

> +
> +- memory-region:
> + Usage: required
> + Value type: <prop-encoded-array>
> + Definition: handle to memory reservation the associated rfsa region.
> +
> +- qcom,client-id:
> + Usage: required
> + Value type: <u32>
> + Definition: identifier of the client to use this region for buffers.

What determines these numbers?

> +
> += EXAMPLE
> +The following example shows the RFSA setup for APQ8016, with the RFSA region
> +for the Hexagon DSP (id #1) located at 0x86700000.
> +
> + reserved-memory {
> + #address-cells = <2>;
> + #size-cells = <2>;
> + ranges;
> +
> + rmtfs: rmtfs@86700000 {

I think you should have a compatible here even with the 2 node approach.

> + reg = <0x0 0x86700000 0x0 0xe0000>;
> + no-map;
> + };
> + };
> +
> + hexagon-rfsa {
> + compatible = "qcom,rfsa";
> + memory-region = <&rmtfs>;
> +
> + qcom,client-id = <1>;
> + };
> --
> 2.12.0
>

2017-04-29 20:03:05

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [RFC 1/3] dt-binding: soc: qcom: Add binding for RFSA

On Fri 28 Apr 10:42 PDT 2017, Rob Herring wrote:

> On Sat, Apr 22, 2017 at 10:35:17AM -0700, Bjorn Andersson wrote:
> > This adds the binding for describing shared memory buffers for
> > implementing the remote filesystem protocol.
> >
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > ---
> >
> > My initial attempt was to mimic the ramoops of just adding the compatible to
> > the reserved-memory node, but I have not been able to figure out a sane way of
> > getting hold of the base address in the case that the memory region is
> > described my a "size" only (done on some platforms).
>
> I prefer the ramoops way.
>

Me too :)

> > The problem is that we create the reserved_mem objects (and remove the
> > memblocks) while we're still operating on the flattened representation, so
> > without a phandle it doesn't seem like we have anything to perform the
> > comparison with later on.
>
> I'm not sure I follow.
>

In my first attempt I extended of_platform_default_populate_init() to
also probe my platform_driver and like ramoops acquired the values of
reg and memremap() these. This works fine.


But for some platforms the memory-region doesn't need a fixed location,
it's just required to be a consecutive chunk of physical ram. So I
replace the "reg = <>" with a "size = <>", this causes
__reserved_mem_alloc_size() to carve out a chunk of memory somewhere
and update the associated reserved_mem entry. The reserved_mem will be
populated with the phandle from the [flattened] of_node.

Later my platform_device is instantiated and I need to figure out the
base address that was picked earlier - so that I know which region to
memremap().

But as my "rfsa-node" stands on its own it will not have any "phandle",
so the reserved_mem->phandle will remain 0. Further more I only have the
unflattened of_node - so I can't just compare the address with the
flattened version previously used.

So the problem at hand is how to match my pdev->dev.of_node to an entry
in the reserved_mem array (in of_reserved_mem.c).

> >
> > .../devicetree/bindings/soc/qcom/qcom,rfsa.txt | 43 ++++++++++++++++++++++
> > 1 file changed, 43 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt
> >
> > diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt b/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt
> > new file mode 100644
> > index 000000000000..b4de0de74e46
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/soc/qcom/qcom,rfsa.txt
> > @@ -0,0 +1,43 @@
> > +Qualcomm Remote File System Access binding
> > +
> > +This binding describes the Qualcomm RFSA, which serves the purpose of managing
> > +the shared memory region used for remote processors to access block device data
> > +using the Remote Filesystem protocol.
> > +
> > +- compatible:
> > + Usage: required
> > + Value type: <stringlist>
> > + Definition: must be:
> > + "qcom,rfsa"
>
> No versioning?
>

The binding is used to describe a chunk of memory and an associated
client-id of this memory, so I'm not sure if we need a version.

> > +
> > +- memory-region:
> > + Usage: required
> > + Value type: <prop-encoded-array>
> > + Definition: handle to memory reservation the associated rfsa region.
> > +
> > +- qcom,client-id:
> > + Usage: required
> > + Value type: <u32>
> > + Definition: identifier of the client to use this region for buffers.
>
> What determines these numbers?
>

The bigger picture of this is that the remote processors (e.g. modem)
needs access to block storage, so it sends request to a service on the
system with storage access (i.e the application processor) which will
read and write from the file system, storing blocks of data in the
rfsa-memory.

So the client-id is the hard coded identifier of each such remote system
requesting IO - each one with its own memory carveout.


In later platforms we need to configure the SMMU for each remote in
order to give them access to these cerveouts, so I don't see that its
possible to mash them together in one chunk and handle the client-id
thing only in the application.

> > +
> > += EXAMPLE
> > +The following example shows the RFSA setup for APQ8016, with the RFSA region
> > +for the Hexagon DSP (id #1) located at 0x86700000.
> > +
> > + reserved-memory {
> > + #address-cells = <2>;
> > + #size-cells = <2>;
> > + ranges;
> > +
> > + rmtfs: rmtfs@86700000 {
>
> I think you should have a compatible here even with the 2 node approach.
>

I'm hoping we can figure out a way to fix the Linux implementation so
that we can describe it fully in one node.

> > + reg = <0x0 0x86700000 0x0 0xe0000>;
> > + no-map;
> > + };
> > + };
> > +
> > + hexagon-rfsa {
> > + compatible = "qcom,rfsa";
> > + memory-region = <&rmtfs>;
> > +
> > + qcom,client-id = <1>;
> > + };
> > --
> > 2.12.0
> >

Regards,
Bjorn