Re: anonymous enums in kernel doc

[Andy Shevchenko]

On Wed, Mar 3, 2021 at 10:44 AM Mauro Carvalho Chehab
<mchehab@xxxxxxxxxx> wrote:
> Em Tue, 16 Feb 2021 19:12:58 +0200
> Andy Shevchenko <andy.shevchenko@xxxxxxxxx> escreveu:
> > On Tue, Feb 16, 2021 at 7:05 PM Jonathan Corbet <corbet@xxxxxxx> wrote:
> > > Andy Shevchenko <andy.shevchenko@xxxxxxxxx> writes:
> > > > On Tue, Feb 16, 2021 at 6:51 PM Jonathan Corbet <corbet@xxxxxxx> wrote:
> > > >>
> > > >> > Mauro, can you do some test cases in your workflow against anonymous
> > > >> > enum in ernel doc, please?
> > > >> >
> > > >> > They are broken again, please fix the script!
> > > >> >
> > > >> > drivers/pinctrl/intel/pinctrl-intel.c:204: warning: wrong kernel-doc
> > > >> > identifier on line:
> > > >> > * enum - Locking variants of the pad configuration
> > > >> >
> > > >> > Above is simply a wrong statement.
> > > >>
> > > >> The real problem, perhaps, is that there seems to be little point in
> > > >> adding kerneldoc comments for anonymous enums; where are you going to
> > > >> use that documentation?
> > > >
> > > > I had been explicitly told during review (IIRC by maintainers) to make
> > > > it such, while the initial version was exactly like you are thinking
> > > > of. So, I'm not the right person to be asked :-)
> >
> > Just for a reference [1].
> >
> > > >>  The error message could perhaps be changed to
> > > >> say that; meanwhile, perhaps this one could be fixed with an action like
> > > >> s%/**%/*% ?
> > > >
> > > > See above. I think regression comes from the kernel doc script,
> > > > earlier it was okay. That said, the author of kernel doc changes has
> > > > to submit a patch to amend the driver and maintainers will review it.
> > >
> > > kerneldoc now warns about various incorrect things that it used to just
> > > silently pass over.  There is no regression here, just a new diagnostic
> > > to point out something that was never going to work right.  Unless you
> > > have a good idea for what kerneldoc should do with a block like that?
> >
> > As it does, put description of individual fields and prepend it with a
> > common part.
> >
> > So,
> >
> > enum - Bla bla bla
> >  @FOO: ABC
> >  @BAR: DEF
> > Description
> >
> > Should go in the doc for the corresponding file like (as an example)
> >
> > Anonymous enumeration Bla bla bla
> > Description
> >
> > FOO ABC
> > BAR DEF
> >
> > (not sure about indentation, emphasizing and separators, but I think
> > you got the idea).
> >
> > > (An alternative fix, of course, would be to give the enum a name so it
> > > can actually be used for type checking.)
> >
> > That enum is not used as an enum, it provides the logically unified constants.
>
> What's the problem of giving it a name?

Because why should I do this? It's a perfect C language which has no
issues so far.

> You could call it as "intel_pinctrl_pad" or something similar.
>
> > Personally I don't see why the kernel doc can't digest this.
>
> It is not hard to add support for this special case. Just sent a
> patch.

Thanks, I will test it!

> Yet, this adds additional complexity on an script that it is
> already complex enough.

> > [1]: https://patchwork.ozlabs.org/project/linux-gpio/patch/20190808132128.13359-1-andriy.shevchenko@xxxxxxxxxxxxxxx/

-- 
With Best Regards,
Andy Shevchenko