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

From: Greg KH <gregkh@linuxfoundation.org>
To: Rishabh Bhatnagar <rishabhb@codeaurora.org>
Cc: linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	bjorn.andersson@linaro.org, mathieu.poirier@linaro.org,
	tsoni@codeaurora.org, psodagud@codeaurora.org,
	sidgup@codeaurora.org
Subject: Re: [PATCH v2 2/3] remoteproc: Add coredump configuration to sysfs
Date: Thu, 10 Sep 2020 19:58:21 +0200	[thread overview]
Message-ID: <20200910175821.GA3076593@kroah.com> (raw)
In-Reply-To: <1598557731-1566-3-git-send-email-rishabhb@codeaurora.org>

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