All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Add character device interface to remoteproc
@ 2020-03-20 23:36 Rishabh Bhatnagar
  2020-03-20 23:36 ` [PATCH 1/2] remoteproc: Add userspace char device driver Rishabh Bhatnagar
  2020-03-20 23:36 ` [PATCH 2/2] remoteproc: core: Register the character device interface Rishabh Bhatnagar
  0 siblings, 2 replies; 7+ messages in thread
From: Rishabh Bhatnagar @ 2020-03-20 23:36 UTC (permalink / raw)
  To: linux-remoteproc-owner, linux-kernel, bjorn.andersson, mathieu.poirier
  Cc: psodagud, tsoni, sidgup, Rishabh Bhatnagar

This patch series adds a character device interface to remoteproc
framework. This interface can be used by userspace clients in order
to boot and shutdown the remote processor.
Currently there is only a sysfs interface which the userspace
clients can use. If a usersapce application crashes after booting
the remote processor through the sysfs interface the remote processor
does not get any indication about the crash. It might still assume
that the  application is running.
For example modem uses remotefs service to data from disk/flash memory.
If the remotefs service crashes, modem still keeps on requesting data
which might lead to crash on modem. Even if the service is restarted the
file handles modem requested previously would become stale.
Adding a character device interface makes the remote processor tightly
coupled with the user space application. A crash of the application
leads to a close on the file descriptors therefore shutting down the
remoteproc.

Rishabh Bhatnagar (2):
  remoteproc: Add userspace char device driver
  remoteproc: core: Register the character device interface

 drivers/remoteproc/Makefile               |   1 +
 drivers/remoteproc/remoteproc_core.c      |   8 +-
 drivers/remoteproc/remoteproc_internal.h  |   3 +
 drivers/remoteproc/remoteproc_userspace.c | 126 ++++++++++++++++++++++++++++++
 include/linux/remoteproc.h                |   2 +
 5 files changed, 139 insertions(+), 1 deletion(-)
 create mode 100644 drivers/remoteproc/remoteproc_userspace.c

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH 1/2] remoteproc: Add userspace char device driver
  2020-03-20 23:36 [PATCH 0/2] Add character device interface to remoteproc Rishabh Bhatnagar
