[PATCH 00/57] media: i2c: Reduce cargo cult

[PATCH 00/57] media: i2c: Reduce cargo cult

[Laurent Pinchart]

Hello,

While working on camera sensor drivers, I got bothered by the amount of
bad practices we have grown slowly as a result of cargo-cult
programming. This patch series is an attempt to improve the situation by
addressing two common problems.

The first problem is addressed in patches 01/57 to 30/57. The subdev
.s_stream() operation is not reentrant, it should never be called to
start an already started subdev, or stop an already stopped one. This
requirement has never been formally documented, which has led many
drivers to track the streaming status and return immediately from
.s_stream() if the requested streaming state matches the current state.

Patch 01/57 first updates the documentation, and the next 29 patches
drop the unneeded checks from camera sensor drivers.

The second problem is addressed in patches 31/57 to 57/57. Starting and
stopping streaming on a camera pipeline requires coordination of the
different blocks. This is handled by the bridge driver, which starts and
stops streaming on the sensor explicitly with the .s_stream() operation.
Despite thus, many sensor drivers track the streaming state internally,
to stop the sensor in the system suspend handler if it is streaming, and
restart it in the system resume handler. This is not needed, and
couldn't work anyway in the general case as starting the sensor before
the bridge can prevent the bridge from synchronizing, for instance for
CSI-2 buses with a continuous clock.

Patches 31/57 to 34/57 refactor and improve the camera sensor
documentation, and the next 23 patches drop system suspend and resume
handlers from camera sensor drivers. Patch 53/57 is a bit of an oddball
as it drops stream handling from the runtime PM handlers instead, which
is even more incorrect as the sensor just can't be streaming already
when the runtime PM resume handler is called.

There are still a few related issues in drivers/media/i2c/ that I have
decided *not* to address in this series:

- The series focusses on camera sensor drivers, the lens controllers,
  flash controllers, and the ADV748x (HDMI/CVBS to CSI-2 converter)
  still implement system PM handlers.

- The et8ek8 driver still implements system PM handlers. While fixable,
  the change will be bigger as the sensor should also move away from the
  .s_power() operation. This requires more testing, which I can't easily
  perform.

- More generally, the series doesn't address drivers that rely on the
  .s_power() operation instead of implementing runtime PM (disregarding
  the drivers other than camera sensors, there are still 17 of them).

The second part of the series unfortunately depends on the first part.
I've decided to split the patches in two parts to clearly show the two
separate issues, and then split them further by driver. If strongly
desired, I could squash patches from the first and second parts together
when they address the same driver, although that will cause quite a bit
of churn to update all commit messages. I prefer keeping the per-driver
split to facilitate backporting.

Laurent Pinchart (57):
  media: v4l2-subdev: Document .s_stream() operation requirements
  media: i2c: hi556: Drop check for reentrant .s_stream()
  media: i2c: hi846: Drop check for reentrant .s_stream()
  media: i2c: imx208: Drop check for reentrant .s_stream()
  media: i2c: imx214: Drop check for reentrant .s_stream()
  media: i2c: imx219: Drop check for reentrant .s_stream()
  media: i2c: imx258: Drop check for reentrant .s_stream()
  media: i2c: imx319: Drop check for reentrant .s_stream()
  media: i2c: imx334: Drop check for reentrant .s_stream()
  media: i2c: imx335: Drop check for reentrant .s_stream()
  media: i2c: imx355: Drop check for reentrant .s_stream()
  media: i2c: imx412: Drop check for reentrant .s_stream()
  media: i2c: mt9m001: Drop check for reentrant .s_stream()
  media: i2c: og01a1b: Drop check for reentrant .s_stream()
  media: i2c: ov01a10: Drop check for reentrant .s_stream()
  media: i2c: ov08d10: Drop check for reentrant .s_stream()
  media: i2c: ov08x40: Drop check for reentrant .s_stream()
  media: i2c: ov13858: Drop check for reentrant .s_stream()
  media: i2c: ov13b10: Drop check for reentrant .s_stream()
  media: i2c: ov2685: Drop check for reentrant .s_stream()
  media: i2c: ov2740: Drop check for reentrant .s_stream()
  media: i2c: ov4689: Drop check for reentrant .s_stream()
  media: i2c: ov5647: Drop check for reentrant .s_stream()
  media: i2c: ov5670: Drop check for reentrant .s_stream()
  media: i2c: ov5675: Drop check for reentrant .s_stream()
  media: i2c: ov5695: Drop check for reentrant .s_stream()
  media: i2c: ov7740: Drop check for reentrant .s_stream()
  media: i2c: ov8856: Drop check for reentrant .s_stream()
  media: i2c: ov9282: Drop check for reentrant .s_stream()
  media: i2c: ov9734: Drop check for reentrant .s_stream()
  Documentation: media: camera-sensor: Fix typo and vocabulary selection
  Documentation: media: camera-sensor: Use link to upstream DT bindings
  Documentation: media: camera-sensor: Move power management section
  Documentation: media: camera-sensor: Improve power management
    documentation
  media: i2c: ar0521: Drop system suspend and resume handlers
  media: i2c: ccs: Drop system suspend and resume handlers
  media: i2c: hi556: Drop system suspend and resume handlers
  media: i2c: hi846: Drop system suspend and resume handlers
  media: i2c: hi847: Drop system suspend and resume handlers
  media: i2c: imx208: Drop system suspend and resume handlers
  media: i2c: imx214: Drop system suspend and resume handlers
  media: i2c: imx219: Drop system suspend and resume handlers
  media: i2c: imx258: Drop system suspend and resume handlers
  media: i2c: imx319: Drop system suspend and resume handlers
  media: i2c: imx355: Drop system suspend and resume handlers
  media: i2c: og01a1b: Drop system suspend and resume handlers
  media: i2c: ov01a10: Drop system suspend and resume handlers
  media: i2c: ov02a10: Drop system suspend and resume handlers
  media: i2c: ov08d10: Drop system suspend and resume handlers
  media: i2c: ov08x40: Drop system suspend and resume handlers
  media: i2c: ov13858: Drop system suspend and resume handlers
  media: i2c: ov2740: Drop system suspend and resume handlers
  media: i2c: ov13b10: Drop stream handling in runtime PM handlers
  media: i2c: ov5670: Drop system suspend and resume handlers
  media: i2c: ov5675: Drop system suspend and resume handlers
  media: i2c: ov8856: Drop system suspend and resume handlers
  media: i2c: ov9734: Drop system suspend and resume handlers

 .../driver-api/media/camera-sensor.rst        | 107 +++++++++++-------
 drivers/media/i2c/ar0521.c                    |  30 +----
 drivers/media/i2c/ccs/ccs-core.c              |  37 ------
 drivers/media/i2c/hi556.c                     |  50 --------
 drivers/media/i2c/hi846.c                     |  28 +----
 drivers/media/i2c/hi847.c                     |  52 ---------
 drivers/media/i2c/imx208.c                    |  47 --------
 drivers/media/i2c/imx214.c                    |  40 -------
 drivers/media/i2c/imx219.c                    |  44 -------
 drivers/media/i2c/imx258.c                    |  40 -------
 drivers/media/i2c/imx319.c                    |  44 -------
 drivers/media/i2c/imx334.c                    |   9 --
 drivers/media/i2c/imx335.c                    |   9 --
 drivers/media/i2c/imx355.c                    |  45 --------
 drivers/media/i2c/imx412.c                    |   9 --
 drivers/media/i2c/mt9m001.c                   |   6 -
 drivers/media/i2c/og01a1b.c                   |  50 --------
 drivers/media/i2c/ov01a10.c                   |  59 +---------
 drivers/media/i2c/ov02a10.c                   |   2 -
 drivers/media/i2c/ov08d10.c                   |  52 ---------
 drivers/media/i2c/ov08x40.c                   |  44 -------
 drivers/media/i2c/ov13858.c                   |  44 -------
 drivers/media/i2c/ov13b10.c                   |  38 +------
 drivers/media/i2c/ov2685.c                    |   7 --
 drivers/media/i2c/ov2740.c                    |  45 --------
 drivers/media/i2c/ov4689.c                    |   7 --
 drivers/media/i2c/ov5647.c                    |   6 -
 drivers/media/i2c/ov5670.c                    |  36 ------
 drivers/media/i2c/ov5675.c                    |  44 -------
 drivers/media/i2c/ov5695.c                    |   6 -
 drivers/media/i2c/ov7740.c                    |   7 --
 drivers/media/i2c/ov8856.c                    |  47 --------
 drivers/media/i2c/ov9282.c                    |   9 --
 drivers/media/i2c/ov9734.c                    |  50 --------
 include/media/v4l2-subdev.h                   |   4 +-
 35 files changed, 76 insertions(+), 1078 deletions(-)


base-commit: 0bb80ecc33a8fb5a682236443c1e740d5c917d1d
-- 
Regards,

Laurent Pinchart

[PATCH 01/57] media: v4l2-subdev: Document .s_stream() operation requirements

[Laurent Pinchart]

