[PATCH v2 1/2] clk: meson: mpll: fix division by zero in rate_from_params
Neil Armstrong
narmstrong at baylibre.com
Fri Apr 7 08:39:18 PDT 2017
On 04/07/2017 05:34 PM, Jerome Brunet wrote:
> From: Martin Blumenstingl <martin.blumenstingl at googlemail.com>
>
> According to the public datasheet all register bits in HHI_MPLL_CNTL7,
> HHI_MPLL_CNTL8 and HHI_MPLL_CNTL9 default to zero. On all GX SoCs these
> seem to be initialized by the bootloader to some default value.
> However, on my Meson8 board they are not initialized, leading to a
> division by zero in rate_from_params as the math is:
> (parent_rate * SDM_DEN) / ((SDM_DEN * 0) + 0)
>
> According to the datasheet, the minimum n2 value is 4. The rate provided
> by the clock when n2 is less than this minimum is unpredictable. In such
> case, we report an error.
>
> Although the rate_from_params function was only introduced recently the
> original bug has been there for much longer. It was only exposed
> recently when the MPLL clocks were added to the Meson8b clock driver.
>
> Fixes: 1c50da4f27 ("clk: meson: add mpll support")
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl at googlemail.com>
> Signed-off-by: Jerome Brunet <jbrunet at baylibre.com>
> ---
> drivers/clk/meson/clk-mpll.c | 26 +++++++++++++++-----------
> 1 file changed, 15 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/clk/meson/clk-mpll.c b/drivers/clk/meson/clk-mpll.c
> index 540dabe5adad..d9462b505dcc 100644
> --- a/drivers/clk/meson/clk-mpll.c
> +++ b/drivers/clk/meson/clk-mpll.c
> @@ -65,18 +65,21 @@
> #include "clkc.h"
>
> #define SDM_DEN 16384
> -#define SDM_MIN 1
> -#define SDM_MAX 16383
> #define N2_MIN 4
> #define N2_MAX 511
>
> #define to_meson_clk_mpll(_hw) container_of(_hw, struct meson_clk_mpll, hw)
>
> -static unsigned long rate_from_params(unsigned long parent_rate,
> +static long rate_from_params(unsigned long parent_rate,
> unsigned long sdm,
> unsigned long n2)
> {
> - return (parent_rate * SDM_DEN) / ((SDM_DEN * n2) + sdm);
> + unsigned long divisor = (SDM_DEN * n2) + sdm;
> +
> + if (n2 < N2_MIN)
> + return -EINVAL;
> +
> + return (parent_rate * SDM_DEN) / divisor;
> }
>
> static void params_from_rate(unsigned long requested_rate,
> @@ -89,17 +92,13 @@ static void params_from_rate(unsigned long requested_rate,
>
> if (div < N2_MIN) {
> *n2 = N2_MIN;
> - *sdm = SDM_MIN;
> + *sdm = 0;
> } else if (div > N2_MAX) {
> *n2 = N2_MAX;
> - *sdm = SDM_MAX;
> + *sdm = SDM_DEN - 1;
> } else {
> *n2 = div;
> *sdm = DIV_ROUND_UP(rem * SDM_DEN, requested_rate);
> - if (*sdm < SDM_MIN)
> - *sdm = SDM_MIN;
> - else if (*sdm > SDM_MAX)
> - *sdm = SDM_MAX;
> }
> }
>
> @@ -109,6 +108,7 @@ static unsigned long mpll_recalc_rate(struct clk_hw *hw,
> struct meson_clk_mpll *mpll = to_meson_clk_mpll(hw);
> struct parm *p;
> unsigned long reg, sdm, n2;
> + long rate;
>
> p = &mpll->sdm;
> reg = readl(mpll->base + p->reg_off);
> @@ -118,7 +118,11 @@ static unsigned long mpll_recalc_rate(struct clk_hw *hw,
> reg = readl(mpll->base + p->reg_off);
> n2 = PARM_GET(p->width, p->shift, reg);
>
> - return rate_from_params(parent_rate, sdm, n2);
> + rate = rate_from_params(parent_rate, sdm, n2);
> + if (rate < 0)
> + return 0;
> +
> + return rate;
> }
>
> static long mpll_round_rate(struct clk_hw *hw,
>
Reviewed-by: Neil Armstrong <narmstrong at baylibre.com>
More information about the linux-amlogic
mailing list