| Message ID | 4391b498-0a75-ff42-6a7e-65aef0fada07@tul.cz (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Petr Cvek <petr.cvek@tul.cz> writes: > The conversion from soc_camera omitted a correct handling of the clock > gating for a sensor. When the pxa_camera driver module was removed it > tried to unregister clk, but this caused a similar warning: > > WARNING: CPU: 0 PID: 6740 at drivers/media/v4l2-core/v4l2-clk.c:278 > v4l2_clk_unregister(): Refusing to unregister ref-counted 0-0030 clock! > > The clock was at time still refcounted by the sensor driver. Before > the removing of the pxa_camera the clock must be dropped by the sensor > driver. This should be triggered by v4l2_async_notifier_unregister() call > which removes sensor driver module too, calls unbind() function and then > tries to probe sensor driver again. Inside unbind() we can safely > unregister the v4l2 clock as the sensor driver got removed. The original > v4l2_clk_unregister() should be put inside test as the clock can be > already unregistered from unbind(). If there was not any bound sensor > the clock is still present. > > The codepath is practically a copy from the old soc_camera. The bug was > tested with a pxa_camera+ov9640 combination during the conversion > of the ov9640 from the soc_camera. > > Signed-off-by: Petr Cvek <petr.cvek@tul.cz> Yeah, it's way better with this patch, especially the insmod/rmmod/insmod/rmmod test. Tested-by: Robert Jarzmik <robert.jarzmik@free.fr> Acked-by: Robert Jarzmik <robert.jarzmik@free.fr> Cheers. -- Robert
Dne 27.4.2017 v 21:20 Robert Jarzmik napsal(a): > Petr Cvek <petr.cvek@tul.cz> writes: > >> The conversion from soc_camera omitted a correct handling of the clock >> gating for a sensor. When the pxa_camera driver module was removed it >> tried to unregister clk, but this caused a similar warning: >> >> WARNING: CPU: 0 PID: 6740 at drivers/media/v4l2-core/v4l2-clk.c:278 >> v4l2_clk_unregister(): Refusing to unregister ref-counted 0-0030 clock! >> >> The clock was at time still refcounted by the sensor driver. Before >> the removing of the pxa_camera the clock must be dropped by the sensor >> driver. This should be triggered by v4l2_async_notifier_unregister() call >> which removes sensor driver module too, calls unbind() function and then >> tries to probe sensor driver again. Inside unbind() we can safely >> unregister the v4l2 clock as the sensor driver got removed. The original >> v4l2_clk_unregister() should be put inside test as the clock can be >> already unregistered from unbind(). If there was not any bound sensor >> the clock is still present. >> >> The codepath is practically a copy from the old soc_camera. The bug was >> tested with a pxa_camera+ov9640 combination during the conversion >> of the ov9640 from the soc_camera. >> >> Signed-off-by: Petr Cvek <petr.cvek@tul.cz> > > Yeah, it's way better with this patch, especially the insmod/rmmod/insmod/rmmod > test. I will post some other bugfixes (and feature adding) for pxa_camera soon. Do you wish to be CC'd? P.S. Who is the the maintainer of pxa_camera BTW? Still Guennadi Liakhovetski? Basically pxa_camera is no longer part of the soc_camera and MAINTAINERS file does not exactly specify pxa_camera.c (I'm asking because I will send the patch for ov9640 soc_camera removal too :-D). Best regards, Petr
Petr Cvek <petr.cvek@tul.cz> writes: > I will post some other bugfixes (and feature adding) for pxa_camera soon. Do you wish to be CC'd? > > P.S. Who is the the maintainer of pxa_camera BTW? Still Guennadi Liakhovetski? Euh no, that's me. I had submitted a patch for that here : https://patchwork.kernel.org/patch/9316499 Hans, do you want to pick it up ? Cheers.
Dne 28.4.2017 v 08:31 Robert Jarzmik napsal(a): > Petr Cvek <petr.cvek@tul.cz> writes: > >> I will post some other bugfixes (and feature adding) for pxa_camera soon. Do you wish to be CC'd? >> >> P.S. Who is the the maintainer of pxa_camera BTW? Still Guennadi Liakhovetski? > Euh no, that's me. OK ... so when I remove the ov9640 driver from soc_camera (palmz72 and magician used it with soc_camera+pxa_camera) does that mean I will be its maintainer? Petr
diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c index 929006f65cc7..6615f80fe059 100644 --- a/drivers/media/platform/pxa_camera.c +++ b/drivers/media/platform/pxa_camera.c @@ -2177,6 +2177,12 @@ static void pxa_camera_sensor_unbind(struct v4l2_async_notifier *notifier, pxa_dma_stop_channels(pcdev); pxa_camera_destroy_formats(pcdev); + + if (pcdev->mclk_clk) { + v4l2_clk_unregister(pcdev->mclk_clk); + pcdev->mclk_clk = NULL; + } + video_unregister_device(&pcdev->vdev); pcdev->sensor = NULL; @@ -2501,7 +2507,13 @@ static int pxa_camera_remove(struct platform_device *pdev) dma_release_channel(pcdev->dma_chans[1]); dma_release_channel(pcdev->dma_chans[2]); - v4l2_clk_unregister(pcdev->mclk_clk); + v4l2_async_notifier_unregister(&pcdev->notifier); + + if (pcdev->mclk_clk) { + v4l2_clk_unregister(pcdev->mclk_clk); + pcdev->mclk_clk = NULL; + } + v4l2_device_unregister(&pcdev->v4l2_dev); dev_info(&pdev->dev, "PXA Camera driver unloaded\n");
The conversion from soc_camera omitted a correct handling of the clock gating for a sensor. When the pxa_camera driver module was removed it tried to unregister clk, but this caused a similar warning: WARNING: CPU: 0 PID: 6740 at drivers/media/v4l2-core/v4l2-clk.c:278 v4l2_clk_unregister(): Refusing to unregister ref-counted 0-0030 clock! The clock was at time still refcounted by the sensor driver. Before the removing of the pxa_camera the clock must be dropped by the sensor driver. This should be triggered by v4l2_async_notifier_unregister() call which removes sensor driver module too, calls unbind() function and then tries to probe sensor driver again. Inside unbind() we can safely unregister the v4l2 clock as the sensor driver got removed. The original v4l2_clk_unregister() should be put inside test as the clock can be already unregistered from unbind(). If there was not any bound sensor the clock is still present. The codepath is practically a copy from the old soc_camera. The bug was tested with a pxa_camera+ov9640 combination during the conversion of the ov9640 from the soc_camera. Signed-off-by: Petr Cvek <petr.cvek@tul.cz> --- drivers/media/platform/pxa_camera.c | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-)