Skip to contentSkip to conversation

ARM: OMAP: fix erroneous mmc2 clock change on mmc3 setup

PATCH New Grazvydas Ignotas 2 participants 6 messages Dec 18, 2011 – Jan 26, 2012

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: OMAP: fix erroneous mmc2 clock change on mmc3 setup

hsmmc23_before_set_reg() can set MMCSDIO2ADPCLKISEL bit, which enables internal clock for MMC2. Currently this function is also called by code handling MMC3, and if .internal_clock is set in platform data (by default it currently is), it will set MMCSDIO2ADPCLKISEL for MMC2 instead of MMC3 (MMC3 doesn't have such bit so nothing actually needs to be done). This breaks 2nd SD slot on pandora.

Fix this by changing hsmmc23_before_set_reg() to only handle MMC2. Note that this removes .remux() call for MMC3, but no board currently needs it and it's also not called for MMC4 and MMC5.

---
applies both on mainline and hsmmc branch.

 arch/arm/mach-omap2/hsmmc.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/hsmmc.c +3 -4
index f4a1020..d90ac08 100644
@@ -171,7 +171,7 @@ static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot,
 	}
 }
 
-static void hsmmc23_before_set_reg(struct device *dev, int slot,
+static void hsmmc2_before_set_reg(struct device *dev, int slot,
 				   int power_on, int vdd)
 {
 	struct omap_mmc_platform_data *mmc = dev->platform_data;
@@ -378,14 +378,13 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 			c->caps &= ~MMC_CAP_8_BIT_DATA;
 			c->caps |= MMC_CAP_4_BIT_DATA;
 		}
-		/* FALLTHROUGH */
-	case 3:
 		if (mmc->slots[0].features & HSMMC_HAS_PBIAS) {
 			/* off-chip level shifting, or none */
-			mmc->slots[0].before_set_reg = hsmmc23_before_set_reg;
+			mmc->slots[0].before_set_reg = hsmmc2_before_set_reg;
 			mmc->slots[0].after_set_reg = NULL;
 		}
 		break;
+	case 3:
 	case 4:
 	case 5:
 		mmc->slots[0].before_set_reg = NULL;
Signature · 1 line
1.7.0.4
#1 lore raw

PATCH 1/1 ARM: OMAP: fix erroneous mmc2 clock change on mmc3 setup

Hi Tony,

Would be nice to have this for -rc as this used to work several releases back when hsmmc23_before_set_reg() was checking for a regulator, which wasn't set for MMC3 so the function would return early, not causing the problem it causes now.

Quoted 51 lines · Grazvydas Ignotas

hsmmc23_before_set_reg() can set MMCSDIO2ADPCLKISEL bit, which enables internal clock for MMC2. Currently this function is also called by code handling MMC3, and if .internal_clock is set in platform data (by default it currently is), it will set MMCSDIO2ADPCLKISEL for MMC2 instead of MMC3 (MMC3 doesn't have such bit so nothing actually needs to be done). This breaks 2nd SD slot on pandora.

Fix this by changing hsmmc23_before_set_reg() to only handle MMC2. Note that this removes .remux() call for MMC3, but no board currently needs it and it's also not called for MMC4 and MMC5.

---
applies both on mainline and hsmmc branch.

?arch/arm/mach-omap2/hsmmc.c | ? ?7 +++----
?1 files changed, 3 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/hsmmc.c +1 -3
index f4a1020..d90ac08 100644
@@ -171,7 +171,7 @@ static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot,
 ? ? ? ?}
 ?}
 
