| Message ID | 20230123125205.622152-53-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: atomisp: Big power-management changes + lots of fixes | expand |
On Mon, Jan 23, 2023 at 01:52:00PM +0100, Hans de Goede wrote: > The comment claims the PWDN pin is active when pulled down in other words, > it is /power-down so it needs to be driven high to get the sensor > powered-up (not powered down) and flag is 1 when powering-up the sensor > so the ! is wrong, drop it. > > This also matches with the schematics which I have which shows GPIO1 also > enables a 3.3v line to the sensor-module which controls the privacy-LED > and indeed before this patch the privacy LED was inverted from what it > should be (and the sensor did not work). It's unclear if this is logical state or raw one? Maybe this the root cause of the initial confusion?
On Mon, Jan 23, 2023 at 01:52:00PM +0100, Hans de Goede wrote: > The comment claims the PWDN pin is active when pulled down in other words, > it is /power-down so it needs to be driven high to get the sensor > powered-up (not powered down) and flag is 1 when powering-up the sensor > so the ! is wrong, drop it. > > This also matches with the schematics which I have which shows GPIO1 also > enables a 3.3v line to the sensor-module which controls the privacy-LED > and indeed before this patch the privacy LED was inverted from what it > should be (and the sensor did not work). Code-wise it's okay Reviewed-by: Andy Shevchenko <andy@kernel.org> > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > index d874e12da8cc..83d036b5d772 100644 > --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c > @@ -512,10 +512,7 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag) > * before PWDN# when turning it on or off. > */ > ret = dev->platform_data->gpio0_ctrl(sd, flag); > - /* > - *ov2722 PWDN# active high when pull down,opposite to the convention > - */ > - ret |= dev->platform_data->gpio1_ctrl(sd, !flag); > + ret |= dev->platform_data->gpio1_ctrl(sd, flag); > return ret; > } > > -- > 2.39.0 >
diff --git a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c index d874e12da8cc..83d036b5d772 100644 --- a/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c +++ b/drivers/staging/media/atomisp/i2c/atomisp-ov2722.c @@ -512,10 +512,7 @@ static int gpio_ctrl(struct v4l2_subdev *sd, bool flag) * before PWDN# when turning it on or off. */ ret = dev->platform_data->gpio0_ctrl(sd, flag); - /* - *ov2722 PWDN# active high when pull down,opposite to the convention - */ - ret |= dev->platform_data->gpio1_ctrl(sd, !flag); + ret |= dev->platform_data->gpio1_ctrl(sd, flag); return ret; }
The comment claims the PWDN pin is active when pulled down in other words, it is /power-down so it needs to be driven high to get the sensor powered-up (not powered down) and flag is 1 when powering-up the sensor so the ! is wrong, drop it. This also matches with the schematics which I have which shows GPIO1 also enables a 3.3v line to the sensor-module which controls the privacy-LED and indeed before this patch the privacy LED was inverted from what it should be (and the sensor did not work). Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/staging/media/atomisp/i2c/atomisp-ov2722.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)