[PATCH] fix misalignment in pxamci

[PATCH] fix misalignment in pxamci

[Marek Vasut]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

Hi,
Philipp Zabel finally made the pxamci issue clear. It turned out, that pxamci 
needs the DMA destination address to be aligned to 8 bytes. In some cases it 
happened, that the address was aligned to 4 bytes causing controller to 
incorrectly transfer data (and resulting into error like "mmc0: unrecognised 
SCR structure version 1"). The following patch allows to debug this issue and 
moreover fixes it by moving one 4 byte entry of mmc_card structure, aligning 
the DMA destination back to 8 bytes.

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

[-- Attachment #2: pxamci-lkml.patch --]
[-- Type: text/x-diff, Size: 1766 bytes --]

Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 65210fc..75b2810 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -153,6 +153,13 @@ static void pxamci_setup_data(struct pxamci_host *host, struct mmc_data *data)
 		if (length & 31 && !(data->flags & MMC_DATA_READ))
 			host->sg_cpu[i].dcmd |= DCMD_ENDIRQEN;
 		if (data->flags & MMC_DATA_READ) {
+#ifdef CONFIG_MMC_DEBUG
+			/* we need destination address to be aligned to 8 bytes
+			   here, if it isn't, we have serious problem */
+			if (sg_dma_address(&data->sg[i]) % 8)
+				pr_debug("%s:%i Misaligned DMA destination\n",
+					__FILE__, __LINE__);
+#endif
 			host->sg_cpu[i].dsadr = host->res->start + MMC_RXFIFO;
 			host->sg_cpu[i].dtadr = sg_dma_address(&data->sg[i]);
 		} else {
diff --git a/include/linux/mmc/card.h b/include/linux/mmc/card.h
index 0d508ac..cb76462 100644
--- a/include/linux/mmc/card.h
+++ b/include/linux/mmc/card.h
@@ -89,6 +89,7 @@ struct mmc_card {
 #define MMC_TYPE_MMC		0		/* MMC card */
 #define MMC_TYPE_SD		1		/* SD card */
 #define MMC_TYPE_SDIO		2		/* SDIO card */
+	unsigned int		sdio_funcs;	/* number of SDIO functions */
 	unsigned int		state;		/* (our) card state */
 #define MMC_STATE_PRESENT	(1<<0)		/* present in sysfs */
 #define MMC_STATE_READONLY	(1<<1)		/* card is read-only */
@@ -104,7 +105,6 @@ struct mmc_card {
 	struct sd_scr		scr;		/* extra SD information */
 	struct sd_switch_caps	sw_caps;	/* switch (CMD6) caps */
 
-	unsigned int		sdio_funcs;	/* number of SDIO functions */
 	struct sdio_cccr	cccr;		/* common card info */
 	struct sdio_cis		cis;		/* common tuple info */
 	struct sdio_func	*sdio_func[SDIO_MAX_FUNCS]; /* SDIO functions (devices) */

Re: [PATCH] fix misalignment in pxamci

[pHilipp Zabel]

On Sat, Jul 5, 2008 at 2:19 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> Hi,
> Philipp Zabel finally made the pxamci issue clear. It turned out, that pxamci
> needs the DMA destination address to be aligned to 8 bytes. In some cases it
> happened, that the address was aligned to 4 bytes causing controller to
> incorrectly transfer data (and resulting into error like "mmc0: unrecognised
> SCR structure version 1"). The following patch allows to debug this issue and
> moreover fixes it by moving one 4 byte entry of mmc_card structure, aligning
> the DMA destination back to 8 bytes.
>
> Signed-off-by: Marek Vasut <marek.vasut@gmail.com>

We can enable byte aligned transfers on the DMA controller. This is
what I came up with yesterday:
(sorry for wrapped lines - the proper patch should probably be a
combination of both
warning/DALGN handling and and moving something in mmc_card around).

regards
Philipp

---
Subject: [PATCH] pxamci: fix byte aligned DMA transfers

The pxa27x DMA controller defaults to 64-bit alignment. This caused
the SCR reads to fail (and, depending on card type, error out) when
card->raw_scr was not aligned on a 8-byte boundary.

I think for performance reasons all scatter-gather addresses passed
to pxamci_request should be aligned on 8-byte boundaries, but right
now enabling byte aligned DMA transfers in the controller fixes those
problems.

Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/mmc/host/pxamci.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/mmc/host/pxamci.c b/drivers/mmc/host/pxamci.c
index 65210fc..ba580d3 100644
--- a/drivers/mmc/host/pxamci.c
+++ b/drivers/mmc/host/pxamci.c
@@ -114,6 +114,7 @@ static void pxamci_setup_data(struct pxamci_host
*host, struct mmc_data *data)
 	unsigned int nob = data->blocks;
 	unsigned long long clks;
 	unsigned int timeout;
+	bool dalgn = 0;
 	u32 dcmd;
 	int i;

@@ -152,6 +153,8 @@ static void pxamci_setup_data(struct pxamci_host
*host, struct mmc_data *data)
 		host->sg_cpu[i].dcmd = dcmd | length;
 		if (length & 31 && !(data->flags & MMC_DATA_READ))
 			host->sg_cpu[i].dcmd |= DCMD_ENDIRQEN;
+		if (sg_dma_address(&data->sg[i]) & 0x7)
+			dalgn = 1;
 		if (data->flags & MMC_DATA_READ) {
 			host->sg_cpu[i].dsadr = host->res->start + MMC_RXFIFO;
 			host->sg_cpu[i].dtadr = sg_dma_address(&data->sg[i]);
@@ -165,6 +168,11 @@ static void pxamci_setup_data(struct pxamci_host
*host, struct mmc_data *data)
 	host->sg_cpu[host->dma_len - 1].ddadr = DDADR_STOP;
 	wmb();

+	if (dalgn) {
+		pr_warning("PXAMCI: byte aligned DMA transfer\n");
+		DALGN |= (1 << host->dma);
+	} else
+		DALGN &= (1 << host->dma);
 	DDADR(host->dma) = host->sg_dma;
 	DCSR(host->dma) = DCSR_RUN;
 }
-- 
1.5.6

Re: [PATCH] fix misalignment in pxamci

[Uli Luckas]

On Saturday, 5. July 2008, pHilipp Zabel wrote:
> On Sat, Jul 5, 2008 at 2:19 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
> > Hi,
> > Philipp Zabel finally made the pxamci issue clear. It turned out, that
> > pxamci needs the DMA destination address to be aligned to 8 bytes. In
> > some cases it happened, that the address was aligned to 4 bytes causing
> > controller to incorrectly transfer data (and resulting into error like
> > "mmc0: unrecognised SCR structure version 1"). The following patch allows
> > to debug this issue and moreover fixes it by moving one 4 byte entry of
> > mmc_card structure, aligning the DMA destination back to 8 bytes.
> >
> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>
> We can enable byte aligned transfers on the DMA controller. This is
> what I came up with yesterday:
> (sorry for wrapped lines - the proper patch should probably be a
> combination of both
> warning/DALGN handling and and moving something in mmc_card around).
>
Hi Philipp,
this driver is not only for pxa27x but for pxa25x as well and pxa25x can't 
handle unaligned DMA.
Shouldn't Marek Vasut's patch be included for the PXA25x case?

regards,
Uli


-- 

------- ROAD ...the handyPC Company - - -  ) ) )

Uli Luckas
Software Development

ROAD GmbH
Bennigsenstr. 14 | 12159 Berlin | Germany
fon: +49 (30) 230069 - 64 | fax: +49 (30) 230069 - 69
url: www.road.de

Amtsgericht Charlottenburg: HRB 96688 B
Managing directors: Hans-Peter Constien, Hubertus von Streit

Re: [PATCH] fix misalignment in pxamci

[pHilipp Zabel]

On Mon, Jul 28, 2008 at 6:23 PM, Uli Luckas <u.luckas@road.de> wrote:
> On Saturday, 5. July 2008, pHilipp Zabel wrote:
>> On Sat, Jul 5, 2008 at 2:19 AM, Marek Vasut <marek.vasut@gmail.com> wrote:
>> > Hi,
>> > Philipp Zabel finally made the pxamci issue clear. It turned out, that
>> > pxamci needs the DMA destination address to be aligned to 8 bytes. In
>> > some cases it happened, that the address was aligned to 4 bytes causing
>> > controller to incorrectly transfer data (and resulting into error like
>> > "mmc0: unrecognised SCR structure version 1"). The following patch allows
>> > to debug this issue and moreover fixes it by moving one 4 byte entry of
>> > mmc_card structure, aligning the DMA destination back to 8 bytes.
>> >
>> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
>>
>> We can enable byte aligned transfers on the DMA controller. This is
>> what I came up with yesterday:
>> (sorry for wrapped lines - the proper patch should probably be a
>> combination of both
>> warning/DALGN handling and and moving something in mmc_card around).
>>
> Hi Philipp,
> this driver is not only for pxa27x but for pxa25x as well and pxa25x can't
> handle unaligned DMA.
> Shouldn't Marek Vasut's patch be included for the PXA25x case?

Argh, DALGN shouldn't be defined in pxa-regs.h. We really need an
aligned SCR target then. Pierre, is there any way we can have the MMC
core align DMA targets for pxa25x?

Just moving elements of the mmc_card structure around seems to be good
enough, but I fear this will break again as soon as the next person
forgets about pxamci's special needs on pxa25x.

regards
Philipp

Re: [PATCH] fix misalignment in pxamci

[Marek Vasut]

Dne Tuesday 29 of July 2008 08:30:09 pHilipp Zabel napsal(a):
> On Mon, Jul 28, 2008 at 6:23 PM, Uli Luckas <u.luckas@road.de> wrote:
> > On Saturday, 5. July 2008, pHilipp Zabel wrote:
> >> On Sat, Jul 5, 2008 at 2:19 AM, Marek Vasut <marek.vasut@gmail.com> 
wrote:
> >> > Hi,
> >> > Philipp Zabel finally made the pxamci issue clear. It turned out, that
> >> > pxamci needs the DMA destination address to be aligned to 8 bytes. In
> >> > some cases it happened, that the address was aligned to 4 bytes
> >> > causing controller to incorrectly transfer data (and resulting into
> >> > error like "mmc0: unrecognised SCR structure version 1"). The
> >> > following patch allows to debug this issue and moreover fixes it by
> >> > moving one 4 byte entry of mmc_card structure, aligning the DMA
> >> > destination back to 8 bytes.
> >> >
> >> > Signed-off-by: Marek Vasut <marek.vasut@gmail.com>
> >>
> >> We can enable byte aligned transfers on the DMA controller. This is
> >> what I came up with yesterday:
> >> (sorry for wrapped lines - the proper patch should probably be a
> >> combination of both
> >> warning/DALGN handling and and moving something in mmc_card around).
> >
> > Hi Philipp,
> > this driver is not only for pxa27x but for pxa25x as well and pxa25x
> > can't handle unaligned DMA.
> > Shouldn't Marek Vasut's patch be included for the PXA25x case?
>
> Argh, DALGN shouldn't be defined in pxa-regs.h. We really need an
> aligned SCR target then. Pierre, is there any way we can have the MMC
> core align DMA targets for pxa25x?
>
> Just moving elements of the mmc_card structure around seems to be good
> enough, but I fear this will break again as soon as the next person
> forgets about pxamci's special needs on pxa25x.

Well cant we just add some comment to mmc_card .... like "your eyes will bulge 
with horror if you add something before this point"? ;-)
>
> regards
> Philipp

Re: [PATCH] fix misalignment in pxamci

[Pierre Ossman]

On Tue, 29 Jul 2008 08:30:09 +0200
"pHilipp Zabel" <philipp.zabel@gmail.com> wrote:

> 
> Argh, DALGN shouldn't be defined in pxa-regs.h. We really need an
> aligned SCR target then. Pierre, is there any way we can have the MMC
> core align DMA targets for pxa25x?
> 

No, not really. So if you can't work around the hardware limitations,
add a bounce buffer or fail the requests that cannot be handled (or
both, if you have a bounce buffer that's smaller than the maximum
request size).


-- 
     -- Pierre Ossman

  Linux kernel, MMC maintainer        http://www.kernel.org
  rdesktop, core developer          http://www.rdesktop.org

  WARNING: This correspondence is being monitored by the
  Swedish government. Make sure your server uses encryption
  for SMTP traffic and consider using PGP for end-to-end
  encryption.