Re: [PATCH v3 07/14] ASoC: SOF: Add DSP firmware logger support

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

 




On 12/11/18 5:21 PM, Andy Shevchenko wrote:
On Tue, Dec 11, 2018 at 03:23:11PM -0600, Pierre-Louis Bossart wrote:
From: Liam Girdwood <liam.r.girdwood@xxxxxxxxxxxxxxx>

This patch adds support for real-time DSP logging (timestamped events
and bespoke binary data) for firmware debug. The current solution
relies on DMA transfers to system memory that is then accessed by
userspace tools such as sof-logger. For Intel platforms, two types of
DMAs are currently used (GP-DMA for Baytrail/CherryTrail and HDaudio
DMA for SKL+)

Due to historical reasons, the driver code follows the DSP firmware
conventions and refers to 'traces', but it is currently unrelated to
the Linux trace subsystem. Future solutions will include support for
more advanced hardware (e.g. MIPI Sys-T), additional formats and the
ability to enable/disable specific traces dynamically.
+	if (count > buffer_size - lpos)
+		count = buffer_size - lpos;
min() / max() ?

+	/* make sure count is <= avail */
+	count = avail > count ? count : avail;
ditto.
Check entire series for such.
yep.

+	dfse->dfsentry = debugfs_create_file("trace", 0444, sdev->debugfs_root,
+					     dfse, &sof_dfs_trace_fops);
+	if (!dfse->dfsentry) {
+		dev_err(sdev->dev,
+			"error: cannot create debugfs entry for trace\n");
+		kfree(dfse);
+		return -ENODEV;
Should not return an error. We must be fine w/o debugfs files.

Not sure I fully agree with the 'must'.

yes execution should proceed, but without debugfs we cannot, well, debug, so this is a real show-stopper


+	}
+
+	return 0;
+}
_______________________________________________
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