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 <[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.
... 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
signature.asc
Description: Digital signature
_______________________________________________ U-Boot mailing list [email protected] http://lists.denx.de/mailman/listinfo/u-boot
