Re: [PATCH] Add OMAP2 camera driver

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

 



On Mon, 8 Dec 2008 19:42:35 -0200
Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxx> wrote:

> On Thu, 27 Nov 2008 00:14:51 +0530
> "Trilok Soni" <soni.trilok@xxxxxxxxx> wrote:
> 
> > +
> > +/*
> > + *
> > + * DMA hardware.
> > + *
> > + */
> > +
> > +/* Ack all interrupt on CSR and IRQSTATUS_L0 */
> > +static void omap24xxcam_dmahw_ack_all(unsigned long base)
> 
> Oh, no! yet another dma video buffers handling...
> 
> Soni, couldn't this be converted to use videobuf?
> 

Just explaining myself: I can see two different parts on your omap driver:

1) omap specific functions (like omap register setups);

2) a scatter/gather DMA driver that seems to be tailored specifically to omap2.

I'm not sure why you didn't just use videobuf-dma-sg for (2). You should
have your reasons. 

However, instead of adding a new DMA S/G handler inside the
driver, the better would be either to:

a) patch the existing one to attend your needs; 
b) if what you need is so different than the existing driver, you may write
another videobuf-dma-sg-foo driver, clearly documenting why you couldn't use
the existing driver, and making it generic enough to be used by other drivers.

Splitting this into two files/drivers make easier for it to be analyzed and
understood.

Btw, I'm now reviewing all the patches from the pending pull request. I may
have other comments later, since this is a big series of patches, and will
require me some time to deeply inspect each one.

Cheers,
Mauro

--
video4linux-list mailing list
Unsubscribe mailto:video4linux-list-request@xxxxxxxxxx?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list

[Index of Archives]     [Linux Media]     [Older V4L]     [Linux DVB]     [Video Disk Recorder]     [Linux Kernel]     [Asterisk]     [DCCP]     [Netdev]     [X.org]     [Util Linux]     [Xfree86]     [Free Photo Albums]     [Fedora Users]     [Fedora Women]     [ALSA Users]     [ALSA Devel]     [SSH]     [DVB Maintainers]     [Linux USB]     [Yosemite Information]
  Powered by Linux