Re: [PATCH v2 2/3] remoteproc: Add coredump configuration to sysfs

Greg KH
2020-09-10T18:18:09Z

On Thu, Aug 27, 2020 at 12:48:50PM -0700, Rishabh Bhatnagar wrote:
> Expose coredump configuration in sysfs under a feature
> flag. This is useful for systems where access to
> debugfs might be limited.
>
> Signed-off-by: Rishabh Bhatnagar <rishabhb@codeaurora.org>
> ---
> Documentation/ABI/testing/sysfs-class-remoteproc | 24 +++++++++
> drivers/remoteproc/remoteproc_debugfs.c | 4 ++
> drivers/remoteproc/remoteproc_sysfs.c | 68 ++++++++++++++++++++++++
> 3 files changed, 96 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-class-remoteproc b/Documentation/ABI/testing/sysfs-class-remoteproc
> index 36094fb..f6c44fa 100644
> --- a/Documentation/ABI/testing/sysfs-class-remoteproc
> +++ b/Documentation/ABI/testing/sysfs-class-remoteproc
> @@ -58,3 +58,27 @@ Description: Remote processor name
> Reports the name of the remote processor. This can be used by
> userspace in exactly identifying a remote processor and ease
> up the usage in modifying the 'firmware' or 'state' files.
> +
> +What: /sys/class/remoteproc/.../coredump
> +Date: July 2020
> +Contact: Bjorn Andersson <bjorn.andersson@linaro.org>, Ohad Ben-Cohen <ohad@wizery.com>
> +Description: Remote processor coredump configuration
> +
> + Reports the coredump configuration of the remote processor,
> + which will be one of:
> +
> + "default"
> + "inline"
> + "disabled"
> +
> + "default" means when the remote processor's coredump is
> + collected it will be copied to a separate buffer and that
> + buffer is exposed to userspace.
> +
> + "inline" means when the remote processor's coredump is
> + collected userspace will directly read from the remote
> + processor's device memory. Extra buffer will not be used to
> + copy the dump. Also recovery process will not proceed until
> + all data is read by usersapce.
> +
> + "disabled" means no dump will be collected.
> diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c
> index 2e3b3e2..48dfd0a 100644
> --- a/drivers/remoteproc/remoteproc_debugfs.c
> +++ b/drivers/remoteproc/remoteproc_debugfs.c
> @@ -27,6 +27,7 @@
> /* remoteproc debugfs parent dir */
> static struct dentry *rproc_dbg;
>
> +#if (!IS_ENABLED(CONFIG_RPROC_SYSFS_CONFIGURATION_SUPPORT))
> /*
> * A coredump-configuration-to-string lookup table, for exposing a
> * human readable configuration via debugfs. Always keep in sync with
> @@ -114,6 +115,7 @@ static const struct file_operations rproc_coredump_fops = {
> .open = simple_open,
> .llseek = generic_file_llseek,
> };
> +#endif
>
> /*
> * Some remote processors may support dumping trace logs into a shared
> @@ -425,8 +427,10 @@ void rproc_create_debug_dir(struct rproc *rproc)
> rproc, &rproc_rsc_table_fops);
> debugfs_create_file("carveout_memories", 0400, rproc->dbg_dir,
> rproc, &rproc_carveouts_fops);
> +#if (!IS_ENABLED(CONFIG_RPROC_SYSFS_CONFIGURATION_SUPPORT))
> debugfs_create_file("coredump", 0600, rproc->dbg_dir,
> rproc, &rproc_coredump_fops);
> +#endif

Why does sysfs support for this have anything to do if you have a
debugfs file present or not? They should both work at the same time if
needed, right?

Same for patch 3/3 in this series...

thanks,

greg k-h