DRA7 clock question

[Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>]

Hi Tero, Tony,

I accidentally stumbled across the following code in drivers/clk/ti/apll.c:

    static int dra7_apll_enable(struct clk_hw *hw)
    {
            struct clk_hw_omap *clk = to_clk_hw_omap(hw);
            int r = 0, i = 0;
            struct dpll_data *ad;
            const char *clk_name;
            u8 state = 1;
            u32 v;

            ad = clk->dpll_data;
            if (!ad)
                    return -EINVAL;

            clk_name = clk_hw_get_name(&clk->hw);

            state <<= __ffs(ad->idlest_mask);

state is shifted to its bit position...

            /* Check is already locked */
            v = ti_clk_ll_ops->clk_readl(&ad->idlest_reg);

            if ((v & ad->idlest_mask) == state)

... and checked.

                    return r;

            v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
            v &= ~ad->enable_mask;
            v |= APLL_FORCE_LOCK << __ffs(ad->enable_mask);
            ti_clk_ll_ops->clk_writel(v, &ad->control_reg);

            state <<= __ffs(ad->idlest_mask);

state is shifted again? ...

            while (1) {
                    v = ti_clk_ll_ops->clk_readl(&ad->idlest_reg);
                    if ((v & ad->idlest_mask) == state)

... and checked again?

                            break;
                    if (i > MAX_APLL_WAIT_TRIES)
                            break;
                    i++;
                    udelay(1);
            }

            if (i == MAX_APLL_WAIT_TRIES) {
                    pr_warn("clock: %s failed transition to '%s'\n",
                            clk_name, (state) ? "locked" : "bypassed");
                    r = -EBUSY;
            } else
                    pr_debug("clock: %s transition to '%s' in %d loops\n",
                             clk_name, (state) ? "locked" : "bypassed", i);

            return r;
    }

    static void dra7_apll_disable(struct clk_hw *hw)
    {
            struct clk_hw_omap *clk = to_clk_hw_omap(hw);
            struct dpll_data *ad;
            u8 state = 1;
            u32 v;

            ad = clk->dpll_data;

            state <<= __ffs(ad->idlest_mask);

state is shifted to its bit position, but it is never used below?
Perhaps one of the check blocks above should be moved here?

I checked git history and the original patch submissions, and even
the oldest submission I could find had the same logic.

            v = ti_clk_ll_ops->clk_readl(&ad->control_reg);
            v &= ~ad->enable_mask;
            v |= APLL_AUTO_IDLE << __ffs(ad->enable_mask);
            ti_clk_ll_ops->clk_writel(v, &ad->control_reg);
    }

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds