public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/i915: simplify gmbus xfer error checks
Date: Tue, 1 Dec 2015 16:45:18 +0200	[thread overview]
Message-ID: <20151201144518.GD4437@intel.com> (raw)
In-Reply-To: <1448980166-23055-1-git-send-email-jani.nikula@intel.com>

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

      parent reply	other threads:[~2015-12-01 14:45 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-01 14:29 [PATCH 1/2] drm/i915: simplify gmbus xfer error checks Jani Nikula
2015-12-01 14:29 ` [PATCH 2/2] drm/i915: abstract i2c bit banging fallback in gmbus xfer Jani Nikula
2015-12-01 14:57   ` Ville Syrjälä
2015-12-01 15:38     ` Jani Nikula
2015-12-01 15:52       ` Ville Syrjälä
2015-12-02 11:35         ` Jani Nikula
2015-12-01 14:45 ` Ville Syrjälä [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151201144518.GD4437@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jani.nikula@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox