Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754854Ab1FVJIK (ORCPT ); Wed, 22 Jun 2011 05:08:10 -0400 Received: from mail-qw0-f46.google.com ([209.85.216.46]:50826 "EHLO mail-qw0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752023Ab1FVJIH convert rfc822-to-8bit (ORCPT ); Wed, 22 Jun 2011 05:08:07 -0400 MIME-Version: 1.0 In-Reply-To: References: <1308699521-20556-1-git-send-email-per.forlin@linaro.org> <1308699521-20556-2-git-send-email-per.forlin@linaro.org> Date: Wed, 22 Jun 2011 11:08:06 +0200 Message-ID: Subject: Re: [PATCH v7 01/11] mmc: add non-blocking mmc request function From: Per Forlin To: "S, Venkatraman" Cc: linaro-dev , Nicolas Pitre , linux-arm-kernel , linux-kernel , linux-mmc , Chris Ball Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7128 Lines: 162 On 22 June 2011 10:53, S, Venkatraman wrote: > On Wed, Jun 22, 2011 at 2:15 PM, Per Forlin wrote: >> On 22 June 2011 09:42, Venkatraman S wrote: >>> On Wed, Jun 22, 2011 at 5:08 AM, Per Forlin wrote: >>>> Previously there has only been one function mmc_wait_for_req() >>>> to start and wait for a request. This patch adds >>>> ?* mmc_start_req() - starts a request wihtout waiting >>>> ? If there is on ongoing request wait for completion >>>> ? of that request and start the new one and return. >>>> ? Does not wait for the new command to complete. >>>> >>>> This patch also adds new function members in struct mmc_host_ops >>>> only called from core.c >>>> ?* pre_req - asks the host driver to prepare for the next job >>>> ?* post_req - asks the host driver to clean up after a completed job >>>> >>>> The intention is to use pre_req() and post_req() to do cache maintenance >>>> while a request is active. pre_req() can be called while a request is active >>>> to minimize latency to start next job. post_req() can be used after the next >>>> job is started to clean up the request. This will minimize the host driver >>>> request end latency. post_req() is typically used before ending the block >>>> request and handing over the buffer to the block layer. >>>> >>>> Add a host-private member in mmc_data to be used by >>>> pre_req to mark the data. The host driver will then >>>> check this mark to see if the data is prepared or not. >>>> >>>> Signed-off-by: Per Forlin >>>> --- >>>> ?drivers/mmc/core/core.c ?| ?110 +++++++++++++++++++++++++++++++++++++++++---- >>>> ?include/linux/mmc/core.h | ? ?6 ++- >>>> ?include/linux/mmc/host.h | ? 21 +++++++++ >>>> ?3 files changed, 126 insertions(+), 11 deletions(-) >>>> >>>> diff --git a/drivers/mmc/core/core.c b/drivers/mmc/core/core.c >>>> index 68091dd..c82fa3b 100644 >>>> --- a/drivers/mmc/core/core.c >>>> +++ b/drivers/mmc/core/core.c >>>> @@ -198,9 +198,106 @@ mmc_start_request(struct mmc_host *host, struct mmc_request *mrq) >>>> >>>> ?static void mmc_wait_done(struct mmc_request *mrq) >>>> ?{ >>>> - ? ? ? complete(mrq->done_data); >>>> + ? ? ? complete(&mrq->completion); >>>> ?} >>>> >>>> +static void __mmc_start_req(struct mmc_host *host, struct mmc_request *mrq) >>>> +{ >>>> + ? ? ? init_completion(&mrq->completion); >>>> + ? ? ? mrq->done = mmc_wait_done; >>>> + ? ? ? mmc_start_request(host, mrq); >>>> +} >>>> + >>>> +static void mmc_wait_for_req_done(struct mmc_host *host, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct mmc_request *mrq) >>>> +{ >>>> + ? ? ? wait_for_completion(&mrq->completion); >>>> +} >>>> + >>>> +/** >>>> + * ? ? mmc_pre_req - Prepare for a new request >>>> + * ? ? @host: MMC host to prepare command >>>> + * ? ? @mrq: MMC request to prepare for >>>> + * ? ? @is_first_req: true if there is no previous started request >>>> + * ? ? ? ? ? ? ? ? ? ? that may run in parellel to this call, otherwise false >>>> + * >>>> + * ? ? mmc_pre_req() is called in prior to mmc_start_req() to let >>>> + * ? ? host prepare for the new request. Preparation of a request may be >>>> + * ? ? performed while another request is running on the host. >>>> + */ >>>> +static void mmc_pre_req(struct mmc_host *host, struct mmc_request *mrq, >>>> + ? ? ? ? ? ? ? ?bool is_first_req) >>>> +{ >>>> + ? ? ? if (host->ops->pre_req) >>>> + ? ? ? ? ? ? ? host->ops->pre_req(host, mrq, is_first_req); >>>> +} >>>> + >>>> +/** >>>> + * ? ? mmc_post_req - Post process a completed request >>>> + * ? ? @host: MMC host to post process command >>>> + * ? ? @mrq: MMC request to post process for >>>> + * ? ? @err: Error, if non zero, clean up any resources made in pre_req >>>> + * >>>> + * ? ? Let the host post process a completed request. Post processing of >>>> + * ? ? a request may be performed while another reuqest is running. >>>> + */ >>>> +static void mmc_post_req(struct mmc_host *host, struct mmc_request *mrq, >>>> + ? ? ? ? ? ? ? ? ? ? ? ?int err) >>>> +{ >>>> + ? ? ? if (host->ops->post_req) >>>> + ? ? ? ? ? ? ? host->ops->post_req(host, mrq, err); >>>> +} >>>> + >>>> +/** >>>> + * ? ? mmc_start_req - start a non-blocking request >>>> + * ? ? @host: MMC host to start command >>>> + * ? ? @areq: async request to start >>>> + * ? ? @error: out parameter returns 0 for success, otherwise non zero >>>> + * >>>> + * ? ? Start a new MMC custom command request for a host. >>>> + * ? ? If there is on ongoing async request wait for completion >>>> + * ? ? of that request and start the new one and return. >>>> + * ? ? Does not wait for the new request to complete. >>>> + * >>>> + * ? ? Returns the completed async request, NULL in case of none completed. >>>> + */ >>>> +struct mmc_async_req *mmc_start_req(struct mmc_host *host, >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? struct mmc_async_req *areq, int *error) >>>> +{ >>>> + ? ? ? int err = 0; >>>> + ? ? ? struct mmc_async_req *data = host->areq; >>>> + >>>> + ? ? ? /* Prepare a new request */ >>>> + ? ? ? if (areq) >>>> + ? ? ? ? ? ? ? mmc_pre_req(host, areq->mrq, !host->areq); >>>> + >>>> + ? ? ? if (host->areq) { >>>> + ? ? ? ? ? ? ? mmc_wait_for_req_done(host, host->areq->mrq); >>>> + ? ? ? ? ? ? ? err = host->areq->err_check(host->card, host->areq); >>>> + ? ? ? ? ? ? ? if (err) { >>>> + ? ? ? ? ? ? ? ? ? ? ? mmc_post_req(host, host->areq->mrq, 0); >>>> + ? ? ? ? ? ? ? ? ? ? ? if (areq) >>>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? mmc_post_req(host, areq->mrq, -EINVAL); >>>> + >>>> + ? ? ? ? ? ? ? ? ? ? ? host->areq = NULL; >>>> + ? ? ? ? ? ? ? ? ? ? ? goto out; >>> In this sequence, would the return value (data) have the previous areq ? >>> Is that intentional - doesn't seem to fit with the description. >> It will return the data that belongs to the completed request. The >> completed request will be the same as the previous request. The >> mmc_start_req will start a new request and return data for the >> completed request, if any. >> > > I meant that in case of an error (err !=0), data is already assigned > to host->areq > and goto out returns 'data'. So my question was 'Does returning a non-null > pointer for a unsuccessful request doesn't fit with the description, does it ?' > Now I get it. Thanks for your patience :) >>>> + struct mmc_async_req *data = host->areq; ... >>>> + mmc_wait_for_req_done(host, host->areq->mrq); >>>> + err = host->areq->err_check(host->card, host->areq); in case of error host->areq is returned. Basically the failing request is returned. block.c needs the failing request in order to do error handling. >>>> + * @error: out parameter returns 0 for success, otherwise non zero This parameter indicates error. Returning NULL is not error. NULL is returned when starting a new request and there is no ongoing (previous) request. Regards, Per -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/