s5p-mmc: Fix ambiguous setting of data transfer width

PATCH New Chander Kashyap 5 participants 6 messages Aug 30, 2011 – Sep 2, 2011 MMC, SD and SDIO

Share & tools

Chander Kashyap

author

2011-08-30 10:55 UTC #0 lore raw

PATCH 1/1 s5p-mmc: Fix ambiguous setting of data transfer width

mmc data transfer width is set as following:
WIDE8[5]:
0 = Depend on WIDE4
1 = 8-bit mode
WIDE4[1]:
1 = 4-bit mode
0 = 1-bit mode

In case of 4-bit mode reset 8-bit mode and
in case of 1-bit mode reset 8-bit mode and 4-bit mode

Signed-off-by Chander Kashyap <[email protected]>

---
 drivers/mmc/s5p_mmc.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

drivers/mmc/s5p_mmc.c +7 -3

index 7786ecf..6e6ad08 100644
@@ -368,12 +368,16 @@ static void mmc_set_ios(struct mmc *mmc)
 	 * 1 = 4-bit mode
 	 * 0 = 1-bit mode
 	 */
-	if (mmc->bus_width == 8)
+	if (mmc->bus_width == 8) {
 		ctrl |= (1 << 5);
-	else if (mmc->bus_width == 4)
+		ctrl &= ~(1 << 1);
+	} else if (mmc->bus_width == 4) {
 		ctrl |= (1 << 1);
-	else
+		ctrl &= ~(1 << 5);
+	} else {
 		ctrl &= ~(1 << 1);
+		ctrl &= ~(1 << 5);
+	}
 
 	/*
 	 * OUTEDGEINV[2]

Signature · 1 line

1.7.4.1

Jaehoon Chung

2011-08-31 08:33 UTC #1 lore raw

PATCH 1/1 s5p-mmc: Fix ambiguous setting of data transfer width

Hi Chander.

this patch is correct.

Acked-by Jaehoon Chung <[email protected]>

Quoted 40 lines · Chander Kashyap

mmc data transfer width is set as following:
WIDE8[5]:
0 = Depend on WIDE4
1 = 8-bit mode
WIDE4[1]:
1 = 4-bit mode
0 = 1-bit mode

In case of 4-bit mode reset 8-bit mode and
in case of 1-bit mode reset 8-bit mode and 4-bit mode

Signed-off-by Chander Kashyap <[email protected]>

---
 drivers/mmc/s5p_mmc.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

drivers/mmc/s5p_mmc.c +7 -3

index 7786ecf..6e6ad08 100644
@@ -368,12 +368,16 @@ static void mmc_set_ios(struct mmc *mmc)
 	 * 1 = 4-bit mode
 	 * 0 = 1-bit mode
 	 */
-	if (mmc->bus_width == 8)
+	if (mmc->bus_width == 8) {
 		ctrl |= (1 << 5);
-	else if (mmc->bus_width == 4)
+		ctrl &= ~(1 << 1);
+	} else if (mmc->bus_width == 4) {
 		ctrl |= (1 << 1);
-	else
+		ctrl &= ~(1 << 5);
+	} else {
 		ctrl &= ~(1 << 1);
+		ctrl &= ~(1 << 5);
+	}
 
 	/*
 	 * OUTEDGEINV[2]

Andy Fleming

2011-09-01 01:23 UTC #2 lore raw

PATCH 1/1 s5p-mmc: Fix ambiguous setting of data transfer width

In reply to Chander Kashyap

Quoted 21 lines · Chander Kashyap

mmc data transfer width is set as following:
WIDE8[5]:
0 = Depend on WIDE4
1 = 8-bit mode
WIDE4[1]:
1 = 4-bit mode
0 = 1-bit mode
In case of 4-bit mode reset 8-bit mode and
in case of 1-bit mode reset 8-bit mode and 4-bit mode
Signed-off-by Chander Kashyap <[email protected]>
---
?drivers/mmc/s5p_mmc.c | ? 10 +++++++---
?1 files changed, 7 insertions(+), 3 deletions(-)
drivers/mmc/s5p_mmc.c +0 -0
index 7786ecf..6e6ad08 100644
@@ -368,12 +368,16 @@ static void mmc_set_ios(struct mmc *mmc)

Quoted 8 lines

? ? ? ? * 1 = 4-bit mode
? ? ? ? * 0 = 1-bit mode
? ? ? ? */
- ? ? ? if (mmc->bus_width == 8)
+ ? ? ? if (mmc->bus_width == 8) {
? ? ? ? ? ? ? ?ctrl |= (1 << 5);
- ? ? ? else if (mmc->bus_width == 4)
+ ? ? ? ? ? ? ? ctrl &= ~(1 << 1);

I know these were like this before, but those numbers are awfully magical. You should really define constants for them.

Also, this seems like a very confusing way to do this? Why not clear both bits@the start, and then set the one that is needed?

Andy

Minkyu Kang

2011-09-01 02:26 UTC #3 lore raw

PATCH 1/1 s5p-mmc: Fix ambiguous setting of data transfer width

Dear Andy Fleming,

Quoted 27 lines · Andy Fleming

Quoted 25 lines · Chander Kashyap
mmc data transfer width is set as following:
WIDE8[5]:
0 = Depend on WIDE4
1 = 8-bit mode
WIDE4[1]:
1 = 4-bit mode
0 = 1-bit mode
In case of 4-bit mode reset 8-bit mode and
in case of 1-bit mode reset 8-bit mode and 4-bit mode
Signed-off-by Chander Kashyap <[email protected]>
---
?drivers/mmc/s5p_mmc.c | ? 10 +++++++---
?1 files changed, 7 insertions(+), 3 deletions(-)
drivers/mmc/s5p_mmc.c +0 -0
index 7786ecf..6e6ad08 100644
@@ -368,12 +368,16 @@ static void mmc_set_ios(struct mmc *mmc)
? ? ? ? * 1 = 4-bit mode
? ? ? ? * 0 = 1-bit mode
? ? ? ? */
- ? ? ? if (mmc->bus_width == 8)

Quoted 8 lines

Quoted 4 lines
+ ? ? ? if (mmc->bus_width == 8) {
? ? ? ? ? ? ? ?ctrl |= (1 << 5);
- ? ? ? else if (mmc->bus_width == 4)
+ ? ? ? ? ? ? ? ctrl &= ~(1 << 1);
I know these were like this before, but those numbers are awfully magical. You should really define constants for them.

We decided to use comments instead of defines.

Quoted 3 lines

Also, this seems like a very confusing way to do this? Why not clear both bits at the start, and then set the one that is needed?

Agreed.

Thanks
Minkyu Kang

Signature · 2 lines

from. prom.
www.promsoft.net

Wolfgang Denk

2011-09-01 05:51 UTC #4 lore raw

PATCH 1/1 s5p-mmc: Fix ambiguous setting of data transfer width

Dear Minkyu Kang,

Quoted 5 lines · In message <CALrBrZ3H+k8bu-1OwF5C5BNHuphj_J8jXXV2xgu6jPO5gmJELg@mail.gmail.com>…

Quoted 3 lines
I know these were like this before, but those numbers are awfully magical. You should really define constants for them.
We decided to use comments instead of defines.

And now we ask you to fix this, because we don;t want such magic constants. So please go on and change this.

Thanks.

Wolfgang Denk

Signature · 6 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
About the use of language: it is impossible to sharpen a pencil  with
a  blunt  ax.  It is equally vain to try to do it with ten blunt axes
instead.                                           -- Edsger Dijkstra

Minkyu Kang

2011-09-02 08:09 UTC #5 lore raw

PATCH 1/1 s5p-mmc: Fix ambiguous setting of data transfer width

Dear Wolfgang Denk,

Quoted 4 lines · Wolfgang Denk

Dear Minkyu Kang,
Quoted 1 line · In message <CALrBrZ3H+k8bu-1OwF5C5BNHuphj_J8jXXV2xgu6jPO5gmJELg@mail.gmail.com>…

Quoted 8 lines

Quoted 4 lines
Quoted 2 lines
I know these were like this before, but those numbers are awfully magical. You should really define constants for them.
We decided to use comments instead of defines.
And now we ask you to fix this, because we don;t want such magic constants. ?So please go on and change this.

OK.

Thanks.
Minkyu Kang

Signature · 2 lines

from. prom.
www.promsoft.net