[U-Boot] [PATCH] MCI support for AT91 family processors.

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Sami Kantoluoto]

Fixed to parse CSD correctly on little endian processors as gcc orders
bitfields differently between big and little endian ones.

Signed-off-by: Sami Kantoluoto <sami.kantoluoto@embedtronics.fi>
---
 drivers/mmc/atmel_mci.c         |   55 ++++++++++++++++++++++++++++++++++++--
 include/asm-arm/arch-at91/clk.h |    5 +++
 2 files changed, 57 insertions(+), 3 deletions(-)

diff --git a/drivers/mmc/atmel_mci.c b/drivers/mmc/atmel_mci.c
index 3946ffe..c2837a8 100644
--- a/drivers/mmc/atmel_mci.c
+++ b/drivers/mmc/atmel_mci.c
@@ -282,6 +282,53 @@ static void sd_parse_cid(struct mmc_cid *cid, unsigned long *resp)
 	cid->mdt = (resp[3] >> 8) & 0x0fff;
 }
 
+static void mmc_parse_csd(struct mmc_csd *csd, unsigned long *resp)
+{
+#if	__BYTE_ORDER == __BIG_ENDIAN
+	memcpy(csd, resp, sizeof(csd));
+#elif	__BYTE_ORDER == __LITTLE_ENDIAN
+	csd->csd_structure = resp[0] >> 30;
+	csd->spec_vers = resp[0] >> 26;
+	csd->rsvd1 = resp[0] >> 24;
+	csd->taac = resp[0] >> 16;
+	csd->nsac = resp[0] >> 8;
+	csd->tran_speed = resp[0] & 0xff;
+	csd->ccc = resp[1] >> 20;
+	csd->read_bl_len = resp[1] >> 16;
+	csd->read_bl_partial = resp[1] >> 15;
+	csd->write_blk_misalign = resp[1] >> 14;
+	csd->read_blk_misalign = resp[1] >> 13;
+	csd->dsr_imp = resp[1] >> 12;
+	csd->rsvd2 = resp[1] >> 10;
+	csd->c_size = (resp[1] << 2) | (resp[2] >> 30);
+	csd->vdd_r_curr_min = resp[2] >> 27;
+	csd->vdd_r_curr_max = resp[2] >> 24;
+	csd->vdd_w_curr_min = resp[2] >> 21;
+	csd->vdd_w_curr_max = resp[2] >> 18;
+	csd->c_size_mult = resp[2] >> 15;
+	csd->sector_size = resp[2] >> 10;
+	csd->erase_grp_size = resp[2] >> 5;
+	csd->wp_grp_size = resp[2] & 0x1f;
+	csd->wp_grp_enable = resp[3] >> 31;
+	csd->default_ecc = resp[3] >> 29;
+	csd->r2w_factor = resp[3] >> 26;
+	csd->write_bl_len = resp[3] >> 22;
+	csd->write_bl_partial = resp[3] >> 21;
+	csd->rsvd3 = resp[3] >> 16;
+
+	csd->file_format_grp = resp[3] >> 15;
+	csd->copy = resp[3] >> 14;
+	csd->perm_write_protect = resp[3] >> 13;
+	csd->tmp_write_protect = resp[3] >> 12;
+	csd->file_format = resp[3] >> 10;
+	csd->ecc = resp[3] >> 8;
+	csd->crc = resp[3] >> 1;
+	csd->one = resp[3] & 1;
+#else
+#error	Unsupported __BYTE_ORDER
+#endif
+}
+
 static void mmc_dump_cid(const struct mmc_cid *cid)
 {
 	printf("Manufacturer ID:       %02X\n", cid->mid);
@@ -298,7 +345,7 @@ static void mmc_dump_csd(const struct mmc_csd *csd)
 {
 	unsigned long *csd_raw = (unsigned long *)csd;
 	printf("CSD data: %08lx %08lx %08lx %08lx\n",
-	       csd_raw[0], csd_raw[1], csd_raw[2], csd_raw[3]);
+	       ntohl(csd_raw[0]), ntohl(csd_raw[1]), ntohl(csd_raw[2]), ntohl(csd_raw[3]));
 	printf("CSD structure version:   1.%u\n", csd->csd_structure);
 	printf("MMC System Spec version: %u\n", csd->spec_vers);
 	printf("Card command classes:    %03x\n", csd->ccc);
@@ -368,7 +415,7 @@ static int sd_init_card(struct mmc_cid *cid, int verbose)
 
 	/* Get RCA of the card that responded */
 	ret = mmc_cmd(SD_CMD_SEND_RELATIVE_ADDR, 0, resp, R6 | NCR);
-	if (ret)
+  	if (ret)
 		return ret;
 
 	mmc_rca = resp[0] >> 16;
@@ -468,6 +515,7 @@ int mmc_legacy_init(int verbose)
 	struct mmc_cid cid;
 	struct mmc_csd csd;
 	unsigned int max_blksz;
+	unsigned long resp[4];
 	int ret;
 
 	/* Initialize controller */
@@ -488,9 +536,10 @@ int mmc_legacy_init(int verbose)
 		return ret;
 
 	/* Get CSD from the card */
-	ret = mmc_cmd(MMC_CMD_SEND_CSD, mmc_rca << 16, &csd, R2 | NCR);
+	ret = mmc_cmd(MMC_CMD_SEND_CSD, mmc_rca << 16, resp, R2 | NCR);
 	if (ret)
 		return ret;
+	mmc_parse_csd(&csd, resp);
 	if (verbose)
 		mmc_dump_csd(&csd);
 
diff --git a/include/asm-arm/arch-at91/clk.h b/include/asm-arm/arch-at91/clk.h
index f642dd9..26b537c 100644
--- a/include/asm-arm/arch-at91/clk.h
+++ b/include/asm-arm/arch-at91/clk.h
@@ -54,6 +54,11 @@ static inline unsigned long get_spi_clk_rate(unsigned int dev_id)
 	return get_mck_clk_rate();
 }
 
