mmc: at91_mci: rework to allow better transfer

Hi Pierre,

Following your announce of future removal of the MMC multiwrite switch and the contributions of several people of the AT91 community, here is a patches series :
Those patches should enhance transfer of data and setup a sane base for a working SDIO support.

Several of those contributions are around since the beginning of this year. I thank all of you for your patient and valuable testing effort. Here are some discussions that led to this patch series:

"mmc: at91_mci: support for block size not modulo 4"
http://lists.arm.linux.org.uk/lurker/message/20080107.141550.ea58119f.en.html

AT91 SDIO + 8686
http://lists.arm.linux.org.uk/lurker/message/20071114.094824.3ebcbe04.en.html

"8686 with SDIO on AT91SAM9263"
http://lists.arm.linux.org.uk/lurker/message/20080123.182026.f3c54a8a.en.html

"[PATCH 0/2] at91_mci: manage timeouts"
http://lists.arm.linux.org.uk/lurker/message/20080125.075058.257a2582.en.html

"[PATCH 0/3] at91_mci: fix SD filesystem corruption on sam9261"
http://lists.arm.linux.org.uk/lurker/message/20080318.145450.7b443217.en.html

Regards,

Nicolas Ferre

From: Marc Pignat <[email protected]>

Implement transfer with size not modulo 4 for at91sam9*. Please note that the at91rm9200 simply can't handle this.

---
drivers/mmc/host/at91_mci.c | 15 ++++++++++-----

1 files changed, 10 insertions(+), 5 deletions(-)

index 8979ad3..543b64b 100644
@@ -233,11 +233,11 @@ static void at91_mci_pre_dma_read(struct at91mci_host *host)
 

		if (i == 0) {
			at91_mci_write(host, ATMEL_PDC_RPR, sg->dma_address);
-			at91_mci_write(host, ATMEL_PDC_RCR, sg->length / 4);
+			at91_mci_write(host, ATMEL_PDC_RCR, (data->blksz & 0x3) ? sg->length : sg->length / 4);
		}
		else {
			at91_mci_write(host, ATMEL_PDC_RNPR, sg->dma_address);
-			at91_mci_write(host, ATMEL_PDC_RNCR, sg->length / 4);
+			at91_mci_write(host, ATMEL_PDC_RNCR, (data->blksz & 0x3) ? sg->length : sg->length / 4);
		}
	}

