Hi Laurent,
Few comments below.
On 07/05/2012 11:38 PM, Laurent Pinchart wrote:
Powering off a device is a "best effort" task: failure to execute one of
the steps should not prevent the next steps to be executed. For
instance, an I2C communication error when putting the chip in stand-by
mode should not prevent the more agressive next step of turning the
chip's power supply off.
Signed-off-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
---
drivers/media/video/soc_camera.c | 9 +++------
1 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/media/video/soc_camera.c b/drivers/media/video/soc_camera.c
index 55b981f..bbd518f 100644
--- a/drivers/media/video/soc_camera.c
+++ b/drivers/media/video/soc_camera.c
@@ -89,18 +89,15 @@ static int soc_camera_power_off(struct soc_camera_device *icd,
struct soc_camera_link *icl)
{
struct v4l2_subdev *sd = soc_camera_to_subdev(icd);
- int ret = v4l2_subdev_call(sd, core, s_power, 0);
+ int ret;
- if (ret < 0 && ret != -ENOIOCTLCMD && ret != -ENODEV)
- return ret;
+ v4l2_subdev_call(sd, core, s_power, 0);
Fair enough. I agree we should not prevent power off because of failure
in this step. But IMO we should not silently bypass it too. How about
an error message?
if (icl->power) {
ret = icl->power(icd->control, 0);
- if (ret < 0) {
+ if (ret < 0)
dev_err(icd->pdev,
"Platform failed to power-off the camera.\n");
- return ret;
- }
}
ret = regulator_bulk_disable(icl->num_regulators,
One more comment. Should this function's return value being based fully
on last action? If any earlier error happened but this last step is
fine, IMO we should not return 0.
Kind regards,
David Cohen
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