The subdev .s_stream() operation must not be called to start an already
started subdev, or stop an already stopped one. This requirement has
never been formally documented. Fix it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/media/v4l2-subdev.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
index d9fca929c10b..282914ea9d45 100644
--- a/include/media/v4l2-subdev.h
+++ b/include/media/v4l2-subdev.h
@@ -446,7 +446,9 @@ enum v4l2_subdev_pre_streamon_flags {
  * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the
  *	sub-device. Failure on stop will remove any resources acquired in
  *	streaming start, while the error code is still returned by the driver.
- *	Also see call_s_stream wrapper in v4l2-subdev.c.
+ *	The caller shall track the subdev state, and refrain from starting or
+ *	stopping an already started or stopped subdev. AAlso see call_s_stream
+ *	wrapper in v4l2-subdev.c.
  *
  * @g_pixelaspect: callback to return the pixelaspect ratio.
  *
-- 
Regards,

Laurent Pinchart

[PATCH 02/57] media: i2c: hi556: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/hi556.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/hi556.c b/drivers/media/i2c/hi556.c
index fd56ba138739..2b00426c5826 100644
--- a/drivers/media/i2c/hi556.c
+++ b/drivers/media/i2c/hi556.c
@@ -976,9 +976,6 @@ static int hi556_set_stream(struct v4l2_subdev *sd, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	if (hi556->streaming == enable)
-		return 0;
-
 	mutex_lock(&hi556->mutex);
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 03/57] media: i2c: hi846: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/hi846.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c
index fa0038749a3b..746e1f75f9d0 100644
--- a/drivers/media/i2c/hi846.c
+++ b/drivers/media/i2c/hi846.c
@@ -1607,9 +1607,6 @@ static int hi846_set_stream(struct v4l2_subdev *sd, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	if (hi846->streaming == enable)
-		return 0;
-
 	mutex_lock(&hi846->mutex);
 
 	if (enable) {
-- 
Regards,

Laurent Pinchart

[PATCH 04/57] media: i2c: imx208: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx208.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/imx208.c b/drivers/media/i2c/imx208.c
index ee5a28675388..871f857cf8b6 100644
--- a/drivers/media/i2c/imx208.c
+++ b/drivers/media/i2c/imx208.c
@@ -714,10 +714,6 @@ static int imx208_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&imx208->imx208_mx);
-	if (imx208->streaming == enable) {
-		mutex_unlock(&imx208->imx208_mx);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_get_sync(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 05/57] media: i2c: imx214: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx214.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
index 2f9c8582f940..e2805173f4b1 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -775,9 +775,6 @@ static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
 	struct imx214 *imx214 = to_imx214(subdev);
 	int ret;
 
-	if (imx214->streaming == enable)
-		return 0;
-
 	if (enable) {
 		ret = pm_runtime_resume_and_get(imx214->dev);
 		if (ret < 0)
-- 
Regards,

Laurent Pinchart

[PATCH 06/57] media: i2c: imx219: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx219.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index a1136fdfbed2..5715bbbc0820 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -979,9 +979,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable)
 
 	state = v4l2_subdev_lock_and_get_active_state(sd);
 
-	if (imx219->streaming == enable)
-		goto unlock;
-
 	if (enable) {
 		/*
 		 * Apply default & customized values
-- 
Regards,

Laurent Pinchart

[PATCH 07/57] media: i2c: imx258: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx258.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index e196565e846e..4be0422b7cda 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -1035,10 +1035,6 @@ static int imx258_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&imx258->mutex);
-	if (imx258->streaming == enable) {
-		mutex_unlock(&imx258->mutex);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 08/57] media: i2c: imx319: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx319.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
index 52ebb096e107..935bcbacd62e 100644
--- a/drivers/media/i2c/imx319.c
+++ b/drivers/media/i2c/imx319.c
@@ -2166,10 +2166,6 @@ static int imx319_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&imx319->mutex);
-	if (imx319->streaming == enable) {
-		mutex_unlock(&imx319->mutex);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 09/57] media: i2c: imx334: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx334.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/i2c/imx334.c b/drivers/media/i2c/imx334.c
index d722c9b7cd31..d8f91a492d14 100644
--- a/drivers/media/i2c/imx334.c
+++ b/drivers/media/i2c/imx334.c
@@ -120,7 +120,6 @@ struct imx334_mode {
  * @mutex: Mutex for serializing sensor controls
  * @menu_skip_mask: Menu skip mask for link_freq_ctrl
  * @cur_code: current selected format code
- * @streaming: Flag indicating streaming state
  */
 struct imx334 {
 	struct device *dev;
@@ -143,7 +142,6 @@ struct imx334 {
 	struct mutex mutex;
 	unsigned long menu_skip_mask;
 	u32 cur_code;
-	bool streaming;
 };
 
 static const s64 link_freq[] = {
@@ -1001,11 +999,6 @@ static int imx334_set_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&imx334->mutex);
 
-	if (imx334->streaming == enable) {
-		mutex_unlock(&imx334->mutex);
-		return 0;
-	}
-
 	if (enable) {
 		ret = pm_runtime_resume_and_get(imx334->dev);
 		if (ret < 0)
@@ -1019,8 +1012,6 @@ static int imx334_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(imx334->dev);
 	}
 
-	imx334->streaming = enable;
-
 	mutex_unlock(&imx334->mutex);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 10/57] media: i2c: imx335: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx335.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/i2c/imx335.c b/drivers/media/i2c/imx335.c
index 482a0b7f040a..ec729126274b 100644
--- a/drivers/media/i2c/imx335.c
+++ b/drivers/media/i2c/imx335.c
@@ -119,7 +119,6 @@ struct imx335_mode {
  * @vblank: Vertical blanking in lines
  * @cur_mode: Pointer to current selected sensor mode
  * @mutex: Mutex for serializing sensor controls
- * @streaming: Flag indicating streaming state
  */
 struct imx335 {
 	struct device *dev;
@@ -140,7 +139,6 @@ struct imx335 {
 	u32 vblank;
 	const struct imx335_mode *cur_mode;
 	struct mutex mutex;
-	bool streaming;
 };
 
 static const s64 link_freq[] = {
@@ -705,11 +703,6 @@ static int imx335_set_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&imx335->mutex);
 
-	if (imx335->streaming == enable) {
-		mutex_unlock(&imx335->mutex);
-		return 0;
-	}
-
 	if (enable) {
 		ret = pm_runtime_resume_and_get(imx335->dev);
 		if (ret)
@@ -723,8 +716,6 @@ static int imx335_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(imx335->dev);
 	}
 
-	imx335->streaming = enable;
-
 	mutex_unlock(&imx335->mutex);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 11/57] media: i2c: imx355: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx355.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
index 9c79ae8dc842..43c09941704e 100644
--- a/drivers/media/i2c/imx355.c
+++ b/drivers/media/i2c/imx355.c
@@ -1436,10 +1436,6 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&imx355->mutex);
-	if (imx355->streaming == enable) {
-		mutex_unlock(&imx355->mutex);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 12/57] media: i2c: imx412: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx412.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/i2c/imx412.c b/drivers/media/i2c/imx412.c
index c7e862ae4040..962b3136c31e 100644
--- a/drivers/media/i2c/imx412.c
+++ b/drivers/media/i2c/imx412.c
@@ -127,7 +127,6 @@ static const char * const imx412_supply_names[] = {
  * @vblank: Vertical blanking in lines
  * @cur_mode: Pointer to current selected sensor mode
  * @mutex: Mutex for serializing sensor controls
- * @streaming: Flag indicating streaming state
  */
 struct imx412 {
 	struct device *dev;
@@ -149,7 +148,6 @@ struct imx412 {
 	u32 vblank;
 	const struct imx412_mode *cur_mode;
 	struct mutex mutex;
-	bool streaming;
 };
 
 static const s64 link_freq[] = {
@@ -857,11 +855,6 @@ static int imx412_set_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&imx412->mutex);
 
-	if (imx412->streaming == enable) {
-		mutex_unlock(&imx412->mutex);
-		return 0;
-	}
-
 	if (enable) {
 		ret = pm_runtime_resume_and_get(imx412->dev);
 		if (ret)
@@ -875,8 +868,6 @@ static int imx412_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(imx412->dev);
 	}
 
-	imx412->streaming = enable;
-
 	mutex_unlock(&imx412->mutex);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 13/57] media: i2c: mt9m001: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/mt9m001.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/i2c/mt9m001.c b/drivers/media/i2c/mt9m001.c
index ce9568e8391c..79192cf79d28 100644
--- a/drivers/media/i2c/mt9m001.c
+++ b/drivers/media/i2c/mt9m001.c
@@ -93,7 +93,6 @@ struct mt9m001 {
 		struct v4l2_ctrl *autoexposure;
 		struct v4l2_ctrl *exposure;
 	};
-	bool streaming;
 	struct mutex mutex;
 	struct v4l2_rect rect;	/* Sensor window */
 	struct clk *clk;
@@ -213,9 +212,6 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&mt9m001->mutex);
 
-	if (mt9m001->streaming == enable)
-		goto done;
-
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
 		if (ret < 0)
@@ -239,8 +235,6 @@ static int mt9m001_s_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	mt9m001->streaming = enable;
-done:
 	mutex_unlock(&mt9m001->mutex);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 14/57] media: i2c: og01a1b: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/og01a1b.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/og01a1b.c b/drivers/media/i2c/og01a1b.c
index 365ce5684583..ab8381c52503 100644
--- a/drivers/media/i2c/og01a1b.c
+++ b/drivers/media/i2c/og01a1b.c
@@ -732,9 +732,6 @@ static int og01a1b_set_stream(struct v4l2_subdev *sd, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	if (og01a1b->streaming == enable)
-		return 0;
-
 	mutex_lock(&og01a1b->mutex);
 	if (enable) {
 		ret = pm_runtime_get_sync(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 15/57] media: i2c: ov01a10: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov01a10.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index 2b9e1b3a3bf4..b43c1c96f47b 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -672,8 +672,6 @@ static int ov01a10_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	state = v4l2_subdev_lock_and_get_active_state(sd);
-	if (ov01a10->streaming == enable)
-		goto unlock;
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 16/57] media: i2c: ov08d10: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov08d10.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/ov08d10.c b/drivers/media/i2c/ov08d10.c
index 7d55d4ca24de..ef811711549c 100644
--- a/drivers/media/i2c/ov08d10.c
+++ b/drivers/media/i2c/ov08d10.c
@@ -1103,9 +1103,6 @@ static int ov08d10_set_stream(struct v4l2_subdev *sd, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	if (ov08d10->streaming == enable)
-		return 0;
-
 	mutex_lock(&ov08d10->mutex);
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 17/57] media: i2c: ov08x40: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov08x40.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/ov08x40.c b/drivers/media/i2c/ov08x40.c
index 637da4df6901..1ea402550837 100644
--- a/drivers/media/i2c/ov08x40.c
+++ b/drivers/media/i2c/ov08x40.c
@@ -2915,10 +2915,6 @@ static int ov08x40_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&ov08x->mutex);
-	if (ov08x->streaming == enable) {
-		mutex_unlock(&ov08x->mutex);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 18/57] media: i2c: ov13858: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov13858.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index 35652b362347..b90ce338a47a 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -1467,10 +1467,6 @@ static int ov13858_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&ov13858->mutex);
-	if (ov13858->streaming == enable) {
-		mutex_unlock(&ov13858->mutex);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 19/57] media: i2c: ov13b10: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov13b10.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/ov13b10.c b/drivers/media/i2c/ov13b10.c
index dbc642c5995b..d191683a0826 100644
--- a/drivers/media/i2c/ov13b10.c
+++ b/drivers/media/i2c/ov13b10.c
@@ -1161,10 +1161,6 @@ static int ov13b10_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&ov13b->mutex);
-	if (ov13b->streaming == enable) {
-		mutex_unlock(&ov13b->mutex);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 20/57] media: i2c: ov2685: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov2685.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/i2c/ov2685.c b/drivers/media/i2c/ov2685.c
index 303793e1f97d..2cde2d767874 100644
--- a/drivers/media/i2c/ov2685.c
+++ b/drivers/media/i2c/ov2685.c
@@ -91,7 +91,6 @@ struct ov2685 {
 	struct gpio_desc	*reset_gpio;
 	struct regulator_bulk_data supplies[OV2685_NUM_SUPPLIES];
 
-	bool			streaming;
 	struct mutex		mutex;
 	struct v4l2_subdev	subdev;
 	struct media_pad	pad;
@@ -513,10 +512,6 @@ static int ov2685_s_stream(struct v4l2_subdev *sd, int on)
 
 	mutex_lock(&ov2685->mutex);
 
-	on = !!on;
-	if (on == ov2685->streaming)
-		goto unlock_and_return;
-
 	if (on) {
 		ret = pm_runtime_resume_and_get(&ov2685->client->dev);
 		if (ret < 0)
@@ -539,8 +534,6 @@ static int ov2685_s_stream(struct v4l2_subdev *sd, int on)
 		pm_runtime_put(&ov2685->client->dev);
 	}
 
-	ov2685->streaming = on;
-
 unlock_and_return:
 	mutex_unlock(&ov2685->mutex);
 
-- 
Regards,

Laurent Pinchart

[PATCH 21/57] media: i2c: ov2740: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov2740.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 41d4f85470fd..59356dd9afd8 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -794,9 +794,6 @@ static int ov2740_set_stream(struct v4l2_subdev *sd, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	if (ov2740->streaming == enable)
-		return 0;
-
 	mutex_lock(&ov2740->mutex);
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 22/57] media: i2c: ov4689: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov4689.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
index fda217d2cb10..cd8d7131e747 100644
--- a/drivers/media/i2c/ov4689.c
+++ b/drivers/media/i2c/ov4689.c
@@ -100,7 +100,6 @@ struct ov4689 {
 	u32 clock_rate;
 
 	struct mutex mutex; /* lock to protect streaming, ctrls and cur_mode */
-	bool streaming;
 	struct v4l2_ctrl_handler ctrl_handler;
 	struct v4l2_ctrl *exposure;
 
@@ -468,10 +467,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
 
 	mutex_lock(&ov4689->mutex);
 
-	on = !!on;
-	if (on == ov4689->streaming)
-		goto unlock_and_return;
-
 	if (on) {
 		ret = pm_runtime_resume_and_get(&client->dev);
 		if (ret < 0)
@@ -504,8 +499,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov4689->streaming = on;
-
 unlock_and_return:
 	mutex_unlock(&ov4689->mutex);
 
-- 
Regards,

Laurent Pinchart

[PATCH 23/57] media: i2c: ov5647: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5647.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/i2c/ov5647.c b/drivers/media/i2c/ov5647.c
index 8de398423b7c..dcfe3129c63a 100644
--- a/drivers/media/i2c/ov5647.c
+++ b/drivers/media/i2c/ov5647.c
@@ -109,7 +109,6 @@ struct ov5647 {
 	struct v4l2_ctrl		*hblank;
 	struct v4l2_ctrl		*vblank;
 	struct v4l2_ctrl		*exposure;
-	bool				streaming;
 };
 
 static inline struct ov5647 *to_sensor(struct v4l2_subdev *sd)
@@ -898,10 +897,6 @@ static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
 	int ret;
 
 	mutex_lock(&sensor->lock);
-	if (sensor->streaming == enable) {
-		mutex_unlock(&sensor->lock);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
@@ -922,7 +917,6 @@ static int ov5647_s_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	sensor->streaming = enable;
 	mutex_unlock(&sensor->lock);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 24/57] media: i2c: ov5670: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5670.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index 29e773a997dd..afd84c8e5ee7 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -2471,8 +2471,6 @@ static int ov5670_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&ov5670->mutex);
-	if (ov5670->streaming == enable)
-		goto unlock_and_return;
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 25/57] media: i2c: ov5675: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5675.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
index d5a2a5f82312..f608ee0c0cec 100644
--- a/drivers/media/i2c/ov5675.c
+++ b/drivers/media/i2c/ov5675.c
@@ -949,9 +949,6 @@ static int ov5675_set_stream(struct v4l2_subdev *sd, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	if (ov5675->streaming == enable)
-		return 0;
-
 	mutex_lock(&ov5675->mutex);
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 26/57] media: i2c: ov5695: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5695.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/media/i2c/ov5695.c b/drivers/media/i2c/ov5695.c
index 3023b7254167..13506a4f9500 100644
--- a/drivers/media/i2c/ov5695.c
+++ b/drivers/media/i2c/ov5695.c
@@ -108,7 +108,6 @@ struct ov5695 {
 	struct v4l2_ctrl	*vblank;
 	struct v4l2_ctrl	*test_pattern;
 	struct mutex		mutex;
-	bool			streaming;
 	const struct ov5695_mode *cur_mode;
 };
 
@@ -942,9 +941,6 @@ static int ov5695_s_stream(struct v4l2_subdev *sd, int on)
 	int ret = 0;
 
 	mutex_lock(&ov5695->mutex);
-	on = !!on;
-	if (on == ov5695->streaming)
-		goto unlock_and_return;
 
 	if (on) {
 		ret = pm_runtime_resume_and_get(&client->dev);
@@ -962,8 +958,6 @@ static int ov5695_s_stream(struct v4l2_subdev *sd, int on)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov5695->streaming = on;
-
 unlock_and_return:
 	mutex_unlock(&ov5695->mutex);
 
-- 
Regards,

Laurent Pinchart

[PATCH 27/57] media: i2c: ov7740: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov7740.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/media/i2c/ov7740.c b/drivers/media/i2c/ov7740.c
index dffdb475e433..2f76029e9a9b 100644
--- a/drivers/media/i2c/ov7740.c
+++ b/drivers/media/i2c/ov7740.c
@@ -120,7 +120,6 @@ struct ov7740 {
 	struct v4l2_ctrl *contrast;
 
 	struct mutex mutex;	/* To serialize asynchronus callbacks */
-	bool streaming;		/* Streaming on/off */
 
 	struct gpio_desc *resetb_gpio;
 	struct gpio_desc *pwdn_gpio;
@@ -618,10 +617,6 @@ static int ov7740_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&ov7740->mutex);
-	if (ov7740->streaming == enable) {
-		mutex_unlock(&ov7740->mutex);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
@@ -635,8 +630,6 @@ static int ov7740_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov7740->streaming = enable;
-
 	mutex_unlock(&ov7740->mutex);
 	return ret;
 
-- 
Regards,

Laurent Pinchart

[PATCH 28/57] media: i2c: ov8856: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov8856.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
index f053c3a7676a..5447a3361976 100644
--- a/drivers/media/i2c/ov8856.c
+++ b/drivers/media/i2c/ov8856.c
@@ -2042,9 +2042,6 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	if (ov8856->streaming == enable)
-		return 0;
-
 	mutex_lock(&ov8856->mutex);
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 29/57] media: i2c: ov9282: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov9282.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/drivers/media/i2c/ov9282.c b/drivers/media/i2c/ov9282.c
index 068c7449f50e..bf6dfce1b5dd 100644
--- a/drivers/media/i2c/ov9282.c
+++ b/drivers/media/i2c/ov9282.c
@@ -165,7 +165,6 @@ struct ov9282_mode {
  * @cur_mode: Pointer to current selected sensor mode
  * @code: Mbus code currently selected
  * @mutex: Mutex for serializing sensor controls
- * @streaming: Flag indicating streaming state
  */
 struct ov9282 {
 	struct device *dev;
@@ -188,7 +187,6 @@ struct ov9282 {
 	const struct ov9282_mode *cur_mode;
 	u32 code;
 	struct mutex mutex;
-	bool streaming;
 };
 
 static const s64 link_freq[] = {
@@ -1037,11 +1035,6 @@ static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
 
 	mutex_lock(&ov9282->mutex);
 
-	if (ov9282->streaming == enable) {
-		mutex_unlock(&ov9282->mutex);
-		return 0;
-	}
-
 	if (enable) {
 		ret = pm_runtime_resume_and_get(ov9282->dev);
 		if (ret)
@@ -1055,8 +1048,6 @@ static int ov9282_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(ov9282->dev);
 	}
 
-	ov9282->streaming = enable;
-
 	mutex_unlock(&ov9282->mutex);
 
 	return 0;
-- 
Regards,

Laurent Pinchart

[PATCH 30/57] media: i2c: ov9734: Drop check for reentrant .s_stream()

[Laurent Pinchart]

The subdev .s_stream() operation shall not be called to start streaming
on an already started subdev, or stop streaming on a stopped subdev.
Remove the check that guards against that condition.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov9734.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/media/i2c/ov9734.c b/drivers/media/i2c/ov9734.c
index b6244772bc59..a5f91499bce4 100644
--- a/drivers/media/i2c/ov9734.c
+++ b/drivers/media/i2c/ov9734.c
@@ -660,10 +660,6 @@ static int ov9734_set_stream(struct v4l2_subdev *sd, int enable)
 	int ret = 0;
 
 	mutex_lock(&ov9734->mutex);
-	if (ov9734->streaming == enable) {
-		mutex_unlock(&ov9734->mutex);
-		return 0;
-	}
 
 	if (enable) {
 		ret = pm_runtime_resume_and_get(&client->dev);
-- 
Regards,

Laurent Pinchart

[PATCH 31/57] Documentation: media: camera-sensor: Fix typo and vocabulary selection

[Laurent Pinchart]

Drop an unneeded double colon, and use 'shall' instead of 'must' for
consistency with the rest of the file.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/driver-api/media/camera-sensor.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index 93f4f2536c25..96cfbc261b6e 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -13,7 +13,7 @@ Handling clocks
 
 Camera sensors have an internal clock tree including a PLL and a number of
 divisors. The clock tree is generally configured by the driver based on a few
-input parameters that are specific to the hardware:: the external clock frequency
+input parameters that are specific to the hardware: the external clock frequency
 and the link frequency. The two parameters generally are obtained from system
 firmware. **No other frequencies should be used in any circumstances.**
 
@@ -123,7 +123,7 @@ Power management
 Always use runtime PM to manage the power states of your device. Camera sensor
 drivers are in no way special in this respect: they are responsible for
 controlling the power state of the device they otherwise control as well. In
-general, the device must be powered on at least when its registers are being
+general, the device shall be powered on at least when its registers are being
 accessed and when it is streaming.
 
 Existing camera sensor drivers may rely on the old
-- 
Regards,

Laurent Pinchart

[PATCH 32/57] Documentation: media: camera-sensor: Use link to upstream DT bindings

[Laurent Pinchart]

The Documentation/devicetree/bindings/clock/clock-bindings.txt file is
deprecated and points to clock-bindings.yaml, which is not hosted in the
kernel source tree. Use an HTTPS link to refer to the YAML binding
document.

While at it, drop "currently" from the paragraph, as the whole file
refers to the current recommended practices except where explicitly
noted.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 Documentation/driver-api/media/camera-sensor.rst | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index 96cfbc261b6e..3510a57ecb9a 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -32,10 +32,11 @@ can rely on this frequency being used.
 Devicetree
 ~~~~~~~~~~
 
-The currently preferred way to achieve this is using ``assigned-clocks``,
-``assigned-clock-parents`` and ``assigned-clock-rates`` properties. See
-``Documentation/devicetree/bindings/clock/clock-bindings.txt`` for more
-information. The driver then gets the frequency using ``clk_get_rate()``.
+The preferred way to achieve this is using ``assigned-clocks``,
+``assigned-clock-parents`` and ``assigned-clock-rates`` properties. See the
+`clock device tree bindings <https://github.com/devicetree-org/dt-schema/blob/main/dtschema/schemas/clock/clock.yaml>`_
+for more information. The driver then gets the frequency using
+``clk_get_rate()``.
 
 This approach has the drawback that there's no guarantee that the frequency
 hasn't been modified directly or indirectly by another driver, or supported by
-- 
Regards,

Laurent Pinchart

[PATCH 33/57] Documentation: media: camera-sensor: Move power management section

[Laurent Pinchart]

Move the power management section up, just after clocks, as it relates
to internal system resources and not features exposed to applications.
The text itself is otherwise unchanged.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../driver-api/media/camera-sensor.rst        | 70 +++++++++----------
 1 file changed, 35 insertions(+), 35 deletions(-)

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index 3510a57ecb9a..8ab166a2138d 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -43,6 +43,41 @@ hasn't been modified directly or indirectly by another driver, or supported by
 the board's clock tree to begin with. Changes to the Common Clock Framework API
 are required to ensure reliability.
 
+Power management
+----------------
+
+Always use runtime PM to manage the power states of your device. Camera sensor
+drivers are in no way special in this respect: they are responsible for
+controlling the power state of the device they otherwise control as well. In
+general, the device shall be powered on at least when its registers are being
+accessed and when it is streaming.
+
+Existing camera sensor drivers may rely on the old
+struct v4l2_subdev_core_ops->s_power() callback for bridge or ISP drivers to
+manage their power state. This is however **deprecated**. If you feel you need
+to begin calling an s_power from an ISP or a bridge driver, instead please add
+runtime PM support to the sensor driver you are using. Likewise, new drivers
+should not use s_power.
+
+Please see examples in e.g. ``drivers/media/i2c/ov8856.c`` and
+``drivers/media/i2c/ccs/ccs-core.c``. The two drivers work in both ACPI
+and DT based systems.
+
+Control framework
+~~~~~~~~~~~~~~~~~
+
+``v4l2_ctrl_handler_setup()`` function may not be used in the device's runtime
+PM ``runtime_resume`` callback, as it has no way to figure out the power state
+of the device. This is because the power state of the device is only changed
+after the power state transition has taken place. The ``s_ctrl`` callback can be
+used to obtain device's power state after the power state transition:
+
+.. c:function:: int pm_runtime_get_if_in_use(struct device *dev);
+
+The function returns a non-zero value if it succeeded getting the power count or
+runtime PM was disabled, in either of which cases the driver may proceed to
+access the device.
+
 Frame size
 ----------
 
@@ -118,41 +153,6 @@ rate) on device level in firmware or hardware. This means lower level controls
 implemented by raw cameras may not be used on uAPI (or even kAPI) to control the
 frame interval on these devices.
 
-Power management
-----------------
-
-Always use runtime PM to manage the power states of your device. Camera sensor
-drivers are in no way special in this respect: they are responsible for
-controlling the power state of the device they otherwise control as well. In
-general, the device shall be powered on at least when its registers are being
-accessed and when it is streaming.
-
-Existing camera sensor drivers may rely on the old
-struct v4l2_subdev_core_ops->s_power() callback for bridge or ISP drivers to
-manage their power state. This is however **deprecated**. If you feel you need
-to begin calling an s_power from an ISP or a bridge driver, instead please add
-runtime PM support to the sensor driver you are using. Likewise, new drivers
-should not use s_power.
-
-Please see examples in e.g. ``drivers/media/i2c/ov8856.c`` and
-``drivers/media/i2c/ccs/ccs-core.c``. The two drivers work in both ACPI
-and DT based systems.
-
-Control framework
-~~~~~~~~~~~~~~~~~
-
-``v4l2_ctrl_handler_setup()`` function may not be used in the device's runtime
-PM ``runtime_resume`` callback, as it has no way to figure out the power state
-of the device. This is because the power state of the device is only changed
-after the power state transition has taken place. The ``s_ctrl`` callback can be
-used to obtain device's power state after the power state transition:
-
-.. c:function:: int pm_runtime_get_if_in_use(struct device *dev);
-
-The function returns a non-zero value if it succeeded getting the power count or
-runtime PM was disabled, in either of which cases the driver may proceed to
-access the device.
-
 Rotation, orientation and flipping
 ----------------------------------
 
-- 
Regards,

Laurent Pinchart

[PATCH 34/57] Documentation: media: camera-sensor: Improve power management documentation

[Laurent Pinchart]

Camera sensor drivers are highly subject to cargo cult programming, with
back practices being copied from old to new drivers. In particular, many
drivers implement system and runtime PM incorrectly. As a first step
towards fixing this situation, refactor and expand the power management
documentation to detail correct usage of system and runtime PM.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../driver-api/media/camera-sensor.rst        | 54 ++++++++++++++-----
 1 file changed, 40 insertions(+), 14 deletions(-)

diff --git a/Documentation/driver-api/media/camera-sensor.rst b/Documentation/driver-api/media/camera-sensor.rst
index 8ab166a2138d..2acc08142a1a 100644
--- a/Documentation/driver-api/media/camera-sensor.rst
+++ b/Documentation/driver-api/media/camera-sensor.rst
@@ -46,22 +46,48 @@ are required to ensure reliability.
 Power management
 ----------------
 
-Always use runtime PM to manage the power states of your device. Camera sensor
-drivers are in no way special in this respect: they are responsible for
-controlling the power state of the device they otherwise control as well. In
-general, the device shall be powered on at least when its registers are being
-accessed and when it is streaming.
+Camera sensors are used in conjunction with other devices to form a camera
+pipeline. They must obey the rules listed herein to ensure coherent power
+management over the pipeline.
 
-Existing camera sensor drivers may rely on the old
-struct v4l2_subdev_core_ops->s_power() callback for bridge or ISP drivers to
-manage their power state. This is however **deprecated**. If you feel you need
-to begin calling an s_power from an ISP or a bridge driver, instead please add
-runtime PM support to the sensor driver you are using. Likewise, new drivers
-should not use s_power.
+Camera sensor drivers are responsible for controlling the power state of the
+device they otherwise control as well. They shall use runtime PM to manage
+power states. Runtime PM shall be enabled at probe time and disabled at remove
+time. Drivers should enable runtime PM autosuspend.
 
-Please see examples in e.g. ``drivers/media/i2c/ov8856.c`` and
-``drivers/media/i2c/ccs/ccs-core.c``. The two drivers work in both ACPI
-and DT based systems.
+The runtime PM handlers shall handle clocks, regulators, GPIOs, and other
+system resources required to power the sensor up and down. For drivers that
+don't use any of those resources (such as drivers that support ACPI systems
+only), the runtime PM handlers may be left unimplemented.
+
+In general, the device shall be powered on at least when its registers are
+being accessed and when it is streaming. Drivers should use
+``pm_runtime_resume_and_get()`` when starting streaming and
+``pm_runtime_put()`` or ``pm_runtime_put_autosuspend()`` when stopping
+streaming. They may power the device up at probe time (for example to read
+identification registers), but should not keep it powered unconditionally after
+probe.
+
+At system suspend time, the whole camera pipeline must stop streaming, and
+restart when the system is resumed. This requires coordination between the
+camera sensor and the rest of the camera pipeline. Bridge drivers are
+responsible for this coordination, and instruct camera sensors to stop and
+restart streaming by calling the appropriate subdev operations
+(``.s_stream()``, ``.enable_streams()`` or ``.disable_streams()``). Camera
+sensor drivers shall therefore **not** keep track of the streaming state to
+stop streaming in the PM suspend handler and restart it in the resume handler.
+Drivers should in general not implement the system PM handlers.
+
+Camera sensor drivers shall **not** implement the subdev ``.s_power()``
+operation, as it is deprecated. While this operation is implemented in some
+existing drivers as they predate the deprecation, new drivers shall use runtime
+PM instead. If you feel you need to begin calling ``.s_power()`` from an ISP or
+a bridge driver, instead add runtime PM support to the sensor driver you are
+using and drop its ``.s_power()`` handler.
+
+See examples of runtime PM handling in e.g. ``drivers/media/i2c/ov8856.c`` and
+``drivers/media/i2c/ccs/ccs-core.c``. The two drivers work in both ACPI and DT
+based systems.
 
 Control framework
 ~~~~~~~~~~~~~~~~~
-- 
Regards,

Laurent Pinchart

[PATCH 35/57] media: i2c: ar0521: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ar0521.c | 30 +-----------------------------
 1 file changed, 1 insertion(+), 29 deletions(-)

diff --git a/drivers/media/i2c/ar0521.c b/drivers/media/i2c/ar0521.c
index a4e39871e8f7..701f36345f1e 100644
--- a/drivers/media/i2c/ar0521.c
+++ b/drivers/media/i2c/ar0521.c
@@ -133,8 +133,6 @@ struct ar0521_dev {
 		u16 mult2;
 		u16 vt_pix;
 	} pll;
-
-	bool streaming;
 };
 
 static inline struct ar0521_dev *to_ar0521_dev(struct v4l2_subdev *sd)
@@ -991,12 +989,9 @@ static int ar0521_s_stream(struct v4l2_subdev *sd, int enable)
 	int ret;
 
 	mutex_lock(&sensor->lock);
-
 	ret = ar0521_set_stream(sensor, enable);
-	if (!ret)
-		sensor->streaming = enable;
-
 	mutex_unlock(&sensor->lock);
+
 	return ret;
 }
 
@@ -1023,28 +1018,6 @@ static const struct v4l2_subdev_ops ar0521_subdev_ops = {
 	.pad = &ar0521_pad_ops,
 };
 
-static int __maybe_unused ar0521_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ar0521_dev *sensor = to_ar0521_dev(sd);
-
-	if (sensor->streaming)
-		ar0521_set_stream(sensor, 0);
-
-	return 0;
-}
-
-static int __maybe_unused ar0521_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ar0521_dev *sensor = to_ar0521_dev(sd);
-
-	if (sensor->streaming)
-		return ar0521_set_stream(sensor, 1);
-
-	return 0;
-}
-
 static int ar0521_probe(struct i2c_client *client)
 {
 	struct v4l2_fwnode_endpoint ep = {
@@ -1183,7 +1156,6 @@ static void ar0521_remove(struct i2c_client *client)
 }
 
 static const struct dev_pm_ops ar0521_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ar0521_suspend, ar0521_resume)
 	SET_RUNTIME_PM_OPS(ar0521_power_off, ar0521_power_on, NULL)
 };
 static const struct of_device_id ar0521_dt_ids[] = {
-- 
Regards,

Laurent Pinchart

[PATCH 36/57] media: i2c: ccs: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ccs/ccs-core.c | 37 --------------------------------
 1 file changed, 37 deletions(-)

diff --git a/drivers/media/i2c/ccs/ccs-core.c b/drivers/media/i2c/ccs/ccs-core.c
index 49e0d9a09530..c1b08a758830 100644
--- a/drivers/media/i2c/ccs/ccs-core.c
+++ b/drivers/media/i2c/ccs/ccs-core.c
@@ -3152,42 +3152,6 @@ static const struct v4l2_subdev_internal_ops ccs_internal_ops = {
  * I2C Driver
  */
 
-static int __maybe_unused ccs_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *subdev = i2c_get_clientdata(client);
-	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
-	bool streaming = sensor->streaming;
-	int rval;
-
-	rval = pm_runtime_resume_and_get(dev);
-	if (rval < 0)
-		return rval;
-
-	if (sensor->streaming)
-		ccs_stop_streaming(sensor);
-
-	/* save state for resume */
-	sensor->streaming = streaming;
-
-	return 0;
-}
-
-static int __maybe_unused ccs_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *subdev = i2c_get_clientdata(client);
-	struct ccs_sensor *sensor = to_ccs_sensor(subdev);
-	int rval = 0;
-
-	pm_runtime_put(dev);
-
-	if (sensor->streaming)
-		rval = ccs_start_streaming(sensor);
-
-	return rval;
-}
-
 static int ccs_get_hwconfig(struct ccs_sensor *sensor, struct device *dev)
 {
 	struct ccs_hwconfig *hwcfg = &sensor->hwcfg;
@@ -3720,7 +3684,6 @@ static const struct of_device_id ccs_of_table[] = {
 MODULE_DEVICE_TABLE(of, ccs_of_table);
 
 static const struct dev_pm_ops ccs_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ccs_suspend, ccs_resume)
 	SET_RUNTIME_PM_OPS(ccs_power_off, ccs_power_on, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 37/57] media: i2c: hi556: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/hi556.c | 47 ---------------------------------------
 1 file changed, 47 deletions(-)

diff --git a/drivers/media/i2c/hi556.c b/drivers/media/i2c/hi556.c
index 2b00426c5826..e6e7049108e7 100644
--- a/drivers/media/i2c/hi556.c
+++ b/drivers/media/i2c/hi556.c
@@ -577,9 +577,6 @@ struct hi556 {
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	/* True if the device has been identified */
 	bool identified;
 };
@@ -995,50 +992,11 @@ static int hi556_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	hi556->streaming = enable;
 	mutex_unlock(&hi556->mutex);
 
 	return ret;
 }
 
