diff mbox

[RESEND] mmc: omap_hsmmc: Enable HSPE bit for high speed cards

Message ID 1351515408-19403-1-git-send-email-gururaja.hebbar@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Hebbar, Gururaja Oct. 29, 2012, 12:56 p.m. UTC
HSMMC IP on AM33xx need a special setting to handle High-speed cards.
Other platforms like TI81xx, OMAP4 may need this as-well. This depends
on the HSMMC IP timing closure done for the high speed cards.

From AM335x TRM (SPRUH73F - 18.3.12 Output Signals Generation)

The MMC/SD/SDIO output signals can be driven on either falling edge or
rising edge depending on the SD_HCTL[2] HSPE bit. This feature allows
to reach better timing performance, and thus to increase data transfer
frequency.

There are few pre-requisites for enabling the HSPE bit
- Controller should support High-Speed-Enable Bit and
- Controller should not be using DDR Mode and
- Controller should advertise that it supports High Speed in
  capabilities register and
- MMC/SD clock coming out of controller > 25MHz

Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
---
Rebased on mmc-next (v3.7.0-rc1)
Only Build tested since EDMA support for AM335x is not yet added

:100644 100644 be76a23... ed271fc... M	Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
:100644 100644 8b4e4f2... 346af5b... M	arch/arm/plat-omap/include/plat/mmc.h
:100644 100644 54bfd0c... 3fd95cb... M	drivers/mmc/host/omap_hsmmc.c
 .../devicetree/bindings/mmc/ti-omap-hsmmc.txt      |    1 +
 arch/arm/plat-omap/include/plat/mmc.h              |    1 +
 drivers/mmc/host/omap_hsmmc.c                      |   30 +++++++++++++++++++-
 3 files changed, 31 insertions(+), 1 deletions(-)

Comments

Felipe Balbi Oct. 29, 2012, 4:17 p.m. UTC | #1
Hi,

On Mon, Oct 29, 2012 at 06:26:48PM +0530, Hebbar, Gururaja wrote:
> HSMMC IP on AM33xx need a special setting to handle High-speed cards.
> Other platforms like TI81xx, OMAP4 may need this as-well. This depends
> on the HSMMC IP timing closure done for the high speed cards.
> 
> From AM335x TRM (SPRUH73F - 18.3.12 Output Signals Generation)
> 
> The MMC/SD/SDIO output signals can be driven on either falling edge or
> rising edge depending on the SD_HCTL[2] HSPE bit. This feature allows
> to reach better timing performance, and thus to increase data transfer
> frequency.
> 
> There are few pre-requisites for enabling the HSPE bit
> - Controller should support High-Speed-Enable Bit and
> - Controller should not be using DDR Mode and
> - Controller should advertise that it supports High Speed in
>   capabilities register and
> - MMC/SD clock coming out of controller > 25MHz
> 
> Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> ---
> Rebased on mmc-next (v3.7.0-rc1)
> Only Build tested since EDMA support for AM335x is not yet added
> 
> :100644 100644 be76a23... ed271fc... M	Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> :100644 100644 8b4e4f2... 346af5b... M	arch/arm/plat-omap/include/plat/mmc.h
> :100644 100644 54bfd0c... 3fd95cb... M	drivers/mmc/host/omap_hsmmc.c
>  .../devicetree/bindings/mmc/ti-omap-hsmmc.txt      |    1 +
>  arch/arm/plat-omap/include/plat/mmc.h              |    1 +
>  drivers/mmc/host/omap_hsmmc.c                      |   30 +++++++++++++++++++-
>  3 files changed, 31 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> index be76a23..ed271fc 100644
> --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> @@ -19,6 +19,7 @@ ti,dual-volt: boolean, supports dual voltage cards
>  "supply-name" examples are "vmmc", "vmmc_aux" etc
>  ti,non-removable: non-removable slot (like eMMC)
>  ti,needs-special-reset: Requires a special softreset sequence
> +ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed

can't you do runtime revision detection for this ?

