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@xxxxxxxxxxxxxxxx>
> >
> > 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
