| Message ID | 4d2e3a91dfb74209735c940b51d7efc9ba2ed69b.1476267249.git.emil@limesaudio.com (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Stephen Boyd |
| Headers | show |
On Wed, Oct 12, 2016 at 7:31 AM, Emil Lundmark <emil@limesaudio.com> wrote: > Since 'parent_rate * mfn' may overflow 32 bits, the result should be > stored using 64 bits. > > The problem was discovered when trying to set the rate of the audio PLL > (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but > the actual rate returned was 192.000570 MHz. The round rate function should > have been able to return 196.608 MHz, i.e., the desired rate. > > Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula") > Cc: Anson Huang <b20788@freescale.com> > Signed-off-by: Emil Lundmark <emil@limesaudio.com> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com> -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/12, Emil Lundmark wrote: > Since 'parent_rate * mfn' may overflow 32 bits, the result should be > stored using 64 bits. > > The problem was discovered when trying to set the rate of the audio PLL > (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but > the actual rate returned was 192.000570 MHz. The round rate function should > have been able to return 196.608 MHz, i.e., the desired rate. > > Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula") > Cc: Anson Huang <b20788@freescale.com> > Signed-off-by: Emil Lundmark <emil@limesaudio.com> > --- Applied to clk-next
Hi Stephen, On Thu, Oct 27, 2016 at 11:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > On 10/12, Emil Lundmark wrote: >> Since 'parent_rate * mfn' may overflow 32 bits, the result should be >> stored using 64 bits. >> >> The problem was discovered when trying to set the rate of the audio PLL >> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but >> the actual rate returned was 192.000570 MHz. The round rate function should >> have been able to return 196.608 MHz, i.e., the desired rate. >> >> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula") >> Cc: Anson Huang <b20788@freescale.com> >> Signed-off-by: Emil Lundmark <emil@limesaudio.com> >> --- > > Applied to clk-next This one fixes a regression caused by ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula"). So it should go to clk-fixes instead with the stable tag: Cc: <stable@vger.kernel.org> # 4.8.x Thanks -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/28, Fabio Estevam wrote: > On Thu, Oct 27, 2016 at 11:41 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > > On 10/12, Emil Lundmark wrote: > >> Since 'parent_rate * mfn' may overflow 32 bits, the result should be > >> stored using 64 bits. > >> > >> The problem was discovered when trying to set the rate of the audio PLL > >> (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but > >> the actual rate returned was 192.000570 MHz. The round rate function should > >> have been able to return 196.608 MHz, i.e., the desired rate. > >> > >> Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula") > >> Cc: Anson Huang <b20788@freescale.com> > >> Signed-off-by: Emil Lundmark <emil@limesaudio.com> > >> --- > > > > Applied to clk-next > > This one fixes a regression caused by ba7f4f557eb6 ("clk: imx: correct > AV PLL rate formula"). > > So it should go to clk-fixes instead with the stable tag: > > Cc: <stable@vger.kernel.org> # 4.8.x > The clk-fixes branch is for patches that fix problems in code merged during the merge window as well as small one-liners and things that are causing great pain for people on the latest release. Given that this fixes a regression in v4.8 and we're trying to stabilize v4.9 it looked like it could wait until v4.10. So is there something going on here where the pain is too much to wait for the next merge window? If so the commit text should mention something about what's causing that pain. Perhaps by referencing the commit that merged outside of clk tree that caused problems.
On Fri, Oct 28, 2016 at 4:13 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > The clk-fixes branch is for patches that fix problems in code > merged during the merge window as well as small one-liners and > things that are causing great pain for people on the latest > release. Given that this fixes a regression in v4.8 and we're > trying to stabilize v4.9 it looked like it could wait until > v4.10. The regression affects 4.8 and 4.9. > So is there something going on here where the pain is too much to > wait for the next merge window? If so the commit text should Yes, people reported HDMI issues because of this bug: https://www.spinics.net/lists/arm-kernel/msg535304.html > mention something about what's causing that pain. Perhaps by > referencing the commit that merged outside of clk tree that > caused problems. This patch clearly states that the problem is caused by ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula"). Since this is a regression, I don't understand why we need to wait until 4.10 to get it applied. -- To unsubscribe from this list: send the line "unsubscribe linux-clk" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/28, Fabio Estevam wrote: > On Fri, Oct 28, 2016 at 4:13 PM, Stephen Boyd <sboyd@codeaurora.org> wrote: > > > The clk-fixes branch is for patches that fix problems in code > > merged during the merge window as well as small one-liners and > > things that are causing great pain for people on the latest > > release. Given that this fixes a regression in v4.8 and we're > > trying to stabilize v4.9 it looked like it could wait until > > v4.10. > > The regression affects 4.8 and 4.9. > > > So is there something going on here where the pain is too much to > > wait for the next merge window? If so the commit text should > > Yes, people reported HDMI issues because of this bug: > https://www.spinics.net/lists/arm-kernel/msg535304.html > > > mention something about what's causing that pain. Perhaps by > > referencing the commit that merged outside of clk tree that > > caused problems. > > This patch clearly states that the problem is caused by ba7f4f557eb6 > ("clk: imx: correct AV PLL rate formula"). That commit isn't outside of clk tree :( > > Since this is a regression, I don't understand why we need to wait > until 4.10 to get it applied. Because we're stabilizing the 4.9-rc series and not the 4.8-rc series and the assumption is people tested code no 4.8 before it was released. But in cases where that doesn't happen and the bugs cause problems with testing the latest rc series we just apply the patch. It sounds like in this case that happened, so I'll move this patch over to fixes.
diff --git a/drivers/clk/imx/clk-pllv3.c b/drivers/clk/imx/clk-pllv3.c index 19f9b622981a..7a6acc3e4a92 100644 --- a/drivers/clk/imx/clk-pllv3.c +++ b/drivers/clk/imx/clk-pllv3.c @@ -223,7 +223,7 @@ static unsigned long clk_pllv3_av_recalc_rate(struct clk_hw *hw, temp64 *= mfn; do_div(temp64, mfd); - return (parent_rate * div) + (u32)temp64; + return parent_rate * div + (unsigned long)temp64; } static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate, @@ -247,7 +247,11 @@ static long clk_pllv3_av_round_rate(struct clk_hw *hw, unsigned long rate, do_div(temp64, parent_rate); mfn = temp64; - return parent_rate * div + parent_rate * mfn / mfd; + temp64 = (u64)parent_rate; + temp64 *= mfn; + do_div(temp64, mfd); + + return parent_rate * div + (unsigned long)temp64; } static int clk_pllv3_av_set_rate(struct clk_hw *hw, unsigned long rate,
Since 'parent_rate * mfn' may overflow 32 bits, the result should be stored using 64 bits. The problem was discovered when trying to set the rate of the audio PLL (pll4_post_div) on an i.MX6Q. The desired rate was 196.608 MHz, but the actual rate returned was 192.000570 MHz. The round rate function should have been able to return 196.608 MHz, i.e., the desired rate. Fixes: ba7f4f557eb6 ("clk: imx: correct AV PLL rate formula") Cc: Anson Huang <b20788@freescale.com> Signed-off-by: Emil Lundmark <emil@limesaudio.com> --- drivers/clk/imx/clk-pllv3.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)