[U-Boot] [PATCH] video: ipu_common: fix build error
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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@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));
- clk->rate = (u64)(clk->parent->rate * 16) / div; + do_div(parent_rate, div); + + clk->rate = parent_rate;
return 0; }
On Thu, Apr 28, 2016 at 10:07:53AM +0800, 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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com
Reviewed-by: Tom Rini trini@konsulko.com
On Thu, Apr 28, 2016 at 3:07 AM, Peng Fan van.freenix@gmail.com 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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com
Tested-by: Peter Robinson pbrobinson@gmail.com
Hi Peter,
Please help test this patch to see whether this fix your issue or not. Thanks for pointing out this issue.
It fixes the build issue, I'll be installing it onto a number of devices over the next couple of days.
Thanks, Peter
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));
clk->rate = (u64)(clk->parent->rate * 16) / div;
do_div(parent_rate, div);clk->rate = parent_rate; return 0;}
2.6.2
On 28/04/2016 16:20, Peter Robinson wrote:
On Thu, Apr 28, 2016 at 3:07 AM, Peng Fan van.freenix@gmail.com 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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com
Tested-by: Peter Robinson pbrobinson@gmail.com
Hi Peter,
Please help test this patch to see whether this fix your issue or not. Thanks for pointing out this issue.
It fixes the build issue, I'll be installing it onto a number of devices over the next couple of days.
Thanks, Peter
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));
clk->rate = (u64)(clk->parent->rate * 16) / div;
do_div(parent_rate, div);clk->rate = parent_rate; return 0;}
2.6.2
Applied to u-boot-imx, thanks !
Best regards, Stefano Babic
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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com Reviewed-by: Tom Rini trini@konsulko.com Tested-by: Peter Robinson pbrobinson@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?
- clk->rate = (u64)(clk->parent->rate * 16) / div;
do_div(parent_rate, div);
clk->rate = parent_rate;
return 0; }
Please advise,
Eric
Hi Eric, 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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com Reviewed-by: Tom Rini trini@konsulko.com Tested-by: Peter Robinson pbrobinson@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.
Thanks, Peng.
- clk->rate = (u64)(clk->parent->rate * 16) / div;
- do_div(parent_rate, div);
- clk->rate = parent_rate; return 0;
}
Please advise,
Eric
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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com Reviewed-by: Tom Rini trini@konsulko.com Tested-by: Peter Robinson pbrobinson@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:
http://git.denx.de/?p=u-boot.git;a=blob;f=drivers/video/ipu_common.c;hb=3cb4...
Regards,
Eric
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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com Reviewed-by: Tom Rini trini@konsulko.com Tested-by: Peter Robinson pbrobinson@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
On 09/06/2017 11:01 PM, Lothar Waßmann wrote:
On Wed, 6 Sep 2017 10:34:33 -0700 Eric Nelson wrote:
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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com Reviewed-by: Tom Rini trini@konsulko.com Tested-by: Peter Robinson pbrobinson@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;
Doh! I clearly missed that.
Thanks Lothar.
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 pbrobinson@gmail.com Signed-off-by: Peng Fan van.freenix@gmail.com Cc: Stefano Babic sbabic@denx.de Cc: Fabio Estevam fabio.estevam@nxp.com Cc: Tom Rini trini@konsulko.com Cc: Anatolij Gustschin agust@denx.de Cc: Peter Robinson pbrobinson@gmail.com Reviewed-by: Tom Rini trini@konsulko.com Tested-by: Peter Robinson pbrobinson@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 you intend to lose a multiply by 16 here?
- clk->rate = (u64)(clk->parent->rate * 16) / div;
do_div(parent_rate, div);
clk->rate = parent_rate;
return 0; }
Please advise,
Eric
Hi Eric,
On Mon, Sep 4, 2017 at 11:49 PM, Eric Nelson eric@nelint.com 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.
Just curious: how does the regression manifest itself?
Hi Fabio,
On 09/05/2017 06:33 AM, Fabio Estevam wrote:
Hi Eric,
On Mon, Sep 4, 2017 at 11:49 PM, Eric Nelson eric@nelint.com 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.
Just curious: how does the regression manifest itself?
With **some** televisions at a client site, on **some** power-on cycles, the HDMI output under Linux doesn't seem to be generated properly.
We haven't been able to reproduce it in-house, so testing is taking a while, and we haven't (yet) determined if the divisor patch has anything to do with the problem.
We are running on an i.MX6DL, but the IPU clock frequency change doesn't fix the issue (running at 19.8MHz instead of 26MHz).
All we know at the moment is that version 2015.07 works and 2016.05 fails with essentially no changes to the board files.
We're doing this remotely across time zones with limited access to failing machine(s), so it may take the rest of the week before we know for sure.
I'll update the thread when we nail it down.
Regards,
Eric
participants (9)
-
Anatolij Gustschin -
Eric Nelson -
Eric Nelson -
Fabio Estevam -
Lothar Waßmann -
Peng Fan -
Peter Robinson -
Stefano Babic -
Tom Rini