+static inline unsigned long get_mci_clk_rate(void)
+{
+	return get_mck_clk_rate();
+}
+
 static inline unsigned long get_twi_clk_rate(unsigned int dev_id)
 {
 	return get_mck_clk_rate();
-- 
1.6.0.4

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Albin Tonnerre]

Hello Sami,

On Sat, Aug 29, 2009 at 08:18:32PM +0300, Sami Kantoluoto wrote :
> Fixed to parse CSD correctly on little endian processors as gcc orders
> bitfields differently between big and little endian ones.

Please also see this patch, which will fix those bugs as weel, while switching
to the new GENRIC_MMC API:
http://lists.denx.de/pipermail/u-boot/2009-August/059456.html
I'd highly appreciate if you could test it, to get some feedback

> diff --git a/include/asm-arm/arch-at91/clk.h b/include/asm-arm/arch-at91/clk.h
> index f642dd9..26b537c 100644
> --- a/include/asm-arm/arch-at91/clk.h
> +++ b/include/asm-arm/arch-at91/clk.h
> @@ -54,6 +54,11 @@ static inline unsigned long get_spi_clk_rate(unsigned int dev_id)
>  	return get_mck_clk_rate();
>  }
>  
> +static inline unsigned long get_mci_clk_rate(void)
> +{
> +	return get_mck_clk_rate();
> +}
> +
>  static inline unsigned long get_twi_clk_rate(unsigned int dev_id)
>  {
>  	return get_mck_clk_rate();

This is still rather incomplete, you're lacking the definition of MMCI_BASE and
the configuration for the MCI controller, which should be done in the
at91sam*_devices.c files. See the attached patch (relying on the patch
mentionned above) that adds such support. It's not complete yet either: it
partially lacks support for models which have 2 MCI controllers, but that's not
the case of the 9G20 anyway (that's just a matter of putting proper ifdefs
before defining AT91_BASE_MCI).
If what I've done in this patch is not the way to go, I'd appreciate guidance on
how to do it correctly.

Regards,
Albin

---
 cpu/arm926ejs/at91/at91cap9_devices.c       |   36 +++++++++++++++++++
 cpu/arm926ejs/at91/at91sam9260_devices.c    |   25 +++++++++++++
 cpu/arm926ejs/at91/at91sam9261_devices.c    |   18 ++++++++++
 cpu/arm926ejs/at91/at91sam9263_devices.c    |   50 +++++++++++++++++++++++++++
 cpu/arm926ejs/at91/at91sam9m10g45_devices.c |   50 +++++++++++++++++++++++++++
 cpu/arm926ejs/at91/at91sam9rl_devices.c     |   22 ++++++++++++
 cpu/arm926ejs/at91/cpu.c                    |    7 ++++
 include/asm-arm/arch-at91/at91_common.h     |    2 +
 include/asm-arm/arch-at91/clk.h             |    5 +++
 include/asm-arm/arch-at91/hardware.h        |    1 +
 include/asm-arm/arch-at91/memory-map.h      |    1 +
 11 files changed, 217 insertions(+), 0 deletions(-)

diff --git a/cpu/arm926ejs/at91/at91cap9_devices.c b/cpu/arm926ejs/at91/at91cap9_devices.c
index 39e405f..2dc1984 100644
--- a/cpu/arm926ejs/at91/at91cap9_devices.c
+++ b/cpu/arm926ejs/at91/at91cap9_devices.c
@@ -79,6 +79,42 @@ void at91_serial_hw_init(void)
 #endif
 }
 
