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

[Andy Shevchenko <andriy.shevchenko@intel.com>]

On Tue, Dec 11, 2018 at 03:23:07PM -0600, Pierre-Louis Bossart wrote:
> From: Liam Girdwood <liam.r.girdwood@linux.intel.com>
> 
> Add debugFS files that can be used to expose DSP memories and
> and peripherals to userspace to assist with firmware debugging.
> 
> TODO: The runtime_pm parts are not final and will be updated.

> +	if (count > size - pos)
> +		count = size - pos;

max() / min()

> +	/* intermediate buffer size must be u32 multiple */
> +	size = (count + 3) & ~3;

Hmm, don't we have something like round_up()?

> +	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;
> +	}

We shouldn't rely on debugfs files.
Thus, error checking needs to be removed.

Something like

void ()
{
struct dentry *d;

d = debugfs_...();
if (!d)
	dev_err();
}


> +	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;
> +	}

Ditto.

> +	sdev->debugfs_root = debugfs_create_dir("sof", NULL);
> +	if (IS_ERR_OR_NULL(sdev->debugfs_root)) {
> +		dev_err(sdev->dev, "error: failed to create debugfs directory\n");
> +		return -EINVAL;
> +	}
> +
> +	/* create debugFS files for platform specific MMIO/DSP memories */
> +	for (i = 0; i < ops->debug_map_count; i++) {
> +		map = &ops->debug_map[i];
> +

> +		err = snd_sof_debugfs_io_create_item(sdev, sdev->bar[map->bar] +
> +						  map->offset, map->size,
> +						  map->name);
> +		if (err < 0)
> +			dev_err(sdev->dev, "error: cannot create debugfs for %s\n",
> +				map->name);

Ditto (error message dups above).

> +	}
> +
> +	return err;
> +}
> +EXPORT_SYMBOL(snd_sof_dbg_init);

-- 
With Best Regards,
Andy Shevchenko