[PATCH 01/12] mmci: use sg_miter API to fix multi-page sg handling

[PATCH 01/12] mmci: use sg_miter API to fix multi-page sg handling

[Rabin Vincent]

The mmci driver's SG list iteration logic assumes that each SG entry
spans only one page, and only maps and flushes one page of the sg.  This
is not a valid assumption.  Fix it by converting the driver to the
sg_miter API, which correctly handles sgs which span multiple pages.
Cache flushing is handled inside the sg_miter API implementation.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   55 ++++++++++++++++++++++------------------------
 drivers/mmc/host/mmci.h |   35 +-----------------------------
 2 files changed, 27 insertions(+), 63 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4917af9..683516b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -26,7 +26,6 @@
 #include <linux/amba/mmci.h>
 #include <linux/regulator/consumer.h>
 
-#include <asm/cacheflush.h>
 #include <asm/div64.h>
 #include <asm/io.h>
 #include <asm/sizes.h>
@@ -98,6 +97,18 @@ static void mmci_stop_data(struct mmci_host *host)
 	host->data = NULL;
 }
 
+static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
+{
+	unsigned int flags = SG_MITER_ATOMIC;
+
+	if (data->flags & MMC_DATA_READ)
+		flags |= SG_MITER_TO_SG;
+	else
+		flags |= SG_MITER_FROM_SG;
+
+	sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags);
+}
+
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
 	unsigned int datactrl, timeout, irqmask;
@@ -205,13 +216,6 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		else if (status & (MCI_TXUNDERRUN|MCI_RXOVERRUN))
 			data->error = -EIO;
 		status |= MCI_DATAEND;
-
-		/*
-		 * We hit an error condition.  Ensure that any data
-		 * partially written to a page is properly coherent.
-		 */
-		if (host->sg_len && data->flags & MMC_DATA_READ)
-			flush_dcache_page(sg_page(host->sg_ptr));
 	}
 	if (status & MCI_DATAEND) {
 		mmci_stop_data(host);
@@ -314,15 +318,18 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem
 static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 {
 	struct mmci_host *host = dev_id;
+	struct sg_mapping_iter *sg_miter = &host->sg_miter;
 	void __iomem *base = host->base;
+	unsigned long flags;
 	u32 status;
 
 	status = readl(base + MMCISTATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "irq1 (pio) %08x\n", status);
 
+	local_irq_save(flags);
+
 	do {
-		unsigned long flags;
 		unsigned int remain, len;
 		char *buffer;
 
@@ -336,11 +343,11 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (!(status & (MCI_TXFIFOHALFEMPTY|MCI_RXDATAAVLBL)))
 			break;
 
-		/*
-		 * Map the current scatter buffer.
-		 */
-		buffer = mmci_kmap_atomic(host, &flags) + host->sg_off;
-		remain = host->sg_ptr->length - host->sg_off;
+		if (!sg_miter_next(sg_miter))
+			break;
+
+		buffer = sg_miter->addr;
+		remain = sg_miter->length;
 
 		len = 0;
 		if (status & MCI_RXACTIVE)
@@ -348,31 +355,21 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (status & MCI_TXACTIVE)
 			len = mmci_pio_write(host, buffer, remain, status);
 
-		/*
-		 * Unmap the buffer.
-		 */
-		mmci_kunmap_atomic(host, buffer, &flags);
+		sg_miter->consumed = len;
 
-		host->sg_off += len;
 		host->size -= len;
 		remain -= len;
 
 		if (remain)
 			break;
 
-		/*
-		 * If we were reading, and we have completed this
-		 * page, ensure that the data cache is coherent.
-		 */
-		if (status & MCI_RXACTIVE)
-			flush_dcache_page(sg_page(host->sg_ptr));
-
-		if (!mmci_next_sg(host))
-			break;
-
 		status = readl(base + MMCISTATUS);
 	} while (1);
 
+	sg_miter_stop(sg_miter);
+
+	local_irq_restore(flags);
+
 	/*
 	 * If we're nearing the end of the read, switch to
 	 * "any data available" mode.
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d77062e..7cb24ab 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -171,42 +171,9 @@ struct mmci_host {
 	struct timer_list	timer;
 	unsigned int		oldstat;
 
-	unsigned int		sg_len;
-
 	/* pio stuff */
-	struct scatterlist	*sg_ptr;
-	unsigned int		sg_off;
+	struct sg_mapping_iter	sg_miter;
 	unsigned int		size;
 	struct regulator	*vcc;
 };
 
-static inline void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
-{
-	/*
-	 * Ideally, we want the higher levels to pass us a scatter list.
-	 */
-	host->sg_len = data->sg_len;
-	host->sg_ptr = data->sg;
-	host->sg_off = 0;
-}
-
-static inline int mmci_next_sg(struct mmci_host *host)
-{
-	host->sg_ptr++;
-	host->sg_off = 0;
-	return --host->sg_len;
-}
-
-static inline char *mmci_kmap_atomic(struct mmci_host *host, unsigned long *flags)
-{
-	struct scatterlist *sg = host->sg_ptr;
-
-	local_irq_save(*flags);
-	return kmap_atomic(sg_page(sg), KM_BIO_SRC_IRQ) + sg->offset;
-}
-
-static inline void mmci_kunmap_atomic(struct mmci_host *host, void *buffer, unsigned long *flags)
-{
-	kunmap_atomic(buffer, KM_BIO_SRC_IRQ);
-	local_irq_restore(*flags);
-}
-- 
1.7.0

[PATCH 01/12] mmci: use sg_miter API to fix multi-page sg handling

[Rabin Vincent]

The mmci driver's SG list iteration logic assumes that each SG entry
spans only one page, and only maps and flushes one page of the sg.  This
is not a valid assumption.  Fix it by converting the driver to the
sg_miter API, which correctly handles sgs which span multiple pages.
Cache flushing is handled inside the sg_miter API implementation.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   55 ++++++++++++++++++++++------------------------
 drivers/mmc/host/mmci.h |   35 +-----------------------------
 2 files changed, 27 insertions(+), 63 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4917af9..683516b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -26,7 +26,6 @@
 #include <linux/amba/mmci.h>
 #include <linux/regulator/consumer.h>
 
-#include <asm/cacheflush.h>
 #include <asm/div64.h>
 #include <asm/io.h>
 #include <asm/sizes.h>
@@ -98,6 +97,18 @@ static void mmci_stop_data(struct mmci_host *host)
 	host->data = NULL;
 }
 
+static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
+{
+	unsigned int flags = SG_MITER_ATOMIC;
+
+	if (data->flags & MMC_DATA_READ)
+		flags |= SG_MITER_TO_SG;
+	else
+		flags |= SG_MITER_FROM_SG;
+
+	sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags);
+}
+
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
 	unsigned int datactrl, timeout, irqmask;
@@ -205,13 +216,6 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		else if (status & (MCI_TXUNDERRUN|MCI_RXOVERRUN))
 			data->error = -EIO;
 		status |= MCI_DATAEND;
-
-		/*
-		 * We hit an error condition.  Ensure that any data
-		 * partially written to a page is properly coherent.
-		 */
-		if (host->sg_len && data->flags & MMC_DATA_READ)
-			flush_dcache_page(sg_page(host->sg_ptr));
 	}
 	if (status & MCI_DATAEND) {
 		mmci_stop_data(host);
@@ -314,15 +318,18 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem
 static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 {
 	struct mmci_host *host = dev_id;
+	struct sg_mapping_iter *sg_miter = &host->sg_miter;
 	void __iomem *base = host->base;
+	unsigned long flags;
 	u32 status;
 
 	status = readl(base + MMCISTATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "irq1 (pio) %08x\n", status);
 
+	local_irq_save(flags);
+
 	do {
-		unsigned long flags;
 		unsigned int remain, len;
 		char *buffer;
 
@@ -336,11 +343,11 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (!(status & (MCI_TXFIFOHALFEMPTY|MCI_RXDATAAVLBL)))
 			break;
 
-		/*
-		 * Map the current scatter buffer.
-		 */
-		buffer = mmci_kmap_atomic(host, &flags) + host->sg_off;
-		remain = host->sg_ptr->length - host->sg_off;
+		if (!sg_miter_next(sg_miter))
+			break;
+
+		buffer = sg_miter->addr;
+		remain = sg_miter->length;
 
 		len = 0;
 		if (status & MCI_RXACTIVE)
@@ -348,31 +355,21 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (status & MCI_TXACTIVE)
 			len = mmci_pio_write(host, buffer, remain, status);
 
-		/*
-		 * Unmap the buffer.
-		 */
-		mmci_kunmap_atomic(host, buffer, &flags);
+		sg_miter->consumed = len;
 
-		host->sg_off += len;
 		host->size -= len;
 		remain -= len;
 
 		if (remain)
 			break;
 
-		/*
-		 * If we were reading, and we have completed this
-		 * page, ensure that the data cache is coherent.
-		 */
-		if (status & MCI_RXACTIVE)
-			flush_dcache_page(sg_page(host->sg_ptr));
-
-		if (!mmci_next_sg(host))
-			break;
-
 		status = readl(base + MMCISTATUS);
 	} while (1);
 
+	sg_miter_stop(sg_miter);
+
+	local_irq_restore(flags);
+
 	/*
 	 * If we're nearing the end of the read, switch to
 	 * "any data available" mode.
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d77062e..7cb24ab 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -171,42 +171,9 @@ struct mmci_host {
 	struct timer_list	timer;
 	unsigned int		oldstat;
 
-	unsigned int		sg_len;
-
 	/* pio stuff */
-	struct scatterlist	*sg_ptr;
-	unsigned int		sg_off;
+	struct sg_mapping_iter	sg_miter;
 	unsigned int		size;
 	struct regulator	*vcc;
 };
 
-static inline void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
-{
-	/*
-	 * Ideally, we want the higher levels to pass us a scatter list.
-	 */
-	host->sg_len = data->sg_len;
-	host->sg_ptr = data->sg;
-	host->sg_off = 0;
-}
-
-static inline int mmci_next_sg(struct mmci_host *host)
-{
-	host->sg_ptr++;
-	host->sg_off = 0;
-	return --host->sg_len;
-}
-
-static inline char *mmci_kmap_atomic(struct mmci_host *host, unsigned long *flags)
-{
-	struct scatterlist *sg = host->sg_ptr;
-
-	local_irq_save(*flags);
-	return kmap_atomic(sg_page(sg), KM_BIO_SRC_IRQ) + sg->offset;
-}
-
-static inline void mmci_kunmap_atomic(struct mmci_host *host, void *buffer, unsigned long *flags)
-{
-	kunmap_atomic(buffer, KM_BIO_SRC_IRQ);
-	local_irq_restore(*flags);
-}
-- 
1.7.0

[PATCH 02/12] mmci: fix multi block transfers

[Rabin Vincent]

Fix the data transfer size to allow multi block transfers to work.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 683516b..84d782b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -120,7 +120,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 		data->blksz, data->blocks, data->flags);
 
 	host->data = data;
-	host->size = data->blksz;
+	host->size = data->blksz * data->blocks;
 	host->data_xfered = 0;
 
 	mmci_init_sg(host, data);
-- 
1.7.0

[PATCH 02/12] mmci: fix multi block transfers

[Rabin Vincent]

Fix the data transfer size to allow multi block transfers to work.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 683516b..84d782b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -120,7 +120,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 		data->blksz, data->blocks, data->flags);
 
 	host->data = data;
-	host->size = data->blksz;
+	host->size = data->blksz * data->blocks;
 	host->data_xfered = 0;
 
 	mmci_init_sg(host, data);
-- 
1.7.0

Re: [PATCH 02/12] mmci: fix multi block transfers

[Russell King - ARM Linux]

On Tue, Jun 22, 2010 at 02:47:37PM +0530, Rabin Vincent wrote:
> Fix the data transfer size to allow multi block transfers to work.
> 
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>

The first two patches improve performance on Versatile, from 20.5s to
read 1MB down to 18.5s - a 10% improvement.

I was worried that the first patch would add too much overhead, and
cause the hardware to overrun - as the ARM version of these primecells
does not have the capability to stop the clock when the FIFO fills
up, any increase in IRQ latency to read the FIFO will result in an
overrun.

So this is good news.  I've applied the first three patches in this
series, but we do need to sort out the card detect situation properly
before continuing.

[PATCH 02/12] mmci: fix multi block transfers

[Russell King - ARM Linux]

On Tue, Jun 22, 2010 at 02:47:37PM +0530, Rabin Vincent wrote:
> Fix the data transfer size to allow multi block transfers to work.
> 
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>

The first two patches improve performance on Versatile, from 20.5s to
read 1MB down to 18.5s - a 10% improvement.

I was worried that the first patch would add too much overhead, and
cause the hardware to overrun - as the ARM version of these primecells
does not have the capability to stop the clock when the FIFO fills
up, any increase in IRQ latency to read the FIFO will result in an
overrun.

So this is good news.  I've applied the first three patches in this
series, but we do need to sort out the card detect situation properly
before continuing.

[PATCH 02/12] mmci: fix multi block transfers

[Colin Tuckley]

> -----Original Message-----
> From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm-
> kernel-bounces at lists.infradead.org] On Behalf Of Russell King - ARM

> So this is good news.  I've applied the first three patches in this
> series, but we do need to sort out the card detect situation properly
> before continuing.

On the subject of card detection. I recently noticed that although it works
on the PB926 it doesn't on the AB926 despite it being the same kernel
binary. On the AB926 the detection is doing something when a card is
inserted but the device never appears.

Colin

--
Colin Tuckley - ARM Ltd.
110 Fulbourn Rd
Cambridge, CB1 9NJ
Tel: +44 1223 400536

[PATCH 02/12] mmci: fix multi block transfers

[Russell King - ARM Linux]

On Thu, Jul 29, 2010 at 02:31:48PM +0100, Colin Tuckley wrote:
> > -----Original Message-----
> > From: linux-arm-kernel-bounces at lists.infradead.org [mailto:linux-arm-
> > kernel-bounces at lists.infradead.org] On Behalf Of Russell King - ARM
> 
> > So this is good news.  I've applied the first three patches in this
> > series, but we do need to sort out the card detect situation properly
> > before continuing.
> 
> On the subject of card detection. I recently noticed that although it works
> on the PB926 it doesn't on the AB926 despite it being the same kernel
> binary.

No, it doesn't work on Versatile PB926 in mainline (I tried it this morning)
because the card detection (via the ->status callback) is inverted - just
like it was for realview.  What happens is you get commands issued to the
card on card removal, and nothing on card insertion.

What I'm proposing is that we get rid of these multiple levels of negation
that we're gaining.  The negations only add additional complexity and
confusion over what's the right thing to return at any given point.  Lets
stick to one positive logic method throughout the code - 0 means no card,
!0 means card inserted.

That means your original patch needs to be reverted, and we need to change
the 'return !status;' to just 'return status;' in mmci.c's get_cd function
as per my patch.

However, I think this will make GPIO on Realview wrong - iirc, the card
detection bits from PL061 are inverted.

[PATCH 02/12] mmci: fix multi block transfers

[Colin Tuckley]

> -----Original Message-----
> From: Russell King - ARM Linux [mailto:linux at arm.linux.org.uk]

> No, it doesn't work on Versatile PB926 in mainline (I tried it this
> morning)
> because the card detection (via the ->status callback) is inverted -
> just
> like it was for realview.  What happens is you get commands issued to
> the
> card on card removal, and nothing on card insertion.

Sorry, I wasn't clear - I was talking about the current ARM tree 2.6.33-arm1
where Catalin and I fixed Realview and Versatile so that it worked with
GPIOLIB where the detection was done with GPIO and to revert to the old
method on boards where the detect pins are not available as gpio.

> What I'm proposing is that we get rid of these multiple levels of
> negation
> that we're gaining.  The negations only add additional complexity and
> confusion over what's the right thing to return at any given point.

Ack.

> That means your original patch needs to be reverted, and we need to
> change
> the 'return !status;' to just 'return status;' in mmci.c's get_cd
> function
> as per my patch.

More massaged than reverted I think, but Ack.

> However, I think this will make GPIO on Realview wrong - iirc, the card
> detection bits from PL061 are inverted.

See above.

Colin

[PATCH 02/12] mmci: fix multi block transfers

[Russell King - ARM Linux]

On Thu, Jul 29, 2010 at 02:42:39PM +0100, Colin Tuckley wrote:
> > However, I think this will make GPIO on Realview wrong - iirc, the card
> > detection bits from PL061 are inverted.
> 
> See above.

I've no idea what the right thing on Realview is because the documentation
is too poor in this area.  Although the Realview EB user manual gives you
the GPIO block signals, it doesn't say what sense they are.

It looks to me from the hardware description diagrams, that the GPIO block
is fed with an inverted signal from the socket card detect pin - which
itself is active low.  That means the GPIO signal is in a positive logic
state.

So I don't think that the "return !status;" is right even for the GPIO
case.

[PATCH 02/12] mmci: fix multi block transfers

[Russell King - ARM Linux]

