> > 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 the
architectures where you have a PCI bus (since the pci structs won't
exist 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 the
drivers. So, what's the sense of creating a header file at the kernel
that 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-tree
drivers, they shouldn't be considered on changes made at kernel
internals. The proper place for a kernel driver is in-kernel, not
outside.
Anyway, a compilation with a driver including video-buf.h currently will
generate an error, indicating that something has changed at v4l
infrastructure. By creating a header like you're proposing won't fix
this.
>
> 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 *.c
bttv-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 changed
without changing its parameters (currently, it is just an abstract
method. In the past, this were the function that were used to initialize
a videobuf queue).
The changes you're proposing won't solve the target you've addressed in
the first place, since it will still compile without warning, if you
forget 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 without
conversion, an error will be generated.
After my considerations, if you still think this is important, I'll
accept a patch renaming the old videobuf_queue_init to a new name.
--
Cheers,
Mauro
_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
