[U-Boot,v6,1/1] ARM: kirkwood: add mvsdio driver

This patch add Marvell kirkwood MVSDIO/MMC driver
and enable it for Sheevaplugs and OpenRD boards.

Signed-off-by: Gerald Kerma <drEagle@doukki.net>
---

 Changes in v6:
 - rename driver to mvebu_mmc
 - renames constants and functions
 - move mmc_init to kirkwood/cpu
 - code cleaning

 Changes in v5:
 - purge uneeded code
 - remove some extra debug

 Changes in v4:
 - rename drivers files to MVSDIO
 - fix MMC clock init which now use dev ID
 - clean debug strings
 - remove MVSDIO_TWEAK_NOSDHS quirk
 - remove dead code

 Changes in v3:
 - Add MVSDIO_TWEAK_NOSDHS quirk
 - Minor clean

 Changes in v2:
 - Fix some typo and missing lines from patch import

 arch/arm/cpu/arm926ejs/kirkwood/cpu.c         |  11 +
 arch/arm/include/asm/arch-kirkwood/kirkwood.h |   1 +
 drivers/mmc/Makefile                          |   1 +
 drivers/mmc/mvebu_mmc.c                       | 355 ++++++++++++++++++++++++++
 include/configs/openrd.h                      |   8 +
 include/configs/sheevaplug.h                  |  17 +-
 include/mvebu_mmc.h                           | 279 ++++++++++++++++++++
 7 files changed, 670 insertions(+), 2 deletions(-)
 create mode 100644 drivers/mmc/mvebu_mmc.c
 create mode 100644 include/mvebu_mmc.h

On 25.07.2014 19:07, Gerald Kerma wrote:
> This patch add Marvell kirkwood MVSDIO/MMC driver
> and enable it for Sheevaplugs and OpenRD boards.
>
> Signed-off-by: Gerald Kerma <drEagle@doukki.net>
> ---
>
>   Changes in v6:
>   - rename driver to mvebu_mmc
>   - renames constants and functions
>   - move mmc_init to kirkwood/cpu
>   - code cleaning

Thanks. This really looks better.

Only some mostly nitpicking comments below.

>
>   Changes in v5:
>   - purge uneeded code
>   - remove some extra debug
>
>   Changes in v4:
>   - rename drivers files to MVSDIO
>   - fix MMC clock init which now use dev ID
>   - clean debug strings
>   - remove MVSDIO_TWEAK_NOSDHS quirk
>   - remove dead code
>
>   Changes in v3:
>   - Add MVSDIO_TWEAK_NOSDHS quirk
>   - Minor clean
>
>   Changes in v2:
>   - Fix some typo and missing lines from patch import
>
>   arch/arm/cpu/arm926ejs/kirkwood/cpu.c         |  11 +
>   arch/arm/include/asm/arch-kirkwood/kirkwood.h |   1 +
>   drivers/mmc/Makefile                          |   1 +
>   drivers/mmc/mvebu_mmc.c                       | 355 ++++++++++++++++++++++++++
>   include/configs/openrd.h                      |   8 +
>   include/configs/sheevaplug.h                  |  17 +-
>   include/mvebu_mmc.h                           | 279 ++++++++++++++++++++
>   7 files changed, 670 insertions(+), 2 deletions(-)
>   create mode 100644 drivers/mmc/mvebu_mmc.c
>   create mode 100644 include/mvebu_mmc.h
>
> diff --git a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
> index da80240..94db160 100644
> --- a/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
> +++ b/arch/arm/cpu/arm926ejs/kirkwood/cpu.c
> @@ -13,6 +13,9 @@
>   #include <asm/io.h>
>   #include <asm/arch/cpu.h>
>   #include <asm/arch/kirkwood.h>
> +#ifdef CONFIG_MVEBU_MMC
> +# include <mvebu_mmc.h>
> +#endif /* CONFIG_MVEBU_MMC */

We try not to add unneeded #ifdef's to the code. This #ifdef for the 
header most likely can be removed. While at it, please change:

+# include <mvebu_mmc.h>

to

+#include <mvebu_mmc.h>

(no space)