-static void hsmmc23_before_set_reg(struct device *dev, int slot,
+static void hsmmc2_before_set_reg(struct device *dev, int slot,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? int power_on, int vdd)
 ?{
 ? ? ? ?struct omap_mmc_platform_data *mmc = dev->platform_data;
@@ -378,14 +378,13 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 ? ? ? ? ? ? ? ? ? ? ? ?c->caps &= ~MMC_CAP_8_BIT_DATA;
 ? ? ? ? ? ? ? ? ? ? ? ?c->caps |= MMC_CAP_4_BIT_DATA;
 ? ? ? ? ? ? ? ?}
- ? ? ? ? ? ? ? /* FALLTHROUGH */
- ? ? ? case 3:
 ? ? ? ? ? ? ? ?if (mmc->slots[0].features & HSMMC_HAS_PBIAS) {

? ? ? ? ? ? ? ? ? ? ? ?/* off-chip level shifting, or none */
- ? ? ? ? ? ? ? ? ? ? ? mmc->slots[0].before_set_reg = hsmmc23_before_set_reg;
+ ? ? ? ? ? ? ? ? ? ? ? mmc->slots[0].before_set_reg = hsmmc2_before_set_reg; ? ? ? ? ? ? ? ? ? ? ? ?mmc->slots[0].after_set_reg = NULL;
? ? ? ? ? ? ? ?}
? ? ? ? ? ? ? ?break;
+ ? ? ? case 3:
? ? ? ?case 4:
? ? ? ?case 5:
? ? ? ? ? ? ? ?mmc->slots[0].before_set_reg = NULL;
--
1.7.0.4

Signature · 1 line
Gra?vydas
maintainer
#2 lore raw

PATCH 1/1 ARM: OMAP: fix erroneous mmc2 clock change on mmc3 setup

Quoted 6 lines · Grazvydas Ignotas

Hi Tony,

Would be nice to have this for -rc as this used to work several releases back when hsmmc23_before_set_reg() was checking for a regulator, which wasn't set for MMC3 so the function would return early, not causing the problem it causes now.

Yeah this looks like a fix. I'll take a look next week as I'll be travelling.

Tony

maintainer
#3 lore raw

PATCH 1/1 ARM: OMAP: fix erroneous mmc2 clock change on mmc3 setup

In reply to Grazvydas Ignotas
Quoted 23 lines · Grazvydas Ignotas

hsmmc23_before_set_reg() can set MMCSDIO2ADPCLKISEL bit, which enables internal clock for MMC2. Currently this function is also called by code handling MMC3, and if .internal_clock is set in platform data (by default it currently is), it will set MMCSDIO2ADPCLKISEL for MMC2 instead of MMC3 (MMC3 doesn't have such bit so nothing actually needs to be done). This breaks 2nd SD slot on pandora.

Fix this by changing hsmmc23_before_set_reg() to only handle MMC2. Note that this removes .remux() call for MMC3, but no board currently needs it and it's also not called for MMC4 and MMC5.

---
applies both on mainline and hsmmc branch.

 arch/arm/mach-omap2/hsmmc.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
arch/arm/mach-omap2/hsmmc.c +0 -0
index f4a1020..d90ac08 100644

arch/arm/mach-omap2/hsmmc.c static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot, · line 177

Quoted 8 lines
arch/arm/mach-omap2/hsmmc.c +1 -1
@@ -171,7 +171,7 @@ static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot,
 	}
 }
 
-static void hsmmc23_before_set_reg(struct device *dev, int slot,
+static void hsmmc2_before_set_reg(struct device *dev, int slot,
 				   int power_on, int vdd)
 {
 	struct omap_mmc_platform_data *mmc = dev->platform_data;

This hunk above changing the function name seems to be a typo?

Regards,

Tony

Quoted 24 lines
arch/arm/mach-omap2/hsmmc.c +2 -3
@@ -378,14 +378,13 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 			c->caps &= ~MMC_CAP_8_BIT_DATA;
 			c->caps |= MMC_CAP_4_BIT_DATA;
 		}
-		/* FALLTHROUGH */
-	case 3:
 		if (mmc->slots[0].features & HSMMC_HAS_PBIAS) {
 			/* off-chip level shifting, or none */
-			mmc->slots[0].before_set_reg = hsmmc23_before_set_reg;
+			mmc->slots[0].before_set_reg = hsmmc2_before_set_reg;
 			mmc->slots[0].after_set_reg = NULL;
 		}
 		break;
+	case 3:
 	case 4:
 	case 5:
 		mmc->slots[0].before_set_reg = NULL;
Signature · 6 lines
1.7.0.4

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
#4 lore raw

PATCH 1/1 ARM: OMAP: fix erroneous mmc2 clock change on mmc3 setup

Quoted 27 lines · Tony Lindgren
Quoted 26 lines · Grazvydas Ignotas

hsmmc23_before_set_reg() can set MMCSDIO2ADPCLKISEL bit, which enables internal clock for MMC2. Currently this function is also called by code handling MMC3, and if .internal_clock is set in platform data (by default it currently is), it will set MMCSDIO2ADPCLKISEL for MMC2 instead of MMC3 (MMC3 doesn't have such bit so nothing actually needs to be done). This breaks 2nd SD slot on pandora.

Fix this by changing hsmmc23_before_set_reg() to only handle MMC2. Note that this removes .remux() call for MMC3, but no board currently needs it and it's also not called for MMC4 and MMC5.

---
applies both on mainline and hsmmc branch.

?arch/arm/mach-omap2/hsmmc.c | ? ?7 +++----
?1 files changed, 3 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/hsmmc.c +0 -0
index f4a1020..d90ac08 100644
@@ -171,7 +171,7 @@ static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot,
 ? ? ? }
 ?}
 
