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);
> > > +}
> >
--
With Best Regards,
Andy Shevchenko
