Hi, On Tue, Oct 30, 2012 at 1:19 PM, Konstantin Dorfman <kdorfman@xxxxxxxxxxxxxx> wrote: > Hello, > > On 10/29/2012 11:40 PM, Per Forlin wrote: >> Hi, >> >> I would like to move the focus of my concerns from root cause analysis >> to the actual patch, >> My first reflection is that the patch is relatively complex and some >> of the code looks duplicated. >> Would it be possible to simplify it and re-use the current execution flow. >> >> Is the following flow feasible? >> >> in mmc_start_req(): >> -------------- >> if (rqc == NULL && not_resend) >> wait_for_both_mmc_and_arrival_of_new_req >> else >> wait_only_for_mmc >> >> if (arrival_of_new_req) { >> set flag to indicate fetch-new_req >> return NULL; >> } >> ----------------- >> >> in queue.c: >> if (fetch-new_req) >> don't overwrite previous req. >> >> BR >> Per > > You are talking about using both waiting mechanisms, old (wait on > completion) and new - wait_event_interruptible()? But how done() > callback, called on host controller irq context, will differentiate > which one is relevant for the request? > > I think it is better to use single wait point for mmc thread. > > Also in future plans to add second reason to wake up mmc thread from > waiting: this is urgent_request - this notification about next-to-fetch > read request, that should be executed out-of-order, using new eMMC HPI > feature (to stop currently running request). > > I will publish this patch soon. > > Your idea with fetch_new_req flag is good, I'll try to simplify current > patch and lets talk again. > I have not thought this through entirely. But it would be nice to add a new func() in areq to add support for this a new mechanism. If the func() is NULL the normal flow will be executed. This could be used in the SDIO case too. At least it should be possible to use only the core API and it still make sense without a mmc block driver on top. How to implement this wait function? There should be one single wait point, I agree. One could share the mmc completion perhaps through the new func(). This mean both mmc-queue and mmc-host and completes the same completion. When waking up from the completion one needs to stop the new func-wait-point and check if a new request is available. If yes, fetch a new request and next time don't re-init the completion (this would overwrite the mmc-host complete value). Let's talk again when you have a new patch set. BR Per > Thanks, > -- > Konstantin Dorfman, > QUALCOMM ISRAEL, on behalf of Qualcomm Innovation Center, > Inc. is a member of Code Aurora Forum, > hosted by The Linux Foundation -- 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