Re: [PATCH v2 2/10] V4L2 patches for Intel Moorestown Camera Imaging Drivers - part 1

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Linux Input]     [Video for Linux]     [Gstreamer Embedded]     [Mplayer Users]     [Linux USB Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux