Re: [PATCH v3 03/14] ASoC: SOF: Add driver debug support.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[combined feedback from Ranjani, Liam and me]


+	/* copy from DSP MMIO */
+	pm_runtime_get_noresume(sdev->dev);
Why are we doing a _get_noresume() here?  It won't actually do anything
to the device...
the reason we currently do a get_noresume() is so that we can still read the debugfs entries in case of DSP panic, where a regular resume would not succeed.

That said this part will be reworked based on feedback from Rafael whose suggestion was to get rid of the pm calls while reading debugfs. We can make sure that all the values are flushed out of the dsp before suspending which will make the need to wake it up redundant.


+
+	memcpy_fromio(buf,  dfse->buf + pos, size);
Extra space?
yep. we run checkpatch.pl --strict all the time and missed this somehow.

+	/*
+	 * TODO: revisit to check if we need mark_last_busy, or if we
+	 * should change to use xxx_put_sync[_suspend]().
+	 */
+	ret = pm_runtime_put_sync_autosuspend(sdev->dev);
+	if (ret < 0)
+		dev_warn(sdev->dev, "warn: debugFS failed to autosuspend %zd\n",
+			 ret);
It rather depends what you're doing...  I'm definitely confused as to
why you need a _sync operation - if you're doing autosuspend stuff
presumably you don't care so much if the device gets powered down
immediately, and I can't in general see why that'd be important.
yes, as stated above we'll rework all this

+	dfse->dfsentry = debugfs_create_file(name, 0444, sdev->debugfs_root,
+					     dfse, &sof_dfs_fops);
+	if (!dfse->dfsentry) {
+		dev_err(sdev->dev, "error: cannot create debugfs entry.\n");
+		return -ENODEV;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(snd_sof_debugfs_io_create_item);
debugfs uses EXPORT_SYMBOL_GPL().

We don't have a burning desire to keep this as EXPORT_SYMBOL since debugfs is really Linux specific, so will move all these statements as EXPORT_SYMBOL_GPL.

Thanks Mark for your time, much appreciated. We've already dealt with most of the feedback from Takashi/Andy/Daniel so should have a new version of these patches in 1-2 weeks.

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux