* [bug report] ASoC: mchp-spdifrx: add driver for SPDIF RX
@ 2020-12-03 14:58 Dan Carpenter
2020-12-03 15:31 ` Codrin.Ciubotariu
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2020-12-03 14:58 UTC (permalink / raw)
To: codrin.ciubotariu; +Cc: alsa-devel
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 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] ASoC: mchp-spdifrx: add driver for SPDIF RX
2020-12-03 14:58 [bug report] ASoC: mchp-spdifrx: add driver for SPDIF RX Dan Carpenter
@ 2020-12-03 15:31 ` Codrin.Ciubotariu
2020-12-03 18:38 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Codrin.Ciubotariu @ 2020-12-03 15:31 UTC (permalink / raw)
To: dan.carpenter; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [bug report] ASoC: mchp-spdifrx: add driver for SPDIF RX
2020-12-03 15:31 ` Codrin.Ciubotariu
@ 2020-12-03 18:38 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-12-03 18:38 UTC (permalink / raw)
To: Codrin.Ciubotariu; +Cc: alsa-devel
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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-12-03 18:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-03 14:58 [bug report] ASoC: mchp-spdifrx: add driver for SPDIF RX Dan Carpenter
2020-12-03 15:31 ` Codrin.Ciubotariu
2020-12-03 18:38 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.