Re: [PATCH 3/5] ASoC: Intel: kbl-rt5660: use .exit() dailink callback to release gpiod

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Cc: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>,
	alsa-devel@alsa-project.org,
	Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>,
	tiwai@suse.de, Curtis Malainey <curtis@malainey.com>,
	broonie@kernel.org
Subject: Re: [PATCH 3/5] ASoC: Intel: kbl-rt5660: use .exit() dailink callback to release gpiod
Date: Mon, 22 Jun 2020 21:20:09 +0300	[thread overview]
Message-ID: <20200622182009.GH2428291@smile.fi.intel.com> (raw)
In-Reply-To: <20200622154241.29053-4-pierre-louis.bossart@linux.intel.com>

On Mon, Jun 22, 2020 at 10:42:39AM -0500, Pierre-Louis Bossart wrote:
> The gpiod handling is inspired from the bdw-rt5677 code. Apply same
> fix to avoid reference count issue while removing modules for
> consistency.

...

> -	ctx->gpio_lo_mute = devm_gpiod_get(component->dev, "lineout-mute",
> -					   GPIOD_OUT_HIGH);
> +	ctx->gpio_lo_mute = gpiod_get(component->dev, "lineout-mute",
> +				      GPIOD_OUT_HIGH);
>  	if (IS_ERR(ctx->gpio_lo_mute)) {
>  		dev_err(component->dev, "Can't find GPIO_MUTE# gpio\n");
>  		return PTR_ERR(ctx->gpio_lo_mute);

Is it fatal? Then IS_ERR() is not needed below. For NULL I already told.

> +	/*
> +	 * The .exit() can be reached without going through the .init()
> +	 * so explicitly test if the gpiod is valid
> +	 */

This comment should be amended after fixing the code.

> +	if (!IS_ERR_OR_NULL(ctx->gpio_lo_mute))
> +		gpiod_put(ctx->gpio_lo_mute);
> +}

-- 
With Best Regards,
Andy Shevchenko