On ti, 2017-02-14 at 20:37 +0100, Michal Wajdeczko wrote: > On Tue, Feb 14, 2017 at 05:15:39PM +0100, Arkadiusz Hiler wrote: > > > > Let intel_guc_init() focus on determining and fetching the correct > > firmware. > > > > This patch introduces intel_sanitize_uc_params() that is called from > > intel_uc_init(). > > > > Then, if we have GuC, we can call intel_guc_init() conditionally and we > > do not have to do internal checks. > > > > Cc: Michal Winiarski <michal.winiar...@intel.com> > > Cc: Michal Wajdeczko <michal.wajdec...@intel.com> > > Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com> > > Signed-off-by: Arkadiusz Hiler <arkadiusz.hi...@intel.com>
<SNIP> > > @@ -726,7 +726,7 @@ void intel_uc_fw_fetch(struct drm_i915_private > > *dev_priv, > > } > > > > /** > > - * intel_guc_init() - define parameters and fetch firmware > > + * intel_guc_init() - determine and fetch firmware > > Again, can we have function name that corresponds to its purpose. > Comment alone is not sufficient I second Michal here, the naming of the functions is rather poor. Something like intel_guc_fetch_firmware would be more descriptive. Making it overly generic when we don't have other use yet, will cause more churn when we get more use (because fortune telling was never our thing). I'd rather see rather standalone functions, which are then be called from central point where more can be added, without changing the existing ones. So this applies to intel_uc_init, too. It'll be easier to add these generic functions iff we start repeating calls to a bunch of functions, and we can then see patterns emerging. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
