On 23 January 2014 15:29, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: > On 23/01/14 15:21, Ulf Hansson wrote: >> On 23 January 2014 11:09, Adrian Hunter <adrian.hunter@xxxxxxxxx> wrote: >>> On 22/01/14 17:00, Ulf Hansson wrote: >>>> Hosts supporting MMC_CAP_WAIT_WHILE_BUSY shall not be waiting for busy >>>> detection completion in the recovery path, which were the case when >>>> using R1B response. >>>> >>>> Start using R1 as response instead to align behavior, no matter if >>>> MMC_CAP_WAIT_WHILE_BUSY is supported or not. >>> >>> This does not make sense to me. If you are sending a STOP command you >>> should use the correct response type. R1B should be OK here because the >>> card should release the busy signal in any case except failure. >> >> For those hosts not supporting MMC_CAP_WAIT_WHILE_BUSY a R1B is >> assumed to be treated same as an R1, which means there are no busy >> detection handled in the host. > > That is not entirely true. For hosts that do not set > MMC_CAP_WAIT_WHILE_BUSY we don't know if they wait or not. I imagine most > do because it is more efficient, but the kernel has always been programmed > to poll the status anyway so you can't tell from the code. You are right, we can't know - unless we dive in into each host driver and check. Surely there could be more than omap_hsmmc and sdhci that support this. Still I think we need to conclude on how to go forward with MMC_CAP_WAIT_WHILE_BUSY, since at the moment it seems a bit of a mess. Obviously we need to be careful to not break anything. > > MMC_CAP_WAIT_WHILE_BUSY was one of my inventions I am afraid. If I recall > correctly it was mainly due to the SLEEP command because you can't poll in > that case and you don't want to delay the system from sleeping - if you are > certain that the controller has waited for busy to de-assert (i.e. > MMC_CAP_WAIT_WHILE_BUSY) then you can exit immediately. I think MMC_CAP_WAIT_WHILE_BUSY was a needed feature, now we only have to make it more mature. :-) > >> >> mmc_blk_cmd_recovery() is the only caller of the send_stop() function. >> Additionally it does not care about to handle busy detection with >> CDM13 polling. >> >> Now, since most hosts don't support MMC_CAP_WAIT_WHILE_BUSY which >> means there no busy detection done, I wanted to align to this >> behaviour - no matter if the host can do HW busy detection or not. >> >> I am not saying this is how it must be done, just trying to provide >> you with some more reasons to why I wanted to change. >> >> If we instead decide keep the R1B for send_stop(), we should implement >> CMD 13 polling to meet the same behaviour for hosts not supporting >> MMC_CAP_WAIT_WHILE_BUSY. In this scenario, we need to set a select a >> busy timeout, do you have any suggestion of what would be a reasonable >> value for it? > > It is hard to tell if waiting is ever going to help more than hinder, so I > would not change this. Fair enough, but certainly we should implement a CMD13 polling mechanism - to align behaviour. Are you then also indirectly suggesting that not specficing "cmd.busy_timeout" should be interpreted by the host as "use whatever timeout you want"? Do note, there are another scenario, which also don't specify a busy timeout, which is when we have used an open ended WRITE transmission and using CMD12 to finalize it. But, in this scenario we do polling with CMD13, also without a timeout. So at least the behaviour are aligned here, but still no timeout specified. > >> >> Kind regards >> Ulf Hansson >> >>> >>>> >>>> Signed-off-by: Ulf Hansson <ulf.hansson@xxxxxxxxxx> >>>> --- >>>> drivers/mmc/card/block.c | 2 +- >>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/mmc/card/block.c b/drivers/mmc/card/block.c >>>> index 87cd2b0..74169fa 100644 >>>> --- a/drivers/mmc/card/block.c >>>> +++ b/drivers/mmc/card/block.c >>>> @@ -728,7 +728,7 @@ static int send_stop(struct mmc_card *card, u32 *status) >>>> int err; >>>> >>>> cmd.opcode = MMC_STOP_TRANSMISSION; >>>> - cmd.flags = MMC_RSP_SPI_R1B | MMC_RSP_R1B | MMC_CMD_AC; >>>> + cmd.flags = MMC_RSP_SPI_R1 | MMC_RSP_R1 | MMC_CMD_AC; >>>> err = mmc_wait_for_cmd(card->host, &cmd, 5); >>>> if (err == 0) >>>> *status = cmd.resp[0]; >>>> >>> >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-mmc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
