I'm trying to use the pxafb driver on mach-pxa, but I can't mmap the
framebuffer memory. I can access it from the driver, filling the entire
screen, but when I access the pointer returned from mmap from a user space
program, the following two things happens:
- the vm_pgoff is ignored and I get the start of the internal buffer, which
caused writing to the palette and DMA descriptors
- when I change the location of the framebuffer to the start of the internal
buffer, I can write to the screen, but only to the first 4 k; any write
after this address is ignored, but no segfault is generated.
Any ideas what I can do to find the reason? I don't think that it is a
kernel bug, but perhaps a wrong configuration for my platform.
--
Frank Bu?, [email protected]
http://www.frank-buss.de, http://www.it4-systems.de
On Thu, Feb 17, 2005 at 06:51:49PM +0100, Frank Buss wrote:
> I'm trying to use the pxafb driver on mach-pxa, but I can't mmap the
> framebuffer memory. I can access it from the driver, filling the entire
> screen, but when I access the pointer returned from mmap from a user space
> program, the following two things happens:
>
> - the vm_pgoff is ignored and I get the start of the internal buffer, which
> caused writing to the palette and DMA descriptors
>
> - when I change the location of the framebuffer to the start of the internal
> buffer, I can write to the screen, but only to the first 4 k; any write
> after this address is ignored, but no segfault is generated.
>
> Any ideas what I can do to find the reason? I don't think that it is a
> kernel bug, but perhaps a wrong configuration for my platform.
Please try this (and revert your changes):
===== arch/arm/mm/consistent.c 1.27 vs edited =====
--- 1.27/arch/arm/mm/consistent.c 2005-01-21 05:02:14 +00:00
+++ edited/arch/arm/mm/consistent.c 2005-02-17 18:11:50 +00:00
@@ -284,13 +284,15 @@
spin_unlock_irqrestore(&consistent_lock, flags);
if (c) {
+ unsigned long off = vma->vm_pgoff;
+
kern_size = (c->vm_end - c->vm_start) >> PAGE_SHIFT;
- if (vma->vm_pgoff < kern_size &&
- user_size <= (kern_size - vma->vm_pgoff)) {
+ if (off < kern_size &&
+ user_size <= (kern_size - off)) {
vma->vm_flags |= VM_RESERVED;
ret = remap_pfn_range(vma, vma->vm_start,
- page_to_pfn(c->vm_pages),
+ page_to_pfn(c->vm_pages) + off,
user_size, vma->vm_page_prot);
}
}
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
> Please try this (and revert your changes):
thanks, this could fix the bug with the vm_pgoff, but I don't think this
will fix the problem with the ignored memory access after the first
PAGE_SIZE from the mapped memory. I'll try it tommorow, when I'm again at my
customers site, where I have access to the board.
--
Frank Bu?, [email protected]
http://www.frank-buss.de, http://www.it4-systems.de
On Thu, Feb 17, 2005 at 08:14:34PM +0100, Frank Buss wrote:
> > Please try this (and revert your changes):
>
> thanks, this could fix the bug with the vm_pgoff, but I don't think this
> will fix the problem with the ignored memory access after the first
> PAGE_SIZE from the mapped memory. I'll try it tommorow, when I'm again at my
> customers site, where I have access to the board.
Since we map the whole lot in one go, if you get one page, there's no
reason why you shouldn't get the lot. This is why I'm wondering if
it has something to do with your other modifications.
--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core
Hi,
Looking at TTY code, I noticed a weird test done in "opost_bock"
located in n_tty.c file. I don't understand why the following test is
done at the start of the function:
if (nr > sizeof(buf))
nr = sizeof(buf);
Actually it limits the size of processing blocks to 4 bytes and I can't
find a reason why.
Second point, a lot of serial drivers call in their interrupt handler
"tty_flip_buffer_push" function. This function must no be called
in interrupt context. Why is it done anyway ?
Thanks for your answers.
Franck
Franck Bui-Huu wrote:
> Looking at TTY code, I noticed a weird test done in "opost_bock"
> located in n_tty.c file. I don't understand why the following test is
> done at the start of the function:
> if (nr > sizeof(buf))
> nr = sizeof(buf);
> Actually it limits the size of processing blocks to 4 bytes and I can't
> find a reason why.
No, it limits the size to 80 bytes,
which is the size of buf.
sizeof returns the size of the char array buf[80]
(standard C)
> Second point, a lot of serial drivers call in their interrupt handler
> "tty_flip_buffer_push" function. This function must no be called
> in interrupt context. Why is it done anyway ?
Calling tty_flip_buffer_push() is fine from interrupt
as long as tty->low_latency is not set. It just queues
work for later.
--
Paul Fulghum
Microgate Systems, Ltd.
Paul Fulghum wrote:
> Franck Bui-Huu wrote:
>
>> Looking at TTY code, I noticed a weird test done in "opost_bock"
>> located in n_tty.c file. I don't understand why the following test is
>> done at the start of the function:
>> if (nr > sizeof(buf))
>> nr = sizeof(buf);
>> Actually it limits the size of processing blocks to 4 bytes and I can't
>> find a reason why.
>
>
> No, it limits the size to 80 bytes,
> which is the size of buf.
>
> sizeof returns the size of the char array buf[80]
> (standard C)
Looking at the code, I think Franck is right. buf is a "const unsigned
char *" for which sizeof(buf) is the size of a pointer.
This certainly looks like a bug.
Since the function doesn't guarantee that nr bytes are written, and the
caller must handle the case of fewer bytes, this probably went unnoticed.
--
Paulo Marques - http://www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
Paulo Marques wrote:
> Paul Fulghum wrote:
>> No, it limits the size to 80 bytes,
>> which is the size of buf.
>>
>> sizeof returns the size of the char array buf[80]
>> (standard C)
>
> Looking at the code, I think Franck is right. buf is a "const unsigned
> char *" for which sizeof(buf) is the size of a pointer.
What kernel version are you looking at?
I'm looking at 2.4.20 n_tty.c opost_block() and
buf is a char array.
--
Paul Fulghum
Microgate Systems, Ltd.
On Fri, 18 Feb 2005, Paulo Marques wrote:
> Franck Bui-Huu wrote:
>
>> Looking at TTY code, I noticed a weird test done in "opost_bock"
>> located in n_tty.c file. I don't understand why the following test is
>> done at the start of the function:
>> if (nr > sizeof(buf))
>> nr = sizeof(buf);
>> Actually it limits the size of processing blocks to 4 bytes and I can't
>> find a reason why.
>
> No, it limits the size to 80 bytes,
> which is the size of buf.
>
> sizeof returns the size of the char array buf[80]
> (standard C)
>
Huh?? "buf" is a pointer. On this system pointers are 4-bytes
in length. What "standard C" are you using? Frank found a bug.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.10 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by Dictator Bush.
98.36% of all statistics are fiction.
Paul Fulghum wrote:
> Paulo Marques wrote:
>
>> Paul Fulghum wrote:
>>
>>> No, it limits the size to 80 bytes,
>>> which is the size of buf.
>>>
>>> sizeof returns the size of the char array buf[80]
>>> (standard C)
>>
>>
>> Looking at the code, I think Franck is right. buf is a "const unsigned
>> char *" for which sizeof(buf) is the size of a pointer.
>
>
> What kernel version are you looking at?
> I'm looking at 2.4.20 n_tty.c opost_block() and
> buf is a char array.
Oops, this should have been clarified sooner...
I was looking at a 2.6.11-rc2 tree I have hanging around here. So maybe
the "buf" type was changed and the condition remained, and that produced
the bug.
--
Paulo Marques - http://www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
On Fri, 18 Feb 2005, Paul Fulghum wrote:
> Paulo Marques wrote:
>> Paul Fulghum wrote:
>>> No, it limits the size to 80 bytes,
>>> which is the size of buf.
>>>
>>> sizeof returns the size of the char array buf[80]
>>> (standard C)
>>
>> Looking at the code, I think Franck is right. buf is a "const unsigned char
>> *" for which sizeof(buf) is the size of a pointer.
>
> What kernel version are you looking at?
> I'm looking at 2.4.20 n_tty.c opost_block() and
> buf is a char array.
>
> --
> Paul Fulghum
> Microgate Systems, Ltd.
Ahaa! That's how the bug got introduced. It used to be an
array and then it got changed to a pointer! linux-2.4.26
also shows a local array.
Cheers,
Dick Johnson
Penguin : Linux version 2.6.10 on an i686 machine (5537.79 BogoMips).
Notice : All mail here is now cached for review by Dictator Bush.
98.36% of all statistics are fiction.
linux-os wrote:
> On Fri, 18 Feb 2005, Paul Fulghum wrote:
>
>> Paulo Marques wrote:
>>
>>> Paul Fulghum wrote:
>>>
>>>> No, it limits the size to 80 bytes,
>>>> which is the size of buf.
>>>>
>>>> sizeof returns the size of the char array buf[80]
>>>> (standard C)
>>>
>>>
>>> Looking at the code, I think Franck is right. buf is a "const
>>> unsigned char *" for which sizeof(buf) is the size of a pointer.
>>
>>
>> What kernel version are you looking at?
>> I'm looking at 2.4.20 n_tty.c opost_block() and
>> buf is a char array.
>>
>> --
>> Paul Fulghum
>> Microgate Systems, Ltd.
>
>
> Ahaa! That's how the bug got introduced. It used to be an
> array and then it got changed to a pointer! linux-2.4.26
> also shows a local array.
Yes, just looked at the revision history in linux.bkbits.net and Linus
just fixed this 67 hours ago... So we're too late :)
--
Paulo Marques - http://www.grupopie.com
All that is necessary for the triumph of evil is that good men do nothing.
Edmund Burke (1729 - 1797)
>>
>> Ahaa! That's how the bug got introduced. It used to be an
>> array and then it got changed to a pointer! linux-2.4.26
>> also shows a local array.
>
>
> Yes, just looked at the revision history in linux.bkbits.net and Linus
> just fixed this 67 hours ago... So we're too late :)
>
ok, maybe next time :)
Franck
>> Second point, a lot of serial drivers call in their interrupt handler
>> "tty_flip_buffer_push" function. This function must no be called
>> in interrupt context. Why is it done anyway ?
>
>
> Calling tty_flip_buffer_push() is fine from interrupt
> as long as tty->low_latency is not set. It just queues
> work for later.
>
I was looking at driver for 8250 in 8250.c file and at the end
of "receive_chars" interrupt handler, it calls "tty_flip_buffer_push"
even if "tty->low_latency" is set since no such test is done before
the call...
I was also wondering why not always calling "schedule_delayed_work"
whatever the state of "tty->latency"?
Franck
Franck Bui-Huu wrote:
> I was looking at driver for 8250 in 8250.c file and at the end
> of "receive_chars" interrupt handler, it calls "tty_flip_buffer_push"
> even if "tty->low_latency" is set since no such test is done before
> the call...
Yes this is a known problem. In the case of SMP kernel
and low_latency, and 8250.c, this causes a dead lock. This has
resurfaced again and again for many months from
different sources.
There is disagreement on what code is
in error and what should be fixed. So it stays broken.
> I was also wondering why not always calling "schedule_delayed_work"
> whatever the state of "tty->latency"?
If low_latency is set and you are not calling
from interrupt, then calling work directly speeds processing.
I submitted a patch that forced schedule_delayed_work
if in interrupt context to avoid this problem. It was rejected.
--
Paul Fulghum
Microgate Systems, Ltd.
Russell King <[email protected]> wrote:
> Since we map the whole lot in one go, if you get one page, there's no
> reason why you shouldn't get the lot. This is why I'm wondering if
> it has something to do with your other modifications.
your patch works, thanks, but only for the problem with the ignored offset,
as expected. Now I can use the original pxafb driver, but with the same
problem: All writes from user space in the mmap'ed region after the first
4096 bytes are ignored.
Perhaps it is not a kernel bug, but a configuration problem with the
platform, because it is a in-house developed platform. Now a colleague is
working on this problem and reading a book about the Linux 2.6 kernel
details over weekend and learning how to use the kernel debugger (until now
we have used printk and flashing new zImages every time, which is very time
consuming). I'll post the solution next week, if we'll found one.
--
Frank Bu?, [email protected]
http://www.frank-buss.de, http://www.it4-systems.de
Russell King <[email protected]> wrote:
> Since we map the whole lot in one go, if you get one page, there's no
> reason why you shouldn't get the lot. This is why I'm wondering if
> it has something to do with your other modifications.
my colleage has found the bug: in the function dma_mmap in
arch/arm/mm/consistent.c the call to remap_pfn_range uses
user_size in PAGE_SIZE units, but looks like it is expected
in bytes. When using (user_size << PAGE_SHIFT), it works.
I don't know, where to fix it: Should the lower level calls
get the size in bytes (most function arguments in Linux
kernel sources are not commented), this means fixing the
dma_mmap, or should PAGE_SIZE be used, then the lower level
functions needs to be fixed.
--
Frank Bu?, [email protected]
http://www.frank-buss.de, http://www.it4-systems.de