lkml.org 
[lkml]   [2009]   [Feb]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [Patch 1/1 v2] via-sdmmc: VIA MSP card reader driver support
Date
On Tuesday 10 February 2009, JosephChan@via.com.tw wrote:
> The following patches provides VIA MSP SD/MMC card reader driver support.
> And these patches are based on kernel 2.6.29-rc4.
> Also, thanks for Arnd's previous suggestions.

Looks mostly good, just two more comments:

> --- a/drivers/mmc/host/via-sdmmc.h 1970-01-01 08:00:00.000000000 +0800
> +++ b/drivers/mmc/host/via-sdmmc.h 2009-02-10 21:34:18.000000000 +0800

The header file is used in only one place, which means that you could
move everything from there into via-sdmmc.c. This would make reading the
code easier later on.

> +
> + if (data->flags & MMC_DATA_WRITE) {
> + for (i = 0; i < len; i++) {
> + tmpbuf = kmap_atomic(sg_page(&sg[i]), KM_BIO_SRC_IRQ);
> + tmpbuf += sg[i].offset;
> + memcpy(host->ddmabuf + offset, tmpbuf, sg[i].length);
> + offset += sg[i].length;
> + kunmap_atomic(tmpbuf, KM_BIO_SRC_IRQ);
> + }
> + }

This loop is broken for sg chaining. You should use for_each_sg() or
sg_next() instead of sg[i] indexing.

> + if (data->flags & MMC_DATA_READ) {
> + struct scatterlist *sg = data->sg;
> + unsigned char *sgbuf;
> + int i;
> +
> + for (i = 0; i < data->sg_len; i++) {
> + sgbuf = kmap_atomic(sg_page(&sg[i]), KM_BIO_SRC_IRQ);
> + memcpy(sgbuf + sg[i].offset, host->ddmabuf + offset,
> + sg[i].length);
> + offset += sg[i].length;
> + kunmap_atomic(sgbuf, KM_BIO_SRC_IRQ);
> + }
> + }
> +

same here.

Arnd <><


\
 
 \ /
  Last update: 2009-02-10 13:21    [W:3.278 / U:0.776 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site