>  Example:
>  	mmc1: mmc@0x4809c000 {
> diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
> index 8b4e4f2..346af5b 100644
> --- a/arch/arm/plat-omap/include/plat/mmc.h
> +++ b/arch/arm/plat-omap/include/plat/mmc.h
> @@ -126,6 +126,7 @@ struct omap_mmc_platform_data {
>  		/* we can put the features above into this variable */
>  #define HSMMC_HAS_PBIAS		(1 << 0)
>  #define HSMMC_HAS_UPDATED_RESET	(1 << 1)
> +#define HSMMC_HAS_HSPE_SUPPORT	(1 << 2)
>  		unsigned features;
>  
>  		int switch_pin;			/* gpio (card detect) */
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index 54bfd0c..3fd95cb 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -62,6 +62,7 @@
>  
>  #define VS18			(1 << 26)
>  #define VS30			(1 << 25)
> +#define HSS			(1 << 21)
>  #define SDVS18			(0x5 << 9)
>  #define SDVS30			(0x6 << 9)
>  #define SDVS33			(0x7 << 9)
> @@ -89,6 +90,7 @@
>  #define MSBS			(1 << 5)
>  #define BCE			(1 << 1)
>  #define FOUR_BIT		(1 << 1)
> +#define HSPE			(1 << 2)
>  #define DDR			(1 << 19)
>  #define DW8			(1 << 5)
>  #define CC			0x1
> @@ -489,6 +491,7 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
>  	struct mmc_ios *ios = &host->mmc->ios;
>  	unsigned long regval;
>  	unsigned long timeout;
> +	unsigned long clkdiv;
>  
>  	dev_vdbg(mmc_dev(host->mmc), "Set clock to %uHz\n", ios->clock);
>  
> @@ -496,7 +499,8 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
>  
>  	regval = OMAP_HSMMC_READ(host->base, SYSCTL);
>  	regval = regval & ~(CLKD_MASK | DTO_MASK);
> -	regval = regval | (calc_divisor(host, ios) << 6) | (DTO << 16);
> +	clkdiv = calc_divisor(host, ios);
> +	regval = regval | (clkdiv << 6) | (DTO << 16);

not part of $SUBJECT

>  	OMAP_HSMMC_WRITE(host->base, SYSCTL, regval);
>  	OMAP_HSMMC_WRITE(host->base, SYSCTL,
>  		OMAP_HSMMC_READ(host->base, SYSCTL) | ICE);
> @@ -507,6 +511,27 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
>  		&& time_before(jiffies, timeout))
>  		cpu_relax();
>  
> +	/*
> +	 * Enable High-Speed Support
> +	 * Pre-Requisites
> +	 *	- Controller should support High-Speed-Enable Bit
> +	 *	- Controller should not be using DDR Mode
> +	 *	- Controller should advertise that it supports High Speed
> +	 *	  in capabilities register
> +	 *	- MMC/SD clock coming out of controller > 25MHz
> +	 */
> +	if ((mmc_slot(host).features & HSMMC_HAS_HSPE_SUPPORT) &&

is this really needed...

> +	    (ios->timing != MMC_TIMING_UHS_DDR50) &&
> +	    ((OMAP_HSMMC_READ(host->base, CAPA) & HSS) == HSS)) {

... provided you check device's register ?

> +		regval = OMAP_HSMMC_READ(host->base, HCTL);
> +		if (clkdiv && (clk_get_rate(host->fclk)/clkdiv) > 25000000)
> +			regval |= HSPE;
> +		else
> +			regval &= ~HSPE;
> +
> +		OMAP_HSMMC_WRITE(host->base, HCTL, regval);
> +	}
> +
>  	omap_hsmmc_start_clock(host);
>  }
>  
> @@ -1700,6 +1725,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
>  	if (of_find_property(np, "ti,needs-special-reset", NULL))
>  		pdata->slots[0].features |= HSMMC_HAS_UPDATED_RESET;
>  
> +	if (of_find_property(np, "ti,needs-special-hs-handling", NULL))
> +		pdata->slots[0].features |= HSMMC_HAS_HSPE_SUPPORT;
> +
>  	return pdata;
>  }
>  #else
> -- 
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
Hebbar, Gururaja Oct. 31, 2012, 5:17 a.m. UTC | #2
On Mon, Oct 29, 2012 at 21:47:19, Balbi, Felipe wrote:
> Hi,
> 
> On Mon, Oct 29, 2012 at 06:26:48PM +0530, Hebbar, Gururaja wrote:
> > HSMMC IP on AM33xx need a special setting to handle High-speed cards.
> > Other platforms like TI81xx, OMAP4 may need this as-well. This depends
> > on the HSMMC IP timing closure done for the high speed cards.
> > 
> > From AM335x TRM (SPRUH73F - 18.3.12 Output Signals Generation)
> > 
> > The MMC/SD/SDIO output signals can be driven on either falling edge or
> > rising edge depending on the SD_HCTL[2] HSPE bit. This feature allows
> > to reach better timing performance, and thus to increase data transfer
> > frequency.
> > 
> > There are few pre-requisites for enabling the HSPE bit
> > - Controller should support High-Speed-Enable Bit and
> > - Controller should not be using DDR Mode and
> > - Controller should advertise that it supports High Speed in
> >   capabilities register and
> > - MMC/SD clock coming out of controller > 25MHz
> > 
> > Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> > ---
> > Rebased on mmc-next (v3.7.0-rc1)
> > Only Build tested since EDMA support for AM335x is not yet added
> > 
> > :100644 100644 be76a23... ed271fc... M	Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > :100644 100644 8b4e4f2... 346af5b... M	arch/arm/plat-omap/include/plat/mmc.h
> > :100644 100644 54bfd0c... 3fd95cb... M	drivers/mmc/host/omap_hsmmc.c
> >  .../devicetree/bindings/mmc/ti-omap-hsmmc.txt      |    1 +
> >  arch/arm/plat-omap/include/plat/mmc.h              |    1 +
> >  drivers/mmc/host/omap_hsmmc.c                      |   30 +++++++++++++++++++-
> >  3 files changed, 31 insertions(+), 1 deletions(-)
> > 
> > diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > index be76a23..ed271fc 100644
> > --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > @@ -19,6 +19,7 @@ ti,dual-volt: boolean, supports dual voltage cards
> >  "supply-name" examples are "vmmc", "vmmc_aux" etc
> >  ti,non-removable: non-removable slot (like eMMC)
> >  ti,needs-special-reset: Requires a special softreset sequence
> > +ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed
> 
> can't you do runtime revision detection for this ?

