On Tue, Mar 03, 2020 at 09:36:29AM -0600, Pierre-Louis Bossart wrote: > > > > > We're passing "&posn" instead of "posn" so it ends up corrupting > > > memory instead of doing something useful. > > [...] > > > /* send IPC to the DSP */ > > > err = sof_ipc_tx_message(sdev->ipc, > > > - stream.hdr.cmd, &stream, sizeof(stream), &posn, > > > + stream.hdr.cmd, &stream, sizeof(stream), posn, > > > sizeof(*posn)); > > > > ack, thanks, this is clearly wrong. This function is not used by current > > platforms, so the bug has gone unnnoticed. Most platforms either rely on > > direct MMIO queries of the DSP position, or the periodic position updates > > DSPs send after each ALSA period. This function for host to query DSP > > position via IPC is thus not used, although it's part of the generic audio > > DSP IPC interface. > > > > For the SOF folks in CC, I wonder should we keep this function at all? > > > > Anyways, that's probably a longer discussion, so while it's there, > > the code should be correct, so for the patch: > > Reviewed-by: Kai Vehmanen <kai.vehmanen@xxxxxxxxxxxxxxx> > > I checked all the way to 5.2 and it was never used, so indeed wondering what > the purpose of this function was. I actually have a patch, removing that function, which I noticed as being unused during my VirtIO work. I think it was used by compressed.c before. Thanks Guennadi