[alsa-devel] [PATCH v5 2/2] soundwire: qcom: add support for SoundWire controller

[Pierre-Louis Bossart]

>>>> +    if (sts & SWRM_INTERRUPT_STATUS_CMD_ERROR) {
>>>> +        ctrl->reg_read(ctrl, SWRM_CMD_FIFO_STATUS, &value);
>>>> +        dev_err_ratelimited(ctrl->dev,
>>>> +                    "CMD error, fifo status 0x%x\n",
>>>> +                     value);
>>>> +        ctrl->reg_write(ctrl, SWRM_CMD_FIFO_CMD, 0x1);
>>>> +    }
>>>> +
>>>> +    if ((sts & SWRM_INTERRUPT_STATUS_NEW_SLAVE_ATTACHED) ||
>>>> +        sts & SWRM_INTERRUPT_STATUS_CHANGE_ENUM_SLAVE_STATUS)
>>>> +        schedule_work(&ctrl->slave_work);
>>>> +
>>>> +    ctrl->reg_write(ctrl, SWRM_INTERRUPT_CLEAR, sts);
>>>
>>> is it intentional to clear the interrupts first, before doing 
>>> additional checks?
>>>
>>
>> No, I can move it to right to the end!
> 
> Reason why I did this was that if we run complete() before irq is 
> cleared complete might trigger another read/write which can raise an 
> interrupt. And with interrupt status not cleared we might miss it. This 
> is very much timing dependent specially with the threaded irq.
> 
> So code needs no change atm!

ok, a comment to keep track of this timing dependency could help future 
generations then...