Re: [PATCH] clk: fractional-divider: cast parent_rate to u64 before multiplying
Hi Mike, Am Dienstag, 2. September 2014, 09:33:21 schrieb Heiko Stübner: > Am Montag, 1. September 2014, 17:26:29 schrieb Mike Turquette: > > Quoting Heiko Stübner (2014-08-28 03:46:10) > > > > > On 32bit architectures, like ARM calculating the fractional rate will > > > do the multiplication before converting the value to u64 when it gets > > > assigned to ret, which can produce overflows. > > > > > > The error in question happened with a parent_rate of 386MHz, m = 3000, > > > n = 6, which resulted in a wrong rate value of 15812Hz. > > > > > > Therefore cast parent_rate to u64 to make sure the multiplication > > > happens in a 64bit space and produces the correct 192MHz in the example. > > > > > > Signed-off-by: Heiko Stuebner > > > > Looks good to me. Have you observed this issue using the vanilla > > kernel.org sources or on a downstream tree? I'm trying to decide if I > > should take this into clk-fixes or clk-next. > > > > If this doesn't happen in practice on any merged platform then I'll take > > it for 3.18. > > It happens on the Rockchip platform with the newly added fractional branches > you just applied ... so it doesn't look like anybody else is affected right > now. will you take this one into clk-next? Thanks Heiko > > > Heiko > > > Regards, > > Mike > > > > > --- > > > > > > drivers/clk/clk-fractional-divider.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/clk/clk-fractional-divider.c > > > b/drivers/clk/clk-fractional-divider.c index ede685c..82a59d0 100644 > > > --- a/drivers/clk/clk-fractional-divider.c > > > +++ b/drivers/clk/clk-fractional-divider.c > > > @@ -36,7 +36,7 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw > > > *hw,> > > > > > > m = (val & fd->mmask) >> fd->mshift; > > > n = (val & fd->nmask) >> fd->nshift; > > > > > > - ret = parent_rate * m; > > > + ret = (u64)parent_rate * m; > > > > > > do_div(ret, n); > > > > > > return ret; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: fractional-divider: cast parent_rate to u64 before multiplying
Hi Mike, Am Dienstag, 2. September 2014, 09:33:21 schrieb Heiko Stübner: Am Montag, 1. September 2014, 17:26:29 schrieb Mike Turquette: Quoting Heiko Stübner (2014-08-28 03:46:10) On 32bit architectures, like ARM calculating the fractional rate will do the multiplication before converting the value to u64 when it gets assigned to ret, which can produce overflows. The error in question happened with a parent_rate of 386MHz, m = 3000, n = 6, which resulted in a wrong rate value of 15812Hz. Therefore cast parent_rate to u64 to make sure the multiplication happens in a 64bit space and produces the correct 192MHz in the example. Signed-off-by: Heiko Stuebner he...@sntech.de Looks good to me. Have you observed this issue using the vanilla kernel.org sources or on a downstream tree? I'm trying to decide if I should take this into clk-fixes or clk-next. If this doesn't happen in practice on any merged platform then I'll take it for 3.18. It happens on the Rockchip platform with the newly added fractional branches you just applied ... so it doesn't look like anybody else is affected right now. will you take this one into clk-next? Thanks Heiko Heiko Regards, Mike --- drivers/clk/clk-fractional-divider.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c index ede685c..82a59d0 100644 --- a/drivers/clk/clk-fractional-divider.c +++ b/drivers/clk/clk-fractional-divider.c @@ -36,7 +36,7 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw, m = (val fd-mmask) fd-mshift; n = (val fd-nmask) fd-nshift; - ret = parent_rate * m; + ret = (u64)parent_rate * m; do_div(ret, n); return ret; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: fractional-divider: cast parent_rate to u64 before multiplying
Am Montag, 1. September 2014, 17:26:29 schrieb Mike Turquette: > Quoting Heiko Stübner (2014-08-28 03:46:10) > > > On 32bit architectures, like ARM calculating the fractional rate will > > do the multiplication before converting the value to u64 when it gets > > assigned to ret, which can produce overflows. > > > > The error in question happened with a parent_rate of 386MHz, m = 3000, > > n = 6, which resulted in a wrong rate value of 15812Hz. > > > > Therefore cast parent_rate to u64 to make sure the multiplication > > happens in a 64bit space and produces the correct 192MHz in the example. > > > > Signed-off-by: Heiko Stuebner > > Looks good to me. Have you observed this issue using the vanilla > kernel.org sources or on a downstream tree? I'm trying to decide if I > should take this into clk-fixes or clk-next. > > If this doesn't happen in practice on any merged platform then I'll take > it for 3.18. It happens on the Rockchip platform with the newly added fractional branches you just applied ... so it doesn't look like anybody else is affected right now. Heiko > > Regards, > Mike > > > --- > > > > drivers/clk/clk-fractional-divider.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/clk/clk-fractional-divider.c > > b/drivers/clk/clk-fractional-divider.c index ede685c..82a59d0 100644 > > --- a/drivers/clk/clk-fractional-divider.c > > +++ b/drivers/clk/clk-fractional-divider.c > > @@ -36,7 +36,7 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw > > *hw,> > > m = (val & fd->mmask) >> fd->mshift; > > n = (val & fd->nmask) >> fd->nshift; > > > > - ret = parent_rate * m; > > + ret = (u64)parent_rate * m; > > > > do_div(ret, n); > > > > return ret; -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] clk: fractional-divider: cast parent_rate to u64 before multiplying
Am Montag, 1. September 2014, 17:26:29 schrieb Mike Turquette: Quoting Heiko Stübner (2014-08-28 03:46:10) On 32bit architectures, like ARM calculating the fractional rate will do the multiplication before converting the value to u64 when it gets assigned to ret, which can produce overflows. The error in question happened with a parent_rate of 386MHz, m = 3000, n = 6, which resulted in a wrong rate value of 15812Hz. Therefore cast parent_rate to u64 to make sure the multiplication happens in a 64bit space and produces the correct 192MHz in the example. Signed-off-by: Heiko Stuebner he...@sntech.de Looks good to me. Have you observed this issue using the vanilla kernel.org sources or on a downstream tree? I'm trying to decide if I should take this into clk-fixes or clk-next. If this doesn't happen in practice on any merged platform then I'll take it for 3.18. It happens on the Rockchip platform with the newly added fractional branches you just applied ... so it doesn't look like anybody else is affected right now. Heiko Regards, Mike --- drivers/clk/clk-fractional-divider.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c index ede685c..82a59d0 100644 --- a/drivers/clk/clk-fractional-divider.c +++ b/drivers/clk/clk-fractional-divider.c @@ -36,7 +36,7 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw, m = (val fd-mmask) fd-mshift; n = (val fd-nmask) fd-nshift; - ret = parent_rate * m; + ret = (u64)parent_rate * m; do_div(ret, n); return ret; -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clk: fractional-divider: cast parent_rate to u64 before multiplying
On 32bit architectures, like ARM calculating the fractional rate will do the multiplication before converting the value to u64 when it gets assigned to ret, which can produce overflows. The error in question happened with a parent_rate of 386MHz, m = 3000, n = 6, which resulted in a wrong rate value of 15812Hz. Therefore cast parent_rate to u64 to make sure the multiplication happens in a 64bit space and produces the correct 192MHz in the example. Signed-off-by: Heiko Stuebner --- drivers/clk/clk-fractional-divider.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c index ede685c..82a59d0 100644 --- a/drivers/clk/clk-fractional-divider.c +++ b/drivers/clk/clk-fractional-divider.c @@ -36,7 +36,7 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw, m = (val & fd->mmask) >> fd->mshift; n = (val & fd->nmask) >> fd->nshift; - ret = parent_rate * m; + ret = (u64)parent_rate * m; do_div(ret, n); return ret; -- 2.0.1 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] clk: fractional-divider: cast parent_rate to u64 before multiplying
On 32bit architectures, like ARM calculating the fractional rate will do the multiplication before converting the value to u64 when it gets assigned to ret, which can produce overflows. The error in question happened with a parent_rate of 386MHz, m = 3000, n = 6, which resulted in a wrong rate value of 15812Hz. Therefore cast parent_rate to u64 to make sure the multiplication happens in a 64bit space and produces the correct 192MHz in the example. Signed-off-by: Heiko Stuebner he...@sntech.de --- drivers/clk/clk-fractional-divider.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/clk-fractional-divider.c b/drivers/clk/clk-fractional-divider.c index ede685c..82a59d0 100644 --- a/drivers/clk/clk-fractional-divider.c +++ b/drivers/clk/clk-fractional-divider.c @@ -36,7 +36,7 @@ static unsigned long clk_fd_recalc_rate(struct clk_hw *hw, m = (val fd-mmask) fd-mshift; n = (val fd-nmask) fd-nshift; - ret = parent_rate * m; + ret = (u64)parent_rate * m; do_div(ret, n); return ret; -- 2.0.1 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
