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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to