On Thu, Jul 29, 2010 at 02:55:11PM +0100, Russell King - ARM Linux wrote:
> On Thu, Jul 29, 2010 at 02:42:39PM +0100, Colin Tuckley wrote:
> > > However, I think this will make GPIO on Realview wrong - iirc, the card
> > > detection bits from PL061 are inverted.
> > 
> > See above.
> 
> I've no idea what the right thing on Realview is because the documentation
> is too poor in this area.  Although the Realview EB user manual gives you
> the GPIO block signals, it doesn't say what sense they are.
> 
> It looks to me from the hardware description diagrams, that the GPIO block
> is fed with an inverted signal from the socket card detect pin - which
> itself is active low.  That means the GPIO signal is in a positive logic
> state.
> 
> So I don't think that the "return !status;" is right even for the GPIO
> case.

Hmm.  Firstly, Versatile doesn't support reading the card detect via GPIO.

Secondly, my Realview hardware appears to have the GPIO card detect status
inverted - zero means card inserted, non-zero means card removed.  That's
contary to what the documentation suggests.

Confused.

[PATCH 03/12] mmci: let core poll for card detection

[Rabin Vincent]

Use the MMC core's ability to poll for card detection.  This also has
the advantage of doing the gpio_get_value from a workqueue instead of
timer, allowing the gpio to be on a sleeping gpiochip.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   22 +---------------------
 1 files changed, 1 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 84d782b..65f0e12 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -548,18 +548,6 @@ static const struct mmc_host_ops mmci_ops = {
 	.get_cd		= mmci_get_cd,
 };
 
-static void mmci_check_status(unsigned long data)
-{
-	struct mmci_host *host = (struct mmci_host *)data;
-	unsigned int status = mmci_get_cd(host->mmc);
-
-	if (status ^ host->oldstat)
-		mmc_detect_change(host->mmc, 0);
-
-	host->oldstat = status;
-	mod_timer(&host->timer, jiffies + HZ);
-}
-
 static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 {
 	struct mmci_platform_data *plat = dev->dev.platform_data;
@@ -666,6 +654,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (host->vcc == NULL)
 		mmc->ocr_avail = plat->ocr_mask;
 	mmc->caps = plat->capabilities;
+	mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	/*
 	 * We can do SGIO
@@ -731,7 +720,6 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
 
 	amba_set_drvdata(dev, mmc);
-	host->oldstat = mmci_get_cd(host->mmc);
 
 	mmc_add_host(mmc);
 
@@ -739,12 +727,6 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 		mmc_hostname(mmc), amba_rev(dev), amba_config(dev),
 		(unsigned long long)dev->res.start, dev->irq[0], dev->irq[1]);
 
-	init_timer(&host->timer);
-	host->timer.data = (unsigned long)host;
-	host->timer.function = mmci_check_status;
-	host->timer.expires = jiffies + HZ;
-	add_timer(&host->timer);
-
 	return 0;
 
  irq0_free:
@@ -778,8 +760,6 @@ static int __devexit mmci_remove(struct amba_device *dev)
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
 
-		del_timer_sync(&host->timer);
-
 		mmc_remove_host(mmc);
 
 		writel(0, host->base + MMCIMASK0);
-- 
1.7.0

[PATCH 03/12] mmci: let core poll for card detection

[Rabin Vincent]

Use the MMC core's ability to poll for card detection.  This also has
the advantage of doing the gpio_get_value from a workqueue instead of
timer, allowing the gpio to be on a sleeping gpiochip.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   22 +---------------------
 1 files changed, 1 insertions(+), 21 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 84d782b..65f0e12 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -548,18 +548,6 @@ static const struct mmc_host_ops mmci_ops = {
 	.get_cd		= mmci_get_cd,
 };
 
-static void mmci_check_status(unsigned long data)
-{
-	struct mmci_host *host = (struct mmci_host *)data;
-	unsigned int status = mmci_get_cd(host->mmc);
-
-	if (status ^ host->oldstat)
-		mmc_detect_change(host->mmc, 0);
-
-	host->oldstat = status;
-	mod_timer(&host->timer, jiffies + HZ);
-}
-
 static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 {
 	struct mmci_platform_data *plat = dev->dev.platform_data;
@@ -666,6 +654,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (host->vcc == NULL)
 		mmc->ocr_avail = plat->ocr_mask;
 	mmc->caps = plat->capabilities;
+	mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	/*
 	 * We can do SGIO
@@ -731,7 +720,6 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
 
 	amba_set_drvdata(dev, mmc);
-	host->oldstat = mmci_get_cd(host->mmc);
 
 	mmc_add_host(mmc);
 
@@ -739,12 +727,6 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 		mmc_hostname(mmc), amba_rev(dev), amba_config(dev),
 		(unsigned long long)dev->res.start, dev->irq[0], dev->irq[1]);
 
-	init_timer(&host->timer);
-	host->timer.data = (unsigned long)host;
-	host->timer.function = mmci_check_status;
-	host->timer.expires = jiffies + HZ;
-	add_timer(&host->timer);
-
 	return 0;
 
  irq0_free:
@@ -778,8 +760,6 @@ static int __devexit mmci_remove(struct amba_device *dev)
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
 
-		del_timer_sync(&host->timer);
-
 		mmc_remove_host(mmc);
 
 		writel(0, host->base + MMCIMASK0);
-- 
1.7.0

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Rabin Vincent]

On some platforms, the GPIO value from the gpio_cd pin doesn't need to
be inverted to get it active high.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c   |    2 +-
 include/linux/amba/mmci.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 65f0e12..7e56726 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -538,7 +538,7 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	else
 		status = gpio_get_value(host->gpio_cd);
 
-	return !status;
+	return !status ^ host->plat->cd_noinvert;
 }
 
 static const struct mmc_host_ops mmci_ops = {
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index 7e466fe..f9d1bb5 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -23,6 +23,7 @@
  * whether a card is present in the MMC slot or not
  * @gpio_wp: read this GPIO pin to see if the card is write protected
  * @gpio_cd: read this GPIO pin to detect card insertion
+ * @cd_noinvert: true if the gpio_cd pin value or status are active high
  * @capabilities: the capabilities of the block as implemented in
  * this platform, signify anything MMC_CAP_* from mmc/host.h
  */
@@ -33,6 +34,7 @@ struct mmci_platform_data {
 	unsigned int (*status)(struct device *);
 	int	gpio_wp;
 	int	gpio_cd;
+	bool	cd_noinvert;
 	unsigned long capabilities;
 };
 
-- 
1.7.0

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Rabin Vincent]

On some platforms, the GPIO value from the gpio_cd pin doesn't need to
be inverted to get it active high.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c   |    2 +-
 include/linux/amba/mmci.h |    2 ++
 2 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 65f0e12..7e56726 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -538,7 +538,7 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	else
 		status = gpio_get_value(host->gpio_cd);
 
-	return !status;
+	return !status ^ host->plat->cd_noinvert;
 }
 
 static const struct mmc_host_ops mmci_ops = {
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index 7e466fe..f9d1bb5 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -23,6 +23,7 @@
  * whether a card is present in the MMC slot or not
  * @gpio_wp: read this GPIO pin to see if the card is write protected
  * @gpio_cd: read this GPIO pin to detect card insertion
+ * @cd_noinvert: true if the gpio_cd pin value or status are active high
  * @capabilities: the capabilities of the block as implemented in
  * this platform, signify anything MMC_CAP_* from mmc/host.h
  */
@@ -33,6 +34,7 @@ struct mmci_platform_data {
 	unsigned int (*status)(struct device *);
 	int	gpio_wp;
 	int	gpio_cd;
+	bool	cd_noinvert;
 	unsigned long capabilities;
 };
 
-- 
1.7.0

Re: [PATCH 04/12] mmci: allow the card detect status not to be inverted

[Russell King - ARM Linux]

On Tue, Jun 22, 2010 at 02:47:39PM +0530, Rabin Vincent wrote:
> On some platforms, the GPIO value from the gpio_cd pin doesn't need to
> be inverted to get it active high.

Argh.

Actually, I think the inversion of the card status is a bug - certainly
for Versatile it is.  It was on Realview as well before b56ba8a changed
the sense for Realview - so it seems b56ba8a was the wrong fix.

So, this patch restores the card detect functionality across the ARM
devel platforms to what it was prior to the addition of GPIOLIB support.

What now needs to happen is the sense of the GPIOLIB support on platforms
needs to be checked - eg, I suspect the card detect via the GPIO primecell
is inverted on Realview - in which case we need a 'cd_invert' rather
than the negative 'cd_noinvert'.

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index d8179ea..f9dc99c 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -243,7 +243,7 @@ static unsigned int realview_mmc_status(struct device *dev)
 		 * way to do this on the PB1176.
 		 */
 		inserted = !inserted;
-		return inserted ? 0 : 1;
+		return inserted ? 1 : 0;
 	}
 
 	if (adev->res.start == REALVIEW_MMCI0_BASE)
@@ -251,7 +251,7 @@ static unsigned int realview_mmc_status(struct device *dev)
 	else
 		mask = 2;
 
-	return !(readl(REALVIEW_SYSMCI) & mask);
+	return readl(REALVIEW_SYSMCI) & mask;
 }
 
 struct mmci_platform_data realview_mmc0_plat_data = {
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 9bf9194..02bf365 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -557,7 +557,7 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	else
 		status = gpio_get_value(host->gpio_cd);
 
-	return !status;
+	return status;
 }
 
 static const struct mmc_host_ops mmci_ops = {

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Russell King - ARM Linux]

On Tue, Jun 22, 2010 at 02:47:39PM +0530, Rabin Vincent wrote:
> On some platforms, the GPIO value from the gpio_cd pin doesn't need to
> be inverted to get it active high.

Argh.

Actually, I think the inversion of the card status is a bug - certainly
for Versatile it is.  It was on Realview as well before b56ba8a changed
the sense for Realview - so it seems b56ba8a was the wrong fix.

So, this patch restores the card detect functionality across the ARM
devel platforms to what it was prior to the addition of GPIOLIB support.

What now needs to happen is the sense of the GPIOLIB support on platforms
needs to be checked - eg, I suspect the card detect via the GPIO primecell
is inverted on Realview - in which case we need a 'cd_invert' rather
than the negative 'cd_noinvert'.

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index d8179ea..f9dc99c 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -243,7 +243,7 @@ static unsigned int realview_mmc_status(struct device *dev)
 		 * way to do this on the PB1176.
 		 */
 		inserted = !inserted;
-		return inserted ? 0 : 1;
+		return inserted ? 1 : 0;
 	}
 
 	if (adev->res.start == REALVIEW_MMCI0_BASE)
@@ -251,7 +251,7 @@ static unsigned int realview_mmc_status(struct device *dev)
 	else
 		mask = 2;
 
-	return !(readl(REALVIEW_SYSMCI) & mask);
+	return readl(REALVIEW_SYSMCI) & mask;
 }
 
 struct mmci_platform_data realview_mmc0_plat_data = {
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 9bf9194..02bf365 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -557,7 +557,7 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	else
 		status = gpio_get_value(host->gpio_cd);
 
-	return !status;
+	return status;
 }
 
 static const struct mmc_host_ops mmci_ops = {

Re: [PATCH 04/12] mmci: allow the card detect status not to be inverted

[Linus Walleij]

2010/7/29 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> What now needs to happen is the sense of the GPIOLIB support on platforms
> needs to be checked - eg, I suspect the card detect via the GPIO primecell
> is inverted on Realview - in which case we need a 'cd_invert' rather
> than the negative 'cd_noinvert'.

Doesn't seem so. I applied this patch and it works like a charm just
like before on PB1176. (PL061 is compiled in, so it's not that other
weird stuff being used.)

I hope I'm doing the right thing, boot with card inserted, this appears
in the bootlog:

mmc0: new SD card at address 80ca
mmcblk0: mmc0:80ca SU512 483 MiB
 mmcblk0: p1

Remove card:
mmc0: card 80ca removed

Insert card:
mmc0: new SD card at address 80ca
mmcblk0: mmc0:80ca SU512 483 MiB
 mmcblk0: p1

And so on.

Yours,
Linus Walleij

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Linus Walleij]

2010/7/29 Russell King - ARM Linux <linux@arm.linux.org.uk>:

> What now needs to happen is the sense of the GPIOLIB support on platforms
> needs to be checked - eg, I suspect the card detect via the GPIO primecell
> is inverted on Realview - in which case we need a 'cd_invert' rather
> than the negative 'cd_noinvert'.

Doesn't seem so. I applied this patch and it works like a charm just
like before on PB1176. (PL061 is compiled in, so it's not that other
weird stuff being used.)

I hope I'm doing the right thing, boot with card inserted, this appears
in the bootlog:

mmc0: new SD card at address 80ca
mmcblk0: mmc0:80ca SU512 483 MiB
 mmcblk0: p1

Remove card:
mmc0: card 80ca removed

Insert card:
mmc0: new SD card at address 80ca
mmcblk0: mmc0:80ca SU512 483 MiB
 mmcblk0: p1

And so on.

Yours,
Linus Walleij

Re: [PATCH 04/12] mmci: allow the card detect status not to be inverted

[Russell King - ARM Linux]

On Thu, Jul 29, 2010 at 03:53:19PM +0200, Linus Walleij wrote:
> 2010/7/29 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> 
> > What now needs to happen is the sense of the GPIOLIB support on platforms
> > needs to be checked - eg, I suspect the card detect via the GPIO primecell
> > is inverted on Realview - in which case we need a 'cd_invert' rather
> > than the negative 'cd_noinvert'.
> 
> Doesn't seem so. I applied this patch and it works like a charm just
> like before on PB1176. (PL061 is compiled in, so it's not that other
> weird stuff being used.)

Well, as I said in the other thread which seems to be dealing with this,
I'm completely confused (as seems to be the case with this) as to what
sense the various card detect signals are on ARMs various hardware
platforms.

It seems to be a totally undocumented - and I've no idea whether the
behaviour I see on my platforms is what other people see (from some
suggestions in these threads, my platforms behave differently.)

So, for now I'm going to go with this patch, which fixes it across the
board for the non-GPIO cases.

When the right sense for the GPIO case can be _properly_ determined,
we can see about fixing that.  What I don't want to get into is a "it
works this way on my realview revision X platform, but not revision Y."
If that's the case, we might as well stick with reading the status from
the reliable MCI register which always uses positive logic.

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index 595be19..02e9fde 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -237,7 +237,7 @@ static unsigned int realview_mmc_status(struct device *dev)
 	else
 		mask = 2;
 
