[PATCH] mmc: omap_hsmmc: Fix check for mmc dt node

[PATCH] mmc: omap_hsmmc: Fix check for mmc dt node

[Jassi Brar]

Proper check should be if the hsmmc driver got its platform_data
via devicetree, and not if the devicetree is populated for the
platform.

Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>
---

The patch is more than a nit-pick because upstream doesn't yet have dt
nodes for mmc on OMAP while the driver wrongly assumes it does.
Which isn't a problem until you use some low-end sdhc card
(Transcend 8GB Class-4 in my experience) which fails to re-initialize,
OCR[30] set, if the first attempt fails due to delay at certain times
during the probe (for ex, simply enable CONFIG_REGULATOR_DUMMY).
Apparently such cards need proper 0v->1.8v power cycle to recover
(another Class-10 card does recover after ~10 attempts by the host).

While this simple patch gets us through the problem, it also tells
that adding mmc dt nodes without PBIAS setup callbacks in place
(without which we can't switch off the regulator or do 1.8v -> 3.3v
switch) is probably not a safe option.

 drivers/mmc/host/omap_hsmmc.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
index c628b95..b5ca4593 100644
--- a/drivers/mmc/host/omap_hsmmc.c
+++ b/drivers/mmc/host/omap_hsmmc.c
@@ -138,6 +138,7 @@ struct omap_hsmmc_next {
 
 struct omap_hsmmc_host {
 	struct	device		*dev;
+	struct	of_device_id	*of_dev;
 	struct	mmc_host	*mmc;
 	struct	mmc_request	*mrq;
 	struct	mmc_command	*cmd;
@@ -249,7 +250,7 @@ static int omap_hsmmc_set_power(struct device *dev, int slot, int power_on,
 	 * the pbias cell programming support is still missing when
 	 * booting with Device tree
 	 */
-	if (of_have_populated_dt() && !vdd)
+	if (host->of_dev && !vdd)
 		return 0;
 
 	if (mmc_slot(host).before_set_reg)
@@ -1550,7 +1551,7 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			 * can't be allowed when booting with device
 			 * tree.
 			 */
-			(!of_have_populated_dt())) {
+			!host->of_dev) {
 				/*
 				 * The mmc_select_voltage fn of the core does
 				 * not seem to set the power_mode to
@@ -1837,6 +1838,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
 	host		= mmc_priv(mmc);
 	host->mmc	= mmc;
 	host->pdata	= pdata;
+	host->of_dev	= match;
 	host->dev	= &pdev->dev;
 	host->use_dma	= 1;
 	host->dev->dma_mask = &pdata->dma_mask;
-- 
1.7.4.1

[PATCH] mmc: omap_hsmmc: Fix check for mmc dt node

[T Krishnamoorthy, Balaji]

On Tue, Apr 17, 2012 at 3:24 PM, Jassi Brar <jaswinder.singh@linaro.org> wrote:
> Proper check should be if the hsmmc driver got its platform_data
> via devicetree, and not if the devicetree is populated for the
> platform.
>
> Signed-off-by: Jassi Brar <jaswinder.singh@linaro.org>

Hi,
Similar patch by Rajendra is in mmc-next

http://git.kernel.org/?p=linux/kernel/git/cjb/mmc.git;a=commit;h=3d9bc09d825f72ef119a94615e0de6baea86981f

> ---
>
> The patch is more than a nit-pick because upstream doesn't yet have dt
> nodes for mmc on OMAP while the driver wrongly assumes it does.
> Which isn't a problem until you use some low-end sdhc card
> (Transcend 8GB Class-4 in my experience) which fails to re-initialize,
> OCR[30] set, if the first attempt fails due to delay at certain times
> during the probe (for ex, simply enable CONFIG_REGULATOR_DUMMY).
> Apparently such cards need proper 0v->1.8v power cycle to recover
> (another Class-10 card does recover after ~10 attempts by the host).
>

Is this issue seen while booting with device tree or non device tree ?

> While this simple patch gets us through the problem, it also tells
> that adding mmc dt nodes without PBIAS setup callbacks in place
> (without which we can't switch off the regulator or do 1.8v -> 3.3v
> switch) is probably not a safe option.
>
> ?drivers/mmc/host/omap_hsmmc.c | ? ?6 ++++--
> ?1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index c628b95..b5ca4593 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -138,6 +138,7 @@ struct omap_hsmmc_next {
>
> ?struct omap_hsmmc_host {
> ? ? ? ?struct ?device ? ? ? ? ?*dev;
> + ? ? ? struct ?of_device_id ? ?*of_dev;
> ? ? ? ?struct ?mmc_host ? ? ? ?*mmc;
> ? ? ? ?struct ?mmc_request ? ? *mrq;
> ? ? ? ?struct ?mmc_command ? ? *cmd;
> @@ -249,7 +250,7 @@ static int omap_hsmmc_set_power(struct device *dev, int slot, int power_on,
> ? ? ? ? * the pbias cell programming support is still missing when
> ? ? ? ? * booting with Device tree
> ? ? ? ? */
> - ? ? ? if (of_have_populated_dt() && !vdd)
> + ? ? ? if (host->of_dev && !vdd)
> ? ? ? ? ? ? ? ?return 0;
>
> ? ? ? ?if (mmc_slot(host).before_set_reg)
> @@ -1550,7 +1551,7 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> ? ? ? ? ? ? ? ? ? ? ? ? * can't be allowed when booting with device
> ? ? ? ? ? ? ? ? ? ? ? ? * tree.
> ? ? ? ? ? ? ? ? ? ? ? ? */
> - ? ? ? ? ? ? ? ? ? ? ? (!of_have_populated_dt())) {
> + ? ? ? ? ? ? ? ? ? ? ? !host->of_dev) {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?/*
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? * The mmc_select_voltage fn of the core does
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? * not seem to set the power_mode to
> @@ -1837,6 +1838,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
> ? ? ? ?host ? ? ? ? ? ?= mmc_priv(mmc);
> ? ? ? ?host->mmc ? ? ? = mmc;
> ? ? ? ?host->pdata ? ? = pdata;
> + ? ? ? host->of_dev ? ?= match;
> ? ? ? ?host->dev ? ? ? = &pdev->dev;
> ? ? ? ?host->use_dma ? = 1;
> ? ? ? ?host->dev->dma_mask = &pdata->dma_mask;
> --
> 1.7.4.1
>

[PATCH] mmc: omap_hsmmc: Fix check for mmc dt node

[Jassi Brar]

On 17 April 2012 15:41, T Krishnamoorthy, Balaji <balajitk@ti.com> wrote:
>>
>> The patch is more than a nit-pick because upstream doesn't yet have dt
>> nodes for mmc on OMAP while the driver wrongly assumes it does.
>> Which isn't a problem until you use some low-end sdhc card
>> (Transcend 8GB Class-4 in my experience) which fails to re-initialize,
>> OCR[30] set, if the first attempt fails due to delay at certain times
>> during the probe (for ex, simply enable CONFIG_REGULATOR_DUMMY).
>> Apparently such cards need proper 0v->1.8v power cycle to recover
>> (another Class-10 card does recover after ~10 attempts by the host).
>>
>
> Is this issue seen while booting with device tree or non device tree ?
>
On Panda-4430/60 with some 'low-class' card , with omap4-panda.dtb and
omap2plus_defconfig + CONFIG_REGULATOR_DUMMY

After applying the following patch to enable booting with dt.

--- a/arch/arm/mach-omap2/board-omap4panda.c
+++ b/arch/arm/mach-omap2/board-omap4panda.c
@@ -574,6 +574,11 @@ static void __init omap4_panda_init(void)
 	omap4_panda_display_init();
 }

+static const char *omap4_panda_match[] __initdata = {
+	"ti,omap4-panda",
+	NULL,
+};
+
 MACHINE_START(OMAP4_PANDA, "OMAP4 Panda board")
 	/* Maintainer: David Anders - Texas Instruments Inc */
 	.atag_offset	= 0x100,
@@ -585,4 +590,5 @@ MACHINE_START(OMAP4_PANDA, "OMAP4 Panda board")
 	.init_machine	= omap4_panda_init,
 	.timer		= &omap4_timer,
 	.restart	= omap_prcm_restart,
+	.dt_compat	= omap4_panda_match,
 MACHINE_END


-jassi

[PATCH] mmc: omap_hsmmc: Fix check for mmc dt node

[Rajendra Nayak]

On Tuesday 17 April 2012 03:24 PM, Jassi Brar wrote:
> Proper check should be if the hsmmc driver got its platform_data
> via devicetree, and not if the devicetree is populated for the
> platform.

This one is already fixed here..
http://www.spinics.net/lists/linux-omap/msg67879.html

>
> Signed-off-by: Jassi Brar<jaswinder.singh@linaro.org>
> ---
>
> The patch is more than a nit-pick because upstream doesn't yet have dt
> nodes for mmc on OMAP while the driver wrongly assumes it does.
> Which isn't a problem until you use some low-end sdhc card
> (Transcend 8GB Class-4 in my experience) which fails to re-initialize,
> OCR[30] set, if the first attempt fails due to delay at certain times
> during the probe (for ex, simply enable CONFIG_REGULATOR_DUMMY).
> Apparently such cards need proper 0v->1.8v power cycle to recover
> (another Class-10 card does recover after ~10 attempts by the host).
>
> While this simple patch gets us through the problem, it also tells
> that adding mmc dt nodes without PBIAS setup callbacks in place
> (without which we can't switch off the regulator or do 1.8v ->  3.3v
> switch) is probably not a safe option.
>
>   drivers/mmc/host/omap_hsmmc.c |    6 ++++--
>   1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mmc/host/omap_hsmmc.c b/drivers/mmc/host/omap_hsmmc.c
> index c628b95..b5ca4593 100644
> --- a/drivers/mmc/host/omap_hsmmc.c
> +++ b/drivers/mmc/host/omap_hsmmc.c
> @@ -138,6 +138,7 @@ struct omap_hsmmc_next {
>
>   struct omap_hsmmc_host {
>   	struct	device		*dev;
> +	struct	of_device_id	*of_dev;
>   	struct	mmc_host	*mmc;
>   	struct	mmc_request	*mrq;
>   	struct	mmc_command	*cmd;
> @@ -249,7 +250,7 @@ static int omap_hsmmc_set_power(struct device *dev, int slot, int power_on,
>   	 * the pbias cell programming support is still missing when
>   	 * booting with Device tree
>   	 */
> -	if (of_have_populated_dt()&&  !vdd)
> +	if (host->of_dev&&  !vdd)
>   		return 0;
>
>   	if (mmc_slot(host).before_set_reg)
> @@ -1550,7 +1551,7 @@ static void omap_hsmmc_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>   			 * can't be allowed when booting with device
>   			 * tree.
>   			 */
> -			(!of_have_populated_dt())) {
> +			!host->of_dev) {
>   				/*
>   				 * The mmc_select_voltage fn of the core does
>   				 * not seem to set the power_mode to
> @@ -1837,6 +1838,7 @@ static int __devinit omap_hsmmc_probe(struct platform_device *pdev)
>   	host		= mmc_priv(mmc);
>   	host->mmc	= mmc;
>   	host->pdata	= pdata;
> +	host->of_dev	= match;
>   	host->dev	=&pdev->dev;
>   	host->use_dma	= 1;
>   	host->dev->dma_mask =&pdata->dma_mask;

[PATCH] mmc: omap_hsmmc: Fix check for mmc dt node

[Jassi Brar]

On 17 April 2012 15:41, Rajendra Nayak <rnayak@ti.com> wrote:
> On Tuesday 17 April 2012 03:24 PM, Jassi Brar wrote:
>>
>> Proper check should be if the hsmmc driver got its platform_data
>> via devicetree, and not if the devicetree is populated for the
>> platform.
>
> This one is already fixed here..
> http://www.spinics.net/lists/linux-omap/msg67879.html
>
Your patch looks better.

-j