Re: [PATCH] [media] vsp1: fix CodingStyle violations on multi-line comments

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Mauro,

On Monday 19 Sep 2016 16:10:31 Mauro Carvalho Chehab wrote:
> Em Mon, 19 Sep 2016 21:35:36 +0300 Laurent Pinchart escreveu:
> > On Monday 19 Sep 2016 15:26:19 Mauro Carvalho Chehab wrote:
> >> Several multi-line comments added at the vsp1 patch series
> >> violate the Kernel CodingStyle. Fix them.
> >> 
> >> Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com>
> > 
> > I prefer the current style but that seems to be a hopeless battle :-) I
> > have a small comment, please see below.
> > 
> >> ---
> >> 
> >>  drivers/media/platform/vsp1/vsp1_bru.c    |  3 ++-
> >>  drivers/media/platform/vsp1/vsp1_clu.c    |  3 ++-
> >>  drivers/media/platform/vsp1/vsp1_dl.c     | 21 ++++++++++++++-------
> >>  drivers/media/platform/vsp1/vsp1_drm.c    |  3 ++-
> >>  drivers/media/platform/vsp1/vsp1_entity.h |  2 +-
> >>  drivers/media/platform/vsp1/vsp1_pipe.c   |  2 +-
> >>  drivers/media/platform/vsp1/vsp1_rpf.c    |  9 ++++++---
> >>  drivers/media/platform/vsp1/vsp1_rwpf.c   |  6 ++++--
> >>  drivers/media/platform/vsp1/vsp1_video.c  | 20 +++++++++++++-------
> >>  drivers/media/platform/vsp1/vsp1_wpf.c    |  9 ++++++---
> >>  10 files changed, 51 insertions(+), 27 deletions(-)

[snip]

> >> diff --git a/drivers/media/platform/vsp1/vsp1_entity.h
> >> b/drivers/media/platform/vsp1/vsp1_entity.h index
> >> 90a4d95c0a50..901146f807b9 100644
> >> --- a/drivers/media/platform/vsp1/vsp1_entity.h
> >> +++ b/drivers/media/platform/vsp1/vsp1_entity.h
> >> @@ -35,7 +35,7 @@ enum vsp1_entity_type {
> >>  	VSP1_ENTITY_WPF,
> >>  };
> >> 
> >> -/*
> >> +/**
> > 
> > Quoting another mail I've sent:
> > 
> > I don't think those comments should become part of the kernel
> > documentation. They're really about driver internals, and meant for the
> > driver developers. In particular only a subset of the driver is
> > documented that way, when I've considered that the code or structures
> > were complex enough to need proper documentation. A generated doc would
> > then be quite incomplete and not very useful, the comments are meant to
> > be read while working on the code.
>
> Just doing the above won't make it part of the Kernel documentation.
> 
> It will only be part of it if you explicitly include the file with
> the ".. kernel-doc::" directive.
> 
> Even if you don't add it at the Kernel documentation, I strongly
> suggest to use the kernel-doc tags and format, due to two reasons:
> 
> 1) If you later want to add a book, there's no need to touch at the
> function/struct documentation. Everything will there already;
> 
> 2) Markus Raiser is writing validation tool for those tags:
> 	install: https://return42.github.io/linuxdoc/install.html
> 	lint:    https://return42.github.io/linuxdoc/cmd-line.html#kernel-> 	lintdoc
> 
> By using his tool, you would be able to check if a patch is keeping
> the documentation documented, as you modify it.

Ah, I wasn't aware of that validation tool. That's a very good point. Given 
that the documentation will not be generated by switching to /** only I agree 
with you that using that tag is a good idea.

Thanks for the patch again.

> Btw, on several places inside the vsp1 documentation, you're using the
> "/**" tag already for other function/struct descriptions.

-- 
Regards,

Laurent Pinchart