@@ -430,7 +430,7 @@ static void at91_mci_send_command(struct at91mci_host *host, struct mmc_command

	if (data) {

- if ( data->blksz & 0x3 ) {

+		if ( cpu_is_at91rm9200() && (data->blksz & 0x3) ) {
			pr_debug("Unsupported block size\n");
			cmd->error = -EINVAL;
			mmc_request_done(host->mmc, host->request);
@@ -482,7 +482,10 @@ static void at91_mci_send_command(struct at91mci_host *host, struct mmc_command
	} else {
		/* zero block length and PDC mode */
		mr = at91_mci_read(host, AT91_MCI_MR) & 0x7fff;
-		at91_mci_write(host, AT91_MCI_MR, mr | (block_length << 16) | AT91_MCI_PDCMODE);
+		mr |= (data->blksz & 0x3) ? AT91_MCI_PDCFBYTE : 0;
+		mr |= (block_length << 16);
+		mr |= AT91_MCI_PDCMODE;

+ at91_mci_write(host, AT91_MCI_MR, mr);

		/*
		 * Disable the PDC controller
@@ -517,7 +520,9 @@ static void at91_mci_send_command(struct at91mci_host *host, struct mmc_command
				pr_debug("Transmitting %d bytes\n", host->total_length);

				at91_mci_write(host, ATMEL_PDC_TPR, host->physical_address);

- at91_mci_write(host, ATMEL_PDC_TCR, host->total_length / 4);
+ at91_mci_write(host, ATMEL_PDC_TCR, (data->blksz & 0x3) ?
+ host->total_length : host->total_length / 4);
+

				ier = AT91_MCI_CMDRDY;
			}
		}

1.5.3.7

From: Marc Pignat <[email protected]>

Detect command timeout (or mci controller hangs).

---
drivers/mmc/host/at91_mci.c | 35 +++++++++++++++++++++++++++++++++--

1 files changed, 33 insertions(+), 2 deletions(-)

index 543b64b..4b4518f 100644
@@ -125,8 +125,33 @@ struct at91mci_host
 

	/* Latest in the scatterlist that has been enabled for transfer */
	int transfer_index;

+
+ /* Timer for timeouts */
+ struct timer_list timer;
};

+static void at91_timeout_timer(unsigned long data)
+{
+ struct at91mci_host *host;
+
+ host = (struct at91mci_host *)data;
+
+ if (host->request) {
+ dev_err(host->mmc->parent, "Timeout waiting end of packet\n");
+
+ if (host->cmd && host->cmd->data) {
+ host->cmd->data->error = -ETIMEDOUT;
+ } else {
+ if (host->cmd)
+ host->cmd->error = -ETIMEDOUT;
+ else
+ host->request->cmd->error = -ETIMEDOUT;
+ }
+
+ mmc_request_done(host->mmc, host->request);
+ }
+}
+
/*
* Copy from sg to a dma block - used for transfers
*/
@@ -557,9 +582,10 @@ static void at91_mci_process_next(struct at91mci_host *host)

	else if ((!(host->flags & FL_SENT_STOP)) && host->request->stop) {
		host->flags |= FL_SENT_STOP;
		at91_mci_send_command(host, host->request->stop);
-	}
-	else
+	} else {
+		del_timer(&host->timer);
		mmc_request_done(host->mmc, host->request);

+ }
}

/*
@@ -618,6 +644,8 @@ static void at91_mci_request(struct mmc_host *mmc, struct mmc_request *mrq)

	host->request = mrq;
	host->flags = 0;

+ mod_timer(&host->timer, jiffies + HZ);
+

	at91_mci_process_next(host);
}

@@ -940,6 +968,8 @@ static int __init at91_mci_probe(struct platform_device *pdev)

	mmc_add_host(mmc);

+ setup_timer(&host->timer, at91_timeout_timer, (unsigned long)host);
+

	/*
	 * monitor card insertion/removal if we can
	 */
@@ -1000,6 +1030,7 @@ static int __exit at91_mci_remove(struct platform_device *pdev)
	}

	at91_mci_disable(host);
+	del_timer_sync(&host->timer);
	mmc_remove_host(mmc);
	free_irq(host->irq, host);

1.5.3.7

From: Marc Pignat <[email protected]>

The at91 mci controller internal state machine seems to often crash. This can be fixed by resetting the controller after each command for at91rm9200 and by setting the MCI_BLKR register on at91sam926*.

---
drivers/mmc/host/at91_mci.c | 48 ++++++++++++++++++++++++++++++++++
include/asm-arm/arch-at91/at91_mci.h | 4 +++

2 files changed, 52 insertions(+), 0 deletions(-)

index 4b4518f..6b9662e 100644
@@ -130,6 +130,43 @@ struct at91mci_host

	struct timer_list timer;
};

+/*
+ * Reset the controller and restore most of the state
+ */
+static void at91_reset_host(struct at91mci_host *host) +{
+ unsigned long flags;
+ u32 mr;
+ u32 sdcr;
+ u32 dtor;
+ u32 imr;
+
+ local_irq_save(flags);
+ imr = at91_mci_read(host, AT91_MCI_IMR);
+
+ at91_mci_write(host, AT91_MCI_IDR, 0xffffffff);
+
+ /* save current state */
+ mr = at91_mci_read(host, AT91_MCI_MR) & 0x7fff;
+ sdcr = at91_mci_read(host, AT91_MCI_SDCR);
+ dtor = at91_mci_read(host, AT91_MCI_DTOR);
+
+ /* reset the controller */
+ at91_mci_write(host, AT91_MCI_CR, AT91_MCI_MCIDIS | AT91_MCI_SWRST);
+
+ /* restore state */
+ at91_mci_write(host, AT91_MCI_CR, AT91_MCI_MCIEN);
+ at91_mci_write(host, AT91_MCI_MR, mr);
+ at91_mci_write(host, AT91_MCI_SDCR, sdcr);
+ at91_mci_write(host, AT91_MCI_DTOR, dtor);
+ at91_mci_write(host, AT91_MCI_IER, imr);
+
+ /* make sure sdio interrupts will fire */
+ at91_mci_read(host, AT91_MCI_SR);
+
+ local_irq_restore(flags);
+}
+
static void at91_timeout_timer(unsigned long data)

