| Message ID | 20090703161140.845950e8.ospite@studenti.unina.it (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Fri, 3 Jul 2009, Antonio Ospite wrote: > On Wed, 1 Jul 2009 20:43:25 +0200 > Antonio Ospite <ospite@studenti.unina.it> wrote: > > > Hi, > > > > I get this with pxa-camera in mainline linux (from today). > > I haven't touched my board code which used to work in 2.6.30 > > > > I think I've tracked down the cause. The board code is triggering a > bug in pxa_camera. The same should apply to mioa701 as well. > > > Linux video capture interface: v2.00 > > Unable to handle kernel NULL pointer dereference at virtual address 00000060 > > pgd = c0004000 > > [00000060] *pgd=00000000 > > Internal error: Oops: f5 [#1] PREEMPT > > Modules linked in: > > CPU: 0 Tainted: G W (2.6.31-rc1-ezxdev #1) > > PC is at dev_driver_string+0x0/0x38 > > LR is at pxa_camera_probe+0x144/0x428 > > The offending dev_driver_str() here is the one in the dev_warn() call in > mclk_get_divisor(). > > This is what is happening: in struct pxacamera_platform_data I have: > .mclk_10khz = 5000, > > which makes the > test in mclk_get_divisor() succeed calling dev_warn > to report that the clock has been limited, but pcdev->soc_host.dev is > still uninitialized at this time. > > I could lower the value in my platform data and avoid the bug, but it > would be good to have this fixed ASAP anyway. > > The attached rough patch fixes the problem, but you will surely come > out with a better one :) Why should I? Your patch seems correct to me so far, thanks. I'll push it for 2.6.31. Please, next time inline your patch as described in Documentation/SubmittingPatches. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 3 Jul 2009 22:03:27 +0200 (CEST) Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > On Fri, 3 Jul 2009, Antonio Ospite wrote: > > > > Linux video capture interface: v2.00 > > > Unable to handle kernel NULL pointer dereference at virtual address 00000060 > > > pgd = c0004000 > > > [00000060] *pgd=00000000 > > > Internal error: Oops: f5 [#1] PREEMPT > > > Modules linked in: > > > CPU: 0 Tainted: G W (2.6.31-rc1-ezxdev #1) > > > PC is at dev_driver_string+0x0/0x38 > > > LR is at pxa_camera_probe+0x144/0x428 > > > > The offending dev_driver_str() here is the one in the dev_warn() call in > > mclk_get_divisor(). > > > > This is what is happening: in struct pxacamera_platform_data I have: > > .mclk_10khz = 5000, > > > > which makes the > test in mclk_get_divisor() succeed calling dev_warn > > to report that the clock has been limited, but pcdev->soc_host.dev is > > still uninitialized at this time. > > > > I could lower the value in my platform data and avoid the bug, but it > > would be good to have this fixed ASAP anyway. > > > > The attached rough patch fixes the problem, but you will surely come > > out with a better one :) > > Why should I? Your patch seems correct to me so far, thanks. I'll push it > for 2.6.31. Please, next time inline your patch as described in > Documentation/SubmittingPatches. > Well, it should be correct, I just thought it could be considered unpretty with the pcdev->soc_host initializations scattered here and there, that's what I was referring to. But, if this is ok to you, it's ok to me too :) Ciao, Antonio
Antonio Ospite <ospite@studenti.unina.it> writes: > On Fri, 3 Jul 2009 22:03:27 +0200 (CEST) > Guennadi Liakhovetski <g.liakhovetski@gmx.de> wrote: > >> On Fri, 3 Jul 2009, Antonio Ospite wrote: >> >> > > Linux video capture interface: v2.00 >> > > Unable to handle kernel NULL pointer dereference at virtual address 00000060 >> > > pgd = c0004000 >> > > [00000060] *pgd=00000000 >> > > Internal error: Oops: f5 [#1] PREEMPT >> > > Modules linked in: >> > > CPU: 0 Tainted: G W (2.6.31-rc1-ezxdev #1) >> > > PC is at dev_driver_string+0x0/0x38 >> > > LR is at pxa_camera_probe+0x144/0x428 >> > >> > The offending dev_driver_str() here is the one in the dev_warn() call in >> > mclk_get_divisor(). >> > >> > This is what is happening: in struct pxacamera_platform_data I have: >> > .mclk_10khz = 5000, >> > >> > which makes the > test in mclk_get_divisor() succeed calling dev_warn >> > to report that the clock has been limited, but pcdev->soc_host.dev is >> > still uninitialized at this time. Antonio, Would you check [1] and see if your stack does correspond to the one I reported some time ago ? As this is fresh in your memory, you'll be far quicker that me. Ah, and by the way, I like your patch too, agree that mioa701 is touched, and I think it should go upstream. Cheers. -- Robert [1] http://osdir.com/ml/linux-media/2009-04/msg00874.html -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Sat, 04 Jul 2009 21:35:22 +0200 Robert Jarzmik <robert.jarzmik@free.fr> wrote: > >> > The offending dev_driver_str() here is the one in the dev_warn() call in > >> > mclk_get_divisor(). > >> > > >> > This is what is happening: in struct pxacamera_platform_data I have: > >> > .mclk_10khz = 5000, > >> > > >> > which makes the > test in mclk_get_divisor() succeed calling dev_warn > >> > to report that the clock has been limited, but pcdev->soc_host.dev is > >> > still uninitialized at this time. > > Antonio, > > Would you check [1] and see if your stack does correspond to the one I reported > some time ago ? As this is fresh in your memory, you'll be far quicker that me. > ... > [1] http://osdir.com/ml/linux-media/2009-04/msg00874.html Yes, I think that is it. The offsets are different of course but the call stack is pretty much the same. Regards, Antonio
mclk_get_divisor uses pcdev->soc_host.dev, make sure it is initialized. Signed-off-by: Antonio Ospite <ospite@studenti.unina.it> diff --git a/drivers/media/video/pxa_camera.c b/drivers/media/video/pxa_camera.c index 46e0d8a..e048d25 100644 --- a/drivers/media/video/pxa_camera.c +++ b/drivers/media/video/pxa_camera.c @@ -1579,6 +1579,7 @@ static int __devinit pxa_camera_probe(struct platform_device *pdev) pcdev->mclk = 20000000; } + pcdev->soc_host.dev = &pdev->dev; pcdev->mclk_divisor = mclk_get_divisor(pcdev); INIT_LIST_HEAD(&pcdev->capture); @@ -1644,7 +1645,6 @@ static int __devinit pxa_camera_probe(struct platform_device *pdev) pcdev->soc_host.drv_name = PXA_CAM_DRV_NAME; pcdev->soc_host.ops = &pxa_soc_camera_host_ops; pcdev->soc_host.priv = pcdev; - pcdev->soc_host.dev = &pdev->dev; pcdev->soc_host.nr = pdev->id; err = soc_camera_host_register(&pcdev->soc_host);