| Message ID | 1350973580-31726-1-git-send-email-Chang-Ming.Huang@freescale.com |
|---|---|
| State | Rejected |
| Delegated to: | Andy Fleming |
| Headers | show |
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
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;