Re: [PATCH v5 12/15] clk: tegra: move PMC, fixed clocks to common files

From: Andrew Bresticker
Date: Fri Oct 18 2013 - 20:01:33 EST


Hi Peter,

> + val = readl_relaxed(clk_base + OSC_CTRL);
> + osc_idx = val >> OSC_CTRL_OSC_FREQ_SHIFT;
> +
> + if (osc_idx < num)
> + *osc_freq = input_freqs[osc_idx];

else *osc_freq = 0?

> + clk = clk_register_fixed_factor(NULL, "pll_ref", "clk_m",
> + CLK_SET_RATE_PARENT, 1, pll_ref_div);

It seems silly to have CLK_SET_RATE_PARENT on a clock that can't have
its rate changed.

> + /* clk_32k */
> + dt_clk = tegra_lookup_dt_id(tegra_clk_clk_32k, tegra_clks);
> + if (dt_clk) {
> + clk = clk_register_fixed_rate(NULL, "clk_32k", NULL,
> + CLK_IS_ROOT, 32768);
> + clk_register_clkdev(clk, "clk_32k", NULL);

Like with the audio clocks, I think this will be done tegra_register_devclks().

> + *dt_clk = clk;
> + }
> +
> + /* clk_m_div2 */
> + dt_clk = tegra_lookup_dt_id(tegra_clk_clk_m_div2, tegra_clks);
> + if (dt_clk) {
> + clk = clk_register_fixed_factor(NULL, "clk_m_div2", "clk_m",
> + CLK_SET_RATE_PARENT, 1, 2);
> + clk_register_clkdev(clk, "clk_m_div2", NULL);

Ditto.

> + *dt_clk = clk;
> + }
> +
> + /* clk_m_div4 */
> + dt_clk = tegra_lookup_dt_id(tegra_clk_clk_m_div4, tegra_clks);
> + if (dt_clk) {
> + clk = clk_register_fixed_factor(NULL, "clk_m_div4", "clk_m",
> + CLK_SET_RATE_PARENT, 1, 4);
> + clk_register_clkdev(clk, "clk_m_div4", NULL);

Ditto.

> + clk = clk_register_gate(NULL, data->gate_name, data->mux_name,
> + 0, pmc_base + PMC_CLK_OUT_CNTRL,
> + data->gate_shift, 0, &clk_out_lock);
> + *dt_clk = clk;
> + clk_register_clkdev(clk, data->dev_name, data->gate_name);

Ditto.

> + }
> +
> + /* blink */
> + clk = clk_register_gate(NULL, "blink_override", "clk_32k", 0,
> + pmc_base + PMC_DPD_PADS_ORIDE,
> + PMC_DPD_PADS_ORIDE_BLINK_ENB, 0, NULL);
> +
> + dt_clk = tegra_lookup_dt_id(tegra_clk_blink, tegra_clks);
> + if (!dt_clk)
> + return;
> +

In tegra*_pmc_clk_init, there's a write to PMC_BLINK_TIMER here.
Why'd that disappear?

> + clk = clk_register_gate(NULL, "blink", "blink_override", 0,
> + pmc_base + PMC_CTRL,
> + PMC_CTRL_BLINK_ENB, 0, NULL);
> + clk_register_clkdev(clk, "blink", NULL);

Same comment about the other register_clkdev calls above.

> + *dt_clk = clk;
> +}

Thanks,
Andrew
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/