diff mbox series

[1/2] media: i2c: ov7251: Set enable GPIO low in probe

Message ID 20250120101123.148482-1-sakari.ailus@linux.intel.com (mailing list archive)
State New
Headers show
Series [1/2] media: i2c: ov7251: Set enable GPIO low in probe | expand

Commit Message

Sakari Ailus Jan. 20, 2025, 10:11 a.m. UTC 
Set the enable GPIO low when acquiring it.

Fixes: d30bb512da3d ("media: Add a driver for the ov7251 camera sensor")
Cc: stable@vger.kernel.org
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/i2c/ov7251.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)


base-commit: 5de275c450c9496031006e0fb3f7c96a8fcaaa55

Comments

Dave Stevenson Jan. 20, 2025, 10:37 a.m. UTC | #1 
Hi Sakari

On Mon, 20 Jan 2025 at 10:11, Sakari Ailus <sakari.ailus@linux.intel.com> wrote:
>
> Set the enable GPIO low when acquiring it.
>
> Fixes: d30bb512da3d ("media: Add a driver for the ov7251 camera sensor")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Agreed that the datasheet says that as the regulators aren't on at
this point, the enable GPIO must be low.

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

> ---
>  drivers/media/i2c/ov7251.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
> index 30f61e04ecaf..f3e2d26bb840 100644
> --- a/drivers/media/i2c/ov7251.c
> +++ b/drivers/media/i2c/ov7251.c
> @@ -1696,7 +1696,7 @@ static int ov7251_probe(struct i2c_client *client)
>                 return PTR_ERR(ov7251->analog_regulator);
>         }
>
> -       ov7251->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
> +       ov7251->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
>         if (IS_ERR(ov7251->enable_gpio)) {
>                 dev_err(dev, "cannot get enable gpio\n");
>                 return PTR_ERR(ov7251->enable_gpio);
>
> base-commit: 5de275c450c9496031006e0fb3f7c96a8fcaaa55
> --
> 2.39.5
>
>
Andy Shevchenko Jan. 20, 2025, 5:24 p.m. UTC | #2 
On Mon, Jan 20, 2025 at 12:11:22PM +0200, Sakari Ailus wrote:
> Set the enable GPIO low when acquiring it.

...

> -	ov7251->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
> +	ov7251->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);

Fine with me, but it's not helpful for MS Surface family of the devices anyway.
diff mbox series

Patch

diff --git a/drivers/media/i2c/ov7251.c b/drivers/media/i2c/ov7251.c
index 30f61e04ecaf..f3e2d26bb840 100644
--- a/drivers/media/i2c/ov7251.c
+++ b/drivers/media/i2c/ov7251.c
@@ -1696,7 +1696,7 @@  static int ov7251_probe(struct i2c_client *client)
 		return PTR_ERR(ov7251->analog_regulator);
 	}
 
-	ov7251->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_HIGH);
+	ov7251->enable_gpio = devm_gpiod_get(dev, "enable", GPIOD_OUT_LOW);
 	if (IS_ERR(ov7251->enable_gpio)) {
 		dev_err(dev, "cannot get enable gpio\n");
 		return PTR_ERR(ov7251->enable_gpio);