diff mbox

v4.9-rc1: smiapp divides by zero

Message ID 20161023102213.GA13705@amd (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Machek Oct. 23, 2016, 10:22 a.m. UTC
Hi!

I tried to update camera code on n900 to v4.9-rc1, and I'm getting
some divide by zero, that eventually cascades into fcam-dev not
working.

mul is zero in my testing, resulting in divide by zero.

(Note that this is going from my patched camera-v4.8 tree to
camera-v4.9 tree.)

Best regards,
								Pavel

Comments

Pali Rohár Oct. 23, 2016, 10:32 a.m. UTC | #1
On Sunday 23 October 2016 12:22:13 Pavel Machek wrote:
> Hi!
> 
> I tried to update camera code on n900 to v4.9-rc1, and I'm getting
> some divide by zero, that eventually cascades into fcam-dev not
> working.
> 
> mul is zero in my testing, resulting in divide by zero.
> 
> (Note that this is going from my patched camera-v4.8 tree to
> camera-v4.9 tree.)
> 
> Best regards,
> 								Pavel

Hi! Ideally look at existing camera patches. I do not know which one is
last, but here are some links:

https://github.com/freemangordon/linux-n900/tree/v4.6-rc4-n900-camera
https://github.com/freemangordon/linux-n900/tree/camera
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera-ivo
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-n900.git/log/?h=n900-camera

> diff --git a/drivers/media/i2c/smiapp-pll.c
> b/drivers/media/i2c/smiapp-pll.c index 5ad1edb..e0a6edd 100644
> --- a/drivers/media/i2c/smiapp-pll.c
> +++ b/drivers/media/i2c/smiapp-pll.c
> @@ -16,6 +16,8 @@
>   * General Public License for more details.
>   */
> 
> +#define DEBUG
> +
>  #include <linux/device.h>
>  #include <linux/gcd.h>
>  #include <linux/lcm.h>
> @@ -457,6 +459,10 @@ int smiapp_pll_calculate(struct device *dev,
>  	i = gcd(pll->pll_op_clk_freq_hz, pll->ext_clk_freq_hz);
>  	mul = div_u64(pll->pll_op_clk_freq_hz, i);
>  	div = pll->ext_clk_freq_hz / i;
> +	if (!mul) {
> +		dev_err(dev, "forcing mul to 1\n");
> +		mul = 1;
> +	}
>  	dev_dbg(dev, "mul %u / div %u\n", mul, div);
> 
>  	min_pre_pll_clk_div =

Is not this patch still enough?
https://patchwork.kernel.org/patch/8921761/
Pavel Machek Oct. 23, 2016, 10:52 a.m. UTC | #2
Hi!

> I tried to update camera code on n900 to v4.9-rc1, and I'm getting
> some divide by zero, that eventually cascades into fcam-dev not
> working.
> 
> mul is zero in my testing, resulting in divide by zero.
> 
> (Note that this is going from my patched camera-v4.8 tree to
> camera-v4.9 tree.)

If I revert the smiapp changes to the ones in camera-v4.8, I get fcam
back, and can get pictures using the main camera.

There are only few patches between v4.8 and v4.8 in smiapp, so I'll
try to find what is going on there.

Best regards,
								Pavel
Sakari Ailus Oct. 23, 2016, 2:09 p.m. UTC | #3
Hi Pavel,

On Sun, Oct 23, 2016 at 12:22:13PM +0200, Pavel Machek wrote:
> Hi!
> 
> I tried to update camera code on n900 to v4.9-rc1, and I'm getting
> some divide by zero, that eventually cascades into fcam-dev not
> working.
> 
> mul is zero in my testing, resulting in divide by zero.
> 
> (Note that this is going from my patched camera-v4.8 tree to
> camera-v4.9 tree.)
> 
> Best regards,
> 								Pavel
> 
> diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
> index 5ad1edb..e0a6edd 100644
> --- a/drivers/media/i2c/smiapp-pll.c
> +++ b/drivers/media/i2c/smiapp-pll.c
> @@ -16,6 +16,8 @@
>   * General Public License for more details.
>   */
>  
> +#define DEBUG
> +
>  #include <linux/device.h>
>  #include <linux/gcd.h>
>  #include <linux/lcm.h>
> @@ -457,6 +459,10 @@ int smiapp_pll_calculate(struct device *dev,
>  	i = gcd(pll->pll_op_clk_freq_hz, pll->ext_clk_freq_hz);
>  	mul = div_u64(pll->pll_op_clk_freq_hz, i);
>  	div = pll->ext_clk_freq_hz / i;
> +	if (!mul) {

Something must be very wrong if you get here.

What are the values of pll->pll_op_clk_freq_hz and pll->ext_clk_freq_hz?
Or... what does dmesg say?

> +		dev_err(dev, "forcing mul to 1\n");
> +		mul = 1;
> +	}
>  	dev_dbg(dev, "mul %u / div %u\n", mul, div);
>  
>  	min_pre_pll_clk_div =
>
Pavel Machek Oct. 23, 2016, 6:33 p.m. UTC | #4
Hi!

> > +#define DEBUG
> > +
> >  #include <linux/device.h>
> >  #include <linux/gcd.h>
> >  #include <linux/lcm.h>
> > @@ -457,6 +459,10 @@ int smiapp_pll_calculate(struct device *dev,
> >  	i = gcd(pll->pll_op_clk_freq_hz, pll->ext_clk_freq_hz);
> >  	mul = div_u64(pll->pll_op_clk_freq_hz, i);
> >  	div = pll->ext_clk_freq_hz / i;
> > +	if (!mul) {
> 
> Something must be very wrong if you get here.
> 
> What are the values of pll->pll_op_clk_freq_hz and pll->ext_clk_freq_hz?
> Or... what does dmesg say?

Yep, it was very wrong. I mismerged the stuff, and hwcfg->lanes
initialization was missing. Now it appears to work.

(I have pushed the changes to camera-v4.9 branch).

Best regards,
									Pavel
diff mbox

Patch

diff --git a/drivers/media/i2c/smiapp-pll.c b/drivers/media/i2c/smiapp-pll.c
index 5ad1edb..e0a6edd 100644
--- a/drivers/media/i2c/smiapp-pll.c
+++ b/drivers/media/i2c/smiapp-pll.c
@@ -16,6 +16,8 @@ 
  * General Public License for more details.
  */
 
+#define DEBUG
+
 #include <linux/device.h>
 #include <linux/gcd.h>
 #include <linux/lcm.h>
@@ -457,6 +459,10 @@  int smiapp_pll_calculate(struct device *dev,
 	i = gcd(pll->pll_op_clk_freq_hz, pll->ext_clk_freq_hz);
 	mul = div_u64(pll->pll_op_clk_freq_hz, i);
 	div = pll->ext_clk_freq_hz / i;
+	if (!mul) {
+		dev_err(dev, "forcing mul to 1\n");
+		mul = 1;
+	}
 	dev_dbg(dev, "mul %u / div %u\n", mul, div);
 
 	min_pre_pll_clk_div =