On Fri, Jun 14, 2019 at 04:14:10PM +0100, Mark Brown wrote:
On Fri, Jun 14, 2019 at 11:47:52AM +0200, Greg Kroah-Hartman wrote:
When calling debugfs functions, there is no need to ever check the return value. The function can work or not, but the code logic should never do something different based on this.
+++ b/sound/soc/sof/debug.c @@ -77,8 +77,7 @@ static ssize_t sof_dfsentry_read(struct file *file, char __user *buffer, if (!pm_runtime_active(sdev->dev) && dfse->access_type == SOF_DEBUGFS_ACCESS_D0_ONLY) { dev_err(sdev->dev,
"error: debugfs entry %s cannot be read in DSP D3\n",dfse->dfsentry->d_name.name);
"error: debugfs entry cannot be read in DSP D3\n"); kfree(buf); return -EINVAL;This appears to be an unrelated change with no description in the changelog, please split it out into a separate change with a description of the change.
The whole "dfsentry" variable is now gone, so it is very related. Why split this out?
@@ -119,13 +119,8 @@ static int trace_debugfs_create(struct snd_sof_dev *sdev) dfse->size = sdev->dmatb.bytes; dfse->sdev = sdev;
- dfse->dfsentry = debugfs_create_file("trace", 0444, sdev->debugfs_root,
dfse, &sof_dfs_trace_fops);- if (!dfse->dfsentry) {
/* can't rely on debugfs, only log error and keep going */dev_err(sdev->dev,"error: cannot create debugfs entry for trace\n");- }
- debugfs_create_file("trace", 0444, sdev->debugfs_root, dfse,
&sof_dfs_trace_fops);I might be missing something but I can't see any error logging in debugfs_create_file() so this isn't equivalent (though the current code is broken, it should be using IS_ERR()). Logging creation failures is helpful to developers trying to figure out what happened to the trace files they're trying to use.
There is no need to log anything in in-kernel, working code. If a developer wants to do this on their own when writing the code the first time and debugging it, great, but for stuff we know works, there's no need for being noisy.
Also, the check is incorrect, there's no way for this function to return NULL, so that code today, will never trigger. So obviously no one counted on this message anyway :)
Just call the function and move on, like the rest of the kernel is being converted over to do.
thanks,
greg k-h