Re: [PATCH V11 2/2] media: i2c: ov02a10: Add OV02A10 image sensor driver

[Andy Shevchenko <andriy.shevchenko@linux.intel.com>]

On Tue, Jun 30, 2020 at 04:21:31PM +0200, Tomasz Figa wrote:
> On Tue, Jun 30, 2020 at 4:19 PM Tomasz Figa <tfiga@chromium.org> wrote:
> > On Tue, Jun 30, 2020 at 11:54 AM Sakari Ailus
> > <sakari.ailus@linux.intel.com> wrote:

...

> > > > +     ov02a10->rst_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> > >
> > >
> > > Shouldn't this be GPIOD_OUT_HIGH? That's the state it'll be after powering
> > > the sensor off the first time. Alternatively make reset signal high in
> > > runtime suspend callback.
> >
> > We might want to keep the signals low when the regulators are powered
> > down, to reduce the leakage.
> 
> Ah, I actually recall that the reset pin was physically active low, so
> we would indeed better keep it at HIGH. Sorry for the noise.

Here HIGH means "asserted", so in the code above it's LOW, means "deasserted",
i.o.w. non-reset state. I dunno what is better, it depends to the firmware /
driver expectations of the device configuration (from the power point of view,
HIGH makes sense here, and not LOW, i.o.w. asserted reset line).

And nice of the logical state that it doesn't depend to the active low / high
(the latter just transparent to the user).

-- 
With Best Regards,
Andy Shevchenko