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

On Mon, Jun 22, 2020 at 01:26:15PM -0500, Pierre-Louis Bossart wrote:

On 6/22/20 1:20 PM, Andy Shevchenko wrote:

...

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.

this patch only fixes a deadlock, whether or not this is fatal or not is a different question. I would assert if if you can't mute your audio is broken.

Ah, sorry, I mean IS_ERR() is not needed for an optional case. Since it is fatal, it's fine.

...

...

• /*

• * 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);
  

+}