Hello Guennadi-san,
(2013/06/13 17:33), Guennadi Liakhovetski wrote:
< snip >
>> +static bool sh_mmcif_send_sbc(struct sh_mmcif_host *host,
>> + struct mmc_request *mrq)
>> +{
>> + struct mmc_request req_orig = *mrq;
>> + long time;
>> +
>> + /* Switch the commands around */
>> + mrq->cmd = mrq->sbc;
>> + mrq->sbc = NULL;
>> + mrq->data = NULL;
>> + mrq->stop = NULL;
>> +
>> + /* Send SBC Cmd */
>> + sh_mmcif_start_cmd(host, mrq);
>> +
>> + /* Normal completion time is less than 1us */
>> + time = wait_for_completion_interruptible_timeout(&host->sbc_complete,
>> + host->timeout);
>
> I'm afraid this doesn't look like a correct approach to me. In commit
> f985da1 "mmc: sh_mmcif: process requests asynchronously" I converted the
> driver to not wait inside its .request() method. This your patch makes a
> part of the .request() processing synchronous again by adding a wait to
> it. Besides you're very much special casing the processing of the SBC
> command. I think, it would be better to process it asynchronously too,
> implementing it as a sequence of two requests, similar to how sdhci.c does
> it (see sdhci_request() nearer the end the "if (mrq->sbc...) handling and
> sdhci_finish_command() below the "Finished CMD23, now send actual
> command" comment). Would that be possible to convert this patch to execute
> similarly and to avoid special-casing as much as possible? Just check for
> an SBC in .request(), if there is one send it instead of the proper
> request. Then in completion check, whether it's the SBC that has just
> completed, and if so, now send the actual request.
Thank you for your comment. I should have checked your patch...
I will modify this SBC patch to remove the wait_for_completion...() in
the .request().
Best regards,
Yoshihiro Shimoda
> Thanks
> Guennadi
>
--
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