-static int __maybe_unused hi556_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct hi556 *hi556 = to_hi556(sd);
-
-	mutex_lock(&hi556->mutex);
-	if (hi556->streaming)
-		hi556_stop_streaming(hi556);
-
-	mutex_unlock(&hi556->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused hi556_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct hi556 *hi556 = to_hi556(sd);
-	int ret;
-
-	mutex_lock(&hi556->mutex);
-	if (hi556->streaming) {
-		ret = hi556_start_streaming(hi556);
-		if (ret)
-			goto error;
-	}
-
-	mutex_unlock(&hi556->mutex);
-
-	return 0;
-
-error:
-	hi556_stop_streaming(hi556);
-	hi556->streaming = 0;
-	mutex_unlock(&hi556->mutex);
-	return ret;
-}
-
 static int hi556_set_format(struct v4l2_subdev *sd,
 			    struct v4l2_subdev_state *sd_state,
 			    struct v4l2_subdev_format *fmt)
@@ -1328,10 +1286,6 @@ static int hi556_probe(struct i2c_client *client)
 	return ret;
 }
 
-static const struct dev_pm_ops hi556_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(hi556_suspend, hi556_resume)
-};
-
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id hi556_acpi_ids[] = {
 	{"INT3537"},
@@ -1344,7 +1298,6 @@ MODULE_DEVICE_TABLE(acpi, hi556_acpi_ids);
 static struct i2c_driver hi556_i2c_driver = {
 	.driver = {
 		.name = "hi556",
-		.pm = &hi556_pm_ops,
 		.acpi_match_table = ACPI_PTR(hi556_acpi_ids),
 	},
 	.probe = hi556_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 38/57] media: i2c: hi846: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

Now that the runtime PM operations are not called from the system PM
handlers, the sensor is guaranteed not to be streaming when the runtime
PM resume handler is called, and to have been stopped already when the
runtime PM suspend handler is called. Drop the manual stream start/stop
from the runtime PM handlers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/hi846.c | 25 +------------------------
 1 file changed, 1 insertion(+), 24 deletions(-)

diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c
index 746e1f75f9d0..d643b0aab998 100644
--- a/drivers/media/i2c/hi846.c
+++ b/drivers/media/i2c/hi846.c
@@ -1677,9 +1677,6 @@ static int __maybe_unused hi846_suspend(struct device *dev)
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	struct hi846 *hi846 = to_hi846(sd);
 
-	if (hi846->streaming)
-		hi846_stop_streaming(hi846);
-
 	return hi846_power_off(hi846);
 }
 
