Skip to contentSkip to conversation

dt-bindings: adv7180: Introduce the 'reset-gpios' property

PATCH Superseded Fabio Estevam 2 participants 6 messages May 30, 2021 – May 31, 2021

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/3 dt-bindings: adv7180: Introduce the 'reset-gpios' property

Introduce the 'reset-gpios' property to describe the GPIO that connects to the ADV7180 reset pin.

---
 Documentation/devicetree/bindings/media/i2c/adv7180.yaml | 3 +++
 1 file changed, 3 insertions(+)
Documentation/devicetree/bindings/media/i2c/adv7180.yaml +3 -0
index bcfd93739b4f..1f1aa46f5724 100644
@@ -35,6 +35,9 @@ properties:
   powerdown-gpios:
     maxItems: 1
 
+  reset-gpios:
+    maxItems: 1
+
   port:
     $ref: /schemas/graph.yaml#/properties/port
 
Signature · 1 line
2.25.1
#1 lore raw

PATCH 2/3 media: i2c: adv7180: Allow the control of the reset pin

On a design where the ADV7180 pin is pulled down, it is not possible to make it functional unless the GPIO connected to this pin goes high.

Add support for the reset GPIO by introducing an optional property called 'reset-gpios'.

Note: the reset operation is still performed via software as recommended by the Analog Devices, but the reset GPIO still needs to go high to make the chip operational.

---
 drivers/media/i2c/adv7180.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