-	return !(readl(REALVIEW_SYSMCI) & mask);
+	return readl(REALVIEW_SYSMCI) & mask;
 }
 
 struct mmci_platform_data realview_mmc0_plat_data = {
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index d250711..c842397 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -241,7 +241,7 @@ static struct platform_device v2m_flash_device = {
 
 static unsigned int v2m_mmci_status(struct device *dev)
 {
-	return !(readl(MMIO_P2V(V2M_SYS_MCI)) & (1 << 0));
+	return readl(MMIO_P2V(V2M_SYS_MCI)) & (1 << 0);
 }
 
 static struct mmci_platform_data v2m_mmci_data = {
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4917af9..8ca38d9 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -541,7 +541,11 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	else
 		status = gpio_get_value(host->gpio_cd);
 
-	return !status;
+	/*
+	 * Use positive logic throughout - status is zero for no card,
+	 * non-zero for card inserted.
+	 */
+	return status;
 }
 
 static const struct mmc_host_ops mmci_ops = {

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Russell King - ARM Linux]

On Thu, Jul 29, 2010 at 03:53:19PM +0200, Linus Walleij wrote:
> 2010/7/29 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> 
> > What now needs to happen is the sense of the GPIOLIB support on platforms
> > needs to be checked - eg, I suspect the card detect via the GPIO primecell
> > is inverted on Realview - in which case we need a 'cd_invert' rather
> > than the negative 'cd_noinvert'.
> 
> Doesn't seem so. I applied this patch and it works like a charm just
> like before on PB1176. (PL061 is compiled in, so it's not that other
> weird stuff being used.)

Well, as I said in the other thread which seems to be dealing with this,
I'm completely confused (as seems to be the case with this) as to what
sense the various card detect signals are on ARMs various hardware
platforms.

It seems to be a totally undocumented - and I've no idea whether the
behaviour I see on my platforms is what other people see (from some
suggestions in these threads, my platforms behave differently.)

So, for now I'm going to go with this patch, which fixes it across the
board for the non-GPIO cases.

When the right sense for the GPIO case can be _properly_ determined,
we can see about fixing that.  What I don't want to get into is a "it
works this way on my realview revision X platform, but not revision Y."
If that's the case, we might as well stick with reading the status from
the reliable MCI register which always uses positive logic.

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index 595be19..02e9fde 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -237,7 +237,7 @@ static unsigned int realview_mmc_status(struct device *dev)
 	else
 		mask = 2;
 
-	return !(readl(REALVIEW_SYSMCI) & mask);
+	return readl(REALVIEW_SYSMCI) & mask;
 }
 
 struct mmci_platform_data realview_mmc0_plat_data = {
diff --git a/arch/arm/mach-vexpress/v2m.c b/arch/arm/mach-vexpress/v2m.c
index d250711..c842397 100644
--- a/arch/arm/mach-vexpress/v2m.c
+++ b/arch/arm/mach-vexpress/v2m.c
@@ -241,7 +241,7 @@ static struct platform_device v2m_flash_device = {
 
 static unsigned int v2m_mmci_status(struct device *dev)
 {
-	return !(readl(MMIO_P2V(V2M_SYS_MCI)) & (1 << 0));
+	return readl(MMIO_P2V(V2M_SYS_MCI)) & (1 << 0);
 }
 
 static struct mmci_platform_data v2m_mmci_data = {
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4917af9..8ca38d9 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -541,7 +541,11 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	else
 		status = gpio_get_value(host->gpio_cd);
 
-	return !status;
+	/*
+	 * Use positive logic throughout - status is zero for no card,
+	 * non-zero for card inserted.
+	 */
+	return status;
 }
 
 static const struct mmc_host_ops mmci_ops = {

Re: [PATCH 04/12] mmci: allow the card detect status not to be inverted

[Rabin VINCENT]

On Thu, Jul 29, 2010 at 16:20:11 +0200, Russell King - ARM Linux wrote:
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 4917af9..8ca38d9 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -541,7 +541,11 @@ static int mmci_get_cd(struct mmc_host *mmc)
>  	else
>  		status = gpio_get_value(host->gpio_cd);
>  
> -	return !status;
> +	/*
> +	 * Use positive logic throughout - status is zero for no card,
> +	 * non-zero for card inserted.
> +	 */
> +	return status;
>  }

Your patch in -next has a !gpio_get_value, so are you OK with the
cd_noinvert addition (for the GPIO case)?  Or a cd_invert and a patch to
existing platforms?

Rabin

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Rabin VINCENT]

On Thu, Jul 29, 2010 at 16:20:11 +0200, Russell King - ARM Linux wrote:
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 4917af9..8ca38d9 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -541,7 +541,11 @@ static int mmci_get_cd(struct mmc_host *mmc)
>  	else
>  		status = gpio_get_value(host->gpio_cd);
>  
> -	return !status;
> +	/*
> +	 * Use positive logic throughout - status is zero for no card,
> +	 * non-zero for card inserted.
> +	 */
> +	return status;
>  }

Your patch in -next has a !gpio_get_value, so are you OK with the
cd_noinvert addition (for the GPIO case)?  Or a cd_invert and a patch to
existing platforms?

Rabin

Re: [PATCH 04/12] mmci: allow the card detect status not to be inverted

[Russell King - ARM Linux]

On Thu, Aug 05, 2010 at 11:44:54AM +0530, Rabin VINCENT wrote:
> On Thu, Jul 29, 2010 at 16:20:11 +0200, Russell King - ARM Linux wrote:
> > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> > index 4917af9..8ca38d9 100644
> > --- a/drivers/mmc/host/mmci.c
> > +++ b/drivers/mmc/host/mmci.c
> > @@ -541,7 +541,11 @@ static int mmci_get_cd(struct mmc_host *mmc)
> >  	else
> >  		status = gpio_get_value(host->gpio_cd);
> >  
> > -	return !status;
> > +	/*
> > +	 * Use positive logic throughout - status is zero for no card,
> > +	 * non-zero for card inserted.
> > +	 */
> > +	return status;
> >  }
> 
> Your patch in -next has a !gpio_get_value,

Yes, because I at least need it for it to work on my Realview platform.
As no one else seemed interested in replying to my questions about it,
I decided that I'd fix it so at least what I had continued to work.

> so are you OK with the cd_noinvert addition (for the GPIO case)?
> Or a cd_invert and a patch to existing platforms?

I'd much prefer positive logic.  Double negatives (eg, not noinverted)
are always bad news.

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Russell King - ARM Linux]

On Thu, Aug 05, 2010 at 11:44:54AM +0530, Rabin VINCENT wrote:
> On Thu, Jul 29, 2010 at 16:20:11 +0200, Russell King - ARM Linux wrote:
> > diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> > index 4917af9..8ca38d9 100644
> > --- a/drivers/mmc/host/mmci.c
> > +++ b/drivers/mmc/host/mmci.c
> > @@ -541,7 +541,11 @@ static int mmci_get_cd(struct mmc_host *mmc)
> >  	else
> >  		status = gpio_get_value(host->gpio_cd);
> >  
> > -	return !status;
> > +	/*
> > +	 * Use positive logic throughout - status is zero for no card,
> > +	 * non-zero for card inserted.
> > +	 */
> > +	return status;
> >  }
> 
> Your patch in -next has a !gpio_get_value,

Yes, because I at least need it for it to work on my Realview platform.
As no one else seemed interested in replying to my questions about it,
I decided that I'd fix it so at least what I had continued to work.

> so are you OK with the cd_noinvert addition (for the GPIO case)?
> Or a cd_invert and a patch to existing platforms?

I'd much prefer positive logic.  Double negatives (eg, not noinverted)
are always bad news.

Re: [PATCH 04/12] mmci: allow the card detect status not to be inverted

[Rabin VINCENT]

On Thu, Aug 05, 2010 at 11:25:31 +0200, Russell King - ARM Linux wrote:
> On Thu, Aug 05, 2010 at 11:44:54AM +0530, Rabin VINCENT wrote:
> > so are you OK with the cd_noinvert addition (for the GPIO case)?
> > Or a cd_invert and a patch to existing platforms?
> 
> I'd much prefer positive logic.  Double negatives (eg, not noinverted)
> are always bad news.

Here's the new patch.  I'll rebase the rest of the unapplied patches in
the series on top of this.

>From 8a8be58873c571703ee03296744a3719ff16a026 Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin.vincent@stericsson.com>
Date: Wed, 16 Jun 2010 15:58:59 +0530
Subject: [PATCH] mmci: allow the card detect GPIO value not to be inverted

On some platforms, the GPIO value from the gpio_cd pin doesn't need to
be inverted to get it active high.  Add a cd_invert platform data
parameter and change existing platforms using GPIO for CD (only
Realview) to enable it.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 arch/arm/mach-realview/core.c |    2 ++
 drivers/mmc/host/mmci.c       |    5 +++--
 include/linux/amba/mmci.h     |    2 ++
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-realview/core.c b/arch/arm/mach-realview/core.c
index 2fa38df..07c0815 100644
--- a/arch/arm/mach-realview/core.c
+++ b/arch/arm/mach-realview/core.c
@@ -259,6 +259,7 @@ struct mmci_platform_data realview_mmc0_plat_data = {
 	.status		= realview_mmc_status,
 	.gpio_wp	= 17,
 	.gpio_cd	= 16,
+	.cd_invert	= true,
 };
 
 struct mmci_platform_data realview_mmc1_plat_data = {
@@ -266,6 +267,7 @@ struct mmci_platform_data realview_mmc1_plat_data = {
 	.status		= realview_mmc_status,
 	.gpio_wp	= 19,
 	.gpio_cd	= 18,
+	.cd_invert	= true,
 };
 
 /*
diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 840b301..9a9aeac 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -570,12 +570,13 @@ static int mmci_get_ro(struct mmc_host *mmc)
 static int mmci_get_cd(struct mmc_host *mmc)
 {
 	struct mmci_host *host = mmc_priv(mmc);
+	struct mmci_platform_data *plat = host->plat;
 	unsigned int status;
 
 	if (host->gpio_cd == -ENOSYS)
-		status = host->plat->status(mmc_dev(host->mmc));
+		status = plat->status(mmc_dev(host->mmc));
 	else
-		status = !gpio_get_value(host->gpio_cd);
+		status = !!gpio_get_value(host->gpio_cd) ^ plat->cd_invert;
 
 	/*
 	 * Use positive logic throughout - status is zero for no card,
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index ca84ce7..f4ee9ac 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -24,6 +24,7 @@
  * whether a card is present in the MMC slot or not
  * @gpio_wp: read this GPIO pin to see if the card is write protected
  * @gpio_cd: read this GPIO pin to detect card insertion
+ * @cd_invert: true if the gpio_cd pin value is active low
  * @capabilities: the capabilities of the block as implemented in
  * this platform, signify anything MMC_CAP_* from mmc/host.h
  */
@@ -35,6 +36,7 @@ struct mmci_platform_data {
 	unsigned int (*status)(struct device *);
 	int	gpio_wp;
 	int	gpio_cd;
+	bool	cd_invert;
 	unsigned long capabilities;
 };
 
-- 
1.7.2.dirty

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Rabin VINCENT]

On Thu, Aug 05, 2010 at 11:25:31 +0200, Russell King - ARM Linux wrote:
> On Thu, Aug 05, 2010 at 11:44:54AM +0530, Rabin VINCENT wrote:
> > so are you OK with the cd_noinvert addition (for the GPIO case)?
> > Or a cd_invert and a patch to existing platforms?
> 
> I'd much prefer positive logic.  Double negatives (eg, not noinverted)
> are always bad news.

Here's the new patch.  I'll rebase the rest of the unapplied patches in
the series on top of this.

Re: [PATCH 04/12] mmci: allow the card detect status not to be inverted

[Russell King - ARM Linux]

On Mon, Aug 09, 2010 at 04:07:05PM +0530, Rabin VINCENT wrote:
> On Thu, Aug 05, 2010 at 11:25:31 +0200, Russell King - ARM Linux wrote:
> > On Thu, Aug 05, 2010 at 11:44:54AM +0530, Rabin VINCENT wrote:
> > > so are you OK with the cd_noinvert addition (for the GPIO case)?
> > > Or a cd_invert and a patch to existing platforms?
> > 
> > I'd much prefer positive logic.  Double negatives (eg, not noinverted)
> > are always bad news.
> 
> Here's the new patch.  I'll rebase the rest of the unapplied patches in
> the series on top of this.

This looks much better, thanks.

[PATCH 04/12] mmci: allow the card detect status not to be inverted

[Russell King - ARM Linux]

On Mon, Aug 09, 2010 at 04:07:05PM +0530, Rabin VINCENT wrote:
> On Thu, Aug 05, 2010 at 11:25:31 +0200, Russell King - ARM Linux wrote:
> > On Thu, Aug 05, 2010 at 11:44:54AM +0530, Rabin VINCENT wrote:
> > > so are you OK with the cd_noinvert addition (for the GPIO case)?
> > > Or a cd_invert and a patch to existing platforms?
> > 
> > I'd much prefer positive logic.  Double negatives (eg, not noinverted)
> > are always bad news.
> 
> Here's the new patch.  I'll rebase the rest of the unapplied patches in
> the series on top of this.

This looks much better, thanks.

[PATCH 05/12] mmci: support card detection interrupts

[Rabin Vincent]

If an IRQ can be requested on the card detected GPIO, use it instead of
polling.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   24 +++++++++++++++++++++++-
 drivers/mmc/host/mmci.h |    1 +
 2 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 7e56726..e6477e6 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -541,6 +541,15 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	return !status ^ host->plat->cd_noinvert;
 }
 
+static irqreturn_t mmci_cd_irq(int irq, void *dev_id)
+{
+	struct mmci_host *host = dev_id;
+
+	mmc_detect_change(host->mmc, msecs_to_jiffies(500));
+
+	return IRQ_HANDLED;
+}
+
 static const struct mmc_host_ops mmci_ops = {
 	.request	= mmci_request,
 	.set_ios	= mmci_set_ios,
@@ -576,6 +585,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 
 	host->gpio_wp = -ENOSYS;
 	host->gpio_cd = -ENOSYS;
+	host->gpio_cd_irq = -1;
 
 	host->hw_designer = amba_manf(dev);
 	host->hw_revision = amba_rev(dev);
@@ -654,7 +664,6 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (host->vcc == NULL)
 		mmc->ocr_avail = plat->ocr_mask;
 	mmc->caps = plat->capabilities;
-	mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	/*
 	 * We can do SGIO
@@ -698,6 +707,12 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 			host->gpio_cd = plat->gpio_cd;
 		else if (ret != -ENOSYS)
 			goto err_gpio_cd;
+
+		ret = request_any_context_irq(gpio_to_irq(plat->gpio_cd),
+					      mmci_cd_irq, 0,
+					      DRIVER_NAME " (cd)", host);
+		if (ret >= 0)
+			host->gpio_cd_irq = gpio_to_irq(plat->gpio_cd);
 	}
 	if (gpio_is_valid(plat->gpio_wp)) {
 		ret = gpio_request(plat->gpio_wp, DRIVER_NAME " (wp)");
@@ -709,6 +724,9 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 			goto err_gpio_wp;
 	}
 
+	if (host->gpio_cd_irq < 0)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	ret = request_irq(dev->irq[0], mmci_irq, IRQF_SHARED, DRIVER_NAME " (cmd)", host);
 	if (ret)
 		goto unmap;
@@ -735,6 +753,8 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (host->gpio_wp != -ENOSYS)
 		gpio_free(host->gpio_wp);
  err_gpio_wp:
+	if (host->gpio_cd_irq >= 0)
+		free_irq(host->gpio_cd_irq, host);
 	if (host->gpio_cd != -ENOSYS)
 		gpio_free(host->gpio_cd);
  err_gpio_cd:
@@ -773,6 +793,8 @@ static int __devexit mmci_remove(struct amba_device *dev)
 
 		if (host->gpio_wp != -ENOSYS)
 			gpio_free(host->gpio_wp);
+		if (host->gpio_cd_irq >= 0)
+			free_irq(host->gpio_cd_irq, host);
 		if (host->gpio_cd != -ENOSYS)
 			gpio_free(host->gpio_cd);
 
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 7cb24ab..516adc6 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -155,6 +155,7 @@ struct mmci_host {
 	struct clk		*clk;
 	int			gpio_cd;
 	int			gpio_wp;
+	int			gpio_cd_irq;
 
 	unsigned int		data_xfered;
 
-- 
1.7.0

[PATCH 05/12] mmci: support card detection interrupts

[Rabin Vincent]

If an IRQ can be requested on the card detected GPIO, use it instead of
polling.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   24 +++++++++++++++++++++++-
 drivers/mmc/host/mmci.h |    1 +
 2 files changed, 24 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 7e56726..e6477e6 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -541,6 +541,15 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	return !status ^ host->plat->cd_noinvert;
 }
 
+static irqreturn_t mmci_cd_irq(int irq, void *dev_id)
+{
+	struct mmci_host *host = dev_id;
+
+	mmc_detect_change(host->mmc, msecs_to_jiffies(500));
+
+	return IRQ_HANDLED;
+}
+
 static const struct mmc_host_ops mmci_ops = {
 	.request	= mmci_request,
 	.set_ios	= mmci_set_ios,
@@ -576,6 +585,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 
 	host->gpio_wp = -ENOSYS;
 	host->gpio_cd = -ENOSYS;
+	host->gpio_cd_irq = -1;
 
 	host->hw_designer = amba_manf(dev);
 	host->hw_revision = amba_rev(dev);
@@ -654,7 +664,6 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (host->vcc == NULL)
 		mmc->ocr_avail = plat->ocr_mask;
 	mmc->caps = plat->capabilities;
-	mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	/*
 	 * We can do SGIO
@@ -698,6 +707,12 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 			host->gpio_cd = plat->gpio_cd;
 		else if (ret != -ENOSYS)
 			goto err_gpio_cd;
+
+		ret = request_any_context_irq(gpio_to_irq(plat->gpio_cd),
+					      mmci_cd_irq, 0,
+					      DRIVER_NAME " (cd)", host);
+		if (ret >= 0)
+			host->gpio_cd_irq = gpio_to_irq(plat->gpio_cd);
 	}
 	if (gpio_is_valid(plat->gpio_wp)) {
 		ret = gpio_request(plat->gpio_wp, DRIVER_NAME " (wp)");
@@ -709,6 +724,9 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 			goto err_gpio_wp;
 	}
 
+	if (host->gpio_cd_irq < 0)
+		mmc->caps |= MMC_CAP_NEEDS_POLL;
+
 	ret = request_irq(dev->irq[0], mmci_irq, IRQF_SHARED, DRIVER_NAME " (cmd)", host);
 	if (ret)
 		goto unmap;
@@ -735,6 +753,8 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (host->gpio_wp != -ENOSYS)
 		gpio_free(host->gpio_wp);
  err_gpio_wp:
+	if (host->gpio_cd_irq >= 0)
+		free_irq(host->gpio_cd_irq, host);
 	if (host->gpio_cd != -ENOSYS)
 		gpio_free(host->gpio_cd);
  err_gpio_cd:
@@ -773,6 +793,8 @@ static int __devexit mmci_remove(struct amba_device *dev)
 
 		if (host->gpio_wp != -ENOSYS)
 			gpio_free(host->gpio_wp);
+		if (host->gpio_cd_irq >= 0)
+			free_irq(host->gpio_cd_irq, host);
 		if (host->gpio_cd != -ENOSYS)
 			gpio_free(host->gpio_cd);
 
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 7cb24ab..516adc6 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -155,6 +155,7 @@ struct mmci_host {
 	struct clk		*clk;
 	int			gpio_cd;
 	int			gpio_wp;
+	int			gpio_cd_irq;
 
 	unsigned int		data_xfered;
 
-- 
1.7.0

[PATCH 06/12] mmci: allow neither ->status nor gpio_cd to be specified

[Rabin Vincent]

The card may be always present on the board, and for these cases neither
a status callback nor a card detect GPIO is required, and card detection
polling can be disabled.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e6477e6..322958f 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -533,9 +533,12 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	struct mmci_host *host = mmc_priv(mmc);
 	unsigned int status;
 
-	if (host->gpio_cd == -ENOSYS)
+	if (host->gpio_cd == -ENOSYS) {
+		if (!host->plat->status)
+			return 1; /* Assume always present */
+
 		status = host->plat->status(mmc_dev(host->mmc));
-	else
+	} else
 		status = gpio_get_value(host->gpio_cd);
 
 	return !status ^ host->plat->cd_noinvert;
