lkml.org 
[lkml]   [2020]   [Sep]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v5 1/2] remoteproc: Move coredump configuration to sysfs
On Fri 18 Sep 10:22 PDT 2020, Rishabh Bhatnagar wrote:
> diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c
[..]
> +static ssize_t coredump_store(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + struct rproc *rproc = to_rproc(dev);
> +
> + if (rproc->state == RPROC_CRASHED) {
> + dev_err(&rproc->dev, "can't change coredump configuration\n");
> + return -EBUSY;
> + }
> +
> + if (sysfs_streq(buf, "disable")) {

As pointed out by Sibi (somewhere) this should be "disabled" to match
what's returned in coredump_show() and what goes into the recovery
file.

> + rproc->dump_conf = RPROC_COREDUMP_DISABLED;
> + } else if (sysfs_streq(buf, "inline")) {
> + rproc->dump_conf = RPROC_COREDUMP_INLINE;
> + } else if (sysfs_streq(buf, "default")) {

As I pointed out in a previous review, I think it makes sense to change
the default for dump_conf to "disabled" (a separate change). But in
doing so using the word "default" in the sysfs interface would be
misleading - and once it's part of sysfs it stays.

So how about changing this to "enabled"?

PS. The change of the default value of dump_conf would, in line with
Mathieu's ask, be a separate/unrelated change.

Regards,
Bjorn

\
 
 \ /
  Last update: 2020-09-26 05:36    [W:0.049 / U:0.344 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site