powerpc/esdhc: force the bus width to 4bit

PATCH New Chang-Ming.Huang at freescale.com 6 participants 16 messages Oct 23, 2012 – Nov 2, 2012 MMC, SD and SDIO

lore

Share & tools

lore

Chang-Ming.Huang at freescale.com

author

2012-10-23 06:26 UTC #0 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

From: Jerry Huang <[email protected]>

For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is support. So for MMC card, we also support 4bit bus width,
otherwiase, we will get the 12bit bus width, which is not correct:
=> mmcinfo
Device: FSL_SDHC
Manufacturer ID: 1e
OEM: ffff
Name: MMC
Tran Speed: 52000000
Rd Block Len: 512
MMC version 4.0
High Capacity: No
Capacity: 1.9 GiB
Bus Width: 12-bit

Signed-off-by Jerry Huang <Chang-Ming.Huang@freescalecom>

CC Andy Fleming <[email protected]>

CC Marek Vasut <[email protected]>

---
 drivers/mmc/fsl_esdhc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/mmc/fsl_esdhc.c +1 -1

index 3f8d30d..7b83dd2 100644
@@ -577,7 +577,7 @@ int fsl_esdhc_initialize(bd_t *bis, struct fsl_esdhc_cfg *cfg)
 		return -1;
 	}
 
-	mmc->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
+	mmc->host_caps = MMC_MODE_4BIT;
 
 	if (caps & ESDHC_HOSTCAPBLT_HSS)
 		mmc->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;

Signature · 1 line

1.7.9.5

Marek Vasut

2012-10-23 07:24 UTC #1 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Dear Chang-Ming.Huang at freescale.com,

Quoted 5 lines

From: Jerry Huang <[email protected]>
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is support. So for MMC card, we also support 4bit bus width,
otherwiase, we will get the 12bit bus width, which is not correct:

Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).

Quoted 24 lines

=> mmcinfo
Device: FSL_SDHC
Manufacturer ID: 1e
OEM: ffff
Name: MMC
Tran Speed: 52000000
Rd Block Len: 512
MMC version 4.0
High Capacity: No
Capacity: 1.9 GiB
Bus Width: 12-bit
Signed-off-by Jerry Huang <Chang-Ming.Huang@freescalecom>
CC Andy Fleming <[email protected]>
CC Marek Vasut <[email protected]>
---
 drivers/mmc/fsl_esdhc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/fsl_esdhc.c +0 -0
index 3f8d30d..7b83dd2 100644

drivers/mmc/fsl_esdhc.c int fsl_esdhc_initialize(bd_t *bis, struct · line 583

Quoted 8 lines

drivers/mmc/fsl_esdhc.c +1 -1

@@ -577,7 +577,7 @@ int fsl_esdhc_initialize(bd_t *bis, struct
 fsl_esdhc_cfg *cfg) return -1;
 	}
 
-	mmc->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
+	mmc->host_caps = MMC_MODE_4BIT;
 
 	if (caps & ESDHC_HOSTCAPBLT_HSS)
 		mmc->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;

Best regards,
Marek Vasut

Huang Changming-R66093

2012-10-23 09:23 UTC #2 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Best Regards
Jerry Huang

Quoted 11 lines

-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Tuesday, October 23, 2012 3:24 PM
To: Huang Changming-R66093
Cc: u-boot at lists.denx.de; Andy Fleming
Subject: Re: [PATCH] powerpc/esdhc: force the bus width to 4bit
Dear Chang-Ming.Huang at freescale.com,
Quoted 2 lines
From: Jerry Huang <[email protected]>

Quoted 8 lines

Quoted 1 line
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is
support.
Quoted 2 lines
So for MMC card, we also support 4bit bus width, otherwiase, we will get the 12bit bus width, which is not correct:
Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).

Yes, the controller support 8bit MMC.

FSL ESDHC driver set the host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT; But, the current codes for MMC card has been changed to:

} else {
	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
                         MMC_MODE_WIDTH_BITS_SHIFT);
	for (; width >= 0; width--) {
	....

So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.

Below is the old codes (width = 2):

} else {
                for (width = EXT_CSD_BUS_WIDTH_8; width >= 0; width--) {

Quoted 35 lines

Quoted 24 lines
=> mmcinfo
Device: FSL_SDHC
Manufacturer ID: 1e
OEM: ffff
Name: MMC
Tran Speed: 52000000
Rd Block Len: 512
MMC version 4.0
High Capacity: No
Capacity: 1.9 GiB
Bus Width: 12-bit
Signed-off-by Jerry Huang <Chang-Ming.Huang@freescalecom>
CC Andy Fleming <[email protected]>
CC Marek Vasut <[email protected]>
---
 drivers/mmc/fsl_esdhc.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/mmc/fsl_esdhc.c index renamed from drivers/mmc/fsl_esdhc.c +0 -0
3f8d30d..7b83dd2 100644
drivers/mmc/fsl_esdhc.c int fsl_esdhc_initialize(bd_t *bis, struct · line 583
Quoted 8 lines
drivers/mmc/fsl_esdhc.c +1 -1
@@ -577,7 +577,7 @@ int fsl_esdhc_initialize(bd_t *bis, struct
 fsl_esdhc_cfg *cfg) return -1;
 	}
 
-	mmc->host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT;
+	mmc->host_caps = MMC_MODE_4BIT;
 
 	if (caps & ESDHC_HOSTCAPBLT_HSS)
 		mmc->host_caps |= MMC_MODE_HS_52MHz | MMC_MODE_HS;
Best regards,
Marek Vasut

Marek Vasut

2012-10-23 09:50 UTC #3 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Dear Huang Changming-R66093,

Quoted 36 lines

Best Regards
Jerry Huang
Quoted 13 lines
-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Tuesday, October 23, 2012 3:24 PM
To: Huang Changming-R66093
Cc: u-boot at lists.denx.de; Andy Fleming
Subject: Re: [PATCH] powerpc/esdhc: force the bus width to 4bit
Dear Chang-Ming.Huang at freescale.com,
Quoted 3 lines
From: Jerry Huang <[email protected]>
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is
Quoted 8 lines
support.
Quoted 3 lines
So for MMC card, we also support 4bit bus width, otherwiase, we will
get the 12bit bus width, which is not correct:
Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).
Yes, the controller support 8bit MMC.
FSL ESDHC driver set the host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT; But, the current codes for MMC card has been changed to:
} else {
	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
                         MMC_MODE_WIDTH_BITS_SHIFT);
	for (; width >= 0; width--) {
	....

Quoted 7 lines

So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.
Below is the old codes (width = 2):
} else {
                for (width = EXT_CSD_BUS_WIDTH_8; width >= 0; width--) {

[...]

Uh, so it's a bug in the MMC subsystem?
Best regards,
Marek Vasut

Huang Changming-R66093

2012-10-24 03:11 UTC #4 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Quoted 35 lines

Quoted 27 lines
Quoted 8 lines
Dear Chang-Ming.Huang at freescale.com,
Quoted 3 lines
From: Jerry Huang <[email protected]>
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is
support.
Quoted 7 lines
Quoted 4 lines
So for MMC card, we also support 4bit bus width, otherwiase, we will
get the 12bit bus width, which is not correct:
Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).
Yes, the controller support 8bit MMC.
FSL ESDHC driver set the host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT; But, the current codes for MMC card has been changed to:
} else {
	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
                         MMC_MODE_WIDTH_BITS_SHIFT);
	for (; width >= 0; width--) {
	....
Quoted 8 lines
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.
Below is the old codes (width = 2):
} else {
                for (width = EXT_CSD_BUS_WIDTH_8; width >= 0; width--)
{

Quoted 5 lines

[...]
Uh, so it's a bug in the MMC subsystem?
Best regards,

I don't know. Maybe Andy can give some comment.

Jaehoon Chung

2012-10-30 08:43 UTC #5 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

In reply to Marek Vasut

Quoted 41 lines · Marek Vasut

Dear Huang Changming-R66093,
Quoted 39 lines
Best Regards
Jerry Huang
Quoted 13 lines
-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Tuesday, October 23, 2012 3:24 PM
To: Huang Changming-R66093
Cc: u-boot at lists.denx.de; Andy Fleming
Subject: Re: [PATCH] powerpc/esdhc: force the bus width to 4bit
Dear Chang-Ming.Huang at freescale.com,
Quoted 3 lines
From: Jerry Huang <[email protected]>
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is
Quoted 8 lines
support.
Quoted 3 lines
So for MMC card, we also support 4bit bus width, otherwiase, we will
get the 12bit bus width, which is not correct:
Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).
Yes, the controller support 8bit MMC.
FSL ESDHC driver set the host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT; But, the current codes for MMC card has been changed to:
} else {
	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
                         MMC_MODE_WIDTH_BITS_SHIFT);
	for (; width >= 0; width--) {
	....
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.

This problem is MMC subsystem's bug.
I think good that will modify the code in mmc.c. If caps is set to MMC_MODE_4BIT | MMC_MODE_8BIT, we can see the 12bit support with using "mmcinfo" command

Best Regards,
Jaehoon Chung

Quoted 15 lines

Quoted 4 lines
Below is the old codes (width = 2):
} else {
                for (width = EXT_CSD_BUS_WIDTH_8; width >= 0; width--) {
[...]
Uh, so it's a bug in the MMC subsystem?
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Lukasz Majewski

2012-10-30 17:35 UTC #6 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Hi Jaehoon,

Quoted 29 lines · Marek Vasut

Quoted 28 lines
Dear Huang Changming-R66093,
Quoted 26 lines
Best Regards
Jerry Huang
Quoted 22 lines
-----Original Message-----
From: Marek Vasut [mailto:marex at denx.de]
Sent: Tuesday, October 23, 2012 3:24 PM
To: Huang Changming-R66093
Cc: u-boot at lists.denx.de; Andy Fleming
Subject: Re: [PATCH] powerpc/esdhc: force the bus width to 4bit
Dear Chang-Ming.Huang at freescale.com,
Quoted 3 lines
From: Jerry Huang <[email protected]>
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is
support.
Quoted 4 lines
So for MMC card, we also support 4bit bus width, otherwiase, we will
get the 12bit bus width, which is not correct:
Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
Yes, the controller support 8bit MMC.
FSL ESDHC driver set the host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT; But, the current codes for MMC card has been changed to:
} else {
	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
                         MMC_MODE_WIDTH_BITS_SHIFT);

Hmm... looks like it is code done by me :-)
So little explanation shall be given.

