Am Montag, den 29.03.2010, 08:40 +0200 schrieb Hans Verkuil: > 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 How to avoid this and similar? "you added your own dma_contig implementation" The answer is very simple. NACK. Hermann -- 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