[PATCH 1/2] media: platform: pxa_camera: add missing sensor power on

[PATCH 1/2] media: platform: pxa_camera: add missing sensor power on

[Robert Jarzmik]

During sensors binding, there is a window where the sensor is switched
off, while there is a call it to set a new format, which can end up in
an access to the sensor, especially an I2C based sensor.

Remove this window by activating the sensor.

Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/platform/pxa_camera.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
index 2978cd6efa63..794c41d24d9f 100644
--- a/drivers/media/platform/pxa_camera.c
+++ b/drivers/media/platform/pxa_camera.c
@@ -2128,17 +2128,22 @@ static int pxa_camera_sensor_bound(struct v4l2_async_notifier *notifier,
 				    pix->bytesperline, pix->height);
 	pix->pixelformat = pcdev->current_fmt->host_fmt->fourcc;
 	v4l2_fill_mbus_format(mf, pix, pcdev->current_fmt->code);
-	err = sensor_call(pcdev, pad, set_fmt, NULL, &format);
+
+	err = sensor_call(pcdev, core, s_power, 1);
 	if (err)
 		goto out;
 
+	err = sensor_call(pcdev, pad, set_fmt, NULL, &format);
+	if (err)
+		goto out_sensor_poweroff;
+
 	v4l2_fill_pix_format(pix, mf);
 	pr_info("%s(): colorspace=0x%x pixfmt=0x%x\n",
 		__func__, pix->colorspace, pix->pixelformat);
 
 	err = pxa_camera_init_videobuf2(pcdev);
 	if (err)
-		goto out;
+		goto out_sensor_poweroff;
 
 	err = video_register_device(&pcdev->vdev, VFL_TYPE_GRABBER, -1);
 	if (err) {
@@ -2149,6 +2154,9 @@ static int pxa_camera_sensor_bound(struct v4l2_async_notifier *notifier,
 			 "PXA Camera driver attached to camera %s\n",
 			 subdev->name);
 	}
+
+out_sensor_poweroff:
+	err = sensor_call(pcdev, core, s_power, 0);
 out:
 	mutex_unlock(&pcdev->mlock);
 	return err;
-- 
2.1.4

[PATCH 2/2] media: mt9m111: add regulator support

[Robert Jarzmik]

In the soc_camera removal, the board specific power callback was
dropped. This at least will remove the power optimization from ezx and
em-x270 pxa based boards.

As to recreate the same level of functionality, make the mt9m111 have a
regulator providing it its power, so that board designers can plug in a
gpio based or ldo regulator, mimicking their former soc_camera power
hook.

Fixes: 5c10113cc668 ("media: mt9m111: make a standalone v4l2 subdevice")
Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
---
 drivers/media/i2c/mt9m111.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/media/i2c/mt9m111.c b/drivers/media/i2c/mt9m111.c
index 72e71b762827..bb8ed4927e44 100644
--- a/drivers/media/i2c/mt9m111.c
+++ b/drivers/media/i2c/mt9m111.c
@@ -13,6 +13,7 @@
 #include <linux/log2.h>
 #include <linux/gpio.h>
 #include <linux/delay.h>
+#include <linux/regulator/consumer.h>
 #include <linux/v4l2-mediabus.h>
 #include <linux/module.h>
 
@@ -215,6 +216,7 @@ struct mt9m111 {
 	int power_count;
 	const struct mt9m111_datafmt *fmt;
 	int lastpage;	/* PageMap cache value */
+	struct regulator *regulator;
 };
 
 /* Find a data format by a pixel code */
@@ -788,6 +790,9 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111)
 	if (ret < 0)
 		return ret;
 