{
	struct at91mci_host *host;
@@ -148,6 +185,7 @@ static void at91_timeout_timer(unsigned long data)
				host->request->cmd->error = -ETIMEDOUT;
		}

+		at91_reset_host(host);
		mmc_request_done(host->mmc, host->request);
	}
}
@@ -512,6 +550,11 @@ static void at91_mci_send_command(struct at91mci_host *host, struct mmc_command
		mr |= AT91_MCI_PDCMODE;
		at91_mci_write(host, AT91_MCI_MR, mr);

+ if (!cpu_is_at91rm9200())
+ at91_mci_write(host, AT91_MCI_BLKR,
+ AT91_MCI_BLKR_BCNT(blocks) |
+ AT91_MCI_BLKR_BLKLEN(block_length));
+

		/*
		 * Disable the PDC controller
		 */
@@ -584,6 +627,11 @@ static void at91_mci_process_next(struct at91mci_host *host)
		at91_mci_send_command(host, host->request->stop);
	} else {
		del_timer(&host->timer);

+ /* the at91rm9200 mci controller hangs after some transfers,
+ * and the workaround is to reset it after each transfer.
+ */
+ if (cpu_is_at91rm9200())

+			at91_reset_host(host);
		mmc_request_done(host->mmc, host->request);
	}
}

index 1551fc2..400ec10 100644
@@ -75,6 +75,10 @@

#define AT91_MCI_TRTYP_MULTIPLE (1 << 19)
#define AT91_MCI_TRTYP_STREAM (2 << 19)

+#define AT91_MCI_BLKR 0x18 /* Block Register */
+#define AT91_MCI_BLKR_BCNT(n) ((0xffff & (n)) << 0) /* Block count */
+#define AT91_MCI_BLKR_BLKLEN(n) ((0xffff & (n)) << 16) /* Block lenght */
+
#define AT91_MCI_RSPR(n) (0x20 + ((n) * 4)) /* Response Registers 0-3 */
#define AT91_MCR_RDR 0x30 /* Receive Data Register */
#define AT91_MCR_TDR 0x34 /* Transmit Data Register */

1.5.3.7

at91_mci is capable of multiwrite. Enable it before it disappears.

---
drivers/mmc/host/at91_mci.c | 1 +

1 files changed, 1 insertions(+), 0 deletions(-)

index 6b9662e..df975e0 100644
@@ -923,6 +923,7 @@ static int __init at91_mci_probe(struct platform_device *pdev)

	mmc->f_min = 375000;
	mmc->f_max = 25000000;
	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;

+ mmc->caps = MMC_CAP_MULTIWRITE;

	mmc->max_blk_size = 4095;
	mmc->max_blk_count = mmc->max_req_size;

1.5.3.7

Modify bytes_xfered value after a write.

That will report, as accurately as possible, the amount of sectors that are effectively written.

This update introduces the check of the busy signal given by the card.

---
drivers/mmc/host/at91_mci.c | 40 ++++++++++++++++++++++++++++++++++------

1 files changed, 34 insertions(+), 6 deletions(-)

index df975e0..b2138f9 100644
@@ -340,8 +340,6 @@ static void at91_mci_post_dma_read(struct at91mci_host *host)
 

		dma_unmap_page(NULL, sg->dma_address, sg->length, DMA_FROM_DEVICE);

- data->bytes_xfered += sg->length;
-

		if (cpu_is_at91rm9200()) {	/* AT91RM9200 errata */
			unsigned int *buffer;
			int index;
@@ -357,6 +355,8 @@ static void at91_mci_post_dma_read(struct at91mci_host *host)
		}

		flush_dcache_page(sg_page(sg));
+
+		data->bytes_xfered += sg->length;
	}

	/* Is there another transfer to trigger? */
@@ -397,10 +397,32 @@ static void at91_mci_handle_transmitted(struct at91mci_host *host)
		at91_mci_write(host, AT91_MCI_IER, AT91_MCI_BLKE);
	} else
		at91_mci_write(host, AT91_MCI_IER, AT91_MCI_NOTBUSY);

