2009-01-22 01:57:00

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH RFC 0/10] Freescale "eSDHC" SDHCI support (was [PATCH] mmc: Add driver for Freescale eSDHC controllers)

On Thu, Jan 15, 2009 at 07:37:00AM +0800, Liu Dave wrote:
> > This patch adds support for the Freescale Enhanced Secure Digital
> > Host Controller Interface as found in some Freescale PowerPC
> > microprocessors (e.g. MPC837x SOCs).
>
> The Freescale ESDHC controller is basically compatible with the
> standard SDHC controller. but the eshdc expand some bits.
>
> The esdhc.c is much like the orignal sdhci.c.
> it is possible to merge them.

Seem to work indeed. Quite a lot of quirks needed though.

Patches on the way...

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2


2009-01-22 01:59:28

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 01/10] sdhci: Add quirk for controllers with no end-of-busy IRQ

From: Ben Dooks <[email protected]>

The Samsung SDHCI (and FSL eSDHC) controller block seems to fail
to generate an INT_DATA_END after the transfer has completed and
the bus busy state finished.

Changes in e809517f6fa5803a5a1cd56026f0e2190fc13d5c to use the
new busy method are the cause of the behaviour change.

Signed-off-by: Ben Dooks <[email protected]>
Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/sdhci.c | 5 ++++-
drivers/mmc/host/sdhci.h | 2 ++
2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 4d010a9..72cb7ef 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1286,8 +1286,11 @@ static void sdhci_cmd_irq(struct sdhci_host *host, u32 intmask)
if (host->cmd->data)
DBG("Cannot wait for busy signal when also "
"doing a data transfer");
- else
+ else if (!(host->quirks & SDHCI_QUIRK_NO_BUSY_IRQ))
return;
+
+ /* The controller does not support the end-of-busy IRQ,
+ * fall through and take the SDHCI_INT_RESPONSE */
}

if (intmask & SDHCI_INT_RESPONSE)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 31f4b15..73c03c6 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -210,6 +210,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_BROKEN_SMALL_PIO (1<<13)
/* Controller supports high speed but doesn't have the caps bit set */
#define SDHCI_QUIRK_FORCE_HIGHSPEED (1<<14)
+/* Controller does not provide transfer-complete interrupt when not busy */
+#define SDHCI_QUIRK_NO_BUSY_IRQ (1<<15)

int irq; /* Device IRQ */
void __iomem * ioaddr; /* Mapped address */
--
1.5.6.5

2009-01-22 02:00:35

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 02/10] sdhci: Add support for bus-specific IO memory accessors

Currently the SDHCI driver works with PCI accessors (write{l,b,w} and
read{l,b,w}).

With this patch drivers may change memory accessors, so that we can
support hosts with "weird" IO memory access requirments.

For example, in "FSL eSDHC" SDHCI hardware all registers are 32 bit
width, with big-endian addressing. That is, readb(0x2f) should turn
into readb(0x2c), and readw(0x2c) should be translated to
le16_to_cpu(readw(0x2e)).

Signed-off-by: Anton Vorontsov <[email protected]>
---

In this patch I swapped write{l,b,w}'s value and address arguments,
I don't like it anymore though. So in the next revision I'll revert
to the traditional arguments order.

Though... with current sdhci_{read,write} accessors it's quite
easy to make a mistake in the arguments, because there is no type
checking.

Pierre, would it be better if I'd use something like this:
host->writel(value, host->ioddr + reg) instead of
sdhci_writel(host, value, reg)? It's longer, but type checked...

drivers/mmc/host/sdhci.c | 205 ++++++++++++++++++++++++++++------------------
drivers/mmc/host/sdhci.h | 53 ++++++++++++
2 files changed, 177 insertions(+), 81 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 72cb7ef..0f5037d 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -43,35 +43,35 @@ static void sdhci_dumpregs(struct sdhci_host *host)
printk(KERN_DEBUG DRIVER_NAME ": ============== REGISTER DUMP ==============\n");

printk(KERN_DEBUG DRIVER_NAME ": Sys addr: 0x%08x | Version: 0x%08x\n",
- readl(host->ioaddr + SDHCI_DMA_ADDRESS),
- readw(host->ioaddr + SDHCI_HOST_VERSION));
+ sdhci_readl(host, SDHCI_DMA_ADDRESS),
+ sdhci_readw(host, SDHCI_HOST_VERSION));
printk(KERN_DEBUG DRIVER_NAME ": Blk size: 0x%08x | Blk cnt: 0x%08x\n",
- readw(host->ioaddr + SDHCI_BLOCK_SIZE),
- readw(host->ioaddr + SDHCI_BLOCK_COUNT));
+ sdhci_readw(host, SDHCI_BLOCK_SIZE),
+ sdhci_readw(host, SDHCI_BLOCK_COUNT));
printk(KERN_DEBUG DRIVER_NAME ": Argument: 0x%08x | Trn mode: 0x%08x\n",
- readl(host->ioaddr + SDHCI_ARGUMENT),
- readw(host->ioaddr + SDHCI_TRANSFER_MODE));
+ sdhci_readl(host, SDHCI_ARGUMENT),
+ sdhci_readw(host, SDHCI_TRANSFER_MODE));
printk(KERN_DEBUG DRIVER_NAME ": Present: 0x%08x | Host ctl: 0x%08x\n",
- readl(host->ioaddr + SDHCI_PRESENT_STATE),
- readb(host->ioaddr + SDHCI_HOST_CONTROL));
+ sdhci_readl(host, SDHCI_PRESENT_STATE),
+ sdhci_readb(host, SDHCI_HOST_CONTROL));
printk(KERN_DEBUG DRIVER_NAME ": Power: 0x%08x | Blk gap: 0x%08x\n",
- readb(host->ioaddr + SDHCI_POWER_CONTROL),
- readb(host->ioaddr + SDHCI_BLOCK_GAP_CONTROL));
+ sdhci_readb(host, SDHCI_POWER_CONTROL),
+ sdhci_readb(host, SDHCI_BLOCK_GAP_CONTROL));
printk(KERN_DEBUG DRIVER_NAME ": Wake-up: 0x%08x | Clock: 0x%08x\n",
- readb(host->ioaddr + SDHCI_WAKE_UP_CONTROL),
- readw(host->ioaddr + SDHCI_CLOCK_CONTROL));
+ sdhci_readb(host, SDHCI_WAKE_UP_CONTROL),
+ sdhci_readw(host, SDHCI_CLOCK_CONTROL));
printk(KERN_DEBUG DRIVER_NAME ": Timeout: 0x%08x | Int stat: 0x%08x\n",
- readb(host->ioaddr + SDHCI_TIMEOUT_CONTROL),
- readl(host->ioaddr + SDHCI_INT_STATUS));
+ sdhci_readb(host, SDHCI_TIMEOUT_CONTROL),
+ sdhci_readl(host, SDHCI_INT_STATUS));
printk(KERN_DEBUG DRIVER_NAME ": Int enab: 0x%08x | Sig enab: 0x%08x\n",
- readl(host->ioaddr + SDHCI_INT_ENABLE),
- readl(host->ioaddr + SDHCI_SIGNAL_ENABLE));
+ sdhci_readl(host, SDHCI_INT_ENABLE),
+ sdhci_readl(host, SDHCI_SIGNAL_ENABLE));
printk(KERN_DEBUG DRIVER_NAME ": AC12 err: 0x%08x | Slot int: 0x%08x\n",
- readw(host->ioaddr + SDHCI_ACMD12_ERR),
- readw(host->ioaddr + SDHCI_SLOT_INT_STATUS));
+ sdhci_readw(host, SDHCI_ACMD12_ERR),
+ sdhci_readw(host, SDHCI_SLOT_INT_STATUS));
printk(KERN_DEBUG DRIVER_NAME ": Caps: 0x%08x | Max curr: 0x%08x\n",
- readl(host->ioaddr + SDHCI_CAPABILITIES),
- readl(host->ioaddr + SDHCI_MAX_CURRENT));
+ sdhci_readl(host, SDHCI_CAPABILITIES),
+ sdhci_readl(host, SDHCI_MAX_CURRENT));

printk(KERN_DEBUG DRIVER_NAME ": ===========================================\n");
}
@@ -82,17 +82,47 @@ static void sdhci_dumpregs(struct sdhci_host *host)
* *
\*****************************************************************************/

