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

From: Nicolas Ferre
Date: Mon Jun 15 2009 - 12:00:55 EST

Hi,

I continue my comments.

Rob Emanuele :
> Updated the at91sam9g20 evaluation kit and device support to make use
> of the updated atmel-mci driver.
>
> This patch shows how an AT91 developer could add support for both SD
> slots on their processors.
>
> Please read the whole set, try it out, and comment.
>
> Thank you,
>
> Rob Emanuele
>
> ---
> arch/arm/mach-at91/Kconfig | 6 ++
> arch/arm/mach-at91/at91sam9260_devices.c | 95 ++++++++++++++++++++++++++++++
> arch/arm/mach-at91/board-sam9g20ek.c | 37 +++++++++++-
> arch/arm/mach-at91/include/mach/board.h | 7 ++
> 4 files changed, 144 insertions(+), 1 deletions(-)
>
> diff --git a/arch/arm/mach-at91/Kconfig b/arch/arm/mach-at91/Kconfig
> index 323b47f..7f6c96a 100644
> --- a/arch/arm/mach-at91/Kconfig
> +++ b/arch/arm/mach-at91/Kconfig
> @@ -326,6 +326,12 @@ config MTD_NAND_ATMEL_BUSWIDTH_16
> On AT91SAM926x boards both types of NAND flash can be present
> (8 and 16 bit data bus width).
>
> +config AT91_2MMC
> + bool "Use both MMC Ports"
> + depends on MACH_AT91SAM9G20EK
> + help
> + Select this if you have connected both MMC Slots. Answer No if unsure.
> +

Well I do not like this configuration addition. If it is created
to differencing your custom board with the -EK, we will have to
consider something else.

> # ----------------------------------------------------------
>
> comment "AT91 Feature Selections"
> diff --git a/arch/arm/mach-at91/at91sam9260_devices.c
> b/arch/arm/mach-at91/at91sam9260_devices.c
> index d74c9ac..e7cc46a 100644
> --- a/arch/arm/mach-at91/at91sam9260_devices.c
> +++ b/arch/arm/mach-at91/at91sam9260_devices.c
> @@ -2,6 +2,7 @@
> * arch/arm/mach-at91/at91sam9260_devices.c
> *
> * Copyright (C) 2006 Atmel
> + * Copyright (C) 2009 Rob Emanuele

In my opinion, no copyright addition.

> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -275,8 +276,102 @@ void __init at91_add_device_mmc(short mmc_id,
> struct at91_mmc_data *data)
> platform_device_register(&at91sam9260_mmc_device);
> }
> #else
> +
> +/* --------------------------------------------------------------------
> + * MMC / SD Slot for Unified Atmel Driver
> + * -------------------------------------------------------------------- */
> +
> +#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
> +static u64 mmc_dmamask = DMA_BIT_MASK(32);
> +static struct mci_platform_data mmc_data;
> +
> +static struct resource mmc_resources[] = {
> + [0] = {
> + .start = AT91SAM9260_BASE_MCI,
> + .end = AT91SAM9260_BASE_MCI + SZ_16K - 1,
> + .flags = IORESOURCE_MEM,
> + },
> + [1] = {
> + .start = AT91SAM9260_ID_MCI,
> + .end = AT91SAM9260_ID_MCI,
> + .flags = IORESOURCE_IRQ,
> + },
> +};
> +
> +static struct platform_device at91sam9260_mmc_device = {
> + .name = "atmel_mci",
> + .id = -1,
> + .dev = {
> + .dma_mask = &mmc_dmamask,
> + .coherent_dma_mask = DMA_BIT_MASK(32),
> + .platform_data = &mmc_data,
> + },
> + .resource = mmc_resources,
> + .num_resources = ARRAY_SIZE(mmc_resources),
> +};
> +
> +void __init at91_add_device_mmc(short mmc_id, struct mci_platform_data *data)
> +{
> + unsigned int i;
> + unsigned int slot_count = 0;
> +
> + if (!data)
> + return;
> +
> + for (i = 0; i < ATMEL_MCI_MAX_NR_SLOTS; i++) {
> + if (data->slot[i].bus_width) {
> + /* input/irq */
> + if (data->slot[i].detect_pin) {
> +
> at91_set_gpio_input(data->slot[i].detect_pin, 1);
> + at91_set_deglitch(data->slot[i].detect_pin, 1);
> + }
> + if (data->slot[i].wp_pin)
> + at91_set_gpio_input(data->slot[i].wp_pin, 1);
> +
> + switch(i) {
> + case 0:
> + /* CMD */
> + at91_set_A_periph(AT91_PIN_PA7, 1);
> + /* DAT0, maybe DAT1..DAT3 */
> + at91_set_A_periph(AT91_PIN_PA6, 1);
> + if (data->slot[i].bus_width == 4) {
> + at91_set_A_periph(AT91_PIN_PA9, 1);
> + at91_set_A_periph(AT91_PIN_PA10, 1);
> + at91_set_A_periph(AT91_PIN_PA11, 1);
> + }
> + break;
> + case 1:
> + /* CMD */
> + at91_set_B_periph(AT91_PIN_PA1, 1);
> + /* DAT0, maybe DAT1..DAT3 */
> + at91_set_B_periph(AT91_PIN_PA0, 1);
> + if (data->slot[i].bus_width == 4) {
> + at91_set_B_periph(AT91_PIN_PA5, 1);
> + at91_set_B_periph(AT91_PIN_PA4, 1);
> + at91_set_B_periph(AT91_PIN_PA3, 1);
> + }
> + break;
> + default:
> + printk("Configuration Error, No MMC Port %d\n",i);
> + break;
> + };
> + slot_count++;
> + }
> + }
> +
> + if (slot_count) {
> + /* CLK */
> + at91_set_A_periph(AT91_PIN_PA8, 0);
> +
> + mmc_data = *data;
> + platform_device_register(&at91sam9260_mmc_device);
> + }
> +}
> +#else
> void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data) {}
> #endif
> +#endif
> +
>
>
> /* --------------------------------------------------------------------
> diff --git a/arch/arm/mach-at91/board-sam9g20ek.c
> b/arch/arm/mach-at91/board-sam9g20ek.c
> index 438efbb..ca70042 100644
> --- a/arch/arm/mach-at91/board-sam9g20ek.c
> +++ b/arch/arm/mach-at91/board-sam9g20ek.c
> @@ -1,6 +1,7 @@
> /*
> * Copyright (C) 2005 SAN People
> * Copyright (C) 2008 Atmel
> + * Copyright (C) 2009 Rob Emanuele

Ditto.

> *
> * This program is free software; you can redistribute it and/or modify
> * it under the terms of the GNU General Public License as published by
> @@ -89,7 +90,7 @@ static struct at91_udc_data __initdata ek_udc_data = {
> * SPI devices.
> */
> static struct spi_board_info ek_spi_devices[] = {
> -#if !defined(CONFIG_MMC_AT91)
> +#if !defined(CONFIG_MMC_AT91) && !defined(CONFIG_MMC_ATMELMCI)
> { /* DataFlash chip */
> .modalias = "mtd_dataflash",
> .chip_select = 1,
> @@ -112,7 +113,11 @@ static struct spi_board_info ek_spi_devices[] = {
> * MACB Ethernet device
> */
> static struct at91_eth_data __initdata ek_macb_data = {
> +#if defined(CONFIG_AT91_2MMC)
> + .phy_irq_pin = AT91_PIN_PC12,
> +#else
> .phy_irq_pin = AT91_PIN_PA7,
> +#endif

Configuration option has nothing to do with Ethernet... You should
create your own board configuration.

> .is_rmii = 1,
> };
>
> @@ -195,11 +200,33 @@ static void __init ek_add_device_nand(void)
> * MCI (SD/MMC)
> * det_pin, wp_pin and vcc_pin are not connected
> */
> +#if defined(CONFIG_MMC_AT91) || defined(CONFIG_MMC_AT91_MODULE)
> static struct at91_mmc_data __initdata ek_mmc_data = {
> .slot_b = 1,
> .wire4 = 1,
> };
> +#elif defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
> +static struct mci_platform_data __initdata ek_mmc_data = {
> + .slot[0] = {
> +#if defined(CONFIG_AT91_2MMC)
> + .bus_width = 4,
> +#else
> + .bus_width = 0,
> +#endif
> + .detect_pin = -ENODEV,
> + .wp_pin = -ENODEV,
> + },
> + .slot[1] = {
> + .bus_width = 4,
> + .detect_pin = -ENODEV,
> + .wp_pin = -ENODEV,
> + },
>
> +};
> +#else
> +static struct amci_platform_data __initdata ek_mmc_data = {
> +};
> +#endif
> /*
> * LEDs
> @@ -207,13 +234,21 @@ static struct at91_mmc_data __initdata ek_mmc_data = {
> static struct gpio_led ek_leds[] = {
> { /* "bottom" led, green, userled1 to be defined */
> .name = "ds5",
> +#if defined(CONFIG_AT91_2MMC)
> + .gpio = AT91_PIN_PB12,
> +#else
> .gpio = AT91_PIN_PA6,
> +#endif

Ditto.

> .active_low = 1,
> .default_trigger = "none",
> },
> { /* "power" led, yellow */
> .name = "ds1",
> +#if defined(CONFIG_AT91_2MMC)
> + .gpio = AT91_PIN_PB13,
> +#else
> .gpio = AT91_PIN_PA9,
> +#endif
> .default_trigger = "heartbeat",
> }
> };
> diff --git a/arch/arm/mach-at91/include/mach/board.h
> b/arch/arm/mach-at91/include/mach/board.h
> index e6afff8..8aa310a 100644
> --- a/arch/arm/mach-at91/include/mach/board.h
> +++ b/arch/arm/mach-at91/include/mach/board.h
> @@ -37,6 +37,9 @@
> #include <linux/leds.h>
> #include <linux/spi/spi.h>
> #include <linux/usb/atmel_usba_udc.h>
> +#if defined(CONFIG_MMC_ATMELMCI) || defined(CONFIG_MMC_ATMELMCI_MODULE)
> +#include <linux/atmel-mci.h>
> +#endif

Not needed.

>
> /* USB Device */
> struct at91_udc_data {
> @@ -63,6 +66,7 @@ struct at91_cf_data {
> extern void __init at91_add_device_cf(struct at91_cf_data *data);
>
> /* MMC / SD */
> +#if defined(CONFIG_MMC_AT91) || defined(CONFIG_MMC_AT91_MODULE)
> struct at91_mmc_data {
> u8 det_pin; /* card detect IRQ */
> unsigned slot_b:1; /* uses Slot B */
> @@ -71,6 +75,9 @@ struct at91_mmc_data {
> u8 vcc_pin; /* power switching (high == on) */
> };
> extern void __init at91_add_device_mmc(short mmc_id, struct
> at91_mmc_data *data);
> +#else
> +extern void __init at91_add_device_mmc(short mmc_id, struct
> mci_platform_data *data);
> +#endif

Maybe we can be much simpler adding another function for atmel-mci
initialization: I propose at91_add_device_mci()


diff --git a/arch/arm/mach-at91/include/mach/board.h b/arch/arm/mach-at91/include/mach/board.h
index 13640b0..7c9a703 100644
--- a/arch/arm/mach-at91/include/mach/board.h
+++ b/arch/arm/mach-at91/include/mach/board.h
@@ -37,6 +37,7 @@
#include <linux/leds.h>
#include <linux/spi/spi.h>
#include <linux/usb/atmel_usba_udc.h>
+#include <linux/atmel-mci.h>

/* USB Device */
struct at91_udc_data {
@@ -71,6 +72,7 @@ struct at91_mmc_data {
u8 vcc_pin; /* power switching (high == on) */
};
extern void __init at91_add_device_mmc(short mmc_id, struct at91_mmc_data *data);
+extern void __init at91_add_device_mci(short mmc_id, struct mci_platform_data *data);

/* Ethernet (EMAC & MACB) */
struct at91_eth_data {


Bye,
--
Nicolas Ferre

--
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/