Re: [PATCH 09/10] drm/i915/uc: Separate firmware selection and preparation

[Arkadiusz Hiler <arkadiusz.hiler@intel.com>]

On Fri, Feb 24, 2017 at 07:29:57PM +0100, Michal Wajdeczko wrote:
> On Fri, Feb 24, 2017 at 04:40:03PM +0100, Arkadiusz Hiler wrote:
> > intel_{h,g}uc_init_fw selects correct firmware and then triggers it's
> > preparation (fetch + initial parsing).
> > 
> > This change separates out select steps, so those can be called by
> > the sanitize_options().
> > 
> > Then, during the init_fw() we prepare the firmware if the firmware was
> > selected.
> > 
> > Cc: Michal Winiarski <michal.winiarski@intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> > ---
> 
> It looks strange that in one series you're changing function names
> or their logic few times ... patch ordering really matters ;)

Is this really that bad? I do not see that much disadvantage to the
current state.

I am seeing the series as a whole. Titles of the commits are pretty
informative (and you have them both in you email client when you are
viewing the whole thread, and in the cover letter) - e.g. you can see that
one of the later commits reworks firmware path handling, and can assume
that the previous changes to that code were intermediary (changing
argument it takes and removing one or two checks as they are now on the
level above).

As of the name changes - those follow a pattern.

If I went with the original name until the final change it looks
inconsistent, and quite probably someone would've commented on that.

If I have had used the final names straight away that would be simply
misleading (in case of intel_guc_init -> init_fw -> select_fw).

> Please consider reorder/squash.

I've tried to rebase with squashing and resplitting two times, and what
I came with was pretty similar to the current series.

But I might have hard-wired my brain to follow those exact steps,
therefore the repetition...

If you have any suggestion how to do that properly, keeping changes
small, logical and ordered please share. I'd love to learn how to make
my series less painful for future reviewers.

-- 
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx