On Tue, Dec 11, 2018 at 03:23:07PM -0600, Pierre-Louis Bossart wrote:
+ /* 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...
+ + memcpy_fromio(buf, dfse->buf + pos, size);
Extra space?
+ /* + * 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.
+ 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().