media: pxa_camera: disable and unprepare the clock source on error

PATCH In review Flavio Ceolin 2 participants 5 messages Dec 6, 2017 – Dec 11, 2017 Media (V4L/DVB)

Share & tools

Flavio Ceolin

author

2017-12-06 16:39 UTC #0 lore raw

PATCH 1/1 media: pxa_camera: disable and unprepare the clock source on error

pxa_camera_probe() was not calling pxa_camera_deactivate(), responsible to call clk_disable_unprepare(), on the failure path. This was leading to unbalancing source clock.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by Flavio Ceolin <[email protected]>

---
 drivers/media/platform/pxa_camera.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

drivers/media/platform/pxa_camera.c +3 -1

index 9d3f0cb..7877037 100644
@@ -2489,7 +2489,7 @@ static int pxa_camera_probe(struct platform_device *pdev)
 	dev_set_drvdata(&pdev->dev, pcdev);
 	err = v4l2_device_register(&pdev->dev, &pcdev->v4l2_dev);
 	if (err)
-		goto exit_free_dma;
+		goto exit_deactivate;
 
 	pcdev->asds[0] = &pcdev->asd;
 	pcdev->notifier.subdevs = pcdev->asds;

@@ -2525,6 +2525,8 @@ static int pxa_camera_probe(struct platform_device *pdev)
 	v4l2_clk_unregister(pcdev->mclk_clk);
 exit_free_v4l2dev:
 	v4l2_device_unregister(&pcdev->v4l2_dev);
+exit_deactivate:
+	pxa_camera_deactivate(pcdev);
 exit_free_dma:
 	dma_release_channel(pcdev->dma_chans[2]);
 exit_free_dma_u:

Signature · 1 line

2.9.5

Laurent Pinchart

Reviewed-by on v1 1/1

2017-12-11 16:51 UTC #1 lore raw

Hi Flavio,

Thank you for the patch.

Quoted 5 lines · On Wednesday, 6 December 2017 18:38:50 EET Flavio Ceolin wrote:

pxa_camera_probe() was not calling pxa_camera_deactivate(), responsible to call clk_disable_unprepare(), on the failure path. This was leading to unbalancing source clock.
Found by Linux Driver Verification project (linuxtesting.org).

Any chance I could sign you up for more work on this driver ? :-)

Quoted 1 line

Signed-off-by Flavio Ceolin <[email protected]>

Reviewed-by Laurent Pinchart <[email protected]>

I expect Hans Verkuil to pick up the patch.

Quoted 26 lines

---
 drivers/media/platform/pxa_camera.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

(quoted diff) +0 -0

b/drivers/media/platform/pxa_camera.c index 9d3f0cb..7877037 100644

drivers/media/platform/pxa_camera.c +3 -1

@@ -2489,7 +2489,7 @@ static int pxa_camera_probe(struct platform_device
 *pdev) dev_set_drvdata(&pdev->dev, pcdev);
 	err = v4l2_device_register(&pdev->dev, &pcdev->v4l2_dev);
 	if (err)
-		goto exit_free_dma;
+		goto exit_deactivate;
 
 	pcdev->asds[0] = &pcdev->asd;
 	pcdev->notifier.subdevs = pcdev->asds;

@@ -2525,6 +2525,8 @@ static int pxa_camera_probe(struct platform_device
 *pdev) v4l2_clk_unregister(pcdev->mclk_clk);
 exit_free_v4l2dev:
 	v4l2_device_unregister(&pcdev->v4l2_dev);
+exit_deactivate:
+	pxa_camera_deactivate(pcdev);
 exit_free_dma:
 	dma_release_channel(pcdev->dma_chans[2]);
 exit_free_dma_u:

Signature · 3 lines

Regards,

Laurent Pinchart

Laurent Pinchart

2017-12-11 21:43 UTC #3 lore raw

Hi Flavio,

Quoted 10 lines · On Monday, 11 December 2017 23:05:46 EET Flavio Ceolin wrote:

Quoted 8 lines · On Wednesday, 6 December 2017 18:38:50 EET Flavio Ceolin wrote:
Quoted 5 lines
pxa_camera_probe() was not calling pxa_camera_deactivate(), responsible to call clk_disable_unprepare(), on the failure path. This was leading to unbalancing source clock.
Found by Linux Driver Verification project (linuxtesting.org).
Any chance I could sign you up for more work on this driver ? :-)
Definetely, this would be great :)

Actually it looks like the work I thought was needed has already been performed. The pxa-camera driver used to make use of the soc-camera framework, which we are trying to remove, and occurrences of soc_camera in the code gave me the wrong idea that the driver was still based on that framework. It seems this isn't the case. Thank you for making me happy :-)

Quoted 35 lines

Quoted 35 lines
Quoted 1 line
Signed-off-by Flavio Ceolin <[email protected]>
Reviewed-by Laurent Pinchart <[email protected]>
I expect Hans Verkuil to pick up the patch.
Quoted 29 lines
---
 drivers/media/platform/pxa_camera.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
(quoted diff) +0 -0
b/drivers/media/platform/pxa_camera.c index 9d3f0cb..7877037 100644
drivers/media/platform/pxa_camera.c +1 -1
@@ -2489,7 +2489,7 @@ static int pxa_camera_probe(struct platform_device
 *pdev)
 	dev_set_drvdata(&pdev->dev, pcdev);
 	err = v4l2_device_register(&pdev->dev, &pcdev->v4l2_dev);
 	if (err)
-		goto exit_free_dma;
+		goto exit_deactivate;
 
 	pcdev->asds[0] = &pcdev->asd;
drivers/media/platform/pxa_camera.c +2 -0
 	pcdev->notifier.subdevs = pcdev->asds;
@@ -2525,6 +2525,8 @@ static int pxa_camera_probe(struct platform_device
 *pdev)
 	v4l2_clk_unregister(pcdev->mclk_clk);
 exit_free_v4l2dev:
 	v4l2_device_unregister(&pcdev->v4l2_dev);
+exit_deactivate:
+	pxa_camera_deactivate(pcdev);
 exit_free_dma:
 	dma_release_channel(pcdev->dma_chans[2]);
exit_free_dma_u:

Signature · 3 lines

Regards,

Laurent Pinchart

Flavio Ceolin

author

2017-12-11 23:36 UTC #4 lore raw

Quoted 12 lines · Laurent Pinchart

Hi Flavio,
Quoted 9 lines · On Monday, 11 December 2017 23:05:46 EET Flavio Ceolin wrote:
Quoted 8 lines · On Wednesday, 6 December 2017 18:38:50 EET Flavio Ceolin wrote:
Quoted 5 lines
pxa_camera_probe() was not calling pxa_camera_deactivate(), responsible to call clk_disable_unprepare(), on the failure path. This was leading to unbalancing source clock.
Found by Linux Driver Verification project (linuxtesting.org).
Any chance I could sign you up for more work on this driver ? :-)

Quoted 7 lines

Quoted 1 line
Definetely, this would be great :)
Actually it looks like the work I thought was needed has already been performed. The pxa-camera driver used to make use of the soc-camera framework, which we are trying to remove, and occurrences of soc_camera in the code gave me the wrong idea that the driver was still based on that framework. It seems this isn't the case. Thank you for making me happy :-)

I'm glad to hear it :)

Quoted 24 lines

Quoted 24 lines
Quoted 24 lines
Quoted 2 lines
Signed-off-by Flavio Ceolin <[email protected]>
Reviewed-by Laurent Pinchart <[email protected]>
I expect Hans Verkuil to pick up the patch.
Quoted 17 lines
---
 drivers/media/platform/pxa_camera.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
(quoted diff) +0 -0
b/drivers/media/platform/pxa_camera.c index 9d3f0cb..7877037 100644
drivers/media/platform/pxa_camera.c +1 -1
@@ -2489,7 +2489,7 @@ static int pxa_camera_probe(struct platform_device
 *pdev)
 	dev_set_drvdata(&pdev->dev, pcdev);
 	err = v4l2_device_register(&pdev->dev, &pcdev->v4l2_dev);
 	if (err)
-		goto exit_free_dma;
+		goto exit_deactivate;
 

Quoted 17 lines

Quoted 12 lines

drivers/media/platform/pxa_camera.c +0 -0

 	pcdev->asds[0] = &pcdev->asd;

drivers/media/platform/pxa_camera.c +2 -0

 	pcdev->notifier.subdevs = pcdev->asds;

@@ -2525,6 +2525,8 @@ static int pxa_camera_probe(struct platform_device
 *pdev)
 	v4l2_clk_unregister(pcdev->mclk_clk);
 exit_free_v4l2dev:
 	v4l2_device_unregister(&pcdev->v4l2_dev);
+exit_deactivate:
+	pxa_camera_deactivate(pcdev);
 exit_free_dma:
 	dma_release_channel(pcdev->dma_chans[2]);

exit_free_dma_u:

Signature · 3 lines

Regards,

Laurent Pinchart

Regards,
Flavio Ceolin