[U-Boot] [PATCH 3/3] mmc: tegra: use bounce buffer APIs
Stephen Warren
swarren at wwwdotorg.org
Tue Nov 6 19:50:22 CET 2012
On 11/05/2012 05:00 PM, Simon Glass wrote:
> Hi Stephen,
>
> On Mon, Nov 5, 2012 at 3:04 PM, Stephen Warren <swarren at wwwdotorg.org> wrote:
>> From: Stephen Warren <swarren at nvidia.com>
>>
>> Tegra's MMC driver does DMA, and hence needs cache-aligned buffers. In
>> some cases (e.g. user load commands) this cannot be guaranteed by callers
>> of the MMC APIs. To solve this, modify the Tegra MMC driver to use the
>> new bounce_buffer_*() APIs.
>> diff --git a/drivers/mmc/tegra_mmc.c b/drivers/mmc/tegra_mmc.c
>> #include <asm/arch-tegra/clk_rst.h>
>> #include <asm/arch-tegra/tegra_mmc.h>
>> #include <mmc.h>
>> +#include <bouncebuf.h>
>
> The order seems wrong here - I think bouncebuf and mmc should go above
> the asm/ ones, and bouncebuf should be first.
Is there a defined order for header files? I suppose I should try and
read and remember more documentation!
>> @@ -180,8 +196,10 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
>> if (data)
>> mmc_set_transfer_mode(host, data);
>>
>> - if ((cmd->resp_type & MMC_RSP_136) && (cmd->resp_type & MMC_RSP_BUSY))
>> - return -1;
>> + if ((cmd->resp_type & MMC_RSP_136) && (cmd->resp_type & MMC_RSP_BUSY)) {
>> + ret = -1;
>> + goto cleanup;
>> + }
>
> You might consider putting this body in a function so you don't need
> these two lines everywhere below.
I'm not quite sure how a function would work here; a function can't
really goto. Do you mean a macro? I'd tend to this a macro would
obfuscate the pretty simple code.
Oh, perhaps you mean having a new top-level function that does:
bounce_buffer_start();
calls a function to do all the work
bounce_buffer_stop();
That would certainly simplify the patch.
>> diff --git a/include/configs/tegra-common-post.h b/include/configs/tegra-common-post.h
>> +#ifdef CONFIG_TEGRA_MMC
>> +#define CONFIG_BOUNCE_BUFFER
>> +#endif
>
> Is there really any harm in just defining this always (say in the
> tegra20-common.h)? The functions should be dropped if not used.
I suppose it'd be fine to always enable this since the linker should
drop the functions when not referenced. Of course, that relies on
bouncebuf.o not having any global side-effects (e.g. registering things
via custom linker segments that are always pulled in). The code above
represents the actual dependency too; hopefully one day U-Boot will
sprout Kconfig, and that logic can be replaced by:
config TEGRA_MMC
select BOUNCE_BUFFER
More information about the U-Boot
mailing list