Re: [PATCH] media: vsp1_dl: add a description for cmdpool field

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
>>> ---
>>>
>>>  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@xxxxxxxxxxxxxxxx>
>>
>> --
>> Kieran
>>
>>>   */
>>>  
>>>  struct vsp1_dl_manager {
> 
> 




[Index of Archives]     [Linux Samsung SOC]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux