Re: [PATCH 3/6] atmel-mci: Optional controller reset before every command

From: Marc Pignat
Date: Wed Jun 17 2009 - 04:32:48 EST


Hi!

On Wednesday 17 June 2009 02:54:36 Rob Emanuele wrote:
> This patch adds the compile time configuration option for the atmel-mci driver to reset the Atmel MCI controller before every command.
>
> This is sometimes needed for the Atmel controller / board to function properly.
>
> Signed-off-by: Rob Emanuele <rob@xxxxxxxxxxx>
> ---
> drivers/mmc/host/Kconfig | 10 ++++++++++
> drivers/mmc/host/atmel-mci.c | 11 ++++++++++-
> 2 files changed, 20 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index a0eaf69..5875125 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -161,6 +161,16 @@ config MMC_ATMELMCI
>
> endchoice
>
> +config MMC_ATMELMCI_ALWAYS_RESET
> + bool "Reset before every request. Sometimes needed for buggy chips."
> + depends on MMC_ATMELMCI
> + help
> + There are reports that some buggy controllers work better
> + with a reset before every command. This may improve your
> + controller's reliability.
> +
> + If unsure, say N.
> +
> config MMC_ATMELMCI_DMA
> bool "Atmel MCI DMA support (EXPERIMENTAL)"
> depends on MMC_ATMELMCI && AVR32 && DMA_ENGINE && EXPERIMENTAL
> diff --git a/drivers/mmc/host/atmel-mci.c b/drivers/mmc/host/atmel-mci.c
> index 98b25de..e5894f6 100644
> --- a/drivers/mmc/host/atmel-mci.c
> +++ b/drivers/mmc/host/atmel-mci.c
> @@ -63,6 +63,15 @@ struct atmel_mci_dma {
> #endif
> };
>
> +/*
> + * Configuration options from the kernel config
> + */
> +#ifdef CONFIG_MMC_ATMELMCI_ALWAYS_RESET
> +#define MMC_ALWAYS_RESET 1
> +#else
> +#define MMC_ALWAYS_RESET 0
> +#endif

Some chips are known to hang after (some) transfer. There is no need for this
configuration option, I think we should just do :

#define MMC_ALWAYS_RESET (cpu_is_at91rm9200() || add_your_cpu_name_here())

> +
> /**
> * struct atmel_mci - MMC controller state shared between all slots
> * @lock: Spinlock protecting the queue and associated data.
> @@ -720,7 +729,7 @@ static void atmci_start_request(struct atmel_mci *host,
> host->completed_events = 0;
> host->data_status = 0;
>
> - if (host->need_reset) {
> + if (host->need_reset || MMC_ALWAYS_RESET) {
> mci_writel(host, CR, MCI_CR_SWRST);
> mci_writel(host, CR, MCI_CR_MCIEN);
> mci_writel(host, MR, host->mode_reg);

Another option is to always reset the controller.

The reset operation won't hurt, it is simple and fast, and the code will be
simpler and better tested.

Best regards

Marc
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/