Hi Marek,
2016-02-10 22:45 GMT+09:00 Marek Vasut <ma...@denx.de>:
> On 02/10/2016 02:28 PM, Masahiro Yamada wrote:
>> Add a driver for the on-chip SD/eMMC host controller used by
>> UniPhier SoC family.
>>
>> Signed-off-by: Masahiro Yamada <yamada.masah...@socionext.com>
>
> 前略 山田さん,
>
> [...]
>
>> +#include <common.h>
>> +#include <clk.h>
>> +#include <fdtdec.h>
>> +#include <mapmem.h>
>> +#include <mmc.h>
>> +#include <dm/device.h>
>> +#include <linux/compat.h>
>> +#include <linux/io.h>
>> +#include <asm/unaligned.h>
>> +#include <asm/dma-mapping.h>
>> +
>> +#define pr_err printf
>> +#define pr_warn printf
>> +#ifdef DEBUG
>> +#define pr_debug printf
>> +#else
>> +#define pr_debug(...)
>> +#endif
>
> This should go into include/ somewhere, your driver shouldn't be a
> platform abstraction library.
Fixed in v3.
>> +static int uniphier_sd_wait_irq(struct uniphier_sd_priv *priv,
>> + unsigned int reg, u32 flag)
>> +{
>> + long wait = 1000000;
>> + int ret;
>
> Replace this with wait_for_bit() please .
It cannot check error during the loop.
I want to check the error flags in the loop
because no reason to wait for the time-out
if some error happens.
>> + while (!(readl(priv->regbase + reg) & flag)) {
>> + if (wait-- < 0) {
>> + pr_err("timeout\n");
>> + return -ETIMEDOUT;
>> + }
>> +
>> + ret = uniphier_sd_check_error(priv);
>> + if (ret)
>> + return ret;
>> +
>> + udelay(1);
>> + }
>> +
>> + return 0;
>> +}
>
> [...]
>
>> +static void uniphier_sd_dma_start(struct uniphier_sd_priv *priv,
>> + dma_addr_t dma_addr)
>> +{
>> + u32 tmp;
>> +
>> + writel(0, priv->regbase + UNIPHIER_SD_DMA_INFO1);
>> + writel(0, priv->regbase + UNIPHIER_SD_DMA_INFO2);
>> +
>> + /* enable DMA */
>> + tmp = readl(priv->regbase + UNIPHIER_SD_EXTMODE);
>> + tmp |= UNIPHIER_SD_EXTMODE_DMA_EN;
>> + writel(tmp, priv->regbase + UNIPHIER_SD_EXTMODE);
>
> I'd say, use setbits_le32(), but could it be that this driver is kept in
> sync with Linux ?
Yes, I am developing the MMC driver
for Linux as well as U-Boot at the same time.
This is the U-Boot counter-part,
although the Linux one has not been upstreamed yet.
It can save my time to copy-paste code snippets between the two.
I want to sync as much code as possible.
--
Best Regards
Masahiro Yamada
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