@ 2020-03-20 23:36 ` Rishabh Bhatnagar
  2020-03-25  4:10   ` Bjorn Andersson
  2020-03-20 23:36 ` [PATCH 2/2] remoteproc: core: Register the character device interface Rishabh Bhatnagar
  1 sibling, 1 reply; 7+ messages in thread
From: Rishabh Bhatnagar @ 2020-03-20 23:36 UTC (permalink / raw)
  To: linux-remoteproc-owner, linux-kernel, bjorn.andersson, mathieu.poirier
  Cc: psodagud, tsoni, sidgup, Rishabh Bhatnagar

Add the driver for creating the character device interface for
userspace applications. The character device interface can be used
in order to boot up and shutdown the remote processor.
This might be helpful for remote processors that are booted by
userspace applications and need to shutdown when the application
crahes/shutsdown.

Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
---
 drivers/remoteproc/Makefile               |   1 +
 drivers/remoteproc/remoteproc_internal.h  |   3 +
 drivers/remoteproc/remoteproc_userspace.c | 126 ++++++++++++++++++++++++++++++
 include/linux/remoteproc.h                |   2 +
 4 files changed, 132 insertions(+)
 create mode 100644 drivers/remoteproc/remoteproc_userspace.c

diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
index e30a1b1..facb3fa 100644
--- a/drivers/remoteproc/Makefile
+++ b/drivers/remoteproc/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_REMOTEPROC)		+= remoteproc.o
 remoteproc-y				:= remoteproc_core.o
 remoteproc-y				+= remoteproc_debugfs.o
 remoteproc-y				+= remoteproc_sysfs.o
+remoteproc-y				+= remoteproc_userspace.o
 remoteproc-y				+= remoteproc_virtio.o
 remoteproc-y				+= remoteproc_elf_loader.o
 obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
index 493ef92..bafaa12 100644
--- a/drivers/remoteproc/remoteproc_internal.h
+++ b/drivers/remoteproc/remoteproc_internal.h
@@ -63,6 +63,9 @@ struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
 struct rproc_mem_entry *
 rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...);
 
+/* from remoteproc_userspace.c */
+extern int rproc_char_device_add(struct rproc *rproc);
+
 static inline
 int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
 {
diff --git a/drivers/remoteproc/remoteproc_userspace.c b/drivers/remoteproc/remoteproc_userspace.c
new file mode 100644
index 0000000..e3017e7
--- /dev/null
+++ b/drivers/remoteproc/remoteproc_userspace.c
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Character device interface driver for Remoteproc framework.
+ *
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ */
+
+#include <linux/module.h>
+#include <linux/fs.h>
+#include <linux/cdev.h>
+#include <linux/mutex.h>
+#include <linux/remoteproc.h>
+
+#include "remoteproc_internal.h"
+
+static LIST_HEAD(rproc_chrdev_list);
+
+struct rproc_char_dev {
+	struct list_head node;
+	dev_t dev_no;
+	struct rproc *rproc;
+};
+
+static DEFINE_MUTEX(rproc_chrdev_lock);
+
+static struct rproc *rproc_get_by_dev_no(int minor)
+{
+	struct rproc_char_dev *r;
+
+	mutex_lock(&rproc_chrdev_lock);
+	list_for_each_entry(r, &rproc_chrdev_list, node) {
+		if (MINOR(r->dev_no) == minor)
+			break;
+	}
+	mutex_unlock(&rproc_chrdev_lock);
+
+	return r->rproc;
+}
+
+static int rproc_open(struct inode *inode, struct file *file)
+{
+	struct rproc *rproc;
+	int retval;
+
+	rproc = rproc_get_by_dev_no(iminor(inode));
+	if (!rproc)
+		return -EINVAL;
+
+	if (!try_module_get(rproc->dev.parent->driver->owner)) {
+		dev_err(&rproc->dev, "can't get owner\n");
+		return -EINVAL;
+	}
+
+	get_device(&rproc->dev);
+	retval = rproc_boot(rproc);
+
+	return retval;
+}
+
+static int rproc_close(struct inode *inode, struct file *file)
+{
+	struct rproc *rproc;
+
+	rproc = rproc_get_by_dev_no(iminor(inode));
+	if (!rproc)
+		return -EINVAL;
+
+	rproc_shutdown(rproc);
+	rproc_put(rproc);
+
+	return 0;
+}
+
+static const struct file_operations rproc_fops = {
+	.open = rproc_open,
+	.release = rproc_close,
+};
+
+int rproc_char_device_add(struct rproc *rproc)
+{
+	int ret = 0;
+	static int major, minor;
+	dev_t dev_no;
+	struct rproc_char_dev *chrdev;
+
+	mutex_lock(&rproc_chrdev_lock);
+	if (!major) {
+		ret = alloc_chrdev_region(&dev_no, 0, 4, "subsys");
+		if (ret < 0) {
+			pr_err("Failed to alloc subsys_dev region, err %d\n",
+									ret);
+			goto fail;
+		}
+		major = MAJOR(dev_no);
+		minor = MINOR(dev_no);
+	} else
+		dev_no = MKDEV(major, minor);
+
+	cdev_init(&rproc->char_dev, &rproc_fops);
+	rproc->char_dev.owner = THIS_MODULE;
+	ret = cdev_add(&rproc->char_dev, dev_no, 1);
+	if (ret < 0)
+		goto fail_unregister_cdev_region;
+
+	rproc->dev.devt = dev_no;
+
+	chrdev = kzalloc(sizeof(struct rproc_char_dev), GFP_KERNEL);
+	if (!chrdev) {
+		ret = -ENOMEM;
+		goto fail_unregister_cdev_region;
+	}
+
+	chrdev->rproc = rproc;
+	chrdev->dev_no = dev_no;
+	list_add(&chrdev->node, &rproc_chrdev_list);
+	++minor;
+	mutex_unlock(&rproc_chrdev_lock);
+
+	return 0;
+
+fail_unregister_cdev_region:
+	unregister_chrdev_region(dev_no, 1);
+fail:
+	mutex_unlock(&rproc_chrdev_lock);
+	return ret;
+}
diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
index 16ad666..c4ca796 100644
--- a/include/linux/remoteproc.h
+++ b/include/linux/remoteproc.h
@@ -37,6 +37,7 @@
 
 #include <linux/types.h>
 #include <linux/mutex.h>
+#include <linux/cdev.h>
 #include <linux/virtio.h>
 #include <linux/completion.h>
 #include <linux/idr.h>
@@ -514,6 +515,7 @@ struct rproc {
 	bool auto_boot;
 	struct list_head dump_segments;
 	int nb_vdev;
+	struct cdev char_dev;
 };
 
 /**
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* [PATCH 2/2] remoteproc: core: Register the character device interface
  2020-03-20 23:36 [PATCH 0/2] Add character device interface to remoteproc Rishabh Bhatnagar
  2020-03-20 23:36 ` [PATCH 1/2] remoteproc: Add userspace char device driver Rishabh Bhatnagar
@ 2020-03-20 23:36 ` Rishabh Bhatnagar
  2020-03-25  4:22   ` Bjorn Andersson
  1 sibling, 1 reply; 7+ messages in thread
From: Rishabh Bhatnagar @ 2020-03-20 23:36 UTC (permalink / raw)
  To: linux-remoteproc-owner, linux-kernel, bjorn.andersson, mathieu.poirier
  Cc: psodagud, tsoni, sidgup, Rishabh Bhatnagar

Add the character device during rproc_add. This would create
a character device node at /dev/subsys_<rproc_name>. Userspace
applications can interact with the remote processor using this
interface rather than using sysfs node. To distinguish between
different remote processor nodes the device name has been changed
to include the rproc name appended to "subsys_" string.

Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
---
 drivers/remoteproc/remoteproc_core.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 097f33e..48a3932 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1907,6 +1907,12 @@ int rproc_add(struct rproc *rproc)
 	struct device *dev = &rproc->dev;
 	int ret;
 
+	ret = rproc_char_device_add(rproc);
+	if (ret) {
+		pr_err("error while adding character device\n");
+		return ret;
+	}
+
 	ret = device_add(dev);
 	if (ret < 0)
 		return ret;
@@ -2044,7 +2050,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 		return NULL;
 	}
 
-	dev_set_name(&rproc->dev, "remoteproc%d", rproc->index);
+	dev_set_name(&rproc->dev, "subsys_%s", rproc->name);
 
 	atomic_set(&rproc->power, 0);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

* Re: [PATCH 1/2] remoteproc: Add userspace char device driver
  2020-03-20 23:36 ` [PATCH 1/2] remoteproc: Add userspace char device driver Rishabh Bhatnagar
@ 2020-03-25  4:10   ` Bjorn Andersson
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2020-03-25  4:10 UTC (permalink / raw)
  To: Rishabh Bhatnagar
  Cc: linux-remoteproc-owner, linux-kernel, mathieu.poirier, psodagud,
	tsoni, sidgup

On Fri 20 Mar 16:36 PDT 2020, Rishabh Bhatnagar wrote:

> Add the driver for creating the character device interface for
> userspace applications. The character device interface can be used
> in order to boot up and shutdown the remote processor.
> This might be helpful for remote processors that are booted by
> userspace applications and need to shutdown when the application
> crahes/shutsdown.
> 
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>

