Re: [bug report] ASoC: mchp-spdifrx: add driver for SPDIF RX

[<Codrin.Ciubotariu@microchip.com>]

On 03.12.2020 16:58, Dan Carpenter wrote:
> Hello Codrin Ciubotariu,
> 
> The patch ef265c55c1ac: "ASoC: mchp-spdifrx: add driver for SPDIF RX"
> from Oct 2, 2020, leads to the following static checker warning:
> 
>          sound/soc/atmel/mchp-spdifrx.c:468 mchp_spdifrx_hw_params()
>          warn: 'dev->gclk' not released on lines: 468.
> 
> sound/soc/atmel/mchp-spdifrx.c
>     442                          params_format(params));
>     443                  return -EINVAL;
>     444          }
>     445
>     446          if (dev->gclk_enabled) {
>     447                  clk_disable_unprepare(dev->gclk);
>     448                  dev->gclk_enabled = 0;
>     449          }
>     450          ret = clk_set_min_rate(dev->gclk, params_rate(params) *
>     451                                            SPDIFRX_GCLK_RATIO_MIN + 1);
>     452          if (ret) {
>     453                  dev_err(dev->dev,
>     454                          "unable to set gclk min rate: rate %u * ratio %u + 1\n",
>     455                          params_rate(params), SPDIFRX_GCLK_RATIO_MIN);
>     456                  return ret;
>     457          }
>     458          ret = clk_prepare_enable(dev->gclk);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
>     459          if (ret) {
>     460                  dev_err(dev->dev, "unable to enable gclk: %d\n", ret);
>     461                  return ret;
>     462          }
>     463          dev->gclk_enabled = 1;
>     464
>     465          dev_dbg(dev->dev, "GCLK range min set to %d\n",
>     466                  params_rate(params) * SPDIFRX_GCLK_RATIO_MIN + 1);
>     467
>     468          return regmap_write(dev->regmap, SPDIFRX_MR, mr);
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Smatch is complaining that if the regmap_write() fails then we should
> disable and unprepare the "dev->gclk".
> 
>     469  }
> 

Thanks for reporting this Dan. hw_free() callback is still called if 
hw_params() fails, which will unprepare gclk, but I guess it doesn't 
hurt to add what you suggested.

Best regards,
Codrin