Hi Xiaolin,
On Sunday 28 March 2010 16:42:30 Zhang, Xiaolin wrote:
> From 1c18c41be33246e4b766d0e95e28a72dded87475 Mon Sep 17 00:00:00 2001
> From: Xiaolin Zhang <xiaolin.zhang@xxxxxxxxx>
> Date: Sun, 28 Mar 2010 21:31:24 +0800
> Subject: [PATCH 2/10] This patch is second part of intel moorestown isp driver and c files collection which is v4l2 implementation.
>
....
> +struct videobuf_dma_contig_memory {
> + u32 magic;
> + void *vaddr;
> + dma_addr_t dma_handle;
> + unsigned long size;
> + int is_userptr;
> +};
> +
> +#define MAGIC_DC_MEM 0x0733ac61
> +#define MAGIC_CHECK(is, should) \
> + if (unlikely((is) != (should))) { \
> + pr_err("magic mismatch: %x expected %x\n", (is), (should)); \
> + BUG(); \
> + }
I will do a more in-depth review in a few days. However, I did notice that
you added your own dma_contig implementation. What were the reasons for doing
this? I've CC-ed Pawel since he will be interested in this as well.
Another question that came up is: what is 'marvin'? It's clearly a codename,
but a codename for what? This should be documented at the top of some source
or header. Apologies if it is already documented, I didn't read everything yet.
A final point I noticed: don't cast away a function result:
(void)ci_isp_set_bp_detection(NULL);
No need for (void). The gcc compiler won't warn about this unless the function
is annotated with __must_check__.
Regards,
Hans
--
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