@@ -724,7 +727,8 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 			goto err_gpio_wp;
 	}
 
-	if (host->gpio_cd_irq < 0)
+	if ((host->plat->status || host->gpio_cd != -ENOSYS)
+	    && host->gpio_cd_irq < 0)
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	ret = request_irq(dev->irq[0], mmci_irq, IRQF_SHARED, DRIVER_NAME " (cmd)", host);
-- 
1.7.0

[PATCH 06/12] mmci: allow neither ->status nor gpio_cd to be specified

[Rabin Vincent]

The card may be always present on the board, and for these cases neither
a status callback nor a card detect GPIO is required, and card detection
polling can be disabled.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index e6477e6..322958f 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -533,9 +533,12 @@ static int mmci_get_cd(struct mmc_host *mmc)
 	struct mmci_host *host = mmc_priv(mmc);
 	unsigned int status;
 
-	if (host->gpio_cd == -ENOSYS)
+	if (host->gpio_cd == -ENOSYS) {
+		if (!host->plat->status)
+			return 1; /* Assume always present */
+
 		status = host->plat->status(mmc_dev(host->mmc));
-	else
+	} else
 		status = gpio_get_value(host->gpio_cd);
 
 	return !status ^ host->plat->cd_noinvert;
@@ -724,7 +727,8 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 			goto err_gpio_wp;
 	}
 
-	if (host->gpio_cd_irq < 0)
+	if ((host->plat->status || host->gpio_cd != -ENOSYS)
+	    && host->gpio_cd_irq < 0)
 		mmc->caps |= MMC_CAP_NEEDS_POLL;
 
 	ret = request_irq(dev->irq[0], mmci_irq, IRQF_SHARED, DRIVER_NAME " (cmd)", host);
-- 
1.7.0

[PATCH 07/12] mmci: pass power_mode to the translate_vdd callback

[Rabin Vincent]

Platforms may have some external power control which need to be
controlled from board specific code.  This is the case for some versions
of the MOP500 board (U8500 platform).

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c   |    3 ++-
 include/linux/amba/mmci.h |    6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 322958f..63eb7f4 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -483,7 +483,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		 * power control behind this translate function.
 		 */
 		if (!host->vcc && host->plat->translate_vdd)