+#ifdef CONFIG_ATMEL_MCI
+#ifdef CONFIG_AT91_MCI0
+void at91_mci0_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9263_ID_MCI0);
+	at91_set_A_periph(AT91_PIN_PA2, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_A_periph(AT91_PIN_PA0, 1);
+			at91_set_A_periph(AT91_PIN_PA1, 1);
+			at91_set_A_periph(AT91_PIN_PA3, 1);
+			at91_set_A_periph(AT91_PIN_PA4, 1);
+			at91_set_A_periph(AT91_PIN_PA5, 1);
+			break;
+	}
+}
+#elif defined(CONFIG_AT91_MCI1
+void at91_mci1_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9263_ID_MCI);
+	at91_set_A_periph(AT91_PIN_PA16, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_A_periph(AT91_PIN_PA17, 1);
+			at91_set_A_periph(AT91_PIN_PA18, 1);
+			at91_set_A_periph(AT91_PIN_PA19, 1);
+			at91_set_A_periph(AT91_PIN_PA20, 1);
+			at91_set_A_periph(AT91_PIN_PA21, 1);
+			break;
+	}
+}
+#endif
+#endif
+
 #ifdef CONFIG_HAS_DATAFLASH
 void at91_spi0_hw_init(unsigned long cs_mask)
 {
diff --git a/cpu/arm926ejs/at91/at91sam9260_devices.c b/cpu/arm926ejs/at91/at91sam9260_devices.c
index f86cb99..413fc8b 100644
--- a/cpu/arm926ejs/at91/at91sam9260_devices.c
+++ b/cpu/arm926ejs/at91/at91sam9260_devices.c
@@ -75,6 +75,31 @@ void at91_serial_hw_init(void)
 #endif
 }
 
+#ifdef CONFIG_ATMEL_MCI
+void at91_mci0_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9260_ID_MCI);
+	at91_set_A_periph(AT91_PIN_PA8, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_A_periph(AT91_PIN_PA6, 1);
+			at91_set_A_periph(AT91_PIN_PA7, 1);
+			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:
+			at91_set_B_periph(AT91_PIN_PA0, 1);
+			at91_set_B_periph(AT91_PIN_PA1, 1);
+			at91_set_B_periph(AT91_PIN_PA3, 1);
+			at91_set_B_periph(AT91_PIN_PA4, 1);
+			at91_set_B_periph(AT91_PIN_PA5, 1);
+			break;
+	}
+}
+#endif /* ATMEL_MCI */
+
 #if defined(CONFIG_HAS_DATAFLASH) || defined(CONFIG_ATMEL_SPI)
 void at91_spi0_hw_init(unsigned long cs_mask)
 {
diff --git a/cpu/arm926ejs/at91/at91sam9261_devices.c b/cpu/arm926ejs/at91/at91sam9261_devices.c
index 16d411f..e777367 100644
--- a/cpu/arm926ejs/at91/at91sam9261_devices.c
+++ b/cpu/arm926ejs/at91/at91sam9261_devices.c
@@ -75,6 +75,24 @@ void at91_serial_hw_init(void)
 #endif
 }
 
+#ifdef CONFIG_ATMEL_MCI
+void at91_mci0_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9261_ID_MCI);
+	at91_set_B_periph(AT91_PIN_PA2, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_B_periph(AT91_PIN_PA0, 1);
+			at91_set_B_periph(AT91_PIN_PA1, 1);
+			at91_set_B_periph(AT91_PIN_PA4, 1);
+			at91_set_B_periph(AT91_PIN_PA5, 1);
+			at91_set_B_periph(AT91_PIN_PA6, 1);
+			break;
+	}
+}
+#endif /* ATMEL_MCI */
+
 #ifdef CONFIG_HAS_DATAFLASH
 void at91_spi0_hw_init(unsigned long cs_mask)
 {
diff --git a/cpu/arm926ejs/at91/at91sam9263_devices.c b/cpu/arm926ejs/at91/at91sam9263_devices.c
index f72efdf..e861108 100644
--- a/cpu/arm926ejs/at91/at91sam9263_devices.c
+++ b/cpu/arm926ejs/at91/at91sam9263_devices.c
@@ -79,6 +79,56 @@ void at91_serial_hw_init(void)
 #endif
 }
 
