| Message ID | 20210520190105.3772683-10-lee.jones@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | Rid W=1 warnings from I2C | expand |
On Thu, May 20, 2021 at 08:00:58PM +0100, Lee Jones wrote: > Fixes the following W=1 kernel build warning(s): > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing0' not described in 'mxs_i2c_dev' > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing1' not described in 'mxs_i2c_dev' > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing2' not described in 'mxs_i2c_dev' > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'dmach' not described in 'mxs_i2c_dev' > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'pio_data' not described in 'mxs_i2c_dev' > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'addr_data' not described in 'mxs_i2c_dev' > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'sg_io' not described in 'mxs_i2c_dev' > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'dma_read' not described in 'mxs_i2c_dev' Disabling kernel-doc works around, does not really fix, or?
On Thu, 03 Jun 2021, Wolfram Sang wrote: > On Thu, May 20, 2021 at 08:00:58PM +0100, Lee Jones wrote: > > Fixes the following W=1 kernel build warning(s): > > > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing0' not described in 'mxs_i2c_dev' > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing1' not described in 'mxs_i2c_dev' > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing2' not described in 'mxs_i2c_dev' > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'dmach' not described in 'mxs_i2c_dev' > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'pio_data' not described in 'mxs_i2c_dev' > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'addr_data' not described in 'mxs_i2c_dev' > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'sg_io' not described in 'mxs_i2c_dev' > > drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'dma_read' not described in 'mxs_i2c_dev' > > Disabling kernel-doc works around, does not really fix, or? You're right. With cases of genuine oversight or a bit of doc-rot, I tend to help where I can. However issues such as these where documentation has been either severely neglected or half-arsed require either the author or another knowledgeable person so provide fix-ups in the way of additional quality descriptions. IMHO, we wouldn't want to foster the impression that it's okay to provide a non-determined effort, safe in the knowledge that someone will come along later and finish the job for them at a later date. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
> IMHO, we wouldn't want to foster the impression that it's okay to > provide a non-determined effort, safe in the knowledge that someone > will come along later and finish the job for them at a later date. Right. The first lesson from that is that maintainers should require documentation of the fields when they get added. This was my oversight because it was back then not reported by checkers, probably. I am sorry. It annoys me, too. If I notice that someone updates a driver which doc-errors, then I ask if that could be fixed by this person, too. It usually works. Not for drivers without attention, of course. But this is why I don't mind doc-errors to stay. If this is considered problematic, then I'd suggest to remove the kernel doc headers like you did, but add a comment like: * FIXME: add missing fields and reenable kernel-doc To make sure, it is obvious even by glimpsing through the code that there is work needed. Can we agree on that?
On Fri, 04 Jun 2021, Wolfram Sang wrote: > > > IMHO, we wouldn't want to foster the impression that it's okay to > > provide a non-determined effort, safe in the knowledge that someone > > will come along later and finish the job for them at a later date. > > Right. > > The first lesson from that is that maintainers should require > documentation of the fields when they get added. This was my oversight > because it was back then not reported by checkers, probably. I am sorry. > It annoys me, too. Sure. When I started this work, there were 18k+ W=1 warnings in the kernel. Now there are more like 3k. I don't think anyone is to blame per say, it's just something that people haven't particularly cared about up until this point. One of my main aims is to clean-up W=1s to the point where enabling them would become normal practice, rather than the situation we're in presently where enabling them just dominates the build-log, making them more trouble than they're worth. > If I notice that someone updates a driver which doc-errors, then I ask > if that could be fixed by this person, too. It usually works. Not for > drivers without attention, of course. But this is why I don't mind > doc-errors to stay. I'd rather they didn't say. This would void the main aim of this effort. > If this is considered problematic, then I'd suggest to remove the kernel > doc headers like you did, but add a comment like: > > * FIXME: add missing fields and reenable kernel-doc > > To make sure, it is obvious even by glimpsing through the code that > there is work needed. > > Can we agree on that? It's the first time this has been requested, but it's your train-set and I will do whatever you ask. -- Lee Jones [李琼斯] Senior Technical Lead - Developer Services Linaro.org │ Open source software for Arm SoCs Follow Linaro: Facebook | Twitter | Blog
diff --git a/drivers/i2c/busses/i2c-mxs.c b/drivers/i2c/busses/i2c-mxs.c index f97243f022311..bcfe48e17072c 100644 --- a/drivers/i2c/busses/i2c-mxs.c +++ b/drivers/i2c/busses/i2c-mxs.c @@ -100,7 +100,7 @@ enum mxs_i2c_devtype { MXS_I2C_V2, }; -/** +/* * struct mxs_i2c_dev - per device, private MXS-I2C data * * @dev: driver model device node
Fixes the following W=1 kernel build warning(s): drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing0' not described in 'mxs_i2c_dev' drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing1' not described in 'mxs_i2c_dev' drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'timing2' not described in 'mxs_i2c_dev' drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'dmach' not described in 'mxs_i2c_dev' drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'pio_data' not described in 'mxs_i2c_dev' drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'addr_data' not described in 'mxs_i2c_dev' drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'sg_io' not described in 'mxs_i2c_dev' drivers/i2c/busses/i2c-mxs.c:131: warning: Function parameter or member 'dma_read' not described in 'mxs_i2c_dev' Cc: Shawn Guo <shawnguo@kernel.org> Cc: Sascha Hauer <s.hauer@pengutronix.de> Cc: Pengutronix Kernel Team <kernel@pengutronix.de> Cc: Fabio Estevam <festevam@gmail.com> Cc: NXP Linux Team <linux-imx@nxp.com> Cc: Wolfram Sang <wsa@kernel.org> Cc: Marek Vasut <marex@denx.de> Cc: linux-i2c@vger.kernel.org Cc: linux-arm-kernel@lists.infradead.org Signed-off-by: Lee Jones <lee.jones@linaro.org> --- drivers/i2c/busses/i2c-mxs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) -- 2.31.1