| Message ID | 20250504101336.18748-9-hdegoede@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | media: mt9m114: Changes to make it work with atomisp devices | expand |
Hi Hans, Thank you for the patch. On Sun, May 04, 2025 at 12:13:29PM +0200, Hans de Goede wrote: > mt9m114_probe() requests the reset GPIO in output low state: > > sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > > and then almost immediately afterwards calls mt9m114_power_on() which does: > > gpiod_set_value(sensor->reset, 1); > fsleep(duration); > gpiod_set_value(sensor->reset, 0); > > which means that if the reset pin was high before this code runs that > it will very briefly be driven low because of passing GPIOD_OUT_LOW when > requesting the GPIO only to be driven high again possibly directly after > that. Such a very brief driving low of the reset pin may put the chip in > a confused state. > > Request the GPIO in high (reset the chip) state instead to avoid this, > turning the initial gpiod_set_value() in mt9m114_power_on() into a no-op. > and the fsleep() ensures that it will stay high long enough to properly > reset the chip. > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/media/i2c/mt9m114.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c > index be3e7bb44ad8..43efcbdf614e 100644 > --- a/drivers/media/i2c/mt9m114.c > +++ b/drivers/media/i2c/mt9m114.c > @@ -2412,7 +2412,7 @@ static int mt9m114_probe(struct i2c_client *client) > } > } > > - sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); > + sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); > if (IS_ERR(sensor->reset)) { > ret = PTR_ERR(sensor->reset); > dev_err_probe(dev, ret, "Failed to get reset GPIO\n");
diff --git a/drivers/media/i2c/mt9m114.c b/drivers/media/i2c/mt9m114.c index be3e7bb44ad8..43efcbdf614e 100644 --- a/drivers/media/i2c/mt9m114.c +++ b/drivers/media/i2c/mt9m114.c @@ -2412,7 +2412,7 @@ static int mt9m114_probe(struct i2c_client *client) } } - sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); + sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_HIGH); if (IS_ERR(sensor->reset)) { ret = PTR_ERR(sensor->reset); dev_err_probe(dev, ret, "Failed to get reset GPIO\n");
mt9m114_probe() requests the reset GPIO in output low state: sensor->reset = devm_gpiod_get_optional(dev, "reset", GPIOD_OUT_LOW); and then almost immediately afterwards calls mt9m114_power_on() which does: gpiod_set_value(sensor->reset, 1); fsleep(duration); gpiod_set_value(sensor->reset, 0); which means that if the reset pin was high before this code runs that it will very briefly be driven low because of passing GPIOD_OUT_LOW when requesting the GPIO only to be driven high again possibly directly after that. Such a very brief driving low of the reset pin may put the chip in a confused state. Request the GPIO in high (reset the chip) state instead to avoid this, turning the initial gpiod_set_value() in mt9m114_power_on() into a no-op. and the fsleep() ensures that it will stay high long enough to properly reset the chip. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/media/i2c/mt9m114.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)