Please use linux-remoteproc@ instead of linux-remoteproc-owner@, to
ensure you're reaching the whole community.

> ---
>  drivers/remoteproc/Makefile               |   1 +
>  drivers/remoteproc/remoteproc_internal.h  |   3 +
>  drivers/remoteproc/remoteproc_userspace.c | 126 ++++++++++++++++++++++++++++++
>  include/linux/remoteproc.h                |   2 +
>  4 files changed, 132 insertions(+)
>  create mode 100644 drivers/remoteproc/remoteproc_userspace.c
> 
> diff --git a/drivers/remoteproc/Makefile b/drivers/remoteproc/Makefile
> index e30a1b1..facb3fa 100644
> --- a/drivers/remoteproc/Makefile
> +++ b/drivers/remoteproc/Makefile
> @@ -7,6 +7,7 @@ obj-$(CONFIG_REMOTEPROC)		+= remoteproc.o
>  remoteproc-y				:= remoteproc_core.o
>  remoteproc-y				+= remoteproc_debugfs.o
>  remoteproc-y				+= remoteproc_sysfs.o
> +remoteproc-y				+= remoteproc_userspace.o
>  remoteproc-y				+= remoteproc_virtio.o
>  remoteproc-y				+= remoteproc_elf_loader.o
>  obj-$(CONFIG_IMX_REMOTEPROC)		+= imx_rproc.o
> diff --git a/drivers/remoteproc/remoteproc_internal.h b/drivers/remoteproc/remoteproc_internal.h
> index 493ef92..bafaa12 100644
> --- a/drivers/remoteproc/remoteproc_internal.h
> +++ b/drivers/remoteproc/remoteproc_internal.h
> @@ -63,6 +63,9 @@ struct resource_table *rproc_elf_find_loaded_rsc_table(struct rproc *rproc,
>  struct rproc_mem_entry *
>  rproc_find_carveout_by_name(struct rproc *rproc, const char *name, ...);
>  
> +/* from remoteproc_userspace.c */
> +extern int rproc_char_device_add(struct rproc *rproc);

Please omit "external" from this.

> +
>  static inline
>  int rproc_fw_sanity_check(struct rproc *rproc, const struct firmware *fw)
>  {
> diff --git a/drivers/remoteproc/remoteproc_userspace.c b/drivers/remoteproc/remoteproc_userspace.c
> new file mode 100644
> index 0000000..e3017e7
> --- /dev/null
> +++ b/drivers/remoteproc/remoteproc_userspace.c
> @@ -0,0 +1,126 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Character device interface driver for Remoteproc framework.
> + *
> + * Copyright (c) 2020, The Linux Foundation. All rights reserved.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/fs.h>
> +#include <linux/cdev.h>
> +#include <linux/mutex.h>
> +#include <linux/remoteproc.h>
> +
> +#include "remoteproc_internal.h"
> +
> +static LIST_HEAD(rproc_chrdev_list);
> +
> +struct rproc_char_dev {
> +	struct list_head node;
> +	dev_t dev_no;
> +	struct rproc *rproc;
> +};

Per below suggestions you don't need this struct (or list).

> +
> +static DEFINE_MUTEX(rproc_chrdev_lock);
> +
> +static struct rproc *rproc_get_by_dev_no(int minor)
> +{
> +	struct rproc_char_dev *r;
> +
> +	mutex_lock(&rproc_chrdev_lock);
> +	list_for_each_entry(r, &rproc_chrdev_list, node) {
> +		if (MINOR(r->dev_no) == minor)
> +			break;
> +	}
> +	mutex_unlock(&rproc_chrdev_lock);
> +
> +	return r->rproc;
> +}
> +
> +static int rproc_open(struct inode *inode, struct file *file)
> +{
> +	struct rproc *rproc;
> +	int retval;
> +

rproc can be found with:
  container_of(inode->i_cdev, struct rproc, char_dev);

so you don't need rproc_get_by_dev_no() and hence not rproc_chrdev_list.

> +	rproc = rproc_get_by_dev_no(iminor(inode));
> +	if (!rproc)
> +		return -EINVAL;
> +
> +	if (!try_module_get(rproc->dev.parent->driver->owner)) {
> +		dev_err(&rproc->dev, "can't get owner\n");
> +		return -EINVAL;
> +	}
> +
> +	get_device(&rproc->dev);
> +	retval = rproc_boot(rproc);

return rproc_boot(); and drop "retval".

> +
> +	return retval;
> +}
> +
> +static int rproc_close(struct inode *inode, struct file *file)

s/close/release/

> +{
> +	struct rproc *rproc;
> +
> +	rproc = rproc_get_by_dev_no(iminor(inode));
> +	if (!rproc)
> +		return -EINVAL;
> +
> +	rproc_shutdown(rproc);
> +	rproc_put(rproc);
> +
> +	return 0;
> +}
> +
> +static const struct file_operations rproc_fops = {
> +	.open = rproc_open,
> +	.release = rproc_close,
> +};
> +
> +int rproc_char_device_add(struct rproc *rproc)
> +{
> +	int ret = 0;

No need to initialize ret, its first use is an assignment.

> +	static int major, minor;

Move these to the top of the file, and use an ida for allocating the
minor.

> +	dev_t dev_no;
> +	struct rproc_char_dev *chrdev;
> +
> +	mutex_lock(&rproc_chrdev_lock);
> +	if (!major) {
> +		ret = alloc_chrdev_region(&dev_no, 0, 4, "subsys");

Please do this during remoteproc_init()

Regards,
Bjorn

> +		if (ret < 0) {
> +			pr_err("Failed to alloc subsys_dev region, err %d\n",
> +									ret);
> +			goto fail;
> +		}
> +		major = MAJOR(dev_no);
> +		minor = MINOR(dev_no);
> +	} else
> +		dev_no = MKDEV(major, minor);
> +
> +	cdev_init(&rproc->char_dev, &rproc_fops);
> +	rproc->char_dev.owner = THIS_MODULE;
> +	ret = cdev_add(&rproc->char_dev, dev_no, 1);
> +	if (ret < 0)
> +		goto fail_unregister_cdev_region;
> +
> +	rproc->dev.devt = dev_no;
> +
> +	chrdev = kzalloc(sizeof(struct rproc_char_dev), GFP_KERNEL);
> +	if (!chrdev) {
> +		ret = -ENOMEM;
> +		goto fail_unregister_cdev_region;
> +	}
> +
> +	chrdev->rproc = rproc;
> +	chrdev->dev_no = dev_no;
> +	list_add(&chrdev->node, &rproc_chrdev_list);
> +	++minor;
> +	mutex_unlock(&rproc_chrdev_lock);
> +
> +	return 0;
> +
> +fail_unregister_cdev_region:
> +	unregister_chrdev_region(dev_no, 1);
> +fail:
> +	mutex_unlock(&rproc_chrdev_lock);
> +	return ret;
> +}
> diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h
> index 16ad666..c4ca796 100644
> --- a/include/linux/remoteproc.h
> +++ b/include/linux/remoteproc.h
> @@ -37,6 +37,7 @@
>  
>  #include <linux/types.h>
>  #include <linux/mutex.h>
> +#include <linux/cdev.h>
>  #include <linux/virtio.h>
>  #include <linux/completion.h>
>  #include <linux/idr.h>
> @@ -514,6 +515,7 @@ struct rproc {
>  	bool auto_boot;
>  	struct list_head dump_segments;
>  	int nb_vdev;
> +	struct cdev char_dev;
>  };
>  
>  /**
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

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

* Re: [PATCH 2/2] remoteproc: core: Register the character device interface
  2020-03-20 23:36 ` [PATCH 2/2] remoteproc: core: Register the character device interface Rishabh Bhatnagar
@ 2020-03-25  4:22   ` Bjorn Andersson
  0 siblings, 0 replies; 7+ messages in thread
From: Bjorn Andersson @ 2020-03-25  4:22 UTC (permalink / raw)
  To: Rishabh Bhatnagar
  Cc: linux-remoteproc-owner, linux-kernel, mathieu.poirier, psodagud,
	tsoni, sidgup

On Fri 20 Mar 16:36 PDT 2020, Rishabh Bhatnagar wrote:

> Add the character device during rproc_add. This would create
> a character device node at /dev/subsys_<rproc_name>. Userspace
> applications can interact with the remote processor using this
> interface rather than using sysfs node. To distinguish between
> different remote processor nodes the device name has been changed
> to include the rproc name appended to "subsys_" string.
> 
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> ---
>  drivers/remoteproc/remoteproc_core.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 097f33e..48a3932 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1907,6 +1907,12 @@ int rproc_add(struct rproc *rproc)
>  	struct device *dev = &rproc->dev;
>  	int ret;
>  
> +	ret = rproc_char_device_add(rproc);
> +	if (ret) {
> +		pr_err("error while adding character device\n");
> +		return ret;
> +	}
> +
>  	ret = device_add(dev);
>  	if (ret < 0)
>  		return ret;
> @@ -2044,7 +2050,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>  		return NULL;
>  	}
>  
> -	dev_set_name(&rproc->dev, "remoteproc%d", rproc->index);
> +	dev_set_name(&rproc->dev, "subsys_%s", rproc->name);

Afaict there's no guarantee that rproc->name is unique and I wonder if
this will break any userspace expectations. Please repost this to
remoteproc@ so we can get further input on this.

And if we can do this, I would like it to be "rproc-%s".

Regards,
Bjorn

>  
>  	atomic_set(&rproc->power, 0);
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

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

* Re: [PATCH 2/2] remoteproc: core: Register the character device interface
  2020-03-26 16:50 ` [PATCH 2/2] remoteproc: core: Register the character device interface Rishabh Bhatnagar
@ 2020-03-30 22:13   ` Mathieu Poirier
  0 siblings, 0 replies; 7+ messages in thread
From: Mathieu Poirier @ 2020-03-30 22:13 UTC (permalink / raw)
  To: Rishabh Bhatnagar
  Cc: linux-kernel, linux-remoteproc, bjorn.andersson, ohad, psodagud,
	tsoni, sidgup

On Thu, Mar 26, 2020 at 09:50:40AM -0700, Rishabh Bhatnagar wrote:
> Add the character device during rproc_add. This would create
> a character device node at /dev/subsys_<rproc_name>. Userspace
> applications can interact with the remote processor using this
> interface rather than using sysfs node. To distinguish between
> different remote processor nodes the device name has been changed
> to include the rproc name appended to "rproc_" string.
> 
> Change-Id: I2114f77f8d2b5fd97e281021ec9905ef5c2fb54c
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> ---
>  drivers/remoteproc/remoteproc_core.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
> index 097f33e..f657983 100644
> --- a/drivers/remoteproc/remoteproc_core.c
> +++ b/drivers/remoteproc/remoteproc_core.c
> @@ -1907,6 +1907,12 @@ int rproc_add(struct rproc *rproc)
>  	struct device *dev = &rproc->dev;
>  	int ret;
>  
> +	ret = rproc_char_device_add(rproc);
> +	if (ret) {
> +		pr_err("error while adding character device\n");
> +		return ret;
> +	}
> +
>  	ret = device_add(dev);
>  	if (ret < 0)
>  		return ret;
> @@ -2044,7 +2050,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
>  		return NULL;
>  	}
>  
> -	dev_set_name(&rproc->dev, "remoteproc%d", rproc->index);
> +	dev_set_name(&rproc->dev, "rproc_%s", rproc->name);

Unfortunately you can't do that. The name of the remoteproc in sysfs is visible
by users and a lot of scripts will fail because of this change.

>  
>  	atomic_set(&rproc->power, 0);
>  
> @@ -2220,6 +2226,7 @@ static int __init remoteproc_init(void)
>  {
>  	rproc_init_sysfs();
>  	rproc_init_debugfs();
> +	rproc_init_cdev();
>  
>  	return 0;
>  }
> @@ -2231,6 +2238,7 @@ static void __exit remoteproc_exit(void)
>  
>  	rproc_exit_debugfs();
>  	rproc_exit_sysfs();
> +	rproc_exit_cdev();
>  }
>  module_exit(remoteproc_exit);
>  
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> a Linux Foundation Collaborative Project

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

* [PATCH 2/2] remoteproc: core: Register the character device interface
  2020-03-26 16:50 [PATCH 0/2] Add character device interface to remoteproc Rishabh Bhatnagar
@ 2020-03-26 16:50 ` Rishabh Bhatnagar
  2020-03-30 22:13   ` Mathieu Poirier
  0 siblings, 1 reply; 7+ messages in thread
From: Rishabh Bhatnagar @ 2020-03-26 16:50 UTC (permalink / raw)
  To: linux-kernel, linux-remoteproc, bjorn.andersson, ohad
  Cc: psodagud, tsoni, sidgup, Rishabh Bhatnagar

Add the character device during rproc_add. This would create
a character device node at /dev/subsys_<rproc_name>. Userspace
applications can interact with the remote processor using this
interface rather than using sysfs node. To distinguish between
different remote processor nodes the device name has been changed
to include the rproc name appended to "rproc_" string.

Change-Id: I2114f77f8d2b5fd97e281021ec9905ef5c2fb54c
Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
---
 drivers/remoteproc/remoteproc_core.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 097f33e..f657983 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1907,6 +1907,12 @@ int rproc_add(struct rproc *rproc)
 	struct device *dev = &rproc->dev;
 	int ret;
 
+	ret = rproc_char_device_add(rproc);
+	if (ret) {
+		pr_err("error while adding character device\n");
+		return ret;
+	}
+
 	ret = device_add(dev);
 	if (ret < 0)
 		return ret;
@@ -2044,7 +2050,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name,
 		return NULL;
 	}
 
-	dev_set_name(&rproc->dev, "remoteproc%d", rproc->index);
+	dev_set_name(&rproc->dev, "rproc_%s", rproc->name);
 
 	atomic_set(&rproc->power, 0);
 
@@ -2220,6 +2226,7 @@ static int __init remoteproc_init(void)
 {
 	rproc_init_sysfs();
 	rproc_init_debugfs();
+	rproc_init_cdev();
 
 	return 0;
 }
@@ -2231,6 +2238,7 @@ static void __exit remoteproc_exit(void)
 
 	rproc_exit_debugfs();
 	rproc_exit_sysfs();
+	rproc_exit_cdev();
 }
 module_exit(remoteproc_exit);
 
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

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

end of thread, other threads:[~2020-03-30 22:13 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-20 23:36 [PATCH 0/2] Add character device interface to remoteproc Rishabh Bhatnagar
2020-03-20 23:36 ` [PATCH 1/2] remoteproc: Add userspace char device driver Rishabh Bhatnagar
2020-03-25  4:10   ` Bjorn Andersson
2020-03-20 23:36 ` [PATCH 2/2] remoteproc: core: Register the character device interface Rishabh Bhatnagar
2020-03-25  4:22   ` Bjorn Andersson
2020-03-26 16:50 [PATCH 0/2] Add character device interface to remoteproc Rishabh Bhatnagar
2020-03-26 16:50 ` [PATCH 2/2] remoteproc: core: Register the character device interface Rishabh Bhatnagar
2020-03-30 22:13   ` Mathieu Poirier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.