Mailing List Archive: [PATCH 4/9] Alchemy: register mmc platform device for db1200/pb1200 boards

[PATCH 4/9] Alchemy: register mmc platform device for db1200/pb1200 boards

May 19, 2008, 1:06 AM

Post #1 of 3 (309 views)

From 248ed1d1f9c8789e64a373efd37251cdcb9d522a Mon Sep 17 00:00:00 2001
From: Manuel Lauss <mlau@msc-ge.com>
Date: Sat, 17 May 2008 17:48:02 +0200
Subject: [PATCH] Alchemy: register mmc platform device for db1200/pb1200 boards

Add au1xmmc platform data for PB1200/DB1200 boards, and wire up
the 2 SD controllers for them.

Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
---
arch/mips/au1000/common/platform.c | 88 ++++++++++++++++++++++++++----------
arch/mips/au1000/pb1200/platform.c | 60 ++++++++++++++++++++++++
2 files changed, 124 insertions(+), 24 deletions(-)

diff --git a/arch/mips/au1000/common/platform.c b/arch/mips/au1000/common/platform.c
index 8cae775..6225e95 100644
--- a/arch/mips/au1000/common/platform.c
+++ b/arch/mips/au1000/common/platform.c
@@ -16,6 +16,8 @@
#include <linux/init.h>

#include <asm/mach-au1x00/au1xxx.h>
+#include <asm/mach-au1x00/au1xxx_dbdma.h>
+#include <asm/mach-au1x00/au1100_mmc.h>

#define PORT(_base, _irq) \
{ \
@@ -162,24 +164,6 @@ static struct resource au1xxx_usb_gdt_resources[] = {
},
};

-static struct resource au1xxx_mmc_resources[] = {
- [0] = {
- .start = SD0_PHYS_ADDR,
- .end = SD0_PHYS_ADDR + 0x40,
- .flags = IORESOURCE_MEM,
- },
- [1] = {
- .start = SD1_PHYS_ADDR,
- .end = SD1_PHYS_ADDR + 0x40,
- .flags = IORESOURCE_MEM,
- },
- [2] = {
- .start = AU1200_SD_INT,
- .end = AU1200_SD_INT,
- .flags = IORESOURCE_IRQ,
- }
-};
-
static u64 udc_dmamask = ~(u32)0;

