On Mon, Dec 19, 2005, Hans Verkuil wrote:
> On Monday 19 December 2005 12:24, Johannes Stezenbach wrote:
> > On Mon, Dec 19, 2005, Hans Verkuil wrote:
> >
> > > + /* The base pointer is a 32-bit pointer into DMA memory.
> > > + The unsigned long cast is to remove a 64-bit compile warning
> > > since + it looks like a 64-bit address is cast to a 32-bit
> > > value. */ + u32 base = (u32)(unsigned long)vv->ov_fb.base;
...
> struct v4l2_framebuffer
> {
> __u32 capability;
> __u32 flags;
> /* FIXME: in theory we should pass something like PCI device + memory
> * region + offset instead of some physical address */
> void* base;
> struct v4l2_pix_format fmt;
> };
>
> So base is a pointer to PCI memory.
>
> In saa7146_set_position() from saa7146_hlp.c the base address is used to
> set DMA addresses which are passed to saa7146_write_out_dma(). And that
> function writes them as 32-bit values to the saa7146.
>
> It is clear that for the saa7146 the base pointer cannot be a 64-bit
> address and should in fact probably be of type dma_addr_t. However, I'm
> not sure if struct v4l2_framebuffer isn't (mis)used in other places and
> for other devices where it can really be a 64-bit pointer. I don't dare
> touch that one.
OK, thanks for clearing this up. Could you please add to your comment
- it's a 32bit physical address that goes into a 32bit DMA register
- FIXME: probably doesn't work on some 64bit platforms, see FIXME
in struct v4l2_framebuffer
...so it passes Al Viro's critical eyes?
Please commit with these changes.
Thanks,
Johannes
