Hi Paul,
On Fri, Apr 24, 2015 at 02:17:25PM +0100, Paul Burton wrote:
> +static int ingenic_clk_set_parent(struct clk_hw *hw, u8 idx)
> +{
> + struct ingenic_clk *ingenic_clk = to_ingenic_clk(hw);
> + struct ingenic_cgu *cgu = ingenic_clk->cgu;
> + const struct ingenic_cgu_clk_info *clk_info;
> + unsigned long flags;
> + u8 curr_idx, hw_idx, num_poss;
> + u32 reg, mask;
> +
> + clk_info = &cgu->clock_info[ingenic_clk->idx];
> +
> + if (clk_info->type & CGU_CLK_MUX) {
> + /*
> + * Convert the parent index to the hardware index by adding
> + * 1 for any -1 in the parents array preceding the given
> + * index. That is, we want the index of idx'th entry in
> + * clk_info->parents which does not equal -1.
> + */
> + hw_idx = curr_idx = 0;
> + num_poss = 1 << clk_info->mux.bits;
> + for (; hw_idx < num_poss; hw_idx++) {
> + if (clk_info->parents[hw_idx] == -1)
> + continue;
> + if (curr_idx == idx)
> + break;
> + curr_idx++;
> + }
> +
> + /* idx should always be a valid parent */
> + BUG_ON(curr_idx != idx);
I think this needs updating.
If idx is the number of valid parents, this won't catch it, e.g. idx =
2, parents = { x, y, -1, -1 }, it'll increment curr_idx twice to 2, and
reach the end of the loop without finding the 3rd valid clock, but
curr_idx == idx == 2 ...
> +
> + mask = GENMASK(clk_info->mux.bits - 1, 0);
> + mask <<= clk_info->mux.shift;
> +
> + spin_lock_irqsave(&cgu->lock, flags);
> +
> + /* write the register */
> + reg = readl(cgu->base + clk_info->mux.reg);
> + reg &= ~mask;
> + reg |= hw_idx << clk_info->mux.shift;
> + writel(reg, cgu->base + clk_info->mux.reg);
> +
> + spin_unlock_irqrestore(&cgu->lock, flags);
> + return 0;
> + }
> +
> + return idx ? -EINVAL : 0;
> +}
Cheers
James
Attachment:
signature.asc
Description: Digital signature
