[PATCH 1/5] media: vimc: stream: remove obsolete function doc

[PATCH 1/5] media: vimc: stream: remove obsolete function doc

[André Almeida]

As a more complete version of vimc_streamer_s_streamer comment was added
at "media: vimc: stream: add missing function documentation" commit in
.c file, remove the old documentation from .h file.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/vimc-streamer.h | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-streamer.h b/drivers/media/platform/vimc/vimc-streamer.h
index 2b3667408794..28c3706e3c21 100644
--- a/drivers/media/platform/vimc/vimc-streamer.h
+++ b/drivers/media/platform/vimc/vimc-streamer.h
@@ -43,14 +43,6 @@ struct vimc_stream {
 	u32 producer_pixfmt;
 };
 
-/**
- * vimc_streamer_s_streamer - start/stop the stream
- *
- * @stream:	the pointer to the stream to start or stop
- * @ved:	The last entity of the streamer pipeline
- * @enable:	any non-zero number start the stream, zero stop
- *
- */
 int vimc_streamer_s_stream(struct vimc_stream *stream,
 			   struct vimc_ent_device *ved,
 			   int enable);
-- 
2.22.0

[PATCH 2/5] media: vimc: stream: fix style of argument description

[André Almeida]

As in "Function parameters" at doc-guide/kernel-doc.rst, "the
continuation of the description should start at the same column as the
previous line". Make the @producer_pixfmt comply with that.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/vimc-streamer.h | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-streamer.h b/drivers/media/platform/vimc/vimc-streamer.h
index 28c3706e3c21..d744a787e0e7 100644
--- a/drivers/media/platform/vimc/vimc-streamer.h
+++ b/drivers/media/platform/vimc/vimc-streamer.h
@@ -26,10 +26,12 @@
  * @pipe_size:		size of @ved_pipeline
  * @kthread:		thread that generates the frames of the stream.
  * @producer_pixfmt:	the pixel format requested from the pipeline. This must
- * be set just before calling vimc_streamer_s_stream(ent, 1). This value is
- * propagated up to the source of the base image (usually a sensor node) and
- * can be modified by entities during s_stream callback to request a different
- * format from rest of the pipeline.
+ *			be set just before calling
+ *			vimc_streamer_s_stream(ent, 1). This value is propagated
+ *			up to the source of the base image (usually a sensor
+ *			node) and can be modified by entities during s_stream
+ *			callback to request a differentformat from rest of
+ *			the pipeline.
  *
  * When the user call stream_on in a video device, struct vimc_stream is
  * used to keep track of all entities and subdevices that generates and
-- 
2.22.0

[PATCH 3/5] media: vimc: stream: format comments as kernel-doc

[André Almeida]

Format the current existing comments as kernel-doc comments, to be
reused at kernel documention. Add opening marks (/**) and return values.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 drivers/media/platform/vimc/vimc-streamer.c | 38 +++++++++++++--------
 1 file changed, 24 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
index 3b3f36357a0e..9970650b0f26 100644
--- a/drivers/media/platform/vimc/vimc-streamer.c
+++ b/drivers/media/platform/vimc/vimc-streamer.c
@@ -20,6 +20,8 @@
  *
  * Helper function that returns the media entity containing the source pad
  * linked with the first sink pad from the given media entity pad list.
+ *
+ * Return: The source pad or NULL, if it wasn't found.
  */
 static struct media_entity *vimc_get_source_entity(struct media_entity *ent)
 {
@@ -35,7 +37,7 @@ static struct media_entity *vimc_get_source_entity(struct media_entity *ent)
 	return NULL;
 }
 
-/*
+/**
  * vimc_streamer_pipeline_terminate - Disable stream in all ved in stream
  *
  * @stream: the pointer to the stream structure with the pipeline to be
@@ -63,15 +65,18 @@ static void vimc_streamer_pipeline_terminate(struct vimc_stream *stream)
 	}
 }
 
-/*
- * vimc_streamer_pipeline_init - initializes the stream structure
+/**
+ * vimc_streamer_pipeline_init - Initializes the stream structure
  *
  * @stream: the pointer to the stream structure to be initialized
  * @ved:    the pointer to the vimc entity initializing the stream
  *
  * Initializes the stream structure. Walks through the entity graph to
  * construct the pipeline used later on the streamer thread.
- * Calls s_stream to enable stream in all entities of the pipeline.
+ * Calls ``vimc_streamer_s_stream`` to enable stream in all entities of
+ * the pipeline.
+ *
+ * Return: 0 if success, error code otherwise.
  */
 static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 				       struct vimc_ent_device *ved)
@@ -122,13 +127,17 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
 	return -EINVAL;
 }
 
-/*
- * vimc_streamer_thread - process frames through the pipeline
+/**
+ * vimc_streamer_thread - Process frames through the pipeline
  *
  * @data:	vimc_stream struct of the current stream
  *
  * From the source to the sink, gets a frame from each subdevice and send to
  * the next one of the pipeline at a fixed framerate.
+ *
+ * Return:
+ * Always zero (created as ``int`` instead of ``void`` to comply with
+ * kthread API).
  */
 static int vimc_streamer_thread(void *data)
 {
@@ -157,19 +166,20 @@ static int vimc_streamer_thread(void *data)
 	return 0;
 }
 
-/*
- * vimc_streamer_s_stream - start/stop the streaming on the media pipeline
+/**
+ * vimc_streamer_s_stream - Start/stop the streaming on the media pipeline
  *
  * @stream:	the pointer to the stream structure of the current stream
  * @ved:	pointer to the vimc entity of the entity of the stream
  * @enable:	flag to determine if stream should start/stop
  *
- * When starting, check if there is no stream->kthread allocated. This should
- * indicate that a stream is already running. Then, it initializes
- * the pipeline, creates and runs a kthread to consume buffers through the
- * pipeline.
- * When stopping, analogously check if there is a stream running, stop
- * the thread and terminates the pipeline.
+ * When starting, check if there is no ``stream->kthread`` allocated. This
+ * should indicate that a stream is already running. Then, it initializes the
+ * pipeline, creates and runs a kthread to consume buffers through the pipeline.
+ * When stopping, analogously check if there is a stream running, stop the
+ * thread and terminates the pipeline.
+ *
+ * Return: 0 if success, error code otherwise.
  */
 int vimc_streamer_s_stream(struct vimc_stream *stream,
 			   struct vimc_ent_device *ved,
-- 
2.22.0

[PATCH 4/5] media: vimc.rst: Add a proper alt attribute to vimc.dot

[André Almeida]

According to W3C, "the content of the alt attribute is: use text that
fulfills the same function as the image". While it's hard to describe
the whole content of this image, replace the actual alt to something
more useful to people with slow connection or that uses screen readers.

Signed-off-by: André Almeida <andrealmeid@collabora.com>
---
 Documentation/media/v4l-drivers/vimc.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
index 4628b12d417f..bece85867424 100644
--- a/Documentation/media/v4l-drivers/vimc.rst
+++ b/Documentation/media/v4l-drivers/vimc.rst
@@ -15,7 +15,7 @@ recompile the driver to achieve your own topology. This is the default topology:
 .. _vimc_topology_graph:
 
 .. kernel-figure:: vimc.dot
-    :alt:   vimc.dot
+    :alt:   Diagram of the default media pipeline topology
     :align: center
 
     Media pipeline graph on vimc
-- 
2.22.0

[PATCH 5/5] media: vimc.rst: add vimc-streamer source documentation

[André Almeida]

Since vimc-streamer.{c, h} are fully documented and conforming with the
kernel-doc syntax, add those files to vimc.rst

Signed-off-by: André Almeida <andrealmeid@collabora.com>
Suggested-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 Documentation/media/v4l-drivers/vimc.rst | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/Documentation/media/v4l-drivers/vimc.rst b/Documentation/media/v4l-drivers/vimc.rst
index bece85867424..406417680db5 100644
--- a/Documentation/media/v4l-drivers/vimc.rst
+++ b/Documentation/media/v4l-drivers/vimc.rst
@@ -96,3 +96,14 @@ those arguments to each subdevice, not to the vimc module. For example::
         Window size to calculate the mean. Note: the window size needs to be an
         odd number, as the main pixel stays in the center of the window,
         otherwise the next odd number is considered (the default value is 3).
+
+Source code documentation
+-------------------------
+
+vimc-streamer
+~~~~~~~~~~~~~
+
+.. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.h
+   :internal:
+
+.. kernel-doc:: drivers/media/platform/vimc/vimc-streamer.c
-- 
2.22.0

Re: [PATCH 3/5] media: vimc: stream: format comments as kernel-doc

[André Almeida]

On 6/23/19 1:40 PM, André Almeida wrote:
> Format the current existing comments as kernel-doc comments, to be
> reused at kernel documention. Add opening marks (/**) and return values.
>
> Signed-off-by: André Almeida <andrealmeid@collabora.com>
> ---
>  drivers/media/platform/vimc/vimc-streamer.c | 38 +++++++++++++--------
>  1 file changed, 24 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/vimc/vimc-streamer.c b/drivers/media/platform/vimc/vimc-streamer.c
> index 3b3f36357a0e..9970650b0f26 100644
> --- a/drivers/media/platform/vimc/vimc-streamer.c
> +++ b/drivers/media/platform/vimc/vimc-streamer.c
> @@ -20,6 +20,8 @@
>   *
>   * Helper function that returns the media entity containing the source pad
>   * linked with the first sink pad from the given media entity pad list.
> + *
> + * Return: The source pad or NULL, if it wasn't found.
>   */
>  static struct media_entity *vimc_get_source_entity(struct media_entity *ent)
>  {
> @@ -35,7 +37,7 @@ static struct media_entity *vimc_get_source_entity(struct media_entity *ent)
>  	return NULL;
>  }
>  
> -/*
> +/**
>   * vimc_streamer_pipeline_terminate - Disable stream in all ved in stream
>   *
>   * @stream: the pointer to the stream structure with the pipeline to be
> @@ -63,15 +65,18 @@ static void vimc_streamer_pipeline_terminate(struct vimc_stream *stream)
>  	}
>  }
>  
> -/*
> - * vimc_streamer_pipeline_init - initializes the stream structure
> +/**
> + * vimc_streamer_pipeline_init - Initializes the stream structure
>   *
>   * @stream: the pointer to the stream structure to be initialized
>   * @ved:    the pointer to the vimc entity initializing the stream
>   *
>   * Initializes the stream structure. Walks through the entity graph to
>   * construct the pipeline used later on the streamer thread.
> - * Calls s_stream to enable stream in all entities of the pipeline.
> + * Calls ``vimc_streamer_s_stream`` to enable stream in all entities of
``vimc_streamer_s_stream`` could also been written as
:c:func:`vimc_streamer_s_stream`. In this latest setup, the
Documentation output would display a nice hyperlink to the documentation
of  vimc_streamer_s_stream function. Is this a good improvement or it
will be too verbose?
> + * the pipeline.
> + *
> + * Return: 0 if success, error code otherwise.
>   */
>  static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  				       struct vimc_ent_device *ved)
> @@ -122,13 +127,17 @@ static int vimc_streamer_pipeline_init(struct vimc_stream *stream,
>  	return -EINVAL;
>  }
>  
> -/*
> - * vimc_streamer_thread - process frames through the pipeline
> +/**
> + * vimc_streamer_thread - Process frames through the pipeline
>   *
>   * @data:	vimc_stream struct of the current stream
>   *
>   * From the source to the sink, gets a frame from each subdevice and send to
>   * the next one of the pipeline at a fixed framerate.
> + *
> + * Return:
> + * Always zero (created as ``int`` instead of ``void`` to comply with
> + * kthread API).
>   */
>  static int vimc_streamer_thread(void *data)
>  {
> @@ -157,19 +166,20 @@ static int vimc_streamer_thread(void *data)
>  	return 0;
>  }
>  
> -/*
> - * vimc_streamer_s_stream - start/stop the streaming on the media pipeline
> +/**
> + * vimc_streamer_s_stream - Start/stop the streaming on the media pipeline
>   *
>   * @stream:	the pointer to the stream structure of the current stream
>   * @ved:	pointer to the vimc entity of the entity of the stream
>   * @enable:	flag to determine if stream should start/stop
>   *
> - * When starting, check if there is no stream->kthread allocated. This should
> - * indicate that a stream is already running. Then, it initializes
> - * the pipeline, creates and runs a kthread to consume buffers through the
> - * pipeline.
> - * When stopping, analogously check if there is a stream running, stop
> - * the thread and terminates the pipeline.
> + * When starting, check if there is no ``stream->kthread`` allocated. This
> + * should indicate that a stream is already running. Then, it initializes the
> + * pipeline, creates and runs a kthread to consume buffers through the pipeline.
> + * When stopping, analogously check if there is a stream running, stop the
> + * thread and terminates the pipeline.
> + *
> + * Return: 0 if success, error code otherwise.
>   */
>  int vimc_streamer_s_stream(struct vimc_stream *stream,
>  			   struct vimc_ent_device *ved,

Re: [PATCH 3/5] media: vimc: stream: format comments as kernel-doc

[Mauro Carvalho Chehab]

Em Sun, 23 Jun 2019 18:27:22 -0300
André Almeida <andrealmeid@collabora.com> escreveu:

> On 6/23/19 1:40 PM, André Almeida wrote:

> > - * Calls s_stream to enable stream in all entities of the pipeline.
> > + * Calls ``vimc_streamer_s_stream`` to enable stream in all entities of  
> ``vimc_streamer_s_stream`` could also been written as
> :c:func:`vimc_streamer_s_stream`. In this latest setup, the
> Documentation output would display a nice hyperlink to the documentation
> of  vimc_streamer_s_stream function. Is this a good improvement or it
> will be too verbose?

The best would be to use: vimc_streamer_s_stream(). Kernel-doc already
handles it (don't remember if it uses :c:func:, but I guess it does),
and this is the recommended way.

Anyway, there's a patch under discussion right now at Linux docs ML that 
will auto-replace these to :c:func`` automatically, not only on kernel-doc
tags, but also within the .rst files. It should be able to recognize
existing :c:func: tags, so no harm done if it is there somewhere.

Thanks,
Mauro

Re: [PATCH 3/5] media: vimc: stream: format comments as kernel-doc

[André Almeida]

On 6/24/19 6:40 AM, Mauro Carvalho Chehab wrote:
> Em Sun, 23 Jun 2019 18:27:22 -0300
> André Almeida <andrealmeid@collabora.com> escreveu:
>
>> On 6/23/19 1:40 PM, André Almeida wrote:
>>> - * Calls s_stream to enable stream in all entities of the pipeline.
>>> + * Calls ``vimc_streamer_s_stream`` to enable stream in all entities of  
>> ``vimc_streamer_s_stream`` could also been written as
>> :c:func:`vimc_streamer_s_stream`. In this latest setup, the
>> Documentation output would display a nice hyperlink to the documentation
>> of  vimc_streamer_s_stream function. Is this a good improvement or it
>> will be too verbose?
> The best would be to use: vimc_streamer_s_stream(). Kernel-doc already
> handles it (don't remember if it uses :c:func:, but I guess it does),
> and this is the recommended way.
Just tested here, and worked fine. I'll send a v2 using this style.
>
> Anyway, there's a patch under discussion right now at Linux docs ML that 
> will auto-replace these to :c:func`` automatically, not only on kernel-doc
> tags, but also within the .rst files. It should be able to recognize
> existing :c:func: tags, so no harm done if it is there somewhere.
>
> Thanks,
> Mauro

Thanks,
    André