RE: [PATCH v2 10/12] mmc: sdhci: enable preset value after uhs initialization

[<subhashj@codeaurora.org>]

> -----Original Message-----
> From: Nath, Arindam [mailto:Arindam.Nath@amd.com]
> Sent: Thursday, March 10, 2011 7:46 PM
> To: subhashj@codeaurora.org; cjb@laptop.org
> Cc: zhangfei.gao@gmail.com; prakity@marvell.com; linux-
> mmc@vger.kernel.org; Su, Henry; Lu, Aaron; anath.amd@gmail.com
> Subject: RE: [PATCH v2 10/12] mmc: sdhci: enable preset value after uhs
> initialization
> 
> 
> 
> > -----Original Message-----
> > From: subhashj@codeaurora.org [mailto:subhashj@codeaurora.org]
> > Sent: Thursday, March 10, 2011 7:43 PM
> > To: Nath, Arindam; cjb@laptop.org
> > Cc: zhangfei.gao@gmail.com; prakity@marvell.com; linux-
> > mmc@vger.kernel.org; Su, Henry; Lu, Aaron; anath.amd@gmail.com
> > Subject: RE: [PATCH v2 10/12] mmc: sdhci: enable preset value after
> uhs
> > initialization
> >
> >
> >
> > > -----Original Message-----
> > > From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> > > owner@vger.kernel.org] On Behalf Of Nath, Arindam
> > > Sent: Thursday, March 10, 2011 7:38 PM
> > > To: subhashj@codeaurora.org; cjb@laptop.org
> > > Cc: zhangfei.gao@gmail.com; prakity@marvell.com; linux-
> > > mmc@vger.kernel.org; Su, Henry; Lu, Aaron; anath.amd@gmail.com
> > > Subject: RE: [PATCH v2 10/12] mmc: sdhci: enable preset value after
> > uhs
> > > initialization
> > >
> > > > -----Original Message-----
> > > > From: subhashj@codeaurora.org [mailto:subhashj@codeaurora.org]
> > > > Sent: Thursday, March 10, 2011 7:34 PM
> > > > To: Nath, Arindam; cjb@laptop.org
> > > > Cc: zhangfei.gao@gmail.com; prakity@marvell.com; linux-
> > > > mmc@vger.kernel.org; Su, Henry; Lu, Aaron; anath.amd@gmail.com
> > > > Subject: RE: [PATCH v2 10/12] mmc: sdhci: enable preset value
> after
> > > uhs
> > > > initialization
> > > >
> > > >
> > > >
> > > > > -----Original Message-----
> > > > > From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> > > > > owner@vger.kernel.org] On Behalf Of Nath, Arindam
> > > > > Sent: Thursday, March 10, 2011 7:20 PM
> > > > > To: subhashj@codeaurora.org; cjb@laptop.org
> > > > > Cc: zhangfei.gao@gmail.com; prakity@marvell.com; linux-
> > > > > mmc@vger.kernel.org; Su, Henry; Lu, Aaron; anath.amd@gmail.com
> > > > > Subject: RE: [PATCH v2 10/12] mmc: sdhci: enable preset value
> > after
> > > > uhs
> > > > > initialization
> > > > >
> > > > > > -----Original Message-----
> > > > > > From: subhashj@codeaurora.org
> [mailto:subhashj@codeaurora.org]
> > > > > > Sent: Thursday, March 10, 2011 7:15 PM
> > > > > > To: Nath, Arindam; cjb@laptop.org
> > > > > > Cc: zhangfei.gao@gmail.com; prakity@marvell.com; linux-
> > > > > > mmc@vger.kernel.org; Su, Henry; Lu, Aaron;
> anath.amd@gmail.com
> > > > > > Subject: RE: [PATCH v2 10/12] mmc: sdhci: enable preset value
> > > after
> > > > > uhs
> > > > > > initialization
> > > > > >
> > > > > >
> > > > > >
> > > > > > > -----Original Message-----
> > > > > > > From: linux-mmc-owner@vger.kernel.org [mailto:linux-mmc-
> > > > > > > owner@vger.kernel.org] On Behalf Of Nath, Arindam
> > > > > > > Sent: Thursday, March 10, 2011 7:01 PM
> > > > > > > To: subhashj@codeaurora.org; cjb@laptop.org
> > > > > > > Cc: zhangfei.gao@gmail.com; prakity@marvell.com; linux-
> > > > > > > mmc@vger.kernel.org; Su, Henry; Lu, Aaron;
> > anath.amd@gmail.com
> > > > > > > Subject: RE: [PATCH v2 10/12] mmc: sdhci: enable preset
> value
> > > > after
> > > > > > uhs
> > > > > > > initialization
> > > > > > >
> > > > > > > Hi Subhash,
> > > > > > >
> > > > > > >
> > > > > > > > -----Original Message-----
> > > > > > > > From: subhashj@codeaurora.org
> > > [mailto:subhashj@codeaurora.org]
> > > > > > > > Sent: Thursday, March 10, 2011 6:54 PM
> > > > > > > > To: Nath, Arindam; cjb@laptop.org
> > > > > > > > Cc: zhangfei.gao@gmail.com; prakity@marvell.com; linux-
> > > > > > > > mmc@vger.kernel.org; Su, Henry; Lu, Aaron;
> > > anath.amd@gmail.com
> > > > > > > > Subject: RE: [PATCH v2 10/12] mmc: sdhci: enable preset
> > value
> > > > > after
> > > > > > > uhs
> > > > > > > > initialization
> > > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > > -----Original Message-----
> > > > > > > > > From: linux-mmc-owner@vger.kernel.org [mailto:linux-
> mmc-
> > > > > > > > > owner@vger.kernel.org] On Behalf Of Arindam Nath
> > > > > > > > > Sent: Friday, March 04, 2011 5:03 PM
> > > > > > > > > To: cjb@laptop.org
> > > > > > > > > Cc: zhangfei.gao@gmail.com; prakity@marvell.com;
> > > > > > > > > subhashj@codeaurora.org; linux-mmc@vger.kernel.org;
> > > > > > > henry.su@amd.com;
> > > > > > > > > aaron.lu@amd.com; anath.amd@gmail.com; Arindam Nath
> > > > > > > > > Subject: [PATCH v2 10/12] mmc: sdhci: enable preset
> value
> > > > after
> > > > > > uhs
> > > > > > > > > initialization
> > > > > > > > >
> > > > > > > > > According to the Host Controller spec v3.00, setting
> > Preset
> > > > > Value
> > > > > > > > > Enable
> > > > > > > > > in the Host Control2 register lets SDCLK Frequency
> > Select,
> > > > > Clock
> > > > > > > > > Generator
> > > > > > > > > Select and Driver Strength Select to be set
> automatically
> > > by
> > > > > the
> > > > > > > Host
> > > > > > > > > Controller based on the UHS-I mode set. This patch
> > enables
> > > > this
> > > > > > > > > feature.
> > > > > > > > > We also reset Preset Value Enable when the card is
> > removed
> > > > from
> > > > > > the
> > > > > > > > > slot.
> > > > > > > >
> > > > > > > > What does this preset_value means? Is this defined in
> > SD3.01
> > > > > spec?
> > > > > > I
> > > > > > > > think
> > > > > > > > it looks like specific to your controller (sdhci). If
> it's
> > > > > specific
> > > > > > > to
> > > > > > > > your
> > > > > > > > controller then this new mmc_ops should not be added.
> > > > > > >
> > > > > > > Preset Value Enable is very much part of the standard SD
> Host
> > > > > > > Controller spec v3.00. Please refer to Host Control 2
> > register.
> > > > > >
> > > > > > I don't think all controllers should be compliant to SD host
> > > > > controller
> > > > > > spec. I don't think controller on our chips has something
> like
> > > > this.
> > > > >
> > > > > But from the spec it looks as if controllers conforming to
> v3.00
> > > > should
> > > > > provide this feature. There is no other bit anywhere else in
> the
> > > spec
> > > > > which can indicate support for Preset Value Enable except for
> the
> > > > > controller version number.
> > > >
> > > > Actually, I am not aware about this controller spec v3.00. can
> you
> > > > please
> > > > point me to the link for this spec (if it's available)?
> > >
> > > The spec is named "SD Host Controller Standard Specification
> > Ver3.00".
> > > I think it can be downloaded from sdcard.org, but you will need to
> > pay
> > > for the license.
> >
> >
> > Thanks. I just read from here:
> > http://www.sdcard.org/developers/tech/host_controller/. I think this
> > spec is
> > just recommendation for implementing Host controller Hw.
> > It doesn't mean all Host controllers on all different chipset should
> > have
> > same register interface as mentioned by the spec. I don't think
> > controller
> > on our chipset complies to this. Controller just complies to provide
> > all SD
> > Physical layer spec v3.00/v3.01.
> 
> In that case I think it is safe enough to check the version of Host
> Controller for v3.00 before enabling Preset Value.

I don't think controller on our chipset is compliant to any of the host
controller spec (2.0 or 3.0).



> 
> Thanks,
> Arindam
> 
> >
> >
> > >
> > > Thanks,
> > > Arindam
> > >
> > > >
> > > >
> > > > >
> > > > > Thanks,
> > > > > Arindam
> > > > >
> > > > > >
> > > > > > >
> > > > > > > Thanks,
> > > > > > > Arindam
> > > > > > >
> > > > > > > >
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Signed-off-by: Arindam Nath <arindam.nath@amd.com>
> > > > > > > > > ---
> > > > > > > > >  drivers/mmc/core/sd.c    |    7 +++++++
> > > > > > > > >  drivers/mmc/host/sdhci.c |   36
> > > > > > > ++++++++++++++++++++++++++++++++++++
> > > > > > > > >  include/linux/mmc/host.h |    1 +
> > > > > > > > >  3 files changed, 44 insertions(+), 0 deletions(-)
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/mmc/core/sd.c
> > b/drivers/mmc/core/sd.c
> > > > > > > > > index 1e2d157..ae7a771 100644
> > > > > > > > > --- a/drivers/mmc/core/sd.c
> > > > > > > > > +++ b/drivers/mmc/core/sd.c
> > > > > > > > > @@ -962,6 +962,13 @@ static int mmc_sd_init_card(struct
> > > > > mmc_host
> > > > > > > > *host,
> > > > > > > > > u32 ocr,
> > > > > > > > >  		}
> > > > > > > > >  	}
> > > > > > > > >
> > > > > > > > > +	/*
> > > > > > > > > +	 * Since initialization is now complete, enable
> > > preset
> > > > > > > > > +	 * value registers.
> > > > > > > > > +	 */
> > > > > > > > > +	if (host->ops->enable_preset_value)
> > > > > > > > > +		host->ops->enable_preset_value(host);
> > > > > > > > > +
> > > > > > > > >  	host->card = card;
> > > > > > > > >  	return 0;
> > > > > > > > >
> > > > > > > > > diff --git a/drivers/mmc/host/sdhci.c
> > > > > b/drivers/mmc/host/sdhci.c
> > > > > > > > > index 8f4f102..1f6e4ad 100644
> > > > > > > > > --- a/drivers/mmc/host/sdhci.c
> > > > > > > > > +++ b/drivers/mmc/host/sdhci.c
> > > > > > > > > @@ -1602,6 +1602,40 @@ out:
> > > > > > > > >  	spin_unlock_irqrestore(&host->lock, flags);
> > > > > > > > >  }
> > > > > > > > >
> > > > > > > > > +static void sdhci_enable_preset_value(struct mmc_host
> > > *mmc)
> > > > > > > > > +{
> > > > > > > > > +	struct sdhci_host *host;
> > > > > > > > > +	u16 ctrl;
> > > > > > > > > +	unsigned long flags;
> > > > > > > > > +
> > > > > > > > > +	host = mmc_priv(mmc);
> > > > > > > > > +
> > > > > > > > > +	/* Host Controller v3.00 defines preset value
> > > registers */
> > > > > > > > > +	if (host->version < SDHCI_SPEC_300)
> > > > > > > > > +		return;
> > > > > > > > > +
> > > > > > > > > +	spin_lock_irqsave(&host->lock, flags);
> > > > > > > > > +
> > > > > > > > > +	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> > > > > > > > > +	ctrl |= SDHCI_CTRL_PRESET_VAL_ENABLE;
> > > > > > > > > +	sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> > > > > > > > > +
> > > > > > > > > +	spin_unlock_irqrestore(&host->lock, flags);
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > > +static void sdhci_disable_preset_value(struct
> sdhci_host
> > > > > *host)
> > > > > > > > > +{
> > > > > > > > > +	u16 ctrl;
> > > > > > > > > +
> > > > > > > > > +	/* Only for Host Controller version >= v3.00 */
> > > > > > > > > +	if (host->version < SDHCI_SPEC_300)
> > > > > > > > > +		return;
> > > > > > > > > +
> > > > > > > > > +	ctrl = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> > > > > > > > > +	ctrl &= ~SDHCI_CTRL_PRESET_VAL_ENABLE;
> > > > > > > > > +	sdhci_writew(host, ctrl, SDHCI_HOST_CONTROL2);
> > > > > > > > > +}
> > > > > > > > > +
> > > > > > > > >  static const struct mmc_host_ops sdhci_ops = {
> > > > > > > > >  	.request	= sdhci_request,
> > > > > > > > >  	.set_ios	= sdhci_set_ios,
> > > > > > > > > @@ -1610,6 +1644,7 @@ static const struct mmc_host_ops
> > > > > sdhci_ops
> > > > > > =
> > > > > > > {
> > > > > > > > >  	.start_signal_voltage_switch	=
> > > > > > > > > sdhci_start_signal_voltage_switch,
> > > > > > > > >  	.get_max_current_180		=
> > > sdhci_get_max_current_180,
> > > > > > > > >  	.execute_tuning			=
sdhci_execute_tuning,
> > > > > > > > > +	.enable_preset_value		=
> > > sdhci_enable_preset_value,
> > > > > > > > >  };
> > > > > > > > >
> > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
> /**********************************************************************
> > > > > > > > > *******\
> > > > > > > > > @@ -1920,6 +1955,7 @@ static irqreturn_t sdhci_irq(int
> > irq,
> > > > > void
> > > > > > > > > *dev_id)
> > > > > > > > >  		sdhci_writel(host, intmask &
> > > (SDHCI_INT_CARD_INSERT
> > > > |
> > > > > > > > >  			SDHCI_INT_CARD_REMOVE),
> > > SDHCI_INT_STATUS);
> > > > > > > > >  		tasklet_schedule(&host->card_tasklet);
> > > > > > > > > +		sdhci_disable_preset_value(host);
> > > > > > > > >  	}
> > > > > > > > >
> > > > > > > > >  	intmask &= ~(SDHCI_INT_CARD_INSERT |
> > > > > > SDHCI_INT_CARD_REMOVE);
> > > > > > > > > diff --git a/include/linux/mmc/host.h
> > > > > b/include/linux/mmc/host.h
> > > > > > > > > index 651e40b..e63e063 100644
> > > > > > > > > --- a/include/linux/mmc/host.h
> > > > > > > > > +++ b/include/linux/mmc/host.h
> > > > > > > > > @@ -130,6 +130,7 @@ struct mmc_host_ops {
> > > > > > > > >  	int	(*start_signal_voltage_switch)(struct
mmc_host
> > > > > > *host);
> > > > > > > > >  	int	(*get_max_current_180)(struct mmc_host
*mmc);
> > > > > > > > >  	void	(*execute_tuning)(struct mmc_host *host);
> > > > > > > > > +	void	(*enable_preset_value)(struct mmc_host
*host);
> > > > > > > > >  };
> > > > > > > > >
> > > > > > > > >  struct mmc_card;
> > > > > > > > > --
> > > > > > > > > 1.7.1
> > > > > > > > >
> > > > > > > > > --
> > > > > > > > > To unsubscribe from this list: send the line
> "unsubscribe
> > > > > linux-
> > > > > > > mmc"
> > > > > > > > in
> > > > > > > > > the body of a message to majordomo@vger.kernel.org
> > > > > > > > > More majordomo info at
> http://vger.kernel.org/majordomo-
> > > > > > info.html
> > > > > > > >
> > > > > > >
> > > > > > >
> > > > > > > --
> > > > > > > To unsubscribe from this list: send the line "unsubscribe
> > > linux-
> > > > > mmc"
> > > > > > in
> > > > > > > the body of a message to majordomo@vger.kernel.org
> > > > > > > More majordomo info at  http://vger.kernel.org/majordomo-
> > > > info.html
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > To unsubscribe from this list: send the line "unsubscribe
> linux-
> > > mmc"
> > > > in
> > > > > the body of a message to majordomo@vger.kernel.org
> > > > > More majordomo info at  http://vger.kernel.org/majordomo-
> > info.html
> > > >
> > >
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-
> mmc"
> > in
> > > the body of a message to majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
>