> > I don't like to create a video-buf.h header. This will make non-pci> > devices dependent on PCI, or will require some additional logic for> > checking kernel Kconfig symbols. I also expect that other newer videobuf> > methods to be created. So, this header will just generate undesirable> > mess.> > This would not create dependencies of non-pci devices on pci -- a> simple header inclusion is optimized by the c compiler -- only needed> methods are considered and are actually included in the build.
Wrong. This has nothing to do with c optimizer. Try this:
Create this as header.h:
struct foo { struct some_pci_struct foo;};
Create this as main.c:
#include "header.h"
int main (void){ return 0;}
You will got the following error:
/tmp/header.h:2: error: field ‘foo’ has incomplete type
If we use your approach, a non-pci videobuf driver will work only at thearchitectures where you have a PCI bus (since the pci structs won'texist on that architecture).
> > What we might do is to rename the generic abstract method to another> > name. This will generate compilation errors, making easier for people to> > realize what happened.> > If we rename it to video-buf.h, it would cover the issue that I have in mind.> It is much more clear to include videobuf-vmalloc or videobuf-dma-sg,depending on the proper videobuf module that will be needed by a driver.
Creating a video-buf.h that just includes the two will be unused by thedrivers. So, what's the sense of creating a header file at the kernelthat aren't used inside kernel?
> > I'm not sure if this is valuable enough, since I don't know any other> > DMA S/G driver using videobuf being developed for their inclusion at> > Kernel.> > Maybe an out of tree driver uses it?
Although there's no intention to make life harder for out-of-treedrivers, they shouldn't be considered on changes made at kernelinternals. The proper place for a kernel driver is in-kernel, notoutside.
Anyway, a compilation with a driver including video-buf.h currently willgenerate an error, indicating that something has changed at v4linfrastructure. By creating a header like you're proposing won't fixthis.> > Maybe there is an in-tree driver that still might have old methods being used but we didnt hit those bugs yet due to incomplete testing methods?
The only in-kernel drivers that use videobuf infrastructure are: cx88, saa7134, bttv, saa7146 and, now, cx23885.
After the patch, all of they are already converted.
grep videobuf_queue_pci_init *.cbttv-driver.c: videobuf_queue_pci_init(&fh->cap, &bttv_video_qops,bttv-driver.c: videobuf_queue_pci_init(&fh->vbi, &bttv_vbi_qops,cx23885-dvb.c: videobuf_queue_pci_init(&port->dvb.dvbq, &dvb_qops,dev->pci, &port->slock,cx88-blackbird.c: videobuf_queue_pci_init(&fh->mpegq,&blackbird_qops,cx88-dvb.c: videobuf_queue_pci_init(&dev->dvb.dvbq, &dvb_qops,cx88-video.c: videobuf_queue_pci_init(&fh->vidq, &cx8800_video_qops,cx88-video.c: videobuf_queue_pci_init(&fh->vbiq, &cx8800_vbi_qops,saa7134-dvb.c: videobuf_queue_pci_init(&dev->dvb.dvbq,&saa7134_ts_qops,saa7134-empress.c: videobuf_queue_pci_init(&dev->empress_tsq,&saa7134_ts_qops,saa7134-video.c: videobuf_queue_pci_init(&fh->cap, &video_qops,saa7134-video.c: videobuf_queue_pci_init(&fh->vbi,&saa7134_vbi_qops,saa7146_vbi.c: videobuf_queue_pci_init(&fh->vbi_q, &vbi_qops,saa7146_video.c: videobuf_queue_pci_init(&fh->video_q,&video_qops,videobuf-dma-sg.c:void videobuf_queue_pci_init(struct videobuf_queue* q,
There's no other driver using the abstract videobuf_queue_init.
A final point for your consideration:
videobuf_queue_init is the only function that had its meaning changedwithout changing its parameters (currently, it is just an abstractmethod. In the past, this were the function that were used to initializea videobuf queue).
The changes you're proposing won't solve the target you've addressed inthe first place, since it will still compile without warning, if youforget to rename it to videobuf_queue_pci_init.
The proper change is simply doing something like:
s/videobuf_queue_init/videobuf_queue_abstract_init/
After this, on all places where videobuf stuff is used withoutconversion, an error will be generated.
After my considerations, if you still think this is important, I'llaccept a patch renaming the old videobuf_queue_init to a new name.
-- Cheers,Mauro
_______________________________________________linux-dvb mailing listlinux-dvb@xxxxxxxxxxxxxxx://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
