[PATCH] mmc: atmel-mci: do not assume idle after atmci_request_end

[PATCH] mmc: atmel-mci: do not assume idle after atmci_request_end

[Jonas Danielsson]

On our AT91SAM9260 board we use the same sdio bus for wifi and for the
sd card slot. This caused the atmel-mci to give the following splat on
the serial console:

  ------------[ cut here ]------------
  WARNING: CPU: 0 PID: 538 at drivers/mmc/host/atmel-mci.c:859 atmci_send_command+0x24/0x44
  Modules linked in:
  CPU: 0 PID: 538 Comm: mmcqd/0 Not tainted 4.14.76 #14
  Hardware name: Atmel AT91SAM9
  [<c000fccc>] (unwind_backtrace) from [<c000d3dc>] (show_stack+0x10/0x14)
  [<c000d3dc>] (show_stack) from [<c0017644>] (__warn+0xd8/0xf4)
  [<c0017644>] (__warn) from [<c0017704>] (warn_slowpath_null+0x1c/0x24)
  [<c0017704>] (warn_slowpath_null) from [<c033bb9c>] (atmci_send_command+0x24/0x44)
  [<c033bb9c>] (atmci_send_command) from [<c033e984>] (atmci_start_request+0x1f4/0x2dc)
  [<c033e984>] (atmci_start_request) from [<c033f3b4>] (atmci_request+0xf0/0x164)
  [<c033f3b4>] (atmci_request) from [<c0327108>] (mmc_start_request+0x280/0x2d0)
  [<c0327108>] (mmc_start_request) from [<c032800c>] (mmc_start_areq+0x230/0x330)
  [<c032800c>] (mmc_start_areq) from [<c03366f8>] (mmc_blk_issue_rw_rq+0xc4/0x310)
  [<c03366f8>] (mmc_blk_issue_rw_rq) from [<c03372c4>] (mmc_blk_issue_rq+0x118/0x5ac)
  [<c03372c4>] (mmc_blk_issue_rq) from [<c033781c>] (mmc_queue_thread+0xc4/0x118)
  [<c033781c>] (mmc_queue_thread) from [<c002daf8>] (kthread+0x100/0x118)
  [<c002daf8>] (kthread) from [<c000a580>] (ret_from_fork+0x14/0x34)
  ---[ end trace 594371ddfa284bd6 ]---

This is:
  WARN_ON(host->cmd);

This was fixed on our board by letting atmci_request_end determine what
state we are in. Instead of unconditionally setting it to STATE_IDLE on
STATE_END_REQUEST.

Signed-off-by: Jonas Danielsson <jonas@orbital-systems.com>
---
 drivers/mmc/host/atmel-mci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index 0a0ebf3a0..c8a591d8a 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -1954,13 +1954,14 @@ static void atmci_tasklet_func(unsigned long priv)
 			}
 
 			atmci_request_end(host, host->mrq);
-			state = STATE_IDLE;
+			goto unlock; /* atmci_request_end() sets host->state */
 			break;
 		}
 	} while (state != prev_state);
 
 	host->state = state;
 
+unlock:
 	spin_unlock(&host->lock);
 }
 
-- 
2.17.2

Re: [PATCH] mmc: atmel-mci: do not assume idle after atmci_request_end

[Ulf Hansson]

On 19 October 2018 at 16:40, Jonas Danielsson <jonas@orbital-systems.com> wrote:
> On our AT91SAM9260 board we use the same sdio bus for wifi and for the
> sd card slot. This caused the atmel-mci to give the following splat on
> the serial console:
>
>   ------------[ cut here ]------------
>   WARNING: CPU: 0 PID: 538 at drivers/mmc/host/atmel-mci.c:859 atmci_send_command+0x24/0x44
>   Modules linked in:
>   CPU: 0 PID: 538 Comm: mmcqd/0 Not tainted 4.14.76 #14
>   Hardware name: Atmel AT91SAM9
>   [<c000fccc>] (unwind_backtrace) from [<c000d3dc>] (show_stack+0x10/0x14)
>   [<c000d3dc>] (show_stack) from [<c0017644>] (__warn+0xd8/0xf4)
>   [<c0017644>] (__warn) from [<c0017704>] (warn_slowpath_null+0x1c/0x24)
>   [<c0017704>] (warn_slowpath_null) from [<c033bb9c>] (atmci_send_command+0x24/0x44)
>   [<c033bb9c>] (atmci_send_command) from [<c033e984>] (atmci_start_request+0x1f4/0x2dc)
>   [<c033e984>] (atmci_start_request) from [<c033f3b4>] (atmci_request+0xf0/0x164)
>   [<c033f3b4>] (atmci_request) from [<c0327108>] (mmc_start_request+0x280/0x2d0)
>   [<c0327108>] (mmc_start_request) from [<c032800c>] (mmc_start_areq+0x230/0x330)
>   [<c032800c>] (mmc_start_areq) from [<c03366f8>] (mmc_blk_issue_rw_rq+0xc4/0x310)
>   [<c03366f8>] (mmc_blk_issue_rw_rq) from [<c03372c4>] (mmc_blk_issue_rq+0x118/0x5ac)
>   [<c03372c4>] (mmc_blk_issue_rq) from [<c033781c>] (mmc_queue_thread+0xc4/0x118)
>   [<c033781c>] (mmc_queue_thread) from [<c002daf8>] (kthread+0x100/0x118)
>   [<c002daf8>] (kthread) from [<c000a580>] (ret_from_fork+0x14/0x34)
>   ---[ end trace 594371ddfa284bd6 ]---
>
> This is:
>   WARN_ON(host->cmd);
>
> This was fixed on our board by letting atmci_request_end determine what
> state we are in. Instead of unconditionally setting it to STATE_IDLE on
> STATE_END_REQUEST.

I am sure this change is just papering over much more bigger issues
[1], which are present when using one host for multiple slots.

In either case, as it seems to improve things, I have queued it up for
4.21, thanks!

Kind regards
Uffe

[1]
https://www.spinics.net/lists/linux-mmc/msg49815.html

>
> Signed-off-by: Jonas Danielsson <jonas@orbital-systems.com>
> ---
>  drivers/mmc/host/atmel-mci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index 0a0ebf3a0..c8a591d8a 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -1954,13 +1954,14 @@ static void atmci_tasklet_func(unsigned long priv)
>                         }
>
>                         atmci_request_end(host, host->mrq);
> -                       state = STATE_IDLE;
> +                       goto unlock; /* atmci_request_end() sets host->state */
>                         break;
>                 }
>         } while (state != prev_state);
>
>         host->state = state;
>
> +unlock:
>         spin_unlock(&host->lock);
>  }
>
> --
> 2.17.2
>