As described on its subject, this series finally get rid of all kernel-doc warnings.
With this series applied (plus my last series fixing other warnings), building
the docs is now clean[1] against next-20200909:
$ make cleandocs >/dev/null 2>/dev/null && make htmldocs
Warning: Documentation/bpf/ringbuf.rst references a file that doesn't exist: Documentation/litmus_tests/bpf-rb/_
rm -f /devel/v4l/docs/Documentation/output/audio.h.rst /devel/v4l/docs/Documentation/output/ca.h.rst /devel/v4l/docs/Documentation/output/dmx.h.rst /devel/v4l/docs/Documentation/output/frontend.h.rst /devel/v4l/docs/Documentation/output/net.h.rst /devel/v4l/docs/Documentation/output/video.h.rst /devel/v4l/docs/Documentation/output/videodev2.h.rst /devel/v4l/docs/Documentation/output/media.h.rst /devel/v4l/docs/Documentation/output/cec.h.rst /devel/v4l/docs/Documentation/output/lirc.h.rst 2>/dev/null
Warning: Documentation/bpf/ringbuf.rst references a file that doesn't exist: Documentation/litmus_tests/bpf-rb/_
SPHINX htmldocs --> file:///devel/v4l/docs/Documentation/output
PARSE include/uapi/linux/dvb/audio.h
PARSE include/uapi/linux/dvb/ca.h
PARSE include/uapi/linux/dvb/dmx.h
PARSE include/uapi/linux/dvb/frontend.h
PARSE include/uapi/linux/dvb/net.h
PARSE include/uapi/linux/dvb/video.h
PARSE include/uapi/linux/videodev2.h
PARSE include/uapi/linux/media.h
PARSE include/uapi/linux/cec.h
PARSE include/uapi/linux/lirc.h
Running Sphinx v2.4.4
enabling CJK for LaTeX builder
building [mo]: targets for 0 po files that are out of date
building [html]: targets for 2672 source files that are out of date
updating environment: [new config] 2672 added, 0 changed, 0 removed
reading sources... [100%] x86/kernel-stacks .. xtensa/mmu
waiting for workers...
/devel/v4l/docs/Documentation/bpf/ringbuf.rst:197: WARNING: Unknown target name: "bench_ringbuf.c".
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
writing output... [100%] w1/w1-netlink .. xtensa/mmu
waiting for workers...
generating indices... genindexdone
writing additional pages... searchdone
copying images... [100%] userspace-api/media/v4l/constraints.svg
copying static files... ... done
copying extra files... done
dumping search index in English (code: en)... done
dumping object inventory... done
build succeeded, 1 warning.
The HTML pages are in Documentation/output.
At least part of those patches won't apply against docs-next, as they depend
on stuff at linux-next. So, it is preferred if they can be applied via each
sub-maintainer's tree.
I'll rebase those during the next merge window. This way, if some
patches ended being missed, they can be applied by the end of the
merge window.
Hopefully, we can make Kernel 5.10 free of documentation warnings.
[1] with the exception of two latmus warnings that seems to require a patch
that it was not merged yet.
Regards,
Mauro
Mauro Carvalho Chehab (14):
locking/refcount: document the new "oldp" pointer value
usb: docs: document altmode register/unregister functions
XArray: docs: add missing kernel-doc parameters for xas_split_alloc()
blk-mq: docs: add kernel-doc description for a new struct member
iio: docs: add description for a new function member
nl80211: docs: add a description for s1g_cap parameter
IB/srpt: docs: add a description for cq_size member
rcu/tree: docs: document bkvcache new members at struct kfree_rcu_cpu
Input: sparse-keymap: add a description for @sw
drm: amdgpu: kernel-doc: update some adev parameters
drm/amd/display: kernel-doc: document force_timing_sync
drm: kernel-doc: document drm_dp_set_subconnector_property() params
drm: kernel-doc: drm_dp_helper.h: fix a typo
gpu: docs: amdgpu.rst: get rid of wrong kernel-doc markups
Documentation/driver-api/usb/typec_bus.rst | 8 +++++++-
Documentation/gpu/amdgpu.rst | 7 -------
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 ++---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 2 ++
drivers/gpu/drm/drm_dp_helper.c | 7 ++++++-
drivers/iio/industrialio-core.c | 2 ++
drivers/infiniband/ulp/srpt/ib_srpt.h | 1 +
include/drm/drm_dp_helper.h | 2 +-
include/linux/blk-mq.h | 2 ++
include/linux/input/sparse-keymap.h | 1 +
include/linux/refcount.h | 7 +++++++
include/linux/usb/typec_altmode.h | 16 ++++++++++++++++
include/net/cfg80211.h | 1 +
kernel/rcu/tree.c | 14 ++++++--------
lib/xarray.c | 11 +++++++++--
17 files changed, 67 insertions(+), 27 deletions(-)
--
2.26.2
Add a description for it, in order to avoids this warning:
./include/linux/input/sparse-keymap.h:43: warning: Function parameter or member 'sw' not described in 'key_entry'
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
include/linux/input/sparse-keymap.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/linux/input/sparse-keymap.h b/include/linux/input/sparse-keymap.h
index d25d1452dc6e..d0dddc14ebc8 100644
--- a/include/linux/input/sparse-keymap.h
+++ b/include/linux/input/sparse-keymap.h
@@ -20,6 +20,7 @@
* private definitions.
* @code: Device-specific data identifying the button/switch
* @keycode: KEY_* code assigned to a key/button
+ * @sw: struct with code/value used by KE_SW and KE_VSW
* @sw.code: SW_* code assigned to a switch
* @sw.value: Value that should be sent in an input even when KE_SW
* switch is toggled. KE_VSW switches ignore this field and
--
2.26.2
Changeset c804af2c1d31 ("IB/srpt: use new shared CQ mechanism")
added a new member for struct srpt_rdma_ch, but didn't add the
corresponding kernel-doc markup, as repoted when doing
"make htmldocs":
./drivers/infiniband/ulp/srpt/ib_srpt.h:331: warning: Function parameter or member 'cq_size' not described in 'srpt_rdma_ch'
Add a description for it.
Fixes: c804af2c1d31 ("IB/srpt: use new shared CQ mechanism")
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/infiniband/ulp/srpt/ib_srpt.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.h b/drivers/infiniband/ulp/srpt/ib_srpt.h
index 41435a699b53..e5d6af14d073 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.h
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.h
@@ -256,6 +256,7 @@ enum rdma_ch_state {
* @rdma_cm: See below.
* @rdma_cm.cm_id: RDMA CM ID associated with the channel.
* @cq: IB completion queue for this channel.
+ * @cq_size: Size of the @cq pool.
* @zw_cqe: Zero-length write CQE.
* @rcu: RCU head.
* @kref: kref for this channel.
--
2.26.2
As reported by kernel-doc:
./drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c:1: warning: no structured comments found
./drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c:1: warning: no structured comments found
Those files only contain
/**
* DOC:
*/
markups, but they're included twice there: one to parse
such markup, and another one to parse internal functions.
In the case of amdgpu_xgmi.c, as it has just one such
markup, we can simply include the file once, and let it
parse the entire file without passing arguments to kernel-doc.
This should place everything altogether.
For amdgpu_ras.c, however, we need to remove the kernel-doc
with just internal. This should be re-introduced if this
file ever gets new non-DOC markups.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
Documentation/gpu/amdgpu.rst | 7 -------
1 file changed, 7 deletions(-)
diff --git a/Documentation/gpu/amdgpu.rst b/Documentation/gpu/amdgpu.rst
index 9656f5f5bbcf..d46bfe52b722 100644
--- a/Documentation/gpu/amdgpu.rst
+++ b/Documentation/gpu/amdgpu.rst
@@ -74,10 +74,6 @@ AMDGPU XGMI Support
===================
.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
- :doc: AMDGPU XGMI Support
-
-.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
- :internal:
AMDGPU RAS Support
==================
@@ -115,9 +111,6 @@ RAS VRAM Bad Pages sysfs Interface
.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
:doc: AMDGPU RAS sysfs gpu_vram_bad_pages Interface
-.. kernel-doc:: drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
- :internal:
-
Sample Code
-----------
Sample code for testing error injection can be found here:
--
2.26.2
The typec_bus.rst asks for documentation of those two
functions, but they don't exist:
./drivers/usb/typec/bus.c:1: warning: 'typec_altmode_unregister_driver' not found
./drivers/usb/typec/bus.c:1: warning: 'typec_altmode_register_driver' not found
Also, they're not declared on bus.c but, instead, at a header
file (typec_altmode.h).
So, add documentation for both functions at the header and
change the kernel-doc markup under typec_bus.rst to point
to the right place.
While here, also place the documentation for both structs
declared on typec_altmode.h at the rst file.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
Documentation/driver-api/usb/typec_bus.rst | 8 +++++++-
include/linux/usb/typec_altmode.h | 16 ++++++++++++++++
2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/Documentation/driver-api/usb/typec_bus.rst b/Documentation/driver-api/usb/typec_bus.rst
index 03dfa9c018b7..21c890ae17e5 100644
--- a/Documentation/driver-api/usb/typec_bus.rst
+++ b/Documentation/driver-api/usb/typec_bus.rst
@@ -91,10 +91,16 @@ their control.
Driver API
----------
+Alternate mode structs
+~~~~~~~~~~~~~~~~~~~~~~
+
+.. kernel-doc:: include/linux/usb/typec_altmode.h
+ :functions: typec_altmode_driver typec_altmode_ops
+
Alternate mode driver registering/unregistering
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-.. kernel-doc:: drivers/usb/typec/bus.c
+.. kernel-doc:: include/linux/usb/typec_altmode.h
:functions: typec_altmode_register_driver typec_altmode_unregister_driver
Alternate mode driver operations
diff --git a/include/linux/usb/typec_altmode.h b/include/linux/usb/typec_altmode.h
index a4b65eaa0f62..5e0a7b7647c3 100644
--- a/include/linux/usb/typec_altmode.h
+++ b/include/linux/usb/typec_altmode.h
@@ -152,10 +152,26 @@ struct typec_altmode_driver {
#define to_altmode_driver(d) container_of(d, struct typec_altmode_driver, \
driver)
+/**
+ * typec_altmode_register_driver - registers a USB Type-C alternate mode
+ * device driver
+ * @drv: pointer to struct typec_altmode_driver
+ *
+ * These drivers will be bind to the partner alternate mode devices. They will
+ * handle all SVID specific communication.
+ */
#define typec_altmode_register_driver(drv) \
__typec_altmode_register_driver(drv, THIS_MODULE)
int __typec_altmode_register_driver(struct typec_altmode_driver *drv,
struct module *module);
+/**
+ * typec_altmode_unregister_driver - unregisters a USB Type-C alternate mode
+ * device driver
+ * @drv: pointer to struct typec_altmode_driver
+ *
+ * These drivers will be bind to the partner alternate mode devices. They will
+ * handle all SVID specific communication.
+ */
void typec_altmode_unregister_driver(struct typec_altmode_driver *drv);
#define module_typec_altmode_driver(__typec_altmode_driver) \
--
2.26.2
Changeset df78a0c0b67d ("nl80211: S1G band and channel definitions")
added a new parameter, but didn't add the corresponding kernel-doc
markup, as repoted when doing "make htmldocs":
./include/net/cfg80211.h:471: warning: Function parameter or member 's1g_cap' not described in 'ieee80211_supported_band'
Add a documentation for it.
Fixes: df78a0c0b67d ("nl80211: S1G band and channel definitions")
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
include/net/cfg80211.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index c9bce9bba511..8f34c1489056 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -449,6 +449,7 @@ struct ieee80211_sta_s1g_cap {
* @n_bitrates: Number of bitrates in @bitrates
* @ht_cap: HT capabilities in this band
* @vht_cap: VHT capabilities in this band
+ * @s1g_cap: S1G capabilities in this band
* @edmg_cap: EDMG capabilities in this band
* @n_iftype_data: number of iftype data entries
* @iftype_data: interface type data entries. Note that the bits in
--
2.26.2
Running "make htmldocs: produce lots of warnings on those files:
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:177: warning: Excess function parameter 'man' description in 'amdgpu_vram_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:177: warning: Excess function parameter 'p_size' description in 'amdgpu_vram_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:211: warning: Excess function parameter 'man' description in 'amdgpu_vram_mgr_fini'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:177: warning: Excess function parameter 'man' description in 'amdgpu_vram_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:177: warning: Excess function parameter 'p_size' description in 'amdgpu_vram_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:211: warning: Excess function parameter 'man' description in 'amdgpu_vram_mgr_fini'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:177: warning: Excess function parameter 'man' description in 'amdgpu_vram_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:177: warning: Excess function parameter 'p_size' description in 'amdgpu_vram_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:211: warning: Excess function parameter 'man' description in 'amdgpu_vram_mgr_fini'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:177: warning: Excess function parameter 'man' description in 'amdgpu_vram_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:177: warning: Excess function parameter 'p_size' description in 'amdgpu_vram_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c:211: warning: Excess function parameter 'man' description in 'amdgpu_vram_mgr_fini'
./drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c:90: warning: Excess function parameter 'man' description in 'amdgpu_gtt_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c:90: warning: Excess function parameter 'p_size' description in 'amdgpu_gtt_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c:134: warning: Excess function parameter 'man' description in 'amdgpu_gtt_mgr_fini'
./drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c:90: warning: Excess function parameter 'man' description in 'amdgpu_gtt_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c:90: warning: Excess function parameter 'p_size' description in 'amdgpu_gtt_mgr_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c:134: warning: Excess function parameter 'man' description in 'amdgpu_gtt_mgr_fini'
./drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:675: warning: Excess function parameter 'dev' description in 'amdgpu_device_asic_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:675: warning: Excess function parameter 'dev' description in 'amdgpu_device_asic_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:675: warning: Excess function parameter 'dev' description in 'amdgpu_device_asic_init'
./drivers/gpu/drm/amd/amdgpu/amdgpu_device.c:675: warning: Excess function parameter 'dev' description in 'amdgpu_device_asic_init'
They're related to the repacement of some parameters by adev,
and due to a few renamed parameters.
Update the kernel-doc documentation accordingly.
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c | 6 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c | 5 ++---
3 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index f7307af76452..ba32b3ea9098 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -667,7 +667,7 @@ static void amdgpu_block_invalid_wreg(struct amdgpu_device *adev,
/**
* amdgpu_device_asic_init - Wrapper for atom asic_init
*
- * @dev: drm_device pointer
+ * @adev: drm_device pointer
*
* Does any asic specific work and then calls atom asic init.
*/
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
index 2b1b7c136343..62c66c8a7a69 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gtt_mgr.c
@@ -81,8 +81,8 @@ static const struct ttm_resource_manager_func amdgpu_gtt_mgr_func;
/**
* amdgpu_gtt_mgr_init - init GTT manager and DRM MM
*
- * @man: TTM memory type manager
- * @p_size: maximum size of GTT
+ * @adev: amdgpu device structure
+ * @gtt_size: maximum size of GTT
*
* Allocate and initialize the GTT manager.
*/
@@ -125,7 +125,7 @@ int amdgpu_gtt_mgr_init(struct amdgpu_device *adev, uint64_t gtt_size)
/**
* amdgpu_gtt_mgr_fini - free and destroy GTT manager
*
- * @man: TTM memory type manager
+ * @adev: amdgpu device structure
*
* Destroy and free the GTT manager, returns -EBUSY if ranges are still
* allocated inside it.
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
index b2adc2abc581..a26cf1c80d62 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vram_mgr.c
@@ -168,8 +168,7 @@ static const struct ttm_resource_manager_func amdgpu_vram_mgr_func;
/**
* amdgpu_vram_mgr_init - init VRAM manager and DRM MM
*
- * @man: TTM memory type manager
- * @p_size: maximum size of VRAM
+ * @adev: amdgpu device structure
*
* Allocate and initialize the VRAM manager.
*/
@@ -202,7 +201,7 @@ int amdgpu_vram_mgr_init(struct amdgpu_device *adev)
/**
* amdgpu_vram_mgr_fini - free and destroy VRAM manager
*
- * @man: TTM memory type manager
+ * @adev: amdgpu device structure
*
* Destroy and free the VRAM manager, returns -EBUSY if ranges are still
* allocated inside it.
--
2.26.2
Changeset e5b92773287c ("drm: report dp downstream port type as a subconnector property")
added a new function to the kAPI, but didn't add any documentation
for the parameters for drm_dp_set_subconnector_property().
Fixes: e5b92773287c ("drm: report dp downstream port type as a subconnector property")
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
drivers/gpu/drm/drm_dp_helper.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ad7c03dac467..e0ca8f19164b 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -599,7 +599,8 @@ EXPORT_SYMBOL(drm_dp_downstream_debug);
/**
* drm_dp_subconnector_type() - get DP branch device type
- *
+ * @dpcd: DisplayPort configuration data
+ * @port_cap: port capabilities
*/
enum drm_mode_subconnector
drm_dp_subconnector_type(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
@@ -650,6 +651,10 @@ EXPORT_SYMBOL(drm_dp_subconnector_type);
/**
* drm_mode_set_dp_subconnector_property - set subconnector for DP connector
+ * @connector: connector to set property on
+ * @status: connector status
+ * @dpcd: DisplayPort configuration data
+ * @port_cap: port capabilities
*
* Called by a driver on every detect event.
*/
--
2.26.2
On Thu, Sep 10, 2020 at 12:23:53PM +0200, Mauro Carvalho Chehab wrote:
> As described on its subject, this series finally get rid of all kernel-doc warnings.
>
> With this series applied (plus my last series fixing other warnings), building
> the docs is now clean[1] against next-20200909:
Thanks, this has been a truly heroic effort.
I'd suggest that we change the kernel build to always run the CHKDOC
instead of at W=1 (or rather, as the patch I just sent out demonstrates,
not at all (oops)). Otherwise you're just going to have to continue
doing this.
At some point, perhaps we can add some other warnings at W=1, like
an EXPORT_SYMBOL of a function which doesn't have kernel-doc.
Em Thu, 10 Sep 2020 19:12:08 +0100
Matthew Wilcox <[email protected]> escreveu:
> On Thu, Sep 10, 2020 at 12:23:53PM +0200, Mauro Carvalho Chehab wrote:
> > As described on its subject, this series finally get rid of all kernel-doc warnings.
> >
> > With this series applied (plus my last series fixing other warnings), building
> > the docs is now clean[1] against next-20200909:
>
> Thanks, this has been a truly heroic effort.
>
> I'd suggest that we change the kernel build to always run the CHKDOC
> instead of at W=1 (or rather, as the patch I just sent out demonstrates,
> not at all (oops)). Otherwise you're just going to have to continue
> doing this.
It sounds a good idea for me to run kernel-doc with W=1 - or even
better - with allyesconfig/allmodconfig (no matter if W=0/W=1/W=2).
> At some point, perhaps we can add some other warnings at W=1, like
> an EXPORT_SYMBOL of a function which doesn't have kernel-doc.
Makes sense, but I suspect that supporting it is not too trivial.
I mean, a script checking for EXPORT_SYMBOL* should check not
only the C file, but also the included header files, as the
kernel-doc markup can be on one of its includes.
An enhanced version of something like this:
</script>
#!/usr/bin/perl
my $file = shift or die "Need a file name";
my @files;
my @exports;
my $dir = $file;
$dir =~ s,[^\/]+$,,;
push @files, $file;
open IN, "<$file";
while (<IN>) {
push @exports, $1 if (m/^EXPORT_SYMBOL.*\(\s*(\S+)\s*\)/);
push @files, "include/$1" if (m/^\s*#\s*include\s+[\<](\S+)[\>]/);
push @files, "$dir/$1" if (m/^\s*#\s*include\s+[\"](\S+)[\"]/);
}
close IN;
my $doc;
foreach my $i (@files) {
$doc .= qx(./scripts/kernel-doc $i 2>/dev/null);
}
foreach my $e (@exports) {
print "$e doesn't have kernel-doc markups\n" if (!($doc =~ m/\b$e\b/));
}
</script>
On simple cases, the above script helps to check what's missing:
$ ./check_exports drivers/acpi/acpi_lpat.c
<nothing returned>
$ ./test drivers/media/v4l2-core/v4l2-common.c
__v4l2_find_nearest_size doesn't have kernel-doc markups
v4l2_apply_frmsize_constraints doesn't have kernel-doc markups
v4l2_fill_pixfmt_mp doesn't have kernel-doc markups
v4l2_fill_pixfmt doesn't have kernel-doc markups
Yet, it the actual script will also need to handle some special
cases:
- it should check if the Makefile used by the file has a "-I" directive.
This could be tricky, due to Makefile recursion.
- it should also check if there is a kernel-doc entry for such header.
a "git grep" could be used in this case.
- It should also handle the optional arguments of kernel-doc, like
:internal", :external", ":no-identifiers", "identifiers", as it is
possible that there is a kernel-doc entry, but this is excluded
by a kernel-doc modifier.
- It should also check if the exported symbol is a function,
in order to exclude static vars that are exported.
I suspect that there are several other border cases.
Thanks,
Mauro
Em Mon, 5 Oct 2020 12:51:11 +0200
Mauro Carvalho Chehab <[email protected]> escreveu:
> Em Thu, 10 Sep 2020 19:12:08 +0100
> Matthew Wilcox <[email protected]> escreveu:
>
> > On Thu, Sep 10, 2020 at 12:23:53PM +0200, Mauro Carvalho Chehab wrote:
> > > As described on its subject, this series finally get rid of all kernel-doc warnings.
> > >
> > > With this series applied (plus my last series fixing other warnings), building
> > > the docs is now clean[1] against next-20200909:
> >
> > Thanks, this has been a truly heroic effort.
> >
> > I'd suggest that we change the kernel build to always run the CHKDOC
> > instead of at W=1 (or rather, as the patch I just sent out demonstrates,
> > not at all (oops)). Otherwise you're just going to have to continue
> > doing this.
>
> It sounds a good idea for me to run kernel-doc with W=1 - or even
> better - with allyesconfig/allmodconfig (no matter if W=0/W=1/W=2).
>
> > At some point, perhaps we can add some other warnings at W=1, like
> > an EXPORT_SYMBOL of a function which doesn't have kernel-doc.
>
> Makes sense, but I suspect that supporting it is not too trivial.
>
> I mean, a script checking for EXPORT_SYMBOL* should check not
> only the C file, but also the included header files, as the
> kernel-doc markup can be on one of its includes.
>
> An enhanced version of something like this:
>
> </script>
> #!/usr/bin/perl
>
> my $file = shift or die "Need a file name";
>
> my @files;
> my @exports;
>
> my $dir = $file;
> $dir =~ s,[^\/]+$,,;
>
> push @files, $file;
> open IN, "<$file";
> while (<IN>) {
> push @exports, $1 if (m/^EXPORT_SYMBOL.*\(\s*(\S+)\s*\)/);
> push @files, "include/$1" if (m/^\s*#\s*include\s+[\<](\S+)[\>]/);
> push @files, "$dir/$1" if (m/^\s*#\s*include\s+[\"](\S+)[\"]/);
> }
> close IN;
>
> my $doc;
>
> foreach my $i (@files) {
> $doc .= qx(./scripts/kernel-doc $i 2>/dev/null);
> }
>
> foreach my $e (@exports) {
> print "$e doesn't have kernel-doc markups\n" if (!($doc =~ m/\b$e\b/));
> }
> </script>
>
> On simple cases, the above script helps to check what's missing:
>
> $ ./check_exports drivers/acpi/acpi_lpat.c
> <nothing returned>
> $ ./test drivers/media/v4l2-core/v4l2-common.c
> __v4l2_find_nearest_size doesn't have kernel-doc markups
> v4l2_apply_frmsize_constraints doesn't have kernel-doc markups
> v4l2_fill_pixfmt_mp doesn't have kernel-doc markups
> v4l2_fill_pixfmt doesn't have kernel-doc markups
>
> Yet, it the actual script will also need to handle some special
> cases:
>
> - it should check if the Makefile used by the file has a "-I" directive.
> This could be tricky, due to Makefile recursion.
Hmm... actually this could be obtained via command line parameters, if
the script is called with the same parameter set as the one passed to the
C compiler.
> - it should also check if there is a kernel-doc entry for such header.
> a "git grep" could be used in this case.
> - It should also handle the optional arguments of kernel-doc, like
> :internal", :external", ":no-identifiers", "identifiers", as it is
> possible that there is a kernel-doc entry, but this is excluded
> by a kernel-doc modifier.
> - It should also check if the exported symbol is a function,
> in order to exclude static vars that are exported.
>
> I suspect that there are several other border cases.
>
> Thanks,
> Mauro
Thanks,
Mauro
While not all EXPORT_SYMBOL*() symbols should be documented,
it seems useful to have a tool which would help to check what
symbols aren't documented.
This is a first step on this direction. The tool has some
limitations. Yet, it could be useful for maintainers to check
about missing documents on their subsystems.
Suggested-by: Matthew Wilcox <[email protected]>
Signed-off-by: Mauro Carvalho Chehab <[email protected]>
---
scripts/check_docs_external_symbols | 127 ++++++++++++++++++++++++++++
1 file changed, 127 insertions(+)
create mode 100755 scripts/check_docs_external_symbols
diff --git a/scripts/check_docs_external_symbols b/scripts/check_docs_external_symbols
new file mode 100755
index 000000000000..cc12562e6cd6
--- /dev/null
+++ b/scripts/check_docs_external_symbols
@@ -0,0 +1,127 @@
+#!/usr/bin/perl
+# SPDX-License-Identifier: GPL-2.0
+
+#
+# Copyright (c) 2020, Huawei Tech. Co., Ltd.
+# Author: Mauro Carvalho Chehab <[email protected]
+#
+# This script helps to check if exported functions are documented at either
+# a file, on at the included headers.
+#
+# The script is not perfect and may produce some false negatives, as
+# currently it doesn't handle Makefile "-I" directives that might be inside
+# a Kernel directory.
+#
+# So, use it with caution.
+#
+# Usage:
+# scripts/check_external docs
+# or:
+# scripts/check_external docs <files and/or directories>
+
+use warnings;
+use strict;
+use File::Find;
+
+sub check_file($) {
+ my $file = shift;
+ my (@files, @exports, @doc, @doc_refs);
+ my $content = "\n";
+
+ return 0 if (!($file =~ /\.[ch]$/));
+
+ my $dir = $file;
+ $dir =~ s,[^\/]+$,,;
+
+ open IN, $file or return 0;
+ while (<IN>) {
+ push @exports, $1 if (m/^EXPORT_SYMBOL.*\(\s*(\S+)\s*\)/);
+
+ if (m/^\s*#\s*include\s+[\<](\S+)[\>]/) {
+ if (-e "include/$1") {
+ push @files, "include/$1";
+ } else {
+ # Currently, can't check if include is elsewhere
+ return 0;
+ }
+ }
+ if (m/^\s*#\s*include\s+[\"](\S+)[\"]/) {
+ if (-e "$dir/$1") {
+ push @files, "$dir/$1";
+ } else {
+ # Currently, can't check if include is elsewhere
+ return 0;
+ }
+ }
+ $content .= $_;
+ }
+ close IN;
+
+ return 0 if ($content eq "\n");
+
+
+ push @files, $file;
+ for (my $i = 0; $i < scalar(@files); $i++) {
+ $doc_refs[$i] = 0;
+ $doc[$i] = qx(./scripts/kernel-doc --sphinx-version 3.2.1 $files[$i] 2>/dev/null);
+ }
+
+ my @missing_exports;
+ my $found = -1;
+ foreach my $e (@exports) {
+ # Check if the symbol is a function
+ if (!($content =~ (m/\n\s*(?:\w+\s+){0,}\*?\s*\b\Q$e\E\b\s*\(/))) {
+ next;
+ }
+ for (my $i = 0; $i < scalar(@files); $i++) {
+ if ($doc[$i] =~ m/\b\Q$e\E\b/) {
+ $found = $i;
+ $doc_refs[$i]++;
+ last;
+ }
+ }
+
+ push @missing_exports, $e if ($found < 0);
+ }
+
+ if (@missing_exports) {
+ print "warning: $file: missing documentation for @missing_exports\n";
+ }
+
+ for (my $i = 0; $i < scalar(@files); $i++) {
+ next if (!$doc_refs[$i]);
+ my $includes = qx(git grep -l "kernel-doc::\\s*$files[$i]" Documentation/);
+
+ printf "warning: %s: file not included at Documentation/\n", $files[$i] if ($includes eq "");
+ }
+ return 1;
+}
+
+sub parse_dir {
+ check_file $File::Find::name;
+}
+
+#
+# main
+#
+
+my $file;
+
+if (@ARGV) {
+ while (@ARGV) {
+ my $file = shift;
+
+ if (-d $file) {
+ find({wanted => \&parse_dir, no_chdir => 1}, $file);
+ } else {
+ check_file $file;
+ }
+ }
+ exit;
+} else {
+ my @files = qx(git grep -l EXPORT_SYMBOL);
+ foreach $file (@files) {
+ $file =~ s/\s+$//;
+ check_file $file;
+ }
+}
--
2.26.2
Hi Matthew/Jon,
Em Tue, 6 Oct 2020 13:53:34 +0200
Mauro Carvalho Chehab <[email protected]> escreveu:
> While not all EXPORT_SYMBOL*() symbols should be documented,
> it seems useful to have a tool which would help to check what
> symbols aren't documented.
>
> This is a first step on this direction. The tool has some
> limitations. Yet, it could be useful for maintainers to check
> about missing documents on their subsystems.
>
> Suggested-by: Matthew Wilcox <[email protected]>
> Signed-off-by: Mauro Carvalho Chehab <[email protected]>
As suggested, I improved the script for it to provide some hints
about what documentation is missing:
$ ./scripts/check_docs_external_symbols drivers/media/v4l2-core/
warning: drivers/media/v4l2-core/videobuf-dma-contig.c: missing documentation for videobuf_queue_dma_contig_init videobuf_to_dma_contig videobuf_dma_contig_free
warning: drivers/media/v4l2-core/v4l2-clk.c: missing documentation for v4l2_clk_get v4l2_clk_put v4l2_clk_enable v4l2_clk_disable v4l2_clk_get_rate v4l2_clk_set_rate v4l2_clk_register v4l2_clk_unregister __v4l2_clk_register_fixed v4l2_clk_unregister_fixed
warning: include/media/v4l2-h264.h: file not included at Documentation/
warning: drivers/media/v4l2-core/v4l2-h264.c: file not included at Documentation/
warning: drivers/media/v4l2-core/videobuf-core.c: missing documentation for videobuf_alloc_vb videobuf_waiton videobuf_iolock videobuf_queue_to_vaddr videobuf_queue_core_init videobuf_queue_is_busy videobuf_queue_cancel videobuf_next_field videobuf_mmap_free __videobuf_mmap_setup videobuf_mmap_setup videobuf_reqbufs videobuf_querybuf videobuf_qbuf videobuf_dqbuf videobuf_streamon videobuf_streamoff videobuf_read_one videobuf_read_start videobuf_read_stop videobuf_stop videobuf_read_stream videobuf_poll_stream videobuf_mmap_mapper
warning: drivers/media/v4l2-core/v4l2-mem2mem.c: file not included at Documentation/
warning: drivers/media/v4l2-core/v4l2-dev.c: missing documentation for video_device_alloc video_device_release video_device_release_empty video_devdata v4l2_prio_init v4l2_prio_change v4l2_prio_open v4l2_prio_close v4l2_prio_max v4l2_prio_check __video_register_device
warning: drivers/media/v4l2-core/v4l2-dev.c: file not included at Documentation/
It also report things that shouldn't be documented, like those:
$ ./scripts/check_docs_external_symbols drivers/media/pci/saa7134
warning: drivers/media/pci/saa7134/saa7134-video.c: missing documentation for saa7134_vb2_buffer_queue saa7134_enum_input saa7134_g_input saa7134_s_input saa7134_querycap saa7134_s_std saa7134_g_std saa7134_querystd saa7134_g_tuner saa7134_s_tuner saa7134_g_frequency saa7134_s_frequency
warning: drivers/media/pci/saa7134/saa7134-core.c: missing documentation for saa7134_stop_streaming saa7134_ts_register saa7134_ts_unregister saa7134_set_gpio saa7134_dmasound_init saa7134_dmasound_exit saa7134_pgtable_free saa7134_pgtable_build saa7134_pgtable_alloc saa7134_set_dmabits
warning: drivers/media/pci/saa7134/saa7134-ts.c: missing documentation for saa7134_ts_buffer_init saa7134_ts_buffer_prepare saa7134_ts_queue_setup saa7134_ts_start_streaming saa7134_ts_stop_streaming
On this specific case, the saa7134 driver was split into multiple
drivers, depending on the PCI sub-interfaces found on some complex
media devices. IMO, it doesn't make any sense to document such
symbols, as they aren't meant to be used outside saa7134 sub-drivers.
-
As noticed on its comments, this script is not perfect. It tries to
avoid reporting false-positives by not processing files that it
can't find all includes and by skipping non-functions.
Yet, it can take a long time for it to parse the entire Kernel
tree. So, at least on its current state, I don't think it would
be a good idea to add it to the default build.
Feel free to either take it as-is, to improve it, or to ignore ;-)
Thanks,
Mauro
On Tue, 06 Oct 2020, Mauro Carvalho Chehab <[email protected]> wrote:
> While not all EXPORT_SYMBOL*() symbols should be documented,
> it seems useful to have a tool which would help to check what
> symbols aren't documented.
>
> This is a first step on this direction. The tool has some
> limitations. Yet, it could be useful for maintainers to check
> about missing documents on their subsystems.
Seems like this should be part of checkpatch.pl somehow.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On Wed, 2020-10-07 at 11:23 +0300, Jani Nikula wrote:
> On Tue, 06 Oct 2020, Mauro Carvalho Chehab <[email protected]> wrote:
> > While not all EXPORT_SYMBOL*() symbols should be documented,
> > it seems useful to have a tool which would help to check what
> > symbols aren't documented.
> >
> > This is a first step on this direction. The tool has some
> > limitations. Yet, it could be useful for maintainers to check
> > about missing documents on their subsystems.
>
> Seems like this should be part of checkpatch.pl somehow.
I don't see how.
Em Wed, 07 Oct 2020 01:32:31 -0700
Joe Perches <[email protected]> escreveu:
> On Wed, 2020-10-07 at 11:23 +0300, Jani Nikula wrote:
> > On Tue, 06 Oct 2020, Mauro Carvalho Chehab <[email protected]> wrote:
> > > While not all EXPORT_SYMBOL*() symbols should be documented,
> > > it seems useful to have a tool which would help to check what
> > > symbols aren't documented.
> > >
> > > This is a first step on this direction. The tool has some
> > > limitations. Yet, it could be useful for maintainers to check
> > > about missing documents on their subsystems.
> >
> > Seems like this should be part of checkpatch.pl somehow.
>
> I don't see how.
Just sent a third version of the script. It should now be able to check
if the RST files have the exported symbols as well:
<snip>
$ scripts/check_docs_external_symbols lib/debugobjects.c
lib/debugobjects.c: export symbol debug_object_active_state not documented at: Documentation/core-api/debug-objects.rst
</snip>
It also checks if an exported symbol has duplicated entries at
kernel-doc as well.
The check logic for it is somewhat complex. so, I won't doubt that
there are some other hidden bugs.
Also, as right now, the script is slow, if one wants to run it
for the entire Kernel.
Thanks,
Mauro