Platforms like AM335x & TI814x both use the same IP. 
But only AM335x needed this bit to be set in order to sustain high
speed transfers. This was concluded after AM335x HSMMC IP timing
closure was completed for High speed Cards.

> 
> >  Example:
> >  	mmc1: mmc@0x4809c000 {
> > diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
> > index 8b4e4f2..346af5b 100644
> > --- a/arch/arm/plat-omap/include/plat/mmc.h
> > +++ b/arch/arm/plat-omap/include/plat/mmc.h
> > @@ -126,6 +126,7 @@ struct omap_mmc_platform_data {
> >  		/* we can put the features above into this variable */
> >  #define HSMMC_HAS_PBIAS		(1 << 0)
> >  #define HSMMC_HAS_UPDATED_RESET	(1 << 1)
> > +#define HSMMC_HAS_HSPE_SUPPORT	(1 << 2)
> >  		unsigned features;
> >  
> >  		int switch_pin;			/* gpio (card detect) */
> > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> > index 54bfd0c..3fd95cb 100644
> > --- a/drivers/mmc/host/omap_hsmmc.c
> > +++ b/drivers/mmc/host/omap_hsmmc.c
> > @@ -62,6 +62,7 @@
> >  
> >  #define VS18			(1 << 26)
> >  #define VS30			(1 << 25)
> > +#define HSS			(1 << 21)
> >  #define SDVS18			(0x5 << 9)
> >  #define SDVS30			(0x6 << 9)
> >  #define SDVS33			(0x7 << 9)
> > @@ -89,6 +90,7 @@
> >  #define MSBS			(1 << 5)
> >  #define BCE			(1 << 1)
> >  #define FOUR_BIT		(1 << 1)
> > +#define HSPE			(1 << 2)
> >  #define DDR			(1 << 19)
> >  #define DW8			(1 << 5)
> >  #define CC			0x1
> > @@ -489,6 +491,7 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> >  	struct mmc_ios *ios = &host->mmc->ios;
> >  	unsigned long regval;
> >  	unsigned long timeout;
> > +	unsigned long clkdiv;
> >  
> >  	dev_vdbg(mmc_dev(host->mmc), "Set clock to %uHz\n", ios->clock);
> >  
> > @@ -496,7 +499,8 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> >  
> >  	regval = OMAP_HSMMC_READ(host->base, SYSCTL);
> >  	regval = regval & ~(CLKD_MASK | DTO_MASK);
> > -	regval = regval | (calc_divisor(host, ios) << 6) | (DTO << 16);
> > +	clkdiv = calc_divisor(host, ios);
> > +	regval = regval | (clkdiv << 6) | (DTO << 16);
> 
> not part of $SUBJECT

The output of the calc_divisor() is reused in the below HSPE checking. So
instead of calling it twice I have taken the output of that function into
a variable and reused it.

> 
> >  	OMAP_HSMMC_WRITE(host->base, SYSCTL, regval);
> >  	OMAP_HSMMC_WRITE(host->base, SYSCTL,
> >  		OMAP_HSMMC_READ(host->base, SYSCTL) | ICE);
> > @@ -507,6 +511,27 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> >  		&& time_before(jiffies, timeout))
> >  		cpu_relax();
> >  
> > +	/*
> > +	 * Enable High-Speed Support
> > +	 * Pre-Requisites
> > +	 *	- Controller should support High-Speed-Enable Bit
> > +	 *	- Controller should not be using DDR Mode
> > +	 *	- Controller should advertise that it supports High Speed
> > +	 *	  in capabilities register
> > +	 *	- MMC/SD clock coming out of controller > 25MHz
> > +	 */
> > +	if ((mmc_slot(host).features & HSMMC_HAS_HSPE_SUPPORT) &&
> 
> is this really needed...

As explained above, this is needed only on platforms which requires the
Bit to be set (currently AM335x). Such platforms will set this
parameter in it .dts file

> 
> > +	    (ios->timing != MMC_TIMING_UHS_DDR50) &&
> > +	    ((OMAP_HSMMC_READ(host->base, CAPA) & HSS) == HSS)) {
> 
> ... provided you check device's register ?

As explained above, both TI814x & AM335x support HSS but only AM335x
require HSPE bit set.


Regards, 
Gururaja

> 
> > +		regval = OMAP_HSMMC_READ(host->base, HCTL);
> > +		if (clkdiv && (clk_get_rate(host->fclk)/clkdiv) > 25000000)
> > +			regval |= HSPE;
> > +		else
> > +			regval &= ~HSPE;
> > +
> > +		OMAP_HSMMC_WRITE(host->base, HCTL, regval);
> > +	}
> > +
> >  	omap_hsmmc_start_clock(host);
> >  }
> >  
> > @@ -1700,6 +1725,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
> >  	if (of_find_property(np, "ti,needs-special-reset", NULL))
> >  		pdata->slots[0].features |= HSMMC_HAS_UPDATED_RESET;
> >  
> > +	if (of_find_property(np, "ti,needs-special-hs-handling", NULL))
> > +		pdata->slots[0].features |= HSMMC_HAS_HSPE_SUPPORT;
> > +
> >  	return pdata;
> >  }
> >  #else
> > -- 
> > 1.7.0.4
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> -- 
> balbi
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Felipe Balbi Oct. 31, 2012, 5:44 a.m. UTC | #3
Hi,

