Hi Oliver and Marco,
The patch looked good to me.
Some comments:
IMO, instead of creating an emum for vidmode, I would instead just store
v4l2_std_id there.
> if (std->id & V4L2_STD_PAL) {
> - av7110->vidmode = VIDEO_MODE_PAL;
> + av7110->vidmode = AV7110_VIDEO_MODE_PAL;
> av7110_set_vidmode(av7110, av7110->vidmode);
> }
> else if (std->id & V4L2_STD_NTSC) {
> - av7110->vidmode = VIDEO_MODE_NTSC;
> + av7110->vidmode = AV7110_VIDEO_MODE_NTSC;
> av7110_set_vidmode(av7110, av7110->vidmode);
> }
I dunno if av7110 does support PAL/60, PAL/M or PAL/N. I did a quick
look on a datasheet I found at the net for
av7110(http://www.cdaniel.de/download/AV711x_3_1.pdfs). It seems that
the only supported PAL ones are PAL/BDGHI. If this is true, the above
code is perfect.
However, if the driver supports other PAL standards, the above code
won't work, since a few PAL standards are not marked as V4L2_STD PAL
[1]. If this the case, the above code is not correct.
[1] On V4L2, V4L2_STD_PAL means only PAL/BGDKHI. IMHO, this is one of
the weird things at V4L2 API.
To support also PAL/60, and PAL/MN, a better coding would be:
if (std->id & V4L2_STD_SECAM) {
printk(KERN_ERR "Secam is not supported\n");
else if (std->id & V4L2_STD_NTSC) {
av7110->vidmode = AV7110_VIDEO_MODE_NTSC;
av7110_set_vidmode(av7110, av7110->vidmode);
} else {
av7110->vidmode = AV7110_VIDEO_MODE_PAL;
av7110_set_vidmode(av7110, av7110->vidmode);
}
Or to use V4L2_STD_525_60 (for 60 Hz standards) and V4L2_STD_625_50 (for
50 Hz standards).
Also, please review the patch with scripts/checkpatch.pl.
Cheers,
Mauro
_______________________________________________
linux-dvb mailing list
linux-dvb@xxxxxxxxxxx
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
