Re: [PATCH 02/10] drm/i915: Added port as parameter to the functions which does read/write of DSI Controller

From: "Singh, Gaurav K" <gaurav.k.singh@intel.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: Shobhit Kumar <shobhit.kumar@intel.com>,
	intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 02/10] drm/i915: Added port as parameter to the	functions which does read/write of DSI Controller
Date: Sat, 06 Dec 2014 02:05:26 +0530	[thread overview]
Message-ID: <5482170E.7040604@intel.com> (raw)
In-Reply-To: <20141205143846.GP20350@phenom.ffwll.local>

On 12/5/2014 8:08 PM, Daniel Vetter wrote:
> On Fri, Dec 05, 2014 at 06:20:43PM +0530, Singh, Gaurav K wrote:
>> On 12/4/2014 4:52 PM, Daniel Vetter wrote:
>>> On Thu, Dec 04, 2014 at 11:14:01AM +0200, Jani Nikula wrote:
>>>> On Thu, 04 Dec 2014, Gaurav K Singh <gaurav.k.singh@intel.com> wrote:
>>>>> This patch is in preparation of DSI dual link panels. For dual link
>>>>> panels, few packets needs to be sent to Port A or Port C or both. Based
>>>>> on the portno from MIPI Sequence Block#53, these sequences needs to be
>>>>> sent accordingly.
>>>>>
>>>>> v2: Addressed review comments by Jani
>>>>>      - port variables named properly
>>>>>
>>>>> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
>>>> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>>> Merged the first two patches, thanks.
>>> -Daniel
>> Hi Daniel,
>>
>> Thanks. I have addressed Jani's review comments too for the rest of the
>> patches.
> Well looking through the resend patches your changes look a bit minimal.
> Which might be the right thing to do, but review is also about
> communication and sharing expert knowledge. So please follow up to the
> questions from Jani where you didn't adjust the code. And if there's any
> follow-up patches from those discussions please submit them.
>
> Another part: Your editor doesn't seem to align continuation lines quite
> how we usually do that. checkpatch --strict will catch that. Two big
> rules:
> - function paramater lists should be aligned to the opening ( on the next
>    line.
> - Continuations of boolean logic checks (e.g. in if checks) same, with the
>    special rule that && or || should be on the previous line as the last
>    thing.
> - continuation of any other long lines should be indented 1 or two spaces
>    (just to make sure it sticks out without looking jarring).
>
> Especially the first two help readability a lot of you're used to them, so
> please do a follow-up patch to rectify this in intel_dsi*.c.
>
> Thanks, Daniel

Done. Fixed the style related issues and comments also added.

With regards,
Gaurav
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx