Re: [PATCH 06/14] ASoC: SOF: add a power status IPC

[Mark Brown <broonie@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1274 bytes --]

On Fri, Mar 20, 2020 at 12:52:03PM +0100, Guennadi Liakhovetski wrote:
> On Fri, Mar 13, 2020 at 02:39:56PM +0000, Mark Brown wrote:
> > On Thu, Mar 12, 2020 at 03:44:21PM +0100, Guennadi Liakhovetski wrote:

> > >  #endif
> > > +	atomic_set(&sdev->reset_count, 0);
> > >  	dev_set_drvdata(dev, sdev);

> > Do we really need to use atomics for this?  They are hard to use
> > correctly.

> This variable is accessed from 2 contexts: it's incremented by the SOF 
> driver, when the firmware has booted and it's read by the SOF
> VirtIO backend vhost-be.c when receiving a resume request from the guest. 
> Timewise the variable will only be incremented during the DSP resume / 
> power up, while the VirtIO back end is waiting for the resume to complete in 
> pm_runtime_get_sync(). And only after that it reads the variable. But that 
> can happen on different CPUs. Whereas I think that runtime PM will sync 
> caches somewhere during the process, I think it is better to access the 
> variable in an SMP-safe way, e.g. using atomic operations.

That doesn't address my concern - to repeat, my concern is that atomics
are hard to use correctly.  Is there no other concurrency primitive (for
example this sounds like a completion) which can be used?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]