On Wed, Oct 31, 2012 at 06:17:02AM +0100, Hebbar, Gururaja wrote:
> On Mon, Oct 29, 2012 at 21:47:19, Balbi, Felipe wrote:
> > Hi,
> > 
> > On Mon, Oct 29, 2012 at 06:26:48PM +0530, Hebbar, Gururaja wrote:
> > > HSMMC IP on AM33xx need a special setting to handle High-speed cards.
> > > Other platforms like TI81xx, OMAP4 may need this as-well. This depends
> > > on the HSMMC IP timing closure done for the high speed cards.
> > > 
> > > From AM335x TRM (SPRUH73F - 18.3.12 Output Signals Generation)
> > > 
> > > The MMC/SD/SDIO output signals can be driven on either falling edge or
> > > rising edge depending on the SD_HCTL[2] HSPE bit. This feature allows
> > > to reach better timing performance, and thus to increase data transfer
> > > frequency.
> > > 
> > > There are few pre-requisites for enabling the HSPE bit
> > > - Controller should support High-Speed-Enable Bit and
> > > - Controller should not be using DDR Mode and
> > > - Controller should advertise that it supports High Speed in
> > >   capabilities register and
> > > - MMC/SD clock coming out of controller > 25MHz
> > > 
> > > Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> > > ---
> > > Rebased on mmc-next (v3.7.0-rc1)
> > > Only Build tested since EDMA support for AM335x is not yet added
> > > 
> > > :100644 100644 be76a23... ed271fc... M	Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > > :100644 100644 8b4e4f2... 346af5b... M	arch/arm/plat-omap/include/plat/mmc.h
> > > :100644 100644 54bfd0c... 3fd95cb... M	drivers/mmc/host/omap_hsmmc.c
> > >  .../devicetree/bindings/mmc/ti-omap-hsmmc.txt      |    1 +
> > >  arch/arm/plat-omap/include/plat/mmc.h              |    1 +
> > >  drivers/mmc/host/omap_hsmmc.c                      |   30 +++++++++++++++++++-
> > >  3 files changed, 31 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > > index be76a23..ed271fc 100644
> > > --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > > +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > > @@ -19,6 +19,7 @@ ti,dual-volt: boolean, supports dual voltage cards
> > >  "supply-name" examples are "vmmc", "vmmc_aux" etc
> > >  ti,non-removable: non-removable slot (like eMMC)
> > >  ti,needs-special-reset: Requires a special softreset sequence
> > > +ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed
> > 
> > can't you do runtime revision detection for this ?
> 
> Platforms like AM335x & TI814x both use the same IP. 
> But only AM335x needed this bit to be set in order to sustain high
> speed transfers. This was concluded after AM335x HSMMC IP timing
> closure was completed for High speed Cards.