@@ -1688,26 +1685,8 @@ static int __maybe_unused hi846_resume(struct device *dev)
 	struct i2c_client *client = to_i2c_client(dev);
 	struct v4l2_subdev *sd = i2c_get_clientdata(client);
 	struct hi846 *hi846 = to_hi846(sd);
-	int ret;
 
-	ret = hi846_power_on(hi846);
-	if (ret)
-		return ret;
-
-	if (hi846->streaming) {
-		ret = hi846_start_streaming(hi846);
-		if (ret) {
-			dev_err(dev, "%s: start streaming failed: %d\n",
-				__func__, ret);
-			goto error;
-		}
-	}
-
-	return 0;
-
-error:
-	hi846_power_off(hi846);
-	return ret;
+	return hi846_power_on(hi846);
 }
 
 static int hi846_set_format(struct v4l2_subdev *sd,
@@ -2170,8 +2149,6 @@ static void hi846_remove(struct i2c_client *client)
 }
 
 static const struct dev_pm_ops hi846_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
-				pm_runtime_force_resume)
 	SET_RUNTIME_PM_OPS(hi846_suspend, hi846_resume, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 39/57] media: i2c: hi847: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/hi847.c | 52 ---------------------------------------
 1 file changed, 52 deletions(-)

diff --git a/drivers/media/i2c/hi847.c b/drivers/media/i2c/hi847.c
index 32547d7a2659..35a334646ff5 100644
--- a/drivers/media/i2c/hi847.c
+++ b/drivers/media/i2c/hi847.c
@@ -2184,9 +2184,6 @@ struct hi847 {
 
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
-
-	/* Streaming on/off */
-	bool streaming;
 };
 
 static u64 to_pixel_rate(u32 f_index)
@@ -2618,9 +2615,6 @@ static int hi847_set_stream(struct v4l2_subdev *sd, int enable)
 	struct i2c_client *client = v4l2_get_subdevdata(sd);
 	int ret = 0;
 
-	if (hi847->streaming == enable)
-		return 0;
-
 	mutex_lock(&hi847->mutex);
 	if (enable) {
 		ret = pm_runtime_get_sync(&client->dev);
@@ -2641,52 +2635,11 @@ static int hi847_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	hi847->streaming = enable;
 	mutex_unlock(&hi847->mutex);
 
 	return ret;
 }
 
-static int __maybe_unused hi847_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct hi847 *hi847 = to_hi847(sd);
-
-	mutex_lock(&hi847->mutex);
-	if (hi847->streaming)
-		hi847_stop_streaming(hi847);
-
-	mutex_unlock(&hi847->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused hi847_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct hi847 *hi847 = to_hi847(sd);
-	int ret;
-
-	mutex_lock(&hi847->mutex);
-	if (hi847->streaming) {
-		ret = hi847_start_streaming(hi847);
-		if (ret)
-			goto error;
-	}
-
-	mutex_unlock(&hi847->mutex);
-
-	return 0;
-
-error:
-	hi847_stop_streaming(hi847);
-	hi847->streaming = 0;
-	mutex_unlock(&hi847->mutex);
-	return ret;
-}
-
 static int hi847_set_format(struct v4l2_subdev *sd,
 			    struct v4l2_subdev_state *sd_state,
 			    struct v4l2_subdev_format *fmt)
@@ -2980,10 +2933,6 @@ static int hi847_probe(struct i2c_client *client)
 	return ret;
 }
 
-static const struct dev_pm_ops hi847_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(hi847_suspend, hi847_resume)
-};
-
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id hi847_acpi_ids[] = {
 	{"HYV0847"},
@@ -2996,7 +2945,6 @@ MODULE_DEVICE_TABLE(acpi, hi847_acpi_ids);
 static struct i2c_driver hi847_i2c_driver = {
 	.driver = {
 		.name = "hi847",
-		.pm = &hi847_pm_ops,
 		.acpi_match_table = ACPI_PTR(hi847_acpi_ids),
 	},
 	.probe = hi847_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 40/57] media: i2c: imx208: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx208.c | 43 --------------------------------------
 1 file changed, 43 deletions(-)

diff --git a/drivers/media/i2c/imx208.c b/drivers/media/i2c/imx208.c
index 871f857cf8b6..35e42444351a 100644
--- a/drivers/media/i2c/imx208.c
+++ b/drivers/media/i2c/imx208.c
@@ -290,9 +290,6 @@ struct imx208 {
 	 */
 	struct mutex imx208_mx;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	/* OTP data */
 	bool otp_read;
 	char otp_data[IMX208_OTP_SIZE];
@@ -732,7 +729,6 @@ static int imx208_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	imx208->streaming = enable;
 	mutex_unlock(&imx208->imx208_mx);
 
 	/* vflip and hflip cannot change during streaming */
@@ -748,40 +744,6 @@ static int imx208_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
-static int __maybe_unused imx208_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct imx208 *imx208 = to_imx208(sd);
-
-	if (imx208->streaming)
-		imx208_stop_streaming(imx208);
-
-	return 0;
-}
-
-static int __maybe_unused imx208_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct imx208 *imx208 = to_imx208(sd);
-	int ret;
-
-	if (imx208->streaming) {
-		ret = imx208_start_streaming(imx208);
-		if (ret)
-			goto error;
-	}
-
-	return 0;
-
-error:
-	imx208_stop_streaming(imx208);
-	imx208->streaming = 0;
-
-	return ret;
-}
-
 /* Verify chip ID */
 static const struct v4l2_subdev_video_ops imx208_video_ops = {
 	.s_stream = imx208_set_stream,
@@ -1077,10 +1039,6 @@ static void imx208_remove(struct i2c_client *client)
 	mutex_destroy(&imx208->imx208_mx);
 }
 
-static const struct dev_pm_ops imx208_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(imx208_suspend, imx208_resume)
-};
-
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id imx208_acpi_ids[] = {
 	{ "INT3478" },
@@ -1093,7 +1051,6 @@ MODULE_DEVICE_TABLE(acpi, imx208_acpi_ids);
 static struct i2c_driver imx208_i2c_driver = {
 	.driver = {
 		.name = "imx208",
-		.pm = &imx208_pm_ops,
 		.acpi_match_table = ACPI_PTR(imx208_acpi_ids),
 	},
 	.probe = imx208_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 41/57] media: i2c: imx214: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx214.c | 37 -------------------------------------
 1 file changed, 37 deletions(-)

diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
index e2805173f4b1..4f77ea02cc27 100644
--- a/drivers/media/i2c/imx214.c
+++ b/drivers/media/i2c/imx214.c
@@ -58,8 +58,6 @@ struct imx214 {
 	 * and start streaming.
 	 */
 	struct mutex mutex;
-
-	bool streaming;
 };
 
 struct reg_8 {
@@ -790,7 +788,6 @@ static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
 		pm_runtime_put(imx214->dev);
 	}
 
-	imx214->streaming = enable;
 	return 0;
 
 err_rpm_put:
@@ -906,39 +903,6 @@ static int imx214_parse_fwnode(struct device *dev)
 	return ret;
 }
 
-static int __maybe_unused imx214_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct imx214 *imx214 = to_imx214(sd);
-
-	if (imx214->streaming)
-		imx214_stop_streaming(imx214);
-
-	return 0;
-}
-
-static int __maybe_unused imx214_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct imx214 *imx214 = to_imx214(sd);
-	int ret;
-
-	if (imx214->streaming) {
-		ret = imx214_start_streaming(imx214);
-		if (ret)
-			goto error;
-	}
-
-	return 0;
-
-error:
-	imx214_stop_streaming(imx214);
-	imx214->streaming = 0;
-	return ret;
-}
-
 static int imx214_probe(struct i2c_client *client)
 {
 	struct device *dev = &client->dev;
@@ -1099,7 +1063,6 @@ static const struct of_device_id imx214_of_match[] = {
 MODULE_DEVICE_TABLE(of, imx214_of_match);
 
 static const struct dev_pm_ops imx214_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(imx214_suspend, imx214_resume)
 	SET_RUNTIME_PM_OPS(imx214_power_off, imx214_power_on, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 42/57] media: i2c: imx219: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx219.c | 41 --------------------------------------
 1 file changed, 41 deletions(-)

diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
index 5715bbbc0820..a431dace0f33 100644
--- a/drivers/media/i2c/imx219.c
+++ b/drivers/media/i2c/imx219.c
@@ -479,9 +479,6 @@ struct imx219 {
 	/* Current mode */
 	const struct imx219_mode *mode;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	/* Two or Four lanes */
 	u8 lanes;
 };
@@ -991,8 +988,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable)
 		imx219_stop_streaming(imx219);
 	}
 
-	imx219->streaming = enable;
-
 unlock:
 	v4l2_subdev_unlock_state(state);
 	return ret;