+#ifdef CONFIG_ATMEL_MCI
+#ifdef CONFIG_AT91_MCI0
+void at91_mci0_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9263_ID_MCI0);
+	at91_set_A_periph(AT91_PIN_PA12, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_A_periph(AT91_PIN_PA0, 1);
+			at91_set_A_periph(AT91_PIN_PA1, 1);
+			at91_set_A_periph(AT91_PIN_PA3, 1);
+			at91_set_A_periph(AT91_PIN_PA4, 1);
+			at91_set_A_periph(AT91_PIN_PA5, 1);
+			break;
+		case 1:
+			at91_set_A_periph(AT91_PIN_PA16, 1);
+			at91_set_A_periph(AT91_PIN_PA17, 1);
+			at91_set_A_periph(AT91_PIN_PA18, 1);
+			at91_set_A_periph(AT91_PIN_PA19, 1);
+			at91_set_A_periph(AT91_PIN_PA20, 1);
+			break;
+	}
+}
+#elif defined(CONFIG_AT91_MCI1
+void at91_mci1_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9263_ID_MCI);
+	at91_set_A_periph(AT91_PIN_PA6, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_A_periph(AT91_PIN_PA7, 1);
+			at91_set_A_periph(AT91_PIN_PA8, 1);
+			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:
+			at91_set_B_periph(AT91_PIN_PA21, 1);
+			at91_set_B_periph(AT91_PIN_PA22, 1);
+			at91_set_B_periph(AT91_PIN_PA23, 1);
+			at91_set_B_periph(AT91_PIN_PA24, 1);
+			at91_set_B_periph(AT91_PIN_PA25, 1);
+			break;
+	}
+}
+#endif
+#endif
+
 #ifdef CONFIG_HAS_DATAFLASH
 void at91_spi0_hw_init(unsigned long cs_mask)
 {
diff --git a/cpu/arm926ejs/at91/at91sam9m10g45_devices.c b/cpu/arm926ejs/at91/at91sam9m10g45_devices.c
index 98d90f2..8b2d210 100644
--- a/cpu/arm926ejs/at91/at91sam9m10g45_devices.c
+++ b/cpu/arm926ejs/at91/at91sam9m10g45_devices.c
@@ -75,6 +75,56 @@ void at91_serial_hw_init(void)
 #endif
 }
 
+#ifdef CONFIG_ATMEL_MCI
+#ifdef CONFIG_AT91_MCI0
+void at91_mci0_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9263_ID_MCI0);
+	at91_set_A_periph(AT91_PIN_PA12, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_A_periph(AT91_PIN_PA0, 1);
+			at91_set_A_periph(AT91_PIN_PA1, 1);
+			at91_set_A_periph(AT91_PIN_PA3, 1);
+			at91_set_A_periph(AT91_PIN_PA4, 1);
+			at91_set_A_periph(AT91_PIN_PA5, 1);
+			break;
+		case 1:
+			at91_set_A_periph(AT91_PIN_PA16, 1);
+			at91_set_A_periph(AT91_PIN_PA17, 1);
+			at91_set_A_periph(AT91_PIN_PA18, 1);
+			at91_set_A_periph(AT91_PIN_PA19, 1);
+			at91_set_A_periph(AT91_PIN_PA20, 1);
+			break;
+	}
+}
+#elif defined(CONFIG_AT91_MCI1
+void at91_mci1_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9263_ID_MCI);
+	at91_set_A_periph(AT91_PIN_PA6, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_A_periph(AT91_PIN_PA7, 1);
+			at91_set_A_periph(AT91_PIN_PA8, 1);
+			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:
+			at91_set_B_periph(AT91_PIN_PA0, 1);
+			at91_set_B_periph(AT91_PIN_PA1, 1);
+			at91_set_B_periph(AT91_PIN_PA3, 1);
+			at91_set_B_periph(AT91_PIN_PA4, 1);
+			at91_set_B_periph(AT91_PIN_PA5, 1);
+			break;
+	}
+}
+#endif
+#endif
+
 #ifdef CONFIG_ATMEL_SPI
 void at91_spi0_hw_init(unsigned long cs_mask)
 {
diff --git a/cpu/arm926ejs/at91/at91sam9rl_devices.c b/cpu/arm926ejs/at91/at91sam9rl_devices.c
index ebed193..1839b0c 100644
--- a/cpu/arm926ejs/at91/at91sam9rl_devices.c
+++ b/cpu/arm926ejs/at91/at91sam9rl_devices.c
@@ -75,6 +75,28 @@ void at91_serial_hw_init(void)
 #endif
 }
 
+/* 
+ * The AT91SAM9RL64 is said to have 2 slots, but the datasheet doesn't
+ * seem to mention to what pins the second slot is assigned
+ */
+#ifdef CONFIG_ATMEL_MCI
+void at91_mci0_hw_init(int slot)
+{
+	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9260_ID_MCI);
+	at91_set_A_periph(AT91_PIN_PA2, 0);
+
+	switch(slot) {
+		case 0:
+			at91_set_A_periph(AT91_PIN_PA0, 1);
+			at91_set_A_periph(AT91_PIN_PA1, 1);
+			at91_set_A_periph(AT91_PIN_PA3, 1);
+			at91_set_A_periph(AT91_PIN_PA4, 1);
+			at91_set_A_periph(AT91_PIN_PA5, 1);
+			break;
+	}
+}
+#endif /* ATMEL_MCI */
+
 #ifdef CONFIG_HAS_DATAFLASH
 void at91_spi0_hw_init(unsigned long cs_mask)
 {
diff --git a/cpu/arm926ejs/at91/cpu.c b/cpu/arm926ejs/at91/cpu.c
index f2f7b62..f96b715 100644
--- a/cpu/arm926ejs/at91/cpu.c
+++ b/cpu/arm926ejs/at91/cpu.c
@@ -52,3 +52,10 @@ int print_cpuinfo(void)
 	return 0;
 }
 #endif
