[alsa-devel] [PATCH 3/3] ASoC: max98090: fix possible race conditions

[Pierre-Louis Bossart]

>> if your interrupt sees an ULK status, it will schedule the pll_work.
>> You only test the status again *after* switching the device on/off, so
>> what is the side effect?
> 
> Both interrupt thread and pll_work thread access the status register.
> The side effect is: interrupt thread could read the status register
> right before pll_work thread's read.
> 
>> I am also wondering if you shouldn't sleep first in the loop, then check
>> the status?
> 
> ACK, "sleep first and check the status" makes more sense.
> 
>> And in case you do have a side effect due to the clear on read, maybe
>> you could save the status in a context and retest in the workqueue, that
>> way you'd have a trace of the ULK event even if the register was cleared.
> 
> I think cache the status couldn't be a good idea.  We couldn't make
> sure the freshness of the cache.

I was thinking more of treating it as a volatile, but it's ugly indeed.

> Can we suppose CPUs always get ULK=1 if PLL is unlocked so that we can
> simply ignore the case (2)?  Since we know the register is volatile
> and read the register via I2C should be much slower than hardware
> raise the bit.

Your option B with the small change to sleep then retest is probably the 
better solution overall.

BTW did you test this on both Baytrail and BSW chromebooks? In the past 
I saw baytrail devices exposed this error a lot more than BSW.