>
>   #define BUFLEN	16
>
> @@ -377,3 +380,11 @@ int cpu_eth_init(bd_t *bis)
>   	return 0;
>   }
>   #endif
> +
> +#ifdef CONFIG_MVEBU_MMC
> +int board_mmc_init(bd_t *bis)
> +{
> +	mvebu_mmc_init(bis);
> +	return 0;
> +}
> +#endif /* CONFIG_MVEBU_MMC */
> diff --git a/arch/arm/include/asm/arch-kirkwood/kirkwood.h b/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> index bc207f5..3ea51d7 100644
> --- a/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> +++ b/arch/arm/include/asm/arch-kirkwood/kirkwood.h
> @@ -39,6 +39,7 @@
>   #define KW_EGIGA0_BASE			(KW_REGISTER(0x72000))
>   #define KW_EGIGA1_BASE			(KW_REGISTER(0x76000))
>   #define KW_SATA_BASE			(KW_REGISTER(0x80000))
> +#define KW_SDIO_BASE			(KW_REGISTER(0x90000))
>
>   /* Kirkwood Sata controller has two ports */
>   #define KW_SATA_PORT0_OFFSET		0x2000
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 34febf5..f5be96f 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -37,3 +37,4 @@ obj-$(CONFIG_SPL_MMC_BOOT) += fsl_esdhc_spl.o
>   else
>   obj-$(CONFIG_GENERIC_MMC) += mmc_write.o
>   endif
> +obj-$(CONFIG_MVEBU_MMC) += mvebu_mmc.o
> diff --git a/drivers/mmc/mvebu_mmc.c b/drivers/mmc/mvebu_mmc.c
> new file mode 100644
> index 0000000..fbcb656
> --- /dev/null
> +++ b/drivers/mmc/mvebu_mmc.c
> @@ -0,0 +1,355 @@
> +/*
> + * Marvell MMC/SD/SDIO driver
> + *
> + * (C) Copyright 2012
> + * Marvell Semiconductor <www.marvell.com>
> + * Written-by: Maen Suleiman, Gerald Kerma
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <part.h>
> +#include <mmc.h>
> +#include <asm/io.h>
> +#include <asm/arch/cpu.h>
> +#include <asm/arch/kirkwood.h>
> +
> +#include <mvebu_mmc.h>
> +
> +#define DRIVER_NAME	"MVEBU_MMC"
> +
> +#define mvebu_mmc_write(offs, val) writel(val, CONFIG_SYS_MMC_BASE + (offs))
> +#define mvebu_mmc_read(offs) readl(CONFIG_SYS_MMC_BASE + (offs))

Wouldn't it be better to use functions instead of macros here?

> +#define MVEBU_MMC_MAKE_CMD(c, f) (((c & 0xff) << 8) | (f & 0xff))
> +
> +static int mvebu_mmc_setup_data(struct mmc_data *data)
> +{
> +	u32 ctrl_reg;
> +
> +	debug("%s, data %s : blocks=%d blksz=%d\n", DRIVER_NAME,
> +	      (data->flags & MMC_DATA_READ) ? "read" : "write",
> +	      data->blocks, data->blocksize);
> +
> +	/* default to maximum timeout */
> +	ctrl_reg = mvebu_mmc_read(SDIO_HOST_CTRL);
> +	ctrl_reg |= SDIO_HOST_CTRL_TMOUT(SDIO_HOST_CTRL_TMOUT_MAX);
> +	mvebu_mmc_write(SDIO_HOST_CTRL, ctrl_reg);
> +
> +	if (data->flags & MMC_DATA_READ) {
> +		mvebu_mmc_write(SDIO_SYS_ADDR_LOW, (u32)data->dest & 0xffff);
> +		mvebu_mmc_write(SDIO_SYS_ADDR_HI, (u32)data->dest >> 16);
> +	} else {
> +		mvebu_mmc_write(SDIO_SYS_ADDR_LOW, (u32)data->src & 0xffff);
> +		mvebu_mmc_write(SDIO_SYS_ADDR_HI, (u32)data->src >> 16);
> +	}
> +
> +	mvebu_mmc_write(SDIO_BLK_COUNT, data->blocks);
> +	mvebu_mmc_write(SDIO_BLK_SIZE, data->blocksize);
> +
> +	return 0;
> +}
> +
> +static int mvebu_mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
> +			      struct mmc_data *data)
> +{
> +	int timeout = 10;
> +	ushort waittype = 0;
> +	ushort resptype = 0;
> +	ushort xfertype = 0;
> +	ushort resp_indx = 0;
> +
> +	debug("cmdidx [0x%x] resp_type[0x%x] cmdarg[0x%x]\n",
> +	      cmd->cmdidx, cmd->resp_type, cmd->cmdarg);
> +
> +	udelay(10*1000);
> +
> +	debug("%s: cmd %d (hw state 0x%04x)\n", DRIVER_NAME,
> +	      cmd->cmdidx, mvebu_mmc_read(SDIO_HW_STATE));
> +
> +	/* Checking if card is busy */
> +	while ((mvebu_mmc_read(SDIO_HW_STATE) & CARD_BUSY)) {
> +		if (timeout == 0) {
> +			printf("%s: card busy!\n", DRIVER_NAME);
> +			return -1;
> +		}
> +		timeout--;
> +		udelay(1000);
> +	}
> +
> +	/* Set up for a data transfer if we have one */
> +	if (data) {
> +		int err = mvebu_mmc_setup_data(data);
> +
> +		if (err)
> +			return err;
> +	}
> +
> +	resptype = SDIO_CMD_INDEX(cmd->cmdidx);
> +
> +	/* Analyzing resptype/xfertype/waittype for the command */
> +	if (cmd->resp_type & MMC_RSP_BUSY)
> +		resptype |= SDIO_CMD_RSP_48BUSY;
> +	else if (cmd->resp_type & MMC_RSP_136)
> +		resptype |= SDIO_CMD_RSP_136;
> +	else if (cmd->resp_type & MMC_RSP_PRESENT)
> +		resptype |= SDIO_CMD_RSP_48;
> +	else
> +		resptype |= SDIO_CMD_RSP_NONE;
> +
> +	if (cmd->resp_type & MMC_RSP_CRC)
> +		resptype |= SDIO_CMD_CHECK_CMDCRC;
> +
> +	if (cmd->resp_type & MMC_RSP_OPCODE)
> +		resptype |= SDIO_CMD_INDX_CHECK;
> +
> +	if (cmd->resp_type & MMC_RSP_PRESENT) {
> +		resptype |= SDIO_UNEXPECTED_RESP;
> +		waittype |= SDIO_NOR_UNEXP_RSP;
> +	}
> +
> +	if (data) {
> +		resptype |= SDIO_CMD_DATA_PRESENT | SDIO_CMD_CHECK_DATACRC16;
> +		xfertype |= SDIO_XFER_MODE_HW_WR_DATA_EN;
> +		if (data->flags & MMC_DATA_READ) {
> +			xfertype |= SDIO_XFER_MODE_TO_HOST;
> +			waittype = SDIO_NOR_DMA_INI;
> +		} else {
> +			waittype |= SDIO_NOR_XFER_DONE;
> +		}
> +	} else {
> +		waittype |= SDIO_NOR_CMD_DONE;
> +	}
> +
> +	/* Setting cmd arguments */
> +	mvebu_mmc_write(SDIO_ARG_LOW, cmd->cmdarg & 0xffff);
> +	mvebu_mmc_write(SDIO_ARG_HI, cmd->cmdarg >> 16);
> +
> +	/* Setting Xfer mode */
> +	mvebu_mmc_write(SDIO_XFER_MODE, xfertype);
> +
> +	mvebu_mmc_write(SDIO_NOR_INTR_STATUS, ~SDIO_NOR_CARD_INT);
> +	mvebu_mmc_write(SDIO_ERR_INTR_STATUS, SDIO_POLL_MASK);
> +
> +	/* Sending command */
> +	mvebu_mmc_write(SDIO_CMD, resptype);
> +
> +	mvebu_mmc_write(SDIO_NOR_INTR_EN, SDIO_POLL_MASK);
> +	mvebu_mmc_write(SDIO_ERR_INTR_EN, SDIO_POLL_MASK);
> +
> +	/* Waiting for completion */
> +	timeout = 1000000;
> +
> +	while (!((mvebu_mmc_read(SDIO_NOR_INTR_STATUS)) & waittype)) {
> +		if (mvebu_mmc_read(SDIO_NOR_INTR_STATUS) & SDIO_NOR_ERROR) {
> +			debug("%s: error! cmdidx : %d, err reg: %04x\n",
> +			      DRIVER_NAME, cmd->cmdidx,
> +			      mvebu_mmc_read(SDIO_ERR_INTR_STATUS));
> +			if (mvebu_mmc_read(SDIO_ERR_INTR_STATUS) &
> +				(SDIO_ERR_CMD_TIMEOUT | SDIO_ERR_DATA_TIMEOUT))
> +				return TIMEOUT;
> +			return COMM_ERR;
> +		}
> +
> +		timeout--;
> +		udelay(1);
> +		if (timeout <= 0) {
> +			printf("%s: command timed out\n", DRIVER_NAME);
> +			return TIMEOUT;
> +		}
> +	}
> +
> +	/* Handling response */
> +	if (cmd->resp_type & MMC_RSP_136) {
> +		uint response[8];
> +
> +		for (resp_indx = 0; resp_indx < 8; resp_indx++)
> +			response[resp_indx]
> +				= mvebu_mmc_read(SDIO_RSP(resp_indx));
> +
> +		cmd->response[0] =	((response[0] & 0x03ff) << 22) |
> +					((response[1] & 0xffff) << 6) |
> +					((response[2] & 0xfc00) >> 10);
> +		cmd->response[1] =	((response[2] & 0x03ff) << 22) |
> +					((response[3] & 0xffff) << 6) |
> +					((response[4] & 0xfc00) >> 10);
> +		cmd->response[2] =	((response[4] & 0x03ff) << 22) |
> +					((response[5] & 0xffff) << 6) |
> +					((response[6] & 0xfc00) >> 10);
> +		cmd->response[3] =	((response[6] & 0x03ff) << 22) |
> +					((response[7] & 0x3fff) << 8);
> +	} else if (cmd->resp_type & MMC_RSP_PRESENT) {
> +		uint response[3];
> +
> +		for (resp_indx = 0; resp_indx < 3; resp_indx++)
> +			response[resp_indx]
> +				= mvebu_mmc_read(SDIO_RSP(resp_indx));
> +
> +		cmd->response[0] =	((response[2] & 0x003f) << (8 - 8)) |
> +					((response[1] & 0xffff) << (14 - 8)) |
> +					((response[0] & 0x03ff) << (30 - 8));
> +		cmd->response[1] =	((response[0] & 0xfc00) >> 10);
> +		cmd->response[2] =	0;
> +		cmd->response[3] =	0;
> +	}
> +
> +	debug("%s: resp[0x%x] ", DRIVER_NAME, cmd->resp_type);
> +	debug("[0x%x] ", cmd->response[0]);
> +	debug("[0x%x] ", cmd->response[1]);
> +	debug("[0x%x] ", cmd->response[2]);
> +	debug("[0x%x] ", cmd->response[3]);
> +	debug("\n");
> +
> +	return 0;
> +}
> +
> +static void mvebu_mmc_power_up(void)
> +{
> +	debug("%s: power up\n", DRIVER_NAME);
> +
> +	/* disable interrupts */
> +	mvebu_mmc_write(SDIO_NOR_INTR_EN, 0);
> +	mvebu_mmc_write(SDIO_ERR_INTR_EN, 0);
> +
> +	/* SW reset */
> +	mvebu_mmc_write(SDIO_SW_RESET, SDIO_SW_RESET_NOW);
> +
> +	mvebu_mmc_write(SDIO_XFER_MODE, 0);
> +
> +	/* enable status */
> +	mvebu_mmc_write(SDIO_NOR_STATUS_EN, SDIO_POLL_MASK);
> +	mvebu_mmc_write(SDIO_ERR_STATUS_EN, SDIO_POLL_MASK);
> +
> +	/* enable interrupts status */
> +	mvebu_mmc_write(SDIO_NOR_INTR_STATUS, SDIO_POLL_MASK);
> +	mvebu_mmc_write(SDIO_ERR_INTR_STATUS, SDIO_POLL_MASK);
> +}
> +
> +static void mvebu_mmc_set_clk(unsigned int clock)
> +{
> +	unsigned int m;
> +
> +	if (clock == 0) {
> +		debug("%s: clock off\n", DRIVER_NAME);
> +		mvebu_mmc_write(SDIO_XFER_MODE, SDIO_XFER_MODE_STOP_CLK);
> +		mvebu_mmc_write(SDIO_CLK_DIV, MVEBU_MMC_BASE_DIV_MAX);
> +	} else {
> +		m = MVEBU_MMC_BASE_FAST_CLOCK/(2*clock) - 1;
> +		if (m > MVEBU_MMC_BASE_DIV_MAX)
> +			m = MVEBU_MMC_BASE_DIV_MAX;
> +		mvebu_mmc_write(SDIO_CLK_DIV, m & MVEBU_MMC_BASE_DIV_MAX);
> +	}

Empty line here.

> +	udelay(10*1000);
> +}
> +
> +static void mvebu_mmc_set_bus(unsigned int bus)
> +{
> +	u32 ctrl_reg = 0;
> +
> +	ctrl_reg = mvebu_mmc_read(SDIO_HOST_CTRL);
> +	ctrl_reg &= ~SDIO_HOST_CTRL_DATA_WIDTH_4_BITS;
> +
> +	switch (bus) {
> +	case 4:
> +		ctrl_reg |= SDIO_HOST_CTRL_DATA_WIDTH_4_BITS;
> +		break;
> +	case 1:
> +	default:
> +		ctrl_reg |= SDIO_HOST_CTRL_DATA_WIDTH_1_BIT;
> +	}

Empty line.

> +	/* default transfer mode */
> +	ctrl_reg |= SDIO_HOST_CTRL_BIG_ENDIAN;
> +	ctrl_reg &= ~SDIO_HOST_CTRL_LSB_FIRST;
> +
> +	/* default to maximum timeout */
> +	ctrl_reg |= SDIO_HOST_CTRL_TMOUT(SDIO_HOST_CTRL_TMOUT_MAX);
> +
> +	ctrl_reg |= SDIO_HOST_CTRL_PUSH_PULL_EN;
> +
> +	ctrl_reg |= SDIO_HOST_CTRL_CARD_TYPE_MEM_ONLY;
> +
> +	debug("%s: ctrl 0x%04x: %s %s %s\n", DRIVER_NAME, ctrl_reg,
> +	      (ctrl_reg & SDIO_HOST_CTRL_PUSH_PULL_EN) ?
> +	      "push-pull" : "open-drain",
> +	      (ctrl_reg & SDIO_HOST_CTRL_DATA_WIDTH_4_BITS) ?
> +	      "4bit-width" : "1bit-width",
> +	      (ctrl_reg & SDIO_HOST_CTRL_HI_SPEED_EN) ?
> +	      "high-speed" : "");
> +
> +	mvebu_mmc_write(SDIO_HOST_CTRL, ctrl_reg);
> +	udelay(10*1000);
> +}
> +
> +static void mvebu_mmc_set_ios(struct mmc *mmc)
> +{
> +	debug("%s: bus[%d] clock[%d]\n", DRIVER_NAME,
> +	      mmc->bus_width, mmc->clock);
> +	mvebu_mmc_set_bus(mmc->bus_width);
> +	mvebu_mmc_set_clk(mmc->clock);
> +}
> +
> +static int mvebu_mmc_initialize(struct mmc *mmc)
> +{
> +	debug("%s: mvebu_mmc_initialize", DRIVER_NAME);
> +
> +/*
> + * Setting host parameters
> + * Initial Host Ctrl : Timeout : max , Normal Speed mode, 4-bit data mode
> + * Big Endian, SD memory Card, Push_pull CMD Line
> + */

Indetation of the comment seems wrong.

> +	mvebu_mmc_write(SDIO_HOST_CTRL,
> +			SDIO_HOST_CTRL_TMOUT(SDIO_HOST_CTRL_TMOUT_MAX) |
> +			SDIO_HOST_CTRL_DATA_WIDTH_4_BITS |
> +			SDIO_HOST_CTRL_BIG_ENDIAN |
> +			SDIO_HOST_CTRL_PUSH_PULL_EN |
> +			SDIO_HOST_CTRL_CARD_TYPE_MEM_ONLY);
> +
> +	mvebu_mmc_write(SDIO_CLK_CTRL, 0);
> +
> +	/* enable status */
> +	mvebu_mmc_write(SDIO_NOR_STATUS_EN, SDIO_POLL_MASK);
> +	mvebu_mmc_write(SDIO_ERR_STATUS_EN, SDIO_POLL_MASK);
> +
> +	/* disable interrupts */
> +	mvebu_mmc_write(SDIO_NOR_INTR_EN, 0);
> +	mvebu_mmc_write(SDIO_ERR_INTR_EN, 0);
> +
> +	/* SW reset */
> +	mvebu_mmc_write(SDIO_SW_RESET, SDIO_SW_RESET_NOW);
> +
> +	udelay(10*1000);
> +
> +	return 0;
> +}
> +
> +static const struct mmc_ops mvebu_mmc_ops = {
> +	.send_cmd	= mvebu_mmc_send_cmd,
> +	.set_ios	= mvebu_mmc_set_ios,
> +	.init		= mvebu_mmc_initialize,
> +};
> +
> +static struct mmc_config mvebu_mmc_cfg = {
> +	.name		= DRIVER_NAME,
> +	.ops		= &mvebu_mmc_ops,
> +	.f_min		= MVEBU_MMC_BASE_FAST_CLOCK / MVEBU_MMC_BASE_DIV_MAX,
> +	.f_max		= MVEBU_MMC_CLOCKRATE_MAX,
> +	.voltages	= MMC_VDD_32_33 | MMC_VDD_33_34,
> +	.host_caps	= MMC_MODE_4BIT | MMC_MODE_HS,
> +	.part_type	= PART_TYPE_DOS,
> +	.b_max		= CONFIG_SYS_MMC_MAX_BLK_COUNT,
> +};
> +
> +int mvebu_mmc_init(bd_t *bis)
> +{
> +	struct mmc *mmc;
> +
> +	mvebu_mmc_power_up();
> +
> +	mmc = mmc_create(&mvebu_mmc_cfg, bis);
> +	if (mmc == NULL)
> +		return -1;
> +
> +	return 0;
> +}
> diff --git a/include/configs/openrd.h b/include/configs/openrd.h
> index 8fab6e6..3eb408f 100644
> --- a/include/configs/openrd.h
> +++ b/include/configs/openrd.h
> @@ -49,6 +49,7 @@
>   #define CONFIG_CMD_DHCP
>   #define CONFIG_CMD_ENV
>   #define CONFIG_CMD_MII
> +#define CONFIG_CMD_MMC
>   #define CONFIG_CMD_NAND
>   #define CONFIG_CMD_PING
>   #define CONFIG_CMD_USB
> @@ -123,4 +124,11 @@
>   #define CONFIG_SYS_ATA_IDE1_OFFSET	MV_SATA_PORT1_OFFSET
>   #endif /*CONFIG_MVSATA_IDE*/
>
> +#ifdef CONFIG_CMD_MMC
> +#define CONFIG_MMC
> +#define CONFIG_GENERIC_MMC
> +#define CONFIG_MVEBU_MMC
> +#define CONFIG_SYS_MMC_BASE KW_SDIO_BASE
> +#endif /* CONFIG_CMD_MMC */
> +
>   #endif /* _CONFIG_OPENRD_BASE_H */
> diff --git a/include/configs/sheevaplug.h b/include/configs/sheevaplug.h
> index ecc93bc..de38ad3 100644
> --- a/include/configs/sheevaplug.h
> +++ b/include/configs/sheevaplug.h
> @@ -31,9 +31,11 @@
>   #define CONFIG_CMD_DHCP
>   #define CONFIG_CMD_ENV
>   #define CONFIG_CMD_MII
> +#define CONFIG_CMD_MMC
>   #define CONFIG_CMD_NAND
>   #define CONFIG_CMD_PING
>   #define CONFIG_CMD_USB
> +
>   /*
>    * mv-common.h should be defined after CMD configs since it used them
>    * to enable certain macros
> @@ -49,13 +51,14 @@
>   #else
>   #define CONFIG_ENV_IS_NOWHERE		1	/* if env in SDRAM */
>   #endif
> +
>   /*
>    * max 4k env size is enough, but in case of nand
>    * it has to be rounded to sector size
>    */
>   #define CONFIG_ENV_SIZE			0x20000	/* 128k */
> -#define CONFIG_ENV_ADDR			0x60000
> -#define CONFIG_ENV_OFFSET		0x60000	/* env starts here */
> +#define CONFIG_ENV_ADDR			0x80000
> +#define CONFIG_ENV_OFFSET		0x80000	/* env starts here */