+static u32 sdhci_def_readl(struct sdhci_host *host, int reg)
+{
+ return readl(host->ioaddr + reg);
+}
+
+static u16 sdhci_def_readw(struct sdhci_host *host, int reg)
+{
+ return readw(host->ioaddr + reg);
+}
+
+static u8 sdhci_def_readb(struct sdhci_host *host, int reg)
+{
+ return readb(host->ioaddr + reg);
+}
+
+static void sdhci_def_writel(struct sdhci_host *host, int reg, u32 val)
+{
+ writel(val, host->ioaddr + reg);
+}
+
+static void sdhci_def_writew(struct sdhci_host *host, int reg, u16 val)
+{
+ writew(val, host->ioaddr + reg);
+}
+
+static void sdhci_def_writeb(struct sdhci_host *host, int reg, u8 val)
+{
+ writeb(val, host->ioaddr + reg);
+}
+
static void sdhci_reset(struct sdhci_host *host, u8 mask)
{
unsigned long timeout;

if (host->quirks & SDHCI_QUIRK_NO_CARD_NO_RESET) {
- if (!(readl(host->ioaddr + SDHCI_PRESENT_STATE) &
+ if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) &
SDHCI_CARD_PRESENT))
return;
}

- writeb(mask, host->ioaddr + SDHCI_SOFTWARE_RESET);
+ sdhci_writeb(host, SDHCI_SOFTWARE_RESET, mask);

if (mask & SDHCI_RESET_ALL)
host->clock = 0;
@@ -101,7 +131,7 @@ static void sdhci_reset(struct sdhci_host *host, u8 mask)
timeout = 100;

/* hw clears the bit when it's done */
- while (readb(host->ioaddr + SDHCI_SOFTWARE_RESET) & mask) {
+ while (sdhci_readb(host, SDHCI_SOFTWARE_RESET) & mask) {
if (timeout == 0) {
printk(KERN_ERR "%s: Reset 0x%x never completed.\n",
mmc_hostname(host->mmc), (int)mask);
@@ -127,26 +157,26 @@ static void sdhci_init(struct sdhci_host *host)
SDHCI_INT_DMA_END | SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
SDHCI_INT_ADMA_ERROR;

- writel(intmask, host->ioaddr + SDHCI_INT_ENABLE);
- writel(intmask, host->ioaddr + SDHCI_SIGNAL_ENABLE);
+ sdhci_writel(host, SDHCI_INT_ENABLE, intmask);
+ sdhci_writel(host, SDHCI_SIGNAL_ENABLE, intmask);
}

static void sdhci_activate_led(struct sdhci_host *host)
{
u8 ctrl;

- ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+ ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
ctrl |= SDHCI_CTRL_LED;
- writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+ sdhci_writeb(host, SDHCI_HOST_CONTROL, ctrl);
}

static void sdhci_deactivate_led(struct sdhci_host *host)
{
u8 ctrl;

- ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+ ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
ctrl &= ~SDHCI_CTRL_LED;
- writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+ sdhci_writeb(host, SDHCI_HOST_CONTROL, ctrl);
}

#ifdef CONFIG_LEDS_CLASS
@@ -200,7 +230,7 @@ static void sdhci_read_block_pio(struct sdhci_host *host)

while (len) {
if (chunk == 0) {
- scratch = readl(host->ioaddr + SDHCI_BUFFER);
+ scratch = sdhci_readl(host, SDHCI_BUFFER);
chunk = 4;
}

@@ -252,7 +282,7 @@ static void sdhci_write_block_pio(struct sdhci_host *host)
len--;

if ((chunk == 4) || ((len == 0) && (blksize == 0))) {
- writel(scratch, host->ioaddr + SDHCI_BUFFER);
+ sdhci_writel(host, SDHCI_BUFFER, scratch);
chunk = 0;
scratch = 0;
}
@@ -287,7 +317,7 @@ static void sdhci_transfer_pio(struct sdhci_host *host)
(host->data->blocks == 1))
mask = ~0;

- while (readl(host->ioaddr + SDHCI_PRESENT_STATE) & mask) {
+ while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
if (host->data->flags & MMC_DATA_READ)
sdhci_read_block_pio(host);
else
@@ -576,7 +606,7 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
host->data_early = 0;

count = sdhci_calc_timeout(host, data);
- writeb(count, host->ioaddr + SDHCI_TIMEOUT_CONTROL);
+ sdhci_writeb(host, SDHCI_TIMEOUT_CONTROL, count);

if (host->flags & SDHCI_USE_DMA)
host->flags |= SDHCI_REQ_USE_DMA;
@@ -656,8 +686,8 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
WARN_ON(1);
host->flags &= ~SDHCI_REQ_USE_DMA;
} else {
- writel(host->adma_addr,
- host->ioaddr + SDHCI_ADMA_ADDRESS);
+ sdhci_writel(host, SDHCI_ADMA_ADDRESS,
+ host->adma_addr);
}
} else {
int sg_cnt;
@@ -676,8 +706,8 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
host->flags &= ~SDHCI_REQ_USE_DMA;
} else {
WARN_ON(sg_cnt != 1);
- writel(sg_dma_address(data->sg),
- host->ioaddr + SDHCI_DMA_ADDRESS);
+ sdhci_writel(host, SDHCI_DMA_ADDRESS,
+ sg_dma_address(data->sg));
}
}
}
@@ -688,14 +718,14 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
* is ADMA.
*/
if (host->version >= SDHCI_SPEC_200) {
- ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+ ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);
ctrl &= ~SDHCI_CTRL_DMA_MASK;
if ((host->flags & SDHCI_REQ_USE_DMA) &&
(host->flags & SDHCI_USE_ADMA))
ctrl |= SDHCI_CTRL_ADMA32;
else
ctrl |= SDHCI_CTRL_SDMA;
- writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+ sdhci_writeb(host, SDHCI_HOST_CONTROL, ctrl);
}

if (!(host->flags & SDHCI_REQ_USE_DMA)) {
@@ -705,9 +735,8 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
}

/* We do not handle DMA boundaries, so set it to max (512 KiB) */
- writew(SDHCI_MAKE_BLKSZ(7, data->blksz),
- host->ioaddr + SDHCI_BLOCK_SIZE);
- writew(data->blocks, host->ioaddr + SDHCI_BLOCK_COUNT);
+ sdhci_writew(host, SDHCI_BLOCK_SIZE, SDHCI_MAKE_BLKSZ(7, data->blksz));
+ sdhci_writew(host, SDHCI_BLOCK_COUNT, data->blocks);
}

static void sdhci_set_transfer_mode(struct sdhci_host *host,
@@ -728,7 +757,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
if (host->flags & SDHCI_REQ_USE_DMA)
mode |= SDHCI_TRNS_DMA;

- writew(mode, host->ioaddr + SDHCI_TRANSFER_MODE);
+ sdhci_writew(host, SDHCI_TRANSFER_MODE, mode);
}

static void sdhci_finish_data(struct sdhci_host *host)
@@ -797,7 +826,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
if (host->mrq->data && (cmd == host->mrq->data->stop))
mask &= ~SDHCI_DATA_INHIBIT;

- while (readl(host->ioaddr + SDHCI_PRESENT_STATE) & mask) {
+ while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
if (timeout == 0) {
printk(KERN_ERR "%s: Controller never released "
"inhibit bit(s).\n", mmc_hostname(host->mmc));
@@ -816,7 +845,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)

sdhci_prepare_data(host, cmd->data);

- writel(cmd->arg, host->ioaddr + SDHCI_ARGUMENT);
+ sdhci_writel(host, SDHCI_ARGUMENT, cmd->arg);

sdhci_set_transfer_mode(host, cmd->data);

@@ -844,8 +873,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
if (cmd->data)
flags |= SDHCI_CMD_DATA;

- writew(SDHCI_MAKE_CMD(cmd->opcode, flags),
- host->ioaddr + SDHCI_COMMAND);
+ sdhci_writew(host, SDHCI_COMMAND, SDHCI_MAKE_CMD(cmd->opcode, flags));
}

