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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <94aefed4-9f0d-f81a-b5c0-0ad4cafc6a96@linux.intel.com>
Date:   Fri, 10 Jan 2020 10:35:19 -0600
From:   Pierre-Louis Bossart <pierre-louis.bossart@...ux.intel.com>
To:     Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        vkoul@...nel.org
Cc:     robh@...nel.org, alsa-devel@...a-project.org,
        bgoswami@...eaurora.org, spapothi@...eaurora.org,
        lgirdwood@...il.com, linux-kernel@...r.kernel.org,
        broonie@...nel.org
Subject: Re: [alsa-devel] [PATCH v5 2/2] soundwire: qcom: add support for
 SoundWire controller


>>>> +    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...