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

[Tzung-Bi Shih]

On Thu, Nov 21, 2019 at 11:20 AM Pierre-Louis Bossart
<pierre-louis.bossart at linux.intel.com> 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?