Re: [PATCH v3] i2c: lpi2c: cache peripheral clock rate

From: Sverdlin, Alexander
Date: Mon May 15 2023 - 09:13:11 EST


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