drivers/media/i2c/adv7180.c +13 -1
index 44bb6fe85644..2811f2c337fa 100644
@@ -205,6 +205,7 @@ struct adv7180_state {
 	struct mutex		mutex; /* mutual excl. when accessing chip */
 	int			irq;
 	struct gpio_desc	*pwdn_gpio;
+	struct gpio_desc	*reset_gpio;
 	v4l2_std_id		curr_norm;
 	bool			powered;
 	bool			streaming;
@@ -473,13 +474,15 @@ static int adv7180_g_frame_interval(struct v4l2_subdev *sd,
 
 static void adv7180_set_power_pin(struct adv7180_state *state, bool on)
 {
-	if (!state->pwdn_gpio)
+	if (!state->pwdn_gpio && !state->reset_gpio)
 		return;
 
 	if (on) {
+		gpiod_set_value_cansleep(state->reset_gpio, 0);
 		gpiod_set_value_cansleep(state->pwdn_gpio, 0);
 		usleep_range(5000, 10000);
 	} else {
+		gpiod_set_value_cansleep(state->reset_gpio, 1);
 		gpiod_set_value_cansleep(state->pwdn_gpio, 1);
 	}
 }
@@ -1338,6 +1341,15 @@ static int adv7180_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	state->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(state->reset_gpio)) {
+		ret = PTR_ERR(state->reset_gpio);
+		v4l_err(client, "request for reset pin failed: %d\n", ret);
+		return ret;
+	}
+
+
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
 		state->csi_client = i2c_new_dummy_device(client->adapter,
 				ADV7180_DEFAULT_CSI_I2C_ADDR);
Signature · 1 line
2.25.1
#2 lore raw

Re: [PATCH 2/3] media: i2c: adv7180: Allow the control of the reset pin

Quoted 29 lines · Fabio Estevam

On a design where the ADV7180 pin is pulled down, it is not possible to make it functional unless the GPIO connected to this pin goes high.

Add support for the reset GPIO by introducing an optional property called 'reset-gpios'.

Note: the reset operation is still performed via software as recommended by the Analog Devices, but the reset GPIO still needs to go high to make the chip operational.

---
 drivers/media/i2c/adv7180.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
drivers/media/i2c/adv7180.c +1 -0
index 44bb6fe85644..2811f2c337fa 100644
@@ -205,6 +205,7 @@ struct adv7180_state {
 	struct mutex		mutex; /* mutual excl. when accessing chip */
 	int			irq;
 	struct gpio_desc	*pwdn_gpio;
+	struct gpio_desc	*reset_gpio;
 	v4l2_std_id		curr_norm;
 	bool			powered;
 	bool			streaming;

drivers/media/i2c/adv7180.c static int adv7180_g_frame_interval(struct v4l2_subdev *sd, · line 482

Quoted 10 lines
drivers/media/i2c/adv7180.c +2 -1
@@ -473,13 +474,15 @@ static int adv7180_g_frame_interval(struct v4l2_subdev *sd,
 
 static void adv7180_set_power_pin(struct adv7180_state *state, bool on)
 {
-	if (!state->pwdn_gpio)
+	if (!state->pwdn_gpio && !state->reset_gpio)
 		return;
 
 	if (on) {
+		gpiod_set_value_cansleep(state->reset_gpio, 0);
 		gpiod_set_value_cansleep(state->pwdn_gpio, 0);

The datasheet specifies a delay of 5 ms between deasserting the PWRDWN and the RESET GPIO. Also this function is named adv7180_set_power_pin() which doesn't fit anymore if we also handle the RESET GPIO here.

As I was recently working with the ADV7280-M, I came up with a similar patch: https://git.kontron-electronics.de/linux/linux/-/commit/3619ed166140a0499ada7b14e5f1846a0ed7d18d.

What do you think?

Quoted 23 lines
drivers/media/i2c/adv7180.c +10 -0
 		usleep_range(5000, 10000);
 	} else {
+		gpiod_set_value_cansleep(state->reset_gpio, 1);
 		gpiod_set_value_cansleep(state->pwdn_gpio, 1);
 	}
 }
@@ -1338,6 +1341,15 @@ static int adv7180_probe(struct i2c_client *client,
 		return ret;
 	}
 
+	state->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
+						     GPIOD_OUT_HIGH);
+	if (IS_ERR(state->reset_gpio)) {
+		ret = PTR_ERR(state->reset_gpio);
+		v4l_err(client, "request for reset pin failed: %d\n", ret);
+		return ret;
+	}
+
+
 	if (state->chip_info->flags & ADV7180_FLAG_MIPI_CSI2) {
 		state->csi_client = i2c_new_dummy_device(client->adapter,
 				ADV7180_DEFAULT_CSI_I2C_ADDR);
#3 lore raw

Re: [PATCH 2/3] media: i2c: adv7180: Allow the control of the reset pin

Hi Frieder,

Quoted 5 lines · Frieder Schrempf

The datasheet specifies a delay of 5 ms between deasserting the PWRDWN and the RESET GPIO. Also this function is named adv7180_set_power_pin() which doesn't fit anymore if we also handle the RESET GPIO here.

As I was recently working with the ADV7280-M, I came up with a similar patch: https://git.kontron-electronics.de/linux/linux/-/commit/3619ed166140a0499ada7b14e5f1846a0ed7d18d.

What do you think?

Thanks for the review. I will send a v2 using your patch instead of mine.

Thanks,

Fabio Estevam

#4 lore raw

PATCH 3/3 media: i2c: adv7180: Print the chip ID on probe

In reply to Fabio Estevam

Improve the probe message by printing the chip ID version.

---
 drivers/media/i2c/adv7180.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
drivers/media/i2c/adv7180.c +10 -2
index 2811f2c337fa..e5ef99f0460c 100644
@@ -1404,11 +1404,19 @@ static int adv7180_probe(struct i2c_client *client,
 	if (ret)
 		goto err_free_irq;
 
-	v4l_info(client, "chip found @ 0x%02x (%s)\n",
-		 client->addr, client->adapter->name);
+	mutex_lock(&state->mutex);
+	ret = adv7180_read(state, ADV7180_REG_IDENT);
+	mutex_unlock(&state->mutex);
+	if (ret < 0)
+		goto err_v4l2_async_unregister;
+
+	v4l_info(client, "chip id 0x%x found @ 0x%02x (%s)\n",
+		 ret, client->addr, client->adapter->name);
 
 	return 0;
 
+err_v4l2_async_unregister:
+	v4l2_async_unregister_subdev(sd);
 err_free_irq:
 	if (state->irq > 0)
 		free_irq(client->irq, state);
Signature · 1 line
2.25.1
Reviewed-by on v1 3/3
#5 lore raw

Re: [PATCH 3/3] media: i2c: adv7180: Print the chip ID on probe

Quoted 3 lines · Fabio Estevam

Improve the probe message by printing the chip ID version.

Quoted 31 lines
---
 drivers/media/i2c/adv7180.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)
drivers/media/i2c/adv7180.c +10 -2
index 2811f2c337fa..e5ef99f0460c 100644
@@ -1404,11 +1404,19 @@ static int adv7180_probe(struct i2c_client *client,
 	if (ret)
 		goto err_free_irq;
 
-	v4l_info(client, "chip found @ 0x%02x (%s)\n",
-		 client->addr, client->adapter->name);
+	mutex_lock(&state->mutex);
+	ret = adv7180_read(state, ADV7180_REG_IDENT);
+	mutex_unlock(&state->mutex);
+	if (ret < 0)
+		goto err_v4l2_async_unregister;
+
+	v4l_info(client, "chip id 0x%x found @ 0x%02x (%s)\n",
+		 ret, client->addr, client->adapter->name);
 
 	return 0;
 
+err_v4l2_async_unregister:
+	v4l2_async_unregister_subdev(sd);
 err_free_irq:
 	if (state->irq > 0)
 		free_irq(client->irq, state);