[U-Boot] [PATCH v3 2/3] mmc : Add SDIO driver for Marvell SoCs (Kirkwood)
Prafulla Wadaskar
prafulla at marvell.com
Sun Nov 21 16:31:54 CET 2010
> -----Original Message-----
> From: Gérald Kerma [mailto:geraker at gmail.com]
> Sent: Sunday, November 21, 2010 4:38 PM
> To: u-boot at lists.denx.de
> Cc: Prafulla Wadaskar; Ashish Karkare; Prabhanjan Sarnaik
> Subject: [PATCH v3 2/3] mmc : Add SDIO driver for Marvell SoCs (Kirkwood)
>
>
> Add SDIO legacy driver for Marvell SoCs (Kirkwood)
>
> Signed-off-by: Gérald Kerma <geraker at gmail.com>
> ---
> v3:
> * GNU Software License compliance
> * Patch more readable
>
> v2:
> * Code cleaning
>
> v1:
> * Fix errors from SD/SDHC detect
>
> ---
> drivers/mmc/Makefile | 1 +
> drivers/mmc/mv_sdio.c | 675
> +++++++++++++++++++++++++++++++++++++++++++++++++
> drivers/mmc/mv_sdio.h | 313 +++++++++++++++++++++++
Can you make use of drivers/mmc/mmc.c to reduce code in this file?
> 3 files changed, 989 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> index 68afd30..2a4e1d4 100644
> --- a/drivers/mmc/Makefile
> +++ b/drivers/mmc/Makefile
> @@ -30,6 +30,7 @@ COBJS-$(CONFIG_BFIN_SDH) += bfin_sdh.o
> COBJS-$(CONFIG_FSL_ESDHC) += fsl_esdhc.o
> COBJS-$(CONFIG_GENERIC_MMC) += mmc.o
> COBJS-$(CONFIG_GENERIC_ATMEL_MCI) += gen_atmel_mci.o
> +COBJS-$(CONFIG_MV_SDIO) += mv_sdio.o
> COBJS-$(CONFIG_MXC_MMC) += mxcmmc.o
> COBJS-$(CONFIG_OMAP3_MMC) += omap3_mmc.o
> COBJS-$(CONFIG_OMAP_HSMMC) += omap_hsmmc.o
> diff --git a/drivers/mmc/mv_sdio.c b/drivers/mmc/mv_sdio.c
> new file mode 100644
> index 0000000..35969d3
> --- /dev/null
> +++ b/drivers/mmc/mv_sdio.c
> @@ -0,0 +1,675 @@
> +/*
> + * (C) Copyright 2009
It should be 2010
> + * Marvell Semiconductor <www.marvell.com>
> + * Written-by: Gérald Kerma <geraker at gmail.com>
> + *
> + * (C) Copyright 2003
> + * Kyle Harris, Nexus Technologies, Inc. kharris at nexus-tech.net
Are these two lines required here?
> + *
> + * 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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#include <config.h>
> +#include <common.h>
> +#include <malloc.h>
> +#include <mmc.h>
> +#include <asm/errno.h>
> +#include <part.h>
> +#include <asm/io.h>
> +#ifdef CONFIG_KIRKWOOD
> +#include <asm/arch/kirkwood.h>
> +#endif
> +#include "mv_sdio.h"
> +
> +#ifdef CONFIG_MMC
> +
> +#define DRIVER_NAME "mv-sdio"
> +
> +#ifdef DEBUG
> +#define pr_debug(fmt, args...) printf(fmt, ##args)
> +#else
> +#define pr_debug(...) do { } while(0)
> +#endif
Please remove all above and simply use debug macros instead of pr_debug in your code, that will make it simple and in sync with other drivers
> +
> +//static mv_sdio_t *mvsd = (mv_sdio_t *)mmc->priv;
> +static mv_sdio_t *mvsd = (mv_sdio_t *)MV_SDIO_BASE;
> +
> +static int is_sdhc;
> +extern int fat_register_device(block_dev_desc_t *dev_desc, int part_no);
> +static block_dev_desc_t mmc_dev;
> +block_dev_desc_t * mmc_get_dev(int dev)
> +{
> + return ((block_dev_desc_t *)&mmc_dev);
> +}
> +
> +/*
> + * FIXME needs to read cid and csd info to determine block size
> + * and other parameters
> + */
> +static uchar mmc_buf[MMC_BLOCK_SIZE];
> +static mv_mmc_csd_t mv_mmc_csd;
> +static int mmc_ready = 0;
> +
> +/* MMC_DEFAULT_RCA should probably be just 1, but this may break other
> code
> + that expects it to be shifted. */
> +static u_int16_t rca = 0;
> +
> +/* used for debug */
> +static u_int32_t mv_mmc_size(const struct mv_mmc_csd *csd)
> +{
> + u_int32_t block_len, mult, blocknr;
> +
> + block_len = csd->read_bl_len << 12;
> + mult = csd->c_size_mult1 << 8;
> + blocknr = (csd->c_size+1) * mult;
> +
> + return blocknr * block_len;
> +}
> +
> +static int isprint (unsigned char ch)
> +{
> + if (ch >= 32 && ch < 127)
> + return (1);
> +
> + return (0);
> +}
> +
> +static int toprint(char *dst, char c)
> +{
> + if (isprint(c)) {
> + *dst = c;
> + return 1;
> + }
> +
> + return sprintf(dst,"\\x%02x", c);
> +
> +}
> +
> +static void print_mmc_cid(mv_mmc_cid_t *cid)
> +{
> + printf("MMC found. Card desciption is:\n");
> + printf("Manufacturer ID = %02x%02x%02x\n",
> + cid->id[0], cid->id[1], cid->id[2]);
> + printf("HW/FW Revision = %x %x\n",cid->hwrev, cid->fwrev);
> + cid->hwrev = cid->fwrev = 0; /* null terminate string */
> + printf("Product Name = %s\n",cid->name);
> + printf("Serial Number = %02x%02x%02x\n",
> + cid->sn[0], cid->sn[1], cid->sn[2]);
> + printf("Month = %d\n",cid->month);
> + printf("Year = %d\n",1997 + cid->year);
> +}
> +
> +static void print_sd_cid(mv_sd_cid_t *cid)
> +{
> + int len;
> + char tbuf[64];
> +
> + printf("SD%s found. Card desciption is:\n", is_sdhc?"HC":"");
> +
> + len = 0;
> + len += toprint(&tbuf[len], cid->oid_0);
> + len += toprint(&tbuf[len], cid->oid_1);
> + tbuf[len] = 0;
> +
> + printf("Manufacturer: 0x%02x, OEM \"%s\"\n",
> + cid->mid, tbuf);
> +
> + len = 0;
> + len += toprint(&tbuf[len], cid->pnm_0);
> + len += toprint(&tbuf[len], cid->pnm_1);
> + len += toprint(&tbuf[len], cid->pnm_2);
> + len += toprint(&tbuf[len], cid->pnm_3);
> + len += toprint(&tbuf[len], cid->pnm_4);
> + tbuf[len] = 0;
> +
> + printf("Product name: \"%s\", revision %d.%d\n",
> + tbuf,
> + cid->prv >> 4, cid->prv & 15);
> +
> + printf("Serial number: %u\n",
> + cid->psn_0 << 24 | cid->psn_1 << 16 | cid->psn_2 << 8 |
> + cid->psn_3);
> + printf("Manufacturing date: %d/%d\n",
> + cid->mdt_1 & 15,
> + 2000+((cid->mdt_0 & 15) << 4)+((cid->mdt_1 & 0xf0) >> 4));
> +
> + printf("CRC: 0x%02x, b0 = %d\n",
> + cid->crc >> 1, cid->crc & 1);
> +}
> +
> +static void mvsdio_set_clock(unsigned int clock)
> +{
> + unsigned int m;
> +
> + m = MVSDMMC_BASE_FAST_CLOCK/(2*clock) - 1;
> +
> + pr_debug("mvsdio_set_clock: dividor = 0x%x clock=%d\n",
> + m, clock);
> +
> +
> + writew(m & 0x7ff, &mvsd->CLK_DIV);
> +
> + if (isprint(1))
> + udelay(10*1000);
> +}
> +
> +/****************************************************/
> +static ulong * mv_mmc_cmd(ulong cmd, ulong arg, ushort xfermode, ushort
> resptype, ushort waittype)
> +/****************************************************/
> +{
> + static ulong resp[4];
> + ushort done ;
> + int err = 0 ;
> + ulong curr, start, diff, hz;
> + ushort response[8];
> +
> + pr_debug("mv_mmc_cmd %x, arg: %x,xfer: %x,resp: %x, wait : %x\n"
> + , (unsigned int)cmd, (unsigned int)arg, xfermode, resptype,
> waittype);
> +
> +
> + /* clear status */
> + writew(0xffff, &mvsd->NOR_INTR_STATUS);
It looks ugly register names in upper case, please define them in lowercase
...snip...
> +#endif /* CONFIG_MMC */
> diff --git a/drivers/mmc/mv_sdio.h b/drivers/mmc/mv_sdio.h
> new file mode 100644
> index 0000000..3383c3d
> --- /dev/null
> +++ b/drivers/mmc/mv_sdio.h
> @@ -0,0 +1,313 @@
> +/*
> + * (C) Copyright 2009
Ditto
> + * Marvell Semiconductor <www.marvell.com>
> + * Written-by: Gérald Kerma <geraker at gmail.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., 51 Franklin Street, Fifth Floor, Boston,
> + * MA 02110-1301 USA
> + */
> +
> +#ifndef _MVSDIO_INCLUDE
> +#define _MVSDIO_INCLUDE
> +
> +//#define SDIO_REG(x) (MV_SDIO_BASE + (x))
Please remove dead code
> +
> +#define MVSDMMC_DMA_SIZE 65536
> +#define MVSDMMC_CMD_TIMEOUT 2 /* 100 usec*/
> +
> +/*
> + * Clock rates
> + */
> +
> +#define MVSD_CLOCKRATE_MAX 50000000
Please maintain common syntax between all definations.
I suggest to to prefix MVSD for all macros and avoid MVSDMMC and MMC
> +#define MVSD_BASE_DIV_MAX 0x7ff
> +
> +#define CONFIG_SYS_MMC_CLK_PP 25000000
> +
> +/*
> + * The base MMC clock rate
> + */
> +
> +#define MVSDMMC_CLOCKRATE_MIN 100000
> +#define MVSDMMC_CLOCKRATE_MAX MVSD_CLOCKRATE_MAX
> +#define MVSDMMC_BASE_FAST_CLOCK CONFIG_SYS_TCLK
> +
> +
> +/*
> + * SDIO register
> + */
> +#ifndef __ASSEMBLY__
Do you need this, is this file get called from any assembly code?
> +
> +/*
> + * Structure for struct SoC access.
> + * Names starting with '_' are fillers.
> + */
> +typedef struct mv_sdio {
Better to use simple c-struct, avoid typedefs so far it is possible
> + /* reg Offset */
> + u32 SYS_ADDR_LOW; /* 0x00 */
Please use lowercase
> + u32 SYS_ADDR_HI; /* 0x04 */
> + u32 BLK_SIZE; /* 0x08 */
> + u32 BLK_COUNT; /* 0x0c */
> + u32 ARG_LOW; /* 0x10 */
> + u32 ARG_HI; /* 0x14 */
> + u32 XFER_MODE; /* 0x18 */
> + u32 CMD; /* 0x1c */
> + u32 RSP0; /* 0x20 */
> + u32 RSP1; /* 0x24 */
> + u32 RSP2; /* 0x28 */
> + u32 RSP3; /* 0x2c */
> + u32 RSP4; /* 0x30 */
> + u32 RSP5; /* 0x34 */
> + u32 RSP6; /* 0x38 */
> + u32 RSP7; /* 0x3c */
> + u32 BUF_DATA_PORT; /* 0x40 */
> + u32 RSVED; /* 0x44 */
> + u32 PRESENT_STATE0; /* 0x48 */
> + u32 PRESENT_STATE1; /* 0x4c */
> + u32 HOST_CTRL; /* 0x50 */
> + u32 BLK_GAP_CTRL; /* 0x54 */
> + u32 CLK_CTRL; /* 0x58 */
> + u32 SW_RESET; /* 0x5c */
> + u32 NOR_INTR_STATUS; /* 0x60 */
> + u32 ERR_INTR_STATUS; /* 0x64 */
> + u32 NOR_STATUS_EN; /* 0x68 */
> + u32 ERR_STATUS_EN; /* 0x6c */
> + u32 NOR_INTR_EN; /* 0x70 */
> + u32 ERR_INTR_EN; /* 0x74 */
> + u32 AUTOCMD12_ERR_STATUS; /* 0x78 */
> + u32 CURR_BYTE_LEFT; /* 0x7c */
> + u32 CURR_BLK_LEFT; /* 0x80 */
> + u32 AUTOCMD12_ARG_LOW; /* 0x84 */
> + u32 AUTOCMD12_ARG_HI; /* 0x88 */
> + u32 AUTOCMD12_INDEX; /* 0x8c */
> + u32 AUTO_RSP0; /* 0x90 */
> + u32 AUTO_RSP1; /* 0x94 */
> + u32 AUTO_RSP2; /* 0x98 */
> + u32 _9c; /* 0x9c */
> + u32 _a0[0x78]; /* 0xa0 */
You should use pad0, pad1[xx] here instead of _9c
> + u32 CLK_DIV; /* 0x128 */
> +
> +} mv_sdio_t;
> +
> +#endif /* __ASSEMBLY__ */
> +
> +/*
> + * 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)
MVSD_ as suggested above is better to use for SDIO_
Once again check if you can use code from drives/mmc/mmc.c enabling CONFIG_GENERIC_MMC
Regards..
Prafulla ..
More information about the U-Boot
mailing list