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

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

[Bjorn Andersson]

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

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---

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

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

[Bjorn Andersson]

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 <bjorn.andersson@linaro.org>
---

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

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

[Frank Rowand]

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 <bjorn.andersson@linaro.org>
> ---
> 
> 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 <frank.rowand@sony.com>

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

[Andy Gross]

On Sat, Apr 22, 2017 at 10:35:18AM -0700, 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 <bjorn.andersson@linaro.org>

Reviewed-by: Andy Gross <andy.gross@linaro.org>

[RFC 3/3] soc: qcom: rfsa driver

[Bjorn Andersson]

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 <bjorn.andersson@linaro.org>
---
 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

Re: [RFC 3/3] soc: qcom: rfsa driver

[Sricharan R]

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 <bjorn.andersson@linaro.org>
> ---
>  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

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

[Rob Herring]

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 <bjorn.andersson@linaro.org>
> ---
> 
> 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
> 

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

[Bjorn Andersson]

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 <bjorn.andersson@linaro.org>
> > ---
> > 
> > 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