Re: [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...