* [PATCH] meye: correct printk of dma_addr_t
@ 2004-02-03 23:36 Randy.Dunlap
2004-02-03 23:57 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Randy.Dunlap @ 2004-02-03 23:36 UTC (permalink / raw)
To: lkml; +Cc: stelian, akpm
description: fix dma_addr_t type error with CONFIG_HIGHMEM64G=y;
product_versions: linux-262-rc3
diffstat:=
drivers/media/video/meye.c | 10 +++++-----
1 files changed, 5 insertions(+), 5 deletions(-)
diff -Naurp ./drivers/media/video/meye.c~meye_dma ./drivers/media/video/meye.c
--- ./drivers/media/video/meye.c~meye_dma 2004-01-08 22:59:44.000000000 -0800
+++ ./drivers/media/video/meye.c 2004-02-03 14:43:42.000000000 -0800
@@ -162,7 +162,7 @@ static void rvfree(void * mem, unsigned
/* return a page table pointing to N pages of locked memory */
static int ptable_alloc(void) {
- u32 *pt;
+ dma_addr_t *pt;
int i;
memset(meye.mchip_ptable, 0, sizeof(meye.mchip_ptable));
@@ -176,7 +176,7 @@ static int ptable_alloc(void) {
return -1;
}
- pt = (u32 *)meye.mchip_ptable[MCHIP_NB_PAGES];
+ pt = (dma_addr_t *)meye.mchip_ptable[MCHIP_NB_PAGES];
for (i = 0; i < MCHIP_NB_PAGES; i++) {
meye.mchip_ptable[i] = dma_alloc_coherent(&meye.mchip_dev->dev,
PAGE_SIZE,
@@ -184,7 +184,7 @@ static int ptable_alloc(void) {
GFP_KERNEL);
if (!meye.mchip_ptable[i]) {
int j;
- pt = (u32 *)meye.mchip_ptable[MCHIP_NB_PAGES];
+ pt = (dma_addr_t *)meye.mchip_ptable[MCHIP_NB_PAGES];
for (j = 0; j < i; ++j) {
dma_free_coherent(&meye.mchip_dev->dev,
PAGE_SIZE,
@@ -200,10 +200,10 @@ static int ptable_alloc(void) {
}
static void ptable_free(void) {
- u32 *pt;
+ dma_addr_t *pt;
int i;
- pt = (u32 *)meye.mchip_ptable[MCHIP_NB_PAGES];
+ pt = (dma_addr_t *)meye.mchip_ptable[MCHIP_NB_PAGES];
for (i = 0; i < MCHIP_NB_PAGES; i++) {
if (meye.mchip_ptable[i])
dma_free_coherent(&meye.mchip_dev->dev,
--
~Randy
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] meye: correct printk of dma_addr_t 2004-02-03 23:36 [PATCH] meye: correct printk of dma_addr_t Randy.Dunlap @ 2004-02-03 23:57 ` Andrew Morton 2004-02-04 0:28 ` Randy.Dunlap 0 siblings, 1 reply; 6+ messages in thread From: Andrew Morton @ 2004-02-03 23:57 UTC (permalink / raw) To: Randy.Dunlap; +Cc: linux-kernel, stelian "Randy.Dunlap" <rddunlap@osdl.org> wrote: > > diff -Naurp ./drivers/media/video/meye.c~meye_dma ./drivers/media/video/meye.c > --- ./drivers/media/video/meye.c~meye_dma 2004-01-08 22:59:44.000000000 -0800 > +++ ./drivers/media/video/meye.c 2004-02-03 14:43:42.000000000 -0800 > @@ -162,7 +162,7 @@ static void rvfree(void * mem, unsigned > > /* return a page table pointing to N pages of locked memory */ > static int ptable_alloc(void) { > - u32 *pt; > + dma_addr_t *pt; > int i; > > memset(meye.mchip_ptable, 0, sizeof(meye.mchip_ptable)); > @@ -176,7 +176,7 @@ static int ptable_alloc(void) { > return -1; > } > > - pt = (u32 *)meye.mchip_ptable[MCHIP_NB_PAGES]; > + pt = (dma_addr_t *)meye.mchip_ptable[MCHIP_NB_PAGES]; > for (i = 0; i < MCHIP_NB_PAGES; i++) { > meye.mchip_ptable[i] = dma_alloc_coherent(&meye.mchip_dev->dev, > PAGE_SIZE, mchip_ptable[] just contains pointers to kernel memory. It doesn't seem appropriate to be casting these to dma_addr_t's ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] meye: correct printk of dma_addr_t 2004-02-03 23:57 ` Andrew Morton @ 2004-02-04 0:28 ` Randy.Dunlap 2004-02-04 8:44 ` Stelian Pop 0 siblings, 1 reply; 6+ messages in thread From: Randy.Dunlap @ 2004-02-04 0:28 UTC (permalink / raw) To: Andrew Morton; +Cc: linux-kernel, stelian On Tue, 3 Feb 2004 15:57:52 -0800 Andrew Morton <akpm@osdl.org> wrote: | "Randy.Dunlap" <rddunlap@osdl.org> wrote: | > | > diff -Naurp ./drivers/media/video/meye.c~meye_dma ./drivers/media/video/meye.c | > --- ./drivers/media/video/meye.c~meye_dma 2004-01-08 22:59:44.000000000 -0800 | > +++ ./drivers/media/video/meye.c 2004-02-03 14:43:42.000000000 -0800 | > @@ -162,7 +162,7 @@ static void rvfree(void * mem, unsigned | > | > /* return a page table pointing to N pages of locked memory */ | > static int ptable_alloc(void) { | > - u32 *pt; | > + dma_addr_t *pt; | > int i; | > | > memset(meye.mchip_ptable, 0, sizeof(meye.mchip_ptable)); | > @@ -176,7 +176,7 @@ static int ptable_alloc(void) { | > return -1; | > } | > | > - pt = (u32 *)meye.mchip_ptable[MCHIP_NB_PAGES]; | > + pt = (dma_addr_t *)meye.mchip_ptable[MCHIP_NB_PAGES]; | > for (i = 0; i < MCHIP_NB_PAGES; i++) { | > meye.mchip_ptable[i] = dma_alloc_coherent(&meye.mchip_dev->dev, | > PAGE_SIZE, | | mchip_ptable[] just contains pointers to kernel memory. It doesn't seem | appropriate to be casting these to dma_addr_t's Ugh... if I am reading this correcly, what I see is that mchip_table[] is mostly used for kernel pointers, like you say. However, the last entry is used for dma handles returned by dma_alloc_coherent() [the ptable toc], and these are not the correct data type when CONFIG_HIGHMEM64G=y. I expect that Stelian will sort this out, but it looks to me like the ptable toc should be a different array (and type). -- ~Randy ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] meye: correct printk of dma_addr_t 2004-02-04 0:28 ` Randy.Dunlap @ 2004-02-04 8:44 ` Stelian Pop 2004-02-04 16:05 ` Randy.Dunlap 0 siblings, 1 reply; 6+ messages in thread From: Stelian Pop @ 2004-02-04 8:44 UTC (permalink / raw) To: Randy.Dunlap; +Cc: Andrew Morton, linux-kernel On Tue, Feb 03, 2004 at 04:28:22PM -0800, Randy.Dunlap wrote: > | mchip_ptable[] just contains pointers to kernel memory. It doesn't seem > | appropriate to be casting these to dma_addr_t's > > > Ugh... if I am reading this correcly, what I see is that > mchip_table[] is mostly used for kernel pointers, like you say. Yep. The meye driver uses 256 PAGE_SIZE buffers. The kernel virtual pointers to these buffers are stored in mchip_ptable[]. The "toc" containing the dma pointers to these buffers must be given to the device as a table having 256 entries, each being 32 bits in length. In the code I used the last entry of mchip_ptable to store the toc, but I could as well construct a different data structure. Anyway, the device expects a dma_addr to be 32 bits in length, so this will not work anyway with HIGHMEM32 (moreover, this is a driver for the motion eye camera which is found only in C1 Vaio Sony laptops, which motherboard is limited to 384 MB...). I can see only two solutions: *) put the toc in a different, dma_addr_t[] structure, so the driver will compile even with HIGHMEM32 (but won't work...) *) exclude meye from kernel config when HIGHMEM32 is set. Which one do you prefer ? Stelian. -- Stelian Pop <stelian@popies.net> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] meye: correct printk of dma_addr_t 2004-02-04 8:44 ` Stelian Pop @ 2004-02-04 16:05 ` Randy.Dunlap 2004-02-05 13:37 ` Stelian Pop 0 siblings, 1 reply; 6+ messages in thread From: Randy.Dunlap @ 2004-02-04 16:05 UTC (permalink / raw) To: Stelian Pop; +Cc: akpm, linux-kernel On Wed, 4 Feb 2004 09:44:37 +0100 Stelian Pop <stelian@popies.net> wrote: | On Tue, Feb 03, 2004 at 04:28:22PM -0800, Randy.Dunlap wrote: | | > | mchip_ptable[] just contains pointers to kernel memory. It doesn't seem | > | appropriate to be casting these to dma_addr_t's | > | > | > Ugh... if I am reading this correcly, what I see is that | > mchip_table[] is mostly used for kernel pointers, like you say. | | Yep. The meye driver uses 256 PAGE_SIZE buffers. The kernel virtual | pointers to these buffers are stored in mchip_ptable[]. The | "toc" containing the dma pointers to these buffers must be given | to the device as a table having 256 entries, each being 32 bits in length. | | In the code I used the last entry of mchip_ptable to store the toc, | but I could as well construct a different data structure. | | Anyway, the device expects a dma_addr to be 32 bits in length, so this | will not work anyway with HIGHMEM32 (moreover, this is a driver for the | motion eye camera which is found only in C1 Vaio Sony laptops, which | motherboard is limited to 384 MB...). | | I can see only two solutions: | *) put the toc in a different, dma_addr_t[] structure, so the | driver will compile even with HIGHMEM32 (but won't work...) | | *) exclude meye from kernel config when HIGHMEM32 is set. for (HIGHMEM64G=y) | Which one do you prefer ? Well, both. :) The toc of dma_addr_t's should be in its own array/structure. Adding some comments or help text about the highmem limitation would also be good. -- ~Randy ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] meye: correct printk of dma_addr_t 2004-02-04 16:05 ` Randy.Dunlap @ 2004-02-05 13:37 ` Stelian Pop 0 siblings, 0 replies; 6+ messages in thread From: Stelian Pop @ 2004-02-05 13:37 UTC (permalink / raw) To: Randy.Dunlap; +Cc: akpm, linux-kernel, Linus Torvalds On Wed, Feb 04, 2004 at 08:05:15AM -0800, Randy.Dunlap wrote: > | Which one do you prefer ? > > > Well, both. :) > > The toc of dma_addr_t's should be in its own array/structure. > > Adding some comments or help text about the highmem limitation > would also be good. I had a feeling you would say that :) Here it is. It compiles and works just fine, please apply. Stelian. ===== drivers/media/video/meye.h 1.11 vs edited ===== --- 1.11/drivers/media/video/meye.h Fri Oct 24 23:14:38 2003 +++ edited/drivers/media/video/meye.h Thu Feb 5 11:43:55 2004 @@ -298,7 +298,8 @@ u8 mchip_fnum; /* current mchip frame number */ unsigned char *mchip_mmregs; /* mchip: memory mapped registers */ - u8 *mchip_ptable[MCHIP_NB_PAGES+1];/* mchip: ptable + ptable toc */ + u8 *mchip_ptable[MCHIP_NB_PAGES];/* mchip: ptable */ + dma_addr_t *mchip_ptable_toc; /* mchip: ptable toc */ dma_addr_t mchip_dmahandle; /* mchip: dma handle to ptable toc */ unsigned char *grab_fbuffer; /* capture framebuffer */ ===== drivers/media/video/meye.c 1.19 vs edited ===== --- 1.19/drivers/media/video/meye.c Wed Aug 27 13:24:51 2003 +++ edited/drivers/media/video/meye.c Thu Feb 5 10:46:29 2004 @@ -160,23 +160,29 @@ } } -/* return a page table pointing to N pages of locked memory */ +/* return a page table pointing to N pages of locked memory + * + * NOTE: The meye device expects dma_addr_t size to be 32 bits + * (the toc must be exactly 1024 entries each of them being 4 bytes + * in size, the whole result being 4096 bytes). We're using here + * dma_addr_t for corectness but the compilation of this driver is + * disabled for HIGHMEM64G=y, where sizeof(dma_addr_t) != 4 */ static int ptable_alloc(void) { - u32 *pt; + dma_addr_t *pt; int i; memset(meye.mchip_ptable, 0, sizeof(meye.mchip_ptable)); - meye.mchip_ptable[MCHIP_NB_PAGES] = dma_alloc_coherent(&meye.mchip_dev->dev, - PAGE_SIZE, - &meye.mchip_dmahandle, - GFP_KERNEL); - if (!meye.mchip_ptable[MCHIP_NB_PAGES]) { + meye.mchip_ptable_toc = dma_alloc_coherent(&meye.mchip_dev->dev, + PAGE_SIZE, + &meye.mchip_dmahandle, + GFP_KERNEL); + if (!meye.mchip_ptable_toc) { meye.mchip_dmahandle = 0; return -1; } - pt = (u32 *)meye.mchip_ptable[MCHIP_NB_PAGES]; + pt = meye.mchip_ptable_toc; for (i = 0; i < MCHIP_NB_PAGES; i++) { meye.mchip_ptable[i] = dma_alloc_coherent(&meye.mchip_dev->dev, PAGE_SIZE, @@ -184,13 +190,18 @@ GFP_KERNEL); if (!meye.mchip_ptable[i]) { int j; - pt = (u32 *)meye.mchip_ptable[MCHIP_NB_PAGES]; + pt = meye.mchip_ptable_toc; for (j = 0; j < i; ++j) { dma_free_coherent(&meye.mchip_dev->dev, PAGE_SIZE, meye.mchip_ptable[j], *pt); pt++; } + dma_free_coherent(&meye.mchip_dev->dev, + PAGE_SIZE, + meye.mchip_ptable_toc, + meye.mchip_dmahandle); + meye.mchip_ptable_toc = 0; meye.mchip_dmahandle = 0; return -1; } @@ -200,10 +211,10 @@ } static void ptable_free(void) { - u32 *pt; + dma_addr_t *pt; int i; - pt = (u32 *)meye.mchip_ptable[MCHIP_NB_PAGES]; + pt = meye.mchip_ptable_toc; for (i = 0; i < MCHIP_NB_PAGES; i++) { if (meye.mchip_ptable[i]) dma_free_coherent(&meye.mchip_dev->dev, @@ -212,13 +223,14 @@ pt++; } - if (meye.mchip_ptable[MCHIP_NB_PAGES]) + if (meye.mchip_ptable_toc) dma_free_coherent(&meye.mchip_dev->dev, PAGE_SIZE, - meye.mchip_ptable[MCHIP_NB_PAGES], + meye.mchip_ptable_toc, meye.mchip_dmahandle); memset(meye.mchip_ptable, 0, sizeof(meye.mchip_ptable)); + meye.mchip_ptable_toc = 0; meye.mchip_dmahandle = 0; } ===== drivers/media/video/Kconfig 1.17 vs edited ===== --- 1.17/drivers/media/video/Kconfig Tue Jan 20 00:20:19 2004 +++ edited/drivers/media/video/Kconfig Thu Feb 5 11:12:49 2004 @@ -205,7 +205,7 @@ config VIDEO_MEYE tristate "Sony Vaio Picturebook Motion Eye Video For Linux (EXPERIMENTAL)" - depends on VIDEO_DEV && SONYPI + depends on VIDEO_DEV && SONYPI && !HIGHMEM64G ---help--- This is the video4linux driver for the Motion Eye camera found in the Vaio Picturebook laptops. Please read the material in -- Stelian Pop <stelian@popies.net> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2004-02-05 13:37 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-02-03 23:36 [PATCH] meye: correct printk of dma_addr_t Randy.Dunlap 2004-02-03 23:57 ` Andrew Morton 2004-02-04 0:28 ` Randy.Dunlap 2004-02-04 8:44 ` Stelian Pop 2004-02-04 16:05 ` Randy.Dunlap 2004-02-05 13:37 ` Stelian Pop
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox