[U-Boot] [PATCH 1/2] PXA: Add MMC driver using the generic MMC framework
Marek Vasut
marek.vasut at gmail.com
Tue Nov 1 13:20:57 CET 2011
> Hi Marek,
Hi Igor,
>
> On 08/29/11 01:02, Marek Vasut wrote:
> > Signed-off-by: Marek Vasut <marek.vasut at gmail.com>
> > ---
> >
> > arch/arm/include/asm/arch-pxa/regs-mmc.h | 155 +++++++++++
> > drivers/mmc/Makefile | 1 +
> > drivers/mmc/pxa_mmc_gen.c | 442
> > ++++++++++++++++++++++++++++++ 3 files changed, 598 insertions(+), 0
> > deletions(-)
> > create mode 100644 arch/arm/include/asm/arch-pxa/regs-mmc.h
> > create mode 100644 drivers/mmc/pxa_mmc_gen.c
> >
> > diff --git a/arch/arm/include/asm/arch-pxa/regs-mmc.h
> > b/arch/arm/include/asm/arch-pxa/regs-mmc.h new file mode 100644
> > index 0000000..fd1eb1e
> > --- /dev/null
> > +++ b/arch/arm/include/asm/arch-pxa/regs-mmc.h
> > @@ -0,0 +1,155 @@
> > +/*
> > + * Copyright (C) 2011 Marek Vasut <marek.vasut at gmail.com>
> > + *
> > + * 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
> > + */
> > +
> > +#ifndef __REGS_MMC_H__
> > +#define __REGS_MMC_H__
> > +
> > +#define MMC0_BASE 0x41100000
> > +#define MMC1_BASE 0x42000000
> > +
> > +int pxa_mmc_register(int card_index);
> > +
> > +struct pxa_mmc_regs {
> > + uint32_t strpcl;
> > + uint32_t stat;
> > + uint32_t clkrt;
> > + uint32_t spi;
> > + uint32_t cmdat;
> > + uint32_t resto;
> > + uint32_t rdto;
> > + uint32_t blklen;
> > + uint32_t nob;
> > + uint32_t prtbuf;
> > + uint32_t i_mask;
> > + uint32_t i_reg;
> > + uint32_t cmd;
> > + uint32_t argh;
> > + uint32_t argl;
> > + uint32_t res;
> > + uint32_t rxfifo;
> > + uint32_t txfifo;
>
> Isn't space would be enough?
> I mean, they all the same type, so no alignment issues...
It's more readable like this, though that's likely an issue of personal taste.
>
> > +};
> > +
> > +/* MMC_STRPCL */
> > +#define MMC_STRPCL_STOP_CLK (1 << 0)
> > +#define MMC_STRPCL_START_CLK (1 << 1)
> > +
> > +/* MMC_STAT */
> > +#define MMC_STAT_END_CMD_RES (1 << 13)
> > +#define MMC_STAT_PRG_DONE (1 << 12)
> > +#define MMC_STAT_DATA_TRAN_DONE (1 << 11)
> > +#define MMC_STAT_CLK_EN (1 << 8)
> > +#define MMC_STAT_RECV_FIFO_FULL (1 << 7)
> > +#define MMC_STAT_XMIT_FIFO_EMPTY (1 << 6)
> > +#define MMC_STAT_RES_CRC_ERROR (1 << 5)
> > +#define MMC_STAT_SPI_READ_ERROR_TOKEN (1 << 4)
> > +#define MMC_STAT_CRC_READ_ERROR (1 << 3)
> > +#define MMC_STAT_CRC_WRITE_ERROR (1 << 2)
> > +#define MMC_STAT_TIME_OUT_RESPONSE (1 << 1)
> > +#define MMC_STAT_READ_TIME_OUT (1 << 0)
> > +
> > +/* MMC_CLKRT */
> > +#define MMC_CLKRT_20MHZ 0
> > +#define MMC_CLKRT_10MHZ 1
> > +#define MMC_CLKRT_5MHZ 2
> > +#define MMC_CLKRT_2_5MHZ 3
> > +#define MMC_CLKRT_1_25MHZ 4
> > +#define MMC_CLKRT_0_625MHZ 5
> > +#define MMC_CLKRT_0_3125MHZ 6
> > +
> > +/* MMC_SPI */
> > +#define MMC_SPI_EN (1 << 0)
> > +#define MMC_SPI_CS_EN (1 << 2)
> > +#define MMC_SPI_CS_ADDRESS (1 << 3)
> > +#define MMC_SPI_CRC_ON (1 << 1)
> > +
> > +/* MMC_CMDAT */
> > +#define MMC_CMDAT_SD_4DAT (1 << 8)
> > +#define MMC_CMDAT_MMC_DMA_EN (1 << 7)
> > +#define MMC_CMDAT_INIT (1 << 6)
> > +#define MMC_CMDAT_BUSY (1 << 5)
> > +#define MMC_CMDAT_BCR (MMC_CMDAT_BUSY |
MMC_CMDAT_INIT)
> > +#define MMC_CMDAT_STREAM (1 << 4)
> > +#define MMC_CMDAT_WRITE (1 << 3)
> > +#define MMC_CMDAT_DATA_EN (1 << 2)
> > +#define MMC_CMDAT_R0 0
> > +#define MMC_CMDAT_R1 1
> > +#define MMC_CMDAT_R2 2
> > +#define MMC_CMDAT_R3 3
> > +
> > +/* MMC_RESTO */
> > +#define MMC_RES_TO_MAX_MASK 0x7f
> > +
> > +/* MMC_RDTO */
> > +#define MMC_READ_TO_MAX_MASK 0xffff
> > +
> > +/* MMC_BLKLEN */
> > +#define MMC_BLK_LEN_MAX_MASK 0x3ff
> > +
> > +/* MMC_PRTBUF */
> > +#define MMC_PRTBUF_BUF_PART_FULL (1 << 0)
> > +
> > +/* MMC_I_MASK */
> > +#define MMC_I_MASK_TXFIFO_WR_REQ (1 << 6)
> > +#define MMC_I_MASK_RXFIFO_RD_REQ (1 << 5)
> > +#define MMC_I_MASK_CLK_IS_OFF (1 << 4)
> > +#define MMC_I_MASK_STOP_CMD (1 << 3)
> > +#define MMC_I_MASK_END_CMD_RES (1 << 2)
> > +#define MMC_I_MASK_PRG_DONE (1 << 1)
> > +#define MMC_I_MASK_DATA_TRAN_DONE (1 << 0)
> > +#define MMC_I_MASK_ALL 0x7f
> > +
> > +
> > +/* MMC_I_REG */
> > +#define MMC_I_REG_TXFIFO_WR_REQ (1 << 6)
> > +#define MMC_I_REG_RXFIFO_RD_REQ (1 << 5)
> > +#define MMC_I_REG_CLK_IS_OFF (1 << 4)
> > +#define MMC_I_REG_STOP_CMD (1 << 3)
> > +#define MMC_I_REG_END_CMD_RES (1 << 2)
> > +#define MMC_I_REG_PRG_DONE (1 << 1)
> > +#define MMC_I_REG_DATA_TRAN_DONE (1 << 0)
> > +
> > +/* MMC_CMD */
> > +#define MMC_CMD_INDEX_MAX 0x6f
> > +#define CMD(x) (x)
>
> You've missed this one
Thanks
>
> > +
> > +#define MMC_R1_IDLE_STATE 0x01
> > +#define MMC_R1_ERASE_STATE 0x02
> > +#define MMC_R1_ILLEGAL_CMD 0x04
> > +#define MMC_R1_COM_CRC_ERR 0x08
> > +#define MMC_R1_ERASE_SEQ_ERR 0x01
> > +#define MMC_R1_ADDR_ERR 0x02
> > +#define MMC_R1_PARAM_ERR 0x04
> > +
> > +#define MMC_R1B_WP_ERASE_SKIP 0x0002
> > +#define MMC_R1B_ERR 0x0004
> > +#define MMC_R1B_CC_ERR 0x0008
> > +#define MMC_R1B_CARD_ECC_ERR 0x0010
> > +#define MMC_R1B_WP_VIOLATION 0x0020
> > +#define MMC_R1B_ERASE_PARAM 0x0040
> > +#define MMC_R1B_OOR 0x0080
> > +#define MMC_R1B_IDLE_STATE 0x0100
> > +#define MMC_R1B_ERASE_RESET 0x0200
> > +#define MMC_R1B_ILLEGAL_CMD 0x0400
> > +#define MMC_R1B_COM_CRC_ERR 0x0800
> > +#define MMC_R1B_ERASE_SEQ_ERR 0x1000
> > +#define MMC_R1B_ADDR_ERR 0x2000
> > +#define MMC_R1B_PARAM_ERR 0x4000
>
> All the above defines are really unnecessarily "tabbed"...
> The values are fine, but why the names?
I'm used to put tabs there ... is there actually any rule against that ?
Checkpatch doesn't complain.
>
> > +
> > +#endif /* __REGS_MMC_H__ */
> > diff --git a/drivers/mmc/Makefile b/drivers/mmc/Makefile
> > index 3968c14..59bda49 100644
> > --- a/drivers/mmc/Makefile
> > +++ b/drivers/mmc/Makefile
> > @@ -38,6 +38,7 @@ COBJS-$(CONFIG_MXC_MMC) += mxcmmc.o
> >
> > COBJS-$(CONFIG_OMAP3_MMC) += omap3_mmc.o
> > COBJS-$(CONFIG_OMAP_HSMMC) += omap_hsmmc.o
> > COBJS-$(CONFIG_PXA_MMC) += pxa_mmc.o
> >
> > +COBJS-$(CONFIG_PXA_MMC_GENERIC) += pxa_mmc_gen.o
> >
> > COBJS-$(CONFIG_S5P_MMC) += s5p_mmc.o
> > COBJS-$(CONFIG_SDHCI) += sdhci.o
> > COBJS-$(CONFIG_SH_MMCIF) += sh_mmcif.o
> >
[...]
Cheers
More information about the U-Boot
mailing list