-			pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd);
+			pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd,
+							 ios->power_mode);
 		/* The ST version does not have this, fall through to POWER_ON */
 		if (host->hw_designer != AMBA_VENDOR_ST) {
 			pwr |= MCI_PWR_UP;
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index f9d1bb5..61b5810 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -17,7 +17,8 @@
  * mmc/host.h
  * @translate_vdd: a callback function to translate a MMC_VDD_*
  * mask into a value to be binary or:ed and written into the
- * MMCIPWR register of the block
+ * MMCIPWR register of the block.  May also control external power
+ * based on the power_mode.
  * @status: if no GPIO read function was given to the block in
  * gpio_wp (below) this function will be called to determine
  * whether a card is present in the MMC slot or not
@@ -30,7 +31,8 @@
 struct mmci_platform_data {
 	unsigned int f_max;
 	unsigned int ocr_mask;
-	u32 (*translate_vdd)(struct device *, unsigned int);
+	u32 (*translate_vdd)(struct device *, unsigned int vdd,
+			     unsigned char power_mode);
 	unsigned int (*status)(struct device *);
 	int	gpio_wp;
 	int	gpio_cd;
-- 
1.7.0

[PATCH 07/12] mmci: pass power_mode to the translate_vdd callback

[Rabin Vincent]

Platforms may have some external power control which need to be
controlled from board specific code.  This is the case for some versions
of the MOP500 board (U8500 platform).

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c   |    3 ++-
 include/linux/amba/mmci.h |    6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 322958f..63eb7f4 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -483,7 +483,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 		 * power control behind this translate function.
 		 */
 		if (!host->vcc && host->plat->translate_vdd)
-			pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd);
+			pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd,
+							 ios->power_mode);
 		/* The ST version does not have this, fall through to POWER_ON */
 		if (host->hw_designer != AMBA_VENDOR_ST) {
 			pwr |= MCI_PWR_UP;
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index f9d1bb5..61b5810 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -17,7 +17,8 @@
  * mmc/host.h
  * @translate_vdd: a callback function to translate a MMC_VDD_*
  * mask into a value to be binary or:ed and written into the
- * MMCIPWR register of the block
+ * MMCIPWR register of the block.  May also control external power
+ * based on the power_mode.
  * @status: if no GPIO read function was given to the block in
  * gpio_wp (below) this function will be called to determine
  * whether a card is present in the MMC slot or not
@@ -30,7 +31,8 @@
 struct mmci_platform_data {
 	unsigned int f_max;
 	unsigned int ocr_mask;
-	u32 (*translate_vdd)(struct device *, unsigned int);
+	u32 (*translate_vdd)(struct device *, unsigned int vdd,
+			     unsigned char power_mode);
 	unsigned int (*status)(struct device *);
 	int	gpio_wp;
 	int	gpio_cd;
-- 
1.7.0

Re: [PATCH 07/12] mmci: pass power_mode to the translate_vdd callback

[Linus Walleij]

Thinking a bit about this patch tonight:

2010/6/22 Rabin Vincent <rabin.vincent@stericsson.com>:

> Platforms may have some external power control which need to be
> controlled from board specific code.  This is the case for some versions
> of the MOP500 board (U8500 platform).
>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
> ---
>  drivers/mmc/host/mmci.c   |    3 ++-
>  include/linux/amba/mmci.h |    6 ++++--
>  2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 322958f..63eb7f4 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -483,7 +483,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>                 * power control behind this translate function.
>                 */
>                if (!host->vcc && host->plat->translate_vdd)
> -                       pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd);
> +                       pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd,
> +                                                        ios->power_mode);
>                /* The ST version does not have this, fall through to POWER_ON */
>                if (host->hw_designer != AMBA_VENDOR_ST) {
>                        pwr |= MCI_PWR_UP;
> diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
> index f9d1bb5..61b5810 100644
> --- a/include/linux/amba/mmci.h
> +++ b/include/linux/amba/mmci.h
> @@ -17,7 +17,8 @@
>  * mmc/host.h
>  * @translate_vdd: a callback function to translate a MMC_VDD_*
>  * mask into a value to be binary or:ed and written into the
> - * MMCIPWR register of the block
> + * MMCIPWR register of the block.  May also control external power
> + * based on the power_mode.

Actually this callback is named like that for a reason: it is to be used
to translate the MMC_VDD_* into a u8:4 bitmask for the MMCIPWR
register, 4 bits possibly routed out of the PL180 block on some
designs (never seen in practice, but could be used in theory).

So it translates a voltage into a 4-bit bitmask, hence the name.

Now the semantics are altered to have other side-effects in
the platform, so the function should be renamed, something like
platform_vdd_handler() would be more appropriate.

Yours,
Linus Walleij

[PATCH 07/12] mmci: pass power_mode to the translate_vdd callback

[Linus Walleij]

Thinking a bit about this patch tonight:

2010/6/22 Rabin Vincent <rabin.vincent@stericsson.com>:

> Platforms may have some external power control which need to be
> controlled from board specific code. ?This is the case for some versions
> of the MOP500 board (U8500 platform).
>
> Acked-by: Linus Walleij <linus.walleij@stericsson.com>
> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
> ---
> ?drivers/mmc/host/mmci.c ? | ? ?3 ++-
> ?include/linux/amba/mmci.h | ? ?6 ++++--
> ?2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
> index 322958f..63eb7f4 100644
> --- a/drivers/mmc/host/mmci.c
> +++ b/drivers/mmc/host/mmci.c
> @@ -483,7 +483,8 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> ? ? ? ? ? ? ? ? * power control behind this translate function.
> ? ? ? ? ? ? ? ? */
> ? ? ? ? ? ? ? ?if (!host->vcc && host->plat->translate_vdd)
> - ? ? ? ? ? ? ? ? ? ? ? pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd);
> + ? ? ? ? ? ? ? ? ? ? ? pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ios->power_mode);
> ? ? ? ? ? ? ? ?/* The ST version does not have this, fall through to POWER_ON */
> ? ? ? ? ? ? ? ?if (host->hw_designer != AMBA_VENDOR_ST) {
> ? ? ? ? ? ? ? ? ? ? ? ?pwr |= MCI_PWR_UP;
> diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
> index f9d1bb5..61b5810 100644
> --- a/include/linux/amba/mmci.h
> +++ b/include/linux/amba/mmci.h
> @@ -17,7 +17,8 @@
> ?* mmc/host.h
> ?* @translate_vdd: a callback function to translate a MMC_VDD_*
> ?* mask into a value to be binary or:ed and written into the
> - * MMCIPWR register of the block
> + * MMCIPWR register of the block. ?May also control external power
> + * based on the power_mode.

Actually this callback is named like that for a reason: it is to be used
to translate the MMC_VDD_* into a u8:4 bitmask for the MMCIPWR
register, 4 bits possibly routed out of the PL180 block on some
designs (never seen in practice, but could be used in theory).

So it translates a voltage into a 4-bit bitmask, hence the name.

Now the semantics are altered to have other side-effects in
the platform, so the function should be renamed, something like
platform_vdd_handler() would be more appropriate.

Yours,
Linus Walleij

Re: [PATCH 07/12] mmci: pass power_mode to the translate_vdd callback

[Rabin VINCENT]

On Wed, Jun 23, 2010 at 00:03:47 +0200, Linus Walleij wrote:
> 2010/6/22 Rabin Vincent <rabin.vincent@stericsson.com>:
> > @@ -17,7 +17,8 @@
> >  * mmc/host.h
> >  * @translate_vdd: a callback function to translate a MMC_VDD_*
> >  * mask into a value to be binary or:ed and written into the
> > - * MMCIPWR register of the block
> > + * MMCIPWR register of the block.  May also control external power
> > + * based on the power_mode.
> 
> Actually this callback is named like that for a reason: it is to be used
> to translate the MMC_VDD_* into a u8:4 bitmask for the MMCIPWR
> register, 4 bits possibly routed out of the PL180 block on some
> designs (never seen in practice, but could be used in theory).
> 
> So it translates a voltage into a 4-bit bitmask, hence the name.
> 
> Now the semantics are altered to have other side-effects in
> the platform, so the function should be renamed, something like
> platform_vdd_handler() would be more appropriate.
> 

This is already inside platform data, so how about just ->vdd_handler()?
Or ->set_power(), like some other drivers?

Note that we'd also like to use this to do the board-specific settings
for the *DIREN and FBCLK bits, which replace the Voltage bits on some ST
variants, like so:

+static u32 mop500_sdi0_translate_vdd(struct device *dev, unsigned int vdd,
+				     unsigned char power_mode)
+{
+	if (power_mode == MMC_POWER_UP)
+		gpio_set_value(gpio_sdmmc_en, 1);
+	else if (power_mode == MMC_POWER_OFF)
+		gpio_set_value(gpio_sdmmc_en, 0);
+
+	return MCI_FBCLKEN | MCI_CMDDIREN | MCI_DATA0DIREN |
+	       MCI_DATA2DIREN | MCI_DATA31DIREN;
+}

Rabin

[PATCH 07/12] mmci: pass power_mode to the translate_vdd callback

[Rabin VINCENT]

On Wed, Jun 23, 2010 at 00:03:47 +0200, Linus Walleij wrote:
> 2010/6/22 Rabin Vincent <rabin.vincent@stericsson.com>:
> > @@ -17,7 +17,8 @@
> > ?* mmc/host.h
> > ?* @translate_vdd: a callback function to translate a MMC_VDD_*
> > ?* mask into a value to be binary or:ed and written into the
> > - * MMCIPWR register of the block
> > + * MMCIPWR register of the block. ?May also control external power
> > + * based on the power_mode.
> 
> Actually this callback is named like that for a reason: it is to be used
> to translate the MMC_VDD_* into a u8:4 bitmask for the MMCIPWR
> register, 4 bits possibly routed out of the PL180 block on some
> designs (never seen in practice, but could be used in theory).
> 
> So it translates a voltage into a 4-bit bitmask, hence the name.
> 
> Now the semantics are altered to have other side-effects in
> the platform, so the function should be renamed, something like
> platform_vdd_handler() would be more appropriate.
> 

This is already inside platform data, so how about just ->vdd_handler()?
Or ->set_power(), like some other drivers?

Note that we'd also like to use this to do the board-specific settings
for the *DIREN and FBCLK bits, which replace the Voltage bits on some ST
variants, like so:

+static u32 mop500_sdi0_translate_vdd(struct device *dev, unsigned int vdd,
+				     unsigned char power_mode)
+{
+	if (power_mode == MMC_POWER_UP)
+		gpio_set_value(gpio_sdmmc_en, 1);
+	else if (power_mode == MMC_POWER_OFF)
+		gpio_set_value(gpio_sdmmc_en, 0);
+
+	return MCI_FBCLKEN | MCI_CMDDIREN | MCI_DATA0DIREN |
+	       MCI_DATA2DIREN | MCI_DATA31DIREN;
+}

Rabin

RE: [PATCH 07/12] mmci: pass power_mode to the translate_vdd callback

[Linus WALLEIJ]

[Rabin]

> On Wed, Jun 23, 2010 at 00:03:47 +0200, Linus Walleij wrote:
> > 2010/6/22 Rabin Vincent <rabin.vincent@stericsson.com>:
> > > @@ -17,7 +17,8 @@
> > >  * mmc/host.h
> > >  * @translate_vdd: a callback function to translate a MMC_VDD_*
> > >  * mask into a value to be binary or:ed and written into the
> > > - * MMCIPWR register of the block
> > > + * MMCIPWR register of the block.  May also control external power
> > > + * based on the power_mode.
> >
> > Actually this callback is named like that for a reason: it is to be
> used
> > to translate the MMC_VDD_* into a u8:4 bitmask for the MMCIPWR
> > register, 4 bits possibly routed out of the PL180 block on some
> > designs (never seen in practice, but could be used in theory).
> >
> > So it translates a voltage into a 4-bit bitmask, hence the name.
> >
> > Now the semantics are altered to have other side-effects in
> > the platform, so the function should be renamed, something like
> > platform_vdd_handler() would be more appropriate.
> >
> 
> This is already inside platform data, so how about just -
> >vdd_handler()?
> Or ->set_power(), like some other drivers?

Any of these look good to me atleast.

But now another thing I came to think of (the fun never
ends):

Since the ux500 also have a regulator for this and since
that mechanism is mutually exclusive in the code (since the
semantics implied that it was only a voltage translation
function) you should also remove the condition that this
only gets called in case there is no regulator.

> Note that we'd also like to use this to do the board-specific settings
> for the *DIREN and FBCLK bits, which replace the Voltage bits on some
> ST
> variants, like so:
> 
> +static u32 mop500_sdi0_translate_vdd(struct device *dev, unsigned int
> vdd,
> +				     unsigned char power_mode)
> +{
> +	if (power_mode == MMC_POWER_UP)
> +		gpio_set_value(gpio_sdmmc_en, 1);
> +	else if (power_mode == MMC_POWER_OFF)
> +		gpio_set_value(gpio_sdmmc_en, 0);
> +
> +	return MCI_FBCLKEN | MCI_CMDDIREN | MCI_DATA0DIREN |
> +	       MCI_DATA2DIREN | MCI_DATA31DIREN;
> +}

OK just update the kerneldoc to reflect the fact that the platform
can OR on any *custom* bits it like to the PWR register in this
function.

Side notice: some of bits are for the ST version only, when you
patch in the latter, can you take this opportunity to also
rename MCI_FBCLKEN to MCI_ST_FBCLKEN and so on for all stuff
that is ST-specific? I think I am the sinner here, we had no
convention for how to do this naming when we first began
modifying this driver. It should be crystal clear that all
custom extensions are named MCI_<VENDOR>_FOO.

As a consequence, anything that is OR:ed into from the
vdd_handler() should typically be MCI_<VENDOR>_FOO, and
it will be clear why it's there.

Yours,
Linus Walleij

[PATCH 07/12] mmci: pass power_mode to the translate_vdd callback

[Linus WALLEIJ]

[Rabin]

> On Wed, Jun 23, 2010 at 00:03:47 +0200, Linus Walleij wrote:
> > 2010/6/22 Rabin Vincent <rabin.vincent@stericsson.com>:
> > > @@ -17,7 +17,8 @@
> > > ?* mmc/host.h
> > > ?* @translate_vdd: a callback function to translate a MMC_VDD_*
> > > ?* mask into a value to be binary or:ed and written into the
> > > - * MMCIPWR register of the block
> > > + * MMCIPWR register of the block. ?May also control external power
> > > + * based on the power_mode.
> >
> > Actually this callback is named like that for a reason: it is to be
> used
> > to translate the MMC_VDD_* into a u8:4 bitmask for the MMCIPWR
> > register, 4 bits possibly routed out of the PL180 block on some
> > designs (never seen in practice, but could be used in theory).
> >
> > So it translates a voltage into a 4-bit bitmask, hence the name.
> >
> > Now the semantics are altered to have other side-effects in
> > the platform, so the function should be renamed, something like
> > platform_vdd_handler() would be more appropriate.
> >
> 
> This is already inside platform data, so how about just -
> >vdd_handler()?
> Or ->set_power(), like some other drivers?

Any of these look good to me atleast.

But now another thing I came to think of (the fun never
ends):

Since the ux500 also have a regulator for this and since
that mechanism is mutually exclusive in the code (since the
semantics implied that it was only a voltage translation
function) you should also remove the condition that this
only gets called in case there is no regulator.

> Note that we'd also like to use this to do the board-specific settings
> for the *DIREN and FBCLK bits, which replace the Voltage bits on some
> ST
> variants, like so:
> 
> +static u32 mop500_sdi0_translate_vdd(struct device *dev, unsigned int
> vdd,
> +				     unsigned char power_mode)
> +{
> +	if (power_mode == MMC_POWER_UP)
> +		gpio_set_value(gpio_sdmmc_en, 1);
> +	else if (power_mode == MMC_POWER_OFF)
> +		gpio_set_value(gpio_sdmmc_en, 0);
> +
> +	return MCI_FBCLKEN | MCI_CMDDIREN | MCI_DATA0DIREN |
> +	       MCI_DATA2DIREN | MCI_DATA31DIREN;
> +}

OK just update the kerneldoc to reflect the fact that the platform
can OR on any *custom* bits it like to the PWR register in this
function.

Side notice: some of bits are for the ST version only, when you
patch in the latter, can you take this opportunity to also
rename MCI_FBCLKEN to MCI_ST_FBCLKEN and so on for all stuff
that is ST-specific? I think I am the sinner here, we had no
convention for how to do this naming when we first began
modifying this driver. It should be crystal clear that all
custom extensions are named MCI_<VENDOR>_FOO.

As a consequence, anything that is OR:ed into from the
vdd_handler() should typically be MCI_<VENDOR>_FOO, and
it will be clear why it's there.

Yours,
Linus Walleij

[PATCHv2 07/12] mmci: pass power_mode to the translate_vdd callback

[Rabin Vincent]

Platforms may have some external power control which need to be
controlled from board specific code.  Rename the translate_vdd()
callback to vdd_handler() and pass it the power mode.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
v2:
  - Call vdd_handler even if regulator is used
  - Rename translate_vdd to vdd_handler

 drivers/mmc/host/mmci.c   |   13 +++----------
 include/linux/amba/mmci.h |   10 ++++++----
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 5ef262b..d8e0ea3 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -477,16 +477,9 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			/* This implicitly enables the regulator */
 			mmc_regulator_set_ocr(host->vcc, ios->vdd);
 #endif
-		/*
-		 * The translate_vdd function is not used if you have
-		 * an external regulator, or your design is really weird.
-		 * Using it would mean sending in power control BOTH using
-		 * a regulator AND the 4 MMCIPWR bits. If we don't have
-		 * a regulator, we might have some other platform specific
-		 * power control behind this translate function.
-		 */
-		if (!host->vcc && host->plat->translate_vdd)
-			pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd);
+		if (host->plat->vdd_handler)
+			pwr |= host->plat->vdd_handler(mmc_dev(mmc), ios->vdd,
+						       ios->power_mode);
 		/* The ST version does not have this, fall through to POWER_ON */
 		if (host->hw_designer != AMBA_VENDOR_ST) {
 			pwr |= MCI_PWR_UP;
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index f9d1bb5..71a2576 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -15,9 +15,10 @@
  * @ocr_mask: available voltages on the 4 pins from the block, this
  * is ignored if a regulator is used, see the MMC_VDD_* masks in
  * mmc/host.h
- * @translate_vdd: a callback function to translate a MMC_VDD_*
- * mask into a value to be binary or:ed and written into the
- * MMCIPWR register of the block
+ * @vdd_handler: a callback function to translate a MMC_VDD_*
+ * mask into a value to be binary (or set some other custom bits
+ * in MMCIPWR) or:ed and written into the MMCIPWR register of the
+ * block.  May also control external power based on the power_mode.
  * @status: if no GPIO read function was given to the block in
  * gpio_wp (below) this function will be called to determine
  * whether a card is present in the MMC slot or not
@@ -30,7 +31,8 @@
 struct mmci_platform_data {
 	unsigned int f_max;
 	unsigned int ocr_mask;
-	u32 (*translate_vdd)(struct device *, unsigned int);
+	u32 (*vdd_handler)(struct device *, unsigned int vdd,
+			   unsigned char power_mode);
 	unsigned int (*status)(struct device *);
 	int	gpio_wp;
 	int	gpio_cd;
-- 
1.7.0

[PATCHv2 07/12] mmci: pass power_mode to the translate_vdd callback

[Rabin Vincent]

Platforms may have some external power control which need to be
controlled from board specific code.  Rename the translate_vdd()
callback to vdd_handler() and pass it the power mode.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
v2:
  - Call vdd_handler even if regulator is used
  - Rename translate_vdd to vdd_handler

 drivers/mmc/host/mmci.c   |   13 +++----------
 include/linux/amba/mmci.h |   10 ++++++----
 2 files changed, 9 insertions(+), 14 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 5ef262b..d8e0ea3 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -477,16 +477,9 @@ static void mmci_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
 			/* This implicitly enables the regulator */
 			mmc_regulator_set_ocr(host->vcc, ios->vdd);
 #endif
-		/*
-		 * The translate_vdd function is not used if you have
-		 * an external regulator, or your design is really weird.
-		 * Using it would mean sending in power control BOTH using
-		 * a regulator AND the 4 MMCIPWR bits. If we don't have
-		 * a regulator, we might have some other platform specific
-		 * power control behind this translate function.
-		 */
-		if (!host->vcc && host->plat->translate_vdd)
-			pwr |= host->plat->translate_vdd(mmc_dev(mmc), ios->vdd);
+		if (host->plat->vdd_handler)
+			pwr |= host->plat->vdd_handler(mmc_dev(mmc), ios->vdd,
+						       ios->power_mode);
 		/* The ST version does not have this, fall through to POWER_ON */
 		if (host->hw_designer != AMBA_VENDOR_ST) {
 			pwr |= MCI_PWR_UP;
diff --git a/include/linux/amba/mmci.h b/include/linux/amba/mmci.h
index f9d1bb5..71a2576 100644
--- a/include/linux/amba/mmci.h
+++ b/include/linux/amba/mmci.h
@@ -15,9 +15,10 @@
  * @ocr_mask: available voltages on the 4 pins from the block, this
  * is ignored if a regulator is used, see the MMC_VDD_* masks in
  * mmc/host.h
- * @translate_vdd: a callback function to translate a MMC_VDD_*
- * mask into a value to be binary or:ed and written into the
- * MMCIPWR register of the block
+ * @vdd_handler: a callback function to translate a MMC_VDD_*
+ * mask into a value to be binary (or set some other custom bits
+ * in MMCIPWR) or:ed and written into the MMCIPWR register of the
+ * block.  May also control external power based on the power_mode.
  * @status: if no GPIO read function was given to the block in
  * gpio_wp (below) this function will be called to determine
  * whether a card is present in the MMC slot or not
@@ -30,7 +31,8 @@
 struct mmci_platform_data {
 	unsigned int f_max;
 	unsigned int ocr_mask;
-	u32 (*translate_vdd)(struct device *, unsigned int);
+	u32 (*vdd_handler)(struct device *, unsigned int vdd,
+			   unsigned char power_mode);
 	unsigned int (*status)(struct device *);
 	int	gpio_wp;
 	int	gpio_cd;
-- 
1.7.0

[PATCH 08/12] mmci: add variant data and default MCICLOCK support

[Rabin Vincent]

Add a variant_data structure to handle the differences between the
various variants of this peripheral.  Add a first quirk for a default
MCICLOCK value, required on the Ux500 variant where the enable bit needs
to be always set, since it controls access to some registers.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   31 ++++++++++++++++++++++++++++++-
 drivers/mmc/host/mmci.h |    2 ++
 2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 63eb7f4..a1d1023 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -36,12 +36,30 @@
 
 static unsigned int fmax = 515633;
 
+/**
+ * struct variant_data - MMCI variant-specific quirks
+ * @clkreg: default value for MCICLOCK register
+ */
+struct variant_data {
+	unsigned int		clkreg;
+};
+
+static struct variant_data variant_arm = {
+};
+
+static struct variant_data variant_u300 = {
+};
+
+static struct variant_data variant_ux500 = {
+	.clkreg			= MCI_CLK_ENABLE,
+};
 /*
  * This must be called with host->lock held
  */
 static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 {
-	u32 clk = 0;
+	struct variant_data *variant = host->variant;
+	u32 clk = variant->clkreg;
 
 	if (desired) {
 		if (desired >= host->mclk) {
@@ -564,6 +582,7 @@ static const struct mmc_host_ops mmci_ops = {
 static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 {
 	struct mmci_platform_data *plat = dev->dev.platform_data;
+	struct variant_data *variant = id->data;
 	struct mmci_host *host;
 	struct mmc_host *mmc;
 	int ret;
@@ -608,6 +627,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 		goto clk_free;
 
 	host->plat = plat;
+	host->variant = variant;
 	host->mclk = clk_get_rate(host->clk);
 	/*
 	 * According to the spec, mclk is max 100 MHz,
@@ -860,19 +880,28 @@ static struct amba_id mmci_ids[] = {
 	{
 		.id	= 0x00041180,
 		.mask	= 0x000fffff,
+		.data	= &variant_arm,
 	},
 	{
 		.id	= 0x00041181,
 		.mask	= 0x000fffff,
+		.data	= &variant_arm,
 	},
 	/* ST Micro variants */
 	{
 		.id     = 0x00180180,
 		.mask   = 0x00ffffff,
+		.data	= &variant_u300,
 	},
 	{
 		.id     = 0x00280180,
 		.mask   = 0x00ffffff,
+		.data	= &variant_u300,
+	},
+	{
+		.id     = 0x00480180,
+		.mask   = 0x00ffffff,
+		.data	= &variant_ux500,
 	},
 	{ 0, 0 },
 };
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 516adc6..5e635e1 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -145,6 +145,7 @@
 #define NR_SG		16
 
 struct clk;
+struct variant_data;
 
 struct mmci_host {
 	void __iomem		*base;
@@ -165,6 +166,7 @@ struct mmci_host {
 	unsigned int		cclk;
 	u32			pwr;
 	struct mmci_platform_data *plat;
+	struct variant_data	*variant;
 
 	u8			hw_designer;
 	u8			hw_revision:4;
-- 
1.7.0

[PATCH 08/12] mmci: add variant data and default MCICLOCK support

[Rabin Vincent]

Add a variant_data structure to handle the differences between the
various variants of this peripheral.  Add a first quirk for a default
MCICLOCK value, required on the Ux500 variant where the enable bit needs
to be always set, since it controls access to some registers.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   31 ++++++++++++++++++++++++++++++-
 drivers/mmc/host/mmci.h |    2 ++
 2 files changed, 32 insertions(+), 1 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 63eb7f4..a1d1023 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -36,12 +36,30 @@
 
 static unsigned int fmax = 515633;
 
+/**
+ * struct variant_data - MMCI variant-specific quirks
+ * @clkreg: default value for MCICLOCK register
+ */
+struct variant_data {
+	unsigned int		clkreg;
+};
+
+static struct variant_data variant_arm = {
+};
+
+static struct variant_data variant_u300 = {
+};
+
+static struct variant_data variant_ux500 = {
+	.clkreg			= MCI_CLK_ENABLE,
+};
 /*
  * This must be called with host->lock held
  */
 static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 {
-	u32 clk = 0;
+	struct variant_data *variant = host->variant;
+	u32 clk = variant->clkreg;
 
 	if (desired) {
 		if (desired >= host->mclk) {
@@ -564,6 +582,7 @@ static const struct mmc_host_ops mmci_ops = {
 static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 {
 	struct mmci_platform_data *plat = dev->dev.platform_data;
+	struct variant_data *variant = id->data;
 	struct mmci_host *host;
 	struct mmc_host *mmc;
 	int ret;
@@ -608,6 +627,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 		goto clk_free;
 
 	host->plat = plat;
+	host->variant = variant;
 	host->mclk = clk_get_rate(host->clk);
 	/*
 	 * According to the spec, mclk is max 100 MHz,
@@ -860,19 +880,28 @@ static struct amba_id mmci_ids[] = {
 	{
 		.id	= 0x00041180,
 		.mask	= 0x000fffff,
+		.data	= &variant_arm,
 	},
 	{
 		.id	= 0x00041181,
 		.mask	= 0x000fffff,
+		.data	= &variant_arm,
 	},
 	/* ST Micro variants */
 	{
 		.id     = 0x00180180,
 		.mask   = 0x00ffffff,
+		.data	= &variant_u300,
 	},
 	{
 		.id     = 0x00280180,
 		.mask   = 0x00ffffff,
+		.data	= &variant_u300,
+	},
+	{
+		.id     = 0x00480180,
+		.mask   = 0x00ffffff,
+		.data	= &variant_ux500,
 	},
 	{ 0, 0 },
 };
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 516adc6..5e635e1 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -145,6 +145,7 @@
 #define NR_SG		16
 
 struct clk;
+struct variant_data;
 
 struct mmci_host {
 	void __iomem		*base;
@@ -165,6 +166,7 @@ struct mmci_host {
 	unsigned int		cclk;
 	u32			pwr;
 	struct mmci_platform_data *plat;
+	struct variant_data	*variant;
 
 	u8			hw_designer;
 	u8			hw_revision:4;
-- 
1.7.0

[PATCH 09/12] mmci: enable hardware flow control on Ux500 variants

[Rabin Vincent]

Although both the U300 and Ux500 use ST variants, the HWFCEN bits are at
different positions, so use the variant_data to store the information.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |    8 ++++++--
 drivers/mmc/host/mmci.h |    2 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index a1d1023..3fca46d 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -39,19 +39,23 @@ static unsigned int fmax = 515633;
 /**
  * struct variant_data - MMCI variant-specific quirks
  * @clkreg: default value for MCICLOCK register
+ * @clkreg_enable: enable value for MMCICLOCK register
  */
 struct variant_data {
 	unsigned int		clkreg;
+	unsigned int		clkreg_enable;
 };
 
 static struct variant_data variant_arm = {
 };
 
 static struct variant_data variant_u300 = {
+	.clkreg_enable		= 1 << 13, /* HWFCEN */
 };
 
 static struct variant_data variant_ux500 = {
 	.clkreg			= MCI_CLK_ENABLE,
+	.clkreg_enable		= 1 << 14, /* HWFCEN */
 };
 /*
  * This must be called with host->lock held
@@ -71,8 +75,8 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 				clk = 255;
 			host->cclk = host->mclk / (2 * (clk + 1));
 		}
-		if (host->hw_designer == AMBA_VENDOR_ST)
-			clk |= MCI_ST_FCEN; /* Bug fix in ST IP block */
+
+		clk |= variant->clkreg_enable;
 		clk |= MCI_CLK_ENABLE;
 		/* This hasn't proven to be worthwhile */
 		/* clk |= MCI_CLK_PWRSAVE; */
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 5e635e1..0a11ddc 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -28,8 +28,6 @@
 #define MCI_4BIT_BUS		(1 << 11)
 /* 8bit wide buses supported in ST Micro versions */
 #define MCI_ST_8BIT_BUS		(1 << 12)
-/* HW flow control on the ST Micro version */
-#define MCI_ST_FCEN		(1 << 13)
 
 #define MMCIARGUMENT		0x008
 #define MMCICOMMAND		0x00c
-- 
1.7.0

[PATCH 09/12] mmci: enable hardware flow control on Ux500 variants

[Rabin Vincent]

Although both the U300 and Ux500 use ST variants, the HWFCEN bits are at
different positions, so use the variant_data to store the information.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |    8 ++++++--
 drivers/mmc/host/mmci.h |    2 --
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index a1d1023..3fca46d 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -39,19 +39,23 @@ static unsigned int fmax = 515633;
 /**
  * struct variant_data - MMCI variant-specific quirks
  * @clkreg: default value for MCICLOCK register
+ * @clkreg_enable: enable value for MMCICLOCK register
  */
 struct variant_data {
 	unsigned int		clkreg;
+	unsigned int		clkreg_enable;
 };
 
 static struct variant_data variant_arm = {
 };
 
 static struct variant_data variant_u300 = {
+	.clkreg_enable		= 1 << 13, /* HWFCEN */
 };
 
 static struct variant_data variant_ux500 = {
 	.clkreg			= MCI_CLK_ENABLE,
+	.clkreg_enable		= 1 << 14, /* HWFCEN */
 };
 /*
  * This must be called with host->lock held
@@ -71,8 +75,8 @@ static void mmci_set_clkreg(struct mmci_host *host, unsigned int desired)
 				clk = 255;
 			host->cclk = host->mclk / (2 * (clk + 1));
 		}
-		if (host->hw_designer == AMBA_VENDOR_ST)
-			clk |= MCI_ST_FCEN; /* Bug fix in ST IP block */
+
+		clk |= variant->clkreg_enable;
 		clk |= MCI_CLK_ENABLE;
 		/* This hasn't proven to be worthwhile */
 		/* clk |= MCI_CLK_PWRSAVE; */
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 5e635e1..0a11ddc 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -28,8 +28,6 @@
 #define MCI_4BIT_BUS		(1 << 11)
 /* 8bit wide buses supported in ST Micro versions */
 #define MCI_ST_8BIT_BUS		(1 << 12)
-/* HW flow control on the ST Micro version */
-#define MCI_ST_FCEN		(1 << 13)
 
 #define MMCIARGUMENT		0x008
 #define MMCICOMMAND		0x00c
-- 
1.7.0

[PATCH 10/12] mmci: support larger MMCIDATALENGTH register

[Rabin Vincent]

The Ux500 variant has a 24-bit MMCIDATALENGTH register, as opposed to
the 16-bit one on the ARM version.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 3fca46d..6c8e41b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -40,22 +40,27 @@ static unsigned int fmax = 515633;
  * struct variant_data - MMCI variant-specific quirks
  * @clkreg: default value for MCICLOCK register
  * @clkreg_enable: enable value for MMCICLOCK register
+ * @datalength_bits: number of bits in the MMCIDATALENGTH register
  */
 struct variant_data {
 	unsigned int		clkreg;
 	unsigned int		clkreg_enable;
+	unsigned int		datalength_bits;
 };
 
 static struct variant_data variant_arm = {
+	.datalength_bits	= 16,
 };
 
 static struct variant_data variant_u300 = {
 	.clkreg_enable		= 1 << 13, /* HWFCEN */
+	.datalength_bits	= 16,
 };
 
 static struct variant_data variant_ux500 = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= 1 << 14, /* HWFCEN */
+	.datalength_bits	= 24,
 };
 /*
  * This must be called with host->lock held
@@ -700,10 +705,11 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	mmc->max_phys_segs = NR_SG;
 
 	/*
-	 * Since we only have a 16-bit data length register, we must
-	 * ensure that we don't exceed 2^16-1 bytes in a single request.
+	 * Since only a certain number of bits are valid in the data length
+	 * register, we must ensure that we don't exceed 2^num-1 bytes in a
+	 * single request.
 	 */
-	mmc->max_req_size = 65535;
+	mmc->max_req_size = (1 << variant->datalength_bits) - 1;
 
 	/*
 	 * Set the maximum segment size.  Since we aren't doing DMA
-- 
1.7.0

[PATCH 10/12] mmci: support larger MMCIDATALENGTH register

[Rabin Vincent]

The Ux500 variant has a 24-bit MMCIDATALENGTH register, as opposed to
the 16-bit one on the ARM version.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 3fca46d..6c8e41b 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -40,22 +40,27 @@ static unsigned int fmax = 515633;
  * struct variant_data - MMCI variant-specific quirks
  * @clkreg: default value for MCICLOCK register
  * @clkreg_enable: enable value for MMCICLOCK register
+ * @datalength_bits: number of bits in the MMCIDATALENGTH register
  */
 struct variant_data {
 	unsigned int		clkreg;
 	unsigned int		clkreg_enable;
+	unsigned int		datalength_bits;
 };
 
 static struct variant_data variant_arm = {
+	.datalength_bits	= 16,
 };
 
 static struct variant_data variant_u300 = {
 	.clkreg_enable		= 1 << 13, /* HWFCEN */
+	.datalength_bits	= 16,
 };
 
 static struct variant_data variant_ux500 = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= 1 << 14, /* HWFCEN */
+	.datalength_bits	= 24,
 };
 /*
  * This must be called with host->lock held
@@ -700,10 +705,11 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	mmc->max_phys_segs = NR_SG;
 
 	/*
-	 * Since we only have a 16-bit data length register, we must
-	 * ensure that we don't exceed 2^16-1 bytes in a single request.
+	 * Since only a certain number of bits are valid in the data length
+	 * register, we must ensure that we don't exceed 2^num-1 bytes in a
+	 * single request.
 	 */
-	mmc->max_req_size = 65535;
+	mmc->max_req_size = (1 << variant->datalength_bits) - 1;
 
 	/*
 	 * Set the maximum segment size.  Since we aren't doing DMA
-- 
1.7.0

[PATCH 11/12] mmci: support different FIFO sizes

[Rabin Vincent]

The Ux500 variant has a 32-word FIFO (TXFIFOEMPTY is asserted when it
has 2 left) and TXFIFOHALFEMPTY is repurposed as TXFIFOBURSTWRITEABLE,
with a burst being defined as 8-words.  Likewise for RX.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   22 +++++++++++++++++++---
 drivers/mmc/host/mmci.h |    7 -------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 6c8e41b..4eb7265 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -41,23 +41,35 @@ static unsigned int fmax = 515633;
  * @clkreg: default value for MCICLOCK register
  * @clkreg_enable: enable value for MMCICLOCK register
  * @datalength_bits: number of bits in the MMCIDATALENGTH register
+ * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
+ *	      is asserted (likewise for RX)
+ * @fifohalfsize: number of bytes that can be written when MCI_TXFIFOHALFEMPTY
+ *		  is asserted (likewise for RX)
  */
 struct variant_data {
 	unsigned int		clkreg;
 	unsigned int		clkreg_enable;
 	unsigned int		datalength_bits;
+	unsigned int		fifosize;
+	unsigned int		fifohalfsize;
 };
 
 static struct variant_data variant_arm = {
+	.fifosize		= 16 * 4,
+	.fifohalfsize		= 8 * 4,
 	.datalength_bits	= 16,
 };
 
 static struct variant_data variant_u300 = {
+	.fifosize		= 16 * 4,
+	.fifohalfsize		= 8 * 4,
 	.clkreg_enable		= 1 << 13, /* HWFCEN */
 	.datalength_bits	= 16,
 };
 
 static struct variant_data variant_ux500 = {
+	.fifosize		= 30 * 4,
+	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= 1 << 14, /* HWFCEN */
 	.datalength_bits	= 24,
@@ -138,6 +150,7 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
 
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
+	struct variant_data *variant = host->variant;
 	unsigned int datactrl, timeout, irqmask;
 	unsigned long long clks;
 	void __iomem *base;
@@ -173,7 +186,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 		 * If we have less than a FIFOSIZE of bytes to transfer,
 		 * trigger a PIO interrupt as soon as any data is available.
 		 */
-		if (host->size < MCI_FIFOSIZE)
+		if (host->size < variant->fifosize)
 			irqmask |= MCI_RXDATAAVLBLMASK;
 	} else {
 		/*
@@ -316,13 +329,15 @@ static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int rema
 
 static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int remain, u32 status)
 {
+	struct variant_data *variant = host->variant;
 	void __iomem *base = host->base;
 	char *ptr = buffer;
 
 	do {
 		unsigned int count, maxcnt;
 
-		maxcnt = status & MCI_TXFIFOEMPTY ? MCI_FIFOSIZE : MCI_FIFOHALFSIZE;
+		maxcnt = status & MCI_TXFIFOEMPTY ?
+			 variant->fifosize : variant->fifohalfsize;
 		count = min(remain, maxcnt);
 
 		writesl(base + MMCIFIFO, ptr, count >> 2);
@@ -346,6 +361,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 {
 	struct mmci_host *host = dev_id;
 	struct sg_mapping_iter *sg_miter = &host->sg_miter;
+	struct variant_data *variant = host->variant;
 	void __iomem *base = host->base;
 	unsigned long flags;
 	u32 status;
@@ -401,7 +417,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 * If we're nearing the end of the read, switch to
 	 * "any data available" mode.
 	 */
-	if (status & MCI_RXACTIVE && host->size < MCI_FIFOSIZE)
+	if (status & MCI_RXACTIVE && host->size < variant->fifosize)
 		writel(MCI_RXDATAAVLBLMASK, base + MMCIMASK1);
 
 	/*
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 0a11ddc..c7d373c 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -133,13 +133,6 @@
 	MCI_DATATIMEOUTMASK|MCI_TXUNDERRUNMASK|MCI_RXOVERRUNMASK|	\
 	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATABLOCKENDMASK)
 
-/*
- * The size of the FIFO in bytes.
- */
-#define MCI_FIFOSIZE	(16*4)
-	
-#define MCI_FIFOHALFSIZE (MCI_FIFOSIZE / 2)
-
 #define NR_SG		16
 
 struct clk;
-- 
1.7.0

[PATCH 11/12] mmci: support different FIFO sizes

[Rabin Vincent]

The Ux500 variant has a 32-word FIFO (TXFIFOEMPTY is asserted when it
has 2 left) and TXFIFOHALFEMPTY is repurposed as TXFIFOBURSTWRITEABLE,
with a burst being defined as 8-words.  Likewise for RX.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   22 +++++++++++++++++++---
 drivers/mmc/host/mmci.h |    7 -------
 2 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 6c8e41b..4eb7265 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -41,23 +41,35 @@ static unsigned int fmax = 515633;
  * @clkreg: default value for MCICLOCK register
  * @clkreg_enable: enable value for MMCICLOCK register
  * @datalength_bits: number of bits in the MMCIDATALENGTH register
+ * @fifosize: number of bytes that can be written when MMCI_TXFIFOEMPTY
+ *	      is asserted (likewise for RX)
+ * @fifohalfsize: number of bytes that can be written when MCI_TXFIFOHALFEMPTY
+ *		  is asserted (likewise for RX)
  */
 struct variant_data {
 	unsigned int		clkreg;
 	unsigned int		clkreg_enable;
 	unsigned int		datalength_bits;
+	unsigned int		fifosize;
+	unsigned int		fifohalfsize;
 };
 
 static struct variant_data variant_arm = {
+	.fifosize		= 16 * 4,
+	.fifohalfsize		= 8 * 4,
 	.datalength_bits	= 16,
 };
 
 static struct variant_data variant_u300 = {
+	.fifosize		= 16 * 4,
+	.fifohalfsize		= 8 * 4,
 	.clkreg_enable		= 1 << 13, /* HWFCEN */
 	.datalength_bits	= 16,
 };
 
 static struct variant_data variant_ux500 = {
+	.fifosize		= 30 * 4,
+	.fifohalfsize		= 8 * 4,
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= 1 << 14, /* HWFCEN */
 	.datalength_bits	= 24,
@@ -138,6 +150,7 @@ static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
 
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
+	struct variant_data *variant = host->variant;
 	unsigned int datactrl, timeout, irqmask;
 	unsigned long long clks;
 	void __iomem *base;
@@ -173,7 +186,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 		 * If we have less than a FIFOSIZE of bytes to transfer,
 		 * trigger a PIO interrupt as soon as any data is available.
 		 */
-		if (host->size < MCI_FIFOSIZE)
+		if (host->size < variant->fifosize)
 			irqmask |= MCI_RXDATAAVLBLMASK;
 	} else {
 		/*
@@ -316,13 +329,15 @@ static int mmci_pio_read(struct mmci_host *host, char *buffer, unsigned int rema
 
 static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int remain, u32 status)
 {
+	struct variant_data *variant = host->variant;
 	void __iomem *base = host->base;
 	char *ptr = buffer;
 
 	do {
 		unsigned int count, maxcnt;
 
-		maxcnt = status & MCI_TXFIFOEMPTY ? MCI_FIFOSIZE : MCI_FIFOHALFSIZE;
+		maxcnt = status & MCI_TXFIFOEMPTY ?
+			 variant->fifosize : variant->fifohalfsize;
 		count = min(remain, maxcnt);
 
 		writesl(base + MMCIFIFO, ptr, count >> 2);
@@ -346,6 +361,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 {
 	struct mmci_host *host = dev_id;
 	struct sg_mapping_iter *sg_miter = &host->sg_miter;
+	struct variant_data *variant = host->variant;
 	void __iomem *base = host->base;
 	unsigned long flags;
 	u32 status;
@@ -401,7 +417,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 * If we're nearing the end of the read, switch to
 	 * "any data available" mode.
 	 */
-	if (status & MCI_RXACTIVE && host->size < MCI_FIFOSIZE)
+	if (status & MCI_RXACTIVE && host->size < variant->fifosize)
 		writel(MCI_RXDATAAVLBLMASK, base + MMCIMASK1);
 
 	/*
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index 0a11ddc..c7d373c 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -133,13 +133,6 @@
 	MCI_DATATIMEOUTMASK|MCI_TXUNDERRUNMASK|MCI_RXOVERRUNMASK|	\
 	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATABLOCKENDMASK)
 
-/*
- * The size of the FIFO in bytes.
- */
-#define MCI_FIFOSIZE	(16*4)
-	
-#define MCI_FIFOHALFSIZE (MCI_FIFOSIZE / 2)
-
 #define NR_SG		16
 
 struct clk;
-- 
1.7.0

[PATCH 12/12] mmci: support variants with only one irq

[Rabin Vincent]

The Ux500 variants have only one IRQ line hooked up.  Allow these to
work by directing the PIO interrupts also to the first IRQ line.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   67 ++++++++++++++++++++++++++++++++++++++++------
 drivers/mmc/host/mmci.h |    5 +++
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4eb7265..5baa3bd 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -45,6 +45,7 @@ static unsigned int fmax = 515633;
  *	      is asserted (likewise for RX)
  * @fifohalfsize: number of bytes that can be written when MCI_TXFIFOHALFEMPTY
  *		  is asserted (likewise for RX)
+ * @singleirq: true if only one IRQ line is available
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -52,6 +53,7 @@ struct variant_data {
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
+	bool			singleirq;
 };
 
 static struct variant_data variant_arm = {
@@ -73,7 +75,9 @@ static struct variant_data variant_ux500 = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= 1 << 14, /* HWFCEN */
 	.datalength_bits	= 24,
+	.singleirq		= true,
 };
+
 /*
  * This must be called with host->lock held
  */
@@ -129,10 +133,27 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 	spin_lock(&host->lock);
 }
 
+static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
+{
+	struct variant_data *variant = host->variant;
+	void __iomem *base = host->base;
+
+	if (variant->singleirq) {
+		unsigned int mask0 = readl(base + MMCIMASK0);
+
+		mask0 &= ~MCI_IRQ1MASK;
+		mask0 |= mask;
+
+		writel(mask0, base + MMCIMASK0);
+	}
+
+	writel(mask, base + MMCIMASK1);
+}
+
 static void mmci_stop_data(struct mmci_host *host)
 {
 	writel(0, host->base + MMCIDATACTRL);
-	writel(0, host->base + MMCIMASK1);
+	mmci_set_mask1(host, 0);
 	host->data = NULL;
 }
 
@@ -198,7 +219,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 
 	writel(datactrl, base + MMCIDATACTRL);
 	writel(readl(base + MMCIMASK0) & ~MCI_DATAENDMASK, base + MMCIMASK0);
-	writel(irqmask, base + MMCIMASK1);
+	mmci_set_mask1(host, irqmask);
 }
 
 static void
@@ -260,6 +281,10 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 	if (status & MCI_DATAEND) {
 		mmci_stop_data(host);
 
+		/* MCI_DATABLOCKEND not used with single irq */
+		if (host->variant->singleirq && !data->error)
+			host->data_xfered = data->blksz * data->blocks;
+
 		if (!data->stop) {
 			mmci_request_end(host, data->mrq);
 		} else {
@@ -418,7 +443,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 * "any data available" mode.
 	 */
 	if (status & MCI_RXACTIVE && host->size < variant->fifosize)
-		writel(MCI_RXDATAAVLBLMASK, base + MMCIMASK1);
+		mmci_set_mask1(host, MCI_RXDATAAVLBLMASK);
 
 	/*
 	 * If we run out of data, disable the data IRQs; this
@@ -427,7 +452,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 * stops us racing with our data end IRQ.
 	 */
 	if (host->size == 0) {
-		writel(0, base + MMCIMASK1);
+		mmci_set_mask1(host, 0);
 		writel(readl(base + MMCIMASK0) | MCI_DATAENDMASK, base + MMCIMASK0);
 	}
 
@@ -440,6 +465,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 static irqreturn_t mmci_irq(int irq, void *dev_id)
 {
 	struct mmci_host *host = dev_id;
+	struct variant_data *variant = host->variant;
 	u32 status;
 	int ret = 0;
 
@@ -450,6 +476,14 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		struct mmc_data *data;
 
 		status = readl(host->base + MMCISTATUS);
+
+		if (variant->singleirq) {
+			if (status & readl(host->base + MMCIMASK1))
+				mmci_pio_irq(irq, dev_id);
+
+			status &= ~MCI_IRQ1MASK;
+		}
+
 		status &= readl(host->base + MMCIMASK0);
 		writel(status, host->base + MMCICLEAR);
 
@@ -610,6 +644,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	struct variant_data *variant = id->data;
 	struct mmci_host *host;
 	struct mmc_host *mmc;
+	unsigned int mask;
 	int ret;
 
 	/* must have platform data */
@@ -782,11 +817,23 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (ret)
 		goto unmap;
 
-	ret = request_irq(dev->irq[1], mmci_pio_irq, IRQF_SHARED, DRIVER_NAME " (pio)", host);
-	if (ret)
-		goto irq0_free;
+	if (!variant->singleirq) {
+		ret = request_irq(dev->irq[1], mmci_pio_irq, IRQF_SHARED,
+				  DRIVER_NAME " (pio)", host);
+		if (ret)
+			goto irq0_free;
+	}
+
+	/*
+	 * MCI_DATABLOCKEND doesn't seem to immediately clear from the status,
+	 * so we can't use it keep count when only one irq is used because the
+	 * irq will hit for other reasons.
+	 */
+	mask = MCI_IRQENABLE;
+	if (variant->singleirq)
+		mask &= ~MCI_DATABLOCKEND;
 
-	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+	writel(mask, host->base + MMCIMASK0);
 
 	amba_set_drvdata(dev, mmc);
 
@@ -830,6 +877,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
 
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
+		struct variant_data *variant = host->variant;
 
 		mmc_remove_host(mmc);
 
@@ -840,7 +888,8 @@ static int __devexit mmci_remove(struct amba_device *dev)
 		writel(0, host->base + MMCIDATACTRL);
 
 		free_irq(dev->irq[0], host);
-		free_irq(dev->irq[1], host);
+		if (!variant->singleirq)
+			free_irq(dev->irq[1], host);
 
 		if (host->gpio_wp != -ENOSYS)
 			gpio_free(host->gpio_wp);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index c7d373c..ba203b8 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -133,6 +133,11 @@
 	MCI_DATATIMEOUTMASK|MCI_TXUNDERRUNMASK|MCI_RXOVERRUNMASK|	\
 	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATABLOCKENDMASK)
 
+/* These interrupts are directed to IRQ1 when two IRQ lines are available */
+#define MCI_IRQ1MASK \
+	(MCI_RXFIFOHALFFULLMASK | MCI_RXDATAAVLBLMASK | \
+	 MCI_TXFIFOHALFEMPTYMASK)
+
 #define NR_SG		16
 
 struct clk;
-- 
1.7.0

[PATCH 12/12] mmci: support variants with only one irq

[Rabin Vincent]

The Ux500 variants have only one IRQ line hooked up.  Allow these to
work by directing the PIO interrupts also to the first IRQ line.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
 drivers/mmc/host/mmci.c |   67 ++++++++++++++++++++++++++++++++++++++++------
 drivers/mmc/host/mmci.h |    5 +++
 2 files changed, 63 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4eb7265..5baa3bd 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -45,6 +45,7 @@ static unsigned int fmax = 515633;
  *	      is asserted (likewise for RX)
  * @fifohalfsize: number of bytes that can be written when MCI_TXFIFOHALFEMPTY
  *		  is asserted (likewise for RX)
+ * @singleirq: true if only one IRQ line is available
  */
 struct variant_data {
 	unsigned int		clkreg;
@@ -52,6 +53,7 @@ struct variant_data {
 	unsigned int		datalength_bits;
 	unsigned int		fifosize;
 	unsigned int		fifohalfsize;
+	bool			singleirq;
 };
 
 static struct variant_data variant_arm = {
@@ -73,7 +75,9 @@ static struct variant_data variant_ux500 = {
 	.clkreg			= MCI_CLK_ENABLE,
 	.clkreg_enable		= 1 << 14, /* HWFCEN */
 	.datalength_bits	= 24,
+	.singleirq		= true,
 };
+
 /*
  * This must be called with host->lock held
  */
@@ -129,10 +133,27 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 	spin_lock(&host->lock);
 }
 
+static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
+{
+	struct variant_data *variant = host->variant;
+	void __iomem *base = host->base;
+
+	if (variant->singleirq) {
+		unsigned int mask0 = readl(base + MMCIMASK0);
+
+		mask0 &= ~MCI_IRQ1MASK;
+		mask0 |= mask;
+
+		writel(mask0, base + MMCIMASK0);
+	}
+
+	writel(mask, base + MMCIMASK1);
+}
+
 static void mmci_stop_data(struct mmci_host *host)
 {
 	writel(0, host->base + MMCIDATACTRL);
-	writel(0, host->base + MMCIMASK1);
+	mmci_set_mask1(host, 0);
 	host->data = NULL;
 }
 
@@ -198,7 +219,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 
 	writel(datactrl, base + MMCIDATACTRL);
 	writel(readl(base + MMCIMASK0) & ~MCI_DATAENDMASK, base + MMCIMASK0);
-	writel(irqmask, base + MMCIMASK1);
+	mmci_set_mask1(host, irqmask);
 }
 
 static void
@@ -260,6 +281,10 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 	if (status & MCI_DATAEND) {
 		mmci_stop_data(host);
 
+		/* MCI_DATABLOCKEND not used with single irq */
+		if (host->variant->singleirq && !data->error)
+			host->data_xfered = data->blksz * data->blocks;
+
 		if (!data->stop) {
 			mmci_request_end(host, data->mrq);
 		} else {
@@ -418,7 +443,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 * "any data available" mode.
 	 */
 	if (status & MCI_RXACTIVE && host->size < variant->fifosize)
-		writel(MCI_RXDATAAVLBLMASK, base + MMCIMASK1);
+		mmci_set_mask1(host, MCI_RXDATAAVLBLMASK);
 
 	/*
 	 * If we run out of data, disable the data IRQs; this
@@ -427,7 +452,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 * stops us racing with our data end IRQ.
 	 */
 	if (host->size == 0) {
-		writel(0, base + MMCIMASK1);
+		mmci_set_mask1(host, 0);
 		writel(readl(base + MMCIMASK0) | MCI_DATAENDMASK, base + MMCIMASK0);
 	}
 
@@ -440,6 +465,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 static irqreturn_t mmci_irq(int irq, void *dev_id)
 {
 	struct mmci_host *host = dev_id;
+	struct variant_data *variant = host->variant;
 	u32 status;
 	int ret = 0;
 
@@ -450,6 +476,14 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		struct mmc_data *data;
 
 		status = readl(host->base + MMCISTATUS);
+
+		if (variant->singleirq) {
+			if (status & readl(host->base + MMCIMASK1))
+				mmci_pio_irq(irq, dev_id);
+
+			status &= ~MCI_IRQ1MASK;
+		}
+
 		status &= readl(host->base + MMCIMASK0);
 		writel(status, host->base + MMCICLEAR);
 
@@ -610,6 +644,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	struct variant_data *variant = id->data;
 	struct mmci_host *host;
 	struct mmc_host *mmc;
+	unsigned int mask;
 	int ret;
 
 	/* must have platform data */
@@ -782,11 +817,23 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (ret)
 		goto unmap;
 
-	ret = request_irq(dev->irq[1], mmci_pio_irq, IRQF_SHARED, DRIVER_NAME " (pio)", host);
-	if (ret)
-		goto irq0_free;
+	if (!variant->singleirq) {
+		ret = request_irq(dev->irq[1], mmci_pio_irq, IRQF_SHARED,
+				  DRIVER_NAME " (pio)", host);
+		if (ret)
+			goto irq0_free;
+	}
+
+	/*
+	 * MCI_DATABLOCKEND doesn't seem to immediately clear from the status,
+	 * so we can't use it keep count when only one irq is used because the
+	 * irq will hit for other reasons.
+	 */
+	mask = MCI_IRQENABLE;
+	if (variant->singleirq)
+		mask &= ~MCI_DATABLOCKEND;
 
-	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+	writel(mask, host->base + MMCIMASK0);
 
 	amba_set_drvdata(dev, mmc);
 
@@ -830,6 +877,7 @@ static int __devexit mmci_remove(struct amba_device *dev)
 
 	if (mmc) {
 		struct mmci_host *host = mmc_priv(mmc);
+		struct variant_data *variant = host->variant;
 
 		mmc_remove_host(mmc);
 
@@ -840,7 +888,8 @@ static int __devexit mmci_remove(struct amba_device *dev)
 		writel(0, host->base + MMCIDATACTRL);
 
 		free_irq(dev->irq[0], host);
-		free_irq(dev->irq[1], host);
+		if (!variant->singleirq)
+			free_irq(dev->irq[1], host);
 
 		if (host->gpio_wp != -ENOSYS)
 			gpio_free(host->gpio_wp);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index c7d373c..ba203b8 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -133,6 +133,11 @@
 	MCI_DATATIMEOUTMASK|MCI_TXUNDERRUNMASK|MCI_RXOVERRUNMASK|	\
 	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATABLOCKENDMASK)
 
+/* These interrupts are directed to IRQ1 when two IRQ lines are available */
+#define MCI_IRQ1MASK \
+	(MCI_RXFIFOHALFFULLMASK | MCI_RXDATAAVLBLMASK | \
+	 MCI_TXFIFOHALFEMPTYMASK)
+
 #define NR_SG		16
 
 struct clk;
-- 
1.7.0

Re: [PATCH 12/12] mmci: support variants with only one irq

[Linus Walleij]

2010/6/22 Rabin Vincent <rabin.vincent@stericsson.com>:

> The Ux500 variants have only one IRQ line hooked up.  Allow these to
> work by directing the PIO interrupts also to the first IRQ line.

I see this one is dangling in the patch tracker, FYI I have regression
tested this on the PB1176MPcore with its unmodified ARM PL180
MMCI block - read/write files copy mount/unmount - no problems.

Yours,
Linus Walleij

[PATCH 12/12] mmci: support variants with only one irq

[Linus Walleij]

2010/6/22 Rabin Vincent <rabin.vincent@stericsson.com>:

> The Ux500 variants have only one IRQ line hooked up. ?Allow these to
> work by directing the PIO interrupts also to the first IRQ line.

I see this one is dangling in the patch tracker, FYI I have regression
tested this on the PB1176MPcore with its unmodified ARM PL180
MMCI block - read/write files copy mount/unmount - no problems.

Yours,
Linus Walleij

[PATCHv2] mmci: work with only one irq

[Rabin Vincent]

The DBx500 variants have only one IRQ line hooked up.  Allow these (and
any other implementations which choose to use only one irq) to work by
directing the PIO interrupts also to the first IRQ line.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
v2: don't depend on variant, as suggested by Russell in the patch tracker

 drivers/mmc/host/mmci.c |   62 ++++++++++++++++++++++++++++++++++++++++-------
 drivers/mmc/host/mmci.h |    6 ++++
 2 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 87b4fc6..8255333 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -129,10 +129,26 @@ mmci_request_end(struct mmci_host *host, struct mmc_request *mrq)
 	spin_lock(&host->lock);
 }
 
+static void mmci_set_mask1(struct mmci_host *host, unsigned int mask)
+{
+	void __iomem *base = host->base;
+
+	if (host->singleirq) {
+		unsigned int mask0 = readl(base + MMCIMASK0);
+
+		mask0 &= ~MCI_IRQ1MASK;
+		mask0 |= mask;
+
+		writel(mask0, base + MMCIMASK0);
+	}
+
+	writel(mask, base + MMCIMASK1);
+}
+
 static void mmci_stop_data(struct mmci_host *host)
 {
 	writel(0, host->base + MMCIDATACTRL);
-	writel(0, host->base + MMCIMASK1);
+	mmci_set_mask1(host, 0);
 	host->data = NULL;
 }
 
@@ -198,7 +214,7 @@ static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 
 	writel(datactrl, base + MMCIDATACTRL);
 	writel(readl(base + MMCIMASK0) & ~MCI_DATAENDMASK, base + MMCIMASK0);
-	writel(irqmask, base + MMCIMASK1);
+	mmci_set_mask1(host, irqmask);
 }
 
 static void
@@ -276,6 +292,10 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 	if (status & MCI_DATAEND) {
 		mmci_stop_data(host);
 
+		/* MCI_DATABLOCKEND not used with single irq */
+		if (host->singleirq && !data->error)
+			host->data_xfered = data->blksz * data->blocks;
+
 		if (!data->stop) {
 			mmci_request_end(host, data->mrq);
 		} else {
@@ -437,7 +457,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 * "any data available" mode.
 	 */
 	if (status & MCI_RXACTIVE && host->size < variant->fifosize)
-		writel(MCI_RXDATAAVLBLMASK, base + MMCIMASK1);
+		mmci_set_mask1(host, MCI_RXDATAAVLBLMASK);
 
 	/*
 	 * If we run out of data, disable the data IRQs; this
@@ -446,7 +466,7 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 	 * stops us racing with our data end IRQ.
 	 */
 	if (host->size == 0) {
-		writel(0, base + MMCIMASK1);
+		mmci_set_mask1(host, 0);
 		writel(readl(base + MMCIMASK0) | MCI_DATAENDMASK, base + MMCIMASK0);
 	}
 
@@ -469,6 +489,14 @@ static irqreturn_t mmci_irq(int irq, void *dev_id)
 		struct mmc_data *data;
 
 		status = readl(host->base + MMCISTATUS);
+
+		if (host->singleirq) {
+			if (status & readl(host->base + MMCIMASK1))
+				mmci_pio_irq(irq, dev_id);
+
+			status &= ~MCI_IRQ1MASK;
+		}
+
 		status &= readl(host->base + MMCIMASK0);
 		writel(status, host->base + MMCICLEAR);
 
@@ -635,6 +663,7 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	struct variant_data *variant = id->data;
 	struct mmci_host *host;
 	struct mmc_host *mmc;
+	unsigned int mask;
 	int ret;
 
 	/* must have platform data */
@@ -806,11 +835,25 @@ static int __devinit mmci_probe(struct amba_device *dev, struct amba_id *id)
 	if (ret)
 		goto unmap;
 
-	ret = request_irq(dev->irq[1], mmci_pio_irq, IRQF_SHARED, DRIVER_NAME " (pio)", host);
-	if (ret)
-		goto irq0_free;
+	if (dev->irq[1] == NO_IRQ)
+		host->singleirq = true;
+	else {
+		ret = request_irq(dev->irq[1], mmci_pio_irq, IRQF_SHARED,
+				  DRIVER_NAME " (pio)", host);
+		if (ret)
+			goto irq0_free;
+	}
+
+	/*
+	 * MCI_DATABLOCKEND doesn't seem to immediately clear from the status,
+	 * so we can't use it keep count when only one irq is used because the
+	 * irq will hit for other reasons.
+	 */
+	mask = MCI_IRQENABLE;
+	if (host->singleirq)
+		mask &= ~MCI_DATABLOCKEND;
 
-	writel(MCI_IRQENABLE, host->base + MMCIMASK0);
+	writel(mask, host->base + MMCIMASK0);
 
 	amba_set_drvdata(dev, mmc);
 
@@ -864,7 +907,8 @@ static int __devexit mmci_remove(struct amba_device *dev)
 		writel(0, host->base + MMCIDATACTRL);
 
 		free_irq(dev->irq[0], host);
-		free_irq(dev->irq[1], host);
+		if (!host->singleirq)
+			free_irq(dev->irq[1], host);
 
 		if (host->gpio_wp != -ENOSYS)
 			gpio_free(host->gpio_wp);
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index c7d373c..b4fdd65 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -133,6 +133,11 @@
 	MCI_DATATIMEOUTMASK|MCI_TXUNDERRUNMASK|MCI_RXOVERRUNMASK|	\
 	MCI_CMDRESPENDMASK|MCI_CMDSENTMASK|MCI_DATABLOCKENDMASK)
 
+/* These interrupts are directed to IRQ1 when two IRQ lines are available */
+#define MCI_IRQ1MASK \
+	(MCI_RXFIFOHALFFULLMASK | MCI_RXDATAAVLBLMASK | \
+	 MCI_TXFIFOHALFEMPTYMASK)
+
 #define NR_SG		16
 
 struct clk;
@@ -148,6 +153,7 @@ struct mmci_host {
 	int			gpio_cd;
 	int			gpio_wp;
 	int			gpio_cd_irq;
+	bool			singleirq;
 
 	unsigned int		data_xfered;
 
-- 
1.7.1

[PATCHv2] mmci: work with only one irq

[Linus Walleij]

2010/10/11 Rabin Vincent <rabin.vincent@stericsson.com>:

> The DBx500 variants have only one IRQ line hooked up. ?Allow these (and
> any other implementations which choose to use only one irq) to work by
> directing the PIO interrupts also to the first IRQ line.
> (...)
> + ? ? ? ? ? ? ? /* MCI_DATABLOCKEND not used with single irq */
> + ? ? ? ? ? ? ? if (host->singleirq && !data->error)
> + ? ? ? ? ? ? ? ? ? ? ? host->data_xfered = data->blksz * data->blocks;
> +

I replaced that last line with:
host->data_xfered += data->blksz * data->blocks;

In my follow-on "support out-of-order IRQ events" patch, does it make sense?

Once this is in the patch system I'll rebase that patch too...

Yours,
Linus Walleij

[PATCHv2] mmci: work with only one irq

[Russell King - ARM Linux]

On Mon, Oct 11, 2010 at 05:36:41AM +0530, Rabin Vincent wrote:
> @@ -276,6 +292,10 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
>  	if (status & MCI_DATAEND) {
>  		mmci_stop_data(host);
>  
> +		/* MCI_DATABLOCKEND not used with single irq */
> +		if (host->singleirq && !data->error)
> +			host->data_xfered = data->blksz * data->blocks;
> +

I'm uneasy about this.  If we don't get datablockend interrupts, then
when there's an error we don't know how much data was transferred, and
with big requests, we're going to have a very long time spent retrying
block by block.

[PATCHv2] mmci: work with only one irq

[Linus Walleij]

2010/10/11 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Mon, Oct 11, 2010 at 05:36:41AM +0530, Rabin Vincent wrote:
>> @@ -276,6 +292,10 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
>> ? ? ? if (status & MCI_DATAEND) {
>> ? ? ? ? ? ? ? mmci_stop_data(host);
>>
>> + ? ? ? ? ? ? /* MCI_DATABLOCKEND not used with single irq */
>> + ? ? ? ? ? ? if (host->singleirq && !data->error)
>> + ? ? ? ? ? ? ? ? ? ? host->data_xfered = data->blksz * data->blocks;
>> +
>
> I'm uneasy about this. ?If we don't get datablockend interrupts, then
> when there's an error we don't know how much data was transferred, and
> with big requests, we're going to have a very long time spent retrying
> block by block.

This one should probably remain a ux500-specific flag, unlike the single
IRQ flag, this is something else.

AFAICT the DBCKEND signal is simply not routed in the ux500
(probably neither on the Nomadik) variant, so we can't wait for it.

On the U300 on the other hand, it is routed and available, if and
only if you're using PIO mode. (However it can arrive *after* the
DATAEND signal, which the other pending patch takes care of).

During DMA this signal is disfunct also in U300...

Yours,
Linus Walleij

[PATCHv2] mmci: work with only one irq

[Linus Walleij]

2010/10/11 Russell King - ARM Linux <linux@arm.linux.org.uk>:
> On Mon, Oct 11, 2010 at 05:36:41AM +0530, Rabin Vincent wrote:
>> @@ -276,6 +292,10 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
>> ? ? ? if (status & MCI_DATAEND) {
>> ? ? ? ? ? ? ? mmci_stop_data(host);
>>
>> + ? ? ? ? ? ? /* MCI_DATABLOCKEND not used with single irq */
>> + ? ? ? ? ? ? if (host->singleirq && !data->error)
>> + ? ? ? ? ? ? ? ? ? ? host->data_xfered = data->blksz * data->blocks;
>> +
>
> I'm uneasy about this. ?If we don't get datablockend interrupts, then
> when there's an error we don't know how much data was transferred, and
> with big requests, we're going to have a very long time spent retrying
> block by block.

I have sent a respin of Rabins patch (he's on vacation, so I got his
permission to fix this up) which removes this offending handling of
MCI_DATABLOCKEND, it's now also in the patch tracker as 6311/2.

On top of this I have updated my patch for out-of-order IRQ arrival
to cover also this case and renamed it
"handle broken MCI_DATABLOCKEND hardware"
it will do away with the confusing U300 #ifdef and adds a comment
block that explains what the problem really is on the Ux hardwares,
basically that under some circumstances in the U300 and under
all circumstances in the Ux500, the MCI_DATABLOCKEND flag
is totally unreliable and "hangs" at 1.

Yours,
Linus Walleij

Re: [PATCH 01/12] mmci: use sg_miter API to fix multi-page sg handling

[Linus Walleij]

I've tested this MMCI patch set on RealView PB1176 and
U300, mounted cards, copied files back and forth, unmount
diff files. No problems.

This should cover all MMCI variants, since the PB1176 is a
vanilla ARM variant.

Just fix the comments on patch #7 and it should be ripe for
the patch tracker methinks.

Yours,
Linus Walleij

[PATCH 01/12] mmci: use sg_miter API to fix multi-page sg handling

[Linus Walleij]

I've tested this MMCI patch set on RealView PB1176 and
U300, mounted cards, copied files back and forth, unmount
diff files. No problems.

This should cover all MMCI variants, since the PB1176 is a
vanilla ARM variant.

Just fix the comments on patch #7 and it should be ripe for
the patch tracker methinks.

Yours,
Linus Walleij

[PATCHv2 01/12] mmci: use sg_miter API to fix multi-page sg handling

[Rabin Vincent]

The mmci driver's SG list iteration logic assumes that each SG entry
spans only one page, and only maps and flushes one page of the sg.  This
is not a valid assumption.  Fix it by converting the driver to the
sg_miter API, which correctly handles sgs which span multiple pages.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
v2:
 - Keep the flush_dcache_page() inside the driver, since the
   flush_kernel_dcache_page() call inside the sg_miter API is inadequate

 drivers/mmc/host/mmci.c |   60 +++++++++++++++++++++++++++++-----------------
 drivers/mmc/host/mmci.h |   35 +--------------------------
 2 files changed, 39 insertions(+), 56 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4917af9..d63d756 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -26,7 +26,6 @@
 #include <linux/amba/mmci.h>
 #include <linux/regulator/consumer.h>
 
-#include <asm/cacheflush.h>
 #include <asm/div64.h>
 #include <asm/io.h>
 #include <asm/sizes.h>
@@ -98,6 +97,18 @@ static void mmci_stop_data(struct mmci_host *host)
 	host->data = NULL;
 }
 
+static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
+{
+	unsigned int flags = SG_MITER_ATOMIC;
+
+	if (data->flags & MMC_DATA_READ)
+		flags |= SG_MITER_TO_SG;
+	else
+		flags |= SG_MITER_FROM_SG;
+
+	sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags);
+}
+
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
 	unsigned int datactrl, timeout, irqmask;
@@ -210,8 +221,17 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		 * We hit an error condition.  Ensure that any data
 		 * partially written to a page is properly coherent.
 		 */
-		if (host->sg_len && data->flags & MMC_DATA_READ)
-			flush_dcache_page(sg_page(host->sg_ptr));
+		if (data->flags & MMC_DATA_READ) {
+			struct sg_mapping_iter *sg_miter = &host->sg_miter;
+			unsigned long flags;
+
+			local_irq_save(flags);
+			if (sg_miter_next(sg_miter)) {
+				flush_dcache_page(sg_miter->page);
+				sg_miter_stop(sg_miter);
+			}
+			local_irq_restore(flags);
+		}
 	}
 	if (status & MCI_DATAEND) {
 		mmci_stop_data(host);
@@ -314,15 +334,18 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem
 static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 {
 	struct mmci_host *host = dev_id;
+	struct sg_mapping_iter *sg_miter = &host->sg_miter;
 	void __iomem *base = host->base;
+	unsigned long flags;
 	u32 status;
 
 	status = readl(base + MMCISTATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "irq1 (pio) %08x\n", status);
 
+	local_irq_save(flags);
+
 	do {
-		unsigned long flags;
 		unsigned int remain, len;
 		char *buffer;
 
@@ -336,11 +359,11 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (!(status & (MCI_TXFIFOHALFEMPTY|MCI_RXDATAAVLBL)))
 			break;
 
-		/*
-		 * Map the current scatter buffer.
-		 */
-		buffer = mmci_kmap_atomic(host, &flags) + host->sg_off;
-		remain = host->sg_ptr->length - host->sg_off;
+		if (!sg_miter_next(sg_miter))
+			break;
+
+		buffer = sg_miter->addr;
+		remain = sg_miter->length;
 
 		len = 0;
 		if (status & MCI_RXACTIVE)
@@ -348,31 +371,24 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (status & MCI_TXACTIVE)
 			len = mmci_pio_write(host, buffer, remain, status);
 
-		/*
-		 * Unmap the buffer.
-		 */
-		mmci_kunmap_atomic(host, buffer, &flags);
+		sg_miter->consumed = len;
 
-		host->sg_off += len;
 		host->size -= len;
 		remain -= len;
 
 		if (remain)
 			break;
 
-		/*
-		 * If we were reading, and we have completed this
-		 * page, ensure that the data cache is coherent.
-		 */
 		if (status & MCI_RXACTIVE)
-			flush_dcache_page(sg_page(host->sg_ptr));
-
-		if (!mmci_next_sg(host))
-			break;
+			flush_dcache_page(sg_miter->page);
 
 		status = readl(base + MMCISTATUS);
 	} while (1);
 
+	sg_miter_stop(sg_miter);
+
+	local_irq_restore(flags);
+
 	/*
 	 * If we're nearing the end of the read, switch to
 	 * "any data available" mode.
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d77062e..7cb24ab 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -171,42 +171,9 @@ struct mmci_host {
 	struct timer_list	timer;
 	unsigned int		oldstat;
 
-	unsigned int		sg_len;
-
 	/* pio stuff */
-	struct scatterlist	*sg_ptr;
-	unsigned int		sg_off;
+	struct sg_mapping_iter	sg_miter;
 	unsigned int		size;
 	struct regulator	*vcc;
 };
 
-static inline void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
-{
-	/*
-	 * Ideally, we want the higher levels to pass us a scatter list.
-	 */
-	host->sg_len = data->sg_len;
-	host->sg_ptr = data->sg;
-	host->sg_off = 0;
-}
-
-static inline int mmci_next_sg(struct mmci_host *host)
-{
-	host->sg_ptr++;
-	host->sg_off = 0;
-	return --host->sg_len;
-}
-
-static inline char *mmci_kmap_atomic(struct mmci_host *host, unsigned long *flags)
-{
-	struct scatterlist *sg = host->sg_ptr;
-
-	local_irq_save(*flags);
-	return kmap_atomic(sg_page(sg), KM_BIO_SRC_IRQ) + sg->offset;
-}
-
-static inline void mmci_kunmap_atomic(struct mmci_host *host, void *buffer, unsigned long *flags)
-{
-	kunmap_atomic(buffer, KM_BIO_SRC_IRQ);
-	local_irq_restore(*flags);
-}
-- 
1.7.0

[PATCHv2 01/12] mmci: use sg_miter API to fix multi-page sg handling

[Rabin Vincent]

The mmci driver's SG list iteration logic assumes that each SG entry
spans only one page, and only maps and flushes one page of the sg.  This
is not a valid assumption.  Fix it by converting the driver to the
sg_miter API, which correctly handles sgs which span multiple pages.

Acked-by: Linus Walleij <linus.walleij@stericsson.com>
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
v2:
 - Keep the flush_dcache_page() inside the driver, since the
   flush_kernel_dcache_page() call inside the sg_miter API is inadequate

 drivers/mmc/host/mmci.c |   60 +++++++++++++++++++++++++++++-----------------
 drivers/mmc/host/mmci.h |   35 +--------------------------
 2 files changed, 39 insertions(+), 56 deletions(-)

diff --git a/drivers/mmc/host/mmci.c b/drivers/mmc/host/mmci.c
index 4917af9..d63d756 100644
--- a/drivers/mmc/host/mmci.c
+++ b/drivers/mmc/host/mmci.c
@@ -26,7 +26,6 @@
 #include <linux/amba/mmci.h>
 #include <linux/regulator/consumer.h>
 
-#include <asm/cacheflush.h>
 #include <asm/div64.h>
 #include <asm/io.h>
 #include <asm/sizes.h>
@@ -98,6 +97,18 @@ static void mmci_stop_data(struct mmci_host *host)
 	host->data = NULL;
 }
 
+static void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
+{
+	unsigned int flags = SG_MITER_ATOMIC;
+
+	if (data->flags & MMC_DATA_READ)
+		flags |= SG_MITER_TO_SG;
+	else
+		flags |= SG_MITER_FROM_SG;
+
+	sg_miter_start(&host->sg_miter, data->sg, data->sg_len, flags);
+}
+
 static void mmci_start_data(struct mmci_host *host, struct mmc_data *data)
 {
 	unsigned int datactrl, timeout, irqmask;
@@ -210,8 +221,17 @@ mmci_data_irq(struct mmci_host *host, struct mmc_data *data,
 		 * We hit an error condition.  Ensure that any data
 		 * partially written to a page is properly coherent.
 		 */
-		if (host->sg_len && data->flags & MMC_DATA_READ)
-			flush_dcache_page(sg_page(host->sg_ptr));
+		if (data->flags & MMC_DATA_READ) {
+			struct sg_mapping_iter *sg_miter = &host->sg_miter;
+			unsigned long flags;
+
+			local_irq_save(flags);
+			if (sg_miter_next(sg_miter)) {
+				flush_dcache_page(sg_miter->page);
+				sg_miter_stop(sg_miter);
+			}
+			local_irq_restore(flags);
+		}
 	}
 	if (status & MCI_DATAEND) {
 		mmci_stop_data(host);
@@ -314,15 +334,18 @@ static int mmci_pio_write(struct mmci_host *host, char *buffer, unsigned int rem
 static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 {
 	struct mmci_host *host = dev_id;
+	struct sg_mapping_iter *sg_miter = &host->sg_miter;
 	void __iomem *base = host->base;
+	unsigned long flags;
 	u32 status;
 
 	status = readl(base + MMCISTATUS);
 
 	dev_dbg(mmc_dev(host->mmc), "irq1 (pio) %08x\n", status);
 
+	local_irq_save(flags);
+
 	do {
-		unsigned long flags;
 		unsigned int remain, len;
 		char *buffer;
 
@@ -336,11 +359,11 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (!(status & (MCI_TXFIFOHALFEMPTY|MCI_RXDATAAVLBL)))
 			break;
 
-		/*
-		 * Map the current scatter buffer.
-		 */
-		buffer = mmci_kmap_atomic(host, &flags) + host->sg_off;
-		remain = host->sg_ptr->length - host->sg_off;
+		if (!sg_miter_next(sg_miter))
+			break;
+
+		buffer = sg_miter->addr;
+		remain = sg_miter->length;
 
 		len = 0;
 		if (status & MCI_RXACTIVE)
@@ -348,31 +371,24 @@ static irqreturn_t mmci_pio_irq(int irq, void *dev_id)
 		if (status & MCI_TXACTIVE)
 			len = mmci_pio_write(host, buffer, remain, status);
 
-		/*
-		 * Unmap the buffer.
-		 */
-		mmci_kunmap_atomic(host, buffer, &flags);
+		sg_miter->consumed = len;
 
-		host->sg_off += len;
 		host->size -= len;
 		remain -= len;
 
 		if (remain)
 			break;
 
-		/*
-		 * If we were reading, and we have completed this
-		 * page, ensure that the data cache is coherent.
-		 */
 		if (status & MCI_RXACTIVE)
-			flush_dcache_page(sg_page(host->sg_ptr));
-
-		if (!mmci_next_sg(host))
-			break;
+			flush_dcache_page(sg_miter->page);
 
 		status = readl(base + MMCISTATUS);
 	} while (1);
 
+	sg_miter_stop(sg_miter);
+
+	local_irq_restore(flags);
+
 	/*
 	 * If we're nearing the end of the read, switch to
 	 * "any data available" mode.
diff --git a/drivers/mmc/host/mmci.h b/drivers/mmc/host/mmci.h
index d77062e..7cb24ab 100644
--- a/drivers/mmc/host/mmci.h
+++ b/drivers/mmc/host/mmci.h
@@ -171,42 +171,9 @@ struct mmci_host {
 	struct timer_list	timer;
 	unsigned int		oldstat;
 
-	unsigned int		sg_len;
-
 	/* pio stuff */
-	struct scatterlist	*sg_ptr;
-	unsigned int		sg_off;
+	struct sg_mapping_iter	sg_miter;
 	unsigned int		size;
 	struct regulator	*vcc;
 };
 
-static inline void mmci_init_sg(struct mmci_host *host, struct mmc_data *data)
-{
-	/*
-	 * Ideally, we want the higher levels to pass us a scatter list.
-	 */
-	host->sg_len = data->sg_len;
-	host->sg_ptr = data->sg;
-	host->sg_off = 0;
-}
-
-static inline int mmci_next_sg(struct mmci_host *host)
-{
-	host->sg_ptr++;
-	host->sg_off = 0;
-	return --host->sg_len;
-}
-
-static inline char *mmci_kmap_atomic(struct mmci_host *host, unsigned long *flags)
-{
-	struct scatterlist *sg = host->sg_ptr;
-
-	local_irq_save(*flags);
-	return kmap_atomic(sg_page(sg), KM_BIO_SRC_IRQ) + sg->offset;
-}
-
-static inline void mmci_kunmap_atomic(struct mmci_host *host, void *buffer, unsigned long *flags)
-{
-	kunmap_atomic(buffer, KM_BIO_SRC_IRQ);
-	local_irq_restore(*flags);
-}
-- 
1.7.0