[U-Boot] [v2] mmc: fsl_esdhc: fix mmc read/write error on T4080

Yangbo Lu yangbo.lu at nxp.com
Thu Jan 21 10:22:30 CET 2016


> -----Original Message-----
> From: Tom Rini [mailto:trini at konsulko.com]
> Sent: Tuesday, January 19, 2016 11:59 PM
> To: Yangbo Lu
> Cc: york sun; Andy Fleming; U-Boot list
> Subject: Re: [U-Boot] [v2] mmc: fsl_esdhc: fix mmc read/write error on
> T4080
> 
> On Mon, Jan 18, 2016 at 07:18:59AM +0000, Yangbo Lu wrote:
> > > -----Original Message-----
> > > From: Tom Rini [mailto:trini at konsulko.com]
> > > Sent: Friday, January 15, 2016 2:09 AM
> > > To: york sun
> > > Cc: Andy Fleming; Yangbo Lu; U-Boot list
> > > Subject: Re: [U-Boot] [v2] mmc: fsl_esdhc: fix mmc read/write error
> > > on
> > > T4080
> > >
> > > On Thu, Jan 14, 2016 at 05:51:32PM +0000, york sun wrote:
> > > > On 01/08/2016 04:23 PM, Andy Fleming wrote:
> > > > > On Thu, Jan 7, 2016 at 2:50 AM, Yangbo Lu <yangbo.lu at nxp.com>
> wrote:
> > > > >> Fill the right command type when using CMD12 to stop data
> transfer.
> > > > >>
> > > > >> Signed-off-by: Yangbo Lu <yangbo.lu at nxp.com>
> > > > >> ---
> > > > >> Changes for v2:
> > > > >>         - Removed fix for T4160 because other patch had done
> > > > >> that
> > > > >> ---
> > > > >>  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 c5054d6..16fb01a 100644
> > > > >> --- a/drivers/mmc/fsl_esdhc.c
> > > > >> +++ b/drivers/mmc/fsl_esdhc.c
> > > > >> @@ -107,7 +107,7 @@ static uint esdhc_xfertyp(struct mmc_cmd
> > > > >> *cmd, struct mmc_data *data)
> > > > >>
> > > > >>  #if defined(CONFIG_MX53) || defined(CONFIG_PPC_T4240) || \
> > > > >>         defined(CONFIG_LS102XA) ||
> > > > >> defined(CONFIG_FSL_LAYERSCAPE) ||
> > > \
> > > > >> -       defined(CONFIG_PPC_T4160)
> > > > >> +       defined(CONFIG_PPC_T4160) || defined(CONFIG_PPC_T4080)
> > > > >>         if (cmd->cmdidx == MMC_CMD_STOP_TRANSMISSION)
> > > > >>                 xfertyp |= XFERTYP_CMDTYP_ABORT;  #endif
> > > > >
> > > > >
> > > > > These sorts of chip-specific #ifdefs are very hard to maintain.
> > > > > It would be far better if there were a single define, like:
> > > > >
> > > > > #ifdef CONFIG_FSL_ESDHC_USES_ABORT_TO_STOP
> > > > >
> > > > > And then define that on any system that needs this code. With
> > > > > the current version, I have no idea why this code is needed. I
> > > > > have guesses, but in order to be sure, I'd have to check several
> > > > > reference manuals. With my suggestion, it is obvious to everyone
> > > > > why this code is here, and it gives a hint to those who are
> > > > > adding support to new chips.
> > > > >
> > > > > Now, I'm not saying you should use my suggestion precisely.
> > > > > Perhaps every version of the esdhc after some point uses this
> > > > > mechanism. Then you could use that information. And perhaps my
> > > > > naming doesn't reflect what is happening. My point is, you're
> > > > > going to have to do this again when you release LS232XB, and that
> seems like a poor use of your time.
> > > > > :)
> > > >
> > > > Yangbo,
> > > >
> > > > I agree with Andy on this. Please make the suggested change.
> > >
> > > ... as some sort of Kconfig entry that's ideally selected rather
> > > than prmopted for when applicable.  Like it would be done in the
> > > kernel :)
> > >
> > > --
> > > Tom
> > [Lu Yangbo-B47093] Hi Tom, I want to reconfirm whether your suggestion
> is same with Andy and York's.
> > Could I define it in include/configs/xxxx.h? Or use Kconfig?
> 
> I'm agreeing with what Andy/York say and noting that the right way to fix
> this is not to add something to include/configs/ but to use Kconfig and
> have the platforms select the symbol (rather than prompt for it), similar
> to how flags like this would work in the Linux kernel.
> 
> --
> Tom

[Lu Yangbo-B47093] Thank you. But I just find I need to remove all the #ifdef rather than add one.
Because the MMC_CMD_STOP_TRANSMISSION command must be set a XFERTYP_CMDTYP_ABORT command type according to SD spec.

A new version patch would be sent later.

Best regards,
Yangbo Lu


More information about the U-Boot mailing list