| Message ID | 22f1d799-a3bb-3d71-a3fd-f6128b205231@gmail.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | clk: meson: pll: adjust timeout in meson_clk_pll_wait_lock() | expand |
On Sun 14 Aug 2022 at 23:29, Heiner Kallweit <hkallweit1@gmail.com> wrote: > Currently we loop over meson_parm_read() up to 24mln times. > This results in a unpredictable timeout period. In my case > it's over 5s on a S905X4-based system. Make the timeout > period predictable and set it to 100ms. > > Whilst we're at it: All callers of this function return -EIO > in case of failure, therefore we can return this value directly > in the timeout case. I'm okay with this change but I'd prefer if one change addressed a single topic. Please split this out. > > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/clk/meson/clk-pll.c | 15 ++++++--------- > 1 file changed, 6 insertions(+), 9 deletions(-) > > diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c > index daa025b6d..d70bee331 100644 > --- a/drivers/clk/meson/clk-pll.c > +++ b/drivers/clk/meson/clk-pll.c > @@ -277,17 +277,17 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw) > { > struct clk_regmap *clk = to_clk_regmap(hw); > struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); > - int delay = 24000000; > + int delay = 5000; > > do { > - /* Is the clock locked now ? */ > + /* Is the clock locked now ? Time out after 100ms. */ > if (meson_parm_read(clk->map, &pll->l)) > return 0; > > - delay--; > - } while (delay > 0); > + udelay(20); > + } while (--delay); > > - return -ETIMEDOUT; > + return -EIO; > } > > static int meson_clk_pll_init(struct clk_hw *hw) > @@ -350,10 +350,7 @@ static int meson_clk_pll_enable(struct clk_hw *hw) > /* Take the pll out reset */ > meson_parm_write(clk->map, &pll->rst, 0); > > - if (meson_clk_pll_wait_lock(hw)) > - return -EIO; > - > - return 0; > + return meson_clk_pll_wait_lock(hw); > } > > static void meson_clk_pll_disable(struct clk_hw *hw)
On 29.08.2022 11:52, Jerome Brunet wrote: > > On Sun 14 Aug 2022 at 23:29, Heiner Kallweit <hkallweit1@gmail.com> wrote: > >> Currently we loop over meson_parm_read() up to 24mln times. >> This results in a unpredictable timeout period. In my case >> it's over 5s on a S905X4-based system. Make the timeout >> period predictable and set it to 100ms. >> >> Whilst we're at it: All callers of this function return -EIO >> in case of failure, therefore we can return this value directly >> in the timeout case. > > I'm okay with this change but I'd prefer if one change addressed a > single topic. Please split this out. > OK, I'll send a v2. >> >> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> >> --- >> drivers/clk/meson/clk-pll.c | 15 ++++++--------- >> 1 file changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c >> index daa025b6d..d70bee331 100644 >> --- a/drivers/clk/meson/clk-pll.c >> +++ b/drivers/clk/meson/clk-pll.c >> @@ -277,17 +277,17 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw) >> { >> struct clk_regmap *clk = to_clk_regmap(hw); >> struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); >> - int delay = 24000000; >> + int delay = 5000; >> >> do { >> - /* Is the clock locked now ? */ >> + /* Is the clock locked now ? Time out after 100ms. */ >> if (meson_parm_read(clk->map, &pll->l)) >> return 0; >> >> - delay--; >> - } while (delay > 0); >> + udelay(20); >> + } while (--delay); >> >> - return -ETIMEDOUT; >> + return -EIO; >> } >> >> static int meson_clk_pll_init(struct clk_hw *hw) >> @@ -350,10 +350,7 @@ static int meson_clk_pll_enable(struct clk_hw *hw) >> /* Take the pll out reset */ >> meson_parm_write(clk->map, &pll->rst, 0); >> >> - if (meson_clk_pll_wait_lock(hw)) >> - return -EIO; >> - >> - return 0; >> + return meson_clk_pll_wait_lock(hw); >> } >> >> static void meson_clk_pll_disable(struct clk_hw *hw) >
diff --git a/drivers/clk/meson/clk-pll.c b/drivers/clk/meson/clk-pll.c index daa025b6d..d70bee331 100644 --- a/drivers/clk/meson/clk-pll.c +++ b/drivers/clk/meson/clk-pll.c @@ -277,17 +277,17 @@ static int meson_clk_pll_wait_lock(struct clk_hw *hw) { struct clk_regmap *clk = to_clk_regmap(hw); struct meson_clk_pll_data *pll = meson_clk_pll_data(clk); - int delay = 24000000; + int delay = 5000; do { - /* Is the clock locked now ? */ + /* Is the clock locked now ? Time out after 100ms. */ if (meson_parm_read(clk->map, &pll->l)) return 0; - delay--; - } while (delay > 0); + udelay(20); + } while (--delay); - return -ETIMEDOUT; + return -EIO; } static int meson_clk_pll_init(struct clk_hw *hw) @@ -350,10 +350,7 @@ static int meson_clk_pll_enable(struct clk_hw *hw) /* Take the pll out reset */ meson_parm_write(clk->map, &pll->rst, 0); - if (meson_clk_pll_wait_lock(hw)) - return -EIO; - - return 0; + return meson_clk_pll_wait_lock(hw); } static void meson_clk_pll_disable(struct clk_hw *hw)
Currently we loop over meson_parm_read() up to 24mln times. This results in a unpredictable timeout period. In my case it's over 5s on a S905X4-based system. Make the timeout period predictable and set it to 100ms. Whilst we're at it: All callers of this function return -EIO in case of failure, therefore we can return this value directly in the timeout case. Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/clk/meson/clk-pll.c | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-)