+
+#ifdef CONFIG_GENERIC_MMC
+int cpu_mmc_init(bd_t *bis)
+{
+	atmel_mci_init(bis);
+}
+#endif /* GENERIC_MMC */
diff --git a/include/asm-arm/arch-at91/at91_common.h b/include/asm-arm/arch-at91/at91_common.h
index 01840ee..4bb9887 100644
--- a/include/asm-arm/arch-at91/at91_common.h
+++ b/include/asm-arm/arch-at91/at91_common.h
@@ -32,6 +32,8 @@ void at91_serial0_hw_init(void);
 void at91_serial1_hw_init(void);
 void at91_serial2_hw_init(void);
 void at91_serial3_hw_init(void);
+void at91_mci0_hw_init(int);
+void at91_mci1_hw_init(int);
 void at91_spi0_hw_init(unsigned long cs_mask);
 void at91_spi1_hw_init(unsigned long cs_mask);
 void at91_uhp_hw_init(void);
diff --git a/include/asm-arm/arch-at91/clk.h b/include/asm-arm/arch-at91/clk.h
index f642dd9..457e6c9 100644
--- a/include/asm-arm/arch-at91/clk.h
+++ b/include/asm-arm/arch-at91/clk.h
@@ -59,5 +59,10 @@ static inline unsigned long get_twi_clk_rate(unsigned int dev_id)
 	return get_mck_clk_rate();
 }
 
+static inline unsigned long get_mci_clk_rate(void)
+{
+	return get_mck_clk_rate();
+}
+
 int at91_clock_init(unsigned long main_clock);
 #endif /* __ASM_ARM_ARCH_CLK_H__ */
diff --git a/include/asm-arm/arch-at91/hardware.h b/include/asm-arm/arch-at91/hardware.h
index de06a10..6430ade 100644
--- a/include/asm-arm/arch-at91/hardware.h
+++ b/include/asm-arm/arch-at91/hardware.h
@@ -20,6 +20,7 @@
 #include <asm/arch/at91rm9200.h>
 #elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
 #include <asm/arch/at91sam9260.h>
+#define AT91_BASE_MCI	AT91SAM9260_BASE_MCI
 #define AT91_BASE_SPI	AT91SAM9260_BASE_SPI0
 #define AT91_ID_UHP	AT91SAM9260_ID_UHP
 #define AT91_PMC_UHP	AT91SAM926x_PMC_UHP
diff --git a/include/asm-arm/arch-at91/memory-map.h b/include/asm-arm/arch-at91/memory-map.h
index f605f37..300b61d 100644
--- a/include/asm-arm/arch-at91/memory-map.h
+++ b/include/asm-arm/arch-at91/memory-map.h
@@ -31,5 +31,6 @@
 #define USART2_BASE AT91_USART2
 #define USART3_BASE (AT91_BASE_SYS + AT91_DBGU)
 #define SPI0_BASE	AT91_BASE_SPI
+#define MMCI_BASE AT91_BASE_MCI
 
 #endif /* __ASM_ARM_ARCH_MEMORYMAP_H__ */
-- 
Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Sami Kantoluoto]

On Sun, Aug 30, 2009 at 01:08:27AM +0200, Albin Tonnerre wrote:
> On Sat, Aug 29, 2009 at 08:18:32PM +0300, Sami Kantoluoto wrote :
> > Fixed to parse CSD correctly on little endian processors as gcc orders
> > bitfields differently between big and little endian ones.
> 
> Please also see this patch, which will fix those bugs as weel, while switching
> to the new GENRIC_MMC API:
> http://lists.denx.de/pipermail/u-boot/2009-August/059456.html
> I'd highly appreciate if you could test it, to get some feedback

Thanks, I'll test when I get some time later this week but I think (by 
reading the patch so I probably missed something) it won't solve the CSD
problem. The real reason of the "CSD problem" of course is that how the
mmc_csd structure is defined (host byte order not taken in account or
at least how gcc handles bitfields).

[snip]

> the configuration for the MCI controller, which should be done in the
> at91sam*_devices.c files. See the attached patch (relying on the patch
> mentionned above) that adds such support. It's not complete yet either: it
> partially lacks support for models which have 2 MCI controllers, but
> that's not the case of the 9G20 anyway (that's just a matter of putting
> proper ifdefs before defining AT91_BASE_MCI).
> If what I've done in this patch is not the way to go, I'd appreciate
> guidance on how to do it correctly.

It seems ok to me except one comment about the interface:

