[U-Boot] [PATCH v2 1/4] Tegra2: mmc: define register field values in tegra2_mmc.h
Tom Warren
TWarren at nvidia.com
Thu Nov 10 17:28:48 CET 2011
Anton,
-----Original Message-----
From: Anton Staaf [mailto:robotboy at chromium.org]
Sent: Wednesday, November 09, 2011 12:46 PM
To: u-boot at lists.denx.de
Cc: Anton Staaf; Andy Fleming; Tom Warren; Stephen Warren; Albert Aribaud
Subject: [PATCH v2 1/4] Tegra2: mmc: define register field values in tegra2_mmc.h
This moves the magic numbers sprinkled about the MMC driver to a single location in the header file and gives them meaningful names.
Signed-off-by: Anton Staaf <robotboy at chromium.org>
Cc: Andy Fleming <afleming at gmail.com>
Cc: Tom Warren <twarren at nvidia.com>
Cc: Stephen Warren <swarren at nvidia.com>
Cc: Albert Aribaud <albert.u.boot at aribaud.net>
---
drivers/mmc/tegra2_mmc.c | 126 ++++++++++++++++------------------------------
drivers/mmc/tegra2_mmc.h | 80 +++++++++++++++++++++++++++++
2 files changed, 123 insertions(+), 83 deletions(-)
diff --git a/drivers/mmc/tegra2_mmc.c b/drivers/mmc/tegra2_mmc.c index 78b1190..2e7746d 100644
--- a/drivers/mmc/tegra2_mmc.c
+++ b/drivers/mmc/tegra2_mmc.c
@@ -73,15 +73,10 @@ static void mmc_prepare_data(struct mmc_host *host, struct mmc_data *data)
(u32)data->dest, data->blocks, data->blocksize);
writel((u32)data->dest, &host->reg->sysad);
- /*
- * DMASEL[4:3]
- * 00 = Selects SDMA
- * 01 = Reserved
- * 10 = Selects 32-bit Address ADMA2
- * 11 = Selects 64-bit Address ADMA2
- */
Why remove the comments about the bit fields? Helps make it obvious what's being set.
+
+ /* Select SDMA */
ctrl = readb(&host->reg->hostctl);
- ctrl &= ~(3 << 3); /* SDMA */
+ ctrl &= ~TEGRA2_MMC_HOSTCTL_DMASEL_MASK;
It's no longer clear that SDMA mode is being chosen here (especially w/o the DMASEL field comment, above). Maybe add a comment?
writeb(ctrl, &host->reg->hostctl);
/* We do not handle DMA boundaries, so set it to max (512 KiB) */ @@ -93,21 +88,15 @@ static void mmc_set_transfer_mode(struct mmc_host *host, struct mmc_data *data) {
unsigned short mode;
debug(" mmc_set_transfer_mode called\n");
- /*
- * TRNMOD
- * MUL1SIN0[5] : Multi/Single Block Select
- * RD1WT0[4] : Data Transfer Direction Select
- * 1 = read
- * 0 = write
- * ENACMD12[2] : Auto CMD12 Enable
- * ENBLKCNT[1] : Block Count Enable
- * ENDMA[0] : DMA Enable
- */
- mode = (1 << 1) | (1 << 0);
+
+ mode = (TEGRA2_MMC_TRNMOD_DMA_EN_ENABLE |
+ TEGRA2_MMC_TRNMOD_BLOCK_COUNT_EN_ENABLE);
+
I know I work for NVIDIA and probably shouldn't say this, but I hate the _EN_ENABLE and _EN_DISABLE
bit names in our HW docs. Can you use more natural names? Like "TRNMOD_DMA_ENABLE, etc.?
if (data->blocks > 1)
- mode |= (1 << 5);
+ mode |= TEGRA2_MMC_TRNMOD_MULTI_BLOCK_SELECT_ENABLE;
+
if (data->flags & MMC_DATA_READ)
- mode |= (1 << 4);
+ mode |= TEGRA2_MMC_TRNMOD_DATA_XFER_DIR_SEL_READ;
writew(mode, &host->reg->trnmod);
}
@@ -125,21 +114,16 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
/* Wait max 10 ms */
timeout = 10;
- /*
- * PRNSTS
- * CMDINHDAT[1] : Command Inhibit (DAT)
- * CMDINHCMD[0] : Command Inhibit (CMD)
- */
- mask = (1 << 0);
+ mask = TEGRA2_MMC_PRNSTS_CMD_INHIBIT_CMD_ACTIVE;
if ((data != NULL) || (cmd->resp_type & MMC_RSP_BUSY))
- mask |= (1 << 1);
+ mask |= TEGRA2_MMC_PRNSTS_CMD_INHIBIT_DAT_ACTIVE;
/*
* We shouldn't wait for data inhibit for stop commands, even
* though they might use busy signaling
*/
if (data)
- mask &= ~(1 << 1);
+ mask &= ~TEGRA2_MMC_PRNSTS_CMD_INHIBIT_DAT_ACTIVE;
while (readl(&host->reg->prnsts) & mask) {
if (timeout == 0) {
@@ -162,33 +146,23 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
if ((cmd->resp_type & MMC_RSP_136) && (cmd->resp_type & MMC_RSP_BUSY))
return -1;
- /*
- * CMDREG
- * CMDIDX[13:8] : Command index
- * DATAPRNT[5] : Data Present Select
- * ENCMDIDX[4] : Command Index Check Enable
- * ENCMDCRC[3] : Command CRC Check Enable
- * RSPTYP[1:0]
- * 00 = No Response
- * 01 = Length 136
- * 10 = Length 48
- * 11 = Length 48 Check busy after response
- */
if (!(cmd->resp_type & MMC_RSP_PRESENT))
- flags = 0;
+ flags = TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_NO_RESPONSE;
else if (cmd->resp_type & MMC_RSP_136)
- flags = (1 << 0);
+ flags = TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_LENGTH_136;
else if (cmd->resp_type & MMC_RSP_BUSY)
- flags = (3 << 0);
+ flags = TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_LENGTH_48_BUSY;
else
- flags = (2 << 0);
+ flags = TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_LENGTH_48;
if (cmd->resp_type & MMC_RSP_CRC)
- flags |= (1 << 3);
+ flags |= TEGRA2_MMC_TRNMOD_CMD_CRC_CHECK_EN_ENABLE;
+
if (cmd->resp_type & MMC_RSP_OPCODE)
- flags |= (1 << 4);
+ flags |= TEGRA2_MMC_TRNMOD_CMD_INDEX_CHECK_EN_ENABLE;
+
if (data)
- flags |= (1 << 5);
+ flags |= TEGRA2_MMC_TRNMOD_DATA_PRESENT_SELECT_DATA_TRANSFER;
debug("cmd: %d\n", cmd->cmdidx);
@@ -197,7 +171,7 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
for (i = 0; i < retry; i++) {
mask = readl(&host->reg->norintsts);
/* Command Complete */
- if (mask & (1 << 0)) {
+ if (mask & TEGRA2_MMC_NORINTSTS_CMD_COMPLETE) {
if (!data)
writel(mask, &host->reg->norintsts);
break;
@@ -209,11 +183,11 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
return TIMEOUT;
}
- if (mask & (1 << 16)) {
+ if (mask & TEGRA2_MMC_NORINTSTS_CMD_TIMEOUT_ERR) {
/* Timeout Error */
debug("timeout: %08x cmd %d\n", mask, cmd->cmdidx);
return TIMEOUT;
- } else if (mask & (1 << 15)) {
+ } else if (mask & TEGRA2_MMC_NORINTSTS_ERR_INTERRUPT) {
/* Error Interrupt */
debug("error: %08x cmd %d\n", mask, cmd->cmdidx);
return -1;
@@ -259,17 +233,17 @@ static int mmc_send_cmd(struct mmc *mmc, struct mmc_cmd *cmd,
while (1) {
mask = readl(&host->reg->norintsts);
- if (mask & (1 << 15)) {
+ if (mask & TEGRA2_MMC_NORINTSTS_ERR_INTERRUPT) {
/* Error Interrupt */
writel(mask, &host->reg->norintsts);
printf("%s: error during transfer: 0x%08x\n",
__func__, mask);
return -1;
- } else if (mask & (1 << 3)) {
+ } else if (mask & TEGRA2_MMC_NORINTSTS_DMA_INTERRUPT) {
/* DMA Interrupt */
debug("DMA end\n");
break;
- } else if (mask & (1 << 1)) {
+ } else if (mask & TEGRA2_MMC_NORINTSTS_XFER_COMPLETE) {
/* Transfer Complete */
debug("r/w is done\n");
break;
@@ -302,20 +276,15 @@ static void mmc_change_clock(struct mmc_host *host, uint clock)
writew(0, &host->reg->clkcon);
- /*
- * CLKCON
- * SELFREQ[15:8] : base clock divided by value
- * ENSDCLK[2] : SD Clock Enable
- * STBLINTCLK[1] : Internal Clock Stable
- * ENINTCLK[0] : Internal Clock Enable
- */
div >>= 1;
- clk = (div << 8) | (1 << 0);
+ clk = ((div << TEGRA2_MMC_CLKCON_SDCLK_FREQ_SEL_SHIFT) |
+ TEGRA2_MMC_CLKCON_INTERNAL_CLOCK_EN_ENABLE);
writew(clk, &host->reg->clkcon);
/* Wait max 10 ms */
timeout = 10;
- while (!(readw(&host->reg->clkcon) & (1 << 1))) {
+ while (!(readw(&host->reg->clkcon) &
+ TEGRA2_MMC_CLKCON_INTERNAL_CLOCK_STABLE_READY)) {
if (timeout == 0) {
printf("%s: timeout error\n", __func__);
return;
@@ -324,7 +293,7 @@ static void mmc_change_clock(struct mmc_host *host, uint clock)
udelay(1000);
}
- clk |= (1 << 2);
+ clk |= TEGRA2_MMC_CLKCON_SD_CLOCK_EN_ENABLE;
writew(clk, &host->reg->clkcon);
debug("mmc_change_clock: clkcon = %08X\n", clk); @@ -370,12 +339,7 @@ static void mmc_reset(struct mmc_host *host)
unsigned int timeout;
debug(" mmc_reset called\n");
- /*
- * RSTALL[0] : Software reset for all
- * 1 = reset
- * 0 = work
- */
- writeb((1 << 0), &host->reg->swrst);
+ writeb(TEGRA2_MMC_SWRST_SW_RESET_FOR_ALL, &host->reg->swrst);
host->clock = 0;
@@ -383,7 +347,7 @@ static void mmc_reset(struct mmc_host *host)
timeout = 100;
/* hw clears the bit when it's done */
- while (readb(&host->reg->swrst) & (1 << 0)) {
+ while (readb(&host->reg->swrst) & TEGRA2_MMC_SWRST_SW_RESET_FOR_ALL) {
if (timeout == 0) {
printf("%s: timeout error\n", __func__);
return;
@@ -409,25 +373,21 @@ static int mmc_core_init(struct mmc *mmc)
writel(0xffffffff, &host->reg->norintsigen);
writeb(0xe, &host->reg->timeoutcon); /* TMCLK * 2^27 */
- /*
- * NORMAL Interrupt Status Enable Register init
- * [5] ENSTABUFRDRDY : Buffer Read Ready Status Enable
- * [4] ENSTABUFWTRDY : Buffer write Ready Status Enable
- * [1] ENSTASTANSCMPLT : Transfre Complete Status Enable
- * [0] ENSTACMDCMPLT : Command Complete Status Enable
- */
+
+ /* * NORMAL Interrupt Status Enable Register init */
mask = readl(&host->reg->norintstsen);
mask &= ~(0xffff);
mask |= (1 << 5) | (1 << 4) | (1 << 1) | (1 << 0);
+ mask |= (TEGRA2_MMC_NORINTSTSEN_CMD_COMPLETE |
+ TEGRA2_MMC_NORINTSTSEN_XFER_COMPLETE |
+ TEGRA2_MMC_NORINTSTSEN_BUFFER_WRITE_READY |
+ TEGRA2_MMC_NORINTSTSEN_BUFFER_READ_READY);
writel(mask, &host->reg->norintstsen);
- /*
- * NORMAL Interrupt Signal Enable Register init
- * [1] ENSTACMDCMPLT : Transfer Complete Signal Enable
- */
+ /* NORMAL Interrupt Signal Enable Register init */
mask = readl(&host->reg->norintsigen);
mask &= ~(0xffff);
- mask |= (1 << 1);
+ mask |= TEGRA2_MMC_NORINTSIGEN_XFER_COMPLETE;
writel(mask, &host->reg->norintsigen);
return 0;
diff --git a/drivers/mmc/tegra2_mmc.h b/drivers/mmc/tegra2_mmc.h index 28698e0..747aaa9 100644
--- a/drivers/mmc/tegra2_mmc.h
+++ b/drivers/mmc/tegra2_mmc.h
@@ -68,6 +68,86 @@ struct tegra2_mmc {
unsigned char res6[0x100]; /* RESERVED, offset 100h-1FFh */
};
+#define TEGRA2_MMC_HOSTCTL_DMASEL_MASK (3 << 3)
+#define TEGRA2_MMC_HOSTCTL_DMASEL_SDMA (0 << 3)
+#define TEGRA2_MMC_HOSTCTL_DMASEL_ADMA2_32BIT (2 << 3)
+#define TEGRA2_MMC_HOSTCTL_DMASEL_ADMA2_64BIT (3 << 3)
+
These are awfully long (and hard to parse with the _EN_ENABLE & _EN_DISABLE NV nomenclature).
Can you shorten them? The TEGRA2_MMC_ isn't really necessary, or at least s/b changed to TEGRA_MMC_,
since T30 won't redefine these bits.
+#define TEGRA2_MMC_TRNMOD_DMA_EN_MASK (1 << 0)
+#define TEGRA2_MMC_TRNMOD_DMA_EN_DISABLE (0 << 0)
+#define TEGRA2_MMC_TRNMOD_DMA_EN_ENABLE (1 << 0)
+
+#define TEGRA2_MMC_TRNMOD_BLOCK_COUNT_EN_MASK (1 << 1)
+#define TEGRA2_MMC_TRNMOD_BLOCK_COUNT_EN_DISABLE (0 << 1)
+#define TEGRA2_MMC_TRNMOD_BLOCK_COUNT_EN_ENABLE (1 << 1)
+
+#define TEGRA2_MMC_TRNMOD_DATA_XFER_DIR_SEL_MASK (1 << 4)
+#define TEGRA2_MMC_TRNMOD_DATA_XFER_DIR_SEL_WRITE (0 << 4)
+#define TEGRA2_MMC_TRNMOD_DATA_XFER_DIR_SEL_READ (1 << 4)
+
+#define TEGRA2_MMC_TRNMOD_MULTI_BLOCK_SELECT_MASK (1 << 5)
+#define TEGRA2_MMC_TRNMOD_MULTI_BLOCK_SELECT_DISABLE (0 << 5)
+#define TEGRA2_MMC_TRNMOD_MULTI_BLOCK_SELECT_ENABLE (1 << 5)
+
+#define TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_MASK (3 << 0)
+#define TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_NO_RESPONSE (0 << 0)
+#define TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_LENGTH_136 (1 << 0)
+#define TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_LENGTH_48 (2 << 0)
+#define TEGRA2_MMC_CMDREG_RESP_TYPE_SELECT_LENGTH_48_BUSY (3 << 0)
+
+#define TEGRA2_MMC_TRNMOD_CMD_CRC_CHECK_EN_MASK (1 << 3)
+#define TEGRA2_MMC_TRNMOD_CMD_CRC_CHECK_EN_DISABLE (0 << 3)
+#define TEGRA2_MMC_TRNMOD_CMD_CRC_CHECK_EN_ENABLE (1 << 3)
+
+#define TEGRA2_MMC_TRNMOD_CMD_INDEX_CHECK_EN_MASK (1 << 4)
+#define TEGRA2_MMC_TRNMOD_CMD_INDEX_CHECK_EN_DISABLE (0 << 4)
+#define TEGRA2_MMC_TRNMOD_CMD_INDEX_CHECK_EN_ENABLE (1 << 4)
+
+#define TEGRA2_MMC_TRNMOD_DATA_PRESENT_SELECT_MASK (1 << 5)
+#define TEGRA2_MMC_TRNMOD_DATA_PRESENT_SELECT_NO_DATA_TRANSFER (0 << 5)
+#define TEGRA2_MMC_TRNMOD_DATA_PRESENT_SELECT_DATA_TRANSFER (1 << 5)
+
+#define TEGRA2_MMC_PRNSTS_CMD_INHIBIT_CMD_MASK (1 << 0)
+#define TEGRA2_MMC_PRNSTS_CMD_INHIBIT_CMD_INACTIVE (0 << 0)
+#define TEGRA2_MMC_PRNSTS_CMD_INHIBIT_CMD_ACTIVE (1 << 0)
+
+#define TEGRA2_MMC_PRNSTS_CMD_INHIBIT_DAT_MASK (1 << 1)
+#define TEGRA2_MMC_PRNSTS_CMD_INHIBIT_DAT_INACTIVE (0 << 1)
+#define TEGRA2_MMC_PRNSTS_CMD_INHIBIT_DAT_ACTIVE (1 << 1)
+
+#define TEGRA2_MMC_CLKCON_INTERNAL_CLOCK_EN_MASK (1 << 0)
+#define TEGRA2_MMC_CLKCON_INTERNAL_CLOCK_EN_DISABLE (0 << 0)
+#define TEGRA2_MMC_CLKCON_INTERNAL_CLOCK_EN_ENABLE (1 << 0)
+
+#define TEGRA2_MMC_CLKCON_INTERNAL_CLOCK_STABLE_MASK (1 << 1)
+#define TEGRA2_MMC_CLKCON_INTERNAL_CLOCK_STABLE_NOT_READY (0 << 1)
+#define TEGRA2_MMC_CLKCON_INTERNAL_CLOCK_STABLE_READY (1 << 1)
+
+#define TEGRA2_MMC_CLKCON_SD_CLOCK_EN_MASK (1 << 2)
+#define TEGRA2_MMC_CLKCON_SD_CLOCK_EN_DISABLE (0 << 2)
+#define TEGRA2_MMC_CLKCON_SD_CLOCK_EN_ENABLE (1 << 2)
+
+#define TEGRA2_MMC_CLKCON_SDCLK_FREQ_SEL_SHIFT 8
+#define TEGRA2_MMC_CLKCON_SDCLK_FREQ_SEL_MASK (0xff << 8)
+
+#define TEGRA2_MMC_SWRST_SW_RESET_FOR_ALL (1 << 0)
+#define TEGRA2_MMC_SWRST_SW_RESET_FOR_CMD_LINE (1 << 1)
+#define TEGRA2_MMC_SWRST_SW_RESET_FOR_DAT_LINE (1 << 2)
+
+#define TEGRA2_MMC_NORINTSTS_CMD_COMPLETE (1 << 0)
+#define TEGRA2_MMC_NORINTSTS_XFER_COMPLETE (1 << 1)
+#define TEGRA2_MMC_NORINTSTS_DMA_INTERRUPT (1 << 3)
+#define TEGRA2_MMC_NORINTSTS_ERR_INTERRUPT (1 << 15)
+#define TEGRA2_MMC_NORINTSTS_CMD_TIMEOUT_ERR (1 << 16)
+
+#define TEGRA2_MMC_NORINTSTSEN_CMD_COMPLETE (1 << 0)
+#define TEGRA2_MMC_NORINTSTSEN_XFER_COMPLETE (1 << 1)
+#define TEGRA2_MMC_NORINTSTSEN_DMA_INTERRUPT (1 << 3)
+#define TEGRA2_MMC_NORINTSTSEN_BUFFER_WRITE_READY (1 << 4)
+#define TEGRA2_MMC_NORINTSTSEN_BUFFER_READ_READY (1 << 5)
+
+#define TEGRA2_MMC_NORINTSIGEN_XFER_COMPLETE (1 << 1)
+
struct mmc_host {
struct tegra2_mmc *reg;
unsigned int version; /* SDHCI spec. version */
Tom
--
1.7.3.1
-----------------------------------------------------------------------------------
This email message is for the sole use of the intended recipient(s) and may contain
confidential information. Any unauthorized review, use, disclosure or distribution
is prohibited. If you are not the intended recipient, please contact the sender by
reply email and destroy all copies of the original message.
-----------------------------------------------------------------------------------
More information about the U-Boot
mailing list