[PATCH] i2c: mux: Fix error path in i2c-arb-gpio

Heiko Schocher hs at denx.de
Thu Aug 1 12:27:28 CEST 2024


Hello Michal,

On 01.08.24 10:01, Michal Simek wrote:
> There is no reason to use goto and just call return. Better is to call
> return directly which is done for some if/else parts.
> 
> Also make no sense to setup ret to -ETIMEDOUT and then to 0.
> Return timeout directly.
> 
> Signed-off-by: Michal Simek <michal.simek at amd.com>
> ---
> 
>   drivers/i2c/muxes/i2c-arb-gpio-challenge.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> index a83d7cb0829d..3d2ce0ca705d 100644
> --- a/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> +++ b/drivers/i2c/muxes/i2c-arb-gpio-challenge.c
> @@ -54,7 +54,7 @@ int i2c_arbitrator_select(struct udevice *mux, struct udevice *bus,
>   		/* Indicate that we want to claim the bus */
>   		ret = dm_gpio_set_value(&priv->ap_claim, 1);
>   		if (ret)
> -			goto err;
> +			return ret;
>   		udelay(priv->slew_delay_us);
>   
>   		/* Wait for the EC to release it */
> @@ -62,7 +62,7 @@ int i2c_arbitrator_select(struct udevice *mux, struct udevice *bus,
>   		while (get_timer(start_retry) < priv->wait_retry_ms) {
>   			ret = dm_gpio_get_value(&priv->ec_claim);
>   			if (ret < 0) {
> -				goto err;
> +				return ret;
>   			} else if (!ret) {
>   				/* We got it, so return */
>   				return 0;
> @@ -75,17 +75,14 @@ int i2c_arbitrator_select(struct udevice *mux, struct udevice *bus,
>   		/* It didn't release, so give up, wait, and try again */
>   		ret = dm_gpio_set_value(&priv->ap_claim, 0);
>   		if (ret)
> -			goto err;
> +			return ret;
>   
>   		mdelay(priv->wait_retry_ms);
>   	} while (get_timer(start) < priv->wait_free_ms);
>   
>   	/* Give up, release our claim */
>   	printf("I2C: Could not claim bus, timeout %lu\n", get_timer(start));
> -	ret = -ETIMEDOUT;
> -	ret = 0;

I wonder here ...

It seems to me current driver returns 0 instead -ETIMEDOUT in this case?

So it is also a bugfix ... ?

> -err:
> -	return ret;
> +	return -ETIMEDOUT;
>   }
>   
>   static int i2c_arbitrator_probe(struct udevice *dev)
> 

bye,
Heiko
-- 
DENX Software Engineering GmbH,      Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-52   Fax: +49-8142-66989-80   Email: hs at denx.de


More information about the U-Boot mailing list