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

Anton Staaf robotboy at chromium.org
Thu Nov 3 02:10:44 CET 2011


On Wed, Nov 2, 2011 at 6:08 PM, Andy Fleming <afleming at gmail.com> wrote:
> 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.

Will do.

Thanks,
     Anton

> Andy
>


More information about the U-Boot mailing list