[U-Boot] [PATCH 1/5] mmc: Tegra2: Support DMA restarts at buffer boundaries

Andy Fleming afleming at gmail.com
Thu Nov 3 02:08:02 CET 2011


On Thu, Oct 13, 2011 at 4:57 PM, Anton Staaf <robotboy at chromium.org> wrote:
> diff --git a/drivers/mmc/tegra2_mmc.c b/drivers/mmc/tegra2_mmc.c
> index 8b6f829..195f89d 100644
> --- a/drivers/mmc/tegra2_mmc.c
> +++ b/drivers/mmc/tegra2_mmc.c
> @@ -256,9 +256,15 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>                                                __func__, mask);
>                                return -1;
>                        } else if (mask & (1 << 3)) {

[...]

> +                               writel((1 << 3), &host->reg->norintsts);
> +                               writel(address, &host->reg->sysad);
>                        } else if (mask & (1 << 1)) {
>                                /* Transfer Complete */
>                                debug("r/w is done\n");
> @@ -442,12 +448,13 @@ static int mmc_core_init(struct mmc *mmc)
>         * NORMAL Interrupt Status Enable Register init
>         * [5] ENSTABUFRDRDY : Buffer Read Ready Status Enable
>         * [4] ENSTABUFWTRDY : Buffer write Ready Status Enable
> +        * [3] ENSTADMAINT : DMA Interrupt Status Enable
>         * [1] ENSTASTANSCMPLT : Transfre Complete Status Enable
>         * [0] ENSTACMDCMPLT : Command Complete Status Enable
> -       */
> +        */
>        mask = readl(&host->reg->norintstsen);
>        mask &= ~(0xffff);
> -       mask |= (1 << 5) | (1 << 4) | (1 << 1) | (1 << 0);
> +       mask |= (1 << 5) | (1 << 4) | (1 << 3) | (1 << 1) | (1 << 0);


I'm pretty sure that a similar patch resulted in a request from
Wolfgang to clean up the magic numbers in the code. Commenting the
numbers above is helpful, but still very prone to maintenance issues.
Why not just set mask:

mask |= (ENSTABUFRDRDY | ENSTABUFWTRDY | ENSTADMAINT | ENSTASTANSCMPLT
| ENSTACMDCMPLT);

If all of the uses of those masks were replaced like that, we'd have
much greater confidence that a random current or future typo wouldn't
break the driver. It would also make it much simpler to identify other
places where that bit was being set/checked.

Please fix, and resubmit.

Andy


More information about the U-Boot mailing list