spi-atmel: scheduling while atomic

spi-atmel: scheduling while atomic

[Alexander Stein]

Hi,

I'm trying to use device-tree on my at91sam9263 based board. Works so far, but I'm getting those BUGs
for each SPI device being probed (2 in my case)

[    2.787000] BUG: scheduling while atomic: spi32766/37/0x00000002                                                                                                                                                                          
[    2.793000] Modules linked in:                                                                                                                                                                                                            
[    2.796000] CPU: 0 PID: 37 Comm: spi32766 Not tainted 3.14.0+ #72                                                                                                                                                                         
[    2.802000] [<c000d8dc>] (unwind_backtrace) from [<c000bcfc>] (show_stack+0x10/0x14)                                                                                                                                                      
[    2.810000] [<c000bcfc>] (show_stack) from [<c0355a54>] (__schedule_bug+0x48/0x64)                                                                                                                                                        
[    2.818000] [<c0355a54>] (__schedule_bug) from [<c0359880>] (__schedule+0x64/0x410)                                                                                                                                                       
[    2.826000] [<c0359880>] (__schedule) from [<c0359554>] (schedule_timeout+0x164/0x190)                                                                                                                                                    
[    2.834000] [<c0359554>] (schedule_timeout) from [<c035a734>] (wait_for_common+0x180/0x1d0)                                                                                                                                               
[    2.842000] [<c035a734>] (wait_for_common) from [<c0242aa8>] (atmel_spi_one_transfer+0x8dc/0xb7c)                                                                                                                                         
[    2.851000] [<c0242aa8>] (atmel_spi_one_transfer) from [<c0242df8>] (atmel_spi_transfer_one_message+0xb0/0x1ac)                                                                                                                           
[    2.861000] [<c0242df8>] (atmel_spi_transfer_one_message) from [<c023f500>] (spi_pump_messages+0x1dc/0x204)                                                                                                                               
[    2.871000] [<c023f500>] (spi_pump_messages) from [<c002fc30>] (kthread_worker_fn+0x154/0x170)                                                                                                                                            
[    2.880000] [<c002fc30>] (kthread_worker_fn) from [<c002fd0c>] (kthread+0xc0/0xd4)                                                                                                                                                        
[    2.887000] [<c002fd0c>] (kthread) from [<c0009630>] (ret_from_fork+0x14/0x24)                                                                                                                                                            

Currently, I can't figure out why this happens. It's a plain v3.14 kernel with some own patches which don't affect SPI.

Best regards,
Alexander

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: spi-atmel: scheduling while atomic

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 606 bytes --]

On Fri, Apr 11, 2014 at 03:25:43PM +0200, Alexander Stein wrote:

> Currently, I can't figure out why this happens. It's a plain v3.14
> kernel with some own patches which don't affect SPI.

It's because atmel_spi_lock() is a spinlock and we're waiting for the
transfer in transfer_one().  Either the scope of the lock needs to be
dramatically reduced or the lock needs to become something we can hold
while scheduling like a mutex.

Converting to use the core transfer_one_message() would also help, we
could then return from the transfer function and have the core implement
the wait for the completion.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Re: spi-atmel: scheduling while atomic

[Alexander Stein]

On Friday 11 April 2014, 17:22:23 wrote Mark Brown:
> On Fri, Apr 11, 2014 at 03:25:43PM +0200, Alexander Stein wrote:
> 
> > Currently, I can't figure out why this happens. It's a plain v3.14
> > kernel with some own patches which don't affect SPI.
> 
> It's because atmel_spi_lock() is a spinlock and we're waiting for the
> transfer in transfer_one().  Either the scope of the lock needs to be
> dramatically reduced or the lock needs to become something we can hold
> while scheduling like a mutex.

Mh, this seems to related to DMA. I don't have an idea how to work an that.

> Converting to use the core transfer_one_message() would also help, we
> could then return from the transfer function and have the core implement
> the wait for the completion.

Mh, this driver already uses that.
> master->transfer_one_message = atmel_spi_transfer_one_message;

Or did you mean transfer_one?

Regards,
Alexander

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Re: spi-atmel: scheduling while atomic

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

On Fri, Apr 11, 2014 at 11:11:29PM +0200, Alexander Stein wrote:
> On Friday 11 April 2014, 17:22:23 wrote Mark Brown:

> > Converting to use the core transfer_one_message() would also help, we
> > could then return from the transfer function and have the core implement
> > the wait for the completion.

> Mh, this driver already uses that.
> > master->transfer_one_message = atmel_spi_transfer_one_message;

> Or did you mean transfer_one?

Yes, I mean the driver should use the core version of
transfer_one_message() rather than providing its own which in turn means
that it has to provide a transfer_one() function.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[PATCH] spi: atmel: Fix scheduling while atomic bug

[Alexander Stein]

atmel_spi_lock does a spin_lock_irqsave, so we need to renable the
interrupts when we want to schedule.

Signed-off-by: Alexander Stein <alexanders83-S0/GAf8tV78@public.gmane.org>
---
I'm not sure what the lock is actually good for, so I kept it and release
it when we are about to schedule, thus reenabling interrupts.

 drivers/spi/spi-atmel.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/spi/spi-atmel.c b/drivers/spi/spi-atmel.c
index 5d7b07f..ae22f3d 100644
--- a/drivers/spi/spi-atmel.c
+++ b/drivers/spi/spi-atmel.c
@@ -1130,8 +1130,11 @@ static int atmel_spi_one_transfer(struct spi_master *master,
 			atmel_spi_next_xfer_pio(master, xfer);
 		}
 
+		/* interrupts are disabled, so free the lock for schedule */
+		atmel_spi_unlock(as);
 		ret = wait_for_completion_timeout(&as->xfer_completion,
 							SPI_DMA_TIMEOUT);
+		atmel_spi_lock(as);
 		if (WARN_ON(ret == 0)) {
 			dev_err(&spi->dev,
 				"spi trasfer timeout, err %d\n", ret);
-- 
1.9.2

--
To unsubscribe from this list: send the line "unsubscribe linux-spi" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] spi: atmel: Fix scheduling while atomic bug

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

On Sun, Apr 13, 2014 at 12:45:10PM +0200, Alexander Stein wrote:
> atmel_spi_lock does a spin_lock_irqsave, so we need to renable the
> interrupts when we want to schedule.
> 
> Signed-off-by: Alexander Stein <alexanders83-S0/GAf8tV78@public.gmane.org>
> ---
> I'm not sure what the lock is actually good for, so I kept it and release
> it when we are about to schedule, thus reenabling interrupts.

Applied, thanks - I don't fully understand the locking either and this
is definitely a conservative fix.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]