[PATCH 0/4] media: ov2680: follow up from initial version

[PATCH 0/4] media: ov2680: follow up from initial version

[Rui Miguel Silva]

Sorry I have Out-of-Office some part of last week, I had v6 of the original
series ready but since I have received the notification from patchwork that the
v5 was accepted, I am sending this as a follow up tha address Fabio comments.

- this adds the power supplies to this sensor
- fix gpio polarity and naming.

Cheers,
   Rui


Rui Miguel Silva (4):
  media: ov2680: dt: add voltage supplies as required
  media: ov2680: dt: rename gpio to reset and fix polarity
  media: ov2680: rename powerdown gpio and fix polarity
  media: ov2680: add regulators to supply control

 .../devicetree/bindings/media/i2c/ov2680.txt  | 18 ++++--
 drivers/media/i2c/ov2680.c                    | 55 +++++++++++++++----
 2 files changed, 57 insertions(+), 16 deletions(-)

-- 
2.17.0

[PATCH 1/4] media: ov2680: dt: add voltage supplies as required

[Rui Miguel Silva]

The ov2680 needs to control 3 regulators for the voltage supplies and that was
missing by the first description, added their description and example here.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 Documentation/devicetree/bindings/media/i2c/ov2680.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
index 0e29f1a113c0..e5d8f130309d 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -4,6 +4,9 @@ Required Properties:
 - compatible: should be "ovti,ov2680".
 - clocks: reference to the xvclk input clock.
 - clock-names: should be "xvclk".
+- DOVDD-supply: Digital I/O voltage supply.
+- DVDD-supply: Digital core voltage supply.
+- AVDD-supply: Analog voltage supply.
 
 Optional Properties:
 - powerdown-gpios: reference to the GPIO connected to the powerdown pin,
@@ -18,7 +21,7 @@ Endpoint node required properties for CSI-2 connection are:
 - remote-endpoint: a phandle to the bus receiver's endpoint node.
 - clock-lanes: should be set to <0> (clock lane on hardware lane 0).
 - data-lanes: should be set to <1> (one CSI-2 lane supported).
