Re: [alsa-devel] [PATCH v4 09/15] soundwire: Add slave status handling

[Pierre-Louis Bossart]

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.