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
