[U-Boot] [PATCH] powerpc/esdhc: force the bus width to 4bit
From: Jerry Huang Chang-Ming.Huang@freescale.com
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 afleming@gmail.com CC: Marek Vasut marex@denx.de --- drivers/mmc/fsl_esdhc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3f8d30d..7b83dd2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -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;
Dear Chang-Ming.Huang@freescale.com,
From: Jerry Huang Chang-Ming.Huang@freescale.com
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).
=> 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 afleming@gmail.com CC: Marek Vasut marex@denx.de
drivers/mmc/fsl_esdhc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3f8d30d..7b83dd2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -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
Best Regards Jerry Huang
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Tuesday, October 23, 2012 3:24 PM To: Huang Changming-R66093 Cc: u-boot@lists.denx.de; Andy Fleming Subject: Re: [PATCH] powerpc/esdhc: force the bus width to 4bit
Dear Chang-Ming.Huang@freescale.com,
From: Jerry Huang Chang-Ming.Huang@freescale.com
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).
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--) {
=> 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 afleming@gmail.com CC: Marek Vasut marex@denx.de
drivers/mmc/fsl_esdhc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/mmc/fsl_esdhc.c b/drivers/mmc/fsl_esdhc.c index 3f8d30d..7b83dd2 100644 --- a/drivers/mmc/fsl_esdhc.c +++ b/drivers/mmc/fsl_esdhc.c @@ -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
Dear Huang Changming-R66093,
Best Regards Jerry Huang
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Tuesday, October 23, 2012 3:24 PM To: Huang Changming-R66093 Cc: u-boot@lists.denx.de; Andy Fleming Subject: Re: [PATCH] powerpc/esdhc: force the bus width to 4bit
Dear Chang-Ming.Huang@freescale.com,
From: Jerry Huang Chang-Ming.Huang@freescale.com
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).
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--) {
[...]
Uh, so it's a bug in the MMC subsystem? Best regards, Marek Vasut
Dear Chang-Ming.Huang@freescale.com,
From: Jerry Huang Chang-Ming.Huang@freescale.com
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).
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--) {
[...]
Uh, so it's a bug in the MMC subsystem? Best regards,
I don't know. Maybe Andy can give some comment.
On 10/23/2012 06:50 PM, Marek Vasut wrote:
Dear Huang Changming-R66093,
Best Regards Jerry Huang
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Tuesday, October 23, 2012 3:24 PM To: Huang Changming-R66093 Cc: u-boot@lists.denx.de; Andy Fleming Subject: Re: [PATCH] powerpc/esdhc: force the bus width to 4bit
Dear Chang-Ming.Huang@freescale.com,
From: Jerry Huang Chang-Ming.Huang@freescale.com
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).
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
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Jaehoon,
On 10/23/2012 06:50 PM, Marek Vasut wrote:
Dear Huang Changming-R66093,
Best Regards Jerry Huang
-----Original Message----- From: Marek Vasut [mailto:marex@denx.de] Sent: Tuesday, October 23, 2012 3:24 PM To: Huang Changming-R66093 Cc: u-boot@lists.denx.de; Andy Fleming Subject: Re: [PATCH] powerpc/esdhc: force the bus width to 4bit
Dear Chang-Ming.Huang@freescale.com,
From: Jerry Huang Chang-Ming.Huang@freescale.com
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).
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.
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?
Best Regards, Jaehoon Chung
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi, Lukasz,
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).
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.
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.
then how about using the width[idx] like kernel?
Best Regards, Jaehoon Chung
I agree, that this code shall be refactored. Lei, what do you think?
Best Regards, Jaehoon Chung
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Hi Jaehoon,
Hi, Lukasz,
> 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).
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.
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.
then how about using the width[idx] like kernel?
I don't mind :-) if it works at kernel.
Best Regards, Jaehoon Chung
I agree, that this code shall be refactored. Lei, what do you think?
Best Regards, Jaehoon Chung
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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
On 10/31/2012 05:20 PM, Lukasz Majewski wrote:
Hi Jaehoon,
Hi, Lukasz,
>> 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).
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.
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.
then how about using the width[idx] like kernel?
I don't mind :-) if it works at kernel.
Best Regards, Jaehoon Chung
I agree, that this code shall be refactored. Lei, what do you think?
Best Regards, Jaehoon Chung
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
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.
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?
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
Best Regards Jerry Huang
-----Original Message----- From: u-boot-bounces@lists.denx.de [mailto:u-boot-bounces@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@lists.denx.de Subject: Re: [U-Boot] [PATCH] powerpc/esdhc: force the bus width to 4bit
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.
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?
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 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.
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
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Thanks, Jaehoon, I saw it.
Best Regards Jerry Huang
-----Original Message----- From: Jaehoon Chung [mailto:jh80.chung@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@lists.denx.de; Wen; Lei@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
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Dear Huang Changming-R66093,
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 :-(
-----Original Message----- From: Jaehoon Chung [mailto:jh80.chung@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@lists.denx.de; Wen; Lei@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
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@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot
Best regards, Marek Vasut
Hi Andy,
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.
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?
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.
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
participants (6)
-
Andy Fleming -
Chang-Ming.Huang＠freescale.com -
Huang Changming-R66093 -
Jaehoon Chung -
Lukasz Majewski -
Marek Vasut