This change seems to be unrelated. And breaks compatibility with the 
environment. Or am I missing something?

>   /*
>    * Default environment variables
> @@ -82,6 +85,16 @@
>   #endif /* CONFIG_CMD_NET */
>
>   /*
> + * SDIO/MMC Card Configuration
> + */
> +#ifdef CONFIG_CMD_MMC
> +#define CONFIG_MMC
> +#define CONFIG_GENERIC_MMC
> +#define CONFIG_MVEBU_MMC
> +#define CONFIG_SYS_MMC_BASE KW_SDIO_BASE
> +#endif /* CONFIG_CMD_MMC */
> +
> +/*
>    * File system
>    */
>   #define CONFIG_CMD_EXT2
> diff --git a/include/mvebu_mmc.h b/include/mvebu_mmc.h
> new file mode 100644
> index 0000000..bdadab5
> --- /dev/null
> +++ b/include/mvebu_mmc.h
> @@ -0,0 +1,279 @@
> +/*
> + * Marvell MMC/SD/SDIO driver
> + *
> + * (C) Copyright 2012
> + * Marvell Semiconductor <www.marvell.com>
> + * Written-by: Maen Suleiman, Gerald Kerma
> + *
> + * SPDX-License-Identifier:	GPL-2.0+
> + */
> +
> +#ifndef __MVEBU_MMC_H__
> +#define __MVEBU_MMC_H__
> +
> +/* needed for the mmc_cfg definition */
> +#include <mmc.h>
> +
> +#define MMC_BLOCK_SIZE				512
> +
> +/*
> + * Clock rates
> + */
> +
> +#define MVEBU_MMC_CLOCKRATE_MAX			50000000
> +#define MVEBU_MMC_BASE_DIV_MAX			0x7ff
> +#define MVEBU_MMC_BASE_FAST_CLOCK		CONFIG_SYS_TCLK
> +#define MVEBU_MMC_BASE_FAST_CLK_100		100000000
> +#define MVEBU_MMC_BASE_FAST_CLK_200		200000000
> +
> +/* SDIO register */
> +#define SDIO_SYS_ADDR_LOW			0x000
> +#define SDIO_SYS_ADDR_HI			0x004
> +#define SDIO_BLK_SIZE				0x008
> +#define SDIO_BLK_COUNT				0x00c
> +#define SDIO_ARG_LOW				0x010
> +#define SDIO_ARG_HI				0x014
> +#define SDIO_XFER_MODE				0x018
> +#define SDIO_CMD				0x01c
> +#define SDIO_RSP(i)				(0x020 + ((i)<<2))
> +#define SDIO_RSP0				0x020
> +#define SDIO_RSP1				0x024
> +#define SDIO_RSP2				0x028
> +#define SDIO_RSP3				0x02c
> +#define SDIO_RSP4				0x030
> +#define SDIO_RSP5				0x034
> +#define SDIO_RSP6				0x038
> +#define SDIO_RSP7				0x03c
> +#define SDIO_BUF_DATA_PORT			0x040
> +#define SDIO_RSVED				0x044
> +#define SDIO_HW_STATE				0x048
> +#define SDIO_PRESENT_STATE0			0x048
> +#define SDIO_PRESENT_STATE1			0x04c
> +#define SDIO_HOST_CTRL				0x050
> +#define SDIO_BLK_GAP_CTRL			0x054
> +#define SDIO_CLK_CTRL				0x058
> +#define SDIO_SW_RESET				0x05c
> +#define SDIO_NOR_INTR_STATUS			0x060
> +#define SDIO_ERR_INTR_STATUS			0x064
> +#define SDIO_NOR_STATUS_EN			0x068
> +#define SDIO_ERR_STATUS_EN			0x06c
> +#define SDIO_NOR_INTR_EN			0x070
> +#define SDIO_ERR_INTR_EN			0x074
> +#define SDIO_AUTOCMD12_ERR_STATUS		0x078
> +#define SDIO_CURR_BYTE_LEFT			0x07c
> +#define SDIO_CURR_BLK_LEFT			0x080
> +#define SDIO_AUTOCMD12_ARG_LOW			0x084
> +#define SDIO_AUTOCMD12_ARG_HI			0x088
> +#define SDIO_AUTOCMD12_INDEX			0x08c
> +#define SDIO_AUTO_RSP(i)			(0x090 + ((i)<<2))
> +#define SDIO_AUTO_RSP0				0x090
> +#define SDIO_AUTO_RSP1				0x094
> +#define SDIO_AUTO_RSP2				0x098
> +#define SDIO_CLK_DIV				0x128
> +
> +#define WINDOW_CTRL(i)				(0x108 + ((i) << 3))
> +#define WINDOW_BASE(i)				(0x10c + ((i) << 3))
> +
> +/* SDIO_PRESENT_STATE */
> +#define CARD_BUSY				(1 << 1)
> +#define CMD_INHIBIT				(1 << 0)
> +#define CMD_TXACTIVE				(1 << 8)
> +#define CMD_RXACTIVE				(1 << 9)
> +#define CMD_AUTOCMD12ACTIVE			(1 << 14)
> +#define CMD_BUS_BUSY				(CMD_AUTOCMD12ACTIVE |	\
> +						CMD_RXACTIVE |	\
> +						CMD_TXACTIVE |	\
> +						CMD_INHIBIT |	\
> +						CARD_BUSY)
> +
> +/*
> + * SDIO_CMD
> + */
> +
> +#define SDIO_CMD_RSP_NONE			(0 << 0)
> +#define SDIO_CMD_RSP_136			(1 << 0)
> +#define SDIO_CMD_RSP_48				(2 << 0)
> +#define SDIO_CMD_RSP_48BUSY			(3 << 0)
> +
> +#define SDIO_CMD_CHECK_DATACRC16		(1 << 2)
> +#define SDIO_CMD_CHECK_CMDCRC			(1 << 3)
> +#define SDIO_CMD_INDX_CHECK			(1 << 4)
> +#define SDIO_CMD_DATA_PRESENT			(1 << 5)
> +#define SDIO_UNEXPECTED_RESP			(1 << 7)
> +
> +#define SDIO_CMD_INDEX(x)			((x) << 8)
> +
> +/*
> + * SDIO_XFER_MODE
> + */
> +
> +#define SDIO_XFER_MODE_STOP_CLK			(1 << 5)
> +#define SDIO_XFER_MODE_HW_WR_DATA_EN		(1 << 1)
> +#define SDIO_XFER_MODE_AUTO_CMD12		(1 << 2)
> +#define SDIO_XFER_MODE_INT_CHK_EN		(1 << 3)
> +#define SDIO_XFER_MODE_TO_HOST			(1 << 4)
> +#define SDIO_XFER_MODE_DMA			(0 << 6)
> +
> +/*
> + * SDIO_HOST_CTRL
> + */
> +
> +#define SDIO_HOST_CTRL_PUSH_PULL_EN		(1 << 0)
> +
> +#define SDIO_HOST_CTRL_CARD_TYPE_MEM_ONLY	(0 << 1)
> +#define SDIO_HOST_CTRL_CARD_TYPE_IO_ONLY	(1 << 1)
> +#define SDIO_HOST_CTRL_CARD_TYPE_IO_MEM_COMBO	(2 << 1)
> +#define SDIO_HOST_CTRL_CARD_TYPE_IO_MMC		(3 << 1)
> +#define SDIO_HOST_CTRL_CARD_TYPE_MASK		(3 << 1)
> +
> +#define SDIO_HOST_CTRL_BIG_ENDIAN		(1 << 3)
> +#define SDIO_HOST_CTRL_LSB_FIRST		(1 << 4)
> +#define SDIO_HOST_CTRL_DATA_WIDTH_1_BIT		(0 << 9)
> +#define SDIO_HOST_CTRL_DATA_WIDTH_4_BITS	(1 << 9)
> +#define SDIO_HOST_CTRL_HI_SPEED_EN		(1 << 10)
> +
> +#define SDIO_HOST_CTRL_TMOUT_MAX		0xf
> +#define SDIO_HOST_CTRL_TMOUT_MASK		(0xf << 11)
> +#define SDIO_HOST_CTRL_TMOUT(x)			((x) << 11)
> +#define SDIO_HOST_CTRL_TMOUT_EN			(1 << 15)
> +
> +/*
> + * SDIO_SW_RESET
> + */
> +
> +#define SDIO_SW_RESET_NOW			(1 << 8)
> +
> +/*
> + * Normal interrupt status bits
> + */
> +
> +#define SDIO_NOR_ERROR				(1 << 15)
> +#define SDIO_NOR_UNEXP_RSP			(1 << 14)
> +#define SDIO_NOR_AUTOCMD12_DONE			(1 << 13)
> +#define SDIO_NOR_SUSPEND_ON			(1 << 12)
> +#define SDIO_NOR_LMB_FF_8W_AVAIL		(1 << 11)
> +#define SDIO_NOR_LMB_FF_8W_FILLED		(1 << 10)
> +#define SDIO_NOR_READ_WAIT_ON			(1 << 9)
> +#define SDIO_NOR_CARD_INT			(1 << 8)
> +#define SDIO_NOR_READ_READY			(1 << 5)
> +#define SDIO_NOR_WRITE_READY			(1 << 4)
> +#define SDIO_NOR_DMA_INI			(1 << 3)
> +#define SDIO_NOR_BLK_GAP_EVT			(1 << 2)
> +#define SDIO_NOR_XFER_DONE			(1 << 1)
> +#define SDIO_NOR_CMD_DONE			(1 << 0)
> +
> +/*
> + * Error status bits
> + */
> +
> +#define SDIO_ERR_CRC_STATUS			(1 << 14)
> +#define SDIO_ERR_CRC_STARTBIT			(1 << 13)
> +#define SDIO_ERR_CRC_ENDBIT			(1 << 12)
> +#define SDIO_ERR_RESP_TBIT			(1 << 11)
> +#define SDIO_ERR_XFER_SIZE			(1 << 10)
> +#define SDIO_ERR_CMD_STARTBIT			(1 << 9)
> +#define SDIO_ERR_AUTOCMD12			(1 << 8)
> +#define SDIO_ERR_DATA_ENDBIT			(1 << 6)
> +#define SDIO_ERR_DATA_CRC			(1 << 5)
> +#define SDIO_ERR_DATA_TIMEOUT			(1 << 4)
> +#define SDIO_ERR_CMD_INDEX			(1 << 3)
> +#define SDIO_ERR_CMD_ENDBIT			(1 << 2)
> +#define SDIO_ERR_CMD_CRC			(1 << 1)
> +#define SDIO_ERR_CMD_TIMEOUT			(1 << 0)
> +/* enable all for polling */
> +#define SDIO_POLL_MASK				0xffff
> +
> +/*
> + * CMD12 error status bits
> + */
> +
> +#define SDIO_AUTOCMD12_ERR_NOTEXE		(1 << 0)
> +#define SDIO_AUTOCMD12_ERR_TIMEOUT		(1 << 1)
> +#define SDIO_AUTOCMD12_ERR_CRC			(1 << 2)
> +#define SDIO_AUTOCMD12_ERR_ENDBIT		(1 << 3)
> +#define SDIO_AUTOCMD12_ERR_INDEX		(1 << 4)
> +#define SDIO_AUTOCMD12_ERR_RESP_T_BIT		(1 << 5)
> +#define SDIO_AUTOCMD12_ERR_RESP_STARTBIT	(1 << 6)
> +
> +#define MMC_RSP_PRESENT				(1 << 0)
> +/* 136 bit response */
> +#define MMC_RSP_136				(1 << 1)
> +/* expect valid crc */
> +#define MMC_RSP_CRC				(1 << 2)
> +/* card may send busy */
> +#define MMC_RSP_BUSY				(1 << 3)
> +/* response contains opcode */
> +#define MMC_RSP_OPCODE				(1 << 4)
> +
> +#define MMC_BUSMODE_OPENDRAIN			1
> +#define MMC_BUSMODE_PUSHPULL			2
> +
> +#define MMC_BUS_WIDTH_1				0
> +#define MMC_BUS_WIDTH_4				2
> +#define MMC_BUS_WIDTH_8				3
> +
> +/* Can the host do 4 bit transfers */
> +#define MMC_CAP_4_BIT_DATA			(1 << 0)
> +/* Can do MMC high-speed timing */
> +#define MMC_CAP_MMC_HIGHSPEED			(1 << 1)
> +/* Can do SD high-speed timing */
> +#define MMC_CAP_SD_HIGHSPEED			(1 << 2)
> +/* Can signal pending SDIO IRQs */
> +#define MMC_CAP_SDIO_IRQ			(1 << 3)
> +/* Talks only SPI protocols */
> +#define MMC_CAP_SPI				(1 << 4)
> +/* Needs polling for card-detection */
> +#define MMC_CAP_NEEDS_POLL			(1 << 5)
> +/* Can the host do 8 bit transfers */
> +#define MMC_CAP_8_BIT_DATA			(1 << 6)
> +
> +/* Nonremovable e.g. eMMC */
> +#define MMC_CAP_NONREMOVABLE			(1 << 8)
> +/* Waits while card is busy */
> +#define MMC_CAP_WAIT_WHILE_BUSY			(1 << 9)
> +/* Allow erase/trim commands */
> +#define MMC_CAP_ERASE				(1 << 10)
> +/* can support DDR mode at 1.8V */
> +#define MMC_CAP_1_8V_DDR			(1 << 11)
> +/* can support DDR mode at 1.2V */
> +#define MMC_CAP_1_2V_DDR			(1 << 12)
> +/* Can power off after boot */
> +#define MMC_CAP_POWER_OFF_CARD			(1 << 13)
> +/* CMD14/CMD19 bus width ok */
> +#define MMC_CAP_BUS_WIDTH_TEST			(1 << 14)
> +/* Host supports UHS SDR12 mode */
> +#define MMC_CAP_UHS_SDR12			(1 << 15)
> +/* Host supports UHS SDR25 mode */
> +#define MMC_CAP_UHS_SDR25			(1 << 16)
> +/* Host supports UHS SDR50 mode */
> +#define MMC_CAP_UHS_SDR50			(1 << 17)
> +/* Host supports UHS SDR104 mode */
> +#define MMC_CAP_UHS_SDR104			(1 << 18)
> +/* Host supports UHS DDR50 mode */
> +#define MMC_CAP_UHS_DDR50			(1 << 19)
> +/* Host supports Driver Type A */
> +#define MMC_CAP_DRIVER_TYPE_A			(1 << 23)
> +/* Host supports Driver Type C */
> +#define MMC_CAP_DRIVER_TYPE_C			(1 << 24)
> +/* Host supports Driver Type D */
> +#define MMC_CAP_DRIVER_TYPE_D			(1 << 25)
> +/* CMD23 supported. */
> +#define MMC_CAP_CMD23				(1 << 30)
> +/* Hardware reset */
> +#define MMC_CAP_HW_RESET			(1 << 31)
> +
> +

One empty line should be enough.

> +struct mvebu_mmc_cfg {
> +	u32	mvebu_mmc_base;
> +	u32	mvebu_mmc_clk;
> +	u8	max_bus_width;
> +	struct mmc_config cfg;
> +};
> +
> +/*
> + * Functions prototypes
> + */
> +
> +int mvebu_mmc_init(bd_t *bis);
> +
> +#endif /* __MVEBU_MMC_H__ */
>

Thanks for your patience!

Stefan

Hi,

On 25-07-14 20:19, Stefan Roese wrote:
> On 25.07.2014 19:07, Gerald Kerma wrote:
>> This patch add Marvell kirkwood MVSDIO/MMC driver
>> and enable it for Sheevaplugs and OpenRD boards.
>>
>> Signed-off-by: Gerald Kerma <drEagle@doukki.net>
>> ---
>>
>>   Changes in v6:
>>   - rename driver to mvebu_mmc
>>   - renames constants and functions
>>   - move mmc_init to kirkwood/cpu
>>   - code cleaning
> <snip>
>> +
>> +#define mvebu_mmc_write(offs, val) writel(val, CONFIG_SYS_MMC_BASE + 
>> (offs))
>> +#define mvebu_mmc_read(offs) readl(CONFIG_SYS_MMC_BASE + (offs))
>
> Wouldn't it be better to use functions instead of macros here?
>
or use structs?