static void sdhci_finish_command(struct sdhci_host *host)
@@ -858,15 +886,15 @@ static void sdhci_finish_command(struct sdhci_host *host)
if (host->cmd->flags & MMC_RSP_136) {
/* CRC is stripped so we need to do some shifting. */
for (i = 0;i < 4;i++) {
- host->cmd->resp[i] = readl(host->ioaddr +
+ host->cmd->resp[i] = sdhci_readl(host,
SDHCI_RESPONSE + (3-i)*4) << 8;
if (i != 3)
host->cmd->resp[i] |=
- readb(host->ioaddr +
+ sdhci_readb(host,
SDHCI_RESPONSE + (3-i)*4-1);
}
} else {
- host->cmd->resp[0] = readl(host->ioaddr + SDHCI_RESPONSE);
+ host->cmd->resp[0] = sdhci_readl(host, SDHCI_RESPONSE);
}
}

@@ -890,7 +918,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
if (clock == host->clock)
return;

- writew(0, host->ioaddr + SDHCI_CLOCK_CONTROL);
+ sdhci_writew(host, SDHCI_CLOCK_CONTROL, 0);

if (clock == 0)
goto out;
@@ -903,11 +931,11 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)

clk = div << SDHCI_DIVIDER_SHIFT;
clk |= SDHCI_CLOCK_INT_EN;
- writew(clk, host->ioaddr + SDHCI_CLOCK_CONTROL);
+ sdhci_writew(host, SDHCI_CLOCK_CONTROL, clk);

/* Wait max 10 ms */
timeout = 10;
- while (!((clk = readw(host->ioaddr + SDHCI_CLOCK_CONTROL))
+ while (!((clk = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
& SDHCI_CLOCK_INT_STABLE)) {
if (timeout == 0) {
printk(KERN_ERR "%s: Internal clock never "
@@ -920,7 +948,7 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
}

clk |= SDHCI_CLOCK_CARD_EN;
- writew(clk, host->ioaddr + SDHCI_CLOCK_CONTROL);
+ sdhci_writew(host, SDHCI_CLOCK_CONTROL, clk);

out:
host->clock = clock;
@@ -934,7 +962,7 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
return;

if (power == (unsigned short)-1) {
- writeb(0, host->ioaddr + SDHCI_POWER_CONTROL);
+ sdhci_writeb(host, SDHCI_POWER_CONTROL, 0);
goto out;
}

@@ -943,7 +971,7 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
* a new value. Some controllers don't seem to like this though.
*/
if (!(host->quirks & SDHCI_QUIRK_SINGLE_POWER_WRITE))
- writeb(0, host->ioaddr + SDHCI_POWER_CONTROL);
+ sdhci_writeb(host, SDHCI_POWER_CONTROL, 0);

pwr = SDHCI_POWER_ON;

@@ -968,10 +996,9 @@ static void sdhci_set_power(struct sdhci_host *host, unsigned short power)
* and set turn on power at the same time, so set the voltage first.
*/
if ((host->quirks & SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER))
- writeb(pwr & ~SDHCI_POWER_ON,
- host->ioaddr + SDHCI_POWER_CONTROL);
+ sdhci_writeb(host, SDHCI_POWER_CONTROL, pwr & ~SDHCI_POWER_ON);

- writeb(pwr, host->ioaddr + SDHCI_POWER_CONTROL);
+ sdhci_writeb(host, SDHCI_POWER_CONTROL, pwr);

out:
host->power = power;
@@ -1000,7 +1027,7 @@ static void sdhci_request(struct mmc_host *mmc, struct mmc_request *mrq)

host->mrq = mrq;

- if (!(readl(host->ioaddr + SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
+ if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)
|| (host->flags & SDHCI_DEVICE_DEAD)) {
host->mrq->cmd->error = -ENOMEDIUM;
tasklet_schedule(&host->finish_tasklet);
@@ -1029,7 +1056,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
* Should clear out any weird states.
*/
if (ios->power_mode == MMC_POWER_OFF) {
- writel(0, host->ioaddr + SDHCI_SIGNAL_ENABLE);
+ sdhci_writel(host, SDHCI_SIGNAL_ENABLE, 0);
sdhci_init(host);
}

@@ -1040,7 +1067,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
else
sdhci_set_power(host, ios->vdd);

- ctrl = readb(host->ioaddr + SDHCI_HOST_CONTROL);
+ ctrl = sdhci_readb(host, SDHCI_HOST_CONTROL);

if (ios->bus_width == MMC_BUS_WIDTH_4)
ctrl |= SDHCI_CTRL_4BITBUS;
@@ -1052,7 +1079,7 @@ static void sdhci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
else
ctrl &= ~SDHCI_CTRL_HISPD;

- writeb(ctrl, host->ioaddr + SDHCI_HOST_CONTROL);
+ sdhci_writeb(host, SDHCI_HOST_CONTROL, ctrl);

/*
* Some (ENE) controllers go apeshit on some ios operation,
@@ -1080,7 +1107,7 @@ static int sdhci_get_ro(struct mmc_host *mmc)
if (host->flags & SDHCI_DEVICE_DEAD)
present = 0;
else
- present = readl(host->ioaddr + SDHCI_PRESENT_STATE);
+ present = sdhci_readl(host, SDHCI_PRESENT_STATE);

spin_unlock_irqrestore(&host->lock, flags);

@@ -1100,14 +1127,14 @@ static void sdhci_enable_sdio_irq(struct mmc_host *mmc, int enable)
if (host->flags & SDHCI_DEVICE_DEAD)
goto out;

- ier = readl(host->ioaddr + SDHCI_INT_ENABLE);
+ ier = sdhci_readl(host, SDHCI_INT_ENABLE);

ier &= ~SDHCI_INT_CARD_INT;
if (enable)
ier |= SDHCI_INT_CARD_INT;

- writel(ier, host->ioaddr + SDHCI_INT_ENABLE);
- writel(ier, host->ioaddr + SDHCI_SIGNAL_ENABLE);
+ sdhci_writel(host, SDHCI_INT_ENABLE, ier);
+ sdhci_writel(host, SDHCI_SIGNAL_ENABLE, ier);

out:
mmiowb();
@@ -1137,7 +1164,7 @@ static void sdhci_tasklet_card(unsigned long param)

spin_lock_irqsave(&host->lock, flags);

- if (!(readl(host->ioaddr + SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)) {
+ if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & SDHCI_CARD_PRESENT)) {
if (host->mrq) {
printk(KERN_ERR "%s: Card removed during transfer!\n",
mmc_hostname(host->mmc));
@@ -1341,8 +1368,8 @@ static void sdhci_data_irq(struct sdhci_host *host, u32 intmask)
* we need to at least restart the transfer.
*/
if (intmask & SDHCI_INT_DMA_END)
- writel(readl(host->ioaddr + SDHCI_DMA_ADDRESS),
- host->ioaddr + SDHCI_DMA_ADDRESS);
+ sdhci_writel(host, SDHCI_DMA_ADDRESS,
+ sdhci_readl(host, SDHCI_DMA_ADDRESS));

if (intmask & SDHCI_INT_DATA_END) {
if (host->cmd) {
@@ -1368,7 +1395,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)

spin_lock(&host->lock);

- intmask = readl(host->ioaddr + SDHCI_INT_STATUS);
+ intmask = sdhci_readl(host, SDHCI_INT_STATUS);

if (!intmask || intmask == 0xffffffff) {
result = IRQ_NONE;
@@ -1379,22 +1406,22 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
mmc_hostname(host->mmc), intmask);

if (intmask & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE)) {
- writel(intmask & (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE),
- host->ioaddr + SDHCI_INT_STATUS);
+ sdhci_writel(host, SDHCI_INT_STATUS, intmask &
+ (SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE));
tasklet_schedule(&host->card_tasklet);
}

intmask &= ~(SDHCI_INT_CARD_INSERT | SDHCI_INT_CARD_REMOVE);

if (intmask & SDHCI_INT_CMD_MASK) {
- writel(intmask & SDHCI_INT_CMD_MASK,
- host->ioaddr + SDHCI_INT_STATUS);
+ sdhci_writel(host, SDHCI_INT_STATUS,
+ intmask & SDHCI_INT_CMD_MASK);
sdhci_cmd_irq(host, intmask & SDHCI_INT_CMD_MASK);
}

if (intmask & SDHCI_INT_DATA_MASK) {
- writel(intmask & SDHCI_INT_DATA_MASK,
- host->ioaddr + SDHCI_INT_STATUS);
+ sdhci_writel(host, SDHCI_INT_STATUS,
+ intmask & SDHCI_INT_DATA_MASK);
sdhci_data_irq(host, intmask & SDHCI_INT_DATA_MASK);
}

@@ -1405,7 +1432,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
if (intmask & SDHCI_INT_BUS_POWER) {
printk(KERN_ERR "%s: Card is consuming too much power!\n",
mmc_hostname(host->mmc));
- writel(SDHCI_INT_BUS_POWER, host->ioaddr + SDHCI_INT_STATUS);
+ sdhci_writel(host, SDHCI_INT_STATUS, SDHCI_INT_BUS_POWER);
}

intmask &= ~SDHCI_INT_BUS_POWER;
@@ -1420,7 +1447,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
mmc_hostname(host->mmc), intmask);
sdhci_dumpregs(host);

- writel(intmask, host->ioaddr + SDHCI_INT_STATUS);
+ sdhci_writel(host, SDHCI_INT_STATUS, intmask);
}

result = IRQ_HANDLED;
@@ -1530,9 +1557,25 @@ int sdhci_add_host(struct sdhci_host *host)
if (debug_quirks)
host->quirks = debug_quirks;

+ if (host->ops->readl) {
+ host->readl = host->ops->readl;
+ host->readw = host->ops->readw;
+ host->readb = host->ops->readb;
+ host->writel = host->ops->writel;
+ host->writew = host->ops->writew;
+ host->writeb = host->ops->writeb;
+ } else {
+ host->readl = sdhci_def_readl;
+ host->readw = sdhci_def_readw;
+ host->readb = sdhci_def_readb;
+ host->writel = sdhci_def_writel;
+ host->writew = sdhci_def_writew;
+ host->writeb = sdhci_def_writeb;
+ }
+
sdhci_reset(host, SDHCI_RESET_ALL);

- host->version = readw(host->ioaddr + SDHCI_HOST_VERSION);
+ host->version = sdhci_readw(host, SDHCI_HOST_VERSION);
host->version = (host->version & SDHCI_SPEC_VER_MASK)
>> SDHCI_SPEC_VER_SHIFT;
if (host->version > SDHCI_SPEC_200) {
@@ -1541,7 +1584,7 @@ int sdhci_add_host(struct sdhci_host *host)
host->version);
}

- caps = readl(host->ioaddr + SDHCI_CAPABILITIES);
+ caps = sdhci_readl(host, SDHCI_CAPABILITIES);

if (host->quirks & SDHCI_QUIRK_FORCE_DMA)
host->flags |= SDHCI_USE_DMA;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 73c03c6..fed0022 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -9,6 +9,7 @@
* your option) any later version.
*/

+#include <linux/types.h>
#include <linux/scatterlist.h>

/*
@@ -263,14 +264,66 @@ struct sdhci_host {

struct timer_list timer; /* Timer for timeouts */

+ /*
+ * These accessors duplicate sdhci_ops, but there are two reasons for
+ * this:
+ * 1. sdhci_ops are const, so the sdhci driver won't able to assign
+ * default ops;
+ * 2. Using host->X instead of host->ops->X saves us one dereference.
+ * This can be useful in PIO mode. (Though the benefit of this
+ * is negligibly small).
+ */
+ u32 (*readl)(struct sdhci_host *host, int reg);
+ u16 (*readw)(struct sdhci_host *host, int reg);
+ u8 (*readb)(struct sdhci_host *host, int reg);
+ void (*writel)(struct sdhci_host *host, int reg, u32 val);
+ void (*writew)(struct sdhci_host *host, int reg, u16 val);
+ void (*writeb)(struct sdhci_host *host, int reg, u8 val);
+
unsigned long private[0] ____cacheline_aligned;
};


struct sdhci_ops {
+ u32 (*readl)(struct sdhci_host *host, int reg);
+ u16 (*readw)(struct sdhci_host *host, int reg);
+ u8 (*readb)(struct sdhci_host *host, int reg);
+ void (*writel)(struct sdhci_host *host, int reg, u32 val);
+ void (*writew)(struct sdhci_host *host, int reg, u16 val);
+ void (*writeb)(struct sdhci_host *host, int reg, u8 val);
+
int (*enable_dma)(struct sdhci_host *host);
};

+static inline void sdhci_writel(struct sdhci_host *host, int reg, u32 val)
+{
+ host->writel(host, reg, val);
+}
+
+static inline void sdhci_writew(struct sdhci_host *host, int reg, u16 val)
+{
+ host->writew(host, reg, val);
+}
+
+static inline void sdhci_writeb(struct sdhci_host *host, int reg, u8 val)
+{
+ host->writeb(host, reg, val);
+}
+
+static inline u32 sdhci_readl(struct sdhci_host *host, int reg)
+{
+ return host->readl(host, reg);
+}
+
+static inline u16 sdhci_readw(struct sdhci_host *host, int reg)
+{
+ return host->readw(host, reg);
+}
+
+static inline u8 sdhci_readb(struct sdhci_host *host, int reg)
+{
+ return host->readb(host, reg);
+}

extern struct sdhci_host *sdhci_alloc_host(struct device *dev,
size_t priv_size);
--
1.5.6.5

2009-01-22 02:00:51

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 03/10] sdhci: Add support for card-detection polling

This patch adds SDHCI_QUIRK_BROKEN_CARD_DETECTION quirk. When specified,
sdhci driver will set MMC_CAP_NEEDS_POLL MMC host capability, and won't
enable card insert/remove interrupts.

This is needed for hosts with unreliable card detection, such as FSL
eSDHC. The original eSDHC driver was tring to "debounce" card-detection
IRQs by reading present state and disabling particular interrupts. But
with this debouncing scheme I noticed that sometimes we miss card
insertion/removal events.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/sdhci.c | 6 ++++++
drivers/mmc/host/sdhci.h | 2 ++
2 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 0f5037d..5a7a584 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -157,6 +157,9 @@ static void sdhci_init(struct sdhci_host *host)
SDHCI_INT_DMA_END | SDHCI_INT_DATA_END | SDHCI_INT_RESPONSE |
SDHCI_INT_ADMA_ERROR;

+ if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+ intmask &= ~(SDHCI_INT_CARD_REMOVE | SDHCI_INT_CARD_INSERT);
+
sdhci_writel(host, SDHCI_INT_ENABLE, intmask);
sdhci_writel(host, SDHCI_SIGNAL_ENABLE, intmask);
}
@@ -1677,6 +1680,9 @@ int sdhci_add_host(struct sdhci_host *host)
mmc->f_max = host->max_clk;
mmc->caps = MMC_CAP_4_BIT_DATA | MMC_CAP_SDIO_IRQ;

+ if (host->quirks & SDHCI_QUIRK_BROKEN_CARD_DETECTION)
+ mmc->caps |= MMC_CAP_NEEDS_POLL;
+
if ((caps & SDHCI_CAN_DO_HISPD) ||
(host->quirks & SDHCI_QUIRK_FORCE_HIGHSPEED))
mmc->caps |= MMC_CAP_SD_HIGHSPEED;
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index fed0022..f357722 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -213,6 +213,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_FORCE_HIGHSPEED (1<<14)
/* Controller does not provide transfer-complete interrupt when not busy */
#define SDHCI_QUIRK_NO_BUSY_IRQ (1<<15)
+/* Controller has unreliable card detection */
+#define SDHCI_QUIRK_BROKEN_CARD_DETECTION (1<<16)

int irq; /* Device IRQ */
void __iomem * ioaddr; /* Mapped address */
--
1.5.6.5

2009-01-22 02:01:19

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 04/10] sdhci: Add support for hosts reporting inverted write-protect state

This patch adds SDHCI_QUIRK_INVERTED_WRITE_PROTECT quirk. When
specified, the sdhci driver will invert WP state.

p.s. Actually, the quirk is more board-specific than
controller-specific.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/sdhci.c | 2 ++
drivers/mmc/host/sdhci.h | 2 ++
2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 5a7a584..63809f5 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1114,6 +1114,8 @@ static int sdhci_get_ro(struct mmc_host *mmc)

spin_unlock_irqrestore(&host->lock, flags);

+ if (host->quirks & SDHCI_QUIRK_INVERTED_WRITE_PROTECT)
+ return !!(present & SDHCI_WRITE_PROTECT);
return !(present & SDHCI_WRITE_PROTECT);
}

diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index f357722..9cd2b78 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -215,6 +215,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_NO_BUSY_IRQ (1<<15)
/* Controller has unreliable card detection */
#define SDHCI_QUIRK_BROKEN_CARD_DETECTION (1<<16)
+/* Controller reports inverted write-protect state */
+#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT (1<<17)

int irq; /* Device IRQ */
void __iomem * ioaddr; /* Mapped address */
--
1.5.6.5

2009-01-22 02:01:36

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 05/10] sdhci: Add support for hosts with strict 32 bit addressing

SDHCI driver must take special care when working with "triggering"
registers on hosts with strict 32 bit addressing.

