[U-Boot] [PATCH 1/2] PXA: Add MMC driver using the generic MMC framework
Igor Grinberg
grinberg at compulab.co.il
Wed Nov 2 09:41:10 CET 2011
On 11/01/11 14:20, Marek Vasut wrote:
>> 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.
Agreed, it is 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.
There is no rule, but it seems redundant,
especially with the 80 characters limit (though it is not a hard limit).
>
>>
>>> +
>>> +#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
>
--
Regards,
Igor.
More information about the U-Boot
mailing list