Regards,
Jeroen

On 25/07/2014 20:19, Stefan Roese wrote:
> On 25.07.2014 19:07, Gerald Kerma wrote:
>> This patch add Marvell kirkwood MVSDIO/MMC driver
>> and enable it for Sheevaplugs and OpenRD boards.
>>
>> Signed-off-by: Gerald Kerma <drEagle@doukki.net>
>
> Thanks. This really looks better.
> 
> Only some mostly nitpicking comments below.

correction added in next version.

>> diff --git a/include/configs/sheevaplug.h b/include/configs/sheevaplug.h
>> index ecc93bc..de38ad3 100644
>> --- a/include/configs/sheevaplug.h
>> +++ b/include/configs/sheevaplug.h
>> @@ -31,9 +31,11 @@
>>   #define CONFIG_CMD_DHCP
>>   #define CONFIG_CMD_ENV
>>   #define CONFIG_CMD_MII
>> +#define CONFIG_CMD_MMC
>>   #define CONFIG_CMD_NAND
>>   #define CONFIG_CMD_PING
>>   #define CONFIG_CMD_USB
>> +
>>   /*
>>    * mv-common.h should be defined after CMD configs since it used them
>>    * to enable certain macros
>> @@ -49,13 +51,14 @@
>>   #else
>>   #define CONFIG_ENV_IS_NOWHERE        1    /* if env in SDRAM */
>>   #endif
>> +
>>   /*
>>    * max 4k env size is enough, but in case of nand
>>    * it has to be rounded to sector size
>>    */
>>   #define CONFIG_ENV_SIZE            0x20000    /* 128k */
>> -#define CONFIG_ENV_ADDR            0x60000
>> -#define CONFIG_ENV_OFFSET        0x60000    /* env starts here */
>> +#define CONFIG_ENV_ADDR            0x80000
>> +#define CONFIG_ENV_OFFSET        0x80000    /* env starts here */
> 
> This change seems to be unrelated. And breaks compatibility with the environment. Or am I missing something?

Removed, but, the new u-boot.kwb image is greater than 0x60000. If you write environments or reset environments that will corrupt uboot and broke it.

May be fixed in another patch. 

> Thanks for your patience!
> 
> Stefan

V7 sent.

Enjoy,