@@ -1044,41 +1039,6 @@ static int imx219_power_off(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused imx219_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct imx219 *imx219 = to_imx219(sd);
-
-	if (imx219->streaming)
-		imx219_stop_streaming(imx219);
-
-	return 0;
-}
-
-static int __maybe_unused imx219_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct imx219 *imx219 = to_imx219(sd);
-	struct v4l2_subdev_state *state;
-	int ret;
-
-	if (imx219->streaming) {
-		state = v4l2_subdev_lock_and_get_active_state(sd);
-		ret = imx219_start_streaming(imx219, state);
-		v4l2_subdev_unlock_state(state);
-		if (ret)
-			goto error;
-	}
-
-	return 0;
-
-error:
-	imx219_stop_streaming(imx219);
-	imx219->streaming = false;
-
-	return ret;
-}
-
 static int imx219_get_regulators(struct imx219 *imx219)
 {
 	struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
@@ -1464,7 +1424,6 @@ static const struct of_device_id imx219_dt_ids[] = {
 MODULE_DEVICE_TABLE(of, imx219_dt_ids);
 
 static const struct dev_pm_ops imx219_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(imx219_suspend, imx219_resume)
 	SET_RUNTIME_PM_OPS(imx219_power_off, imx219_power_on, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 43/57] media: i2c: imx258: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx258.c | 36 ------------------------------------
 1 file changed, 36 deletions(-)

diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 4be0422b7cda..b3827f4bc0eb 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -622,9 +622,6 @@ struct imx258 {
 	 */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	struct clk *clk;
 };
 
@@ -1053,7 +1050,6 @@ static int imx258_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	imx258->streaming = enable;
 	mutex_unlock(&imx258->mutex);
 
 	return ret;
@@ -1066,37 +1062,6 @@ static int imx258_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
-static int __maybe_unused imx258_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct imx258 *imx258 = to_imx258(sd);
-
-	if (imx258->streaming)
-		imx258_stop_streaming(imx258);
-
-	return 0;
-}
-
-static int __maybe_unused imx258_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct imx258 *imx258 = to_imx258(sd);
-	int ret;
-
-	if (imx258->streaming) {
-		ret = imx258_start_streaming(imx258);
-		if (ret)
-			goto error;
-	}
-
-	return 0;
-
-error:
-	imx258_stop_streaming(imx258);
-	imx258->streaming = 0;
-	return ret;
-}
-
 /* Verify chip ID */
 static int imx258_identify_module(struct imx258 *imx258)
 {
@@ -1365,7 +1330,6 @@ static void imx258_remove(struct i2c_client *client)
 }
 
 static const struct dev_pm_ops imx258_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(imx258_suspend, imx258_resume)
 	SET_RUNTIME_PM_OPS(imx258_power_off, imx258_power_on, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 44/57] media: i2c: imx319: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx319.c | 40 --------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/media/i2c/imx319.c b/drivers/media/i2c/imx319.c
index 935bcbacd62e..5378f607f340 100644
--- a/drivers/media/i2c/imx319.c
+++ b/drivers/media/i2c/imx319.c
@@ -138,8 +138,6 @@ struct imx319 {
 	 */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
 	/* True if the device has been identified */
 	bool identified;
 };
@@ -2184,8 +2182,6 @@ static int imx319_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	imx319->streaming = enable;
-
 	/* vflip and hflip cannot change during streaming */
 	__v4l2_ctrl_grab(imx319->vflip, enable);
 	__v4l2_ctrl_grab(imx319->hflip, enable);
@@ -2202,37 +2198,6 @@ static int imx319_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
-static int __maybe_unused imx319_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct imx319 *imx319 = to_imx319(sd);
-
-	if (imx319->streaming)
-		imx319_stop_streaming(imx319);
-
-	return 0;
-}
-
-static int __maybe_unused imx319_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct imx319 *imx319 = to_imx319(sd);
-	int ret;
-
-	if (imx319->streaming) {
-		ret = imx319_start_streaming(imx319);
-		if (ret)
-			goto error;
-	}
-
-	return 0;
-
-error:
-	imx319_stop_streaming(imx319);
-	imx319->streaming = 0;
-	return ret;
-}
-
 static const struct v4l2_subdev_core_ops imx319_subdev_core_ops = {
 	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
 	.unsubscribe_event = v4l2_event_subdev_unsubscribe,
@@ -2538,10 +2503,6 @@ static void imx319_remove(struct i2c_client *client)
 	mutex_destroy(&imx319->mutex);
 }
 
-static const struct dev_pm_ops imx319_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(imx319_suspend, imx319_resume)
-};
-
 static const struct acpi_device_id imx319_acpi_ids[] __maybe_unused = {
 	{ "SONY319A" },
 	{ /* sentinel */ }
@@ -2551,7 +2512,6 @@ MODULE_DEVICE_TABLE(acpi, imx319_acpi_ids);
 static struct i2c_driver imx319_i2c_driver = {
 	.driver = {
 		.name = "imx319",
-		.pm = &imx319_pm_ops,
 		.acpi_match_table = ACPI_PTR(imx319_acpi_ids),
 	},
 	.probe = imx319_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 45/57] media: i2c: imx355: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/imx355.c | 41 --------------------------------------
 1 file changed, 41 deletions(-)

diff --git a/drivers/media/i2c/imx355.c b/drivers/media/i2c/imx355.c
index 43c09941704e..9c58c1a80cba 100644
--- a/drivers/media/i2c/imx355.c
+++ b/drivers/media/i2c/imx355.c
@@ -123,9 +123,6 @@ struct imx355 {
 	 * Protect access to sensor v4l2 controls.
 	 */
 	struct mutex mutex;
-
-	/* Streaming on/off */
-	bool streaming;
 };
 
 static const struct imx355_reg imx355_global_regs[] = {
@@ -1454,8 +1451,6 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	imx355->streaming = enable;
-
 	/* vflip and hflip cannot change during streaming */
 	__v4l2_ctrl_grab(imx355->vflip, enable);
 	__v4l2_ctrl_grab(imx355->hflip, enable);
@@ -1472,37 +1467,6 @@ static int imx355_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
-static int __maybe_unused imx355_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct imx355 *imx355 = to_imx355(sd);
-
-	if (imx355->streaming)
-		imx355_stop_streaming(imx355);
-
-	return 0;
-}
-
-static int __maybe_unused imx355_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct imx355 *imx355 = to_imx355(sd);
-	int ret;
-
-	if (imx355->streaming) {
-		ret = imx355_start_streaming(imx355);
-		if (ret)
-			goto error;
-	}
-
-	return 0;
-
-error:
-	imx355_stop_streaming(imx355);
-	imx355->streaming = 0;
-	return ret;
-}
-
 /* Verify chip ID */
 static int imx355_identify_module(struct imx355 *imx355)
 {
@@ -1825,10 +1789,6 @@ static void imx355_remove(struct i2c_client *client)
 	mutex_destroy(&imx355->mutex);
 }
 
-static const struct dev_pm_ops imx355_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(imx355_suspend, imx355_resume)
-};
-
 static const struct acpi_device_id imx355_acpi_ids[] __maybe_unused = {
 	{ "SONY355A" },
 	{ /* sentinel */ }
@@ -1838,7 +1798,6 @@ MODULE_DEVICE_TABLE(acpi, imx355_acpi_ids);
 static struct i2c_driver imx355_i2c_driver = {
 	.driver = {
 		.name = "imx355",
-		.pm = &imx355_pm_ops,
 		.acpi_match_table = ACPI_PTR(imx355_acpi_ids),
 	},
 	.probe = imx355_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 46/57] media: i2c: og01a1b: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/og01a1b.c | 47 -------------------------------------
 1 file changed, 47 deletions(-)

diff --git a/drivers/media/i2c/og01a1b.c b/drivers/media/i2c/og01a1b.c
index ab8381c52503..3e0de797aa4d 100644
--- a/drivers/media/i2c/og01a1b.c
+++ b/drivers/media/i2c/og01a1b.c
@@ -434,9 +434,6 @@ struct og01a1b {
 
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
-
-	/* Streaming on/off */
-	bool streaming;
 };
 
 static u64 to_pixel_rate(u32 f_index)
@@ -752,50 +749,11 @@ static int og01a1b_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	og01a1b->streaming = enable;
 	mutex_unlock(&og01a1b->mutex);
 
 	return ret;
 }
 
-static int __maybe_unused og01a1b_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct og01a1b *og01a1b = to_og01a1b(sd);
-
-	mutex_lock(&og01a1b->mutex);
-	if (og01a1b->streaming)
-		og01a1b_stop_streaming(og01a1b);
-
-	mutex_unlock(&og01a1b->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused og01a1b_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct og01a1b *og01a1b = to_og01a1b(sd);
-	int ret;
-
-	mutex_lock(&og01a1b->mutex);
-	if (og01a1b->streaming) {
-		ret = og01a1b_start_streaming(og01a1b);
-		if (ret) {
-			og01a1b->streaming = false;
-			og01a1b_stop_streaming(og01a1b);
-			mutex_unlock(&og01a1b->mutex);
-			return ret;
-		}
-	}
-
-	mutex_unlock(&og01a1b->mutex);
-
-	return 0;
-}
-
 static int og01a1b_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_state *sd_state,
 			      struct v4l2_subdev_format *fmt)
@@ -1093,10 +1051,6 @@ static int og01a1b_probe(struct i2c_client *client)
 	return ret;
 }
 
-static const struct dev_pm_ops og01a1b_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(og01a1b_suspend, og01a1b_resume)
-};
-
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id og01a1b_acpi_ids[] = {
 	{"OVTI01AC"},
@@ -1109,7 +1063,6 @@ MODULE_DEVICE_TABLE(acpi, og01a1b_acpi_ids);
 static struct i2c_driver og01a1b_i2c_driver = {
 	.driver = {
 		.name = "og01a1b",
-		.pm = &og01a1b_pm_ops,
 		.acpi_match_table = ACPI_PTR(og01a1b_acpi_ids),
 	},
 	.probe = og01a1b_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 47/57] media: i2c: ov01a10: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov01a10.c | 57 ++-----------------------------------
 1 file changed, 3 insertions(+), 54 deletions(-)

diff --git a/drivers/media/i2c/ov01a10.c b/drivers/media/i2c/ov01a10.c
index b43c1c96f47b..bbd5740d2280 100644
--- a/drivers/media/i2c/ov01a10.c
+++ b/drivers/media/i2c/ov01a10.c
@@ -287,9 +287,6 @@ struct ov01a10 {
 	struct v4l2_ctrl *exposure;
 
 	const struct ov01a10_mode *cur_mode;
-
-	/* streaming state */
-	bool streaming;
 };
 
 static inline struct ov01a10 *to_ov01a10(struct v4l2_subdev *subdev)
@@ -683,60 +680,17 @@ static int ov01a10_set_stream(struct v4l2_subdev *sd, int enable)
 			pm_runtime_put(&client->dev);
 			goto unlock;
 		}
-
-		goto done;
+	} else {
+		ov01a10_stop_streaming(ov01a10);
+		pm_runtime_put(&client->dev);
 	}
 
-	ov01a10_stop_streaming(ov01a10);
-	pm_runtime_put(&client->dev);
-done:
-	ov01a10->streaming = enable;
 unlock:
 	v4l2_subdev_unlock_state(state);
 
 	return ret;
 }
 
-static int __maybe_unused ov01a10_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov01a10 *ov01a10 = to_ov01a10(sd);
-	struct v4l2_subdev_state *state;
-
-	state = v4l2_subdev_lock_and_get_active_state(sd);
-	if (ov01a10->streaming)
-		ov01a10_stop_streaming(ov01a10);
-
-	v4l2_subdev_unlock_state(state);
-
-	return 0;
-}
-
-static int __maybe_unused ov01a10_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov01a10 *ov01a10 = to_ov01a10(sd);
-	struct v4l2_subdev_state *state;
-	int ret = 0;
-
-	state = v4l2_subdev_lock_and_get_active_state(sd);
-	if (!ov01a10->streaming)
-		goto exit;
-
-	ret = ov01a10_start_streaming(ov01a10);
-	if (ret) {
-		ov01a10->streaming = false;
-		ov01a10_stop_streaming(ov01a10);
-	}
-
-exit:
-	v4l2_subdev_unlock_state(state);
-
-	return ret;
-}
-
 static int ov01a10_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_state *sd_state,
 			      struct v4l2_subdev_format *fmt)
@@ -971,10 +925,6 @@ static int ov01a10_probe(struct i2c_client *client)
 	return ret;
 }
 
-static const struct dev_pm_ops ov01a10_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov01a10_suspend, ov01a10_resume)
-};
-
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ov01a10_acpi_ids[] = {
 	{ "OVTI01A0" },
@@ -987,7 +937,6 @@ MODULE_DEVICE_TABLE(acpi, ov01a10_acpi_ids);
 static struct i2c_driver ov01a10_i2c_driver = {
 	.driver = {
 		.name = "ov01a10",
-		.pm = &ov01a10_pm_ops,
 		.acpi_match_table = ACPI_PTR(ov01a10_acpi_ids),
 	},
 	.probe = ov01a10_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 48/57] media: i2c: ov02a10: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov02a10.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/media/i2c/ov02a10.c b/drivers/media/i2c/ov02a10.c
index 741d977a76f3..848e47a464ac 100644
--- a/drivers/media/i2c/ov02a10.c
+++ b/drivers/media/i2c/ov02a10.c
@@ -570,8 +570,6 @@ static int ov02a10_s_stream(struct v4l2_subdev *sd, int on)
 }
 
 static const struct dev_pm_ops ov02a10_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
-				pm_runtime_force_resume)
 	SET_RUNTIME_PM_OPS(ov02a10_power_off, ov02a10_power_on, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 49/57] media: i2c: ov08d10: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov08d10.c | 49 -------------------------------------
 1 file changed, 49 deletions(-)

diff --git a/drivers/media/i2c/ov08d10.c b/drivers/media/i2c/ov08d10.c
index ef811711549c..3d49e3fa8e56 100644
--- a/drivers/media/i2c/ov08d10.c
+++ b/drivers/media/i2c/ov08d10.c
@@ -536,9 +536,6 @@ struct ov08d10 {
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	/* lanes index */
 	u8 nlanes;
 
@@ -1122,8 +1119,6 @@ static int ov08d10_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov08d10->streaming = enable;
-
 	/* vflip and hflip cannot change during streaming */
 	__v4l2_ctrl_grab(ov08d10->vflip, enable);
 	__v4l2_ctrl_grab(ov08d10->hflip, enable);
@@ -1133,45 +1128,6 @@ static int ov08d10_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
-static int __maybe_unused ov08d10_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov08d10 *ov08d10 = to_ov08d10(sd);
-
-	mutex_lock(&ov08d10->mutex);
-	if (ov08d10->streaming)
-		ov08d10_stop_streaming(ov08d10);
-
-	mutex_unlock(&ov08d10->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused ov08d10_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov08d10 *ov08d10 = to_ov08d10(sd);
-	int ret;
-
-	mutex_lock(&ov08d10->mutex);
-
-	if (ov08d10->streaming) {
-		ret = ov08d10_start_streaming(ov08d10);
-		if (ret) {
-			ov08d10->streaming = false;
-			ov08d10_stop_streaming(ov08d10);
-			mutex_unlock(&ov08d10->mutex);
-			return ret;
-		}
-	}
-
-	mutex_unlock(&ov08d10->mutex);
-
-	return 0;
-}
-
 static int ov08d10_set_format(struct v4l2_subdev *sd,
 			      struct v4l2_subdev_state *sd_state,
 			      struct v4l2_subdev_format *fmt)
@@ -1498,10 +1454,6 @@ static int ov08d10_probe(struct i2c_client *client)
 	return ret;
 }
 
