On Thu, Dec 03, 2020 at 03:31:33PM +0000, Codrin.Ciubotariu@microchip.com wrote:

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.

It might hurt if it leads to a double free... Just leave it as is. This is a new warning message for Smatch and I'm still working out the kinks.

regards, dan carpenter