RE: [PATCH v1 1/1] mmc: core: hs200 fixes

["Subhash Jadavani" <subhashj@codeaurora.org>]

> -----Original Message-----
> From: Girish K S [mailto:girish.shivananjappa@linaro.org]
> Sent: Tuesday, March 06, 2012 5:39 PM
> To: Subhash Jadavani
> Cc: linux-mmc@vger.kernel.org; linux-arm-msm@vger.kernel.org;
> cjb@laptop.org
> Subject: Re: [PATCH v1 1/1] mmc: core: hs200 fixes
> 
> On 6 March 2012 17:25, Subhash Jadavani <subhashj@codeaurora.org>
> wrote:
> > Hi Girish,
> >
> >> -----Original Message-----
> >> From: linux-arm-msm-owner@vger.kernel.org [mailto:linux-arm-msm-
> >> owner@vger.kernel.org] On Behalf Of Girish K S
> >> Sent: Tuesday, March 06, 2012 3:30 PM
> >> To: Subhash Jadavani
> >> Cc: linux-mmc@vger.kernel.org; linux-arm-msm@vger.kernel.org;
> >> cjb@laptop.org
> >> Subject: Re: [PATCH v1 1/1] mmc: core: hs200 fixes
> >>
> >> On 6 March 2012 12:54, Subhash Jadavani <subhashj@codeaurora.org>
> >> wrote:
> >> > This patch fixes following issues when HS200 is enabled:
> >> >
> >> > 1. If executing_tuning() host ops is called without
> >> > mmc_host_clk_hold(),
> >> >   card clocks might get turned off (if MMC_CLK_GATING is enabled)
> >> >   while execute_tuning() is in progress. So this patch makes sure
> >> >   that execute_tuning() is called with mmc_host_clk_hold().
> >> >
> >> > 2. If host timing mode is set to HS200 mode, there should not be
> >> >   any communication with the card until execute_tuning() is
completed.
> >> >   But there is a chance that CMD6 might be sent to enable set
> >> > HPI_EN
> >> >   (of HPI_MGMT field in EXT_CSD) before execute_tuning() is called.
> >> >   So this patch moves this operation after execute_tuning() is
> > completed.
> >> >
> >> > Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
> >> > ---
> >> >  drivers/mmc/core/mmc.c |   37
> >> > ++++++++++++++++++++-----------------
> >> >  1 files changed, 20 insertions(+), 17 deletions(-)
> >> >
> >> > diff --git a/drivers/mmc/core/mmc.c b/drivers/mmc/core/mmc.c index
> >> > a480663..08574ae 100644
> >> > --- a/drivers/mmc/core/mmc.c
> >> > +++ b/drivers/mmc/core/mmc.c
> >> > @@ -1030,22 +1030,6 @@ static int mmc_init_card(struct mmc_host
> >> > *host,
> >> > u32 ocr,
> >> >        }
> >> >
> >> >        /*
> >> > -        * Enable HPI feature (if supported)
> >> > -        */
> >> > -       if (card->ext_csd.hpi) {
> >> > -               err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >> > -                       EXT_CSD_HPI_MGMT, 1, 0);
> >> > -               if (err && err != -EBADMSG)
> >> > -                       goto free_card;
> >> > -               if (err) {
> >> > -                       pr_warning("%s: Enabling HPI failed\n",
> >> > -                                  mmc_hostname(card->host));
> >> > -                       err = 0;
> >> > -               } else
> >> > -                       card->ext_csd.hpi_en = 1;
> >> > -       }
> >> > -
> >> > -       /*
> >> >         * Compute bus speed.
> >> >         */
> >> >        max_dtr = (unsigned int)-1;
> >> > @@ -1094,9 +1078,12 @@ static int mmc_init_card(struct mmc_host
> >> > *host,
> >> > u32 ocr,
> >> >                 * 4. execute tuning for HS200
> >> >                 */
> >> >                if ((host->caps2 & MMC_CAP2_HS200) &&
> >> > -                   card->host->ops->execute_tuning)
> >> > +                   card->host->ops->execute_tuning) {
> >> > +                       mmc_host_clk_hold(card->host);
> >> >                        err =
> >> > card->host->ops->execute_tuning(card->host,
> >> >                                MMC_SEND_TUNING_BLOCK_HS200);
> >> > +                       mmc_host_clk_release(card->host);
> >> > +               }
> >> >                if (err) {
> >> >                        pr_warning("%s: tuning execution failed\n",
> >> >                                   mmc_hostname(card->host)); @@
> >> > -1216,6 +1203,22 @@ static int mmc_init_card(struct mmc_host *host,
> >> > u32 ocr,
> >> >        }
> >> >
> >> >        /*
> >> > +        * Enable HPI feature (if supported)
> >> > +        */
> >> > +       if (card->ext_csd.hpi) {
> >> > +               err = mmc_switch(card, EXT_CSD_CMD_SET_NORMAL,
> >> > +                       EXT_CSD_HPI_MGMT, 1, 0);
> >
> >> i think timeout value should be generic timeout value. Rest all looks
> > fine.
> >
> > This is not introduced in this patch as I just moved this mmc_switch()
> > as it is. Do you still want me to fix this timeout value as part of this
patch?
> I think so. Other than sanitize/bkops/cache ctrl other commands should use
> generic time out. Please change it.

Ok. I will do it in V2.

> >
> >> > +               if (err && err != -EBADMSG)
> >> > +                       goto free_card;
> >> > +               if (err) {
> >> > +                       pr_warning("%s: Enabling HPI failed\n",
> >> > +                                  mmc_hostname(card->host));
> >> > +                       err = 0;
> >> > +               } else
> >> > +                       card->ext_csd.hpi_en = 1;
> >> > +       }
> >> > +
> >> > +       /*
> >> >         * If cache size is higher than 0, this indicates
> >> >         * the existence of cache and it can be turned on.
> >> >         */
> >> > --
> >> > 1.7.1.1
> >> >
> >> > --
> >> > Sent by a consultant of the Qualcomm Innovation Center, Inc.
> >> > The Qualcomm Innovation Center, Inc. is a member of the Code Aurora
> >> Forum.
> >> >
> >> --
> >> To unsubscribe from this list: send the line "unsubscribe
linux-arm-msm"
> > in
> >> the body of a message to majordomo@vger.kernel.org More majordomo
> >> info at  http://vger.kernel.org/majordomo-info.html
> >