In FSL eSDHC hosts all registers are 32 bit width, writing to the
first half of any register will cause [undefined?] write the second
half of the register. That is, 16 bit write to the TRANSFER_MODE
register, makes hardware see a bogus write to the COMMAND register
(these two registers are adjacent).

This patch adds SDHCI_QUIRK_32BIT_REGISTERS quirk. When specified,
the sdhci driver will try to "pack" all dangerous writes into single
32 bit write transaction.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/sdhci.c | 18 +++++++++++++-----
drivers/mmc/host/sdhci.h | 4 ++++
2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 63809f5..c3737fe 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -742,13 +742,13 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
sdhci_writew(host, SDHCI_BLOCK_COUNT, data->blocks);
}

-static void sdhci_set_transfer_mode(struct sdhci_host *host,
+static u16 sdhci_get_transfer_mode(struct sdhci_host *host,
struct mmc_data *data)
{
u16 mode;

if (data == NULL)
- return;
+ return 0;

WARN_ON(!host->data);

@@ -760,7 +760,7 @@ static void sdhci_set_transfer_mode(struct sdhci_host *host,
if (host->flags & SDHCI_REQ_USE_DMA)
mode |= SDHCI_TRNS_DMA;

- sdhci_writew(host, SDHCI_TRANSFER_MODE, mode);
+ return mode;
}

static void sdhci_finish_data(struct sdhci_host *host)
@@ -812,6 +812,7 @@ static void sdhci_finish_data(struct sdhci_host *host)
static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
{
int flags;
+ u16 xfer_mode;
u32 mask;
unsigned long timeout;

@@ -850,7 +851,7 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)

sdhci_writel(host, SDHCI_ARGUMENT, cmd->arg);

- sdhci_set_transfer_mode(host, cmd->data);
+ xfer_mode = sdhci_get_transfer_mode(host, cmd->data);

if ((cmd->flags & MMC_RSP_136) && (cmd->flags & MMC_RSP_BUSY)) {
printk(KERN_ERR "%s: Unsupported response type!\n",
@@ -876,7 +877,14 @@ static void sdhci_send_command(struct sdhci_host *host, struct mmc_command *cmd)
if (cmd->data)
flags |= SDHCI_CMD_DATA;

- sdhci_writew(host, SDHCI_COMMAND, SDHCI_MAKE_CMD(cmd->opcode, flags));
+ if (host->quirks & SDHCI_QUIRK_32BIT_REGISTERS) {
+ sdhci_writel(host, SDHCI_TRANSFER_MODE,
+ SDHCI_MAKE_CMD_32BIT(cmd->opcode, flags, xfer_mode));
+ } else {
+ sdhci_writew(host, SDHCI_TRANSFER_MODE, xfer_mode);
+ sdhci_writew(host, SDHCI_COMMAND,
+ SDHCI_MAKE_CMD(cmd->opcode, flags));
+ }
}

static void sdhci_finish_command(struct sdhci_host *host)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 9cd2b78..2747340 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -44,6 +44,8 @@
#define SDHCI_CMD_RESP_SHORT_BUSY 0x03

#define SDHCI_MAKE_CMD(c, f) (((c & 0xff) << 8) | (f & 0xff))
+#define SDHCI_MAKE_CMD_32BIT(c, f, m) \
+ ((((c) & 0xff) << 24) | (((f) & 0xff) << 16) | ((m) & 0x37))

#define SDHCI_RESPONSE 0x10

@@ -217,6 +219,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_BROKEN_CARD_DETECTION (1<<16)
/* Controller reports inverted write-protect state */
#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT (1<<17)
+/* Controller has all registers of 32 bit width */
+#define SDHCI_QUIRK_32BIT_REGISTERS (1<<18)

int irq; /* Device IRQ */
void __iomem * ioaddr; /* Mapped address */
--
1.5.6.5

2009-01-22 02:01:52

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 06/10] sdhci: Add quirk to suppress PIO interrupts during DMA transfers

Some hosts (that is, FSL eSDHC) throw PIO interrupts during DMA
transfers, this causes tons of unneeded interrupts, and thus highly
degraded speed.

This patch adds SDHCI_QUIRK_PIO_IRQS_DURING_DMA quirk. When specified,
the sdhci driver will disable PIO interrupts during DMA transfers.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/sdhci.c | 30 ++++++++++++++++++++++++++++++
drivers/mmc/host/sdhci.h | 3 +++
2 files changed, 33 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index c3737fe..57b8ffe 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -589,6 +589,33 @@ static u8 sdhci_calc_timeout(struct sdhci_host *host, struct mmc_data *data)
return count;
}

+static void sdhci_set_pio_irqs(struct sdhci_host *host, bool state)
+{
+ bool current_state = !(host->flags & SDHCI_PIO_DISABLED);
+ u32 ier;
+
+ /*
+ * We only care about PIO IRQs if the host issues PIO IRQs during
+ * DMA transfers. Otherwise we can keep the irqs always enabled.
+ */
+ if (!(host->quirks & SDHCI_QUIRK_PIO_IRQS_DURING_DMA))
+ return;
+
+ if (current_state == state)
+ return;
+
+ ier = sdhci_readl(host, SDHCI_INT_ENABLE);
+ if (state) {
+ ier |= SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL;
+ host->flags &= ~SDHCI_PIO_DISABLED;
+ } else {
+ ier &= ~(SDHCI_INT_DATA_AVAIL | SDHCI_INT_SPACE_AVAIL);
+ host->flags |= SDHCI_PIO_DISABLED;
+ }
+ sdhci_writel(host, SDHCI_INT_ENABLE, ier);
+ sdhci_writel(host, SDHCI_SIGNAL_ENABLE, ier);
+}
+
static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
{
u8 count;
@@ -735,6 +762,9 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
sg_miter_start(&host->sg_miter,
data->sg, data->sg_len, SG_MITER_ATOMIC);
host->blocks = data->blocks;
+ sdhci_set_pio_irqs(host, true);
+ } else {
+ sdhci_set_pio_irqs(host, false);
}

/* We do not handle DMA boundaries, so set it to max (512 KiB) */
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 2747340..7e3b01f 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -221,6 +221,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_INVERTED_WRITE_PROTECT (1<<17)
/* Controller has all registers of 32 bit width */
#define SDHCI_QUIRK_32BIT_REGISTERS (1<<18)
+/* Controller issues PIO interrupts during DMA transfers */
+#define SDHCI_QUIRK_PIO_IRQS_DURING_DMA (1<<19)

int irq; /* Device IRQ */
void __iomem * ioaddr; /* Mapped address */
@@ -242,6 +244,7 @@ struct sdhci_host {
#define SDHCI_USE_ADMA (1<<1) /* Host is ADMA capable */
#define SDHCI_REQ_USE_DMA (1<<2) /* Use DMA for this req. */
#define SDHCI_DEVICE_DEAD (1<<3) /* Device unresponsive */
+#define SDHCI_PIO_DISABLED (1<<4) /* PIO IRQs disabled */

unsigned int version; /* SDHCI spec. version */

--
1.5.6.5

2009-01-22 02:02:18

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 07/10] sdhci: Add support for hosts that don't specify clocks in the cap. register

FSL eSDHC hosts don't provide clocks bits in the capabilities register,
instead we're getting clocks values from the device tree.

There is somewhat similar change[1] from Ben Dooks, the change adds
callbacks for getting the clocks. But for eSDHC the callbacks are
superfluous, since the clocks are static.

[1] http://lkml.org/lkml/2008/12/2/157

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/sdhci.c | 31 +++++++++++++++----------------
1 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 57b8ffe..9ac088a 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -1693,24 +1693,23 @@ int sdhci_add_host(struct sdhci_host *host)
mmc_dev(host->mmc)->dma_mask = &host->dma_mask;
}

