Hi Eric,
Am 02.10.2015 um 21:54 schrieb Eric Anholt:
This adds support for enabling, disabling, and setting the rate of the
audio domain clocks. It will be necessary for setting the pixel clock
for HDMI in the VC4 driver and let us write a cpufreq driver. It will
also improve compatibility with user changes to the firmware's
config.txt, since our previous fixed clocks are unaware of it.
The firmware also has support for configuring the clocks through the
mailbox channel, but the pixel clock setup by the firmware doesn't
work, and it's Raspberry Pi specific anyway. The only conflicts we
should have with the firmware would be if we made firmware calls that
result in clock management (like opening firmware V3D or ISP access,
which we don't support in upstream), or on hardware over-thermal or
under-voltage (when the firmware would rewrite PLLB to take the ARM
out of overclock). If that happens, our cached .recalc_rate() results
would be incorrect, but that's no worse than our current state where
we used fixed clocks.
The existing fixed clocks in the code are left in place to provide
backwards compatibility with old device tree files.
only a few nits.
Signed-off-by: Eric Anholt <eric@xxxxxxxxxx>
Tested-by: Martin Sperl <kernel@xxxxxxxxxxxxxxxx>
---
This is the remaining driver patch to go on the clock tree's
clk-bcm2385 (oops, spelling :) ) tree for the bcm2835 driver.
v2: Fix onecell->clks[] allocation size.
v3: '/*' on otherwise-empty line for multiline comments, fix top
comment, use more named initializers, do fewer separate
allocations on probe, unwind allocations on failure in probe (from
review by Stephen Warren). Use new clk_hw_get_name(). Switch
fb_prediv_bit to be fb_prediv_mask to avoid typing BIT() so many
times.
v4: Incorporate feedback from Stephen Boyd, and use devm_kasprintf instead
of bare kasprintf in driver init.
drivers/clk/bcm/clk-bcm2835.c | 1509 ++++++++++++++++++++++++++++++++++++++++-
1 file changed, 1508 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index dd295e4..9498fd9 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
[...]
+/*
+ * PLLA is the auxiliary PLL, used to drive the CCP2 (Compact Camera
+ * Port 2) transmitter clock.
+ *
+ * It is in the PX LDO power domain, which is on when the AUDIO domain
+ * is on.
+ */
+static const struct bcm2835_pll_data bcm2835_plla_data = {
+ .name = "plla",
+ .cm_ctrl_reg = CM_PLLA,
+ .a2w_ctrl_reg = A2W_PLLA_CTRL,
+ .frac_reg = A2W_PLLA_FRAC,
+ .ana_reg_base = A2W_PLLA_ANA0,
+ .reference_enable_mask = A2W_XOSC_CTRL_PLLA_ENABLE,
+ .lock_mask = CM_LOCK_FLOCKA,
+
+ .ana = &bcm2835_ana_default,
+
+ .min_rate = 600000000u,
+ .max_rate = 2400000000u,
+ .max_fb_rate = 1750000000u,
How about using a define for the max_fb_rate and use it for all PLLs?
+ [...]
+static int bcm2835_pll_set_rate(struct clk_hw *hw,
+ unsigned long rate, unsigned long parent_rate)
+{
+ struct bcm2835_pll *pll = container_of(hw, struct bcm2835_pll, hw);
+ struct bcm2835_cprman *cprman = pll->cprman;
+ const struct bcm2835_pll_data *data = pll->data;
+ bool was_using_prediv, use_fb_prediv, do_ana_setup_first;
+ u32 ndiv, fdiv, pdiv = 1, a2w_ctl;
+ u32 ana[4];
+ int i;
+
+ if (rate < data->min_rate || rate > data->max_rate) {
+ dev_err(cprman->dev, "%s: rate out of spec: %ld vs (%ld, %ld)\n",
The format specifier looks wrong to me:
s/%ld/%lu
+ clk_hw_get_name(hw), rate,
+ data->min_rate, data->max_rate);
+ return -EINVAL;
+ }
+
+ if (rate > data->max_fb_rate) {
+ use_fb_prediv = true;
+ rate /= 2;
+ } else {
+ use_fb_prediv = false;
+ }
+
+ bcm2835_pll_choose_ndiv_and_fdiv(rate, parent_rate, &ndiv, &fdiv);
+
+ for (i = 3; i >= 0; i--)
+ ana[i] = cprman_read(cprman, data->ana_reg_base + i * 4);
+
+ was_using_prediv = ana[1] & data->ana->fb_prediv_mask;
+
+ ana[0] &= ~data->ana->mask0;
+ ana[0] |= data->ana->set0;
+ ana[1] &= ~data->ana->mask1;
+ ana[1] |= data->ana->set1;
+ ana[3] &= ~data->ana->mask3;
+ ana[3] |= data->ana->set3;
+
+ if (was_using_prediv && !use_fb_prediv) {
+ ana[1] &= ~data->ana->fb_prediv_mask;
+ do_ana_setup_first = true;
+ } else if (!was_using_prediv && use_fb_prediv) {
+ ana[1] |= data->ana->fb_prediv_mask;
+ do_ana_setup_first = false;
+ } else {
+ do_ana_setup_first = true;
+ }
+
+ /* Unmask the reference clock from the oscillator. */
+ cprman_write(cprman, A2W_XOSC_CTRL,
+ cprman_read(cprman, A2W_XOSC_CTRL) |
+ data->reference_enable_mask);
+
+ if (do_ana_setup_first)
+ bcm2835_pll_write_ana(cprman, data->ana_reg_base, ana);
+
+ /* Set the PLL multiplier from the oscillator. */
+ cprman_write(cprman, data->frac_reg, fdiv);
+
+ a2w_ctl = cprman_read(cprman, data->a2w_ctrl_reg);
+ a2w_ctl &= ~A2W_PLL_CTRL_NDIV_MASK;
+ a2w_ctl |= ndiv << A2W_PLL_CTRL_NDIV_SHIFT;
+ a2w_ctl &= ~A2W_PLL_CTRL_PDIV_MASK;
+ a2w_ctl |= pdiv << A2W_PLL_CTRL_PDIV_SHIFT;
Looks like pdiv is never changed and could be replaced by 1.
+ cprman_write(cprman, data->a2w_ctrl_reg, a2w_ctl);
+
+ if (!do_ana_setup_first)
+ bcm2835_pll_write_ana(cprman, data->ana_reg_base, ana);
+
+ bcm2835_pll_get_rate(&pll->hw, parent_rate);
+
+ return 0;
+}
+
+ [...]
+static int bcm2835_pll_divider_set_rate(struct clk_hw *hw,
+ unsigned long rate,
+ unsigned long parent_rate)
+{
+ struct bcm2835_pll_divider *divider = bcm2835_pll_divider_from_hw(hw);
+ struct bcm2835_cprman *cprman = divider->cprman;
+ const struct bcm2835_pll_divider_data *data = divider->data;
+ u32 cm;
+
+ clk_divider_ops.set_rate(hw, rate, parent_rate);
Is it safe to ignore the return value here?
+
+ cm = cprman_read(cprman, data->cm_reg);
+ cprman_write(cprman, data->cm_reg, cm | data->load_mask);
+ cprman_write(cprman, data->cm_reg, cm & ~data->load_mask);
+
+ return 0;
+}
+
+ [...]
+static struct platform_driver bcm2835_clk_driver = {
+ .driver = {
+ .name = "bcm2835-clk",
+ .of_match_table = bcm2835_clk_of_match,
+ },
+ .probe = bcm2835_clk_probe,
+};
checkpatch.pl --strict suggests a blank line after the struct.
+builtin_platform_driver(bcm2835_clk_driver);
+
+MODULE_AUTHOR("Eric Anholt <eric@xxxxxxxxxx>");
+MODULE_DESCRIPTION("BCM2835 clock driver");
+MODULE_LICENSE("GPL v2");
Thanks
Stefan
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
