On 23/03/2021 09:23, Michael Tretter wrote: > Hi Hans, > > On Tue, 23 Mar 2021 08:57:53 +0100, Hans Verkuil wrote: >> On 23/03/2021 08:52, Michael Tretter wrote: >>> On Tue, 23 Mar 2021 08:49:13 +0100, Hans Verkuil wrote: >>>> Give typo in kernel-doc documentation: hdr -> hrd >>>> >>>> Fixes this warning: >>>> >>>> drivers/media/platform/allegro-dvt/nal-h264.h:33: warning: expecting prototype for struct nal_h264_hdr_parameters. Prototype was for struct >>>> nal_h264_hrd_parameters instead >>>> >>>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xxxxxxxxx> >>> >>> Reviewed-by: Michael Tretter <m.tretter@xxxxxxxxxxxxxx> >> >> Since you made the mistake of replying :-) I want to point out that the allegro headers >> produce a large number of kernel-doc warnings since none of the parameters are documented. > > :) > >> >> You can look at the daily build log for all the warnings, but I've copied them below as >> well. >> >> I think you should either document it all, or change /** to /*. > > IMO documenting the parameters is rather pointless, because they are straight > copies from the specifications and the documentation would be "see H264 > specification" for every single one of them. I guess, I'll go for changing /** > to /*. > > Actually, I thought about using the sps/pps structs defined in > include/media/v4l2-ctrls.h. I was not convinced, because these are userspace > facing API. Are the sps/pps definitions something, that would help other > drivers, too, or should we rather avoid global definitions to discourage > sps/pps parsing/generation in drivers? There is nothing wrong with using v4l2_ctrl_h264_sps/pps in your driver instead of your own structs. Just note that these structs are now part of the uAPI, so can't be changed. If you need allegro specific data as well, then you might be better off keeping your own structs. I'm not sure what you mean with your last question. If a driver needs to do sps/pps parsing/generation, then why would it matter if it uses global or local definitions? It still has to do it, right? And having reusable code might help others. Regards, Hans > > Michael >