Quoted 7 lines

arch/arm/mach-omap2/hsmmc.c static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot, · line 177

Quoted 5 lines
arch/arm/mach-omap2/hsmmc.c +1 -1
-static void hsmmc23_before_set_reg(struct device *dev, int slot,
+static void hsmmc2_before_set_reg(struct device *dev, int slot,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int power_on, int vdd)
 ?{
 ? ? ? struct omap_mmc_platform_data *mmc = dev->platform_data;

This hunk above changing the function name seems to be a typo?

No it's not, this function is changing MMC2 clock source and is no longer called for MMC3 after this patch (the call for MMC3 was the problem), so it loses hsmmc23 name.

Quoted 24 lines
Quoted 24 lines
arch/arm/mach-omap2/hsmmc.c +0 -2
@@ -378,14 +378,13 @@ static int __init omap_hsmmc_pdata_init(struct omap2_hsmmc_info *c,
 ? ? ? ? ? ? ? ? ? ? ? c->caps &= ~MMC_CAP_8_BIT_DATA;
 ? ? ? ? ? ? ? ? ? ? ? c->caps |= MMC_CAP_4_BIT_DATA;
 ? ? ? ? ? ? ? }
- ? ? ? ? ? ? /* FALLTHROUGH */
- ? ? case 3:
 ? ? ? ? ? ? ? if (mmc->slots[0].features & HSMMC_HAS_PBIAS) {

? ? ? ? ? ? ? ? ? ? ? /* off-chip level shifting, or none */
- ? ? ? ? ? ? ? ? ? ? mmc->slots[0].before_set_reg = hsmmc23_before_set_reg;
+ ? ? ? ? ? ? ? ? ? ? mmc->slots[0].before_set_reg = hsmmc2_before_set_reg; ? ? ? ? ? ? ? ? ? ? ? mmc->slots[0].after_set_reg = NULL;
? ? ? ? ? ? ? }
? ? ? ? ? ? ? break;
+ ? ? case 3:
? ? ? case 4:
? ? ? case 5:
? ? ? ? ? ? ? mmc->slots[0].before_set_reg = NULL;
--
1.7.0.4

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

Signature · 1 line
Gra?vydas
maintainer
#5 lore raw

PATCH 1/1 ARM: OMAP: fix erroneous mmc2 clock change on mmc3 setup

Quoted 36 lines · Grazvydas Ignotas
Quoted 27 lines · Tony Lindgren
Quoted 26 lines · Grazvydas Ignotas

hsmmc23_before_set_reg() can set MMCSDIO2ADPCLKISEL bit, which enables internal clock for MMC2. Currently this function is also called by code handling MMC3, and if .internal_clock is set in platform data (by default it currently is), it will set MMCSDIO2ADPCLKISEL for MMC2 instead of MMC3 (MMC3 doesn't have such bit so nothing actually needs to be done). This breaks 2nd SD slot on pandora.

Fix this by changing hsmmc23_before_set_reg() to only handle MMC2. Note that this removes .remux() call for MMC3, but no board currently needs it and it's also not called for MMC4 and MMC5.

---
applies both on mainline and hsmmc branch.

?arch/arm/mach-omap2/hsmmc.c | ? ?7 +++----
?1 files changed, 3 insertions(+), 4 deletions(-)

arch/arm/mach-omap2/hsmmc.c +0 -0
index f4a1020..d90ac08 100644
@@ -171,7 +171,7 @@ static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot,
 ? ? ? }
 ?}
 
Quoted 7 lines

arch/arm/mach-omap2/hsmmc.c static void omap4_hsmmc1_after_set_reg(struct device *dev, int slot, · line 177

Quoted 5 lines
arch/arm/mach-omap2/hsmmc.c +1 -1
-static void hsmmc23_before_set_reg(struct device *dev, int slot,
+static void hsmmc2_before_set_reg(struct device *dev, int slot,
 ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?int power_on, int vdd)
 ?{
 ? ? ? struct omap_mmc_platform_data *mmc = dev->platform_data;

This hunk above changing the function name seems to be a typo?

Quoted 3 lines

No it's not, this function is changing MMC2 clock source and is no longer called for MMC3 after this patch (the call for MMC3 was the problem), so it loses hsmmc23 name.

OK thanks, applying into fixes.

Tony