From mboxrd@z Thu Jan  1 00:00:00 1970
From: Marek Vasut <marex@denx.de>
Date: Wed, 10 Feb 2016 14:45:48 +0100
Subject: [U-Boot] [PATCH v2 1/3] mmc: uniphier: add driver for UniPhier
 SD/MMC host controller
In-Reply-To: <1455110911-12224-2-git-send-email-yamada.masahiro@socionext.com>
References: <1455110911-12224-1-git-send-email-yamada.masahiro@socionext.com>
 <1455110911-12224-2-git-send-email-yamada.masahiro@socionext.com>
Message-ID: <56BB3F0C.2040002@denx.de>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.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.masahiro@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.

> +DECLARE_GLOBAL_DATA_PTR;
> +
> +#define UNIPHIER_SD_CMD			0x000	/* command */

[...]

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

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

> +	writel(dma_addr & U32_MAX, priv->regbase + UNIPHIER_SD_DMA_ADDR_L);
> +
> +	/* suppress the warning "right shift count >= width of type" */
> +	dma_addr >>= min_t(int, 32, 8 * sizeof(dma_addr));
> +
> +	writel(dma_addr & U32_MAX, priv->regbase + UNIPHIER_SD_DMA_ADDR_H);
> +
> +	writel(UNIPHIER_SD_DMA_CTL_START, priv->regbase + UNIPHIER_SD_DMA_CTL);
> +}
> +
> +static int uniphier_sd_dma_wait_irq(struct uniphier_sd_priv *priv, u32 flag,
> +				    unsigned int blocks)
> +{
> +	long wait = 1000000 + 10 * blocks;

wait_for_bit() again.

> +	while (!(readl(priv->regbase + UNIPHIER_SD_DMA_INFO1) & flag)) {
> +		if (wait-- < 0) {
> +			pr_err("timeout during DMA\n");
> +			return -ETIMEDOUT;
> +		}
> +
> +		udelay(10);
> +	}
> +
> +	if (readl(priv->regbase + UNIPHIER_SD_DMA_INFO2)) {
> +		pr_err("error during DMA\n");
> +		return -EIO;
> +	}
> +
> +	return 0;
> +}

??