* [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