> diff --git a/include/asm-arm/arch-at91/at91_common.h b/include/asm-arm/arch-at91/at91_common.h
>
> +void at91_mci0_hw_init(int);
> +void at91_mci1_hw_init(int);

I'm just wondering if the bus width should be configurable. It's probably
quite unusual to use just one data bit but not impossible though. So maybe
the interface call could be:

void at91_mci0_hw_init(int bwForSlotA, bwForSlotB)

Where the bwForSlot<X> is either 0 (which means slot is not used), 1 or 4.


     -sk

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Albin Tonnerre]

On Mon, Aug 31, 2009 at 02:22:47PM +0300, Sami Kantoluoto wrote :
> On Sun, Aug 30, 2009 at 01:08:27AM +0200, Albin Tonnerre wrote:
> > On Sat, Aug 29, 2009 at 08:18:32PM +0300, Sami Kantoluoto wrote :
> > > Fixed to parse CSD correctly on little endian processors as gcc orders
> > > bitfields differently between big and little endian ones.
> > 
> > Please also see this patch, which will fix those bugs as weel, while switching
> > to the new GENRIC_MMC API:
> > http://lists.denx.de/pipermail/u-boot/2009-August/059456.html
> > I'd highly appreciate if you could test it, to get some feedback
> 
> Thanks, I'll test when I get some time later this week but I think (by 
> reading the patch so I probably missed something) it won't solve the CSD
> problem. The real reason of the "CSD problem" of course is that how the
> mmc_csd structure is defined (host byte order not taken in account or
> at least how gcc handles bitfields).
> 

drivers/mmc/mmc.c does not actually use the bitfield to parse the csd struct,
and got fixed a while back to work no matter what endianness you're using, so it
should solve it anyway.

> [snip]
> 
> > the configuration for the MCI controller, which should be done in the
> > at91sam*_devices.c files. See the attached patch (relying on the patch
> > mentionned above) that adds such support. It's not complete yet either: it
> > partially lacks support for models which have 2 MCI controllers, but
> > that's not the case of the 9G20 anyway (that's just a matter of putting
> > proper ifdefs before defining AT91_BASE_MCI).
> > If what I've done in this patch is not the way to go, I'd appreciate
> > guidance on how to do it correctly.
> 
> It seems ok to me except one comment about the interface:
> 
> > diff --git a/include/asm-arm/arch-at91/at91_common.h b/include/asm-arm/arch-at91/at91_common.h
> >
> > +void at91_mci0_hw_init(int);
> > +void at91_mci1_hw_init(int);
> 
> I'm just wondering if the bus width should be configurable. It's probably
> quite unusual to use just one data bit but not impossible though. So maybe
> the interface call could be:
> 
> void at91_mci0_hw_init(int bwForSlotA, bwForSlotB)
> 
> Where the bwForSlot<X> is either 0 (which means slot is not used), 1 or 4.

I guess I'll just use something similar to what the spi init function does (eg.
take only 1 argument, and use 1 << 0 for slot A/B, 1 << 1 for 1wire/4wires)

Thanks for your comments.

Regards,
-- 
Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Sami Kantoluoto]

On Mon, Aug 31, 2009 at 01:39:26PM +0200, Albin Tonnerre wrote:
> On Mon, Aug 31, 2009 at 02:22:47PM +0300, Sami Kantoluoto wrote :
> > On Sun, Aug 30, 2009 at 01:08:27AM +0200, Albin Tonnerre wrote:
> > > On Sat, Aug 29, 2009 at 08:18:32PM +0300, Sami Kantoluoto wrote :
> > > > Fixed to parse CSD correctly on little endian processors as gcc orders
> > > > bitfields differently between big and little endian ones.
> > > 
> > > Please also see this patch, which will fix those bugs as weel, while switching
> > > to the new GENRIC_MMC API:
> > > http://lists.denx.de/pipermail/u-boot/2009-August/059456.html
> > > I'd highly appreciate if you could test it, to get some feedback
> > 
> > Thanks, I'll test when I get some time later this week but I think (by 
> > reading the patch so I probably missed something) it won't solve the CSD
> > problem. The real reason of the "CSD problem" of course is that how the
> > mmc_csd structure is defined (host byte order not taken in account or
> > at least how gcc handles bitfields).
> 
> drivers/mmc/mmc.c does not actually use the bitfield to parse the csd struct,
> and got fixed a while back to work no matter what endianness you're
> using, so it should solve it anyway.

Ok. Like I said (in other post) I was using the older u-boot version before
so I'm not too familiar with current one (and wasn't actually with the old
one either ;-). I'll try to do my homework better next time!


     -sk

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Sami Kantoluoto]

