Re: [RFC] [PATCH 3/3] drivers: spi: spi.c: Don't use the message queue if possible in spi_sync

[David Jander <david@xxxxxxxxxxx>]

On Tue, 7 Jun 2022 19:30:55 +0100
Mark Brown <broonie@xxxxxxxxxx> wrote:

> On Wed, May 25, 2022 at 04:46:03PM +0200, David Jander wrote:
> > David Jander <david@xxxxxxxxxxx> wrote:  
> 
> > > +static void __spi_transfer_message_noqueue(struct spi_controller *ctlr, struct spi_message *msg)
> > > +{
> > > +	bool was_busy;
> > > +	int ret;
> > > +
> > > +	mutex_lock(&ctlr->io_mutex);
> > > +
> > > +	/* If another context is idling the device then wait */
> > > +	while (ctlr->idling) {
> > > +		printk(KERN_INFO "spi sync message processing: controller is idling!\n");
> > > +		usleep_range(10000, 11000);
> > > +	}  
> 
> > This is dead ugly of course, and it needs to be removed. Not yet sure how,
> > hence the RFC. Maybe the idle -> not busy transition can be included inside
> > the io_mutex? That way this while will never be hit and can be removed...  
> 
> I'm not sure it's even quite right from a safety point of view - idling
> is protected by queue_lock but this now only takes io_mutex.  

True. This is broken.

> Moving idling (and all the was_busy stuff) within the io_mutex would
> definitely resolve the issue, the async submission context is the only one
> that really needs the spinlock and it doesn't care about idling.  I can't
> think what you could do with the io_mutex when idling so it seems to
> fit.

Ok, so we could agree on a way to fix this particular issue: put the idling
transition into the io_mutex. Thanks.

Looking forward to read comments on the rest of the code, and the general idea
of what I am trying to accomplish.

Best regards,

-- 
David Jander