Quoting Geert Uytterhoeven (2021-03-30 00:02:03)
> Hi Stephen,
>
> On Tue, Mar 30, 2021 at 4:22 AM Stephen Boyd <sboyd@xxxxxxxxxx> wrote:
> > Quoting Geert Uytterhoeven (2021-03-26 05:00:59)
> > > +}
> > > +
> > > +static int cpg_pll_clk_set_rate(struct clk_hw *hw, unsigned long rate,
> > > + unsigned long parent_rate)
> > > +{
> > > + struct cpg_pll_clk *pll_clk = to_pll_clk(hw);
> > > + unsigned int mult, i;
> > > + u32 val;
> > > +
> > > + mult = DIV_ROUND_CLOSEST_ULL(rate, parent_rate * pll_clk->fixed_mult);
> > > + mult = clamp(mult, 1U, 128U);
> > > +
> > > + val = readl(pll_clk->pllcr_reg);
> > > + val &= ~CPG_PLLnCR_STC_MASK;
> > > + val |= (mult - 1) << __ffs(CPG_PLLnCR_STC_MASK);
> > > + writel(val, pll_clk->pllcr_reg);
> > > +
> > > + for (i = 1000; i; i--) {
> > > + if (readl(pll_clk->pllecr_reg) & pll_clk->pllecr_pllst_mask)
> > > + return 0;
> > > +
> > > + cpu_relax();
> > > + }
> >
> > Why not readl_poll_timeout()?
>
> I had considered that. But then I noticed readl_poll_timeout() does not
> call cpu_relax() if sleep_us == 0. Perhaps such a call should be added
> (at the risk of introducing a subtle regression elsewhere)?
>
Risk seems low. Maybe introduce a readl_poll_timeout_relaxed() if it's a
concern. At the least, a patch to document why it doesn't use
cpu_relax() right now would be good to have.
