linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] media: vsp1_dl: add a description for cmdpool field
@ 2018-08-07 10:18 Mauro Carvalho Chehab
  2018-08-07 10:36 ` Kieran Bingham
  0 siblings, 1 reply; 6+ messages in thread
From: Mauro Carvalho Chehab @ 2018-08-07 10:18 UTC (permalink / raw)
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
	Mauro Carvalho Chehab, Kieran Bingham, Laurent Pinchart,
	linux-renesas-soc

Gets rid of this build warning:
	drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or member 'cmdpool' not described in 'vsp1_dl_manager'

Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command pools")
Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
---
 drivers/media/platform/vsp1/vsp1_dl.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
index 9255b5ee2cb8..af60d95ec4f8 100644
--- a/drivers/media/platform/vsp1/vsp1_dl.c
+++ b/drivers/media/platform/vsp1/vsp1_dl.c
@@ -211,6 +211,7 @@ struct vsp1_dl_list {
  * @queued: list queued to the hardware (written to the DL registers)
  * @pending: list waiting to be queued to the hardware
  * @pool: body pool for the display list bodies
+ * @cmdpool: Display List commands pool
  * @autofld_cmds: command pool to support auto-fld interlaced mode
  */
 struct vsp1_dl_manager {
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] media: vsp1_dl: add a description for cmdpool field
  2018-08-07 10:18 [PATCH] media: vsp1_dl: add a description for cmdpool field Mauro Carvalho Chehab
@ 2018-08-07 10:36 ` Kieran Bingham
  2018-08-07 16:10   ` Kieran Bingham
  2018-08-07 16:17   ` Laurent Pinchart
  0 siblings, 2 replies; 6+ messages in thread
From: Kieran Bingham @ 2018-08-07 10:36 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Laurent Pinchart,
	linux-renesas-soc

Hi Mauro,

Thank you for the patch,

On 07/08/18 11:18, Mauro Carvalho Chehab wrote:
> Gets rid of this build warning:
> 	drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or member 'cmdpool' not described in 'vsp1_dl_manager'
> 
> Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command pools")
> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> ---
>  drivers/media/platform/vsp1/vsp1_dl.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
> index 9255b5ee2cb8..af60d95ec4f8 100644
> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> @@ -211,6 +211,7 @@ struct vsp1_dl_list {
>   * @queued: list queued to the hardware (written to the DL registers)
>   * @pending: list waiting to be queued to the hardware
>   * @pool: body pool for the display list bodies
> + * @cmdpool: Display List commands pool

Unfortunately this isn't quite right...


>   * @autofld_cmds: command pool to support auto-fld interlaced mode

This ^ was the original documentation line, but it got missed in a
rename. Sorry about that.

The pool is now more 'generic' so the line probably should mention the
auto-fld directly, so your line is worded appropriately enough, We
probably just# need to remove the autofld_cmds line.


With that line removed:

Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>

--
Kieran



>   */
>  struct vsp1_dl_manager {
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] media: vsp1_dl: add a description for cmdpool field
  2018-08-07 10:36 ` Kieran Bingham
@ 2018-08-07 16:10   ` Kieran Bingham
  2018-08-07 16:17   ` Laurent Pinchart
  1 sibling, 0 replies; 6+ messages in thread
From: Kieran Bingham @ 2018-08-07 16:10 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Linux Media Mailing List, Mauro Carvalho Chehab, Laurent Pinchart,
	linux-renesas-soc

On 07/08/18 11:36, Kieran Bingham wrote:
> Hi Mauro,
> 
> Thank you for the patch,
> 
> On 07/08/18 11:18, Mauro Carvalho Chehab wrote:
>> Gets rid of this build warning:
>> 	drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or member 'cmdpool' not described in 'vsp1_dl_manager'
>>
>> Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command pools")
>> Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
>> ---
>>  drivers/media/platform/vsp1/vsp1_dl.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c b/drivers/media/platform/vsp1/vsp1_dl.c
>> index 9255b5ee2cb8..af60d95ec4f8 100644
>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>> @@ -211,6 +211,7 @@ struct vsp1_dl_list {
>>   * @queued: list queued to the hardware (written to the DL registers)
>>   * @pending: list waiting to be queued to the hardware
>>   * @pool: body pool for the display list bodies
>> + * @cmdpool: Display List commands pool
> 
> Unfortunately this isn't quite right...
> 
> 
>>   * @autofld_cmds: command pool to support auto-fld interlaced mode
> 
> This ^ was the original documentation line, but it got missed in a
> rename. Sorry about that.
> 
> The pool is now more 'generic' so the line probably should mention the

Ahem, clearly I meant "shouldn't" mention.

> auto-fld directly, so your line is worded appropriately enough, We
> probably just# need to remove the autofld_cmds line.
> 
> 
> With that line removed:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> --
> Kieran
> 
> 
> 
>>   */
>>  struct vsp1_dl_manager {
>>
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] media: vsp1_dl: add a description for cmdpool field
  2018-08-07 10:36 ` Kieran Bingham
  2018-08-07 16:10   ` Kieran Bingham
@ 2018-08-07 16:17   ` Laurent Pinchart
  2018-08-07 16:19     ` Kieran Bingham
  1 sibling, 1 reply; 6+ messages in thread
From: Laurent Pinchart @ 2018-08-07 16:17 UTC (permalink / raw)
  To: kieran.bingham+renesas
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
	Mauro Carvalho Chehab, linux-renesas-soc

Hello,

On Tuesday, 7 August 2018 13:36:59 EEST Kieran Bingham wrote:
> On 07/08/18 11:18, Mauro Carvalho Chehab wrote:
> > Gets rid of this build warning:
> > drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or
> > 	member 'cmdpool' not described in 'vsp1_dl_manager'> 
> > Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command
> > pools") Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
> > ---
> > 
> >  drivers/media/platform/vsp1/vsp1_dl.c | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> > b/drivers/media/platform/vsp1/vsp1_dl.c index 9255b5ee2cb8..af60d95ec4f8
> > 100644
> > --- a/drivers/media/platform/vsp1/vsp1_dl.c
> > +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> > @@ -211,6 +211,7 @@ struct vsp1_dl_list {
> >   * @queued: list queued to the hardware (written to the DL registers)
> >   * @pending: list waiting to be queued to the hardware
> >   * @pool: body pool for the display list bodies
> > + * @cmdpool: Display List commands pool
> 
> Unfortunately this isn't quite right...
> 
> >   * @autofld_cmds: command pool to support auto-fld interlaced mode
> 
> This ^ was the original documentation line, but it got missed in a
> rename. Sorry about that.
> 
> The pool is now more 'generic' so the line probably should mention the
> auto-fld directly, so your line is worded appropriately enough, We
> probably just# need to remove the autofld_cmds line.

And I also wouldn't capitalize display list:

	 * @cmdpool: display list commands pool

or possibly a bit more descriptive:

	 * @cmdpool: commands pool for extended display list

Kieran, which version do you like best ?

> 
> With that line removed:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> 
> --
> Kieran
> 
> >   */
> >  
> >  struct vsp1_dl_manager {


-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] media: vsp1_dl: add a description for cmdpool field
  2018-08-07 16:17   ` Laurent Pinchart
@ 2018-08-07 16:19     ` Kieran Bingham
  2018-08-07 16:26       ` Laurent Pinchart
  0 siblings, 1 reply; 6+ messages in thread
From: Kieran Bingham @ 2018-08-07 16:19 UTC (permalink / raw)
  To: Laurent Pinchart
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
	Mauro Carvalho Chehab, linux-renesas-soc

On 07/08/18 17:17, Laurent Pinchart wrote:
> Hello,
> 
> On Tuesday, 7 August 2018 13:36:59 EEST Kieran Bingham wrote:
>> On 07/08/18 11:18, Mauro Carvalho Chehab wrote:
>>> Gets rid of this build warning:
>>> drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter or
>>> 	member 'cmdpool' not described in 'vsp1_dl_manager'> 
>>> Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command
>>> pools") Signed-off-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>
>>> ---
>>>
>>>  drivers/media/platform/vsp1/vsp1_dl.c | 1 +
>>>  1 file changed, 1 insertion(+)
>>>
>>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
>>> b/drivers/media/platform/vsp1/vsp1_dl.c index 9255b5ee2cb8..af60d95ec4f8
>>> 100644
>>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
>>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
>>> @@ -211,6 +211,7 @@ struct vsp1_dl_list {
>>>   * @queued: list queued to the hardware (written to the DL registers)
>>>   * @pending: list waiting to be queued to the hardware
>>>   * @pool: body pool for the display list bodies
>>> + * @cmdpool: Display List commands pool
>>
>> Unfortunately this isn't quite right...
>>
>>>   * @autofld_cmds: command pool to support auto-fld interlaced mode
>>
>> This ^ was the original documentation line, but it got missed in a
>> rename. Sorry about that.
>>
>> The pool is now more 'generic' so the line probably should mention the
>> auto-fld directly, so your line is worded appropriately enough, We
>> probably just# need to remove the autofld_cmds line.
> 
> And I also wouldn't capitalize display list:
> 
> 	 * @cmdpool: display list commands pool
> 
> or possibly a bit more descriptive:
> 
> 	 * @cmdpool: commands pool for extended display list
> 
> Kieran, which version do you like best ?

If there's a respin anyway, then

 	 * @cmdpool: commands pool for extended display list

is certainly better, because this commands are specific to the extended
portion of the display list.

--
Regards

Kieran


> 
>>
>> With that line removed:
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
>>
>> --
>> Kieran
>>
>>>   */
>>>  
>>>  struct vsp1_dl_manager {
> 
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] media: vsp1_dl: add a description for cmdpool field
  2018-08-07 16:19     ` Kieran Bingham
@ 2018-08-07 16:26       ` Laurent Pinchart
  0 siblings, 0 replies; 6+ messages in thread
From: Laurent Pinchart @ 2018-08-07 16:26 UTC (permalink / raw)
  To: kieran.bingham+renesas
  Cc: Mauro Carvalho Chehab, Linux Media Mailing List,
	Mauro Carvalho Chehab, linux-renesas-soc

Hi,

On Tuesday, 7 August 2018 19:19:41 EEST Kieran Bingham wrote:
> On 07/08/18 17:17, Laurent Pinchart wrote:
> > On Tuesday, 7 August 2018 13:36:59 EEST Kieran Bingham wrote:
> >> On 07/08/18 11:18, Mauro Carvalho Chehab wrote:
> >>> Gets rid of this build warning:
> >>> drivers/media/platform/vsp1/vsp1_dl.c:229: warning: Function parameter
> >>> or member 'cmdpool' not described in 'vsp1_dl_manager'>
> >>> 
> >>> Fixes: f3b98e3c4d2e ("media: vsp1: Provide support for extended command
> >>> pools") Signed-off-by: Mauro Carvalho Chehab
> >>> <mchehab+samsung@kernel.org>
> >>> ---
> >>> 
> >>>  drivers/media/platform/vsp1/vsp1_dl.c | 1 +
> >>>  1 file changed, 1 insertion(+)
> >>> 
> >>> diff --git a/drivers/media/platform/vsp1/vsp1_dl.c
> >>> b/drivers/media/platform/vsp1/vsp1_dl.c index 9255b5ee2cb8..af60d95ec4f8
> >>> 100644
> >>> --- a/drivers/media/platform/vsp1/vsp1_dl.c
> >>> +++ b/drivers/media/platform/vsp1/vsp1_dl.c
> >>> @@ -211,6 +211,7 @@ struct vsp1_dl_list {
> >>>   * @queued: list queued to the hardware (written to the DL registers)
> >>>   * @pending: list waiting to be queued to the hardware
> >>>   * @pool: body pool for the display list bodies
> >>> + * @cmdpool: Display List commands pool
> >> 
> >> Unfortunately this isn't quite right...
> >> 
> >>>   * @autofld_cmds: command pool to support auto-fld interlaced mode
> >> 
> >> This ^ was the original documentation line, but it got missed in a
> >> rename. Sorry about that.
> >> 
> >> The pool is now more 'generic' so the line probably should mention the
> >> auto-fld directly, so your line is worded appropriately enough, We
> >> probably just# need to remove the autofld_cmds line.
> > 
> > And I also wouldn't capitalize display list:
> > 
> > 	 * @cmdpool: display list commands pool
> > 
> > or possibly a bit more descriptive:
> > 
> > 	 * @cmdpool: commands pool for extended display list
> > 
> > Kieran, which version do you like best ?
> 
> If there's a respin anyway, then
> 
>  	 * @cmdpool: commands pool for extended display list
> 
> is certainly better, because this commands are specific to the extended
> portion of the display list.

I like that better as well. Mauro, could you please use that for v2 ?

> >> With that line removed:
> >> 
> >> Reviewed-by: Kieran Bingham <kieran.bingham+renesas@ideasonboard.com>
> >> 
> >>>   */
> >>>  
> >>>  struct vsp1_dl_manager {

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2018-08-07 18:41 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-08-07 10:18 [PATCH] media: vsp1_dl: add a description for cmdpool field Mauro Carvalho Chehab
2018-08-07 10:36 ` Kieran Bingham
2018-08-07 16:10   ` Kieran Bingham
2018-08-07 16:17   ` Laurent Pinchart
2018-08-07 16:19     ` Kieran Bingham
2018-08-07 16:26       ` Laurent Pinchart

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).