static struct platform_device au1xxx_usb_gdt_device = {
@@ -248,15 +232,70 @@ static struct platform_device au1200_lcd_device = {

static u64 au1xxx_mmc_dmamask = ~(u32)0;

-static struct platform_device au1xxx_mmc_device = {
+extern struct au1xmmc_platform_data au1xmmc_platdata[2];
+
+static struct resource au1200_mmc0_resources[] = {
+ [0] = {
+ .start = SD0_PHYS_ADDR,
+ .end = SD0_PHYS_ADDR + 0x7ffff,
+ .flags = IORESOURCE_MEM,
+ },
+ [1] = {
+ .start = AU1200_SD_INT,
+ .flags = IORESOURCE_IRQ,
+ },
+ [2] = {
+ .start = DSCR_CMD0_SDMS_TX0,
+ .flags = IORESOURCE_DMA,
+ },
+ [3] = {
+ .start = DSCR_CMD0_SDMS_RX0,
+ .flags = IORESOURCE_DMA,
+ }
+};
+
+static struct resource au1200_mmc1_resources[] = {
+ [0] = {
+ .start = SD1_PHYS_ADDR,
+ .end = SD1_PHYS_ADDR + 0x7ffff,
+ .flags = IORESOURCE_MEM,
+ },
+ [1] = {
+ .start = AU1200_SD_INT,
+ .flags = IORESOURCE_IRQ,
+ },
+ [2] = {
+ .start = DSCR_CMD0_SDMS_TX1,
+ .flags = IORESOURCE_DMA,
+ },
+ [3] = {
+ .start = DSCR_CMD0_SDMS_RX1,
+ .flags = IORESOURCE_DMA,
+ }
+};
+
+static struct platform_device au1200_mmc0_device = {
.name = "au1xxx-mmc",
.id = 0,
.dev = {
- .dma_mask = &au1xxx_mmc_dmamask,
- .coherent_dma_mask = 0xffffffff,
+ .dma_mask = &au1xxx_mmc_dmamask,
+ .coherent_dma_mask = 0xffffffff,
+ .platform_data = &au1xmmc_platdata[0],
+ },
+ .num_resources = ARRAY_SIZE(au1200_mmc0_resources),
+ .resource = au1200_mmc0_resources,
+};
+
+static struct platform_device au1200_mmc1_device = {
+ .name = "au1xxx-mmc",
+ .id = 1,
+ .dev = {
+ .dma_mask = &au1xxx_mmc_dmamask,
+ .coherent_dma_mask = 0xffffffff,
+ .platform_data = &au1xmmc_platdata[1],
},
- .num_resources = ARRAY_SIZE(au1xxx_mmc_resources),
- .resource = au1xxx_mmc_resources,
+ .num_resources = ARRAY_SIZE(au1200_mmc1_resources),
+ .resource = au1200_mmc1_resources,
};
#endif /* #ifdef CONFIG_SOC_AU1200 */

@@ -295,7 +334,8 @@ static struct platform_device *au1xxx_platform_devices[] __initdata = {
&au1xxx_usb_gdt_device,
&au1xxx_usb_otg_device,
&au1200_lcd_device,
- &au1xxx_mmc_device,
+ &au1200_mmc0_device,
+ &au1200_mmc1_device,
#endif
#ifdef SMBUS_PSC_BASE
&pbdb_smbus_device,
diff --git a/arch/mips/au1000/pb1200/platform.c b/arch/mips/au1000/pb1200/platform.c
index 5930110..f329a38 100644
--- a/arch/mips/au1000/pb1200/platform.c
+++ b/arch/mips/au1000/pb1200/platform.c
@@ -22,6 +22,66 @@
#include <linux/platform_device.h>

#include <asm/mach-au1x00/au1xxx.h>
+#include <asm/mach-au1x00/au1100_mmc.h>
+
+static void pb1200mmc0_set_power(void *mmc_host, int state)
+{
+ if (state)
+ bcsr->board |= BCSR_BOARD_SD0PWR;
+ else
+ bcsr->board &= ~BCSR_BOARD_SD0PWR;
+
+ au_sync_delay(1);
+}
+
+static int pb1200mmc0_card_readonly(void *mmc_host)
+{
+ return (bcsr->status & BCSR_STATUS_SD0WP) ? 1 : 0;
+}
+
+static int pb1200mmc0_card_inserted(void *mmc_host)
+{
+ return (bcsr->sig_status & BCSR_INT_SD0INSERT) ? 1 : 0;
+}
+
+#ifndef CONFIG_MIPS_DB1200
+static void pb1200mmc1_set_power(void *mmc_host, int state)
+{
+ if (state)
+ bcsr->board |= BCSR_BOARD_SD1PWR;
+ else
+ bcsr->board &= ~BCSR_BOARD_SD1PWR;
+
+ au_sync_delay(1);
+}
+
+static int pb1200mmc1_card_readonly(void *mmc_host)
+{
+ return (bcsr->status & BCSR_STATUS_SD1WP) ? 1 : 0;
+}
+
+static int pb1200mmc1_card_inserted(void *mmc_host)
+{
+ return (bcsr->sig_status & BCSR_INT_SD1INSERT) ? 1 : 0;
+}
+#endif
+
+const struct au1xmmc_platform_data au1xmmc_platdata[2] = {
+ [0] = {
+ .set_power = pb1200mmc0_set_power,
+ .card_inserted = pb1200mmc0_card_inserted,
+ .card_readonly = pb1200mmc0_card_readonly,
+ .cd_setup = NULL, /* use poll-timer in driver */
+ },
+#ifndef CONFIG_MIPS_DB1200
+ [1] = {
+ .set_power = pb1200mmc1_set_power,
+ .card_inserted = pb1200mmc1_card_inserted,
+ .card_readonly = pb1200mmc1_card_readonly,
+ .cd_setup = NULL, /* use poll-timer in driver */
+ },
+#endif
+};

static struct resource ide_resources[] = {
[0] = {
--
1.5.5.1

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

Re: [PATCH 4/9] Alchemy: register mmc platform device for db1200/pb1200 boards [ In reply to ]

sshtylyov at ru

May 21, 2008, 5:32 AM

Post #2 of 3 (283 views)

Permalink

Hello.

Manuel Lauss wrote:

> Add au1xmmc platform data for PB1200/DB1200 boards, and wire up
> the 2 SD controllers for them.

> Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>

OK, this definitely looks better (and shorter :-).

> diff --git a/arch/mips/au1000/common/platform.c b/arch/mips/au1000/common/platform.c
> index 8cae775..6225e95 100644
> --- a/arch/mips/au1000/common/platform.c
> +++ b/arch/mips/au1000/common/platform.c

[...]

> @@ -248,15 +232,70 @@ static struct platform_device au1200_lcd_device = {
>
> static u64 au1xxx_mmc_dmamask = ~(u32)0;
>
> -static struct platform_device au1xxx_mmc_device = {
> +extern struct au1xmmc_platform_data au1xmmc_platdata[2];
> +
> +static struct resource au1200_mmc0_resources[] = {
> + [0] = {
> + .start = SD0_PHYS_ADDR,
> + .end = SD0_PHYS_ADDR + 0x7ffff,
> + .flags = IORESOURCE_MEM,
> + },
> + [1] = {
> + .start = AU1200_SD_INT,
> + .flags = IORESOURCE_IRQ,
> + },
> + [2] = {
> + .start = DSCR_CMD0_SDMS_TX0,
> + .flags = IORESOURCE_DMA,
> + },
> + [3] = {
> + .start = DSCR_CMD0_SDMS_RX0,
> + .flags = IORESOURCE_DMA,
> + }
> +};
> +
> +static struct resource au1200_mmc1_resources[] = {
> + [0] = {
> + .start = SD1_PHYS_ADDR,
> + .end = SD1_PHYS_ADDR + 0x7ffff,
> + .flags = IORESOURCE_MEM,
> + },
> + [1] = {
> + .start = AU1200_SD_INT,
> + .flags = IORESOURCE_IRQ,
> + },
> + [2] = {
> + .start = DSCR_CMD0_SDMS_TX1,
> + .flags = IORESOURCE_DMA,
> + },
> + [3] = {
> + .start = DSCR_CMD0_SDMS_RX1,
> + .flags = IORESOURCE_DMA,
> + }
> +};
> +

Shouldn't the IRQ/DMA resources also have their 'end' field set?

[...]

> diff --git a/arch/mips/au1000/pb1200/platform.c b/arch/mips/au1000/pb1200/platform.c
> index 5930110..f329a38 100644
> --- a/arch/mips/au1000/pb1200/platform.c
> +++ b/arch/mips/au1000/pb1200/platform.c
> @@ -22,6 +22,66 @@
> #include <linux/platform_device.h>
>
> #include <asm/mach-au1x00/au1xxx.h>
> +#include <asm/mach-au1x00/au1100_mmc.h>
> +
> +static void pb1200mmc0_set_power(void *mmc_host, int state)
> +{
> + if (state)
> + bcsr->board |= BCSR_BOARD_SD0PWR;
> + else
> + bcsr->board &= ~BCSR_BOARD_SD0PWR;
> +
> + au_sync_delay(1);
> +}
> +
> +static int pb1200mmc0_card_readonly(void *mmc_host)
> +{
> + return (bcsr->status & BCSR_STATUS_SD0WP) ? 1 : 0;
> +}
> +
> +static int pb1200mmc0_card_inserted(void *mmc_host)
> +{
> + return (bcsr->sig_status & BCSR_INT_SD0INSERT) ? 1 : 0;
> +}
> +
> +#ifndef CONFIG_MIPS_DB1200
> +static void pb1200mmc1_set_power(void *mmc_host, int state)
> +{
> + if (state)
> + bcsr->board |= BCSR_BOARD_SD1PWR;
> + else
> + bcsr->board &= ~BCSR_BOARD_SD1PWR;
> +
> + au_sync_delay(1);
> +}
> +
> +static int pb1200mmc1_card_readonly(void *mmc_host)
> +{
> + return (bcsr->status & BCSR_STATUS_SD1WP) ? 1 : 0;
> +}
> +
> +static int pb1200mmc1_card_inserted(void *mmc_host)
> +{
> + return (bcsr->sig_status & BCSR_INT_SD1INSERT) ? 1 : 0;
> +}
> +#endif

These 2 separate versions could be converted into single one by using the
data arrays holding info BCSR bits -- something like the MMC driver has currently.

> +
> +const struct au1xmmc_platform_data au1xmmc_platdata[2] = {
> + [0] = {
> + .set_power = pb1200mmc0_set_power,
> + .card_inserted = pb1200mmc0_card_inserted,
> + .card_readonly = pb1200mmc0_card_readonly,
> + .cd_setup = NULL, /* use poll-timer in driver */
> + },
> +#ifndef CONFIG_MIPS_DB1200
> + [1] = {
> + .set_power = pb1200mmc1_set_power,
> + .card_inserted = pb1200mmc1_card_inserted,
> + .card_readonly = pb1200mmc1_card_readonly,
> + .cd_setup = NULL, /* use poll-timer in driver */
> + },
> +#endif
> +};

You don't have to explicitly set 'cd_setup' to NULL...

PS: BTW, I've noticed that include/asm-mips/mach-db1x00/db1x00.h defines
macros mmc_card_insterted() and mmc_power_on() which no code seems to be using
(might have been intended for use by the MMC driver but why no such macros in
other board headers?). Care to remove those while you're working on MMC?

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

Re: [PATCH 4/9] Alchemy: register mmc platform device for db1200/pb1200 boards [ In reply to ]

mano at roarinelk

May 21, 2008, 6:10 AM

Post #3 of 3 (288 views)

Permalink

Hi Sergei,

> Manuel Lauss wrote:
>
>> Add au1xmmc platform data for PB1200/DB1200 boards, and wire up
>> the 2 SD controllers for them.
>
>> Signed-off-by: Manuel Lauss <mano@roarinelk.homelinux.net>
>
> OK, this definitely looks better (and shorter :-).

Thanks ;-)

>> +static struct resource au1200_mmc0_resources[] = {
>> + [0] = {
>> + .start = SD0_PHYS_ADDR,
>> + .end = SD0_PHYS_ADDR + 0x7ffff,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + [1] = {
>> + .start = AU1200_SD_INT,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + [2] = {
>> + .start = DSCR_CMD0_SDMS_TX0,
>> + .flags = IORESOURCE_DMA,
>> + },
>> + [3] = {
>> + .start = DSCR_CMD0_SDMS_RX0,
>> + .flags = IORESOURCE_DMA,
>> + }
>> +};
>> +
>> +static struct resource au1200_mmc1_resources[] = {
>> + [0] = {
>> + .start = SD1_PHYS_ADDR,
>> + .end = SD1_PHYS_ADDR + 0x7ffff,
>> + .flags = IORESOURCE_MEM,
>> + },
>> + [1] = {
>> + .start = AU1200_SD_INT,
>> + .flags = IORESOURCE_IRQ,
>> + },
>> + [2] = {
>> + .start = DSCR_CMD0_SDMS_TX1,
>> + .flags = IORESOURCE_DMA,
>> + },
>> + [3] = {
>> + .start = DSCR_CMD0_SDMS_RX1,
>> + .flags = IORESOURCE_DMA,
>> + }
>> +};
>> +
>
> Shouldn't the IRQ/DMA resources also have their 'end' field set?

Not sure, but I'll add them too.

>> +static void pb1200mmc0_set_power(void *mmc_host, int state)
>> +{
>> + if (state)
>> + bcsr->board |= BCSR_BOARD_SD0PWR;
>> + else
>> + bcsr->board &= ~BCSR_BOARD_SD0PWR;
>> +
>> + au_sync_delay(1);
>> +}
>> +
>> +static int pb1200mmc0_card_readonly(void *mmc_host)
>> +{
>> + return (bcsr->status & BCSR_STATUS_SD0WP) ? 1 : 0;
>> +}
>> +
>> +static int pb1200mmc0_card_inserted(void *mmc_host)
>> +{
>> + return (bcsr->sig_status & BCSR_INT_SD0INSERT) ? 1 : 0;
>> +}
>> +
>> +#ifndef CONFIG_MIPS_DB1200
>> +static void pb1200mmc1_set_power(void *mmc_host, int state)
>> +{
>> + if (state)
>> + bcsr->board |= BCSR_BOARD_SD1PWR;
>> + else
>> + bcsr->board &= ~BCSR_BOARD_SD1PWR;
>> +
>> + au_sync_delay(1);
>> +}
>> +
>> +static int pb1200mmc1_card_readonly(void *mmc_host)
>> +{
>> + return (bcsr->status & BCSR_STATUS_SD1WP) ? 1 : 0;
>> +}
>> +
>> +static int pb1200mmc1_card_inserted(void *mmc_host)
>> +{
>> + return (bcsr->sig_status & BCSR_INT_SD1INSERT) ? 1 : 0;
>> +}
>> +#endif
>
> These 2 separate versions could be converted into single one by using
> the data arrays holding info BCSR bits -- something like the MMC driver has
> currently.

Well, that's what my initial submission did, but it required access to the
mmc drivers host structure (moved from au1xmmc.h to au1100_mmc.h) to determine
which controller wants the attention, and you weren't fond of that.

>> +
>> +const struct au1xmmc_platform_data au1xmmc_platdata[2] = {
>> + [0] = {
>> + .set_power = pb1200mmc0_set_power,
>> + .card_inserted = pb1200mmc0_card_inserted,
>> + .card_readonly = pb1200mmc0_card_readonly,
>> + .cd_setup = NULL, /* use poll-timer in driver */
>> + },
>> +#ifndef CONFIG_MIPS_DB1200
>> + [1] = {
>> + .set_power = pb1200mmc1_set_power,
>> + .card_inserted = pb1200mmc1_card_inserted,
>> + .card_readonly = pb1200mmc1_card_readonly,
>> + .cd_setup = NULL, /* use poll-timer in driver */
>> + },
>> +#endif
>> +};
>
> You don't have to explicitly set 'cd_setup' to NULL...

The comment explains what setting to NULL does, so I'd like to
keep it (for people implementing this for other boards and wondering what
to do with thse members...)

> PS: BTW, I've noticed that include/asm-mips/mach-db1x00/db1x00.h defines
> macros mmc_card_insterted() and mmc_power_on() which no code seems to be
> using (might have been intended for use by the MMC driver but why no such
> macros in other board headers?). Care to remove those while you're working
> on MMC?

Sure, I'll add another patch to the pile.

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