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

[Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>]

On Fri, Mar 20, 2020 at 04:39:48PM +0000, Mark Brown wrote:
> On Fri, Mar 20, 2020 at 04:07:27PM +0100, Guennadi Liakhovetski wrote:
> > On Fri, Mar 20, 2020 at 02:27:32PM +0100, Guennadi Liakhovetski wrote:
> 
> > > No, this isn't a completion - it's a counter. I've used atomic variables 
> > > before, I cannot remember seeing any difficulties with their correct use 
> > > described. Do you have a pointer?
> 
> > Actually I'd even say this isn't a problem. I think it's safe to say, you 
> > won't suspend and resume your audio interface more often than every 10 
> > seconds. That makes under 3200000 cycles per year. Even with 31 bits for a 
> > signed integer that makes more than 600 years. I think we're safe.
> 
> The problem is that atomics are just incredibly error prone - for
> example they're just a plain number, they're usually counting something
> which is not being locked so you have to be careful any time you do
> anything around them.  Their lack of structure makes them harder to
> reason about than most other locking primitives, often harder than it's
> worth.

Ok, understand, but do you see any problems with this specific use case? 
Thinking about possible replacements - it isn't a case for a ref-count, 
because it isn't a get-put scenario. We really just need to count a 
specific event - DSP reboots. It can be the case that this counter doesn't 
need to be atomic at all. When it is read, the DSP is guaranteed to be up 
and running - I think. So no race would be possible. I can try to think 
about this more carefully and maybe make it a normal unprotected counter. 
But I don't think it has to be protected even better than what these 
patches are doing.

Thanks
Guennadi