Re: [PATCH 03/13] cpufreq: cpufreq_governor: Demote store_sampling_rate() header to standard comment block

[Lee Jones <lee.jones@linaro.org>]

On Wed, 15 Jul 2020, Viresh Kumar wrote:

> On 15-07-20, 07:45, Lee Jones wrote:
> > On Wed, 15 Jul 2020, Viresh Kumar wrote:
> > > On 14-07-20, 15:50, Lee Jones wrote:
> > > > diff --git a/drivers/cpufreq/cpufreq_governor.c b/drivers/cpufreq/cpufreq_governor.c
> > > > index f99ae45efaea7..63f7c219062b9 100644
> > > > --- a/drivers/cpufreq/cpufreq_governor.c
> > > > +++ b/drivers/cpufreq/cpufreq_governor.c
> > > > @@ -26,7 +26,7 @@ static DEFINE_PER_CPU(struct cpu_dbs_info, cpu_dbs);
> > > >  static DEFINE_MUTEX(gov_dbs_data_mutex);
> > > >  
> > > >  /* Common sysfs tunables */
> > > > -/**
> > > > +/*
> > > 
> > > This is an important routine with good documentation details already
> > > there, though internal to governors and so I would rather keep it.
> > 
> > It maybe documented, but it isn't kerneldoc, for 2 reasons; a) it
> > doesn't meet the standards required qualify as kerneldoc i.e. it's
> > missing descriptions for each of the function parameters, which is why
> > the kerneldoc checker is complaining about it
> 
> Right, so this is a mistake and not intentional probably.
> 
> > and b) it is not
> > referenced by any *.rst file:
> > 
> >  git grep kernel-doc::.*cpufreq_governor.c
> >  /* no results */
> 
> I believed (and it may be wrong) that there are two categories of
> routines/structures which can be put in kernel documentation, the
> exported ones and the internal ones which are important and are very
> useful in understanding the algorithms/logic in the drivers.
> 
> I did try to go and look into Documentation/doc-guide/ but couldn't
> find any details on this.
> 
> You said that it needs to be referenced from some *.rst file, but why
> is that necessary ? What if people don't add any documentation in
> Documentation/ for their framework or driver but still want stuff to
> appear in kernel-doc as they can keep the documentation in comments
> more up to date.

I'm not sure what you mean.  Kerneldoc headers are designed to be
extracted and converted into mediums which are easy to read/browse.
For example, see the online documentation for 'debug_object_init':

 https://www.kernel.org/doc/html/latest/core-api/debug-objects.html?highlight=debug_object_init#c.debug_object_init

They are generally meant to be referenced/consumed.  There is even a
script provided inside the kernel to find offending instances where
kerneldoc headers are provided, but not *yet* referenced:

 `scripts/find-unused-docs.sh`

HINT: There are many.

There *could* be and argument to use kerneldoc *just* so you can use
the kerneldoc checker `scripts/kernel-doc` (which is invoked by W=1
builds), in order to ensure the parameter descriptions are kept in
check.

However, in this case, there are no descriptions provided.  So, in
reference to my previous question, what are your reasons for wanting
to keep kerneldoc here?

-- 
Lee Jones [李琼斯]
Senior Technical Lead - Developer Services
Linaro.org │ Open source software for Arm SoCs
Follow Linaro: Facebook | Twitter | Blog