mchehab: ping mchehab: I've reviewed v6 I haven't commented on 4/8 and 5/8, but the comments I made on patches 1/8 to 4/8 apply there pinchartl: thanks for the review. I'll take a look on them tomorrow pinchartl: FYI, we can't use ftrace due to things like: http://pastebin.com/3v2T2Wt0 hverkuil: Hello, thanks for your review! one fast question. Can I modify saa7164 so it accespts CLASS_MPEG and V4L2_CTRL_WHICH_CUR_VAL ? ribalda: I wouldn't spend time on it. It should be converted to the control framework instead. it is not a big deal. Takes two second to change it don't bother. Leaving this out is just another incentive to finally convert this driver. ribalda: I'm working on a quick saa7164 conversion to the ctrl framework. couldn't stand it anymore :-) hverkuil: could you take a fast look to this? https://github.com/ribalda/linux/commit/41e45f8c4ae61e26f72e94d6bb92566790e1b95a is this what you mean on your first mail? ribalda: yes, but you forgot to do the same update in Documentation/ Just do a git grep on the full kernel so you're certain you got it all. Note: the only place where ctrl_class should still appear is in the VIDIOC_G_EXT_CTRLS DocBook documentation. hverkuil: Better now? https://github.com/ribalda/linux/commit/e801d93820d31ce7aafc610a620e7610f860f0a8 yes ok, I pack it and ship it as v2. Thanks for the hyperfast review :). oh, one more: in v4l2-ioctl.c, v4l_print_ext_controls, the pr_cont call uses "class=0x%x", that should become "which=0x%x" goot catch, sorry about that mchehab: I'm not sure to see why that prevents using ftrace :-) did you see my email? need to get this at serial console, before Kernel hangs due to some OOPS and printed debug messages should be displayed at the order they occur no I haven't checked the e-mails yet pinchartl: btw, you mentioned about DocBook compilation on one of the emails... where are the media controller DocBook header files located after calling make htmldocs? mchehab: good question :-) let me check (I'm in a meeting so I'll be a bit slow) I tried here: ack -a media_entity_operations Documentation/DocBook/ but it didn't find anything it might not be there and media_entity_operations has the tags by the way make htmldocs fails on the media docbook on master :-/ just fixed that ok :-) let me pull a </para> missing I sent the patch not merged upstream yet ok let me apply it then :-) ok make htmldocs still fails :-/ well, make htmldocs currently causes *LOTS* of warnings due to a bug at htmlto s/htmlto/xmlto/ what I do to generate the main xml is: make DOCBOOKS=media_api.xml htmldocs 2>&1 | grep -v "element.*: validity error : ID .* already defined" (media stuff) warning: failed to load external entity "/home/laurent/src/kernel/media.git/Documentation/DocBook/pixfmt-y16-be.xml" /home/laurent/src/kernel/media.git/Documentation/DocBook/pixfmt.xml:1513: parser error : Failure to process entity sub-y16-be &sub-y16-be; ^ /home/laurent/src/kernel/media.git/Documentation/DocBook/pixfmt.xml:1513: parser error : Entity 'sub-y16-be' not defined &sub-y16-be; $ make htmldocs GEN Documentation/DocBook//v4l2.xml rm -rf Documentation/DocBook/index.html; echo '<h1>Linux Kernel HTML Documentation</h1>' >> Documentation/DocBook/index.html && echo '<h2>Kernel Version: 4.2.0-rc2</h2>' >> Documentation/DocBook/index.html && cat Documentation/DocBook/80211.html Documentation/DocBook/alsa-driver-api.html Documentation/DocBook/crypto-API.html Documentation/DocBook/debugobjects.html Documentation/DocBook/device-drivers.html Documentation/DocBook/deviceiobook.html Documentation/DocBook/drm.html Documentation/DocBook/filesystems.html Documentation/DocBook/gadget.html Documentation/DocBook/genericirq.html Documentation/DocBook/kernel-api.html Documentation/DocBook/kernel-hacking.html Documentation/DocBook/kernel-locking.html Documentation/DocBook/kgdb.html Documentation/DocBook/libata.html Documentation/DocBook/librs.html Documentation/DocBook/lsm.html Documentation/DocBook/media_api.html Documentation/DocBook/mtdnand.html Documentation/DocBook/networking.html Documentation/DocBook/rapidio.html Documentation/DocBook/regulator.html Documentation/DocBook/s390-drivers.html Documentation/DocBook/scsi.html Documentation/DocBook/sh.html Documentation/DocBook/tracepoint.html Documentation/DocBook/uio-howto.html Documentation/DocBook/usb.html Documentation/DocBook/w1.html Documentation/DocBook/writing-an-alsa-driver.html Documentation/DocBook/writing_musb_glue_layer.html Documentation/DocBook/writing_usb_driver.html Documentation/DocBook/z8530book.html >> Documentation/DocBook/index.html plus the patch I just submitted --- a/Documentation/DocBook/media/dvb/intro.xml +++ b/Documentation/DocBook/media/dvb/intro.xml @@ -164,7 +164,7 @@ are called:</para> from 0, and M enumerates the devices of each type within each adapter, starting from 0, too. We will omit the “ <constant>/dev/dvb/adapterN/</constant>” in the further discussion -of these devices. +of these devices.</para> diff --git a/Documentation/DocBook/media/dvb/intro.xml b/Documentation/DocBook/media/dvb/intro.xml index d30751338493..51db15648099 100644 --- a/Documentation/DocBook/media/dvb/intro.xml +++ b/Documentation/DocBook/media/dvb/intro.xml @@ -167 +167 @@ adapter, starting from 0, too. We will omit the “ -of these devices. +of these devices.</para> weird unable to parse /home/laurent/src/kernel/media.git/Documentation/DocBook/media_api.xml are you using it with O= parameter? no weird just make htmldocs perhaps some leftovers? try make cleandocs https://paste.kde.org/pp9jqq12c really weird perhaps a broken xmlto toolchain let me cleandocs it helped, thanks looks like we're not processing any header neither MC nor V4L2 yep it shouldn't be difficult to fix it's not a prerequisite for your patch series, but it would be nice to be able to test kerneldoc in the not too distant future I'll try to reply to your replies later today reply to the replies and answer the answers :-) intro.xml is broken, there is a missing </para>. We never changed that, so it comes from upstream. noticed the same problem yesterday never mind. I see the patch. commit ed099f66dadd8bac93571fb28b05bdae066b39a2 Author: Masanari Iida <standby24x7@gmail.com> Date: Mon Jul 13 20:36:50 2015 -0300 [media] DocBook: Fix typo in intro.xml it has your SoB ;-) pinchartl: adding things there are easy... but one should do the hardwork of fixing the already broken things http://pastebin.com/JcXsqcVH just with core stuff that really shows we have never compiled it :-) yep patch enabling it sent to ML won't apply until someone have time to fix things Actually, just two errors: Error(.//drivers/media/dvb-core/dvb_math.h:35): duplicate section name 'example' Error(.//drivers/media/dvb-core/dvb_math.h:39): duplicate section name 'example' easily fixable, it seems still lots of: .//include/media/videobuf2-memops.h - Document generation inconsistency at the generated docbook as long as compilation doesn't break we can fix that as we go int - callback function for use with vb2_thread Synopsis typedef int ( * vb2_thread_fnc); Arguments vb2_thread_fnc -- undescribed -- Description This is called whenever a buffer is dequeued in the thread. that's funny... we just invented a function called "int" :) somewhere in VB2 pinchartl: I guess the best is to add for now just the stuff that minimally produces something useable... fix then and move to the other stuff that are just plain broken (even if compilation won't cause errors) like this: http://pastebin.com/XP0s3WJr still, lots of warnings to fix: http://pastebin.com/Ld916dFi pinchartl, hverkuil: sent a version 2 of the patch enabling it mchehab: agreed it should also be in a clearly separated section of the docbook documentation as it documents the in-kernel API, not the userspace API pinchartl: it is I added it at the device-drivers section, just after Sound drivers mchehab: I haven't had a chance to review all the patches yet. It might be easier to review them with a use-case - is it possible to show how these interfaces re used btw - we can discuss this later in detail. I am in a meeting and have to head to the airport soon