[U-Boot] [PATCH v2 1/5] Add Marvell SDIO/MMC driver for Kirkwood SoC

Prafulla Wadaskar prafulla at marvell.com
Mon Sep 24 10:42:29 CEST 2012



> -----Original Message-----
> From: u-boot-bounces at lists.denx.de [mailto:u-boot-
> bounces at lists.denx.de] On Behalf Of DrEagle
> Sent: 22 September 2012 12:46
> To: DrEagle
> Cc: Lior Amsalem; u-boot at lists.denx.de; uboot at doukki.net
> Subject: [U-Boot] [PATCH v2 1/5] Add Marvell SDIO/MMC driver for
> Kirkwood SoC
>
>
> Add Marvell SDIO/MMC driver for Kirkwood SoC
>
> Signed-off-by: Gérald Kerma <uboot at doukki.net>
> Signed-off-by: Lior Amsalem <alior at marvell.com>
> ---
>  drivers/mmc/Makefile   |    1 +
>  drivers/mmc/mrvl_mmc.c |  277

The file name should be kirkwood_mmc.*

> ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/mrvl_mmc.h     |  191 +++++++++++++++++++++++++++++++++

Ditto

>  3 files changed, 469 insertions(+)
>  create mode 100644 drivers/mmc/mrvl_mmc.c
>  create mode 100644 include/mrvl_mmc.h
>
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 2b96cdc..7f3b7c1 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -37,6 +37,7 @@ COBJS-$(CONFIG_GENERIC_MMC) += mmc.o
>  COBJS-$(CONFIG_GENERIC_ATMEL_MCI) += gen_atmel_mci.o
>  COBJS-$(CONFIG_MMC_SPI) += mmc_spi.o
>  COBJS-$(CONFIG_ARM_PL180_MMCI) += arm_pl180_mmci.o
> +COBJS-$(CONFIG_MRVL_MMC) += mrvl_mmc.o

This should be CONFIG_KIRKWOOD_MMC

>  COBJS-$(CONFIG_MV_SDHCI) += mv_sdhci.o
>  COBJS-$(CONFIG_MXC_MMC) += mxcmmc.o
>  COBJS-$(CONFIG_MXS_MMC) += mxsmmc.o
> diff --git a/drivers/mmc/mrvl_mmc.c b/drivers/mmc/mrvl_mmc.c
> new file mode 100644
> index 0000000..8127961
> --- /dev/null
> +++ b/drivers/mmc/mrvl_mmc.c
> @@ -0,0 +1,277 @@
> +/*
> + * Driver for Marvell SDIO/MMC controller
> + *
> + * (C) Copyright 2012
> + * Marvell Semiconductor <www.marvell.com>
> + * Written-by: Lior Amsalem <alior at marvell.com>
> + * See file CREDITS for list of people who contributed to this
> + * project.
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of
> + * the License, or (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
> + * MA 02111-1307 USA
> + */
> +
> +#include <common.h>
> +#include <malloc.h>
> +#include <part.h>
> +#include <mmc.h>
> +
> +#include <mrvl_mmc.h>
> +#define MRVL_MMC_MAKE_CMD(c, f) (((c & 0xff) << 8) | (f & 0xff))

Since this is Kirkwood MMC driver you should use KW_ suffix for all macros and kw_ suffix for all function/variable names.

MRVL is generic to marvell and valid if this driver is applicable to all Marvell SOCs.

