[U-Boot] [PATCH v2 1/3] mmc: uniphier: add driver for UniPhier SD/MMC host controller

[Marek Vasut <marex@denx.de>]

On 02/16/2016 09:18 AM, Masahiro Yamada wrote:
> Hi Marek,

Hi!

[...]

>>> +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.

You have a point there.

I wonder if it'd make sense to extend the wait_for_bit with some
callback maybe ?

>>> +     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.

Understood, thanks!