On Thu, Nov 21, 2019 at 11:20 AM Pierre-Louis Bossart <pierre-louis.bossart@xxxxxxxxxxxxxxx> wrote: > Still you may want to clarify your second point and the commit message. > The first race condition was clear, the second one not so much. > > what did you mean with 'assert ULK == 0' and what does this map to in > pll_work The case (2) race condition is based on the previous patch: https://mailman.alsa-project.org/pipermail/alsa-devel/2019-November/158852.html After applying the patch, the code would be something like: (in the case, two threads read 0x01.) static void max98090_pll_work(struct work_struct *work) { [snip] snd_soc_component_update_bits(component, M98090_REG_DEVICE_SHUTDOWN, M98090_SHDNN_MASK, 0); snd_soc_component_update_bits(component, M98090_REG_DEVICE_SHUTDOWN, M98090_SHDNN_MASK, M98090_SHDNN_MASK); for (i = 0; i < 10; ++i) { /* Check lock status */ pll = snd_soc_component_read32( component, M98090_REG_DEVICE_STATUS); if (!(pll & M98090_ULK_MASK)) break; /* Give PLL time to lock */ usleep_range(1000, 1200); } } I guess we have 2 choices: i. stick to the original plan: option B, remove the second thread of execution. In the case, we would be punished by at most 10~12ms delay in the interrupt handler thread. I browsed the max98090_interrupt() and tend to think it should be fine if we delay jack detection for extra 10ms. ii. adopt option A, play with the masks; and drop the previous patch to avoid multiple threads access 0x01 We would be forced to wait >10ms before re-enabling ULK interrupt. Notably, Documentation/timers/timers-howto.txt states "msleep(1~20) may not do what the caller intends, and will often sleep longer".Documentation/timers/timers-howto.txt". Either way should be fine. Which one would you suggest? _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx https://mailman.alsa-project.org/mailman/listinfo/alsa-devel
