Re: [PATCH] [media] V4L: soc-camera: change order of removing device

2011-11-29 Thread Guennadi Liakhovetski
Hi Lei

On Tue, 29 Nov 2011, Lei Wen wrote:

 Hi,
 
 On Tue, Nov 22, 2011 at 10:04 PM, Lei Wen lei...@marvell.com wrote:
  As our general practice, we use stream off before we close
  the video node. So that the drivers its stream off function
  would be called before its remove function.
 
  But for the case for ctrl+c, the program would be force closed.
  We have no chance to call that vb2 stream off from user space,
  but directly call the remove function in soc_camera.
 
  In that common code of soc_camera:
 
                 ici-ops-remove(icd);
                 if (ici-ops-init_videobuf2)
                         vb2_queue_release(icd-vb2_vidq);
 
  It would first call the device remove function, then release vb2,
  in which stream off function is called. Thus it create different
  order for the driver.
 
  This patch change the order to make driver see the same sequence
  to make it happy.

This is a change, that would affect all soc-camera host drivers. Since you 
don't describe the actual problem and the platform, on which it occurs, I 
suppose, it is not a regression, and, possibly, it only affects an 
off-tree driver from your employer. Therefore I don't think it would be 
reasonable to push it in the middle of an -rc* period. Still, this change 
seems logical, and I'll consider including it in the 3.3 kernel. I'll come 
back to you if I have further questions.

Thanks
Guennadi

 
  Signed-off-by: Lei Wen lei...@marvell.com
  ---
   drivers/media/video/soc_camera.c |    2 +-
   1 files changed, 1 insertions(+), 1 deletions(-)
 
  diff --git a/drivers/media/video/soc_camera.c 
  b/drivers/media/video/soc_camera.c
  index b72580c..fdc6167 100644
  --- a/drivers/media/video/soc_camera.c
  +++ b/drivers/media/video/soc_camera.c
  @@ -600,9 +600,9 @@ static int soc_camera_close(struct file *file)
                 pm_runtime_suspend(icd-vdev-dev);
                 pm_runtime_disable(icd-vdev-dev);
 
  -               ici-ops-remove(icd);
                 if (ici-ops-init_videobuf2)
                         vb2_queue_release(icd-vb2_vidq);
  +               ici-ops-remove(icd);
 
                 soc_camera_power_off(icd, icl);
         }
  --
  1.7.0.4
 
  --
  To unsubscribe from this list: send the line unsubscribe linux-media in
  the body of a message to majord...@vger.kernel.org
  More majordomo info at  http://vger.kernel.org/majordomo-info.html
 
 
 Any comments?
 
 Thanks,
 Lei
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

---
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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] [media] V4L: soc-camera: change order of removing device

2011-11-28 Thread Lei Wen
Hi,

On Tue, Nov 22, 2011 at 10:04 PM, Lei Wen lei...@marvell.com wrote:
 As our general practice, we use stream off before we close
 the video node. So that the drivers its stream off function
 would be called before its remove function.

 But for the case for ctrl+c, the program would be force closed.
 We have no chance to call that vb2 stream off from user space,
 but directly call the remove function in soc_camera.

 In that common code of soc_camera:

                ici-ops-remove(icd);
                if (ici-ops-init_videobuf2)
                        vb2_queue_release(icd-vb2_vidq);

 It would first call the device remove function, then release vb2,
 in which stream off function is called. Thus it create different
 order for the driver.

 This patch change the order to make driver see the same sequence
 to make it happy.

 Signed-off-by: Lei Wen lei...@marvell.com
 ---
  drivers/media/video/soc_camera.c |    2 +-
  1 files changed, 1 insertions(+), 1 deletions(-)

 diff --git a/drivers/media/video/soc_camera.c 
 b/drivers/media/video/soc_camera.c
 index b72580c..fdc6167 100644
 --- a/drivers/media/video/soc_camera.c
 +++ b/drivers/media/video/soc_camera.c
 @@ -600,9 +600,9 @@ static int soc_camera_close(struct file *file)
                pm_runtime_suspend(icd-vdev-dev);
                pm_runtime_disable(icd-vdev-dev);

 -               ici-ops-remove(icd);
                if (ici-ops-init_videobuf2)
                        vb2_queue_release(icd-vb2_vidq);
 +               ici-ops-remove(icd);

                soc_camera_power_off(icd, icl);
        }
 --
 1.7.0.4

 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html


Any comments?

Thanks,
Lei
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] [media] V4L: soc-camera: change order of removing device

2011-11-22 Thread Lei Wen
As our general practice, we use stream off before we close
the video node. So that the drivers its stream off function
would be called before its remove function.

But for the case for ctrl+c, the program would be force closed.
We have no chance to call that vb2 stream off from user space,
but directly call the remove function in soc_camera.

In that common code of soc_camera:

ici-ops-remove(icd);
if (ici-ops-init_videobuf2)
vb2_queue_release(icd-vb2_vidq);

It would first call the device remove function, then release vb2,
in which stream off function is called. Thus it create different
order for the driver.

This patch change the order to make driver see the same sequence
to make it happy.

Signed-off-by: Lei Wen lei...@marvell.com
---
 drivers/media/video/soc_camera.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index b72580c..fdc6167 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -600,9 +600,9 @@ static int soc_camera_close(struct file *file)
pm_runtime_suspend(icd-vdev-dev);
pm_runtime_disable(icd-vdev-dev);
 
-   ici-ops-remove(icd);
if (ici-ops-init_videobuf2)
vb2_queue_release(icd-vb2_vidq);
+   ici-ops-remove(icd);
 
soc_camera_power_off(icd, icl);
}
-- 
1.7.0.4

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html