| Message ID | 20230901073640.4973-1-quic_devipriy@quicinc.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f7b7d30158cff246667273bd2a62fc93ee0725d2 |
| Headers | show |
| Series | [V2] clk: qcom: clk-rcg2: Fix clock rate overflow for high parent frequencies | expand |
On 2023-09-01 13:06:40, Devi Priya wrote: > If the parent clock rate is greater than unsigned long max/2 then > integer overflow happens when calculating the clock rate on 32-bit systems. > As RCG2 uses half integer dividers, the clock rate is first being > multiplied by 2 which will overflow the unsigned long max value. > Hence, replace the common pattern of doing 64-bit multiplication Wasn't it doing 32-bit multiplication on 32-bit systems? Glad to see the u64 mul and div cleaned up in the if (mode) block either way though. > and then a do_div() call with simpler mult_frac call. mul_frac() > > Fixes: bcd61c0f535a ("clk: qcom: Add support for root clock generators (RCGs)") > Signed-off-by: Devi Priya <quic_devipriy@quicinc.com> > --- > Changes in V2: > - Replaced 64-bit multiplication & a do_div call with mult_frac > call as suggested by Marijn Suijten. Don't forget to add reviewers to CC on followup revisions :) Reviewed-by: Marijn Suijten <marijn.suijten@somainline.org> > - Updated the subject title > - Added Fixes tag > - Did not pick up the R-b tag due to the above changes. > > drivers/clk/qcom/clk-rcg2.c | 8 ++------ > 1 file changed, 2 insertions(+), 6 deletions(-) > > diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c > index a42f661550ca..f64d69164547 100644 > --- a/drivers/clk/qcom/clk-rcg2.c > +++ b/drivers/clk/qcom/clk-rcg2.c > @@ -159,15 +159,11 @@ static unsigned long > calc_rate(unsigned long rate, u32 m, u32 n, u32 mode, u32 hid_div) > { > if (hid_div) { > - rate *= 2; > - rate /= hid_div + 1; > + rate = mult_frac(rate, 2, hid_div + 1); > } > > if (mode) { > - u64 tmp = rate; > - tmp *= m; > - do_div(tmp, n); > - rate = tmp; > + rate = mult_frac(rate, m, n); > } > > return rate; > -- > 2.34.1 >
On Fri, 01 Sep 2023 13:06:40 +0530, Devi Priya wrote: > If the parent clock rate is greater than unsigned long max/2 then > integer overflow happens when calculating the clock rate on 32-bit systems. > As RCG2 uses half integer dividers, the clock rate is first being > multiplied by 2 which will overflow the unsigned long max value. > Hence, replace the common pattern of doing 64-bit multiplication > and then a do_div() call with simpler mult_frac call. > > [...] Applied, thanks! [1/1] clk: qcom: clk-rcg2: Fix clock rate overflow for high parent frequencies commit: f7b7d30158cff246667273bd2a62fc93ee0725d2 Best regards,
diff --git a/drivers/clk/qcom/clk-rcg2.c b/drivers/clk/qcom/clk-rcg2.c index a42f661550ca..f64d69164547 100644 --- a/drivers/clk/qcom/clk-rcg2.c +++ b/drivers/clk/qcom/clk-rcg2.c @@ -159,15 +159,11 @@ static unsigned long calc_rate(unsigned long rate, u32 m, u32 n, u32 mode, u32 hid_div) { if (hid_div) { - rate *= 2; - rate /= hid_div + 1; + rate = mult_frac(rate, 2, hid_div + 1); } if (mode) { - u64 tmp = rate; - tmp *= m; - do_div(tmp, n); - rate = tmp; + rate = mult_frac(rate, m, n); } return rate;
If the parent clock rate is greater than unsigned long max/2 then integer overflow happens when calculating the clock rate on 32-bit systems. As RCG2 uses half integer dividers, the clock rate is first being multiplied by 2 which will overflow the unsigned long max value. Hence, replace the common pattern of doing 64-bit multiplication and then a do_div() call with simpler mult_frac call. Fixes: bcd61c0f535a ("clk: qcom: Add support for root clock generators (RCGs)") Signed-off-by: Devi Priya <quic_devipriy@quicinc.com> --- Changes in V2: - Replaced 64-bit multiplication & a do_div call with mult_frac call as suggested by Marijn Suijten. - Updated the subject title - Added Fixes tag - Did not pick up the R-b tag due to the above changes. drivers/clk/qcom/clk-rcg2.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-)