Hello Inki,
Inki Dae wrote: > 2017년 04월 11일 17:17에 Tobias Jakobi 이(가) 쓴 글: >> Another thing that I noticed. Why wasn't the v2 that ended up in your >> git ever submitted to the mailing list? Because it should have, in >> particular to spot these obvious errors. > > Only comment about this. > > This patch cleans up description of struct exynos_drm_clk - removed > unnecessary descriptions and adds one missed before. > I think no problem to update the description without v2 because no code > change and description enough. I think you miss the point here. I checked the mail thread again and you responded with "More simple and looks better." when I suggested to put the largest part of your description in front of 'struct exynos_drm_clk'. I even went so far as to prepare the comment for you. And then you proceed by ignoring everything and merging some completly different patch. I find this disrespectful. I'm quoting your words here (from [1]): > I'd like to say *maintainer is really not a place for power*, and maintainer > would implicitly have a role to encourage in contribution activity of > contributer. If you really mean this, you should also act accordingly. And that does not mean saying "A" to someone and then doing "B". > If you want to update the description more then you can post it. > Ps. I am not a leisurely person to respond to every little thing. I don't expect you to respond to every little thing. But I expect proper and honest communication. Also a response delay of four weeks is simply not acceptable. And I don't think I'm the only one on dri-devel that thinks that way. With best wishes, Tobias [1] http://www.spinics.net/lists/dri-devel/msg131304.html > > Thanks, > Inki Dae > >> >> - Tobias >> >> >> Tobias Jakobi wrote: >>> Inki Dae wrote: >>>> >>>> >>>> 2017년 04월 10일 19:29에 Tobias Jakobi 이(가) 쓴 글: >>>>> Inki Dae wrote: >>>>>> 2017-04-07 20:40 GMT+09:00 Tobias Jakobi <tjak...@math.uni-bielefeld.de>: >>>>>>> Hello Inki, >>>>>>> >>>>>>> >>>>>>> Inki Dae wrote: >>>>>>>> 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....@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" >>>>>>> Not what I meant. You're describing 'struct exynos_drm_clk', and that >>>>>>> does not belong here. If you describe 'struct exynos_drm_clk', then this >>>>>>> should go in front of the struct's definition. >>>>>>> >>>>>>> How abouting something like this in front of the struct's definition:: >>>>>>>> /* >>>>>>>> * Exynos DRM pipeline clock structure. >>>>>>>> * >>>>>>>> * @enable: callback to enable/disable the clock. >>>>>>>> * >>>>>>>> * Used for proper clock synchronization of components belonging >>>>>>>> * to the same pipeline. Used e.g. for enabling the DP clock of >>>>>>>> * the FIMD or the HDMI PHY clock. >>>>>>>> */ >>>>>>>> struct exynos_drm_clk { >>>>>>>> <snip> >>>>>>> >>>>>>> For 'pipe_clk' I would just go with this: >>>>>>>> @pipe_clk: A pointet to the crtc's pipeline clock. >>>>>> >>>>>> More simple and looks better. >>>>> In this form (commit a07d468cb2efd347a9e279e4f68661f0f370d10f in >>>>> exynos-drm-next), the description is incomplete. Please read my mails >>>>> again. >>>> >>>> Many patches in mainline used same form. Please git log | grep "commit-id" >>>> -n10. >>>> Sorry but no update and no comment anymore but will use the generic form >>>> later. >>> I'm not referring to your use of commit-id, but to you totally ignoring >>> the documentation bits for 'struct exynos_drm_clk'. Please be more >>> careful when reading my mails. >>> >>> - Tobias >>> >>> >>> >>>> Thanks, >>>> Inki Dae >>>> >>>>> >>>>> - Tobias >>>>> >>>>> >>>>>> >>>>>> Thanks, >>>>>> Inki Dae >>>>>> >>>>>>> >>>>>>> I hope this helps. >>>>>>> >>>>>>> - Tobias >>>>>>> >>>>>>> >>>>>>>> Thanks, >>>>>>>> Inki Dae >>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> - Tobias >>>>>>>>> >>>>>>>>>> */ >>>>>>>>>> struct exynos_drm_crtc { >>>>>>>>>> struct drm_crtc base; >>>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>> >>>>>>> -- >>>>>>> To unsubscribe from this list: send the line "unsubscribe >>>>>>> linux-samsung-soc" in >>>>>>> the body of a message to majord...@vger.kernel.org >>>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>> -- >>>>>> To unsubscribe from this list: send the line "unsubscribe >>>>>> linux-samsung-soc" in >>>>>> the body of a message to majord...@vger.kernel.org >>>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>>> >>>>> >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe >>>>> linux-samsung-soc" in >>>>> the body of a message to majord...@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>>> >>>>> >>>>> >>>> -- >>>> To unsubscribe from this list: send the line "unsubscribe >>>> linux-samsung-soc" in >>>> the body of a message to majord...@vger.kernel.org >>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>>> >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe >>> linux-samsung-soc" in >>> the body of a message to majord...@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> >> >> >> >> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
