Re: [PATCH v2 3/7] drm/i2c: tda998x: fix error cleanup paths

[Hans Verkuil <hverkuil@xs4all.nl>]

On 12/06/17 13:35, Russell King wrote:
> If tda998x_get_audio_ports() fails, and we requested the interrupt, we
> fail to free the interrupt before returning failure.  Rework the failure
> cleanup code and exit paths so that we always clean up properly after an
> error, and always propagate the error code.
> 
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/gpu/drm/i2c/tda998x_drv.c | 31 ++++++++++++++++++-------------
>  1 file changed, 18 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 4aeac2127974..661cb8915f2f 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -1499,10 +1499,15 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  
>  	/* read version: */
>  	rev_lo = reg_read(priv, REG_VERSION_LSB);
> +	if (rev_lo < 0) {
> +		dev_err(&client->dev, "failed to read version: %d\n", rev_lo);
> +		return rev_lo;
> +	}
> +
>  	rev_hi = reg_read(priv, REG_VERSION_MSB);
> -	if (rev_lo < 0 || rev_hi < 0) {
> -		ret = rev_lo < 0 ? rev_lo : rev_hi;
> -		goto fail;
> +	if (rev_hi < 0) {
> +		dev_err(&client->dev, "failed to read version: %d\n", rev_hi);
> +		return rev_hi;
>  	}
>  
>  	priv->rev = rev_lo | rev_hi << 8;
> @@ -1526,7 +1531,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  	default:
>  		dev_err(&client->dev, "found unsupported device: %04x\n",
>  			priv->rev);
> -		goto fail;
> +		return -ENXIO;
>  	}
>  
>  	/* after reset, enable DDC: */
> @@ -1568,7 +1573,7 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  			dev_err(&client->dev,
>  				"failed to request IRQ#%u: %d\n",
>  				client->irq, ret);
> -			goto fail;
> +			goto err_irq;
>  		}
>  
>  		/* enable HPD irq */
> @@ -1591,19 +1596,19 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv)
>  
>  	ret = tda998x_get_audio_ports(priv, np);
>  	if (ret)
> -		goto fail;
> +		goto err_audio;
>  
>  	if (priv->audio_port[0].format != AFMT_UNUSED)
>  		tda998x_audio_codec_init(priv, &client->dev);
>  
>  	return 0;
> -fail:
> -	/* if encoder_init fails, the encoder slave is never registered,
> -	 * so cleanup here:
> -	 */
> -	if (priv->cec)
> -		i2c_unregister_device(priv->cec);
> -	return -ENXIO;
> +
> +err_audio:
> +	if (client->irq)
> +		free_irq(client->irq, priv);
> +err_irq:
> +	i2c_unregister_device(priv->cec);
> +	return ret;
>  }
>  
>  static void tda998x_encoder_prepare(struct drm_encoder *encoder)
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel