On Tue, 19 Mar 2019 at 10:14, Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> wrote: > > Move it from dw_mci_start_command() to dw_mci_request(). > Then dw_mci_wait_while_busy() isn't called with host's > lock hold. So, I decided to have a closer look at this. > > Signed-off-by: Shawn Lin <shawn.lin@xxxxxxxxxxxxxx> > Tested-by: Ziyuan Xu <xzy.xu@xxxxxxxxxxxxxx> > --- > > Changes in v3: None > Changes in v2: None > > drivers/mmc/host/dw_mmc.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c > index 80dc2fd..703dedf 100644 > --- a/drivers/mmc/host/dw_mmc.c > +++ b/drivers/mmc/host/dw_mmc.c > @@ -426,7 +426,6 @@ static void dw_mci_start_command(struct dw_mci *host, > > mci_writel(host, CMDARG, cmd->arg); > wmb(); /* drain writebuffer */ > - dw_mci_wait_while_busy(host, cmd_flags); > > mci_writel(host, CMD, cmd_flags | SDMMC_CMD_START); > > @@ -1419,6 +1418,10 @@ static void dw_mci_request(struct mmc_host *mmc, struct mmc_request *mrq) > return; > } > > + if ((mrq->cmd->opcode != MMC_SEND_STATUS && mrq->cmd->data) && > + !(mrq->cmd->opcode == SD_SWITCH_VOLTAGE)) > + dw_mci_wait_while_busy(host, SDMMC_CMD_PRV_DAT_WAIT); > + This looks weird, because according to the change-log it sounds like you are moving things around to just avoid having the "lock" held. To me, there seems to be more changes because of the new checks for the "opcode" above, no? Moreover, dw_mci_wait_while_busy() is also called from mci_send_cmd(), which means it may still be called with the "lock" is held. That is really confusing and needs more explanation. > spin_lock_bh(&host->lock); > > dw_mci_queue_request(host, slot, mrq); > -- > 1.9.1 > > > Finally, I am not sure I understand why dw_mci_wait_while_busy() is called before starting a command, at all. That seems wrong to me. Instead, shouldn't we look at cmd->flags & MMC_RSP_BUSY, for the request in question and don't call mmc_request_done() for it, before the card have stop signaled busy. At least that the behavior the mmc core expects by the driver. Kind regards Uffe
