On Sun, Dec 03, 2017 at 09:11:39PM -0600, Pierre-Louis Bossart wrote:
> On 12/3/17 11:11 AM, Vinod Koul wrote:
> >On Fri, Dec 01, 2017 at 05:52:03PM -0600, Pierre-Louis Bossart wrote:
> >
> >>>+ status = sdw_read(slave, SDW_DP0_INT);
> >>>+ if (status < 0) {
> >>>+ dev_err(slave->bus->dev,
> >>>+ "SDW_DP0_INT read failed:%d", status);
> >>>+ return status;
> >>>+ }
> >>>+
> >>>+ count++;
> >>>+
> >>>+ /* we can get alerts while processing so keep retrying */
> >>
> >>This is not incorrect, but this goes beyond what the spec requires.
> >>
> >>The additional read is to make sure some interrupts are not lost due to a
> >>known race condition. It would be enough to mask the status read the second
> >>time to only check if the interrupts sources which were cleared are still
> >>signaling something.
> >>
> >>With the code as it is, you may catch *new* interrupt sources, which could
> >>impact the arbitration/priority/policy in handling interrupts. It's not
> >>necessarily bad, but you'd need to document whether you want to deal with
> >>the race condition described in the MIPI spec or try to be smarter.
> >
> >This was based on your last comment, lets discuss more offline on this to
> >see what else is required here.
>
> I am fine if you leave the code as is for now, it's not bad but can be
> optimized.
Not bad is not good here :)
Okay I still havent grabbed my coffee, so help me out here. I am not sure I
understand here, can you point me to the part of spec handling you were
referring and what should be *ideally* done
--
~Vinod
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
