Hello Alexander,
On Fri, 2023-04-21 at 15:48 +0200, Alexander Stein wrote:
> > + lpi2c_imx->clk_change_nb.notifier_call =
> > lpi2c_imx_clk_change_cb;
> > + ret = devm_clk_notifier_register(&pdev->dev, lpi2c_imx-
> > >clks[0].clk,
> > + &lpi2c_imx-
> > >clk_change_nb);
> > + if (ret)
> > + return dev_err_probe(&pdev->dev, ret,
> > + "can't register peripheral
> > clock
> notifier\n");
> > + lpi2c_imx->rate_per = clk_get_rate(lpi2c_imx->clks[0].clk);
> > + if (!lpi2c_imx->rate_per) {
> > + dev_err(&pdev->dev, "can't get I2C peripheral clock
> rate\n");
> > + return -EINVAL;
> > + }
> > +
>
> I would switch the order of the calls to devm_clk_notifier_register()
> and
> clk_get_rate(). AFAICS this looks like a possible lost update. The
> notifier
> might change rate_per before the (old) value from clk_get_rate is
> actually
> assigned.
just swapping would not be enough I believe, in the case update event
happens between clk_get_rate() and devm_clk_notifier_register(). I'll
think about it...
--
Alexander Sverdlin
Siemens AG
www.siemens.com
