[PATCH 1/6] Unified AVR32/AT91 MCI Driver

[PATCH 1/6] Unified AVR32/AT91 MCI Driver

[Rob Emanuele]

Unification of the atmel-mci driver to support the AT91 processors MCI
interface.  The atmel-mci driver currently supports the AVR32 and this
patch adds AT91 support.

To use this new driver on a at91 the platform driver for your board
needs to updated.  See the following patch for an example of how to do
that.

Please read the whole set, try it out, and comment.

Thank you,

Rob Emanuele

---
 drivers/mmc/host/Kconfig     |   16 ++++++++++++----
 drivers/mmc/host/atmel-mci.c |   33 +++++++++++++++++++++++++++++----
 2 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index b4cf691..6e2f52b 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -124,6 +124,12 @@ config MMC_AU1X

 	  If unsure, say N.

+choice
+	prompt "Atmel MMC Driver"
+	default MMC_ATMELMCI
+	help
+	  Choose which driver to use for the Atmel MCI Silicon
+
 config MMC_AT91
 	tristate "AT91 SD/MMC Card Interface support"
 	depends on ARCH_AT91
@@ -134,17 +140,19 @@ config MMC_AT91

 config MMC_ATMELMCI
 	tristate "Atmel Multimedia Card Interface support"
-	depends on AVR32
+	depends on AVR32 || ARCH_AT91
 	help
 	  This selects the Atmel Multimedia Card Interface driver. If
-	  you have an AT32 (AVR32) platform with a Multimedia Card
-	  slot, say Y or M here.
+	  you have an AT32 (AVR32) or AT91 platform with a Multimedia
+	  Card slot, say Y or M here.

 	  If unsure, say N.

+endchoice
+
 config MMC_ATMELMCI_DMA
 	bool "Atmel MCI DMA support (EXPERIMENTAL)"
-	depends on MMC_ATMELMCI && DMA_ENGINE && EXPERIMENTAL
+	depends on MMC_ATMELMCI && AVR32 && DMA_ENGINE && EXPERIMENTAL
 	help
 	  Say Y here to have the Atmel MCI driver use a DMA engine to
 	  do data transfers and thus increase the throughput and
diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
index cf6a100..497dd51 100644
--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -2,11 +2,13 @@
  * Atmel MultiMedia Card Interface driver
  *
  * Copyright (C) 2004-2008 Atmel Corporation
+ * Copyright (C) 2009 Rob Emanuele
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 2 as
  * published by the Free Software Foundation.
  */
+
 #include <linux/blkdev.h>
 #include <linux/clk.h>
 #include <linux/debugfs.h>
@@ -30,11 +32,14 @@
 #include <asm/io.h>
 #include <asm/unaligned.h>

+#include <mach/cpu.h>
 #include <mach/board.h>

 #include "atmel-mci-regs.h"