- 
+
 Example:
 
 &i2c2 {
@@ -28,6 +31,9 @@ Example:
 		clocks = <&osc>;
 		clock-names = "xvclk";
 		powerdown-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+		DOVDD-supply = <&sw2_reg>;
+		DVDD-supply = <&sw2_reg>;
+		AVDD-supply = <&reg_peri_3p15v>;
 
 		port {
 			ov2680_mipi_ep: endpoint {
-- 
2.17.0

[PATCH 2/4] media: ov2680: dt: rename gpio to reset and fix polarity

[Rui Miguel Silva]

Since the GPIO 3 controls both reset and powerdown, we rename it to reset. Fix
the polarity of this, as it is a low active signal and not an high active.

As at it, rename the mipi endpoint in the example to be coherent with the a real
case that will be introduce for imx7 media driver.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 Documentation/devicetree/bindings/media/i2c/ov2680.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
index e5d8f130309d..11e925ed9dad 100644
--- a/Documentation/devicetree/bindings/media/i2c/ov2680.txt
+++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt
@@ -9,8 +9,8 @@ Required Properties:
 - AVDD-supply: Analog voltage supply.
 
 Optional Properties:
-- powerdown-gpios: reference to the GPIO connected to the powerdown pin,
-		     if any. This is an active high signal to the OV2680.
+- reset-gpios: reference to the GPIO connected to the powerdown/reset pin,
+               if any. This is an active low signal to the OV2680.
 
 The device node must contain one 'port' child node for its digital output
 video port, and this port must have a single endpoint in accordance with
@@ -30,14 +30,14 @@ Example:
 		reg = <0x36>;
 		clocks = <&osc>;
 		clock-names = "xvclk";
-		powerdown-gpios = <&gpio1 3 GPIO_ACTIVE_HIGH>;
+		reset-gpios = <&gpio1 3 GPIO_ACTIVE_LOW>;
 		DOVDD-supply = <&sw2_reg>;
 		DVDD-supply = <&sw2_reg>;
 		AVDD-supply = <&reg_peri_3p15v>;
 
 		port {
-			ov2680_mipi_ep: endpoint {
-				remote-endpoint = <&mipi_sensor_ep>;
+			ov2680_to_mipi: endpoint {
+				remote-endpoint = <&mipi_from_sensor>;
 				clock-lanes = <0>;
 				data-lanes = <1>;
 			};
-- 
2.17.0

[PATCH 3/4] media: ov2680: rename powerdown gpio and fix polarity

[Rui Miguel Silva]

Rename the power control gpio to reset, since it is the same, and fix the
polarity code since this gpio is active at low and not at high as controlled
before.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 drivers/media/i2c/ov2680.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
index cc3d06096d51..8962b397211a 100644
--- a/drivers/media/i2c/ov2680.c
+++ b/drivers/media/i2c/ov2680.c
@@ -98,7 +98,7 @@ struct ov2680_dev {
 	struct clk			*xvclk;
 	u32				xvclk_freq;
 
-	struct gpio_desc		*pwdn_gpio;
+	struct gpio_desc		*reset_gpio;
 	struct mutex			lock; /* protect members */
 
 	bool				mode_pending_changes;
@@ -295,19 +295,19 @@ static int ov2680_load_regs(struct ov2680_dev *sensor,
 
 static void ov2680_power_up(struct ov2680_dev *sensor)
 {
-	if (!sensor->pwdn_gpio)
+	if (!sensor->reset_gpio)
 		return;
 
-	gpiod_set_value(sensor->pwdn_gpio, 1);
+	gpiod_set_value(sensor->reset_gpio, 0);
 	usleep_range(5000, 10000);
 }
 
 static void ov2680_power_down(struct ov2680_dev *sensor)
 {
-	if (!sensor->pwdn_gpio)
+	if (!sensor->reset_gpio)
 		return;
 
-	gpiod_set_value(sensor->pwdn_gpio, 0);
+	gpiod_set_value(sensor->reset_gpio, 1);
 	usleep_range(5000, 10000);
 }
 
@@ -535,7 +535,7 @@ static int ov2680_power_on(struct ov2680_dev *sensor)
 	if (sensor->is_enabled)
 		return 0;
 
-	if (!sensor->pwdn_gpio) {
+	if (!sensor->reset_gpio) {
 		ret = ov2680_write_reg(sensor, OV2680_REG_SOFT_RESET, 0x01);
 		if (ret != 0) {
 			dev_err(dev, "sensor soft reset failed\n");
@@ -990,11 +990,11 @@ static int ov2860_parse_dt(struct ov2680_dev *sensor)
 	struct device *dev = ov2680_to_dev(sensor);
 	int ret;
 
-	sensor->pwdn_gpio = devm_gpiod_get_optional(dev, "powerdown",
-						    GPIOD_OUT_HIGH);
-	ret = PTR_ERR_OR_ZERO(sensor->pwdn_gpio);
+	sensor->reset_gpio = devm_gpiod_get_optional(dev, "reset",
+						     GPIOD_OUT_HIGH);
+	ret = PTR_ERR_OR_ZERO(sensor->reset_gpio);
 	if (ret < 0) {
-		dev_dbg(dev, "error while getting powerdown gpio: %d\n", ret);
+		dev_dbg(dev, "error while getting reset gpio: %d\n", ret);
 		return ret;
 	}
 
-- 
2.17.0

[PATCH 4/4] media: ov2680: add regulators to supply control

[Rui Miguel Silva]

Add the code to control the regulators for the analogue and digital power
supplies.

Signed-off-by: Rui Miguel Silva <rui.silva@linaro.org>
---
 drivers/media/i2c/ov2680.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/media/i2c/ov2680.c b/drivers/media/i2c/ov2680.c
index 8962b397211a..07bb475c970c 100644
--- a/drivers/media/i2c/ov2680.c
+++ b/drivers/media/i2c/ov2680.c
@@ -19,6 +19,7 @@
 #include <linux/module.h>
 #include <linux/of_device.h>
 #include <linux/gpio/consumer.h>
+#include <linux/regulator/consumer.h>
 
 #include <media/v4l2-common.h>
 #include <media/v4l2-ctrls.h>
@@ -65,6 +66,14 @@ struct reg_value {
 	u8 val;
 };
 
+static const char * const ov2680_supply_name[] = {
+	"DOVDD",
+	"DVDD",
+	"AVDD",
+};
+
+#define OV2680_NUM_SUPPLIES ARRAY_SIZE(ov2680_supply_name)
+
 struct ov2680_mode_info {
 	const char *name;
 	enum ov2680_mode_id id;
@@ -97,6 +106,7 @@ struct ov2680_dev {
 	struct media_pad		pad;
 	struct clk			*xvclk;
 	u32				xvclk_freq;
+	struct regulator_bulk_data	supplies[OV2680_NUM_SUPPLIES];
 
 	struct gpio_desc		*reset_gpio;
 	struct mutex			lock; /* protect members */
@@ -522,6 +532,7 @@ static int ov2680_power_off(struct ov2680_dev *sensor)
 
 	clk_disable_unprepare(sensor->xvclk);
 	ov2680_power_down(sensor);
+	regulator_bulk_disable(OV2680_NUM_SUPPLIES, sensor->supplies);
 	sensor->is_enabled = false;
 
 	return 0;
@@ -535,6 +546,12 @@ static int ov2680_power_on(struct ov2680_dev *sensor)
 	if (sensor->is_enabled)
 		return 0;
 
+	ret = regulator_bulk_enable(OV2680_NUM_SUPPLIES, sensor->supplies);
+	if (ret < 0) {
+		dev_err(dev, "failed to enable regulators: %d\n", ret);
+		return ret;
+	}
+
 	if (!sensor->reset_gpio) {
 		ret = ov2680_write_reg(sensor, OV2680_REG_SOFT_RESET, 0x01);
 		if (ret != 0) {
@@ -962,6 +979,18 @@ static int ov2680_v4l2_init(struct ov2680_dev *sensor)
 	return ret;
 }
 
+static int ov2680_get_regulators(struct ov2680_dev *sensor)
+{
+	int i;
+
+	for (i = 0; i < OV2680_NUM_SUPPLIES; i++)
+		sensor->supplies[i].supply = ov2680_supply_name[i];
+
+	return devm_regulator_bulk_get(&sensor->i2c_client->dev,
+				       OV2680_NUM_SUPPLIES,
+				       sensor->supplies);
+}
+
 static int ov2680_check_id(struct ov2680_dev *sensor)
 {
 	struct device *dev = ov2680_to_dev(sensor);
@@ -1034,6 +1063,12 @@ static int ov2680_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	ret = ov2680_get_regulators(sensor);
+	if (ret < 0) {
+		dev_err(dev, "failed to get regulators\n");
+		return ret;
+	}
+
 	mutex_init(&sensor->lock);
 
 	ret = ov2680_v4l2_init(sensor);
-- 
2.17.0

Re: [PATCH 0/4] media: ov2680: follow up from initial version

[Fabio Estevam]

Hi Rui,

On Mon, May 7, 2018 at 12:56 PM, Rui Miguel Silva <rui.silva@linaro.org> wrote:
> Sorry I have Out-of-Office some part of last week, I had v6 of the original
> series ready but since I have received the notification from patchwork that the
> v5 was accepted, I am sending this as a follow up tha address Fabio comments.
>
> - this adds the power supplies to this sensor
> - fix gpio polarity and naming.
>
> Cheers,
>    Rui
>
>
> Rui Miguel Silva (4):
>   media: ov2680: dt: add voltage supplies as required
>   media: ov2680: dt: rename gpio to reset and fix polarity
>   media: ov2680: rename powerdown gpio and fix polarity
>   media: ov2680: add regulators to supply control

As the initial ov2680 series has not been applied, I think it would be
better if you send a new version with all these fixes.

Re: [PATCH 0/4] media: ov2680: follow up from initial version

[Rui Miguel Silva]

Ola Fabio,
On Tue 08 May 2018 at 13:29, Fabio Estevam wrote:
> Hi Rui,
>
> On Mon, May 7, 2018 at 12:56 PM, Rui Miguel Silva 
> <rui.silva@linaro.org> wrote:
>> Sorry I have Out-of-Office some part of last week, I had v6 of 
>> the original
>> series ready but since I have received the notification from 
>> patchwork that the
>> v5 was accepted, I am sending this as a follow up tha address 
>> Fabio comments.
>>
>> - this adds the power supplies to this sensor
>> - fix gpio polarity and naming.
>>
>> Cheers,
>>    Rui
>>
>>
>> Rui Miguel Silva (4):
>>   media: ov2680: dt: add voltage supplies as required
>>   media: ov2680: dt: rename gpio to reset and fix polarity
>>   media: ov2680: rename powerdown gpio and fix polarity
>>   media: ov2680: add regulators to supply control
>
> As the initial ov2680 series has not been applied, I think it 
> would be
> better if you send a new version with all these fixes.

What I've got was this from patchwork:
Hello,

The following patches (submitted by you) have been updated in 
patchwork:

 * linux-media: [v5,1/2] media: ov2680: dt: Add bindings for 
 OV2680
     - http://patchwork.linuxtv.org/patch/48819/
     - for: Linux Media kernel patches
    was: New
    now: Accepted

 * linux-media: [v4,1/2] media: ov2680: dt: Add bindings for 
 OV2680
     - http://patchwork.linuxtv.org/patch/48357/
     - for: Linux Media kernel patches
    was: New
    now: Accepted

This email is a notification only - you do not need to respond.

-

Patches submitted to linux-media@vger.kernel.org have the 
following
possible states:

<snip>

Accepted: when some driver maintainer says that the patch will be 
applied
	  via his tree, or when everything is ok and it got 
	  applied
	  either at the main tree or via some other tree (fixes 
	  tree;
	  some other maintainer's tree - when it belongs to other 
	  subsystems,
	  etc);

So, my understand is that the patches will be applied or are 
already
applied to someone tree (strange patchwork does not send who or 
which
tree), but since I do not want to break someone workflow I will 
wait for
some maintainer word on this... If it is better to send the 
original series
or the follow up patches.

Thanks for your feedback.
---
Cheers,
	Rui

Re: [PATCH 0/4] media: ov2680: follow up from initial version

[Rui Miguel Silva]

Ola Fabio,
On Tue 08 May 2018 at 13:29, Fabio Estevam wrote:
> Hi Rui,
>
> On Mon, May 7, 2018 at 12:56 PM, Rui Miguel Silva 
> <rui.silva@linaro.org> wrote:
>> Sorry I have Out-of-Office some part of last week, I had v6 of 
>> the original
>> series ready but since I have received the notification from 
>> patchwork that the
>> v5 was accepted, I am sending this as a follow up tha address 
>> Fabio comments.
>>
>> - this adds the power supplies to this sensor
>> - fix gpio polarity and naming.
>>
>> Cheers,
>>    Rui
>>
>>
>> Rui Miguel Silva (4):
>>   media: ov2680: dt: add voltage supplies as required
>>   media: ov2680: dt: rename gpio to reset and fix polarity
>>   media: ov2680: rename powerdown gpio and fix polarity
>>   media: ov2680: add regulators to supply control
>
> As the initial ov2680 series has not been applied, I think it 
> would be
> better if you send a new version with all these fixes.

What I've got was this from patchwork:
Hello,

The following patches (submitted by you) have been updated in 
patchwork:

 * linux-media: [v5,1/2] media: ov2680: dt: Add bindings for 
 OV2680
     - http://patchwork.linuxtv.org/patch/48819/
     - for: Linux Media kernel patches
    was: New
    now: Accepted

 * linux-media: [v4,1/2] media: ov2680: dt: Add bindings for 
 OV2680
     - http://patchwork.linuxtv.org/patch/48357/
     - for: Linux Media kernel patches
    was: New
    now: Accepted

This email is a notification only - you do not need to respond.

-

Patches submitted to linux-media@vger.kernel.org have the 
following
possible states:

<snip>

Accepted: when some driver maintainer says that the patch will be 
applied
	  via his tree, or when everything is ok and it got 
	  applied
	  either at the main tree or via some other tree (fixes 
	  tree;
	  some other maintainer's tree - when it belongs to other 
	  subsystems,
	  etc);

So, my understand is that the patches will be applied or are 
already
applied to someone tree (strange patchwork does not send who or 
which
tree), but since I do not want to break someone workflow I will 
wait for
some maintainer word on this... If it is better to send the 
original series
or the follow up patches.

Thanks for your feedback.
---
Cheers,
	Rui

Re: [PATCH 0/4] media: ov2680: follow up from initial version

[Fabio Estevam]

Hi Rui,

On Tue, May 8, 2018 at 10:52 AM, Rui Miguel Silva <rui.silva@linaro.org> wrote:

> So, my understand is that the patches will be applied or are already
> applied to someone tree (strange patchwork does not send who or which
> tree), but since I do not want to break someone workflow I will wait for
> some maintainer word on this... If it is better to send the original series
> or the follow up patches.

Ah, didn't know it has been applied. It seems it will soon appear in
linux-next then.

In this case, the follow up series makes sense.

Thanks

Re: [PATCH 0/4] media: ov2680: follow up from initial version

[Sakari Ailus]

Hi,

On Mon, May 07, 2018 at 04:56:51PM +0100, Rui Miguel Silva wrote:
> Sorry I have Out-of-Office some part of last week, I had v6 of the original
> series ready but since I have received the notification from patchwork that the
> v5 was accepted, I am sending this as a follow up tha address Fabio comments.

I think I somehow managed to get the patchwork state wrong; the original
ov2680 set hasn't been applied yet. The latest patches I have applied are
here:

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.18-3>

Could you merge this to the original set and resend, please?

-- 
Kind regards,

Sakari Ailus
sakari.ailus@linux.intel.com

Re: [PATCH 0/4] media: ov2680: follow up from initial version

[Rui Miguel Silva]

Hi Sakari,
On Tue 08 May 2018 at 20:03, Sakari Ailus wrote:
> Hi,
>
> On Mon, May 07, 2018 at 04:56:51PM +0100, Rui Miguel Silva 
> wrote:
>> Sorry I have Out-of-Office some part of last week, I had v6 of 
>> the original
>> series ready but since I have received the notification from 
>> patchwork that the
>> v5 was accepted, I am sending this as a follow up tha address 
>> Fabio comments.
>
> I think I somehow managed to get the patchwork state wrong; the 
> original
> ov2680 set hasn't been applied yet. The latest patches I have 
> applied are
> here:
>
> <URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=for-4.18-3>
>
> Could you merge this to the original set and resend, please?

Ok, no problem, I will send it as v6 of the original series.

---
Cheers,
	Rui