Re: [PATCH 4/4] clk: bcm2835: Make sure the PLL is gated before changing its rate

[Eric Anholt <eric@anholt.net>]

[-- Attachment #1: Type: text/plain, Size: 2161 bytes --]

Boris Brezillon <boris.brezillon@bootlin.com> writes:

> All bcm2835 PLLs should be gated before their rate can be changed.
> Setting CLK_SET_RATE_GATE will let the core enforce that, but this is
> not enough to make the code work in all situations. Indeed, the
> CLK_SET_RATE_GATE flag prevents a user from changing the rate while
> the clock is enabled, but this check only guarantees there's no Linux
> users. In our case, the clock might have been enabled by the
> bootloader/FW, and, because we have CLK_IGNORE_UNUSED set, Linux never
> disables the PLL. So we have to make sure the PLL is actually disabled
> before changing the rate.
>
> Fixes: 41691b8862e2 ("clk: bcm2835: Add support for programming the audio domain clocks")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Boris Brezillon <boris.brezillon@bootlin.com>
> ---
>  drivers/clk/bcm/clk-bcm2835.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c
> index 6c5d4a8e426c..051ce769c109 100644
> --- a/drivers/clk/bcm/clk-bcm2835.c
> +++ b/drivers/clk/bcm/clk-bcm2835.c
> @@ -678,6 +678,18 @@ static int bcm2835_pll_set_rate(struct clk_hw *hw,
>  	u32 ana[4];
>  	int i;
>  
> +	/*
> +	 * Normally, the CLK_SET_RATE_GATE flag prevents a user from changing
> +	 * the rate while the clock is enabled, but this check only makes sure
> +	 * there's no Linux users.
> +	 * In our case, the clock might have been enabled by the bootloader/FW,
> +	 * and, since CLK_IGNORE_UNUSED flag is set, Linux never disables it.
> +	 * So we have to make sure the clk is actually disabled before changing
> +	 * the rate.
> +	 */
> +	if (bcm2835_pll_is_on(hw))
> +		bcm2835_pll_off(hw);
> +

I'm not sure this improves the situation.  If the PLL was on, then
presumably there's a divider using it and a CM clock using that, so
we'll probably end up driving some glitches on them.

Does the common clk framework have a way to disable unused clocks from
the leaf clocks up to this root, before the general
disable-unused-clocks path happens late in the boot process?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]