Patchwork [10/11] sdhci: Add quirk for Freescale eSDHC controllers

login
register
mail settings
Submitter Anton Vorontsov
Date Feb. 6, 2009, 6:07 p.m.
Message ID <20090206180701.GJ11548@oksana.dev.rtsoft.ru>
Download mbox | patch
Permalink /patch/22400/
State Superseded
Delegated to: Kumar Gala
Headers show

Comments

Anton Vorontsov - Feb. 6, 2009, 6:07 p.m.
This patch adds SDHCI_QUIRK_FSL quirk. The quirk is used to instruct
the sdhci driver about various FSL eSDHC host incompatibilities:

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

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

2) sdhci_init() is needed after error conditions.

   (Can we safely do this for all controllers?)

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

Signed-off-by: Anton Vorontsov <avorontsov@ru.mvista.com>
---
 drivers/mmc/host/sdhci.c |   36 ++++++++++++++++++++++++++++++------
 drivers/mmc/host/sdhci.h |    3 +++
 2 files changed, 33 insertions(+), 6 deletions(-)
Pierre Ossman - Feb. 8, 2009, 9:12 p.m.
On Fri, 6 Feb 2009 21:07:01 +0300
Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

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

No device quirks please. They should be for specific bugs, not lumping
things together like this. Otherwise we'll soon have an unmanageable
mess.

> 1) FSL eSDHC controllers can support maximum block size up to 4096
>    bytes. The MBL (Maximum Block Length) field in the capabilities
>    register extended by one bit.
> 
>    (Should we implement a dedicated quirk for this? I.e.
>     SDHCI_QUIRK_MAX_BLK_SZ_4096?)
> 

Yes please. It would have to mean "always support 4096" though, not
"turn reserved bit 18 into a block length bit".

> 2) sdhci_init() is needed after error conditions.
> 
>    (Can we safely do this for all controllers?)
> 

Please investigate which part of sdhci_init() is needed. How does it
break without this?

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

Please try to investigate more, but if you cannot improve it further
then a specific quirk can be added.

Rgds
Anton Vorontsov - Feb. 13, 2009, 2:42 p.m.
On Sun, Feb 08, 2009 at 10:12:09PM +0100, Pierre Ossman wrote:
> On Fri, 6 Feb 2009 21:07:01 +0300
> Anton Vorontsov <avorontsov@ru.mvista.com> wrote:
> 
> > This patch adds SDHCI_QUIRK_FSL quirk. The quirk is used to instruct
> > the sdhci driver about various FSL eSDHC host incompatibilities:
> > 
> 
> No device quirks please. They should be for specific bugs, not lumping
> things together like this. Otherwise we'll soon have an unmanageable
> mess.

OK.

> > 1) FSL eSDHC controllers can support maximum block size up to 4096
> >    bytes. The MBL (Maximum Block Length) field in the capabilities
> >    register extended by one bit.
> > 
> >    (Should we implement a dedicated quirk for this? I.e.
> >     SDHCI_QUIRK_MAX_BLK_SZ_4096?)
> > 
> 
> Yes please. It would have to mean "always support 4096" though, not
> "turn reserved bit 18 into a block length bit".

OK.

> > 2) sdhci_init() is needed after error conditions.
> > 
> >    (Can we safely do this for all controllers?)
> > 
> 
> Please investigate which part of sdhci_init() is needed. How does it
> break without this?

After reset eSDHC lose signal/interrupt enable states:

Before reset:

sdhci: ============== REGISTER DUMP ==============
sdhci: Sys addr: 0x00000008 | Version:  0x00000000
sdhci: Blk size: 0x00000008 | Blk cnt:  0x00000000
sdhci: Argument: 0x000001aa | Trn mode: 0x00000000
sdhci: Present:  0xff850000 | Host ctl: 0x00000021
sdhci: Power:    0x00000000 | Blk gap:  0x00000000
sdhci: Wake-up:  0x00000000 | Clock:    0x00001077
sdhci: Timeout:  0x00000000 | Int stat: 0x00000000
sdhci: Int enab: 0x007f0003 | Sig enab: 0x007f0003
sdhci: AC12 err: 0x00000000 | Slot int: 0x00000001
sdhci: Caps:     0x01e30000 | Max curr: 0x00000000
sdhci: ===========================================

after sdhci_reset(host, SDHCI_RESET_CMD):

sdhci: ============== REGISTER DUMP ==============
sdhci: Sys addr: 0x00000008 | Version:  0x00000000
sdhci: Blk size: 0x00000008 | Blk cnt:  0x00000000
sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
sdhci: Present:  0xff850000 | Host ctl: 0x00000021
sdhci: Power:    0x00000000 | Blk gap:  0x00000000
sdhci: Wake-up:  0x00000000 | Clock:    0x00001077
sdhci: Timeout:  0x00000000 | Int stat: 0x00000000
sdhci: Int enab: 0x017f0003 | Sig enab: 0x00700002
sdhci: AC12 err: 0x00000000 | Slot int: 0x00000001
sdhci: Caps:     0x01e30000 | Max curr: 0x00000000
sdhci: ===========================================

After sdhci_reset(host, SDHCI_RESET_DATA):

sdhci: ============== REGISTER DUMP ==============
sdhci: Sys addr: 0x00000008 | Version:  0x00000000
sdhci: Blk size: 0x00000008 | Blk cnt:  0x00000000
sdhci: Argument: 0x00000000 | Trn mode: 0x00000000
sdhci: Present:  0xff850000 | Host ctl: 0x00000021
sdhci: Power:    0x00000000 | Blk gap:  0x00000000
sdhci: Wake-up:  0x00000000 | Clock:    0x00001077
sdhci: Timeout:  0x00000000 | Int stat: 0x00000000
sdhci: Int enab: 0x117f003f | Sig enab: 0x00000000
sdhci: AC12 err: 0x00000000 | Slot int: 0x00000001
sdhci: Caps:     0x01e30000 | Max curr: 0x00000000
sdhci: ===========================================

> > 3) Small udelay is needed to make eSDHC work in PIO mode. Without
> >    the delay reading causes endless interrupt storm, and writing
> >    corrupts data. The first guess would be that we must wait for
> >    some bit in some register, but I didn't find any reliable bits
> >    that changes before and after the delay. Though, more investigation
> >    on this is in my todo list.
> 
> Please try to investigate more, but if you cannot improve it further
> then a specific quirk can be added.

No luck so far... :-/


Thanks,

Patch

diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
index 3c1f1d5..87ba0ed 100644
--- a/drivers/mmc/host/sdhci.c
+++ b/drivers/mmc/host/sdhci.c
@@ -328,6 +328,10 @@  static void sdhci_transfer_pio(struct sdhci_host *host)
 		mask = ~0;
 
 	while (sdhci_readl(host, SDHCI_PRESENT_STATE) & mask) {
+		/* FIXME */
+		if (host->quirks & SDHCI_QUIRK_FSL)
+			udelay(100);
+
 		if (host->data->flags & MMC_DATA_READ)
 			sdhci_read_block_pio(host);
 		else
@@ -625,6 +629,7 @@  static void sdhci_set_pio_irqs(struct sdhci_host *host, bool state)
 
 static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 {
+	u16 blksz;
 	u8 count;
 	u8 ctrl;
 	int ret;
@@ -775,7 +780,12 @@  static void sdhci_prepare_data(struct sdhci_host *host, struct mmc_data *data)
 	}
 
 	/* We do not handle DMA boundaries, so set it to max (512 KiB) */
-	sdhci_writew(host, SDHCI_MAKE_BLKSZ(7, data->blksz), SDHCI_BLOCK_SIZE);
+	if (host->quirks & SDHCI_QUIRK_FSL)
+		blksz = data->blksz;
+	else
+		blksz = SDHCI_MAKE_BLKSZ(7, data->blksz);
+
+	sdhci_writew(host, blksz, SDHCI_BLOCK_SIZE);
 	sdhci_writew(host, data->blocks, SDHCI_BLOCK_COUNT);
 }
 
@@ -1280,6 +1290,9 @@  static void sdhci_tasklet_finish(unsigned long param)
 		   controllers do not like that. */
 		sdhci_reset(host, SDHCI_RESET_CMD);
 		sdhci_reset(host, SDHCI_RESET_DATA);
+		/* FSL controllers need this. */
+		if (host->quirks & SDHCI_QUIRK_FSL)
+			sdhci_init(host);
 	}
 
 	host->mrq = NULL;
@@ -1789,13 +1802,24 @@  int sdhci_add_host(struct sdhci_host *host)
 	 * Maximum block size. This varies from controller to controller and
 	 * is specified in the capabilities register.
 	 */
-	mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >> SDHCI_MAX_BLOCK_SHIFT;
-	if (mmc->max_blk_size >= 3) {
+	if (host->quirks & SDHCI_QUIRK_FSL) {
+		mmc->max_blk_size = (caps & SDHCI_FSL_MAX_BLOCK_MASK) >>
+				SDHCI_MAX_BLOCK_SHIFT;
+		if (mmc->max_blk_size >= 4)
+			mmc->max_blk_size = ~0;
+	} else {
+		mmc->max_blk_size = (caps & SDHCI_MAX_BLOCK_MASK) >>
+				SDHCI_MAX_BLOCK_SHIFT;
+		if (mmc->max_blk_size >= 3)
+			mmc->max_blk_size = ~0;
+	}
+
+	if (mmc->max_blk_size == ~0) {
 		printk(KERN_WARNING "%s: Invalid maximum block size, "
 			"assuming 512 bytes\n", mmc_hostname(mmc));
-		mmc->max_blk_size = 512;
-	} else
-		mmc->max_blk_size = 512 << mmc->max_blk_size;
+		mmc->max_blk_size = 0;
+	}
+	mmc->max_blk_size = 512 << mmc->max_blk_size;
 
 	/*
 	 * Maximum block count.
diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h
index 497276b..cb575f3 100644
--- a/drivers/mmc/host/sdhci.h
+++ b/drivers/mmc/host/sdhci.h
@@ -152,6 +152,7 @@  static inline int sdhci_off(sdhci_reg_t reg)
 #define  SDHCI_CLOCK_BASE_MASK	0x00003F00
 #define  SDHCI_CLOCK_BASE_SHIFT	8
 #define  SDHCI_MAX_BLOCK_MASK	0x00030000
+#define  SDHCI_FSL_MAX_BLOCK_MASK 0x00070000
 #define  SDHCI_MAX_BLOCK_SHIFT  16
 #define  SDHCI_CAN_DO_ADMA2	0x00080000
 #define  SDHCI_CAN_DO_ADMA1	0x00100000
@@ -237,6 +238,8 @@  struct sdhci_host {
 #define SDHCI_QUIRK_32BIT_REGISTERS			(1<<18)
 /* Controller issues PIO interrupts during DMA transfers */
 #define SDHCI_QUIRK_PIO_IRQS_DURING_DMA			(1<<19)
+/* Controller is Freescale eSDHC */
+#define SDHCI_QUIRK_FSL					(1<<20)
 
 	int			irq;		/* Device IRQ */
 	void __iomem *		ioaddr;		/* Mapped address */