On Mon, Aug 31, 2009 at 01:39:26PM +0200, Albin Tonnerre wrote:
> On Mon, Aug 31, 2009 at 02:22:47PM +0300, Sami Kantoluoto wrote :
> > On Sun, Aug 30, 2009 at 01:08:27AM +0200, Albin Tonnerre wrote:
> > > On Sat, Aug 29, 2009 at 08:18:32PM +0300, Sami Kantoluoto wrote :
> > > > Fixed to parse CSD correctly on little endian processors as gcc orders
> > > > bitfields differently between big and little endian ones.
> > > 
> > > Please also see this patch, which will fix those bugs as weel, while switching
> > > to the new GENRIC_MMC API:
> > > http://lists.denx.de/pipermail/u-boot/2009-August/059456.html
> > > I'd highly appreciate if you could test it, to get some feedback
> > 
> > Thanks, I'll test when I get some time later this week but I think (by 
> > reading the patch so I probably missed something) it won't solve the CSD
> > problem. The real reason of the "CSD problem" of course is that how the
> > mmc_csd structure is defined (host byte order not taken in account or
> > at least how gcc handles bitfields).
> > 
> 
> drivers/mmc/mmc.c does not actually use the bitfield to parse the csd struct,
> and got fixed a while back to work no matter what endianness you're using, so it
> should solve it anyway.

I just tested the patch (+ your at91_mci patch + my new patches) and this
seems to work just find.

Thanks!


Best Regards,

    -sk

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Jean-Christophe PLAGNIOL-VILLARD]

> 
> diff --git a/cpu/arm926ejs/at91/at91cap9_devices.c b/cpu/arm926ejs/at91/at91cap9_devices.c
> index 39e405f..2dc1984 100644
> --- a/cpu/arm926ejs/at91/at91cap9_devices.c
> +++ b/cpu/arm926ejs/at91/at91cap9_devices.c
> @@ -79,6 +79,42 @@ void at91_serial_hw_init(void)
>  #endif
>  }
>  
> +#ifdef CONFIG_ATMEL_MCI
> +#ifdef CONFIG_AT91_MCI0
> +void at91_mci0_hw_init(int slot)
> +{
> +	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9263_ID_MCI0);
please use correct SOC cloclk marco everywhere
> +	at91_set_A_periph(AT91_PIN_PA2, 0);
> +
> +	switch(slot) {
why switch slot?
you are already in mci0 and there is only one for the cap9
> +		case 0:
> +			at91_set_A_periph(AT91_PIN_PA0, 1);
> +			at91_set_A_periph(AT91_PIN_PA1, 1);
> +			at91_set_A_periph(AT91_PIN_PA3, 1);
> +			at91_set_A_periph(AT91_PIN_PA4, 1);
> +			at91_set_A_periph(AT91_PIN_PA5, 1);
you need to add an option to PA3,4,5 only in 4 wire mode
and it's will be also available for the other hw_init
> +			break;
> +	}
> +}
> +#elif defined(CONFIG_AT91_MCI1
miss a ')' and on some other
> +void at91_mci1_hw_init(int slot)
> +{
> +	at91_sys_write(AT91_PMC_PCER, 1 << AT91SAM9263_ID_MCI);
> +	at91_set_A_periph(AT91_PIN_PA16, 0);
> +
> +	switch(slot) {
why switch slot?
you are already in mci1 and there is only one for the cap9
> +		case 0:
> +			at91_set_A_periph(AT91_PIN_PA17, 1);
> +			at91_set_A_periph(AT91_PIN_PA18, 1);
> +			at91_set_A_periph(AT91_PIN_PA19, 1);
> +			at91_set_A_periph(AT91_PIN_PA20, 1);
> +			at91_set_A_periph(AT91_PIN_PA21, 1);
> +			break;
> +	}
> +}
> +#endif
> +#endif
> +
> diff --git a/include/asm-arm/arch-at91/hardware.h b/include/asm-arm/arch-at91/hardware.h
> index de06a10..6430ade 100644
> --- a/include/asm-arm/arch-at91/hardware.h
> +++ b/include/asm-arm/arch-at91/hardware.h
> @@ -20,6 +20,7 @@
>  #include <asm/arch/at91rm9200.h>
>  #elif defined(CONFIG_AT91SAM9260) || defined(CONFIG_AT91SAM9G20)
>  #include <asm/arch/at91sam9260.h>
> +#define AT91_BASE_MCI	AT91SAM9260_BASE_MCI
please use MCI0_BASE and MCI1_BASE so we can detect if the soc support
multiple mci and please move it to soc header
I'll send a patch to clean the other
>  #define AT91_BASE_SPI	AT91SAM9260_BASE_SPI0
>  #define AT91_ID_UHP	AT91SAM9260_ID_UHP
>  #define AT91_PMC_UHP	AT91SAM926x_PMC_UHP

Best Regards,
J.

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Albin Tonnerre]

Oh ... I had a more recent patch, but looks like it didn't make it to the list.
It already fixes a large parts of your comments. Thanks for the review

> please use MCI0_BASE and MCI1_BASE so we can detect if the soc support
> multiple mci and please move it to soc header
> I'll send a patch to clean the other

I'm not sure what other you're talking about

Regards,
-- 
Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: Digital signature
Url : http://lists.denx.de/pipermail/u-boot/attachments/20090905/fe88b078/attachment.pgp 

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Jean-Christophe PLAGNIOL-VILLARD]

On 11:03 Sat 05 Sep     , Albin Tonnerre wrote:
> Oh ... I had a more recent patch, but looks like it didn't make it to the list.
> It already fixes a large parts of your comments. Thanks for the review
> 
> > please use MCI0_BASE and MCI1_BASE so we can detect if the soc support
> > multiple mci and please move it to soc header
> > I'll send a patch to clean the other
> 
> I'm not sure what other you're talking about
take a look in the atmel_spi driver

the number of MCI will be detected depending on the MCIx_BASE declared in the
soc header

Best Regards,
J.

[U-Boot] [PATCH] MCI support for AT91 family processors.

[Albin Tonnerre]

[Ccing Haavard, as he's the maintainer for the atmel_mci driver]

Hi Sami,

On Sat, 29 Aug 2009 20:18 +0300, Sami Kantoluoto wrote :
> Fixed to parse CSD correctly on little endian processors as gcc orders
> bitfields differently between big and little endian ones.
> 
> Signed-off-by: Sami Kantoluoto <sami.kantoluoto@embedtronics.fi>

>  static void mmc_dump_cid(const struct mmc_cid *cid)
>  {
>  	printf("Manufacturer ID:       %02X\n", cid->mid);
> @@ -298,7 +345,7 @@ static void mmc_dump_csd(const struct mmc_csd *csd)
>  {
>  	unsigned long *csd_raw = (unsigned long *)csd;
>  	printf("CSD data: %08lx %08lx %08lx %08lx\n",
> -	       csd_raw[0], csd_raw[1], csd_raw[2], csd_raw[3]);
> +	       ntohl(csd_raw[0]), ntohl(csd_raw[1]), ntohl(csd_raw[2]), ntohl(csd_raw[3]));

I guess it's mostly a matter of preference, but I'd tend to use be32_to_cpu for
things that aren't network-related.

>  	printf("CSD structure version:   1.%u\n", csd->csd_structure);
>  	printf("MMC System Spec version: %u\n", csd->spec_vers);
>  	printf("Card command classes:    %03x\n", csd->ccc);
> @@ -368,7 +415,7 @@ static int sd_init_card(struct mmc_cid *cid, int verbose)
>  
>  	/* Get RCA of the card that responded */
>  	ret = mmc_cmd(SD_CMD_SEND_RELATIVE_ADDR, 0, resp, R6 | NCR);
> -	if (ret)
> +  	if (ret)
>  		return ret;

Looks like a random whitespace change, it's generally best to avoid that.

>  	mmc_rca = resp[0] >> 16;
> @@ -468,6 +515,7 @@ int mmc_legacy_init(int verbose)
>  	struct mmc_cid cid;
>  	struct mmc_csd csd;
>  	unsigned int max_blksz;
> +	unsigned long resp[4];
>  	int ret;
>  
>  	/* Initialize controller */
> @@ -488,9 +536,10 @@ int mmc_legacy_init(int verbose)
>  		return ret;
>  
>  	/* Get CSD from the card */
> -	ret = mmc_cmd(MMC_CMD_SEND_CSD, mmc_rca << 16, &csd, R2 | NCR);
> +	ret = mmc_cmd(MMC_CMD_SEND_CSD, mmc_rca << 16, resp, R2 | NCR);
>  	if (ret)
>  		return ret;
> +	mmc_parse_csd(&csd, resp);
>  	if (verbose)
>  		mmc_dump_csd(&csd);
>  
> diff --git a/include/asm-arm/arch-at91/clk.h b/include/asm-arm/arch-at91/clk.h
> index f642dd9..26b537c 100644
> --- a/include/asm-arm/arch-at91/clk.h
> +++ b/include/asm-arm/arch-at91/clk.h
> @@ -54,6 +54,11 @@ static inline unsigned long get_spi_clk_rate(unsigned int dev_id)
>  	return get_mck_clk_rate();
>  }
>  
> +static inline unsigned long get_mci_clk_rate(void)
> +{
> +	return get_mck_clk_rate();
> +}
> +
>  static inline unsigned long get_twi_clk_rate(unsigned int dev_id)
>  {
>  	return get_mck_clk_rate();

Please remove this part, as it's not related to the bug you're fixing in
atmel_mci ? and that's part of the AT91 MCI support patch I sent anyway :)

Other than that, I've tried it on my AT91-based boards and that works fine.
Thanks!

Tested-by: Albin Tonnerre <albin.tonnerre@free-electrons.com>

Regards,
-- 
Albin Tonnerre, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com