Hi, On Wed, 6 Sep 2017 10:34:33 -0700 Eric Nelson wrote: > Thanks Peng. > > On 09/05/2017 06:16 PM, Peng Fan wrote: > > On Mon, Sep 04, 2017 at 07:48:56PM -0700, Eric Nelson wrote: > >> Hi Peng, > >> > >> Can you tell that I'm hunting a bug in an old version? > >> > >> I'm seeing a **very** intermittent regression between U-Boot > >> versions 2015.07 and 2016.05 and happened to spot something > >> in this patch. > >> > >> On 04/27/2016 07:07 PM, Peng Fan wrote: > >>> Some toolchains fail to build > >>> "clk->rate = (u64)(clk->parent->rate * 16) / div;" > >>> And the cast usage is wrong. > >>> > >>> Use the following code to fix the issue, > >>> " > >>> do_div(parent_rate, div); > >>> clk->rate = parent_rate; > >>> " > >>> > >>> Reported-by: Peter Robinson <pbrobin...@gmail.com> > >>> Signed-off-by: Peng Fan <van.free...@gmail.com> > >>> Cc: Stefano Babic <sba...@denx.de> > >>> Cc: Fabio Estevam <fabio.este...@nxp.com> > >>> Cc: Tom Rini <tr...@konsulko.com> > >>> Cc: Anatolij Gustschin <ag...@denx.de> > >>> Cc: Peter Robinson <pbrobin...@gmail.com> > >>> Reviewed-by: Tom Rini <tr...@konsulko.com> > >>> Tested-by: Peter Robinson <pbrobin...@gmail.com> > >>> --- > >>> > >>> Hi Peter, > >>> > >>> Please help test this patch to see whether this fix your issue or not. > >>> Thanks for pointing out this issue. > >>> > >>> Thanks, > >>> Peng. > >>> > >>> drivers/video/ipu_common.c | 4 +++- > >>> 1 file changed, 3 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/video/ipu_common.c b/drivers/video/ipu_common.c > >>> index 36d4b23..5676a0f 100644 > >>> --- a/drivers/video/ipu_common.c > >>> +++ b/drivers/video/ipu_common.c > >>> @@ -352,7 +352,9 @@ static int ipu_pixel_clk_set_rate(struct clk *clk, > >>> unsigned long rate) > >>> */ > >>> __raw_writel((div / 16) << 16, DI_BS_CLKGEN1(clk->id)); > >> > >> Did we lose a multiply by 16 in this change? > > > > We already have "parent_rate = (unsigned long long)clk->parent->rate * 16;" > > in this function. > > > > Hmmm. So this patch also fixed a bug, since we previously had > **two** multiply-by-16's: > No! The 'second' multiply by 16 used the clk->parent->rate, not the 'parent_rate' which was multiplied by 16...
| parent_rate = (unsigned long long)clk->parent->rate * 16; [...] | clk->rate = (u64)(clk->parent->rate * 16) / div; Lothar Waßmann _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot