2016-04-26 19:39:50

by Eric Anholt

[permalink] [raw]
Subject: [PATCH 1/2] clk: bcm2835: Mark the VPU clock as critical

The VPU clock is also the clock for our AXI bus, so we really can't
disable it. This might have happened during boot if, for example,
uart1 (aux_uart clock) probed and was then disabled before the other
consumers of the VPU clock had probed.

Signed-off-by: Eric Anholt <[email protected]>
---
drivers/clk/bcm/clk-bcm2835.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 5a7e3eca5d12..14f3066194ac 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1267,6 +1267,7 @@ static struct clk *bcm2835_register_clock(struct bcm2835_cprman *cprman,

if (data->is_vpu_clock) {
init.ops = &bcm2835_vpu_clock_clk_ops;
+ init.flags |= CLK_IS_CRITICAL;
} else {
init.ops = &bcm2835_clock_clk_ops;
init.flags |= CLK_SET_RATE_GATE | CLK_SET_PARENT_GATE;
--
2.8.0.rc3


2016-04-26 19:39:55

by Eric Anholt

[permalink] [raw]
Subject: [PATCH 2/2] clk: bcm2835: Skip PLLC clocks when deciding on a new clock parent

If the firmware had set up a clock to source from PLLC, go along with
it. But if we're looking for a new parent, we don't want to switch it
to PLLC because the firmware will force PLLC (and thus the AXI bus
clock) to different frequencies during over-temp/under-voltage,
without notification to Linux.

On my system, this moves the Linux-enabled HDMI state machine and DSI1
escape clock over to plld_per from pllc_per. EMMC still ends up on
pllc_per, because the firmware had set it up to use that.

Signed-off-by: Eric Anholt <[email protected]>
Fixes: 41691b8862e2 ("clk: bcm2835: Add support for programming the audio domain clocks")
---
drivers/clk/bcm/clk-bcm2835.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
index 14f3066194ac..870c5745d649 100644
--- a/drivers/clk/bcm/clk-bcm2835.c
+++ b/drivers/clk/bcm/clk-bcm2835.c
@@ -1035,16 +1035,28 @@ static int bcm2835_clock_set_rate(struct clk_hw *hw,
return 0;
}

+static bool
+bcm2835_clk_is_pllc(struct clk_hw *hw)
+{
+ if (!hw)
+ return false;
+
+ return strncmp(clk_hw_get_name(hw), "pllc", 4) == 0;
+}
+
static int bcm2835_clock_determine_rate(struct clk_hw *hw,
struct clk_rate_request *req)
{
struct bcm2835_clock *clock = bcm2835_clock_from_hw(hw);
struct clk_hw *parent, *best_parent = NULL;
+ bool current_parent_is_pllc;
unsigned long rate, best_rate = 0;
unsigned long prate, best_prate = 0;
size_t i;
u32 div;

+ current_parent_is_pllc = bcm2835_clk_is_pllc(clk_hw_get_parent(hw));
+
/*
* Select parent clock that results in the closest but lower rate
*/
@@ -1052,6 +1064,17 @@ static int bcm2835_clock_determine_rate(struct clk_hw *hw,
parent = clk_hw_get_parent_by_index(hw, i);
if (!parent)
continue;
+
+ /*
+ * Don't choose a PLLC-derived clock as our parent
+ * unless it had been manually set that way. PLLC's
+ * frequency gets adjusted by the firmware due to
+ * over-temp or under-voltage conditions, without
+ * prior notification to our clock consumer.
+ */
+ if (bcm2835_clk_is_pllc(parent) && !current_parent_is_pllc)
+ continue;
+
prate = clk_hw_get_rate(parent);
div = bcm2835_clock_choose_div(hw, req->rate, prate, true);
rate = bcm2835_clock_rate_from_divisor(clock, prate, div);
--
2.8.0.rc3

2016-04-30 09:28:30

by Martin Sperl

[permalink] [raw]
Subject: Re: [PATCH 2/2] clk: bcm2835: Skip PLLC clocks when deciding on a new clock parent


> On 26.04.2016, at 21:39, Eric Anholt <[email protected]> wrote:
>
> If the firmware had set up a clock to source from PLLC, go along with
> it. But if we're looking for a new parent, we don't want to switch it
> to PLLC because the firmware will force PLLC (and thus the AXI bus
> clock) to different frequencies during over-temp/under-voltage,
> without notification to Linux.
>
> On my system, this moves the Linux-enabled HDMI state machine and DSI1
> escape clock over to plld_per from pllc_per. EMMC still ends up on
> pllc_per, because the firmware had set it up to use that.
>
> Signed-off-by: Eric Anholt <[email protected]>
> Fixes: 41691b8862e2 ("clk: bcm2835: Add support for programming the audio domain clocks")
> —

I guess this patch looks to me as if it is a policy inside the kernel,
which is AFAIK frowned upon.

I am looking into making "assigned-clock-parents” inside the dt
work with the driver.

Could look something like this:
i2s: i2s@7e203000 {
assigned-clock-parents = <&cprman BCM2835_PLLD_PER>, <&clk_osc>;
assigned-clocks = <&cprman BCM2835_CLOCK_PCM>, <&cprman BCM2835_CLOCK_PCM>;
};
(not sure if that works really - the same clock in assigned-clocks looks suspicious)

This would move the policy out of the kernel into the device-tree,
which - i guess is a better solution.

Martin