+}
+
+/*
+ * Update bytes tranfered count during a write operation
+ */
+static void at91_mci_update_bytes_xfered(struct at91mci_host *host) +{
+ struct mmc_data *data;

- data->bytes_xfered = host->total_length;
+ /* always deal with the effective request (and not the current cmd) */
+
+ if (host->request->cmd && host->request->cmd->error != 0)
+ return;
+
+ if (host->request->data) {
+ data = host->request->data;
+ if (data->flags & MMC_DATA_WRITE) {
+ /* card is in IDLE mode now */
+ pr_debug("-> bytes_xfered %d, total_length = %d\n",
+ data->bytes_xfered, host->total_length);
+ data->bytes_xfered = host->total_length;
+ }
+ }
}

+
/*Handle after command sent ready*/
static int at91_mci_handle_cmdrdy(struct at91mci_host *host)
{
@@ -413,8 +435,7 @@ static int at91_mci_handle_cmdrdy(struct at91mci_host *host)

		} else return 1;
	} else if (host->cmd->data->flags & MMC_DATA_WRITE) {
		/*After sendding multi-block-write command, start DMA transfer*/
-		at91_mci_write(host, AT91_MCI_IER, AT91_MCI_TXBUFE);
-		at91_mci_write(host, AT91_MCI_IER, AT91_MCI_BLKE);
+		at91_mci_write(host, AT91_MCI_IER, AT91_MCI_TXBUFE | AT91_MCI_BLKE);
		at91_mci_write(host, ATMEL_PDC_PTCR, ATMEL_PDC_TXTEN);
	}

@@ -817,6 +838,7 @@ static irqreturn_t at91_mci_irq(int irq, void *devid)

		if (int_status & AT91_MCI_NOTBUSY) {
			pr_debug("Card is ready\n");
+			at91_mci_update_bytes_xfered(host);
			completed = 1;
		}

@@ -825,7 +847,13 @@ static irqreturn_t at91_mci_irq(int irq, void *devid)

		if (int_status & AT91_MCI_BLKE) {
			pr_debug("Block transfer has ended\n");

- completed = 1;
+ if (host->request->data && host->request->data->blocks > 1) {
+ /* multi block write : complete multi write
+ * command and send stop */
+ completed = 1;
+ } else {
+ at91_mci_write(host, AT91_MCI_IER, AT91_MCI_NOTBUSY);
+ }

		}

		if (int_status & AT91_MCI_TXRDY)

1.5.3.7

Doesn't this patch depend on patch 4? IOW, shouldn't you be correcting this before you enable the features which require this to be accurate?

From: Eric Benard <[email protected]>

Enable SDIO interrupt handling.

---
drivers/mmc/host/at91_mci.c | 26 ++++++++++++++++++++++----

1 files changed, 22 insertions(+), 4 deletions(-)

index b2138f9..92a0200 100644
@@ -665,7 +665,7 @@ static void at91_mci_completed_command(struct at91mci_host *host)

	struct mmc_command *cmd = host->cmd;
	unsigned int status;

- at91_mci_write(host, AT91_MCI_IDR, 0xffffffff);
+ at91_mci_write(host, AT91_MCI_IDR, 0xffffffff & ~(AT91_MCI_SDIOIRQA | AT91_MCI_SDIOIRQB));

	cmd->resp[0] = at91_mci_read(host, AT91_MCI_RSPR(0));
	cmd->resp[1] = at91_mci_read(host, AT91_MCI_RSPR(1));
@@ -856,6 +856,12 @@ static irqreturn_t at91_mci_irq(int irq, void *devid)
			}
		}

+ if (int_status & AT91_MCI_SDIOIRQA)
+ mmc_signal_sdio_irq(host->mmc);
+
+ if (int_status & AT91_MCI_SDIOIRQB)
+ mmc_signal_sdio_irq(host->mmc);
+

		if (int_status & AT91_MCI_TXRDY)
			pr_debug("Ready to transmit\n");