+	if (mt9m111->regulator)
+		regulator_enable(mt9m111->regulator);
+
 	ret = mt9m111_resume(mt9m111);
 	if (ret < 0) {
 		dev_err(&client->dev, "Failed to resume the sensor: %d\n", ret);
@@ -800,6 +805,8 @@ static int mt9m111_power_on(struct mt9m111 *mt9m111)
 static void mt9m111_power_off(struct mt9m111 *mt9m111)
 {
 	mt9m111_suspend(mt9m111);
+	if (mt9m111->regulator)
+		regulator_disable(mt9m111->regulator);
 	v4l2_clk_disable(mt9m111->clk);
 }
 
@@ -943,6 +950,13 @@ static int mt9m111_probe(struct i2c_client *client,
 	if (!mt9m111)
 		return -ENOMEM;
 
+	mt9m111->regulator = devm_regulator_get(&client->dev, "vdd");
+	if (IS_ERR(mt9m111->regulator)) {
+		dev_err(&client->dev, "regulator not found: %d\n",
+			PTR_ERR(mt9m111->regulator));
+		return PTR_ERR(mt9m111->regulator);
+	}
+
 	mt9m111->clk = v4l2_clk_get(&client->dev, "mclk");
 	if (IS_ERR(mt9m111->clk))
 		return -EPROBE_DEFER;
-- 
2.1.4

Re: [PATCH 1/2] media: platform: pxa_camera: add missing sensor power on

[Robert Jarzmik]

Robert Jarzmik <robert.jarzmik@free.fr> writes:

> During sensors binding, there is a window where the sensor is switched
> off, while there is a call it to set a new format, which can end up in
> an access to the sensor, especially an I2C based sensor.
>
> Remove this window by activating the sensor.
Hi guys,

I can't remember if I have review issues I have to address for this serie or
not. My mailer seems to tell no, but let's check again.

This serie is adding back the "power on" of the sensors through the generic
regulator API, and is my prequisite for pxa submitted changes, which were
formerly a "hook" in soc_camera_link structure, see:
         https://www.spinics.net/lists/kernel/msg2350167.html

Cheers.

--
Robert

[1] Remaining of the patch for reference

> Signed-off-by: Robert Jarzmik <robert.jarzmik@free.fr>
> ---
>  drivers/media/platform/pxa_camera.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/pxa_camera.c b/drivers/media/platform/pxa_camera.c
> index 2978cd6efa63..794c41d24d9f 100644
> --- a/drivers/media/platform/pxa_camera.c
> +++ b/drivers/media/platform/pxa_camera.c
> @@ -2128,17 +2128,22 @@ static int pxa_camera_sensor_bound(struct v4l2_async_notifier *notifier,
>  				    pix->bytesperline, pix->height);
>  	pix->pixelformat = pcdev->current_fmt->host_fmt->fourcc;
>  	v4l2_fill_mbus_format(mf, pix, pcdev->current_fmt->code);
> -	err = sensor_call(pcdev, pad, set_fmt, NULL, &format);
> +
> +	err = sensor_call(pcdev, core, s_power, 1);
>  	if (err)
>  		goto out;
>  
> +	err = sensor_call(pcdev, pad, set_fmt, NULL, &format);
> +	if (err)
> +		goto out_sensor_poweroff;
> +
>  	v4l2_fill_pix_format(pix, mf);
>  	pr_info("%s(): colorspace=0x%x pixfmt=0x%x\n",
>  		__func__, pix->colorspace, pix->pixelformat);
>  
>  	err = pxa_camera_init_videobuf2(pcdev);
>  	if (err)
> -		goto out;
> +		goto out_sensor_poweroff;
>  
>  	err = video_register_device(&pcdev->vdev, VFL_TYPE_GRABBER, -1);
>  	if (err) {
> @@ -2149,6 +2154,9 @@ static int pxa_camera_sensor_bound(struct v4l2_async_notifier *notifier,
>  			 "PXA Camera driver attached to camera %s\n",
>  			 subdev->name);
>  	}
> +
> +out_sensor_poweroff:
> +	err = sensor_call(pcdev, core, s_power, 0);
>  out:
>  	mutex_unlock(&pcdev->mlock);
>  	return err;

-- 
Robert