- host->max_clk =
- (caps & SDHCI_CLOCK_BASE_MASK) >> SDHCI_CLOCK_BASE_SHIFT;
- if (host->max_clk == 0) {
- printk(KERN_ERR "%s: Hardware doesn't specify base clock "
- "frequency.\n", mmc_hostname(mmc));
- return -ENODEV;
+ if (!host->max_clk) {
+ host->max_clk = (caps & SDHCI_CLOCK_BASE_MASK) >>
+ SDHCI_CLOCK_BASE_SHIFT;
+ if (host->max_clk == 0) {
+ printk(KERN_ERR "%s: Hardware doesn't specify base "
+ "clock frequency.\n", mmc_hostname(mmc));
+ return -ENODEV;
+ }
+ host->max_clk *= 1000000;
}
- host->max_clk *= 1000000;

- host->timeout_clk =
- (caps & SDHCI_TIMEOUT_CLK_MASK) >> SDHCI_TIMEOUT_CLK_SHIFT;
- if (host->timeout_clk == 0) {
- printk(KERN_ERR "%s: Hardware doesn't specify timeout clock "
- "frequency.\n", mmc_hostname(mmc));
- return -ENODEV;
+ if (!host->timeout_clk) {
+ host->timeout_clk = (caps & SDHCI_TIMEOUT_CLK_MASK) >>
+ SDHCI_TIMEOUT_CLK_SHIFT;
+ if (caps & SDHCI_TIMEOUT_CLK_UNIT)
+ host->timeout_clk *= 1000;
}
- if (caps & SDHCI_TIMEOUT_CLK_UNIT)
- host->timeout_clk *= 1000;

/*
* Set host parameters.
--
1.5.6.5

2009-01-22 02:02:38

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 08/10] sdhci: Add set_clock callback

FSL eSDHC hosts have incompatible register map to manage the SDCLK.
This patch adds set_clock callback so that drivers could overwrite
set_clock behaviour.

Similar patch[1] was posted by Ben Dooks, though in Ben's version the
callback is named change_clock, plus the patch has some unrelated bits
that makes the patch difficult to reuse.

[1] http://lkml.org/lkml/2008/12/2/160

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/sdhci.c | 5 +++++
drivers/mmc/host/sdhci.h | 2 ++
2 files changed, 7 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 9ac088a..e456ae0 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -959,6 +959,11 @@ static void sdhci_set_clock(struct sdhci_host *host, unsigned int clock)
if (clock == host->clock)
return;

+ if (host->ops->set_clock) {
+ host->ops->set_clock(host, clock);
+ return;
+ }
+
sdhci_writew(host, SDHCI_CLOCK_CONTROL, 0);

if (clock == 0)
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 7e3b01f..39cfd5a 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -303,6 +303,8 @@ struct sdhci_ops {
void (*writew)(struct sdhci_host *host, int reg, u16 val);
void (*writeb)(struct sdhci_host *host, int reg, u8 val);

+ void (*set_clock)(struct sdhci_host *host, unsigned int clock);
+
int (*enable_dma)(struct sdhci_host *host);
};

--
1.5.6.5

2009-01-22 02:02:58

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 09/10] sdhci: Add quirk for Freescale eSDHC controllers

This patch adds SDHCI_QUIRK_FSL quirk. The quirk is used to instruct
the sdhci driver about various FSL eSDHC host incompatibilities:

1) FSL eSDHC controllers can support maximum block size up to 4096
bytes. The MBL (Maximum Block Length) field in the capabilities
register extended by one bit.

(Should we implement a dedicated quirk for this? I.e.
SDHCI_QUIRK_MAX_BLK_SZ_4096?)

2) sdhci_init() is needed after error conditions.

(Can we safely do this for all controllers?)

3) Small udelay is needed to make eSDHC work in PIO mode. Without
the delay reading causes endless interrupt storm, and writing
corrupts data. The first guess would be that we must wait for
some bit in some register, but I didn't find any reliable bits
that changes before and after the delay. Though, more investigation
on this is in my todo list.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/sdhci.c | 36 ++++++++++++++++++++++++++++++------
drivers/mmc/host/sdhci.h | 3 +++
2 files changed, 33 insertions(+), 6 deletions(-)

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index e456ae0..82085e4 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -321,6 +321,10 @@ static void sdhci_transfer_pio(struct sdhci_host *host)
mask = ~0;

while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
+ /* FIXME */
+ if (host->quirks & SDHCI_QUIRK_FSL)
+ udelay(100);
+
if (host->data->flags & MMC_DATA_READ)
sdhci_read_block_pio(host);
else
@@ -618,6 +622,7 @@ static void sdhci_set_pio_irqs(struct sdhci_host *host, bool state)

static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
{
+ u16 blksz;
u8 count;
u8 ctrl;
int ret;
@@ -768,7 +773,12 @@ static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
}

/* We do not handle DMA boundaries, so set it to max (512 KiB) */
- sdhci_writew(host, SDHCI_BLOCK_SIZE, SDHCI_MAKE_BLKSZ(7, data->blksz));
+ if (host->quirks & SDHCI_QUIRK_FSL)
+ blksz = data->blksz;
+ else
+ blksz = SDHCI_MAKE_BLKSZ(7, data->blksz);
+
+ sdhci_writew(host, SDHCI_BLOCK_SIZE, blksz);
sdhci_writew(host, SDHCI_BLOCK_COUNT, data->blocks);
}

@@ -1270,6 +1280,9 @@ static void sdhci_tasklet_finish(unsigned long param)
controllers do not like that. */
sdhci_reset(host, SDHCI_RESET_CMD);
sdhci_reset(host, SDHCI_RESET_DATA);
+ /* FSL controllers need this. */
+ if (host->quirks & SDHCI_QUIRK_FSL)
+ sdhci_init(host);
}

host->mrq = NULL;
@@ -1779,13 +1792,24 @@ int sdhci_add_host(struct sdhci_host *host)
* Maximum block size. This varies from controller to controller and
* is specified in the capabilities register.
*/
- mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >> SDHCI_MAX_BLOCK_SHIFT;
- if (mmc->max_blk_size >= 3) {
+ if (host->quirks & SDHCI_QUIRK_FSL) {
+ mmc->max_blk_size = (caps & SDHCI_FSL_MAX_BLOCK_MASK) >>
+ SDHCI_MAX_BLOCK_SHIFT;
+ if (mmc->max_blk_size >= 4)
+ mmc->max_blk_size = ~0;
+ } else {
+ mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >>
+ SDHCI_MAX_BLOCK_SHIFT;
+ if (mmc->max_blk_size >= 3)
+ mmc->max_blk_size = ~0;
+ }
+
+ if (mmc->max_blk_size == ~0) {
printk(KERN_WARNING "%s: Invalid maximum block size, "
"assuming 512 bytes\n", mmc_hostname(mmc));
- mmc->max_blk_size = 512;
- } else
- mmc->max_blk_size = 512 << mmc->max_blk_size;
+ mmc->max_blk_size = 0;
+ }
+ mmc->max_blk_size = 512 << mmc->max_blk_size;

/*
* Maximum block count.
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 39cfd5a..7b98ec6 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -138,6 +138,7 @@
#define SDHCI_CLOCK_BASE_MASK 0x00003F00
#define SDHCI_CLOCK_BASE_SHIFT 8
#define SDHCI_MAX_BLOCK_MASK 0x00030000
+#define SDHCI_FSL_MAX_BLOCK_MASK 0x00070000
#define SDHCI_MAX_BLOCK_SHIFT 16
#define SDHCI_CAN_DO_ADMA2 0x00080000
#define SDHCI_CAN_DO_ADMA1 0x00100000
@@ -223,6 +224,8 @@ struct sdhci_host {
#define SDHCI_QUIRK_32BIT_REGISTERS (1<<18)
/* Controller issues PIO interrupts during DMA transfers */
#define SDHCI_QUIRK_PIO_IRQS_DURING_DMA (1<<19)
+/* Controller is Freescale eSDHC */
+#define SDHCI_QUIRK_FSL (1<<20)

int irq; /* Device IRQ */
void __iomem * ioaddr; /* Mapped address */
--
1.5.6.5

2009-01-22 02:03:24

by Anton Vorontsov

[permalink] [raw]
Subject: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

This patch adds a new driver: sdhci-of. The driver is similar to
the sdhci-pci, it contains common probe code, and controller-specific
ops and quirks.

So far there are only Freescale eSDHC ops and quirks.

Signed-off-by: Anton Vorontsov <[email protected]>
---
drivers/mmc/host/Kconfig | 10 ++
drivers/mmc/host/Makefile | 1 +
drivers/mmc/host/sdhci-of.c | 274 +++++++++++++++++++++++++++++++++++++++++++
3 files changed, 285 insertions(+), 0 deletions(-)
create mode 100644 drivers/mmc/host/sdhci-of.c

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index dfa585f..8b5cdef 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -65,6 +65,16 @@ config MMC_RICOH_MMC

If unsure, say Y.

