diff mbox

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

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

Commit Message

Hebbar, Gururaja Oct. 31, 2012, 11:57 a.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

Note:
The implementation reuses the output of calc_divisor() so as to reduce
code addition.

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

Changes in V2
	- Added note in commit message about code re-use
	- replaced ((a & BIT(n) == BIT(n))) with (a & BIT(n)) since
	  effectively both are same.

Changes in V3
	- use symbolic define for hardcoded value (25000000)
	- use of_property_read_bool() instead of of_find_property()

: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 5434fd8... 4f7fcee... 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                      |   31 +++++++++++++++++++-
 3 files changed, 32 insertions(+), 1 deletions(-)

Comments

Felipe Balbi Oct. 31, 2012, 12:26 p.m. UTC | #1
Hi,

On Wed, Oct 31, 2012 at 05:27:36PM +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
> 
> Note:
> The implementation reuses the output of calc_divisor() so as to reduce
> code addition.
> 
> Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>

this looks good to my eyes, hopefully I haven't missed anything:

Reviewed-by: Felipe Balbi <balbi@ti.com>

> ---
> Rebased on mmc-next (v3.7.0-rc1)
> Only Build tested since EDMA support for AM335x is not yet added
> 
> Changes in V2
> 	- Added note in commit message about code re-use
> 	- replaced ((a & BIT(n) == BIT(n))) with (a & BIT(n)) since
> 	  effectively both are same.
> 
> Changes in V3
> 	- use symbolic define for hardcoded value (25000000)
> 	- use of_property_read_bool() instead of of_find_property()
> 
> :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 5434fd8... 4f7fcee... 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                      |   31 +++++++++++++++++++-
>  3 files changed, 32 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
>  
>  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 5434fd8..4f7fcee 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -63,6 +63,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)
> @@ -90,6 +91,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
> @@ -113,6 +115,7 @@
>  #define MMC_TIMEOUT_MS		20
>  #define OMAP_MMC_MIN_CLOCK	400000
>  #define OMAP_MMC_MAX_CLOCK	52000000
> +#define HSMMC_CLKOUT	25000000
>  #define DRIVER_NAME		"omap_hsmmc"
>  
>  /*
> @@ -495,6 +498,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);
>  
> @@ -502,7 +506,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);
> @@ -513,6 +518,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)) {
> +		regval = OMAP_HSMMC_READ(host->base, HCTL);
> +		if (clkdiv && (clk_get_rate(host->fclk)/clkdiv) > HSMMC_CLKOUT)
> +			regval |= HSPE;
> +		else
> +			regval &= ~HSPE;
> +
> +		OMAP_HSMMC_WRITE(host->base, HCTL, regval);
> +	}
> +
>  	omap_hsmmc_start_clock(host);
>  }
>  
> @@ -1709,6 +1735,9 @@ static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
>  	if (!of_property_read_u32(np, "max-frequency", &max_freq))
>  		pdata->max_freq = max_freq;
>  
> +	if (of_property_read_bool(np, "ti,needs-special-hs-handling"))
> +		pdata->slots[0].features |= HSMMC_HAS_HSPE_SUPPORT;
> +
>  	return pdata;
>  }
>  #else
> -- 
> 1.7.0.4
>
Venkatraman S Oct. 31, 2012, 7:51 p.m. UTC | #2
On Wed, Oct 31, 2012 at 5:56 PM, Felipe Balbi <balbi@ti.com> wrote:
> Hi,
>
> On Wed, Oct 31, 2012 at 05:27:36PM +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
>>
>> Note:
>> The implementation reuses the output of calc_divisor() so as to reduce
>> code addition.
>>
>> Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
>
> this looks good to my eyes, hopefully I haven't missed anything:
>
> Reviewed-by: Felipe Balbi <balbi@ti.com>
>

Except for the excessively verbose comments which are just duplicating the code,
<Quote>
>> +      * 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
>> +      */
</Quote>

I'm ok with this patch as well. I'm putting a few patches under test
including this one,
and will send it to Chris as part of that series.
I'll strip out the above mentioned comments, unless there are any objections.

Thanks,
Venkat.
--
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, 8:16 p.m. UTC | #3
Hi,

On Thu, Nov 01, 2012 at 01:21:36AM +0530, Venkatraman S wrote:
> On Wed, Oct 31, 2012 at 5:56 PM, Felipe Balbi <balbi@ti.com> wrote:
> > Hi,
> >
> > On Wed, Oct 31, 2012 at 05:27:36PM +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
> >>
> >> Note:
> >> The implementation reuses the output of calc_divisor() so as to reduce
> >> code addition.
> >>
> >> Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> >
> > this looks good to my eyes, hopefully I haven't missed anything:
> >
> > Reviewed-by: Felipe Balbi <balbi@ti.com>
> >
> 
> Except for the excessively verbose comments which are just duplicating the code,
> <Quote>
> >> +      * 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
> >> +      */
> </Quote>
> 
> I'm ok with this patch as well. I'm putting a few patches under test
> including this one,
> and will send it to Chris as part of that series.
> I'll strip out the above mentioned comments, unless there are any
> objections.

please don't. Detailing the pre-requisites for getting HSP mode working
isn't bad at all. Should someone decide to change the behavior and ends
up breaking it, the comment will help putting things back together.

my 2 cents, you've got the final decision though.
Hebbar, Gururaja Nov. 2, 2012, 5:32 a.m. UTC | #4
On Thu, Nov 01, 2012 at 01:46:26, Balbi, Felipe wrote:
> Hi,
> 
> On Thu, Nov 01, 2012 at 01:21:36AM +0530, Venkatraman S wrote:
> > On Wed, Oct 31, 2012 at 5:56 PM, Felipe Balbi <balbi@ti.com> wrote:
> > > Hi,
> > >
> > > On Wed, Oct 31, 2012 at 05:27:36PM +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
> > >>
> > >> Note:
> > >> The implementation reuses the output of calc_divisor() so as to reduce
> > >> code addition.
> > >>
> > >> Signed-off-by: Hebbar, Gururaja <gururaja.hebbar@ti.com>
> > >
> > > this looks good to my eyes, hopefully I haven't missed anything:
> > >
> > > Reviewed-by: Felipe Balbi <balbi@ti.com>
> > >
> > 
> > Except for the excessively verbose comments which are just duplicating the code,
> > <Quote>
> > >> +      * 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
> > >> +      */
> > </Quote>
> > 
> > I'm ok with this patch as well. I'm putting a few patches under test
> > including this one,
> > and will send it to Chris as part of that series.
> > I'll strip out the above mentioned comments, unless there are any
> > objections.
> 
> please don't. Detailing the pre-requisites for getting HSP mode working
> isn't bad at all. Should someone decide to change the behavior and ends
> up breaking it, the comment will help putting things back together.
> 
> my 2 cents, you've got the final decision though.

Same here. Description is required in commit message since it will help
in during git bisect.

> 
> -- 
> 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 5434fd8..4f7fcee 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -63,6 +63,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)
@@ -90,6 +91,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
@@ -113,6 +115,7 @@ 
 #define MMC_TIMEOUT_MS		20
 #define OMAP_MMC_MIN_CLOCK	400000
 #define OMAP_MMC_MAX_CLOCK	52000000
+#define HSMMC_CLKOUT	25000000
 #define DRIVER_NAME		"omap_hsmmc"
 
 /*
@@ -495,6 +498,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);
 
@@ -502,7 +506,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);
@@ -513,6 +518,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)) {
+		regval = OMAP_HSMMC_READ(host->base, HCTL);
+		if (clkdiv && (clk_get_rate(host->fclk)/clkdiv) > HSMMC_CLKOUT)
+			regval |= HSPE;
+		else
+			regval &= ~HSPE;
+
+		OMAP_HSMMC_WRITE(host->base, HCTL, regval);
+	}
+
 	omap_hsmmc_start_clock(host);
 }
 
@@ -1709,6 +1735,9 @@  static struct omap_mmc_platform_data *of_get_hsmmc_pdata(struct device *dev)
 	if (!of_property_read_u32(np, "max-frequency", &max_freq))
 		pdata->max_freq = max_freq;
 
+	if (of_property_read_bool(np, "ti,needs-special-hs-handling"))
+		pdata->slots[0].features |= HSMMC_HAS_HSPE_SUPPORT;
+
 	return pdata;
 }
 #else