@@ -870,10 +876,10 @@ static irqreturn_t at91_mci_irq(int irq, void *devid)

	if (completed) {
		pr_debug("Completed command\n");
-		at91_mci_write(host, AT91_MCI_IDR, 0xffffffff);
+		at91_mci_write(host, AT91_MCI_IDR, 0xffffffff & ~(AT91_MCI_SDIOIRQA | AT91_MCI_SDIOIRQB));
		at91_mci_completed_command(host);
	} else

- at91_mci_write(host, AT91_MCI_IDR, int_status);
+ at91_mci_write(host, AT91_MCI_IDR, int_status & ~(AT91_MCI_SDIOIRQA | AT91_MCI_SDIOIRQB));

	return IRQ_HANDLED;
}
@@ -917,10 +923,22 @@ static int at91_mci_get_ro(struct mmc_host *mmc)
	return read_only;
}

+static void at91_mci_enable_sdio_irq(struct mmc_host *mmc, int enable) +{
+ struct at91mci_host *host = mmc_priv(mmc);
+
+ pr_debug("%s: sdio_irq %c : %s\n", mmc_hostname(host->mmc),
+ host->board->slot_b ? 'B':'A', enable ? "enable" : "disable");
+ at91_mci_write(host, enable ? AT91_MCI_IER : AT91_MCI_IDR,
+ host->board->slot_b ? AT91_MCI_SDIOIRQB : AT91_MCI_SDIOIRQA);
+
+}
+

static const struct mmc_host_ops at91_mci_ops = {
	.request	= at91_mci_request,
	.set_ios	= at91_mci_set_ios,
	.get_ro		= at91_mci_get_ro,

+ .enable_sdio_irq = at91_mci_enable_sdio_irq,
};

/*
@@ -951,7 +969,7 @@ static int __init at91_mci_probe(struct platform_device *pdev)

	mmc->f_min = 375000;
	mmc->f_max = 25000000;
	mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;

- mmc->caps = MMC_CAP_MULTIWRITE;
+ mmc->caps = MMC_CAP_MULTIWRITE | MMC_CAP_SDIO_IRQ;

	mmc->max_blk_size = 4095;
	mmc->max_blk_count = mmc->max_req_size;

1.5.3.7

Reading AT91_MCI_SR again at the end of transfer can corrupt the error reporting. Some fields in the SR register are read-and-clear.

---
drivers/mmc/host/at91_mci.c | 12 +++++-------

1 files changed, 5 insertions(+), 7 deletions(-)

index 92a0200..9948fe1 100644
@@ -660,10 +660,9 @@ static void at91_mci_process_next(struct at91mci_host *host)

/*
* Handle a command that has been completed
*/
-static void at91_mci_completed_command(struct at91mci_host *host) +static void at91_mci_completed_command(struct at91mci_host *host, unsigned int status)

{
	struct mmc_command *cmd = host->cmd;

- unsigned int status;

	at91_mci_write(host, AT91_MCI_IDR, 0xffffffff & ~(AT91_MCI_SDIOIRQA | AT91_MCI_SDIOIRQB));

@@ -677,10 +676,9 @@ static void at91_mci_completed_command(struct at91mci_host *host)

		host->buffer = NULL;
	}