fair enough.

> > >  Example:
> > >  	mmc1: mmc@0x4809c000 {
> > > diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
> > > index 8b4e4f2..346af5b 100644
> > > --- a/arch/arm/plat-omap/include/plat/mmc.h
> > > +++ b/arch/arm/plat-omap/include/plat/mmc.h
> > > @@ -126,6 +126,7 @@ struct omap_mmc_platform_data {
> > >  		/* we can put the features above into this variable */
> > >  #define HSMMC_HAS_PBIAS		(1 << 0)
> > >  #define HSMMC_HAS_UPDATED_RESET	(1 << 1)
> > > +#define HSMMC_HAS_HSPE_SUPPORT	(1 << 2)
> > >  		unsigned features;
> > >  
> > >  		int switch_pin;			/* gpio (card detect) */
> > > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> > > index 54bfd0c..3fd95cb 100644
> > > --- a/drivers/mmc/host/omap_hsmmc.c
> > > +++ b/drivers/mmc/host/omap_hsmmc.c
> > > @@ -62,6 +62,7 @@
> > >  
> > >  #define VS18			(1 << 26)
> > >  #define VS30			(1 << 25)
> > > +#define HSS			(1 << 21)
> > >  #define SDVS18			(0x5 << 9)
> > >  #define SDVS30			(0x6 << 9)
> > >  #define SDVS33			(0x7 << 9)
> > > @@ -89,6 +90,7 @@
> > >  #define MSBS			(1 << 5)
> > >  #define BCE			(1 << 1)
> > >  #define FOUR_BIT		(1 << 1)
> > > +#define HSPE			(1 << 2)
> > >  #define DDR			(1 << 19)
> > >  #define DW8			(1 << 5)
> > >  #define CC			0x1
> > > @@ -489,6 +491,7 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> > >  	struct mmc_ios *ios = &host->mmc->ios;
> > >  	unsigned long regval;
> > >  	unsigned long timeout;
> > > +	unsigned long clkdiv;
> > >  
> > >  	dev_vdbg(mmc_dev(host->mmc), "Set clock to %uHz\n", ios->clock);
> > >  
> > > @@ -496,7 +499,8 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> > >  
> > >  	regval = OMAP_HSMMC_READ(host->base, SYSCTL);
> > >  	regval = regval & ~(CLKD_MASK | DTO_MASK);
> > > -	regval = regval | (calc_divisor(host, ios) << 6) | (DTO << 16);
> > > +	clkdiv = calc_divisor(host, ios);
> > > +	regval = regval | (clkdiv << 6) | (DTO << 16);
> > 
> > not part of $SUBJECT
> 
> The output of the calc_divisor() is reused in the below HSPE checking. So
> instead of calling it twice I have taken the output of that function into
> a variable and reused it.

looks like this could/should go into commit log ;-)

> > >  	OMAP_HSMMC_WRITE(host->base, SYSCTL, regval);
> > >  	OMAP_HSMMC_WRITE(host->base, SYSCTL,
> > >  		OMAP_HSMMC_READ(host->base, SYSCTL) | ICE);
> > > @@ -507,6 +511,27 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> > >  		&& time_before(jiffies, timeout))
> > >  		cpu_relax();
> > >  
> > > +	/*
> > > +	 * Enable High-Speed Support
> > > +	 * Pre-Requisites
> > > +	 *	- Controller should support High-Speed-Enable Bit
> > > +	 *	- Controller should not be using DDR Mode
> > > +	 *	- Controller should advertise that it supports High Speed
> > > +	 *	  in capabilities register
> > > +	 *	- MMC/SD clock coming out of controller > 25MHz
> > > +	 */
> > > +	if ((mmc_slot(host).features & HSMMC_HAS_HSPE_SUPPORT) &&
> > 
> > is this really needed...
> 
> As explained above, this is needed only on platforms which requires the
> Bit to be set (currently AM335x). Such platforms will set this
> parameter in it .dts file
> 
> > 
> > > +	    (ios->timing != MMC_TIMING_UHS_DDR50) &&
> > > +	    ((OMAP_HSMMC_READ(host->base, CAPA) & HSS) == HSS)) {
> > 
> > ... provided you check device's register ?
> 
> As explained above, both TI814x & AM335x support HSS but only AM335x
> require HSPE bit set.

ok, understood. Just one small simplification:

if (a & BIT(n)) will have the same effect as if ((a & BIT(n) == BIT(n))),
so you can simplify that a little bit.
Hebbar, Gururaja Oct. 31, 2012, 6:59 a.m. UTC | #4
On Wed, Oct 31, 2012 at 11:14:28, Balbi, Felipe wrote:
> Hi,
> 
> On Wed, Oct 31, 2012 at 06:17:02AM +0100, Hebbar, Gururaja wrote:
> > On Mon, Oct 29, 2012 at 21:47:19, Balbi, Felipe wrote:
> > > Hi,
> > > 
> > > On Mon, Oct 29, 2012 at 06:26:48PM +0530, Hebbar, Gururaja wrote:
> > > > HSMMC IP on AM33xx need a special setting to handle High-speed cards.
> > > > Other platforms like TI81xx, OMAP4 may need this as-well. This depends
> > > > on the HSMMC IP timing closure done for the high speed cards.
> > > > 
> > > > From AM335x TRM (SPRUH73F - 18.3.12 Output Signals Generation)
> > > > 
> > > > The MMC/SD/SDIO output signals can be driven on either falling edge or
> > > > rising edge depending on the SD_HCTL[2] HSPE bit. This feature allows
> > > > to reach better timing performance, and thus to increase data transfer
> > > > frequency.
> > > > 
> > > > There are few pre-requisites for enabling the HSPE bit
> > > > - Controller should support High-Speed-Enable Bit and
> > > > - Controller should not be using DDR Mode and
> > > > - Controller should advertise that it supports High Speed in
> > > >   capabilities register and
> > > > - MMC/SD clock coming out of controller > 25MHz
> > > > 
> > > > Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> > > > ---
> > > > Rebased on mmc-next (v3.7.0-rc1)
> > > > Only Build tested since EDMA support for AM335x is not yet added
> > > > 
> > > > :100644 100644 be76a23... ed271fc... M	Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > > > :100644 100644 8b4e4f2... 346af5b... M	arch/arm/plat-omap/include/plat/mmc.h
> > > > :100644 100644 54bfd0c... 3fd95cb... M	drivers/mmc/host/omap_hsmmc.c
> > > >  .../devicetree/bindings/mmc/ti-omap-hsmmc.txt      |    1 +
> > > >  arch/arm/plat-omap/include/plat/mmc.h              |    1 +
> > > >  drivers/mmc/host/omap_hsmmc.c                      |   30 +++++++++++++++++++-
> > > >  3 files changed, 31 insertions(+), 1 deletions(-)
> > > > 
> > > > diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > > > index be76a23..ed271fc 100644
> > > > --- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > > > +++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
> > > > @@ -19,6 +19,7 @@ ti,dual-volt: boolean, supports dual voltage cards
> > > >  "supply-name" examples are "vmmc", "vmmc_aux" etc
> > > >  ti,non-removable: non-removable slot (like eMMC)
> > > >  ti,needs-special-reset: Requires a special softreset sequence
> > > > +ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed
> > > 
> > > can't you do runtime revision detection for this ?
> > 
> > Platforms like AM335x & TI814x both use the same IP. 
> > But only AM335x needed this bit to be set in order to sustain high
> > speed transfers. This was concluded after AM335x HSMMC IP timing
> > closure was completed for High speed Cards.
> 
> fair enough.
> 
> > > >  Example:
> > > >  	mmc1: mmc@0x4809c000 {
> > > > diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
> > > > index 8b4e4f2..346af5b 100644
> > > > --- a/arch/arm/plat-omap/include/plat/mmc.h
> > > > +++ b/arch/arm/plat-omap/include/plat/mmc.h
> > > > @@ -126,6 +126,7 @@ struct omap_mmc_platform_data {
> > > >  		/* we can put the features above into this variable */
> > > >  #define HSMMC_HAS_PBIAS		(1 << 0)
> > > >  #define HSMMC_HAS_UPDATED_RESET	(1 << 1)
> > > > +#define HSMMC_HAS_HSPE_SUPPORT	(1 << 2)
> > > >  		unsigned features;
> > > >  
> > > >  		int switch_pin;			/* gpio (card detect) */
> > > > diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> > > > index 54bfd0c..3fd95cb 100644
> > > > --- a/drivers/mmc/host/omap_hsmmc.c
> > > > +++ b/drivers/mmc/host/omap_hsmmc.c
> > > > @@ -62,6 +62,7 @@
> > > >  
> > > >  #define VS18			(1 << 26)
> > > >  #define VS30			(1 << 25)
> > > > +#define HSS			(1 << 21)
> > > >  #define SDVS18			(0x5 << 9)
> > > >  #define SDVS30			(0x6 << 9)
> > > >  #define SDVS33			(0x7 << 9)
> > > > @@ -89,6 +90,7 @@
> > > >  #define MSBS			(1 << 5)
> > > >  #define BCE			(1 << 1)
> > > >  #define FOUR_BIT		(1 << 1)
> > > > +#define HSPE			(1 << 2)
> > > >  #define DDR			(1 << 19)
> > > >  #define DW8			(1 << 5)
> > > >  #define CC			0x1
> > > > @@ -489,6 +491,7 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> > > >  	struct mmc_ios *ios = &host->mmc->ios;
> > > >  	unsigned long regval;
> > > >  	unsigned long timeout;
> > > > +	unsigned long clkdiv;
> > > >  
> > > >  	dev_vdbg(mmc_dev(host->mmc), "Set clock to %uHz\n", ios->clock);
> > > >  
> > > > @@ -496,7 +499,8 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> > > >  
> > > >  	regval = OMAP_HSMMC_READ(host->base, SYSCTL);
> > > >  	regval = regval & ~(CLKD_MASK | DTO_MASK);
> > > > -	regval = regval | (calc_divisor(host, ios) << 6) | (DTO << 16);
> > > > +	clkdiv = calc_divisor(host, ios);
> > > > +	regval = regval | (clkdiv << 6) | (DTO << 16);
> > > 
> > > not part of $SUBJECT
> > 
> > The output of the calc_divisor() is reused in the below HSPE checking. So
> > instead of calling it twice I have taken the output of that function into
> > a variable and reused it.
> 
> looks like this could/should go into commit log ;-)

Ok I will update the commit message in V2.

> 
> > > >  	OMAP_HSMMC_WRITE(host->base, SYSCTL, regval);
> > > >  	OMAP_HSMMC_WRITE(host->base, SYSCTL,
> > > >  		OMAP_HSMMC_READ(host->base, SYSCTL) | ICE);
> > > > @@ -507,6 +511,27 @@ static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
> > > >  		&& time_before(jiffies, timeout))
> > > >  		cpu_relax();
> > > >  
> > > > +	/*
> > > > +	 * Enable High-Speed Support
> > > > +	 * Pre-Requisites
> > > > +	 *	- Controller should support High-Speed-Enable Bit
> > > > +	 *	- Controller should not be using DDR Mode
> > > > +	 *	- Controller should advertise that it supports High Speed
> > > > +	 *	  in capabilities register
> > > > +	 *	- MMC/SD clock coming out of controller > 25MHz
> > > > +	 */
> > > > +	if ((mmc_slot(host).features & HSMMC_HAS_HSPE_SUPPORT) &&
> > > 
> > > is this really needed...
> > 
> > As explained above, this is needed only on platforms which requires the
> > Bit to be set (currently AM335x). Such platforms will set this
> > parameter in it .dts file
> > 
> > > 
> > > > +	    (ios->timing != MMC_TIMING_UHS_DDR50) &&
> > > > +	    ((OMAP_HSMMC_READ(host->base, CAPA) & HSS) == HSS)) {
> > > 
> > > ... provided you check device's register ?
> > 
> > As explained above, both TI814x & AM335x support HSS but only AM335x
> > require HSPE bit set.
> 
> ok, understood. Just one small simplification:
> 
> if (a & BIT(n)) will have the same effect as if ((a & BIT(n) == BIT(n))),
> so you can simplify that a little bit.

Ok I will correct it in V2. 

Thanks for the review.

> 
> -- 
> balbi
> 


Regards, 
Gururaja
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
index be76a23..ed271fc 100644
--- a/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
+++ b/Documentation/devicetree/bindings/mmc/ti-omap-hsmmc.txt
@@ -19,6 +19,7 @@  ti,dual-volt: boolean, supports dual voltage cards
 "supply-name" examples are "vmmc", "vmmc_aux" etc
 ti,non-removable: non-removable slot (like eMMC)
 ti,needs-special-reset: Requires a special softreset sequence
+ti,needs-special-hs-handling: HSMMC IP needs special setting for handling High Speed
 
 Example:
 	mmc1: mmc@0x4809c000 {
diff --git a/arch/arm/plat-omap/include/plat/mmc.h b/arch/arm/plat-omap/include/plat/mmc.h
index 8b4e4f2..346af5b 100644
--- a/arch/arm/plat-omap/include/plat/mmc.h
+++ b/arch/arm/plat-omap/include/plat/mmc.h
@@ -126,6 +126,7 @@  struct omap_mmc_platform_data {
 		/* we can put the features above into this variable */
 #define HSMMC_HAS_PBIAS		(1 << 0)
 #define HSMMC_HAS_UPDATED_RESET	(1 << 1)
+#define HSMMC_HAS_HSPE_SUPPORT	(1 << 2)
 		unsigned features;
 
 		int switch_pin;			/* gpio (card detect) */
diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index 54bfd0c..3fd95cb 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -62,6 +62,7 @@ 
 
 #define VS18			(1 << 26)
 #define VS30			(1 << 25)
+#define HSS			(1 << 21)
 #define SDVS18			(0x5 << 9)
 #define SDVS30			(0x6 << 9)
 #define SDVS33			(0x7 << 9)
@@ -89,6 +90,7 @@ 
 #define MSBS			(1 << 5)
 #define BCE			(1 << 1)
 #define FOUR_BIT		(1 << 1)
+#define HSPE			(1 << 2)
 #define DDR			(1 << 19)
 #define DW8			(1 << 5)
 #define CC			0x1
@@ -489,6 +491,7 @@  static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
 	struct mmc_ios *ios = &host->mmc->ios;
 	unsigned long regval;
 	unsigned long timeout;
+	unsigned long clkdiv;
 
 	dev_vdbg(mmc_dev(host->mmc), "Set clock to %uHz\n", ios->clock);
 
@@ -496,7 +499,8 @@  static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
 
 	regval = OMAP_HSMMC_READ(host->base, SYSCTL);
 	regval = regval & ~(CLKD_MASK | DTO_MASK);
-	regval = regval | (calc_divisor(host, ios) << 6) | (DTO << 16);
+	clkdiv = calc_divisor(host, ios);
+	regval = regval | (clkdiv << 6) | (DTO << 16);
 	OMAP_HSMMC_WRITE(host->base, SYSCTL, regval);
 	OMAP_HSMMC_WRITE(host->base, SYSCTL,
 		OMAP_HSMMC_READ(host->base, SYSCTL) | ICE);
@@ -507,6 +511,27 @@  static void omap_hsmmc_set_clock(struct omap_hsmmc_host *host)
 		&& time_before(jiffies, timeout))
 		cpu_relax();
 
+	/*
+	 * Enable High-Speed Support
+	 * Pre-Requisites
+	 *	- Controller should support High-Speed-Enable Bit
+	 *	- Controller should not be using DDR Mode
+	 *	- Controller should advertise that it supports High Speed
+	 *	  in capabilities register
+	 *	- MMC/SD clock coming out of controller > 25MHz
+	 */
+	if ((mmc_slot(host).features & HSMMC_HAS_HSPE_SUPPORT) &&
+	    (ios->timing != MMC_TIMING_UHS_DDR50) &&
+	    ((OMAP_HSMMC_READ(host->base, CAPA) & HSS) == HSS)) {
+		regval = OMAP_HSMMC_READ(host->base, HCTL);
+		if (clkdiv && (clk_get_rate(host->fclk)/clkdiv) > 25000000)
+			regval |= HSPE;
+		else
+			regval &= ~HSPE;
+
+		OMAP_HSMMC_WRITE(host->base, HCTL, regval);
+	}
+
 	omap_hsmmc_start_clock(host);
 }
 
@@ -1700,6 +1725,9 @@  static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 	if (of_find_property(np, "ti,needs-special-reset", NULL))
 		pdata->slots[0].features |= HSMMC_HAS_UPDATED_RESET;
 
+	if (of_find_property(np, "ti,needs-special-hs-handling", NULL))
+		pdata->slots[0].features |= HSMMC_HAS_HSPE_SUPPORT;
+
 	return pdata;
 }
 #else