This patch series contains stability fixes and error handling for remoteproc.
The changes included in this series do the following: Patch 1: Fixes the creation of the rproc character device. Patch 2: Validates rproc as the first step of rproc_add(). Patch 3: Fixes the rproc cdev remove and the order of dev_del() and cdev_del(). Patch 4: Adds error handling in rproc_add().
v1 -> v2: - Added extra patch which addresses Bjorn's comments on patch 3 from v1. - Fixed commit text for patch 2 (s/calling making/making).
Siddharth Gupta (4): remoteproc: core: Move cdev add before device add remoteproc: core: Move validate before device add remoteproc: core: Fix cdev remove and rproc del remoteproc: core: Cleanup device in case of failure
0000-cover-letter.patch.backup | 26 ++++++++++++++++++++++++++ drivers/remoteproc/remoteproc_cdev.c | 2 +- drivers/remoteproc/remoteproc_core.c | 27 ++++++++++++++++++--------- 3 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 0000-cover-letter.patch.backup
When cdev_add is called after device_add has been called there is no way for the userspace to know about the addition of a cdev as cdev_add itself doesn't trigger a uevent notification, or for the kernel to know about the change to devt. This results in two problems: - mknod is never called for the cdev and hence no cdev appears on devtmpfs. - sysfs links to the new cdev are not established.
The cdev needs to be added and devt assigned before device_add() is called in order for the relevant sysfs and devtmpfs entries to be created and the uevent to be properly populated.
Signed-off-by: Siddharth Gupta sidgup@codeaurora.org Reviewed-by: Bjorn Andersson bjorn.andersson@linaro.org --- drivers/remoteproc/remoteproc_core.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 6348aaa..9ad8c5f 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -2333,6 +2333,11 @@ int rproc_add(struct rproc *rproc) struct device *dev = &rproc->dev; int ret;
+ /* add char device for this remoteproc */ + ret = rproc_char_device_add(rproc); + if (ret < 0) + return ret; + ret = device_add(dev); if (ret < 0) return ret; @@ -2346,11 +2351,6 @@ int rproc_add(struct rproc *rproc) /* create debugfs entries */ rproc_create_debug_dir(rproc);
- /* add char device for this remoteproc */ - ret = rproc_char_device_add(rproc); - if (ret < 0) - return ret; - /* if rproc is marked always-on, request it to boot */ if (rproc->auto_boot) { ret = rproc_trigger_auto_boot(rproc);
We can validate whether the remoteproc is correctly setup before making the cdev_add and device_add calls. This saves us the trouble of cleaning up later on.
Signed-off-by: Siddharth Gupta sidgup@codeaurora.org Reviewed-by: Bjorn Andersson bjorn.andersson@linaro.org --- drivers/remoteproc/remoteproc_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index 9ad8c5f..b65fce3 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -2333,16 +2333,16 @@ int rproc_add(struct rproc *rproc) struct device *dev = &rproc->dev; int ret;
- /* add char device for this remoteproc */ - ret = rproc_char_device_add(rproc); + ret = rproc_validate(rproc); if (ret < 0) return ret;
- ret = device_add(dev); + /* add char device for this remoteproc */ + ret = rproc_char_device_add(rproc); if (ret < 0) return ret;
- ret = rproc_validate(rproc); + ret = device_add(dev); if (ret < 0) return ret;
The rproc_char_device_remove() call currently unmaps the cdev region instead of simply deleting the cdev that was added as a part of the rproc_char_device_add() call. This change fixes that behaviour, and also fixes the order in which device_del() and cdev_del() need to be called.
Signed-off-by: Siddharth Gupta sidgup@codeaurora.org --- 0000-cover-letter.patch.backup | 26 ++++++++++++++++++++++++++ drivers/remoteproc/remoteproc_cdev.c | 2 +- drivers/remoteproc/remoteproc_core.c | 2 +- 3 files changed, 28 insertions(+), 2 deletions(-) create mode 100644 0000-cover-letter.patch.backup
diff --git a/0000-cover-letter.patch.backup b/0000-cover-letter.patch.backup new file mode 100644 index 0000000..837bcdb --- /dev/null +++ b/0000-cover-letter.patch.backup @@ -0,0 +1,26 @@ +From 78fcb27d574d2f7b06e37b77b257f98858a88d9d Mon Sep 17 00:00:00 2001 +From: Siddharth Gupta sidgup@codeaurora.org +Date: Mon, 17 May 2021 13:33:25 -0700 +Subject: [PATCH 0/3] remoteproc: core: Fixes for rproc cdev and add +To: bjorn.andersson@linaro.org,ohad@wizery.com,linux-remoteproc@vger.kernel.org +Cc: linux-kernel@vger.kernel.org,linux-arm-msm@vger.kernel.org,linux-arm-kernel@lists.infradead.org,psodagud@codeaurora.org,stable@vger.kernel.org + +This patch series contains stability fixes and error handling for remoteproc. + +The changes included in this series do the following: +Patch 1: Fixes the creation of the rproc character device. +Patch 2: Validates rproc as the first step of rproc_add(). +Patch 3: Adds error handling in rproc_add(). + +Siddharth Gupta (3): + remoteproc: core: Move cdev add before device add + remoteproc: core: Move validate before device add + remoteproc: core: Cleanup device in case of failure + + drivers/remoteproc/remoteproc_core.c | 25 +++++++++++++++++-------- + 1 file changed, 17 insertions(+), 8 deletions(-) + +-- +Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, +a Linux Foundation Collaborative Project + diff --git a/drivers/remoteproc/remoteproc_cdev.c b/drivers/remoteproc/remoteproc_cdev.c index 0b8a84c..4ad98b0 100644 --- a/drivers/remoteproc/remoteproc_cdev.c +++ b/drivers/remoteproc/remoteproc_cdev.c @@ -124,7 +124,7 @@ int rproc_char_device_add(struct rproc *rproc)
void rproc_char_device_remove(struct rproc *rproc) { - __unregister_chrdev(MAJOR(rproc->dev.devt), rproc->index, 1, "remoteproc"); + cdev_del(&rproc->cdev); }
void __init rproc_init_cdev(void) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index b65fce3..b874280 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -2619,7 +2619,6 @@ int rproc_del(struct rproc *rproc) mutex_unlock(&rproc->lock);
rproc_delete_debug_dir(rproc); - rproc_char_device_remove(rproc);
/* the rproc is downref'ed as soon as it's removed from the klist */ mutex_lock(&rproc_list_mutex); @@ -2630,6 +2629,7 @@ int rproc_del(struct rproc *rproc) synchronize_rcu();
device_del(&rproc->dev); + rproc_char_device_remove(rproc);
return 0; }
When a failure occurs in rproc_add() it returns an error, but does not cleanup after itself. This change adds the failure path in such cases.
Signed-off-by: Siddharth Gupta sidgup@codeaurora.org --- drivers/remoteproc/remoteproc_core.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index b874280..d823f70 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -2343,8 +2343,10 @@ int rproc_add(struct rproc *rproc) return ret;
ret = device_add(dev); - if (ret < 0) - return ret; + if (ret < 0) { + put_device(dev); + goto rproc_remove_cdev; + }
dev_info(dev, "%s is available\n", rproc->name);
@@ -2355,7 +2357,7 @@ int rproc_add(struct rproc *rproc) if (rproc->auto_boot) { ret = rproc_trigger_auto_boot(rproc); if (ret < 0) - return ret; + goto rproc_remove_dev; }
/* expose to rproc_get_by_phandle users */ @@ -2364,6 +2366,13 @@ int rproc_add(struct rproc *rproc) mutex_unlock(&rproc_list_mutex);
return 0; + +rproc_remove_dev: + rproc_delete_debug_dir(rproc); + device_del(dev); +rproc_remove_cdev: + rproc_char_device_remove(rproc); + return ret; } EXPORT_SYMBOL(rproc_add);
On Mon 14 Jun 15:40 CDT 2021, Siddharth Gupta wrote:
This patch series contains stability fixes and error handling for remoteproc.
Thanks for respinning this Sid, I think patch 3 looks good now as well, but it seems you accidentally got an additional file in the same patch.
Can you please fix that up and resend the patches?
Regards, Bjorn
The changes included in this series do the following: Patch 1: Fixes the creation of the rproc character device. Patch 2: Validates rproc as the first step of rproc_add(). Patch 3: Fixes the rproc cdev remove and the order of dev_del() and cdev_del(). Patch 4: Adds error handling in rproc_add().
v1 -> v2:
- Added extra patch which addresses Bjorn's comments on patch 3 from v1.
- Fixed commit text for patch 2 (s/calling making/making).
Siddharth Gupta (4): remoteproc: core: Move cdev add before device add remoteproc: core: Move validate before device add remoteproc: core: Fix cdev remove and rproc del remoteproc: core: Cleanup device in case of failure
0000-cover-letter.patch.backup | 26 ++++++++++++++++++++++++++ drivers/remoteproc/remoteproc_cdev.c | 2 +- drivers/remoteproc/remoteproc_core.c | 27 ++++++++++++++++++--------- 3 files changed, 45 insertions(+), 10 deletions(-) create mode 100644 0000-cover-letter.patch.backup
-- Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
linux-stable-mirror@lists.linaro.org