+config MMC_SDHCI_OF
+ tristate "SDHCI support on OpenFirmware platforms"
+ depends on MMC_SDHCI && PPC_OF
+ help
+ This selects the OF Secure Digital Host Controller Interface.
+ So far there is only Freescale eSDHC controller known to exists
+ on OF platforms.
+
+ If unsure, say N.
+
config MMC_OMAP
tristate "TI OMAP Multimedia Card Interface support"
depends on ARCH_OMAP
diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
index f485328..5296793 100644
--- a/drivers/mmc/host/Makefile
+++ b/drivers/mmc/host/Makefile
@@ -12,6 +12,7 @@ obj-$(CONFIG_MMC_IMX) += imxmmc.o
obj-$(CONFIG_MMC_SDHCI) += sdhci.o
obj-$(CONFIG_MMC_SDHCI_PCI) += sdhci-pci.o
obj-$(CONFIG_MMC_RICOH_MMC) += ricoh_mmc.o
+obj-$(CONFIG_MMC_SDHCI_OF) += sdhci-of.o
obj-$(CONFIG_MMC_WBSD) += wbsd.o
obj-$(CONFIG_MMC_AU1X) += au1xmmc.o
obj-$(CONFIG_MMC_OMAP) += omap.o
diff --git a/drivers/mmc/host/sdhci-of.c b/drivers/mmc/host/sdhci-of.c
new file mode 100644
index 0000000..8ee89a1
--- /dev/null
+++ b/drivers/mmc/host/sdhci-of.c
@@ -0,0 +1,274 @@
+/*
+ * OpenFirmware bindings for Secure Digital Host Controller Interface.
+ *
+ * Copyright (c) 2007 Freescale Semiconductor, Inc.
+ * Copyright (c) 2009 MontaVista Software, Inc.
+ *
+ * Authors: Xiaobo Xie <[email protected]>
+ * Anton Vorontsov <[email protected]>
+ *
+ * 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.
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/io.h>
+#include <linux/interrupt.h>
+#include <linux/delay.h>
+#include <linux/of.h>
+#include <linux/of_platform.h>
+#include <linux/mmc/host.h>
+#include "sdhci.h"
+
+struct sdhci_of_data {
+ unsigned int quirks;
+ struct sdhci_ops ops;
+};
+
+/*
+ * Ops and quirks for the Freescale eSDHC controller.
+ */
+
+#define ESDHC_DMA_SYSCTL 0x40c
+#define ESDHC_DMA_SNOOP 0x00000040
+
+#define ESDHC_SYSTEM_CONTROL 0x2c
+#define ESDHC_CLOCK_MASK 0x0000fff0
+#define ESDHC_PREDIV_SHIFT 8
+#define ESDHC_DIVIDER_SHIFT 4
+#define ESDHC_CLOCK_PEREN 0x00000004
+#define ESDHC_CLOCK_HCKEN 0x00000002
+#define ESDHC_CLOCK_IPGEN 0x00000001
+
+static u32 esdhc_readl(struct sdhci_host *host, int reg)
+{
+ return in_be32(host->ioaddr + reg);
+}
+
+static u16 esdhc_readw(struct sdhci_host *host, int reg)
+{
+ return in_be16(host->ioaddr + (reg ^ 0x2));
+}
+
+static u8 esdhc_readb(struct sdhci_host *host, int reg)
+{
+ return in_8(host->ioaddr + (reg ^ 0x3));
+}
+
+static void esdhc_writel(struct sdhci_host *host, int reg, u32 val)
+{
+ out_be32(host->ioaddr + reg, val);
+}
+
+static void esdhc_writew(struct sdhci_host *host, int reg, u16 val)
+{
+ int base = reg & ~0x3;
+ int shift = (reg & 0x2) * 8;
+
+ clrsetbits_be32(host->ioaddr + base, 0xffff << shift, val << shift);
+}
+
+static void esdhc_writeb(struct sdhci_host *host, int reg, u8 val)
+{
+ int base = reg & ~0x3;
+ int shift = (reg & 0x3) * 8;
+
+ clrsetbits_be32(host->ioaddr + base , 0xff << shift, val << shift);
+}
+
+static void esdhc_set_clock(struct sdhci_host *host, unsigned int clock)
+{
+ int div;
+ int pre_div = 1;
+
+ setbits32(host->ioaddr + ESDHC_SYSTEM_CONTROL, ESDHC_CLOCK_IPGEN |
+ ESDHC_CLOCK_HCKEN | ESDHC_CLOCK_PEREN);
+
+ clrbits32(host->ioaddr + ESDHC_SYSTEM_CONTROL, ESDHC_CLOCK_MASK);
+
+ if (clock == 0)
+ goto out;
+
+ if (host->max_clk / 16 > clock) {
+ for (; pre_div < 256; pre_div *= 2) {
+ if (host->max_clk / pre_div < clock*16)
+ break;
+ }
+ }
+
+ for (div = 1; div <= 16; div++) {
+ if (host->max_clk / (div * pre_div) <= clock)
+ break;
+ }
+
+ pre_div >>= 1;
+ div -= 1;
+
+ setbits32(host->ioaddr + ESDHC_SYSTEM_CONTROL,
+ div << ESDHC_DIVIDER_SHIFT | pre_div << ESDHC_PREDIV_SHIFT);
+ mdelay(10);
+out:
+ host->clock = clock;
+}
+
+static int esdhc_enable_dma(struct sdhci_host *host)
+{
+ setbits32(host->ioaddr + ESDHC_DMA_SYSCTL, ESDHC_DMA_SNOOP);
+ return 0;
+}
+
+static struct sdhci_of_data sdhci_esdhc = {
+ .quirks = SDHCI_QUIRK_FSL |
+ SDHCI_QUIRK_32BIT_REGISTERS |
+ SDHCI_QUIRK_PIO_IRQS_DURING_DMA |
+ SDHCI_QUIRK_BROKEN_CARD_DETECTION |
+ SDHCI_QUIRK_INVERTED_WRITE_PROTECT |
+ SDHCI_QUIRK_NO_BUSY_IRQ |
+ SDHCI_QUIRK_NO_CARD_NO_RESET,
+ .ops = {
+ .readl = esdhc_readl,
+ .readw = esdhc_readw,
+ .readb = esdhc_readb,
+ .writel = esdhc_writel,
+ .writew = esdhc_writew,
+ .writeb = esdhc_writeb,
+ .set_clock = esdhc_set_clock,
+ .enable_dma = esdhc_enable_dma,
+ },
+};
+
+#ifdef CONFIG_PM
+
+static int sdhci_of_suspend(struct of_device *ofdev, pm_message_t state)
+{
+ struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
+
+ return mmc_suspend_host(host->mmc, state);
+}
+
+static int sdhci_of_resume(struct of_device *ofdev)
+{
+ struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
+
+ return mmc_resume_host(host->mmc);
+}
+
+#else
+
+#define sdhci_of_suspend NULL
+#define sdhci_of_resume NULL
+
+#endif
+
+static int __devinit sdhci_of_probe(struct of_device *ofdev,
+ const struct of_device_id *match)
+{
+ struct device_node *np = ofdev->node;
+ struct sdhci_of_data *sdhci_of_data = match->data;
+ struct sdhci_host *host;
+ struct resource mem;
+ const u32 *clk;
+ int size;
+ int ret;
+
+ host = sdhci_alloc_host(&ofdev->dev, sizeof(*host));
+ if (!host)
+ return -ENOMEM;
+
+ dev_set_drvdata(&ofdev->dev, host);
+
+ ret = of_address_to_resource(np, 0, &mem);
+ if (ret)
+ goto err_no_addr;
+
+ host->ioaddr = ioremap(mem.start, resource_size(&mem));
+ if (!host->ioaddr) {
+ ret = -ENOMEM;
+ goto err_addr_map;
+ }
+
+ host->irq = irq_of_parse_and_map(np, 0);
+ if (!host->irq) {
+ ret = -EINVAL;
+ goto err_no_irq;
+ }
+
+ host->hw_name = dev_name(&ofdev->dev);
+ if (sdhci_of_data) {
+ host->quirks = sdhci_of_data->quirks;
+ host->ops = &sdhci_of_data->ops;
+ }
+
+ clk = of_get_property(np, "clock-frequency", &size);
+ if (clk && size == sizeof(*clk) && *clk) {
+ host->max_clk = *clk;
+ host->timeout_clk = *clk / 1000;
+ }
+
+ ret = sdhci_add_host(host);
+ if (ret)
+ goto err_add_host;
+
+ return 0;
+
+err_add_host:
+ irq_dispose_mapping(host->irq);
+err_no_irq:
+ iounmap(host->ioaddr);
+err_addr_map:
+err_no_addr:
+ sdhci_free_host(host);
+ return ret;
+}
+
+static int __devexit sdhci_of_remove(struct of_device *ofdev)
+{
+ struct sdhci_host *host = dev_get_drvdata(&ofdev->dev);
+
+ sdhci_remove_host(host, 0);
+ sdhci_free_host(host);
+ irq_dispose_mapping(host->irq);
+ iounmap(host->ioaddr);
+ return 0;
+}
+
+static const struct of_device_id sdhci_of_match[] = {
+ {
+ .compatible = "fsl,esdhc",
+ .data = &sdhci_esdhc,
+ },
+ {
+ .compatible = "generic-sdhci",
+ },
+ {},
+};
+MODULE_DEVICE_TABLE(of, sdhci_of_match);
+
+static struct of_platform_driver sdhci_of_driver = {
+ .driver.name = "sdhci-of",
+ .match_table = sdhci_of_match,
+ .probe = sdhci_of_probe,
+ .remove = __devexit_p(sdhci_of_remove),
+ .suspend = sdhci_of_suspend,
+ .resume = sdhci_of_resume,
+};
+
+static int __init sdhci_of_init(void)
+{
+ return of_register_platform_driver(&sdhci_of_driver);
+}
+module_init(sdhci_of_init);
+
+static void __exit sdhci_of_exit(void)
+{
+ of_unregister_platform_driver(&sdhci_of_driver);
+}
+module_exit(sdhci_of_exit);
+
+MODULE_DESCRIPTION("Secure Digital Host Controller Interface OF driver");
+MODULE_AUTHOR("Xiaobo Xie <[email protected]>, "
+ "Anton Vorontsov <[email protected]>");
+MODULE_LICENSE("GPL");
--
1.5.6.5

