| Message ID | 20190623164024.9836-3-andrealmeid@collabora.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/5] media: vimc: stream: remove obsolete function doc | expand |
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,
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
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é
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,
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(-)