On Mon, Sep 12, 2022 at 06:47:56PM +0200, Mauro Carvalho Chehab wrote: > Hi Matt, > > Em Mon, 12 Sep 2022 08:09:57 -0700 > Matt Roper <matthew.d.roper@xxxxxxxxx> escreveu: > > > > --- a/drivers/gpu/drm/i915/gt/intel_context_types.h > > > +++ b/drivers/gpu/drm/i915/gt/intel_context_types.h > > > > Several of the comments in this file do appear to be kerneldoc (in fact > > kerneldoc that was specifically requested during code review at > > https://patchwork.freedesktop.org/patch/448456/#comment_804252) and this > > file is included from Documentation/gpu/i915.rst, so I think some of > > these changes might be moving in the wrong direction. Should we instead > > focus on fixing up the comments that aren't quite formatted properly? > > Those *appear* to be kernel-doc markups, but they aren't, because > the structs themselves are not properly marked. See, for instance > struct intel_context. > > scripts/kerneldoc will *only* consider what's there as a proper > comment if you add: > > /** > * struct intel_context - describes an i915 context > * <add a proper description for it> > */ > struct intel_context { > union { > /** @ref: a kernel object reference */ > struct kref ref; /* no kref_get_unless_zero()! */ > /** @rcu: a rcu header */ > struct rcu_head rcu; > }; > ... > } > > Describing all fields inside the struct. Just placing > /** something */ > on some random places in the middle doesn't make it a kernel-doc. Right, what we have today is incomplete/incorrect. But I think we should be fixing that by adding the missing documentation on the structure, rather than giving up and removing the kerneldoc. The end goal should be to have proper generated documentation, not just silence the warnings while leaving the actual output incomplete. Matt > > If you actually run kernel-doc in Werror mode: > > ./scripts/kernel-doc -Werror -sphinx-version 2.4.4 drivers/gpu/drm/i915/gt/intel_context_types.h | echo "ERROR!" > ERROR! > drivers/gpu/drm/i915/gt/intel_context_types.h:1: warning: no structured comments found > 1 warnings as Errors > > you'll see that this is currently broken. > > Thanks, > Mauro -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation