From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751778AbdJXQNz (ORCPT ); Tue, 24 Oct 2017 12:13:55 -0400 Received: from mga04.intel.com ([192.55.52.120]:37039 "EHLO mga04.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751682AbdJXQNw (ORCPT ); Tue, 24 Oct 2017 12:13:52 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,428,1503385200"; d="scan'208";a="913235965" From: Jani Nikula To: SF Markus Elfring , dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org, David Airlie , Joonas Lahtinen , Rodrigo Vivi Cc: LKML , kernel-janitors@vger.kernel.org Subject: Re: [PATCH 2/2] drm/i915/dp: Use common error handling code in intel_dp_sink_crc_stop() In-Reply-To: <1d3d0395-957a-2a87-3cb5-0d7355d04b65@users.sourceforge.net> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <668a4b56-091d-d4e7-2204-daaec6675e11@users.sourceforge.net> <1d3d0395-957a-2a87-3cb5-0d7355d04b65@users.sourceforge.net> Date: Tue, 24 Oct 2017 19:13:27 +0300 Message-ID: <873768h6c8.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 24 Oct 2017, SF Markus Elfring wrote: > From: Markus Elfring > Date: Tue, 24 Oct 2017 15:40:47 +0200 > > Adjust jump targets so that a specific error code assignment > will be in the implementation only at the end of this function. This is not an issue that needs to be "fixed". > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring > --- > drivers/gpu/drm/i915/intel_dp.c | 30 ++++++++++++++---------------- > 1 file changed, 14 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 342f1a1fa085..3dd514a77008 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -3894,27 +3894,19 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp) > int count = 0; > int attempts = 10; > > - if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) { > - DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n"); > - ret = -EIO; > - goto out; > - } > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK, &buf) < 0) > + goto report_failure; > > if (drm_dp_dpcd_writeb(&intel_dp->aux, DP_TEST_SINK, > - buf & ~DP_TEST_SINK_START) < 0) { > - DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n"); > - ret = -EIO; > - goto out; I'd rather change the debug messages to distinguish the cases. I also want to keep ret = -EIO in each individual branch, to distinguish from the cases where it's set to something other than -EIO. > - } > + buf & ~DP_TEST_SINK_START) < 0) > + goto report_failure; > > do { > intel_wait_for_vblank(dev_priv, intel_crtc->pipe); > + if (drm_dp_dpcd_readb(&intel_dp->aux, DP_TEST_SINK_MISC, &buf) > + < 0) > + goto e_io; > > - if (drm_dp_dpcd_readb(&intel_dp->aux, > - DP_TEST_SINK_MISC, &buf) < 0) { > - ret = -EIO; > - goto out; > - } This is good as it is. The change makes it worse. > count = buf & DP_TEST_COUNT_MASK; > } while (--attempts && count); > > @@ -3923,9 +3915,15 @@ static int intel_dp_sink_crc_stop(struct intel_dp *intel_dp) > ret = -ETIMEDOUT; > } > > - out: > +enable_ips: > hsw_enable_ips(intel_crtc); > return ret; > + > +report_failure: > + DRM_DEBUG_KMS("Sink CRC couldn't be stopped properly\n"); > +e_io: > + ret = -EIO; > + goto enable_ips; *shudder* Please no. BR, Jani. > } > > static int intel_dp_sink_crc_start(struct intel_dp *intel_dp) -- Jani Nikula, Intel Open Source Technology Center