Re: [PATCH] drm/exynos: clean up description of exynos_drm_crtc

[Inki Dae <inki.dae@samsung.com>]

Hello Tobias,


2017년 04월 07일 02:10에 Tobias Jakobi 이(가) 쓴 글:
> Hello Inki,
> 
> 
> Inki Dae wrote:
>> This patch removes unnecessary descriptions on
>> exynos_drm_crtc structure and adds one description
>> which specifies what pipe_clk member does.
>>
>> pipe_clk support had been added by below patch without any description,
>> 	 drm/exynos: add support for pipeline clock to the framework
> I would put the commit id here. That makes it easier to look up the
> commit your refer to.
> 
> 
>> Signed-off-by: Inki Dae <inki.dae@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h | 6 ++----
>>  1 file changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 527bf1d..b0462cc 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -152,12 +152,10 @@ struct exynos_drm_clk {
>>   *
>>   * @base: crtc object.
>>   * @type: one of EXYNOS_DISPLAY_TYPE_LCD and HDMI.
>> - * @enabled: if the crtc is enabled or not
>> - * @event: vblank event that is currently queued for flip
>> - * @wait_update: wait all pending planes updates to finish
>> - * @pending_update: number of pending plane updates in this crtc
>>   * @ops: pointer to callbacks for exynos drm specific functionality
>>   * @ctx: A pointer to the crtc's implementation specific context
>> + * @pipe_clk: A pipe clock structure which includes a callback function
>> +	      for enabling DP clock of FIMD and HDMI PHY clock.
> This is wrong/inconsistent with the rest of the documentation. pipe_clk
> is not a struct, but a pointer.
> 
> I would suggest to follow. Just document that we have a pointer to <add
> escription> here (compare docu for 'ops' and 'ctx').
> And then put the later bits ("...callback function for enabling DP
> clock...") directly to the definition of 'struct exynos_drm_clk' (which
> is currently lacking any kind of docu).

Thanks for pointing it out. My mistake. :)

How about this simply?
"A pointer to exynos_drm_clk structure for enabling and disabling DP clock of FIMD and HDMI PHY clocks"

Thanks,
Inki Dae

> 
> 
> - Tobias
> 
>>   */
>>  struct exynos_drm_crtc {
>>  	struct drm_crtc			base;
>>
> 
> 
> 
>