Re: [PATCH 1/2] drm/i915: simplify gmbus xfer error checks

Ville Syrjälä
2015-12-01T14:45:29Z

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

--
Ville Syrjälä
Intel OTC