-static const struct dev_pm_ops ov08d10_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov08d10_suspend, ov08d10_resume)
-};
-
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ov08d10_acpi_ids[] = {
 	{ "OVTI08D1" },
@@ -1514,7 +1466,6 @@ MODULE_DEVICE_TABLE(acpi, ov08d10_acpi_ids);
 static struct i2c_driver ov08d10_i2c_driver = {
 	.driver = {
 		.name = "ov08d10",
-		.pm = &ov08d10_pm_ops,
 		.acpi_match_table = ACPI_PTR(ov08d10_acpi_ids),
 	},
 	.probe = ov08d10_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 50/57] media: i2c: ov08x40: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov08x40.c | 40 -------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/media/i2c/ov08x40.c b/drivers/media/i2c/ov08x40.c
index 1ea402550837..b41b6866a0ab 100644
--- a/drivers/media/i2c/ov08x40.c
+++ b/drivers/media/i2c/ov08x40.c
@@ -2432,9 +2432,6 @@ struct ov08x40 {
 
 	/* Mutex for serialized access */
 	struct mutex mutex;
-
-	/* Streaming on/off */
-	bool streaming;
 };
 
 #define to_ov08x40(_sd)	container_of(_sd, struct ov08x40, sd)
@@ -2933,7 +2930,6 @@ static int ov08x40_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov08x->streaming = enable;
 	mutex_unlock(&ov08x->mutex);
 
 	return ret;
@@ -2946,37 +2942,6 @@ static int ov08x40_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
-static int __maybe_unused ov08x40_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov08x40 *ov08x = to_ov08x40(sd);
-
-	if (ov08x->streaming)
-		ov08x40_stop_streaming(ov08x);
-
-	return 0;
-}
-
-static int __maybe_unused ov08x40_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov08x40 *ov08x = to_ov08x40(sd);
-	int ret;
-
-	if (ov08x->streaming) {
-		ret = ov08x40_start_streaming(ov08x);
-		if (ret)
-			goto error;
-	}
-
-	return 0;
-
-error:
-	ov08x40_stop_streaming(ov08x);
-	ov08x->streaming = false;
-	return ret;
-}
-
 /* Verify chip ID */
 static int ov08x40_identify_module(struct ov08x40 *ov08x)
 {
@@ -3290,10 +3255,6 @@ static void ov08x40_remove(struct i2c_client *client)
 	pm_runtime_set_suspended(&client->dev);
 }
 
-static const struct dev_pm_ops ov08x40_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov08x40_suspend, ov08x40_resume)
-};
-
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ov08x40_acpi_ids[] = {
 	{"OVTI08F4"},
@@ -3306,7 +3267,6 @@ MODULE_DEVICE_TABLE(acpi, ov08x40_acpi_ids);
 static struct i2c_driver ov08x40_i2c_driver = {
 	.driver = {
 		.name = "ov08x40",
-		.pm = &ov08x40_pm_ops,
 		.acpi_match_table = ACPI_PTR(ov08x40_acpi_ids),
 	},
 	.probe = ov08x40_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 51/57] media: i2c: ov13858: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov13858.c | 40 -------------------------------------
 1 file changed, 40 deletions(-)

diff --git a/drivers/media/i2c/ov13858.c b/drivers/media/i2c/ov13858.c
index b90ce338a47a..4c419014dd7b 100644
--- a/drivers/media/i2c/ov13858.c
+++ b/drivers/media/i2c/ov13858.c
@@ -1044,9 +1044,6 @@ struct ov13858 {
 
 	/* Mutex for serialized access */
 	struct mutex mutex;
-
-	/* Streaming on/off */
-	bool streaming;
 };
 
 #define to_ov13858(_sd)	container_of(_sd, struct ov13858, sd)
@@ -1485,7 +1482,6 @@ static int ov13858_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov13858->streaming = enable;
 	mutex_unlock(&ov13858->mutex);
 
 	return ret;
@@ -1498,37 +1494,6 @@ static int ov13858_set_stream(struct v4l2_subdev *sd, int enable)
 	return ret;
 }
 
-static int __maybe_unused ov13858_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov13858 *ov13858 = to_ov13858(sd);
-
-	if (ov13858->streaming)
-		ov13858_stop_streaming(ov13858);
-
-	return 0;
-}
-
-static int __maybe_unused ov13858_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov13858 *ov13858 = to_ov13858(sd);
-	int ret;
-
-	if (ov13858->streaming) {
-		ret = ov13858_start_streaming(ov13858);
-		if (ret)
-			goto error;
-	}
-
-	return 0;
-
-error:
-	ov13858_stop_streaming(ov13858);
-	ov13858->streaming = false;
-	return ret;
-}
-
 /* Verify chip ID */
 static int ov13858_identify_module(struct ov13858 *ov13858)
 {
@@ -1783,10 +1748,6 @@ static const struct i2c_device_id ov13858_id_table[] = {
 
 MODULE_DEVICE_TABLE(i2c, ov13858_id_table);
 
-static const struct dev_pm_ops ov13858_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov13858_suspend, ov13858_resume)
-};
-
 #ifdef CONFIG_ACPI
 static const struct acpi_device_id ov13858_acpi_ids[] = {
 	{"OVTID858"},
@@ -1799,7 +1760,6 @@ MODULE_DEVICE_TABLE(acpi, ov13858_acpi_ids);
 static struct i2c_driver ov13858_i2c_driver = {
 	.driver = {
 		.name = "ov13858",
-		.pm = &ov13858_pm_ops,
 		.acpi_match_table = ACPI_PTR(ov13858_acpi_ids),
 	},
 	.probe = ov13858_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 52/57] media: i2c: ov2740: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov2740.c | 42 --------------------------------------
 1 file changed, 42 deletions(-)

diff --git a/drivers/media/i2c/ov2740.c b/drivers/media/i2c/ov2740.c
index 59356dd9afd8..3a9700fbbe8c 100644
--- a/drivers/media/i2c/ov2740.c
+++ b/drivers/media/i2c/ov2740.c
@@ -339,9 +339,6 @@ struct ov2740 {
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	/* NVM data inforamtion */
 	struct nvm_data *nvm;
 
@@ -813,47 +810,11 @@ static int ov2740_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov2740->streaming = enable;
 	mutex_unlock(&ov2740->mutex);
 
 	return ret;
 }
 
-static int ov2740_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov2740 *ov2740 = to_ov2740(sd);
-
-	mutex_lock(&ov2740->mutex);
-	if (ov2740->streaming)
-		ov2740_stop_streaming(ov2740);
-
-	mutex_unlock(&ov2740->mutex);
-
-	return 0;
-}
-
-static int ov2740_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov2740 *ov2740 = to_ov2740(sd);
-	int ret = 0;
-
-	mutex_lock(&ov2740->mutex);
-	if (!ov2740->streaming)
-		goto exit;
-
-	ret = ov2740_start_streaming(ov2740);
-	if (ret) {
-		ov2740->streaming = false;
-		ov2740_stop_streaming(ov2740);
-	}
-
-exit:
-	mutex_unlock(&ov2740->mutex);
-	return ret;
-}
-
 static int ov2740_set_format(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *sd_state,
 			     struct v4l2_subdev_format *fmt)
@@ -1197,8 +1158,6 @@ static int ov2740_probe(struct i2c_client *client)
 	return ret;
 }
 
-static DEFINE_SIMPLE_DEV_PM_OPS(ov2740_pm_ops, ov2740_suspend, ov2740_resume);
-
 static const struct acpi_device_id ov2740_acpi_ids[] = {
 	{"INT3474"},
 	{}
@@ -1209,7 +1168,6 @@ MODULE_DEVICE_TABLE(acpi, ov2740_acpi_ids);
 static struct i2c_driver ov2740_i2c_driver = {
 	.driver = {
 		.name = "ov2740",
-		.pm = pm_sleep_ptr(&ov2740_pm_ops),
 		.acpi_match_table = ov2740_acpi_ids,
 	},
 	.probe = ov2740_probe,
-- 
Regards,

Laurent Pinchart

[PATCH 53/57] media: i2c: ov13b10: Drop stream handling in runtime PM handlers

[Laurent Pinchart]

The sensor is guaranteed not to be streaming when the runtime PM resume
handler is called, as the resume handler is only called from the
.s_stream() operation when starting streaming. Similarly, the sensor has
been stopped when the runtime PM suspend handler is called. There is
thus no need for manual stream start/stop from the runtime PM handlers.
Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov13b10.c | 34 +---------------------------------
 1 file changed, 1 insertion(+), 33 deletions(-)

diff --git a/drivers/media/i2c/ov13b10.c b/drivers/media/i2c/ov13b10.c
index d191683a0826..ce157c9aa8b7 100644
--- a/drivers/media/i2c/ov13b10.c
+++ b/drivers/media/i2c/ov13b10.c
@@ -594,9 +594,6 @@ struct ov13b10 {
 	/* Mutex for serialized access */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	/* True if the device has been identified */
 	bool identified;
 };
@@ -1179,7 +1176,6 @@ static int ov13b10_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov13b->streaming = enable;
 	mutex_unlock(&ov13b->mutex);
 
 	return ret;
@@ -1194,12 +1190,6 @@ static int ov13b10_set_stream(struct v4l2_subdev *sd, int enable)
 
 static int ov13b10_suspend(struct device *dev)
 {
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov13b10 *ov13b = to_ov13b10(sd);
-
-	if (ov13b->streaming)
-		ov13b10_stop_streaming(ov13b);
-
 	ov13b10_power_off(dev);
 
 	return 0;
@@ -1207,29 +1197,7 @@ static int ov13b10_suspend(struct device *dev)
 
 static int ov13b10_resume(struct device *dev)
 {
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov13b10 *ov13b = to_ov13b10(sd);
-	int ret;
-
-	ret = ov13b10_power_on(dev);
-	if (ret)
-		goto pm_fail;
-
-	if (ov13b->streaming) {
-		ret = ov13b10_start_streaming(ov13b);
-		if (ret)
-			goto stop_streaming;
-	}
-
-	return 0;
-
-stop_streaming:
-	ov13b10_stop_streaming(ov13b);
-	ov13b10_power_off(dev);
-pm_fail:
-	ov13b->streaming = false;
-
-	return ret;
+	return ov13b10_power_on(dev);
 }
 
 static const struct v4l2_subdev_video_ops ov13b10_video_ops = {
-- 
Regards,

Laurent Pinchart

[PATCH 54/57] media: i2c: ov5670: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5670.c | 34 ----------------------------------
 1 file changed, 34 deletions(-)

diff --git a/drivers/media/i2c/ov5670.c b/drivers/media/i2c/ov5670.c
index afd84c8e5ee7..e80db3ecd4f8 100644
--- a/drivers/media/i2c/ov5670.c
+++ b/drivers/media/i2c/ov5670.c
@@ -1882,8 +1882,6 @@ struct ov5670 {
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
 	/* True if the device has been identified */
 	bool identified;
 };
@@ -2484,7 +2482,6 @@ static int ov5670_set_stream(struct v4l2_subdev *sd, int enable)
 		ret = ov5670_stop_streaming(ov5670);
 		pm_runtime_put(&client->dev);
 	}
-	ov5670->streaming = enable;
 	goto unlock_and_return;
 
 error:
@@ -2539,34 +2536,6 @@ static int __maybe_unused ov5670_runtime_suspend(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused ov5670_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov5670 *ov5670 = to_ov5670(sd);
-
-	if (ov5670->streaming)
-		ov5670_stop_streaming(ov5670);
-
-	return 0;
-}
-
-static int __maybe_unused ov5670_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov5670 *ov5670 = to_ov5670(sd);
-	int ret;
-
-	if (ov5670->streaming) {
-		ret = ov5670_start_streaming(ov5670);
-		if (ret) {
-			ov5670_stop_streaming(ov5670);
-			return ret;
-		}
-	}
-
-	return 0;
-}
-
 static const struct v4l2_subdev_core_ops ov5670_core_ops = {
 	.log_status = v4l2_ctrl_subdev_log_status,
 	.subscribe_event = v4l2_ctrl_subdev_subscribe_event,
@@ -2769,8 +2738,6 @@ static int ov5670_probe(struct i2c_client *client)
 		goto error_handler_free;
 	}
 
-	ov5670->streaming = false;
-
 	/* Set the device's state to active if it's in D0 state. */
 	if (full_power)
 		pm_runtime_set_active(&client->dev);
@@ -2825,7 +2792,6 @@ static void ov5670_remove(struct i2c_client *client)
 }
 
 static const struct dev_pm_ops ov5670_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov5670_suspend, ov5670_resume)
 	SET_RUNTIME_PM_OPS(ov5670_runtime_suspend, ov5670_runtime_resume, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 55/57] media: i2c: ov5675: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov5675.c | 41 --------------------------------------
 1 file changed, 41 deletions(-)

diff --git a/drivers/media/i2c/ov5675.c b/drivers/media/i2c/ov5675.c
index f608ee0c0cec..e63d9d402d34 100644
--- a/drivers/media/i2c/ov5675.c
+++ b/drivers/media/i2c/ov5675.c
@@ -513,9 +513,6 @@ struct ov5675 {
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	/* True if the device has been identified */
 	bool identified;
 };
@@ -968,7 +965,6 @@ static int ov5675_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov5675->streaming = enable;
 	mutex_unlock(&ov5675->mutex);
 
 	return ret;
@@ -1024,42 +1020,6 @@ static int ov5675_power_on(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused ov5675_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov5675 *ov5675 = to_ov5675(sd);
-
-	mutex_lock(&ov5675->mutex);
-	if (ov5675->streaming)
-		ov5675_stop_streaming(ov5675);
-
-	mutex_unlock(&ov5675->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused ov5675_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov5675 *ov5675 = to_ov5675(sd);
-	int ret;
-
-	mutex_lock(&ov5675->mutex);
-	if (ov5675->streaming) {
-		ret = ov5675_start_streaming(ov5675);
-		if (ret) {
-			ov5675->streaming = false;
-			ov5675_stop_streaming(ov5675);
-			mutex_unlock(&ov5675->mutex);
-			return ret;
-		}
-	}
-
-	mutex_unlock(&ov5675->mutex);
-
-	return 0;
-}
-
 static int ov5675_set_format(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *sd_state,
 			     struct v4l2_subdev_format *fmt)
@@ -1406,7 +1366,6 @@ static int ov5675_probe(struct i2c_client *client)
 }
 
 static const struct dev_pm_ops ov5675_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov5675_suspend, ov5675_resume)
 	SET_RUNTIME_PM_OPS(ov5675_power_off, ov5675_power_on, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 56/57] media: i2c: ov8856: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov8856.c | 44 --------------------------------------
 1 file changed, 44 deletions(-)

