| Message ID | 1448980166-23055-1-git-send-email-jani.nikula@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Dec 01, 2015 at 04:29:25PM +0200, Jani Nikula wrote: > Shorter, easier to follow code with no functional changes. In all cases, > the return value ultimately comes from gmbus_wait_hw_status() anyway. > > Signed-off-by: Jani Nikula <jani.nikula@intel.com> > --- > drivers/gpu/drm/i915/intel_i2c.c | 12 ++++-------- > 1 file changed, 4 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c > index 1110c83953cf..ccb522c176bd 100644 > --- a/drivers/gpu/drm/i915/intel_i2c.c > +++ b/drivers/gpu/drm/i915/intel_i2c.c > @@ -505,17 +505,13 @@ retry: > ret = gmbus_xfer_write(dev_priv, &msgs[i]); > } > > + if (!ret) > + ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE, > + GMBUS_HW_WAIT_EN); > if (ret == -ETIMEDOUT) > goto timeout; > - if (ret == -ENXIO) > + else if (ret) > goto clear_err; > - > - ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE, > - GMBUS_HW_WAIT_EN); > - if (ret == -ENXIO) > - goto clear_err; > - if (ret) > - goto timeout; Hmm. So gmbus_waut_hw_status() only ever returns -ENXIO,-ETIMEDOUT,0. And since the xfer functions return value also comes from gmbus_wait_hw_status() the same holds for them. For -ENXIO we want to go to clear_err, for -ETIMEDOUT we want to go to timeout. Other error values can't happen so the old 'if (ret)' case actually checks for timeouts. So even if you sort of reversed the checks there, it actually still checks for the same thing. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > } > > /* Generate a STOP condition on the bus. Note that gmbus can't generata > -- > 2.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c index 1110c83953cf..ccb522c176bd 100644 --- a/drivers/gpu/drm/i915/intel_i2c.c +++ b/drivers/gpu/drm/i915/intel_i2c.c @@ -505,17 +505,13 @@ retry: ret = gmbus_xfer_write(dev_priv, &msgs[i]); } + if (!ret) + ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE, + GMBUS_HW_WAIT_EN); if (ret == -ETIMEDOUT) goto timeout; - if (ret == -ENXIO) + else if (ret) goto clear_err; - - ret = gmbus_wait_hw_status(dev_priv, GMBUS_HW_WAIT_PHASE, - GMBUS_HW_WAIT_EN); - if (ret == -ENXIO) - goto clear_err; - if (ret) - goto timeout; } /* Generate a STOP condition on the bus. Note that gmbus can't generata
Shorter, easier to follow code with no functional changes. In all cases, the return value ultimately comes from gmbus_wait_hw_status() anyway. Signed-off-by: Jani Nikula <jani.nikula@intel.com> --- drivers/gpu/drm/i915/intel_i2c.c | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-)