This code is necessary for some targets (like Samsung's Goni) which can only support 4 bit MMC mode.

Quoted 11 lines

Quoted 6 lines
Quoted 6 lines
	for (; width >= 0; width--) {
	....
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.
This problem is MMC subsystem's bug.
I think good that will modify the code in mmc.c. If caps is set to MMC_MODE_4BIT | MMC_MODE_8BIT, we can see the 12bit support with using "mmcinfo" command

The mmc_set_bus_width(mmc, 4 * width) in conjunction to above code causes the problem.

I agree, that this code shall be refactored.
Lei, what do you think?

Quoted 24 lines

Best Regards,
Jaehoon Chung
Quoted 8 lines
Quoted 5 lines
Below is the old codes (width = 2):
} else {
                for (width = EXT_CSD_BUS_WIDTH_8; width >= 0;
width--) {
[...]
Quoted 8 lines
Uh, so it's a bug in the MMC subsystem?
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group

Jaehoon Chung

2012-10-31 04:51 UTC #7 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Hi, Lukasz,

Quoted 33 lines

Quoted 12 lines
Quoted 12 lines
Quoted 12 lines
Quoted 11 lines
Quoted 1 line
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is
support.
Quoted 4 lines
So for MMC card, we also support 4bit bus width, otherwiase, we will
get the 12bit bus width, which is not correct:
Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
Yes, the controller support 8bit MMC.
FSL ESDHC driver set the host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT; But, the current codes for MMC card has been changed to:
} else {
	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
                         MMC_MODE_WIDTH_BITS_SHIFT);
Hmm... looks like it is code done by me :-)
So little explanation shall be given.
This code is necessary for some targets (like Samsung's Goni) which can only support 4 bit MMC mode.
Quoted 6 lines
Quoted 6 lines
Quoted 6 lines
	for (; width >= 0; width--) {
	....
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.

Quoted 8 lines

Quoted 5 lines
This problem is MMC subsystem's bug.
I think good that will modify the code in mmc.c. If caps is set to MMC_MODE_4BIT | MMC_MODE_8BIT, we can see the 12bit support with using "mmcinfo" command
The mmc_set_bus_width(mmc, 4 * width) in conjunction to above code causes the problem.

then how about using the width[idx] like kernel?

Best Regards,
Jaehoon Chung

Quoted 32 lines

I agree, that this code shall be refactored.
Lei, what do you think?
Quoted 24 lines
Best Regards,
Jaehoon Chung
Quoted 8 lines
Quoted 5 lines
Below is the old codes (width = 2):
} else {
                for (width = EXT_CSD_BUS_WIDTH_8; width >= 0;
width--) {
[...]
Quoted 8 lines
Uh, so it's a bug in the MMC subsystem?
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Lukasz Majewski

2012-10-31 08:20 UTC #8 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Hi Jaehoon,

Quoted 42 lines

Hi, Lukasz,
Quoted 41 lines
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
Quoted 5 lines
Quoted 2 lines
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is
support.
Quoted 7 lines
Quoted 4 lines
So for MMC card, we also support 4bit bus width, otherwiase, we will
get the 12bit bus width, which is not correct:
Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).
Yes, the controller support 8bit MMC.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
FSL ESDHC driver set the host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT; But, the current codes for MMC card has been changed to:
} else {
	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
                         MMC_MODE_WIDTH_BITS_SHIFT);
Hmm... looks like it is code done by me :-)
So little explanation shall be given.
This code is necessary for some targets (like Samsung's Goni) which can only support 4 bit MMC mode.
Quoted 11 lines
Quoted 6 lines
Quoted 6 lines
	for (; width >= 0; width--) {
	....
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.
This problem is MMC subsystem's bug.
I think good that will modify the code in mmc.c. If caps is set to MMC_MODE_4BIT | MMC_MODE_8BIT, we can see the 12bit support with using "mmcinfo" command

Quoted 3 lines

Quoted 2 lines
The mmc_set_bus_width(mmc, 4 * width) in conjunction to above code causes the problem.
then how about using the width[idx] like kernel?

I don't mind :-) if it works at kernel.

Quoted 36 lines

Best Regards,
Jaehoon Chung
Quoted 32 lines
I agree, that this code shall be refactored.
Lei, what do you think?
Quoted 24 lines
Best Regards,
Jaehoon Chung
Quoted 8 lines
Quoted 5 lines
Below is the old codes (width = 2):
} else {
                for (width = EXT_CSD_BUS_WIDTH_8; width >= 0;
width--) {
[...]
Quoted 8 lines
Uh, so it's a bug in the MMC subsystem?
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group

Jaehoon Chung

2012-10-31 10:43 UTC #9 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Hi Lukasz,

	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
	                          MMC_MODE_WIDTH_BITS_SHIFT);

This code has the problem. If width is set to 0x3, then BUS_WIDTH field of ext_csd register is set to 0x3. Value 0x3 is nothing.(It's reserved)
If we want to set 4-bit, then that value is set to 0x2. This problem need to fix.
I will send the patch for this problem.

Best Regards,
Jaehoon Chung

Quoted 89 lines · Lukasz Majewski

Hi Jaehoon,
Quoted 42 lines
Hi, Lukasz,
Quoted 41 lines
Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
Quoted 5 lines
Quoted 2 lines
For the current u-boot codes, only 4bit/1bit SD/SDHC bus width is
support.
Quoted 7 lines
Quoted 4 lines
So for MMC card, we also support 4bit bus width, otherwiase, we will
get the 12bit bus width, which is not correct:
Andy ... can you please explain? I don't quite understand the problem, I thought we had no problem supporting 8bit mmc (esp. if the controller handles that for us mostly).
Yes, the controller support 8bit MMC.
Quoted 7 lines
Quoted 7 lines
Quoted 7 lines
FSL ESDHC driver set the host_caps = MMC_MODE_4BIT | MMC_MODE_8BIT; But, the current codes for MMC card has been changed to:
} else {
	width = ((mmc->host_caps & MMC_MODE_MASK_WIDTH_BITS) >>
                         MMC_MODE_WIDTH_BITS_SHIFT);
Hmm... looks like it is code done by me :-)
So little explanation shall be given.
This code is necessary for some targets (like Samsung's Goni) which can only support 4 bit MMC mode.
Quoted 11 lines
Quoted 6 lines
Quoted 6 lines
	for (; width >= 0; width--) {
	....
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.
This problem is MMC subsystem's bug.
I think good that will modify the code in mmc.c. If caps is set to MMC_MODE_4BIT | MMC_MODE_8BIT, we can see the 12bit support with using "mmcinfo" command
Quoted 3 lines
Quoted 2 lines
The mmc_set_bus_width(mmc, 4 * width) in conjunction to above code causes the problem.
then how about using the width[idx] like kernel?
I don't mind :-) if it works at kernel.
Quoted 36 lines
Best Regards,
Jaehoon Chung
Quoted 32 lines
I agree, that this code shall be refactored.
Lei, what do you think?
Quoted 24 lines
Best Regards,
Jaehoon Chung
Quoted 8 lines
Quoted 5 lines
Below is the old codes (width = 2):
} else {
                for (width = EXT_CSD_BUS_WIDTH_8; width >= 0;
width--) {
[...]
Quoted 8 lines
Uh, so it's a bug in the MMC subsystem?
Best regards,
Marek Vasut
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Andy Fleming

2012-11-01 04:09 UTC #10 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

In reply to Lukasz Majewski

Quoted 19 lines

Hmm... looks like it is code done by me :-)
So little explanation shall be given.
This code is necessary for some targets (like Samsung's Goni) which can only support 4 bit MMC mode.
Quoted 11 lines
Quoted 6 lines
Quoted 6 lines
   for (; width >= 0; width--) {
   ....
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.
This problem is MMC subsystem's bug.
I think good that will modify the code in mmc.c. If caps is set to MMC_MODE_4BIT | MMC_MODE_8BIT, we can see the 12bit support with using "mmcinfo" command

Quoted 5 lines

The mmc_set_bus_width(mmc, 4 * width) in conjunction to above code causes the problem.
I agree, that this code shall be refactored.
Lei, what do you think?

I am... very confused by this whole thread. And the code associated with it. The host_caps field has a bitmask which declares the widths supported by a given controller.

What would possess you to index them by addition, and convert their values by multiplication?? It's a bitfield! I'm embarrassed that I allowed this code in, and will review future submissions from you with a very skeptical eye.

Ah, and further review indicates it is Lei Wen who introduced the idea of iterating through a bitfield by subtraction, though I can see how iterating through the EXT_CSD *field* definition (which looks a lot like a bitfield, but may not be) *might* be considered reasonable.

Meanwhile, Huang Changming, why would you see this broken code, and then decide the best workaround was to cripple our controller by eliminating support for 8-bit?

I'm going to fix this right now. Probably in the quite sensible way that Jaehoon Chung suggested.

Andy

Huang Changming-R66093

2012-11-02 01:54 UTC #11 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Best Regards
Jerry Huang

Quoted 43 lines

-----Original Message-----
From: u-boot-bounces at lists.denx.de [mailto:u-boot-bounces at lists.denx.de] On Behalf Of Andy Fleming
Sent: Thursday, November 01, 2012 12:09 PM
To: Lukasz Majewski
Cc: Jaehoon Chung; Marek Vasut; Lei Wen; u-boot at lists.denx.de
Subject: Re: [U-Boot] [PATCH] powerpc/esdhc: force the bus width to 4bit
Quoted 19 lines
Hmm... looks like it is code done by me :-) So little explanation shall be given.
This code is necessary for some targets (like Samsung's Goni) which can only support 4 bit MMC mode.
Quoted 11 lines
Quoted 6 lines
Quoted 6 lines
   for (; width >= 0; width--) {
   ....
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.
This problem is MMC subsystem's bug.
I think good that will modify the code in mmc.c.
If caps is set to MMC_MODE_4BIT | MMC_MODE_8BIT, we can see the 12bit support with using "mmcinfo" command
Quoted 5 lines
The mmc_set_bus_width(mmc, 4 * width) in conjunction to above code causes the problem.
I agree, that this code shall be refactored.
Lei, what do you think?
I am... very confused by this whole thread. And the code associated with it. The host_caps field has a bitmask which declares the widths supported by a given controller.
What would possess you to index them by addition, and convert their values by multiplication?? It's a bitfield! I'm embarrassed that I allowed this code in, and will review future submissions from you with a very skeptical eye.

Quoted 8 lines

Ah, and further review indicates it is Lei Wen who introduced the idea of iterating through a bitfield by subtraction, though I can see how iterating through the EXT_CSD *field* definition (which looks a lot like a bitfield, but may not be) *might* be considered reasonable.
Meanwhile, Huang Changming, why would you see this broken code, and then decide the best workaround was to cripple our controller by eliminating support for 8-bit?

I see 12bit width when using mmcinfo, then found out the mmc.c has been changed, and I assume this change is right, So I have to force our controller to just support 4bit width, otherwise, the 12bit bus width is not correct.

Jaehoon Chung

2012-11-02 02:03 UTC #12 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Hi,

Andy has sent the patch related with this problem.
Check the patch "[RFC] mmc: Properly determine maximum supported bus width"

Best Regards,
Jaehoon Chung

Quoted 11 lines

Quoted 2 lines
decide the best workaround was to cripple our controller by eliminating support for 8-bit?
I see 12bit width when using mmcinfo, then found out the mmc.c has been changed, and I assume this change is right, So I have to force our controller to just support 4bit width, otherwise, the 12bit bus width is not correct.
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Huang Changming-R66093

2012-11-02 02:15 UTC #13 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Thanks, Jaehoon, I saw it.

Best Regards
Jerry Huang

Quoted 30 lines

-----Original Message-----
From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
Sent: Friday, November 02, 2012 10:03 AM
To: Huang Changming-R66093
Cc: Andy Fleming; Lukasz Majewski; Jaehoon Chung; Marek Vasut; u- boot at lists.denx.de; Wen; Lei at theia.denx.de
Subject: Re: [U-Boot] [PATCH] powerpc/esdhc: force the bus width to 4bit
Hi,
Andy has sent the patch related with this problem.
Check the patch "[RFC] mmc: Properly determine maximum supported bus width"
Best Regards,
Jaehoon Chung
Quoted 4 lines
Quoted 2 lines
decide the best workaround was to cripple our controller by eliminating support for 8-bit?
I see 12bit width when using mmcinfo, then found out the mmc.c has been changed, and I assume this change is right, So I have to force our
controller to just support 4bit width, otherwise, the 12bit bus width is not correct.
Quoted 7 lines
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Marek Vasut

2012-11-02 03:40 UTC #14 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

Dear Huang Changming-R66093,

Quoted 4 lines

Thanks, Jaehoon, I saw it.
Best Regards
Jerry Huang

I wonder if you're just making fun of all of us or what you're actually trying to acomplish here with the constant top-posting ... I'm really stunned :-(

Quoted 23 lines

Quoted 23 lines
-----Original Message-----
From: Jaehoon Chung [mailto:jh80.chung at samsung.com]
Sent: Friday, November 02, 2012 10:03 AM
To: Huang Changming-R66093
Cc: Andy Fleming; Lukasz Majewski; Jaehoon Chung; Marek Vasut; u- boot at lists.denx.de; Wen; Lei at theia.denx.de
Subject: Re: [U-Boot] [PATCH] powerpc/esdhc: force the bus width to 4bit
Hi,
Andy has sent the patch related with this problem.
Check the patch "[RFC] mmc: Properly determine maximum supported bus width"
Best Regards,
Jaehoon Chung
Quoted 5 lines
Quoted 2 lines
decide the best workaround was to cripple our controller by eliminating support for 8-bit?
I see 12bit width when using mmcinfo, then found out the mmc.c has been changed, and I assume this change is right, So I have to force our

Quoted 7 lines

Quoted 7 lines
controller to just support 4bit width, otherwise, the 12bit bus width is not correct.
Quoted 4 lines
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Best regards,
Marek Vasut

Lukasz Majewski

2012-11-02 07:38 UTC #15 lore raw

PATCH 1/1 powerpc/esdhc: force the bus width to 4bit

In reply to Andy Fleming

Hi Andy,

Quoted 26 lines

Quoted 19 lines
Hmm... looks like it is code done by me :-)
So little explanation shall be given.
This code is necessary for some targets (like Samsung's Goni) which can only support 4 bit MMC mode.
Quoted 11 lines
Quoted 6 lines
Quoted 6 lines
   for (; width >= 0; width--) {
   ....
So for FSL ESDHC, the width = 3, after implement mmc_switch successfully, will set the bus to 4 * width. Therefore, I will get the 12bit (4 x 3) bus width.
This problem is MMC subsystem's bug.
I think good that will modify the code in mmc.c. If caps is set to MMC_MODE_4BIT | MMC_MODE_8BIT, we can see the 12bit support with using "mmcinfo" command
Quoted 5 lines
The mmc_set_bus_width(mmc, 4 * width) in conjunction to above code causes the problem.
I agree, that this code shall be refactored.
Lei, what do you think?

Quoted 8 lines

I am... very confused by this whole thread. And the code associated with it. The host_caps field has a bitmask which declares the widths supported by a given controller.
What would possess you to index them by addition, and convert their values by multiplication?? It's a bitfield! I'm embarrassed that I allowed this code in, and will review future submissions from you with a very skeptical eye.

I admit, that this code was a hack, I will even say more (after the "big picture" presented by You) - I've delivered very low quality code.

Shame on me - my fault. Period.

Quoted 14 lines

Ah, and further review indicates it is Lei Wen who introduced the idea of iterating through a bitfield by subtraction, though I can see how iterating through the EXT_CSD *field* definition (which looks a lot like a bitfield, but may not be) *might* be considered reasonable.
Meanwhile, Huang Changming, why would you see this broken code, and then decide the best workaround was to cripple our controller by eliminating support for 8-bit?
I'm going to fix this right now. Probably in the quite sensible way that Jaehoon Chung suggested.
Andy

Signature · 5 lines

Best regards,

Lukasz Majewski

Samsung Poland R&D Center | Linux Platform Group