From: Wolfgang Muees <[email protected]>
Fixes two pointer errors, one which leads to memory overwrites if used with
large chunks of data.
Signed-off-by: Wolfgang Muees <[email protected]>
Signed-off-by: Nicolas Ferre <[email protected]>
---
drivers/mmc/host/at91_mci.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index 63924e0..6835104 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -227,11 +227,13 @@ static inline void at91_mci_sg_to_dma(struct at91mci_host *host, struct mmc_data
for (index = 0; index < (amount / 4); index++)
*dmabuf++ = swab32(sgbuffer[index]);
} else {
- memcpy(dmabuf, sgbuffer, amount);
- dmabuf += amount;
+ char *tmpv = (char *)dmabuf;
+ memcpy(tmpv, sgbuffer, amount);
+ tmpv += amount;
+ dmabuf = (unsigned *)tmpv;
}
- kunmap_atomic(sgbuffer, KM_BIO_SRC_IRQ);
+ kunmap_atomic(((void *)sgbuffer)-sg->offset, KM_BIO_SRC_IRQ);
if (size == 0)
break;
--
1.5.6.5
From: Wolfgang Muees <[email protected]>
This patch converts the read to use the DMA buffer as well. The old
code was doing double-buffering DMA with the PDC; no way to make it
work. Replace it with a single-PDC approach.
It also simplify things removing the need for a pre_dma_read() function.
Signed-off-by: Wolfgang Muees <[email protected]>
[[email protected] coding style modifications]
Signed-off-by: Nicolas Ferre <[email protected]>
---
drivers/mmc/host/at91_mci.c | 128 +++++++++++--------------------------------
1 files changed, 32 insertions(+), 96 deletions(-)
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index fd01195..28fd565 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -251,80 +251,14 @@ static inline void at91_mci_sg_to_dma(struct at91mci_host *host, struct mmc_data
}
/*
- * Prepare a dma read
- */
-static void at91_mci_pre_dma_read(struct at91mci_host *host)
-{
- int i;
- struct scatterlist *sg;
- struct mmc_command *cmd;
- struct mmc_data *data;
-
- pr_debug("pre dma read\n");
-
- cmd = host->cmd;
- if (!cmd) {
- pr_debug("no command\n");
- return;
- }
-
- data = cmd->data;
- if (!data) {
- pr_debug("no data\n");
- return;
- }
-
- for (i = 0; i < 2; i++) {
- /* nothing left to transfer */
- if (host->transfer_index >= data->sg_len) {
- pr_debug("Nothing left to transfer (index = %d)\n", host->transfer_index);
- break;
- }
-
- /* Check to see if this needs filling */
- if (i == 0) {
- if (at91_mci_read(host, ATMEL_PDC_RCR) != 0) {
- pr_debug("Transfer active in current\n");
- continue;
- }
- }
- else {
- if (at91_mci_read(host, ATMEL_PDC_RNCR) != 0) {
- pr_debug("Transfer active in next\n");
- continue;
- }
- }
-
- /* Setup the next transfer */
- pr_debug("Using transfer index %d\n", host->transfer_index);
-
- sg = &data->sg[host->transfer_index++];
- pr_debug("sg = %p\n", sg);
-
- sg->dma_address = dma_map_page(NULL, sg_page(sg), sg->offset, sg->length, DMA_FROM_DEVICE);
-
- pr_debug("dma address = %08X, length = %d\n", sg->dma_address, sg->length);
-
- if (i == 0) {
- at91_mci_write(host, ATMEL_PDC_RPR, sg->dma_address);
- 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, (data->blksz & 0x3) ? sg->length : sg->length / 4);
- }
- }
-
- pr_debug("pre dma read done\n");
-}
-
-/*
* Handle after a dma read
*/
static void at91_mci_post_dma_read(struct at91mci_host *host)
{
struct mmc_command *cmd;
struct mmc_data *data;
+ unsigned int len, i, size;
+ unsigned *dmabuf = host->buffer;
pr_debug("post dma read\n");
@@ -340,42 +274,39 @@ static void at91_mci_post_dma_read(struct at91mci_host *host)
return;
}
- while (host->in_use_index < host->transfer_index) {
- struct scatterlist *sg;
+ size = data->blksz * data->blocks;
+ len = data->sg_len;
- pr_debug("finishing index %d\n", host->in_use_index);
+ at91_mci_write(host, AT91_MCI_IDR, AT91_MCI_ENDRX);
+ at91_mci_write(host, AT91_MCI_IER, AT91_MCI_RXBUFF);
- sg = &data->sg[host->in_use_index++];
+ for (i = 0; i < len; i++) {
+ struct scatterlist *sg;
+ int amount;
+ unsigned int *sgbuffer;
- pr_debug("Unmapping page %08X\n", sg->dma_address);
+ sg = &data->sg[i];
- dma_unmap_page(NULL, sg->dma_address, sg->length, DMA_FROM_DEVICE);
+ sgbuffer = kmap_atomic(sg_page(sg), KM_BIO_SRC_IRQ) + sg->offset;
+ amount = min(size, sg->length);
+ size -= amount;
if (cpu_is_at91rm9200()) { /* AT91RM9200 errata */
- unsigned int *buffer;
int index;
-
- /* Swap the contents of the buffer */
- buffer = kmap_atomic(sg_page(sg), KM_BIO_SRC_IRQ) + sg->offset;
- pr_debug("buffer = %p, length = %d\n", buffer, sg->length);
-
- for (index = 0; index < (sg->length / 4); index++)
- buffer[index] = swab32(buffer[index]);
-
- kunmap_atomic(buffer, KM_BIO_SRC_IRQ);
+ for (index = 0; index < (amount / 4); index++)
+ sgbuffer[index] = swab32(*dmabuf++);
+ } else {
+ char *tmpv = (char *)dmabuf;
+ memcpy(sgbuffer, tmpv, amount);
+ tmpv += amount;
+ dmabuf = (unsigned *)tmpv;
}
- flush_dcache_page(sg_page(sg));
-
- data->bytes_xfered += sg->length;
- }
-
- /* Is there another transfer to trigger? */
- if (host->transfer_index < data->sg_len)
- at91_mci_pre_dma_read(host);
- else {
- at91_mci_write(host, AT91_MCI_IDR, AT91_MCI_ENDRX);
- at91_mci_write(host, AT91_MCI_IER, AT91_MCI_RXBUFF);
+ kunmap_atomic(((void *)sgbuffer)-sg->offset, KM_BIO_SRC_IRQ);
+ dmac_flush_range((void *)sgbuffer, ((void *)sgbuffer) + amount);
+ data->bytes_xfered += amount;
+ if (size == 0)
+ break;
}
pr_debug("post dma read done\n");
@@ -610,7 +541,12 @@ static void at91_mci_send_command(struct at91mci_host *host, struct mmc_command
*/
host->total_length = 0;
- at91_mci_pre_dma_read(host);
+ at91_mci_write(host, ATMEL_PDC_RPR, host->physical_address);
+ at91_mci_write(host, ATMEL_PDC_RCR, (data->blksz & 0x3) ?
+ (blocks * block_length) : (blocks * block_length) / 4);
+ at91_mci_write(host, ATMEL_PDC_RNPR, 0);
+ at91_mci_write(host, ATMEL_PDC_RNCR, 0);
+
ier = AT91_MCI_ENDRX /* | AT91_MCI_RXBUFF */;
}
else {
--
1.5.6.5
From: Wolfgang Muees <[email protected]>
Fix two timeout errors, one for slow SDHC cards and one for slow users while
inserting SD cards.
Signed-off-by: Wolfgang Muees <[email protected]>
Signed-off-by: Nicolas Ferre <[email protected]>
---
drivers/mmc/host/at91_mci.c | 6 ++++--
1 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index 6835104..f2cb716 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -756,7 +756,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);
+ /* more than 1s timeout needed with slow SD cards */
+ mod_timer(&host->timer, jiffies + msecs_to_jiffies(2000));
at91_mci_process_next(host);
}
@@ -944,7 +945,8 @@ static irqreturn_t at91_mmc_det_irq(int irq, void *_host)
pr_debug("****** Resetting SD-card bus width ******\n");
at91_mci_write(host, AT91_MCI_SDCR, at91_mci_read(host, AT91_MCI_SDCR) & ~AT91_MCI_SDCBUS);
}
- mmc_detect_change(host->mmc, msecs_to_jiffies(100));
+ /* 0.5s needed because of early card detect switch firing */
+ mmc_detect_change(host->mmc, msecs_to_jiffies(500));
}
return IRQ_HANDLED;
}
--
1.5.6.5
According to the datasheets AT91SAM9261 does not support
SDIO interrupts, and AT91SAM9260/9263 have an erratum
requiring 4bit mode while using slot B for the interrupt
to work.
Signed-off-by: Nicolas Ferre <[email protected]>
---
drivers/mmc/host/at91_mci.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index 4c30c56..e78cdc8 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -929,7 +929,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_SDIO_IRQ;
+ mmc->caps = 0;
mmc->max_blk_size = MCI_MAXBLKSIZE;
mmc->max_blk_count = MCI_BLKATONCE;
@@ -958,6 +958,13 @@ static int __init at91_mci_probe(struct platform_device *pdev)
goto fail5;
}
+ /* Add SDIO capability when available */
+ if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) {
+ /* AT91SAM9260/9263 erratum */
+ if (host->board->wire4 || !host->board->slot_b)
+ mmc->caps |= MMC_CAP_SDIO_IRQ;
+ }
+
/*
* Reserve GPIOs ... board init code makes sure these pins are set
* up as GPIOs with the right direction (input, except for vcc)
--
1.5.6.5
From: Wolfgang Muees <[email protected]>
This patch is setting some max_ variables for the IO elevator, so the elevator
will put requests for large data blocks to the driver. This is critical for
a) speed
and
b) wear leveling of the flash chip controller: Otherwise the controller will
treat the SD card badly with millions of single 4 KByte write commands. This
will lead to a shorter life time for the SD cards.
Signed-off-by: Wolfgang Muees <[email protected]>
Signed-off-by: Nicolas Ferre <[email protected]>
---
drivers/mmc/host/at91_mci.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index 28fd565..4c30c56 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -934,6 +934,9 @@ static int __init at91_mci_probe(struct platform_device *pdev)
mmc->max_blk_size = MCI_MAXBLKSIZE;
mmc->max_blk_count = MCI_BLKATONCE;
mmc->max_req_size = MCI_BUFSIZE;
+ mmc->max_phys_segs = MCI_BLKATONCE;
+ mmc->max_hw_segs = MCI_BLKATONCE;
+ mmc->max_seg_size = MCI_BUFSIZE;
host = mmc_priv(mmc);
host->mmc = mmc;
--
1.5.6.5
We used to manage features and differences on a per-cpu basis. As several cpus
share the same mci revision, this patch aggregates cpus that have the same IP
revision in one defined constant.
We use the at91mci_is_mci1rev2xx() funtion name not to mess with newer Atmel
sd/mmc IP called "MCI2". _rev2 naming could have been confusing...
Signed-off-by: Nicolas Ferre <[email protected]>
---
drivers/mmc/host/at91_mci.c | 27 +++++++++++++++++++--------
1 files changed, 19 insertions(+), 8 deletions(-)
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index e78cdc8..07fed42 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -78,6 +78,17 @@
#define DRIVER_NAME "at91_mci"
+static inline int at91mci_is_mci1rev2xx(void)
+{
+ return ( cpu_is_at91sam9260()
+ || cpu_is_at91sam9263()
+ || cpu_is_at91cap9()
+ || cpu_is_at91sam9rl()
+ || cpu_is_at91sam9g10()
+ || cpu_is_at91sam9g20()
+ );
+}
+
#define FL_SENT_COMMAND (1 << 0)
#define FL_SENT_STOP (1 << 1)
@@ -204,8 +215,8 @@ static inline void at91_mci_sg_to_dma(struct at91mci_host *host, struct mmc_data
size = data->blksz * data->blocks;
len = data->sg_len;
- /* AT91SAM926[0/3] Data Write Operation and number of bytes erratum */
- if (cpu_is_at91sam9260() || cpu_is_at91sam9263())
+ /* MCI1 rev2xx Data Write Operation and number of bytes erratum */
+ if (at91mci_is_mci1rev2xx())
if (host->total_length == 12)
memset(dmabuf, 0, 12);
@@ -398,7 +409,7 @@ static void at91_mci_enable(struct at91mci_host *host)
at91_mci_write(host, AT91_MCI_DTOR, AT91_MCI_DTOMUL_1M | AT91_MCI_DTOCYC);
mr = AT91_MCI_PDCMODE | 0x34a;
- if (cpu_is_at91sam9260() || cpu_is_at91sam9263())
+ if (at91mci_is_mci1rev2xx())
mr |= AT91_MCI_RDPROOF | AT91_MCI_WRPROOF;
at91_mci_write(host, AT91_MCI_MR, mr);
@@ -555,10 +566,10 @@ static void at91_mci_send_command(struct at91mci_host *host, struct mmc_command
*/
host->total_length = block_length * blocks;
/*
- * AT91SAM926[0/3] Data Write Operation and
+ * MCI1 rev2xx Data Write Operation and
* number of bytes erratum
*/
- if (cpu_is_at91sam9260 () || cpu_is_at91sam9263())
+ if (at91mci_is_mci1rev2xx())
if (host->total_length < 12)
host->total_length = 12;
@@ -943,7 +954,7 @@ static int __init at91_mci_probe(struct platform_device *pdev)
host->bus_mode = 0;
host->board = pdev->dev.platform_data;
if (host->board->wire4) {
- if (cpu_is_at91sam9260() || cpu_is_at91sam9263())
+ if (at91mci_is_mci1rev2xx())
mmc->caps |= MMC_CAP_4_BIT_DATA;
else
dev_warn(&pdev->dev, "4 wire bus mode not supported"
@@ -959,8 +970,8 @@ static int __init at91_mci_probe(struct platform_device *pdev)
}
/* Add SDIO capability when available */
- if (cpu_is_at91sam9260() || cpu_is_at91sam9263()) {
- /* AT91SAM9260/9263 erratum */
+ if (at91mci_is_mci1rev2xx()) {
+ /* at91mci MCI1 rev2xx sdio interrupt erratum */
if (host->board->wire4 || !host->board->slot_b)
mmc->caps |= MMC_CAP_SDIO_IRQ;
}
--
1.5.6.5
From: Wolfgang Muees <[email protected]>
The TX DMA buffer is allocated only once, because the allocation/deallocation
of the buffer for EACH chunk of data is time-consuming and prone to memory
fragmentation.
Using a coherent DMA buffer avoids extra data cache calls.
Signed-off-by: Wolfgang Muees <[email protected]>
[[email protected]: coding style modifications]
Signed-off-by: Nicolas Ferre <[email protected]>
---
drivers/mmc/host/at91_mci.c | 49 +++++++++++++++++++++----------------------
1 files changed, 24 insertions(+), 25 deletions(-)
diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
index f2cb716..fd01195 100644
--- a/drivers/mmc/host/at91_mci.c
+++ b/drivers/mmc/host/at91_mci.c
@@ -88,6 +88,10 @@
#define at91_mci_read(host, reg) __raw_readl((host)->baseaddr + (reg))
#define at91_mci_write(host, reg, val) __raw_writel((val), (host)->baseaddr + (reg))
+#define MCI_BLKSIZE 512
+#define MCI_MAXBLKSIZE 4095
+#define MCI_BLKATONCE 256
+#define MCI_BUFSIZE (MCI_BLKSIZE * MCI_BLKATONCE)
/*
* Low level type for this driver
@@ -604,7 +608,6 @@ static void at91_mci_send_command(struct at91mci_host *host, struct mmc_command
/*
* Handle a read
*/
- host->buffer = NULL;
host->total_length = 0;
at91_mci_pre_dma_read(host);
@@ -623,20 +626,8 @@ static void at91_mci_send_command(struct at91mci_host *host, struct mmc_command
if (host->total_length < 12)
host->total_length = 12;
- host->buffer = kmalloc(host->total_length, GFP_KERNEL);
- if (!host->buffer) {
- pr_debug("Can't alloc tx buffer\n");
- cmd->error = -ENOMEM;
- mmc_request_done(host->mmc, host->request);
- return;
- }
-
at91_mci_sg_to_dma(host, data);
- host->physical_address = dma_map_single(NULL,
- host->buffer, host->total_length,
- DMA_TO_DEVICE);
-
pr_debug("Transmitting %d bytes\n", host->total_length);
at91_mci_write(host, ATMEL_PDC_TPR, host->physical_address);
@@ -703,14 +694,6 @@ static void at91_mci_completed_command(struct at91mci_host *host, unsigned int s
cmd->resp[2] = at91_mci_read(host, AT91_MCI_RSPR(2));
cmd->resp[3] = at91_mci_read(host, AT91_MCI_RSPR(3));
- if (host->buffer) {
- dma_unmap_single(NULL,
- host->physical_address, host->total_length,
- DMA_TO_DEVICE);
- kfree(host->buffer);
- host->buffer = NULL;
- }
-
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]);
@@ -1012,12 +995,12 @@ static int __init at91_mci_probe(struct platform_device *pdev)
mmc->ocr_avail = MMC_VDD_32_33 | MMC_VDD_33_34;
mmc->caps = MMC_CAP_SDIO_IRQ;
- mmc->max_blk_size = 4095;
- mmc->max_blk_count = mmc->max_req_size;
+ mmc->max_blk_size = MCI_MAXBLKSIZE;
+ mmc->max_blk_count = MCI_BLKATONCE;
+ mmc->max_req_size = MCI_BUFSIZE;
host = mmc_priv(mmc);
host->mmc = mmc;
- host->buffer = NULL;
host->bus_mode = 0;
host->board = pdev->dev.platform_data;
if (host->board->wire4) {
@@ -1028,6 +1011,14 @@ static int __init at91_mci_probe(struct platform_device *pdev)
" - using 1 wire\n");
}
+ host->buffer = dma_alloc_coherent(&pdev->dev, MCI_BUFSIZE,
+ &host->physical_address, GFP_KERNEL);
+ if (!host->buffer) {
+ ret = -ENOMEM;
+ dev_err(&pdev->dev, "Can't allocate transmit buffer\n");
+ goto fail5;
+ }
+
/*
* Reserve GPIOs ... board init code makes sure these pins are set
* up as GPIOs with the right direction (input, except for vcc)
@@ -1036,7 +1027,7 @@ static int __init at91_mci_probe(struct platform_device *pdev)
ret = gpio_request(host->board->det_pin, "mmc_detect");
if (ret < 0) {
dev_dbg(&pdev->dev, "couldn't claim card detect pin\n");
- goto fail5;
+ goto fail4b;
}
}
if (host->board->wp_pin) {
@@ -1136,6 +1127,10 @@ fail3:
fail4:
if (host->board->det_pin)
gpio_free(host->board->det_pin);
+fail4b:
+ if (host->buffer)
+ dma_free_coherent(&pdev->dev, MCI_BUFSIZE,
+ host->buffer, host->physical_address);
fail5:
mmc_free_host(mmc);
fail6:
@@ -1158,6 +1153,10 @@ static int __exit at91_mci_remove(struct platform_device *pdev)
host = mmc_priv(mmc);
+ if (host->buffer)
+ dma_free_coherent(&pdev->dev, MCI_BUFSIZE,
+ host->buffer, host->physical_address);
+
if (host->board->det_pin) {
if (device_can_wakeup(&pdev->dev))
free_irq(gpio_to_irq(host->board->det_pin), host);
--
1.5.6.5
On Fri, Feb 26, 2010 at 07:39:29PM +0100, Nicolas Ferre wrote:
> From: Wolfgang Muees <[email protected]>
>
> Fixes two pointer errors, one which leads to memory overwrites if used with
> large chunks of data.
>
> Signed-off-by: Wolfgang Muees <[email protected]>
> Signed-off-by: Nicolas Ferre <[email protected]>
> ---
> drivers/mmc/host/at91_mci.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
> index 63924e0..6835104 100644
> --- a/drivers/mmc/host/at91_mci.c
> +++ b/drivers/mmc/host/at91_mci.c
> @@ -227,11 +227,13 @@ static inline void at91_mci_sg_to_dma(struct at91mci_host *host, struct mmc_data
> for (index = 0; index < (amount / 4); index++)
> *dmabuf++ = swab32(sgbuffer[index]);
> } else {
> - memcpy(dmabuf, sgbuffer, amount);
> - dmabuf += amount;
> + char *tmpv = (char *)dmabuf;
> + memcpy(tmpv, sgbuffer, amount);
> + tmpv += amount;
> + dmabuf = (unsigned *)tmpv;
> }
>
> - kunmap_atomic(sgbuffer, KM_BIO_SRC_IRQ);
> + kunmap_atomic(((void *)sgbuffer)-sg->offset, KM_BIO_SRC_IRQ);
Please put spaces around the minus sign.
kunmap_atomic(((void *)sgbuffer) - sg->offset, KM_BIO_SRC_IRQ);
regards,
dan carpenter
>
> if (size == 0)
> break;
> --
> 1.5.6.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
Hi Andrew, Nicolas,
On Fri, Feb 26, 2010 at 07:39:29PM +0100, Nicolas Ferre wrote:
> From: Wolfgang Muees <[email protected]>
>
> Fixes two pointer errors, one which leads to memory overwrites if used with
> large chunks of data.
>
> Signed-off-by: Wolfgang Muees <[email protected]>
> Signed-off-by: Nicolas Ferre <[email protected]>
> ---
> drivers/mmc/host/at91_mci.c | 8 +++++---
> 1 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
> index 63924e0..6835104 100644
> --- a/drivers/mmc/host/at91_mci.c
> +++ b/drivers/mmc/host/at91_mci.c
> @@ -227,11 +227,13 @@ static inline void at91_mci_sg_to_dma(struct at91mci_host *host, struct mmc_data
> for (index = 0; index < (amount / 4); index++)
> *dmabuf++ = swab32(sgbuffer[index]);
> } else {
> - memcpy(dmabuf, sgbuffer, amount);
> - dmabuf += amount;
> + char *tmpv = (char *)dmabuf;
> + memcpy(tmpv, sgbuffer, amount);
> + tmpv += amount;
> + dmabuf = (unsigned *)tmpv;
> }
>
> - kunmap_atomic(sgbuffer, KM_BIO_SRC_IRQ);
> + kunmap_atomic(((void *)sgbuffer)-sg->offset, KM_BIO_SRC_IRQ);
>
> if (size == 0)
> break;
> --
> 1.5.6.5
Looks like only the first half of this patch was applied? The
kunmap_atomic() line is still present as before in Linus' tree.
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child
On Fri, Aug 27, 2010 at 08:33:03PM +0100, Chris Ball wrote:
> Looks like only the first half of this patch was applied? The
> kunmap_atomic() line is still present as before in Linus' tree.
Might be related to Dan Carpenter's suggestion that the spacing for that
line should change to:
kunmap_atomic(((void *)sgbuffer) - sg->offset, KM_BIO_SRC_IRQ);
--
Chris Ball <[email protected]> <http://printf.net/>
One Laptop Per Child
On 08/28/2010 07:33 AM, Chris Ball wrote:
> Hi Andrew, Nicolas,
>
> On Fri, Feb 26, 2010 at 07:39:29PM +0100, Nicolas Ferre wrote:
>> From: Wolfgang Muees <[email protected]>
>>
>> Fixes two pointer errors, one which leads to memory overwrites if used with
>> large chunks of data.
>>
>> Signed-off-by: Wolfgang Muees <[email protected]>
>> Signed-off-by: Nicolas Ferre <[email protected]>
>> ---
>> drivers/mmc/host/at91_mci.c | 8 +++++---
>> 1 files changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/mmc/host/at91_mci.c b/drivers/mmc/host/at91_mci.c
>> index 63924e0..6835104 100644
>> --- a/drivers/mmc/host/at91_mci.c
>> +++ b/drivers/mmc/host/at91_mci.c
>> @@ -227,11 +227,13 @@ static inline void at91_mci_sg_to_dma(struct at91mci_host *host, struct mmc_data
>> for (index = 0; index < (amount / 4); index++)
>> *dmabuf++ = swab32(sgbuffer[index]);
>> } else {
>> - memcpy(dmabuf, sgbuffer, amount);
>> - dmabuf += amount;
>> + char *tmpv = (char *)dmabuf;
>> + memcpy(tmpv, sgbuffer, amount);
>> + tmpv += amount;
>> + dmabuf = (unsigned *)tmpv;
I see this is already applied, but why all the type trickery here? Why
not just:
memcpy(dmabuf, sgbuffer, amount);
dmabuf += amount / sizeof(dmabuf);
>> }
>>
>> - kunmap_atomic(sgbuffer, KM_BIO_SRC_IRQ);
>> + kunmap_atomic(((void *)sgbuffer)-sg->offset, KM_BIO_SRC_IRQ);
void * arithmetic is a gcc-ism. Isn't it better to cast to char *, since
the meaning will be a bit more clear.
~Ryan
--
Bluewater Systems Ltd - ARM Technology Solution Centre
Ryan Mallon 5 Amuri Park, 404 Barbadoes St
[email protected] PO Box 13 889, Christchurch 8013
http://www.bluewatersys.com New Zealand
Phone: +64 3 3779127 Freecall: Australia 1800 148 751
Fax: +64 3 3779135 USA 1800 261 2934
Wolfgang M?es wrote:
> Hi,
>
> Am 29.08.2010 23:39, schrieb Ryan Mallon:
>> I see this is already applied, but why all the type trickery here? Why
>> not just:
>>
>> memcpy(dmabuf, sgbuffer, amount);
>> dmabuf += amount / sizeof(dmabuf);
>
> Because this is wrong?
>
> Maybe you mean sizeof(*dmabuf)?
Sorry, yes.
> The exact style of the bug fix is not important, it's important that
> it's fixed.
I disagree, it's also important that it is fixed in a clear,
understandable way. I found the fix confusing, especially since there
are casts to both signed and unsigned char.
~Ryan
Hi,
Am 27.08.2010 21:34, schrieb Chris Ball:
> On Fri, Aug 27, 2010 at 08:33:03PM +0100, Chris Ball wrote:
>> Looks like only the first half of this patch was applied? The
>> kunmap_atomic() line is still present as before in Linus' tree.
It was argued that the kunmap_atomic() will truncate the pointer to the
start of the page anyway, so the subtraction is not needed.
best regards
i. A. Wolfgang Muees
Hardware-Entwicklung
Leiter Pruefen & Layout
--
Auerswald Gesellschaft fuer Datensysteme mbH
Phone: +49 5306-9219-562
Fax: +49 5306-9200-94
--------------------------------------------------------------
Auerswald Gesellschaft fuer Datensysteme mbH,
Vor den Grashoefen 1, 38162 Cremlingen
Registriert beim AG Braunschweig HRB 7499
Gesch?ftsf?hrer: Dipl-Ing. Gerhard Auerswald
Hi,
Am 29.08.2010 23:39, schrieb Ryan Mallon:
> I see this is already applied, but why all the type trickery here? Why
> not just:
>
> memcpy(dmabuf, sgbuffer, amount);
> dmabuf += amount / sizeof(dmabuf);
Because this is wrong?
Maybe you mean sizeof(*dmabuf)?
The exact style of the bug fix is not important, it's important that
it's fixed.
best regards
i. A. Wolfgang Muees
Hardware-Entwicklung
Leiter Pruefen & Layout
--
Auerswald Gesellschaft fuer Datensysteme mbH
Phone: +49 5306-9219-562
Fax: +49 5306-9200-94
--------------------------------------------------------------
Auerswald Gesellschaft fuer Datensysteme mbH,
Vor den Grashoefen 1, 38162 Cremlingen
Registriert beim AG Braunschweig HRB 7499
Gesch?ftsf?hrer: Dipl-Ing. Gerhard Auerswald