| Message ID | 20200915073416.20864-1-sibis@codeaurora.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | remoteproc: Fixup coredump debugfs disable request | expand |
On Tue 15 Sep 07:34 UTC 2020, Sibi Sankar wrote: > Currently the coredump debugfs entry takes in "disable" to set the > coredump state to "disabled". Let's just accept the expected state > instead. > I like this patch, but rather than arguing that it should match the name of the internal state I think you should either argue that when read you get "disabled" back or that "disabled" would make it consistent with the recovery. Regards, Bjorn > Fixes: 3afdc59e43904 ("remoteproc: Add coredump debugfs entry") > Signed-off-by: Sibi Sankar <sibis@codeaurora.org> > --- > drivers/remoteproc/remoteproc_debugfs.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c > index 2e3b3e22e1d01..7ca823f6aa638 100644 > --- a/drivers/remoteproc/remoteproc_debugfs.c > +++ b/drivers/remoteproc/remoteproc_debugfs.c > @@ -94,7 +94,7 @@ static ssize_t rproc_coredump_write(struct file *filp, > goto out; > } > > - if (!strncmp(buf, "disable", count)) { > + if (!strncmp(buf, "disabled", count)) { > rproc->dump_conf = RPROC_COREDUMP_DISABLED; > } else if (!strncmp(buf, "inline", count)) { > rproc->dump_conf = RPROC_COREDUMP_INLINE; > -- > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, > a Linux Foundation Collaborative Project >
Hey Bjorn, Thanks for taking time to review the patch. On 2020-09-15 20:48, Bjorn Andersson wrote: > On Tue 15 Sep 07:34 UTC 2020, Sibi Sankar wrote: > >> Currently the coredump debugfs entry takes in "disable" to set the >> coredump state to "disabled". Let's just accept the expected state >> instead. >> > > I like this patch, but rather than arguing that it should match the > name > of the internal state I think you should either argue that when read > you > get "disabled" back or that "disabled" would make it consistent with > the > recovery. Sure, I'll re-word the commit message. I probably choose the read back argument because that's what was odd about it in the first place. > > Regards, > Bjorn > >> Fixes: 3afdc59e43904 ("remoteproc: Add coredump debugfs entry") >> Signed-off-by: Sibi Sankar <sibis@codeaurora.org> >> --- >> drivers/remoteproc/remoteproc_debugfs.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/remoteproc/remoteproc_debugfs.c >> b/drivers/remoteproc/remoteproc_debugfs.c >> index 2e3b3e22e1d01..7ca823f6aa638 100644 >> --- a/drivers/remoteproc/remoteproc_debugfs.c >> +++ b/drivers/remoteproc/remoteproc_debugfs.c >> @@ -94,7 +94,7 @@ static ssize_t rproc_coredump_write(struct file >> *filp, >> goto out; >> } >> >> - if (!strncmp(buf, "disable", count)) { >> + if (!strncmp(buf, "disabled", count)) { >> rproc->dump_conf = RPROC_COREDUMP_DISABLED; >> } else if (!strncmp(buf, "inline", count)) { >> rproc->dump_conf = RPROC_COREDUMP_INLINE; >> -- >> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora >> Forum, >> a Linux Foundation Collaborative Project >>
diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c index 2e3b3e22e1d01..7ca823f6aa638 100644 --- a/drivers/remoteproc/remoteproc_debugfs.c +++ b/drivers/remoteproc/remoteproc_debugfs.c @@ -94,7 +94,7 @@ static ssize_t rproc_coredump_write(struct file *filp, goto out; } - if (!strncmp(buf, "disable", count)) { + if (!strncmp(buf, "disabled", count)) { rproc->dump_conf = RPROC_COREDUMP_DISABLED; } else if (!strncmp(buf, "inline", count)) { rproc->dump_conf = RPROC_COREDUMP_INLINE;
Currently the coredump debugfs entry takes in "disable" to set the coredump state to "disabled". Let's just accept the expected state instead. Fixes: 3afdc59e43904 ("remoteproc: Add coredump debugfs entry") Signed-off-by: Sibi Sankar <sibis@codeaurora.org> --- drivers/remoteproc/remoteproc_debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)