2016-10-23 07:33:28

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] media: add et8ek8 camera sensor driver and documentation

Hi!

> This series adds driver for Toshiba et8ek8 camera sensor found in Nokia N900
>
> Changes from v2:
>
> - fix build when CONFIG_PM is not defined
>
> Changes from v1:
>
> - driver and documentation split into separate patches
> - removed custom controls
> - code changed according to the comments on v1

> Ivaylo Dimitrov (2):
> media: Driver for Toshiba et8ek8 5MP sensor
> media: et8ek8: Add documentation

Is there any progress here? Is there any way I could help?

Best regards,
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (651.00 B)
signature.asc (181.00 B)
Digital signature
Download all attachments

2016-10-23 10:22:20

by Pavel Machek

[permalink] [raw]
Subject: v4.9-rc1: smiapp divides by zero

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) {
+ 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 =

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (1.11 kB)
signature.asc (181.00 B)
Digital signature
Download all attachments

2016-10-23 10:32:28

by Pali Rohár

[permalink] [raw]
Subject: Re: v4.9-rc1: smiapp divides by zero

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/

--
Pali Rohár
[email protected]


Attachments:
signature.asc (198.00 B)
This is a digitally signed message part.

2016-10-23 10:52:18

by Pavel Machek

[permalink] [raw]
Subject: Re: v4.9-rc1: smiapp divides by zero

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


--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (692.00 B)
signature.asc (181.00 B)
Digital signature
Download all attachments

2016-10-23 14:09:23

by Sakari Ailus

[permalink] [raw]
Subject: Re: v4.9-rc1: smiapp divides by zero

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 =
>

--
Kind regards,

Sakari Ailus
e-mail: [email protected] XMPP: [email protected]

2016-10-23 18:05:23

by Ivaylo Dimitrov

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] media: add et8ek8 camera sensor driver and documentation

Hi,

On 23.10.2016 10:33, Pavel Machek wrote:
> Hi!
>
>> This series adds driver for Toshiba et8ek8 camera sensor found in Nokia N900
>>
>> Changes from v2:
>>
>> - fix build when CONFIG_PM is not defined
>>
>> Changes from v1:
>>
>> - driver and documentation split into separate patches
>> - removed custom controls
>> - code changed according to the comments on v1
>
>> Ivaylo Dimitrov (2):
>> media: Driver for Toshiba et8ek8 5MP sensor
>> media: et8ek8: Add documentation
>
> Is there any progress here? Is there any way I could help?
>

There were some notes I need to address, unfortunately no spare time
lately :( . Feel free to fix those for me and resend the patches. If
not, I really don't know when I will have the time needed to focus on it.

Regards,
Ivo

2016-10-23 18:17:58

by Pavel Machek

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] media: add et8ek8 camera sensor driver and documentation

Hi!

> >>This series adds driver for Toshiba et8ek8 camera sensor found in Nokia N900
> >>
> >>Changes from v2:
> >>
> >> - fix build when CONFIG_PM is not defined
> >>
> >>Changes from v1:
> >>
> >> - driver and documentation split into separate patches
> >> - removed custom controls
> >> - code changed according to the comments on v1
> >
> >>Ivaylo Dimitrov (2):
> >> media: Driver for Toshiba et8ek8 5MP sensor
> >> media: et8ek8: Add documentation
> >
> >Is there any progress here? Is there any way I could help?
> >
>
> There were some notes I need to address, unfortunately no spare time lately
> :( . Feel free to fix those for me and resend the patches. If not, I really
> don't know when I will have the time needed to focus on it.

So good start would be taking these two, address the comments, and try
to merge them?

Date: Sat, 11 Jun 2016 18:39:52 +0300
Subject: [PATCH v3 1/2] media: Driver for Toshiba et8ek8 5MP sensor

Date: Wed, 15 Jun 2016 22:24:40 +0300
Subject: Re: [PATCH v3 2/2] media: et8ek8: Add documentation

Thanks and best regards,
Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (1.19 kB)
signature.asc (181.00 B)
Digital signature
Download all attachments

2016-10-23 18:33:40

by Pavel Machek

[permalink] [raw]
Subject: Re: v4.9-rc1: smiapp divides by zero

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
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


Attachments:
(No filename) (847.00 B)
signature.asc (181.00 B)
Digital signature
Download all attachments

2016-10-23 18:36:11

by Ivaylo Dimitrov

[permalink] [raw]
Subject: Re: [PATCH v3 0/2] media: add et8ek8 camera sensor driver and documentation



On 23.10.2016 21:17, Pavel Machek wrote:
> Hi!
>
>>>> This series adds driver for Toshiba et8ek8 camera sensor found in Nokia N900
>>>>
>>>> Changes from v2:
>>>>
>>>> - fix build when CONFIG_PM is not defined
>>>>
>>>> Changes from v1:
>>>>
>>>> - driver and documentation split into separate patches
>>>> - removed custom controls
>>>> - code changed according to the comments on v1
>>>
>>>> Ivaylo Dimitrov (2):
>>>> media: Driver for Toshiba et8ek8 5MP sensor
>>>> media: et8ek8: Add documentation
>>>
>>> Is there any progress here? Is there any way I could help?
>>>
>>
>> There were some notes I need to address, unfortunately no spare time lately
>> :( . Feel free to fix those for me and resend the patches. If not, I really
>> don't know when I will have the time needed to focus on it.
>
> So good start would be taking these two, address the comments, and try
> to merge them?
>

Yep, the whole history should be at
https://patchwork.kernel.org/patch/9171067/ and
http://www.gossamer-threads.com/lists/linux/kernel/2462501

Thanks,
Ivo