From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754508Ab1FVIx2 (ORCPT ); Wed, 22 Jun 2011 04:53:28 -0400 Received: from na3sys009aog102.obsmtp.com ([74.125.149.69]:57789 "EHLO na3sys009aog102.obsmtp.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753364Ab1FVIx1 convert rfc822-to-8bit (ORCPT ); Wed, 22 Jun 2011 04:53:27 -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 14:23:24 +0530 Message-ID: Subject: Re: [PATCH v7 01/11] mmc: add non-blocking mmc request function From: "S, Venkatraman" To: Per Forlin 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 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 ?' Regards, Venkat. From mboxrd@z Thu Jan 1 00:00:00 1970 From: svenkatr@ti.com (S, Venkatraman) Date: Wed, 22 Jun 2011 14:23:24 +0530 Subject: [PATCH v7 01/11] mmc: add non-blocking mmc request function In-Reply-To: References: <1308699521-20556-1-git-send-email-per.forlin@linaro.org> <1308699521-20556-2-git-send-email-per.forlin@linaro.org> Message-ID: To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 ?' Regards, Venkat.