> +
> +
> +static int mrvl_mmc_setup_data(struct mmc_data *data)
> +{
> +     if (data->flags & MMC_DATA_READ) {
> +             SDIO_REG_WRITE16(SDIO_SYS_ADDR_LOW,((ulong)(data->dest)) &

No need to define these macros use standard readl/writel calls for register r/w operation.

> 0xffff);
> +             SDIO_REG_WRITE16(SDIO_SYS_ADDR_HI,(((ulong)data->dest) >> 16)
> & 0xffff);
> +     } else {
> +             SDIO_REG_WRITE16(SDIO_SYS_ADDR_LOW,((ulong)(data->src)) &
> 0xffff);
> +             SDIO_REG_WRITE16(SDIO_SYS_ADDR_HI,(((ulong)data->src) >> 16) &
> 0xffff);
> +     }
> +
> +     SDIO_REG_WRITE16(SDIO_BLK_SIZE, data->blocksize);
> +     SDIO_REG_WRITE16(SDIO_BLK_COUNT, data->blocks);
> +
> +     return 0;
> +}
> +
> +static int mrvl_mmc_send_cmd (struct mmc *mmc, struct mmc_cmd *cmd,
> struct mmc_data *data)
> +{
> +     int     timeout = 10;
> +     ushort waittype = 0;
> +     int err = 0 ;
> +     ushort response[8], resp_indx = 0;
> +     ushort resptype = 0;
> +     ushort xfertype = 0;
> +
> +#ifdef DEBUG
> +     printf("cmd [0x%x] resp_type[0x%x] arg[0x%x]\n", cmd->cmdidx, cmd-
> >resp_type, cmd->cmdarg);
> +#endif
> +
> +     /* clear status */
> +     SDIO_REG_WRITE16(SDIO_NOR_INTR_STATUS, 0xffff);
> +     SDIO_REG_WRITE16(SDIO_ERR_INTR_STATUS, 0xffff);
> +
> +     /* Checking if card is busy */
> +     while ((SDIO_REG_READ16(SDIO_PRESENT_STATE0) & CARD_BUSY)) {
> +             if (timeout == 0) {
> +                     printf("MRVL MMC: card busy!\n");
> +                     return -1;
> +             }
> +             timeout--;
> +             udelay(1000);

Can you avoid this delay, may you increase timeout value and avoid this udelay?

> +     }
> +
> +     /* Set up for a data transfer if we have one */
> +     if (data) {
> +             int err;

Defined should be done at the top, but you can avoid this by directly using function call in if statement

> +             err = mrvl_mmc_setup_data(data);
> +             if(err)
> +                     return err;

You can reduce the code here to two lines.

> +     }
> +
> +     /* 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 */
> +     SDIO_REG_WRITE16(SDIO_ARG_LOW, (ushort)(cmd->cmdarg & 0xffff));
> +     SDIO_REG_WRITE16(SDIO_ARG_HI,  (ushort)(cmd->cmdarg >> 16) );

Avoid hard coding, use macros, some comments too

> +
> +     /* Setting Xfer mode */
> +     SDIO_REG_WRITE16(SDIO_XFER_MODE, xfertype);
> +
> +     /* Sending command */
> +     SDIO_REG_WRITE16(SDIO_CMD, MRVL_MMC_MAKE_CMD(cmd->cmdidx,
> resptype));
> +
> +     /* Waiting for completion */
> +     timeout = 1000000;
> +
> +     while (!((SDIO_REG_READ16(SDIO_NOR_INTR_STATUS)) & waittype)) {
> +             udelay(1);
> +             if (SDIO_REG_READ16(SDIO_NOR_INTR_STATUS) & SDIO_NOR_ERROR) {
> +#ifdef DEBUG
> +                     printf("mrvl_mmc_send_cmd: error! cmd : %d, err reg:
> %04x\n", cmd->cmdidx, SDIO_REG_READ16(SDIO_ERR_INTR_STATUS));
> +#endif
> +                     if (SDIO_REG_READ16(SDIO_ERR_INTR_STATUS) &
> (SDIO_ERR_CMD_TIMEOUT | SDIO_ERR_DATA_TIMEOUT))
> +                             return TIMEOUT;
> +                     return COMM_ERR;
> +             }
> +             timeout--;
> +             if (timeout <= 0) {
> +                     printf("MRVL MMC: command timed out\n");
> +                     return TIMEOUT;
> +             }
> +     }
> +
> +     /* Handling response */
> +     for (resp_indx = 0 ; resp_indx < 8; resp_indx++)



More information about the U-Boot mailing list