On 01/08/2016 04:23 PM, Andy Fleming wrote: > On Thu, Jan 7, 2016 at 2:50 AM, Yangbo Lu <[email protected]> wrote: >> Fill the right command type when using CMD12 to stop data transfer. >> >> Signed-off-by: Yangbo Lu <[email protected]> >> --- >> 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. York _______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