diff --git a/drivers/media/i2c/ov8856.c b/drivers/media/i2c/ov8856.c
index 5447a3361976..a0f673a24e52 100644
--- a/drivers/media/i2c/ov8856.c
+++ b/drivers/media/i2c/ov8856.c
@@ -1438,9 +1438,6 @@ struct ov8856 {
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
 
-	/* Streaming on/off */
-	bool streaming;
-
 	/* lanes index */
 	u8 nlanes;
 
@@ -2061,7 +2058,6 @@ static int ov8856_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov8856->streaming = enable;
 	mutex_unlock(&ov8856->mutex);
 
 	return ret;
@@ -2122,45 +2118,6 @@ static int ov8856_power_off(struct device *dev)
 	return 0;
 }
 
-static int __maybe_unused ov8856_suspend(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov8856 *ov8856 = to_ov8856(sd);
-
-	mutex_lock(&ov8856->mutex);
-	if (ov8856->streaming)
-		ov8856_stop_streaming(ov8856);
-
-	ov8856_power_off(dev);
-	mutex_unlock(&ov8856->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused ov8856_resume(struct device *dev)
-{
-	struct v4l2_subdev *sd = dev_get_drvdata(dev);
-	struct ov8856 *ov8856 = to_ov8856(sd);
-	int ret;
-
-	mutex_lock(&ov8856->mutex);
-
-	ov8856_power_on(dev);
-	if (ov8856->streaming) {
-		ret = ov8856_start_streaming(ov8856);
-		if (ret) {
-			ov8856->streaming = false;
-			ov8856_stop_streaming(ov8856);
-			mutex_unlock(&ov8856->mutex);
-			return ret;
-		}
-	}
-
-	mutex_unlock(&ov8856->mutex);
-
-	return 0;
-}
-
 static int ov8856_set_format(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *sd_state,
 			     struct v4l2_subdev_format *fmt)
@@ -2498,7 +2455,6 @@ static int ov8856_probe(struct i2c_client *client)
 }
 
 static const struct dev_pm_ops ov8856_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov8856_suspend, ov8856_resume)
 	SET_RUNTIME_PM_OPS(ov8856_power_off, ov8856_power_on, NULL)
 };
 
-- 
Regards,

Laurent Pinchart

[PATCH 57/57] media: i2c: ov9734: Drop system suspend and resume handlers

[Laurent Pinchart]

Stopping streaming on a camera pipeline at system suspend time, and
restarting it at system resume time, requires coordinated action between
the bridge driver and the camera sensor driver. This is handled by the
bridge driver calling the sensor's .s_stream() handler at system suspend
and resume time. There is thus no need for the sensor to independently
implement system sleep PM operations. Drop them.

The streaming field of the driver's private structure is now unused,
drop it as well.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/media/i2c/ov9734.c | 46 --------------------------------------
 1 file changed, 46 deletions(-)

diff --git a/drivers/media/i2c/ov9734.c b/drivers/media/i2c/ov9734.c
index a5f91499bce4..ee3315299605 100644
--- a/drivers/media/i2c/ov9734.c
+++ b/drivers/media/i2c/ov9734.c
@@ -337,9 +337,6 @@ struct ov9734 {
 
 	/* To serialize asynchronus callbacks */
 	struct mutex mutex;
-
-	/* Streaming on/off */
-	bool streaming;
 };
 
 static inline struct ov9734 *to_ov9734(struct v4l2_subdev *subdev)
@@ -679,49 +676,11 @@ static int ov9734_set_stream(struct v4l2_subdev *sd, int enable)
 		pm_runtime_put(&client->dev);
 	}
 
-	ov9734->streaming = enable;
 	mutex_unlock(&ov9734->mutex);
 
 	return ret;
 }
 
-static int __maybe_unused ov9734_suspend(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov9734 *ov9734 = to_ov9734(sd);
-
-	mutex_lock(&ov9734->mutex);
-	if (ov9734->streaming)
-		ov9734_stop_streaming(ov9734);
-
-	mutex_unlock(&ov9734->mutex);
-
-	return 0;
-}
-
-static int __maybe_unused ov9734_resume(struct device *dev)
-{
-	struct i2c_client *client = to_i2c_client(dev);
-	struct v4l2_subdev *sd = i2c_get_clientdata(client);
-	struct ov9734 *ov9734 = to_ov9734(sd);
-	int ret = 0;
-
-	mutex_lock(&ov9734->mutex);
-	if (!ov9734->streaming)
-		goto exit;
-
-	ret = ov9734_start_streaming(ov9734);
-	if (ret) {
-		ov9734->streaming = false;
-		ov9734_stop_streaming(ov9734);
-	}
-
-exit:
-	mutex_unlock(&ov9734->mutex);
-	return ret;
-}
-
 static int ov9734_set_format(struct v4l2_subdev *sd,
 			     struct v4l2_subdev_state *sd_state,
 			     struct v4l2_subdev_format *fmt)
@@ -1007,10 +966,6 @@ static int ov9734_probe(struct i2c_client *client)
 	return ret;
 }
 
-static const struct dev_pm_ops ov9734_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(ov9734_suspend, ov9734_resume)
-};
-
 static const struct acpi_device_id ov9734_acpi_ids[] = {
 	{ "OVTI9734", },
 	{}
@@ -1021,7 +976,6 @@ MODULE_DEVICE_TABLE(acpi, ov9734_acpi_ids);
 static struct i2c_driver ov9734_i2c_driver = {
 	.driver = {
 		.name = "ov9734",
-		.pm = &ov9734_pm_ops,
 		.acpi_match_table = ov9734_acpi_ids,
 	},
 	.probe = ov9734_probe,
-- 
Regards,

Laurent Pinchart

Re: [PATCH 35/57] media: i2c: ar0521: Drop system suspend and resume handlers

[Krzysztof Hałasa]

Hi Laurent,

Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes:

> Stopping streaming on a camera pipeline at system suspend time, and
> restarting it at system resume time, requires coordinated action between
> the bridge driver and the camera sensor driver. This is handled by the
> bridge driver calling the sensor's .s_stream() handler at system suspend
> and resume time. There is thus no need for the sensor to independently
> implement system sleep PM operations. Drop them.

If that's the case, good.

> The streaming field of the driver's private structure is now unused,
> drop it as well.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Acked-by: Krzysztof Hałasa <khalasa@piap.pl>

> ---
>  drivers/media/i2c/ar0521.c | 30 +-----------------------------

-- 
Krzysztof "Chris" Hałasa

Sieć Badawcza Łukasiewicz
Przemysłowy Instytut Automatyki i Pomiarów PIAP
Al. Jerozolimskie 202, 02-486 Warszawa

Re: [PATCH 06/57] media: i2c: imx219: Drop check for reentrant .s_stream()

[Dave Stevenson]

Hi Laurent

On Thu, 14 Sept 2023 at 19:16, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The subdev .s_stream() operation shall not be called to start streaming
> on an already started subdev, or stop streaming on a stopped subdev.
> Remove the check that guards against that condition.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

> ---
>  drivers/media/i2c/imx219.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index a1136fdfbed2..5715bbbc0820 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -979,9 +979,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable)
>
>         state = v4l2_subdev_lock_and_get_active_state(sd);
>
> -       if (imx219->streaming == enable)
> -               goto unlock;
> -
>         if (enable) {
>                 /*
>                  * Apply default & customized values
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH 42/57] media: i2c: imx219: Drop system suspend and resume handlers

[Dave Stevenson]

Hi Laurent

On Thu, 14 Sept 2023 at 19:17, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Stopping streaming on a camera pipeline at system suspend time, and
> restarting it at system resume time, requires coordinated action between
> the bridge driver and the camera sensor driver. This is handled by the
> bridge driver calling the sensor's .s_stream() handler at system suspend
> and resume time. There is thus no need for the sensor to independently
> implement system sleep PM operations. Drop them.
>
> The streaming field of the driver's private structure is now unused,
> drop it as well.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Presumably this is superseding effectively the same patch in your
other imx219 series [1]
That's fine by me, just an observation.

Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>

[1] https://patchwork.linuxtv.org/project/linux-media/patch/20230913135638.26277-16-laurent.pinchart@ideasonboard.com/

> ---
>  drivers/media/i2c/imx219.c | 41 --------------------------------------
>  1 file changed, 41 deletions(-)
>
> diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> index 5715bbbc0820..a431dace0f33 100644
> --- a/drivers/media/i2c/imx219.c
> +++ b/drivers/media/i2c/imx219.c
> @@ -479,9 +479,6 @@ struct imx219 {
>         /* Current mode */
>         const struct imx219_mode *mode;
>
> -       /* Streaming on/off */
> -       bool streaming;
> -
>         /* Two or Four lanes */
>         u8 lanes;
>  };
> @@ -991,8 +988,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable)
>                 imx219_stop_streaming(imx219);
>         }
>
> -       imx219->streaming = enable;
> -
>  unlock:
>         v4l2_subdev_unlock_state(state);
>         return ret;
> @@ -1044,41 +1039,6 @@ static int imx219_power_off(struct device *dev)
>         return 0;
>  }
>
> -static int __maybe_unused imx219_suspend(struct device *dev)
> -{
> -       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> -       struct imx219 *imx219 = to_imx219(sd);
> -
> -       if (imx219->streaming)
> -               imx219_stop_streaming(imx219);
> -
> -       return 0;
> -}
> -
> -static int __maybe_unused imx219_resume(struct device *dev)
> -{
> -       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> -       struct imx219 *imx219 = to_imx219(sd);
> -       struct v4l2_subdev_state *state;
> -       int ret;
> -
> -       if (imx219->streaming) {
> -               state = v4l2_subdev_lock_and_get_active_state(sd);
> -               ret = imx219_start_streaming(imx219, state);
> -               v4l2_subdev_unlock_state(state);
> -               if (ret)
> -                       goto error;
> -       }
> -
> -       return 0;
> -
> -error:
> -       imx219_stop_streaming(imx219);
> -       imx219->streaming = false;
> -
> -       return ret;
> -}
> -
>  static int imx219_get_regulators(struct imx219 *imx219)
>  {
>         struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
> @@ -1464,7 +1424,6 @@ static const struct of_device_id imx219_dt_ids[] = {
>  MODULE_DEVICE_TABLE(of, imx219_dt_ids);
>
>  static const struct dev_pm_ops imx219_pm_ops = {
> -       SET_SYSTEM_SLEEP_PM_OPS(imx219_suspend, imx219_resume)
>         SET_RUNTIME_PM_OPS(imx219_power_off, imx219_power_on, NULL)
>  };
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH 42/57] media: i2c: imx219: Drop system suspend and resume handlers

[Laurent Pinchart]

Hi Dave,

On Fri, Sep 15, 2023 at 11:53:53AM +0100, Dave Stevenson wrote:
> On Thu, 14 Sept 2023 at 19:17, Laurent Pinchart wrote:
> >
> > Stopping streaming on a camera pipeline at system suspend time, and
> > restarting it at system resume time, requires coordinated action between
> > the bridge driver and the camera sensor driver. This is handled by the
> > bridge driver calling the sensor's .s_stream() handler at system suspend
> > and resume time. There is thus no need for the sensor to independently
> > implement system sleep PM operations. Drop them.
> >
> > The streaming field of the driver's private structure is now unused,
> > drop it as well.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Presumably this is superseding effectively the same patch in your
> other imx219 series [1]
> That's fine by me, just an observation.

Good catch :-) The two series indeed fix the same issue. I have included
the imx219 changes in this series for completeness, I'll drop it from
one of the two series depending on which of them gets merged first.

Sorry for forgetting to mention this in the cover letter.

