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

Andy Fleming afleming at gmail.com
Sat Jan 9 01:23:07 CET 2016


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.
:)

Andy


More information about the U-Boot mailing list