- status = at91_mci_read(host, AT91_MCI_SR);
-
- pr_debug("Status = %08X [%08X %08X %08X %08X]\n",
- status, cmd->resp[0], cmd->resp[1], cmd->resp[2], cmd->resp[3]);
+ pr_debug("Status = %08X/%08x [%08X %08X %08X %08X]\n",
+ status, at91_mci_read(host, AT91_MCI_SR),
+ cmd->resp[0], cmd->resp[1], cmd->resp[2], cmd->resp[3]);

	if (status & AT91_MCI_ERRORS) {
		if ((status & AT91_MCI_RCRCE) && !(mmc_resp_type(cmd) & MMC_RSP_CRC)) {
@@ -877,7 +875,7 @@ static irqreturn_t at91_mci_irq(int irq, void *devid)
	if (completed) {
		pr_debug("Completed command\n");
		at91_mci_write(host, AT91_MCI_IDR, 0xffffffff & ~(AT91_MCI_SDIOIRQA | AT91_MCI_SDIOIRQB));
-		at91_mci_completed_command(host);
+		at91_mci_completed_command(host, int_status);
	} else
		at91_mci_write(host, AT91_MCI_IDR, int_status & ~(AT91_MCI_SDIOIRQA | AT91_MCI_SDIOIRQB));

1.5.3.7

Hi Nicolas!
On Friday 30 May 2008, Nicolas Ferre wrote:> From: Marc Pignat <[email protected]>> > Implement transfer with size not modulo 4 for at91sam9*. Please note that the> at91rm9200 simply can't handle this.> > Signed-off-by: Marc Pignat <[email protected]>> Signed-off-by: Nicolas Ferre <[email protected]>> ---
I just want we don't forget that at91sam9260 errata, pointed by someone when Iposted this patch...
44.2.4.4 Data Write Operation and number of bytes                     The Data Write operation with a number of bytes less than 12 is impossible.                     Problem Fix/Workaround                     The PDC counters must always be equal to 12 bytes for data transfers lower than 12 bytes. The                     BLKLEN or BCNT field are used to specify the real count number. We should probably fix that thing in the next release and verify if otherchips are affected!

Best regards Marcÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±þG«�éÿŠ{ayºʇڙë,j­¢f£¢·hš�ï�êÿ‘êçz_è®(­éšŽŠÝ¢j"�ú¶m§ÿÿ¾«þG«�éÿ¢¸?™¨è­Ú&£ø§~�á¶iO•æ¬z·švØ^¶m§ÿÿÃÿ¶ìÿ¢¸?–I¥

This doesn't look safe. MULTIWRITE is not about whether you can do multi-writes, but whether you can properly report how many bytes were successfully written to the card _and_ the card acknowledged as having been successfully written.

It looks to me as if the AT91 MMC driver reports no bytes, or all bytes if the FIFO reports that it became empty, which presumably it may do if you're unable to keep the FIFO full?

(Dropped Hans-JA¼rgen from the CC list since my mailer seems to corrupt the address.)

If Pierre wants to remove the MULTIWRITE flag, I'd like to hear his solution for the pxamci driver, where the only way to ascertain how many bytes were transmitted may be to walk the SG list comparing the DMA pointer with what was in the hardware DMA engine at the time. Maybe.

You set bytes_xfered to 0. As mentioned in my previous mail, I had a chat with Jens about this and upper layers can only expect to get the lower bound in how many bytes were written. Other hardware/drivers already behaves like this so there is no point in crippling the MMC layer in an effort to give nicer guarantees.

This is why I asked people to audit their drivers to make sure it's the lower bound that's returned, but I've not received much in the way of replies.

Rgds
Pierre

You won't do from me concerning pxamci - I don't look after it anymore after one of my MMC cards got eaten by the misbehaving bugger. I don't think anyone else is looking after it, so that means the driver's effectively unmaintained.

pxamxi is marked as orphaned in MAINTAINERS, so I didn't expect an answer about that one from anyone. :)

You're still listed for the primecell driver though. What's the status on that one?

Rgds
Pierre

It's always reported the number of bytes transferred to/from the card (which I think includes the wait for the busy signal to deassert.)

Probably the best thing is to mark MMCI as orphaned as well - for much the same reason. I've only one remaining MMC card which I use for other purposes. I'm not willing to sacrifice that one for further testing, neither am I willing to buy another card to just be used for testing purposes.

If only all hardware was like that...

Fair enough. Could you send a patch, preferably with some relevant list(s) cc:d in case someone else wants to take over the work?

Rgds
Pierre

I have no big concerns about the patches. There were some comments from other people though. Do you want to have a look at those first?

Rgds
Pierre

Pierre Ossman :

Ok, good !

Russell King highlights that patch #4 is not well inserted in the series : it have to be after #5 (after correcting bytes_xfered update).

If you want I can reorder them... or you manage to do it.

I will take into account Marc's comment and new Ville patch series in another round ;-)

Btw, you can also add the "manage cmd error and data error independently" patch I have just sent.

Kind regards,

Nicolas Ferre

Will you redo this series, or are you planning further patches? (i.e. should I merge this set or wait for an update?)

Pierre Ossman :

You can merge this patch series (7 patches)

You can also merge
http://lists.arm.linux.org.uk/lurker/message/20080610.092729.beef9e64.en.html
"at91_mci: manage cmd error and data error independently" (1 patch)

And 3 patches from Ville Syrjälä that I have just signed:
http://lists.arm.linux.org.uk/lurker/message/20080614.172720.dd0e776e.en.html
"at91_mci: AT91SAM9260/9263 12 byte write erratum (v2)"

http://lists.arm.linux.org.uk/lurker/message/20080609.190644.3f64ff91.en.html
"at91_mci: Cover more AT91RM9200 and AT91SAM9261 errata."

http://lists.arm.linux.org.uk/lurker/message/20080609.190645.2998e511.en.html
"at91_mci: Fix byte mode transitions."

=> 11 patches.

Thanks a lot, regards,

Nicolas Ferre

On Wed, Jun 18, 2008 at 12:04:49PM +0200, Nicolas Ferre wrote:

[...]

Could you all drop =?ISO-8859-1?Q?Hans-J=FCrgen_?= from the Cc list in this thread? The address is invalid because it has no domain part and hence causes quite some bounces on the list because some strict email checkers reject invalid addresses in the Cc list.

I'm trying to figure out what went wrong, but in the mean time please drop =?ISO-8859-1?Q?Hans-J=FCrgen_?= from the Cc list.

Erik
[linux-arm-kernel-owner #2]

Erik Mouw -- [email protected]

Found it, Mailman is to blame. On the linux-kernel mailing list, the address shows up ok: "=?ISO-8859-1?Q?Hans-J=FCrgen_?= =?ISO-8859-1?Q?Koch?= <[email protected]>".

What probably happens: Mailman tries to check if the addresses in the To and CC fields are subscribed to the list (linux-arm-kernel). If that is the case and the subscriber enabled "prevent doubles", it won't send that particular subscriber the message. Of course, this should have been implemented by a simple strncmp() on the To and CC fields, but I guess Mailman first breaks up the To and CC fields, then does the "prevent double" check, and then reassembles the To and CC fields. Apparently there is an error in the break up or reassembly. Now how to fix this without breaking Mailman alltogether...

In the mean time, could you just use Hans-Jürgen's address ([email protected]>) and not his name in the CC field?

Erik
[linux-arm-kernel-owner #2]
(thanks to David Woodhouse for helping to debug the problem)

Erik Mouw -- [email protected]

I avoided the German umlaut in my name by using the abbreviation "Hans J. Koch" on mailing lists. Unfortunately, I still have one mail client that uses the full name. I'll fix that to avoid similar trouble.

On the other hand, I used that for quite a while now, and didn't have problems. It shouldn't be too difficult to have these programs use UTF-8 (or any other charset) properly these days.

Thanks,
Hans

I'm not convinced it's a problem with rfc2047 handling; I think it's simpler brain-damage than that on mailman's part. It's to do with line wrapping, iirc -- if an address spans two lines, like: David Woodhouse
<[email protected]>, then mailman will brokenly insert a comma where
the line break happened -- or something like that.

dwmw2

It's a bit more complicated cause Mailman gets this right:

ARM Linux Mailing List
<[email protected]>,

It also gets RFC2407 right:

=?ISO-8859-1?Q?Ville_Syrj=E4l=E4?= <[email protected]>

I guess it's a combination that goes wrong:

=?ISO-8859-1?Q?Hans-J=FCrgen_?= =?ISO-8859-1?Q?Koch?= <[email protected]>

Maybe if Jürgen's mailer made that something like:

=?ISO-8859-1Q!Hans-J=FCrgen_Koch?=

Mailman wouldn't have crippled the address?

Oh well, rmk isn't available for a few days so I can't really debug the Mailman version on lists.arm.linux.org.uk :(

Erik

Erik Mouw -- [email protected]

I can set up a test list for you to screw around with if you like.

dwmw2

No need for that, I can install Mailman on my laptop. Thanks for the offer, though.

Erik

Erik Mouw -- [email protected]

Everything merged up. It'll be hitting kernel.org soon.

Rgds

     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Use end-to-end encryption where
  possible.

This will be in my -next branch, based on mainline linux. For anything else, you'll have to ask the maintainer of the relevant tree.

     -- Pierre Ossman

  WARNING: This correspondence is being monitored by the
  Swedish government. Use end-to-end encryption where
  possible.