> Reviewed-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> [1] https://patchwork.linuxtv.org/project/linux-media/patch/20230913135638.26277-16-laurent.pinchart@ideasonboard.com/
> 
> > ---
> >  drivers/media/i2c/imx219.c | 41 --------------------------------------
> >  1 file changed, 41 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx219.c b/drivers/media/i2c/imx219.c
> > index 5715bbbc0820..a431dace0f33 100644
> > --- a/drivers/media/i2c/imx219.c
> > +++ b/drivers/media/i2c/imx219.c
> > @@ -479,9 +479,6 @@ struct imx219 {
> >         /* Current mode */
> >         const struct imx219_mode *mode;
> >
> > -       /* Streaming on/off */
> > -       bool streaming;
> > -
> >         /* Two or Four lanes */
> >         u8 lanes;
> >  };
> > @@ -991,8 +988,6 @@ static int imx219_set_stream(struct v4l2_subdev *sd, int enable)
> >                 imx219_stop_streaming(imx219);
> >         }
> >
> > -       imx219->streaming = enable;
> > -
> >  unlock:
> >         v4l2_subdev_unlock_state(state);
> >         return ret;
> > @@ -1044,41 +1039,6 @@ static int imx219_power_off(struct device *dev)
> >         return 0;
> >  }
> >
> > -static int __maybe_unused imx219_suspend(struct device *dev)
> > -{
> > -       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > -       struct imx219 *imx219 = to_imx219(sd);
> > -
> > -       if (imx219->streaming)
> > -               imx219_stop_streaming(imx219);
> > -
> > -       return 0;
> > -}
> > -
> > -static int __maybe_unused imx219_resume(struct device *dev)
> > -{
> > -       struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > -       struct imx219 *imx219 = to_imx219(sd);
> > -       struct v4l2_subdev_state *state;
> > -       int ret;
> > -
> > -       if (imx219->streaming) {
> > -               state = v4l2_subdev_lock_and_get_active_state(sd);
> > -               ret = imx219_start_streaming(imx219, state);
> > -               v4l2_subdev_unlock_state(state);
> > -               if (ret)
> > -                       goto error;
> > -       }
> > -
> > -       return 0;
> > -
> > -error:
> > -       imx219_stop_streaming(imx219);
> > -       imx219->streaming = false;
> > -
> > -       return ret;
> > -}
> > -
> >  static int imx219_get_regulators(struct imx219 *imx219)
> >  {
> >         struct i2c_client *client = v4l2_get_subdevdata(&imx219->sd);
> > @@ -1464,7 +1424,6 @@ static const struct of_device_id imx219_dt_ids[] = {
> >  MODULE_DEVICE_TABLE(of, imx219_dt_ids);
> >
> >  static const struct dev_pm_ops imx219_pm_ops = {
> > -       SET_SYSTEM_SLEEP_PM_OPS(imx219_suspend, imx219_resume)
> >         SET_RUNTIME_PM_OPS(imx219_power_off, imx219_power_on, NULL)
> >  };
> >

-- 
Regards,

Laurent Pinchart

Re: [PATCH 22/57] media: i2c: ov4689: Drop check for reentrant .s_stream()

[Mikhail Rudenko]

On 2023-09-14 at 21:16 +03, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> The subdev .s_stream() operation shall not be called to start streaming
> on an already started subdev, or stop streaming on a stopped subdev.
> Remove the check that guards against that condition.
>
> The streaming field of the driver's private structure is now unused,
> drop it as well.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/i2c/ov4689.c | 7 -------
>  1 file changed, 7 deletions(-)
>
> diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
> index fda217d2cb10..cd8d7131e747 100644
> --- a/drivers/media/i2c/ov4689.c
> +++ b/drivers/media/i2c/ov4689.c
> @@ -100,7 +100,6 @@ struct ov4689 {
>  	u32 clock_rate;
>
>  	struct mutex mutex; /* lock to protect streaming, ctrls and cur_mode */
> -	bool streaming;

We can now drop the mention of "streaming" from the comment one line
above. Aside from that,

Reviewed-by: Mikhail Rudenko <mike.rudenko@gmail.com>

>  	struct v4l2_ctrl_handler ctrl_handler;
>  	struct v4l2_ctrl *exposure;
>
> @@ -468,10 +467,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
>
>  	mutex_lock(&ov4689->mutex);
>
> -	on = !!on;
> -	if (on == ov4689->streaming)
> -		goto unlock_and_return;
> -
>  	if (on) {
>  		ret = pm_runtime_resume_and_get(&client->dev);
>  		if (ret < 0)
> @@ -504,8 +499,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
>  		pm_runtime_put(&client->dev);
>  	}
>
> -	ov4689->streaming = on;
> -
>  unlock_and_return:
>  	mutex_unlock(&ov4689->mutex);


--
Best regards,
Mikhail Rudenko

Re: [PATCH 22/57] media: i2c: ov4689: Drop check for reentrant .s_stream()

[Sakari Ailus]

Hi Mikhail, Laurent,

On Fri, Sep 15, 2023 at 10:26:28PM +0300, Mikhail Rudenko wrote:
> 
> On 2023-09-14 at 21:16 +03, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> 
> > The subdev .s_stream() operation shall not be called to start streaming
> > on an already started subdev, or stop streaming on a stopped subdev.
> > Remove the check that guards against that condition.
> >
> > The streaming field of the driver's private structure is now unused,
> > drop it as well.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  drivers/media/i2c/ov4689.c | 7 -------
> >  1 file changed, 7 deletions(-)
> >
> > diff --git a/drivers/media/i2c/ov4689.c b/drivers/media/i2c/ov4689.c
> > index fda217d2cb10..cd8d7131e747 100644
> > --- a/drivers/media/i2c/ov4689.c
> > +++ b/drivers/media/i2c/ov4689.c
> > @@ -100,7 +100,6 @@ struct ov4689 {
> >  	u32 clock_rate;
> >
> >  	struct mutex mutex; /* lock to protect streaming, ctrls and cur_mode */
> > -	bool streaming;
> 
> We can now drop the mention of "streaming" from the comment one line
> above. Aside from that,

I'll remove that while applying.

> 
> Reviewed-by: Mikhail Rudenko <mike.rudenko@gmail.com>

Thanks!

> 
> >  	struct v4l2_ctrl_handler ctrl_handler;
> >  	struct v4l2_ctrl *exposure;
> >
> > @@ -468,10 +467,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
> >
> >  	mutex_lock(&ov4689->mutex);
> >
> > -	on = !!on;
> > -	if (on == ov4689->streaming)
> > -		goto unlock_and_return;
> > -
> >  	if (on) {
> >  		ret = pm_runtime_resume_and_get(&client->dev);
> >  		if (ret < 0)
> > @@ -504,8 +499,6 @@ static int ov4689_s_stream(struct v4l2_subdev *sd, int on)
> >  		pm_runtime_put(&client->dev);
> >  	}
> >
> > -	ov4689->streaming = on;
> > -
> >  unlock_and_return:
> >  	mutex_unlock(&ov4689->mutex);
> 
> 
> --
> Best regards,
> Mikhail Rudenko

-- 
Sakari Ailus

Re: [PATCH 01/57] media: v4l2-subdev: Document .s_stream() operation requirements

[Ricardo Ribalda Delgado]

Hi Laurent

On Thu, Sep 14, 2023 at 8:16 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The subdev .s_stream() operation must not be called to start an already
> started subdev, or stop an already stopped one. This requirement has
> never been formally documented. Fix it.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/media/v4l2-subdev.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> index d9fca929c10b..282914ea9d45 100644
> --- a/include/media/v4l2-subdev.h
> +++ b/include/media/v4l2-subdev.h
> @@ -446,7 +446,9 @@ enum v4l2_subdev_pre_streamon_flags {
>   * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the
>   *     sub-device. Failure on stop will remove any resources acquired in
>   *     streaming start, while the error code is still returned by the driver.
> - *     Also see call_s_stream wrapper in v4l2-subdev.c.
> + *     The caller shall track the subdev state, and refrain from starting or

What about

s/refrain/ shall not/ ?

I refrain (and fail) from eating chocolate, but I shall not eat rat poison :P

> + *     stopping an already started or stopped subdev. AAlso see call_s_stream
> + *     wrapper in v4l2-subdev.c.

s/AAlso/Also/

>   *
>   * @g_pixelaspect: callback to return the pixelaspect ratio.
>   *
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH 05/57] media: i2c: imx214: Drop check for reentrant .s_stream()

[Ricardo Ribalda Delgado]

Hi Laurent

On Thu, Sep 14, 2023 at 8:17 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> The subdev .s_stream() operation shall not be called to start streaming
> on an already started subdev, or stop streaming on a stopped subdev.
> Remove the check that guards against that condition.
>

In general I agree with the patch, but I think it would be safer to
land it in two stages.

first:

       if WARN_ON(imx214->streaming == enable)
               return 0;

and in the next version replace it completely.

otherwise

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/i2c/imx214.c | 3 ---
>  1 file changed, 3 deletions(-)
>
> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> index 2f9c8582f940..e2805173f4b1 100644
> --- a/drivers/media/i2c/imx214.c
> +++ b/drivers/media/i2c/imx214.c
> @@ -775,9 +775,6 @@ static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
>         struct imx214 *imx214 = to_imx214(subdev);
>         int ret;
>
> -       if (imx214->streaming == enable)
> -               return 0;
> -
>         if (enable) {
>                 ret = pm_runtime_resume_and_get(imx214->dev);
>                 if (ret < 0)
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH 41/57] media: i2c: imx214: Drop system suspend and resume handlers

[Ricardo Ribalda Delgado]

Hi Laurent

On Thu, Sep 14, 2023 at 8:17 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Stopping streaming on a camera pipeline at system suspend time, and
> restarting it at system resume time, requires coordinated action between
> the bridge driver and the camera sensor driver. This is handled by the
> bridge driver calling the sensor's .s_stream() handler at system suspend
> and resume time. There is thus no need for the sensor to independently
> implement system sleep PM operations. Drop them.
>
> The streaming field of the driver's private structure is now unused,
> drop it as well.
>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> ---
>  drivers/media/i2c/imx214.c | 37 -------------------------------------
>  1 file changed, 37 deletions(-)
>
> diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> index e2805173f4b1..4f77ea02cc27 100644
> --- a/drivers/media/i2c/imx214.c
> +++ b/drivers/media/i2c/imx214.c
> @@ -58,8 +58,6 @@ struct imx214 {
>          * and start streaming.
>          */
>         struct mutex mutex;
> -
> -       bool streaming;
>  };
>
>  struct reg_8 {
> @@ -790,7 +788,6 @@ static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
>                 pm_runtime_put(imx214->dev);
>         }
>
> -       imx214->streaming = enable;
>         return 0;
>
>  err_rpm_put:
> @@ -906,39 +903,6 @@ static int imx214_parse_fwnode(struct device *dev)
>         return ret;
>  }
>
> -static int __maybe_unused imx214_suspend(struct device *dev)
> -{
> -       struct i2c_client *client = to_i2c_client(dev);
> -       struct v4l2_subdev *sd = i2c_get_clientdata(client);
> -       struct imx214 *imx214 = to_imx214(sd);
> -
> -       if (imx214->streaming)
> -               imx214_stop_streaming(imx214);
> -
> -       return 0;
> -}
> -
> -static int __maybe_unused imx214_resume(struct device *dev)
> -{
> -       struct i2c_client *client = to_i2c_client(dev);
> -       struct v4l2_subdev *sd = i2c_get_clientdata(client);
> -       struct imx214 *imx214 = to_imx214(sd);
> -       int ret;
> -
> -       if (imx214->streaming) {
> -               ret = imx214_start_streaming(imx214);
> -               if (ret)
> -                       goto error;
> -       }
> -
> -       return 0;
> -
> -error:
> -       imx214_stop_streaming(imx214);
> -       imx214->streaming = 0;
> -       return ret;
> -}
> -
>  static int imx214_probe(struct i2c_client *client)
>  {
>         struct device *dev = &client->dev;
> @@ -1099,7 +1063,6 @@ static const struct of_device_id imx214_of_match[] = {
>  MODULE_DEVICE_TABLE(of, imx214_of_match);
>
>  static const struct dev_pm_ops imx214_pm_ops = {
> -       SET_SYSTEM_SLEEP_PM_OPS(imx214_suspend, imx214_resume)
>         SET_RUNTIME_PM_OPS(imx214_power_off, imx214_power_on, NULL)
>  };
>
> --
> Regards,
>
> Laurent Pinchart
>

Re: [PATCH 05/57] media: i2c: imx214: Drop check for reentrant .s_stream()

[Laurent Pinchart]

Hi Ricardo,

On Mon, Sep 18, 2023 at 09:52:32AM +0200, Ricardo Ribalda Delgado wrote:
> On Thu, Sep 14, 2023 at 8:17 PM Laurent Pinchart wrote:
> >
> > The subdev .s_stream() operation shall not be called to start streaming
> > on an already started subdev, or stop streaming on a stopped subdev.
> > Remove the check that guards against that condition.
> >
> 
> In general I agree with the patch, but I think it would be safer to
> land it in two stages.
> 
> first:
> 
>        if WARN_ON(imx214->streaming == enable)
>                return 0;
> 
> and in the next version replace it completely.

I don't want to send another series of 57 patches to remove the
WARN_ON() in a few kernel versions :-S Fortunately, it seems we have a
better option, I can centralize the check in call_s_stream() !

And, on a side note, how many kernel versions would we need before
removing the WARN_ON() ?

Something I have on my radar is replacing all direct .s_stream() calls
with v4l2_subdev_enable_streams() and v4l2_subdev_disable_streams(), but
that's more work than I can tackle at the moment.

> otherwise
> 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> Reviewed-by: Ricardo Ribalda <ribalda@chromium.org>
> > ---
> >  drivers/media/i2c/imx214.c | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/drivers/media/i2c/imx214.c b/drivers/media/i2c/imx214.c
> > index 2f9c8582f940..e2805173f4b1 100644
> > --- a/drivers/media/i2c/imx214.c
> > +++ b/drivers/media/i2c/imx214.c
> > @@ -775,9 +775,6 @@ static int imx214_s_stream(struct v4l2_subdev *subdev, int enable)
> >         struct imx214 *imx214 = to_imx214(subdev);
> >         int ret;
> >
> > -       if (imx214->streaming == enable)
> > -               return 0;
> > -
> >         if (enable) {
> >                 ret = pm_runtime_resume_and_get(imx214->dev);
> >                 if (ret < 0)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/57] media: v4l2-subdev: Document .s_stream() operation requirements

[Laurent Pinchart]

Hi Ricardo,

On Mon, Sep 18, 2023 at 09:49:27AM +0200, Ricardo Ribalda Delgado wrote:
> On Thu, Sep 14, 2023 at 8:16 PM Laurent Pinchart wrote:
> >
> > The subdev .s_stream() operation must not be called to start an already
> > started subdev, or stop an already stopped one. This requirement has
> > never been formally documented. Fix it.
> >
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  include/media/v4l2-subdev.h | 4 +++-
> >  1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> > index d9fca929c10b..282914ea9d45 100644
> > --- a/include/media/v4l2-subdev.h
> > +++ b/include/media/v4l2-subdev.h
> > @@ -446,7 +446,9 @@ enum v4l2_subdev_pre_streamon_flags {
> >   * @s_stream: start (enabled == 1) or stop (enabled == 0) streaming on the
> >   *     sub-device. Failure on stop will remove any resources acquired in
> >   *     streaming start, while the error code is still returned by the driver.
> > - *     Also see call_s_stream wrapper in v4l2-subdev.c.
> > + *     The caller shall track the subdev state, and refrain from starting or
> 
> What about
> 
> s/refrain/ shall not/ ?
> 
> I refrain (and fail) from eating chocolate, but I shall not eat rat poison :P

I know someone who would tell you chocolate is at least as bad as rat
poison. That's a fairly extreme point of view, I agree with your
suggestion. I'll update this in v2.

> > + *     stopping an already started or stopped subdev. AAlso see call_s_stream
> > + *     wrapper in v4l2-subdev.c.
> 
> s/AAlso/Also/
> 
> >   *
> >   * @g_pixelaspect: callback to return the pixelaspect ratio.
> >   *

-- 
Regards,

Laurent Pinchart

Re: [PATCH 03/57] media: i2c: hi846: Drop check for reentrant .s_stream()

[Martin Kepplinger]

Am Donnerstag, dem 14.09.2023 um 21:16 +0300 schrieb Laurent Pinchart:
> The subdev .s_stream() operation shall not be called to start
> streaming
> on an already started subdev, or stop streaming on a stopped subdev.
> Remove the check that guards against that condition.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  drivers/media/i2c/hi846.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/media/i2c/hi846.c b/drivers/media/i2c/hi846.c
> index fa0038749a3b..746e1f75f9d0 100644
> --- a/drivers/media/i2c/hi846.c
> +++ b/drivers/media/i2c/hi846.c
> @@ -1607,9 +1607,6 @@ static int hi846_set_stream(struct v4l2_subdev
> *sd, int enable)
>         struct i2c_client *client = v4l2_get_subdevdata(sd);
>         int ret = 0;
>  
> -       if (hi846->streaming == enable)
> -               return 0;
> -
>         mutex_lock(&hi846->mutex);
>  
>         if (enable) {


Tested-by: Martin Kepplinger <martink@posteo.de>

thanks Laurent,

                                 martin