-#define ATMCI_DATA_ERROR_FLAGS	(MCI_DCRCE | MCI_DTOE | MCI_OVRE | MCI_UNRE)
+#define ATMCI_DATA_ERROR_FLAGS	(MCI_RINDE | MCI_RDIRE | MCI_RCRCE  \
+				 | MCI_RENDE | MCI_RTOE | MCI_DCRCE \
+				 | MCI_DTOE | MCI_OVRE | MCI_UNRE)
 #define ATMCI_DMA_THRESHOLD	16

 enum {
@@ -208,6 +213,18 @@ struct atmel_mci_slot {
 	set_bit(event, &host->pending_events)

 /*
+ * Enable or disable features/registers based on
+ * whether the processor supports them
+ */
+static bool mci_has_rwproof(void)
+{
+	if (cpu_is_at91sam9261() || cpu_is_at91rm9200())
+		return false;
+	else
+		return true;
+}
+
+/*
  * The debugfs stuff below is mostly optimized away when
  * CONFIG_DEBUG_FS is not set.
  */
@@ -274,8 +291,12 @@ static void atmci_show_status_reg(struct seq_file *s,
 		[3]	= "BLKE",
 		[4]	= "DTIP",
 		[5]	= "NOTBUSY",
+		[6]	= "ENDRX",
+		[7]	= "ENDTX",
 		[8]	= "SDIOIRQA",
 		[9]	= "SDIOIRQB",
+		[14]	= "RXBUFF",
+		[15]	= "TXBUFE",
 		[16]	= "RINDE",
 		[17]	= "RDIRE",
 		[18]	= "RCRCE",
@@ -847,13 +868,15 @@ static void atmci_set_ios(struct mmc_host *mmc,
struct mmc_ios *ios)
 			clkdiv = 255;
 		}

+		host->mode_reg = MCI_MR_CLKDIV(clkdiv);
+
 		/*
 		 * WRPROOF and RDPROOF prevent overruns/underruns by
 		 * stopping the clock when the FIFO is full/empty.
 		 * This state is not expected to last for long.
 		 */
-		host->mode_reg = MCI_MR_CLKDIV(clkdiv) | MCI_MR_WRPROOF
-					| MCI_MR_RDPROOF;
+		if (mci_has_rwproof())
+			host->mode_reg |= (MCI_MR_WRPROOF | MCI_MR_RDPROOF);

 		if (list_empty(&host->queue))
 			mci_writel(host, MR, host->mode_reg);
@@ -1642,8 +1665,10 @@ static int __init atmci_probe(struct
platform_device *pdev)
 			nr_slots++;
 	}

-	if (!nr_slots)
+	if (!nr_slots) {
+		printk(KERN_ERR "Atmel MCI controller init failed.  atmci_init_slot
error or no slots with bus_width > 0.\n");
 		goto err_init_slot;
+	}

 	dev_info(&pdev->dev,
 			"Atmel MCI controller at 0x%08lx irq %d, %u slots\n",

Re: [PATCH 1/6] Unified AVR32/AT91 MCI Driver

[Nicolas Ferre]

Hi,

First of all thank you for this splitting effort. It is definitively the
good way of presenting things and ease reading a lot.

Rob Emanuele :
> Unification of the atmel-mci driver to support the AT91 processors MCI
> interface.  The atmel-mci driver currently supports the AVR32 and this
> patch adds AT91 support.
> 
> To use this new driver on a at91 the platform driver for your board
> needs to updated.  See the following patch for an example of how to do
> that.
> 
> Please read the whole set, try it out, and comment.

Ok, I try to make my comments. I try to collect your ideas and sometimes
submit modified patches. Comment them back if the modifications I make
to your code seams no so good to you.

> ---
>  drivers/mmc/host/Kconfig     |   16 ++++++++++++----
>  drivers/mmc/host/atmel-mci.c |   33 +++++++++++++++++++++++++++++----
>  2 files changed, 41 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index b4cf691..6e2f52b 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -124,6 +124,12 @@ config MMC_AU1X
> 
>  	  If unsure, say N.
> 
> +choice
> +	prompt "Atmel MMC Driver"
> +	default MMC_ATMELMCI
> +	help
> +	  Choose which driver to use for the Atmel MCI Silicon

I suggest :

diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
index b4cf691..e779049 100644
--- a/drivers/mmc/host/Kconfig
+++ b/drivers/mmc/host/Kconfig
@@ -124,6 +124,12 @@ config MMC_AU1X

 	  If unsure, say N.

+choice
+	prompt "Atmel SD/MMC Driver"
+	default MMC_ATMELMCI if AVR32
+	help
+	  Choose which driver to use for the Atmel MCI Silicon
+
 config MMC_AT91
 	tristate "AT91 SD/MMC Card Interface support"
 	depends on ARCH_AT91

It will keep present default configuration for both AVR32 and AT91.

[..]

> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index cf6a100..497dd51 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -2,11 +2,13 @@
>   * Atmel MultiMedia Card Interface driver
>   *
>   * Copyright (C) 2004-2008 Atmel Corporation
> + * Copyright (C) 2009 Rob Emanuele

I think you can add your copyright on a file that you create or copy and
modify from another file.

>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License version 2 as
>   * published by the Free Software Foundation.
>   */
> +

Not needed. Can be the subject of a "presentation" only patch.

>  #include <linux/blkdev.h>
>  #include <linux/clk.h>
>  #include <linux/debugfs.h>
> @@ -30,11 +32,14 @@
>  #include <asm/io.h>
>  #include <asm/unaligned.h>
> 
> +#include <mach/cpu.h>
>  #include <mach/board.h>
> 
>  #include "atmel-mci-regs.h"
> 
> -#define ATMCI_DATA_ERROR_FLAGS	(MCI_DCRCE | MCI_DTOE | MCI_OVRE | MCI_UNRE)
> +#define ATMCI_DATA_ERROR_FLAGS	(MCI_RINDE | MCI_RDIRE | MCI_RCRCE  \
> +				 | MCI_RENDE | MCI_RTOE | MCI_DCRCE \
> +				 | MCI_DTOE | MCI_OVRE | MCI_UNRE)

We will have to consider what Haavard said: some are not data errors.
But indeed, if you experienced errors not beeing addressed, we have to
deal with them.

>  #define ATMCI_DMA_THRESHOLD	16
> 
>  enum {
> @@ -208,6 +213,18 @@ struct atmel_mci_slot {
>  	set_bit(event, &host->pending_events)
> 
>  /*
> + * Enable or disable features/registers based on
> + * whether the processor supports them
> + */
> +static bool mci_has_rwproof(void)
> +{
> +	if (cpu_is_at91sam9261() || cpu_is_at91rm9200())
> +		return false;
> +	else
> +		return true;
> +}
> +
> +/*

Ok for this.

>   * The debugfs stuff below is mostly optimized away when
>   * CONFIG_DEBUG_FS is not set.
>   */
> @@ -274,8 +291,12 @@ static void atmci_show_status_reg(struct seq_file *s,
>  		[3]	= "BLKE",
>  		[4]	= "DTIP",
>  		[5]	= "NOTBUSY",
> +		[6]	= "ENDRX",
> +		[7]	= "ENDTX",
>  		[8]	= "SDIOIRQA",
>  		[9]	= "SDIOIRQB",
> +		[14]	= "RXBUFF",
> +		[15]	= "TXBUFE",
>  		[16]	= "RINDE",
>  		[17]	= "RDIRE",
>  		[18]	= "RCRCE",

I add some MCI2 bits:

--- a/drivers/mmc/host/atmel-mci.c
+++ b/drivers/mmc/host/atmel-mci.c
@@ -276,8 +276,13 @@ static void atmci_show_status_reg(struct seq_file *s,
 		[3]	= "BLKE",
 		[4]	= "DTIP",
 		[5]	= "NOTBUSY",
+		[6]	= "ENDRX",
+		[7]	= "ENDTX",
 		[8]	= "SDIOIRQA",
 		[9]	= "SDIOIRQB",
+		[12]	= "SDIOWAIT",
+		[14]	= "RXBUFF",
+		[15]	= "TXBUFE",
 		[16]	= "RINDE",
 		[17]	= "RDIRE",
 		[18]	= "RCRCE",
@@ -285,6 +290,11 @@ static void atmci_show_status_reg(struct seq_file *s,
 		[20]	= "RTOE",
 		[21]	= "DCRCE",
 		[22]	= "DTOE",
+		[23]	= "CSTOE",
+		[24]	= "BLKOVRE",
+		[25]	= "DMADONE",
+		[26]	= "FIFOEMPTY",
+		[27]	= "XFRDONE",
 		[30]	= "OVRE",
 		[31]	= "UNRE",
 	};


> @@ -847,13 +868,15 @@ static void atmci_set_ios(struct mmc_host *mmc,
> struct mmc_ios *ios)
>  			clkdiv = 255;
>  		}
> 
> +		host->mode_reg = MCI_MR_CLKDIV(clkdiv);
> +
>  		/*
>  		 * WRPROOF and RDPROOF prevent overruns/underruns by
>  		 * stopping the clock when the FIFO is full/empty.
>  		 * This state is not expected to last for long.
>  		 */
> -		host->mode_reg = MCI_MR_CLKDIV(clkdiv) | MCI_MR_WRPROOF
> -					| MCI_MR_RDPROOF;
> +		if (mci_has_rwproof())
> +			host->mode_reg |= (MCI_MR_WRPROOF | MCI_MR_RDPROOF);
> 
>  		if (list_empty(&host->queue))
>  			mci_writel(host, MR, host->mode_reg);

Ok for this.

> @@ -1642,8 +1665,10 @@ static int __init atmci_probe(struct
> platform_device *pdev)
>  			nr_slots++;
>  	}
> 
> -	if (!nr_slots)
> +	if (!nr_slots) {
> +		printk(KERN_ERR "Atmel MCI controller init failed.  atmci_init_slot
> error or no slots with bus_width > 0.\n");
>  		goto err_init_slot;
> +	}
> 
>  	dev_info(&pdev->dev,
>  			"Atmel MCI controller at 0x%08lx irq %d, %u slots\n",

Ok (maybe in a different patch)

Best regards,
-- 
Nicolas Ferre

Re: [PATCH 1/6] Unified AVR32/AT91 MCI Driver

[Rob Emanuele]

Hi Nicolas,

> First of all thank you for this splitting effort. It is definitively the
> good way of presenting things and ease reading a lot.

My pleasure.

>> ---
>>  drivers/mmc/host/Kconfig     |   16 ++++++++++++----
>>  drivers/mmc/host/atmel-mci.c |   33 +++++++++++++++++++++++++++++----
>>  2 files changed, 41 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
>> index b4cf691..6e2f52b 100644
>> --- a/drivers/mmc/host/Kconfig
>> +++ b/drivers/mmc/host/Kconfig
>> @@ -124,6 +124,12 @@ config MMC_AU1X
>>
>>         If unsure, say N.
>>
>> +choice
>> +     prompt "Atmel MMC Driver"
>> +     default MMC_ATMELMCI
>> +     help
>> +       Choose which driver to use for the Atmel MCI Silicon
>
> I suggest :
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index b4cf691..e779049 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -124,6 +124,12 @@ config MMC_AU1X
>
>          If unsure, say N.
>
> +choice
> +       prompt "Atmel SD/MMC Driver"
> +       default MMC_ATMELMCI if AVR32
> +       help
> +         Choose which driver to use for the Atmel MCI Silicon
> +
>  config MMC_AT91
>        tristate "AT91 SD/MMC Card Interface support"
>        depends on ARCH_AT91
>
> It will keep present default configuration for both AVR32 and AT91.
>

Fine with me.

>> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
>> index cf6a100..497dd51 100644
>> --- a/drivers/mmc/host/atmel-mci.c
>> +++ b/drivers/mmc/host/atmel-mci.c
>> @@ -2,11 +2,13 @@
>>   * Atmel MultiMedia Card Interface driver
>>   *
>>   * Copyright (C) 2004-2008 Atmel Corporation
>> + * Copyright (C) 2009 Rob Emanuele
>
> I think you can add your copyright on a file that you create or copy and
> modify from another file.

Noted for future reference.  I wasn't sure what the ettiquete was.
Just wanted to make sure that if there were questions about the
changes I introduced people could find me.  Now that I know more about
the maintaining process I see that is unnecessary.

>>
>> -#define ATMCI_DATA_ERROR_FLAGS       (MCI_DCRCE | MCI_DTOE | MCI_OVRE | MCI_UNRE)
>> +#define ATMCI_DATA_ERROR_FLAGS       (MCI_RINDE | MCI_RDIRE | MCI_RCRCE  \
>> +                              | MCI_RENDE | MCI_RTOE | MCI_DCRCE \
>> +                              | MCI_DTOE | MCI_OVRE | MCI_UNRE)
>
> We will have to consider what Haavard said: some are not data errors.
> But indeed, if you experienced errors not beeing addressed, we have to
> deal with them.
>

We'll have to keep looking into this.  Would someone on the AVR side
be able to evaluate the effects of this on their platform?

I appreciate your time looking this over.  Will you need me to further
revise this patch or will you incorporate the parts that your
comfortable with?

Thank you,

Rob Emanuele