2009-01-22 11:57:05

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 02/10] sdhci: Add support for bus-specific IO memory accessors

On Thursday 22 January 2009, Anton Vorontsov wrote:
> +       /*
> +        * These accessors duplicate sdhci_ops, but there are two reasons for
> +        * this:
> +        * 1. sdhci_ops are const, so the sdhci driver won't able to assign
> +        *    default ops;

You could assign the pointer to a const default_sdhci_ops structure,
which IMHO would be cleaner than copying the function pointers
separately.

> +        * 2. Using host->X instead of host->ops->X saves us one dereference.
> +        *    This can be useful in PIO mode. (Though the benefit of this
> +        *    is negligibly small).
> +        */

I doubt that this is even measurable. If it was, you could still use a copy
of that structure, like

struct sdhci_host {
...
struct sdhci_ops ops; /* not struct sdhci_ops *ops */
...
};

and do an assignment of the structure, like

static void assign_ops(struct sdhci_host *host, struct sdhci_ops *ops)
{
host->ops = *ops;
}

Arnd <><

2009-01-22 12:05:47

by Arnd Bergmann

[permalink] [raw]
Subject: Re: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

On Thursday 22 January 2009, Anton Vorontsov wrote:
> This patch adds a new driver: sdhci-of. The driver is similar to
> the sdhci-pci, it contains common probe code, and controller-specific
> ops and quirks.
>
> So far there are only Freescale eSDHC ops and quirks.

Looks very good overall.

Acked-by: Arnd Bergmann <[email protected]>

> + ret = of_address_to_resource(np, 0, &mem);
> + if (ret)
> + goto err_no_addr;
> +
> + host->ioaddr = ioremap(mem.start, resource_size(&mem));
> + if (!host->ioaddr) {
> + ret = -ENOMEM;
> + goto err_addr_map;
> + }

Minor improvement: you could use of_iomap to do this in one step.

Arnd <><

2009-01-22 18:31:26

by Anton Vorontsov

[permalink] [raw]
Subject: Re: [PATCH 02/10] sdhci: Add support for bus-specific IO memory accessors

On Thu, Jan 22, 2009 at 12:55:48PM +0100, Arnd Bergmann wrote:
> On Thursday 22 January 2009, Anton Vorontsov wrote:
> > +       /*
> > +        * These accessors duplicate sdhci_ops, but there are two reasons for
> > +        * this:
> > +        * 1. sdhci_ops are const, so the sdhci driver won't able to assign
> > +        *    default ops;
>
> You could assign the pointer to a const default_sdhci_ops structure,
> which IMHO would be cleaner than copying the function pointers
> separately.

This won't work because ops also specify non-memory accessors ops
(i.e. enable_dma, set_clock, ...), so we can't just assign
default_ops ptr.

> > +        * 2. Using host->X instead of host->ops->X saves us one dereference.
> > +        *    This can be useful in PIO mode. (Though the benefit of this
> > +        *    is negligibly small).
> > +        */
>
> I doubt that this is even measurable.

Yeah, I didn't even bother w/ measuring. ;-)

> If it was, you could still use a copy of that structure, like
>
> struct sdhci_host {
> ...
> struct sdhci_ops ops; /* not struct sdhci_ops *ops */
> ...
> };
>
> and do an assignment of the structure, like
>
> static void assign_ops(struct sdhci_host *host, struct sdhci_ops *ops)
> {
> host->ops = *ops;
> }

This will work. Pierre, what do you think? That way drivers
will call this routine instead of "host->ops = driver_ops_ptr".

The other option would be to remove const from the ops. Or implement
another non-const ops struct (struct sdhci_io_ops?).

In any way, the type-checked variant will be even longer to type:

host->mem_ops->writel(val, host->ioaddr + reg);
host->ops.writel(val, host->ioaddr + reg);
host->writel(val, host->ioaddr + reg);


Thanks,

--
Anton Vorontsov
email: [email protected]
irc://irc.freenode.net/bd2

2009-01-22 19:22:53

by Kumar Gala

[permalink] [raw]
Subject: Re: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver


On Jan 21, 2009, at 8:00 PM, Anton Vorontsov wrote:

> This patch adds a new driver: sdhci-of. The driver is similar to
> the sdhci-pci, it contains common probe code, and controller-specific
> ops and quirks.
>
> So far there are only Freescale eSDHC ops and quirks.
>
> Signed-off-by: Anton Vorontsov <[email protected]>
> ---
> drivers/mmc/host/Kconfig | 10 ++
> drivers/mmc/host/Makefile | 1 +
> drivers/mmc/host/sdhci-of.c | 274 ++++++++++++++++++++++++++++++++++
> +++++++++
> 3 files changed, 285 insertions(+), 0 deletions(-)
> create mode 100644 drivers/mmc/host/sdhci-of.c

Still griping about lack of OF binding docs for this.

:)

- k

2009-01-25 06:42:38

by Matt Sealey

[permalink] [raw]
Subject: Re: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

Anton Vorontsov wrote:
>
> If unsure, say Y.
>
> +config MMC_SDHCI_OF
> + tristate "SDHCI support on OpenFirmware platforms"
> + depends on MMC_SDHCI && PPC_OF
> + help
> + This selects the OF Secure Digital Host Controller Interface.
> + So far there is only Freescale eSDHC controller known to exists
> + on OF platforms.

Nit; would probably be better English as

This selects the OF support for Secure Digital Host Controller
Interfaces. So far, only the Freescale eSDHC controller is known
to exist on OF platforms.

-- Matt Sealey <[email protected]>

2009-01-26 22:27:44

by Warner Losh

[permalink] [raw]
Subject: Re: [PATCH 10/10] mmc: Add OpenFirmware bindings for SDHCI driver

In message: <[email protected]>
Matt Sealey <[email protected]> writes:
: Anton Vorontsov wrote:
: >
: > If unsure, say Y.
: >
: > +config MMC_SDHCI_OF
: > + tristate "SDHCI support on OpenFirmware platforms"
: > + depends on MMC_SDHCI && PPC_OF
: > + help
: > + This selects the OF Secure Digital Host Controller Interface.
: > + So far there is only Freescale eSDHC controller known to exists
: > + on OF platforms.
:
: Nit; would probably be better English as
:
: This selects the OF support for Secure Digital Host Controller
: Interfaces. So far, only the Freescale eSDHC controller is known
: to exist on OF platforms.

Is this the SD Association standard host controller interface, or a
custom one by Freescale? If the former, then I'd suggest:

This selects OF support for the SD Association standard Secure
Digital Host Controller Interface (version x.xx). So far,
only the Freescale eSDHC controller is the only example on OF
platforms.

if the latter:

This selects OF support for Freescale's custom Secure Digital
Host Controller Interface. So far, only the Freescale eSDHC
controller is the only example on OF platforms.

Warner