On Tue, Jun 30, 2020 at 4:37 PM Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote: > > 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@xxxxxxxxxxxx> wrote: > > > On Tue, Jun 30, 2020 at 11:54 AM Sakari Ailus > > > <sakari.ailus@xxxxxxxxxxxxxxx> 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). Yeah, after reading through the GPIO subsystem documentation and discussing with a bunch of people on how to interpret it, we concluded that the driver needs to deal only with the logical state of the signal. Actually, I later realized that the problem of leakage should likely be solved by pinctrl suspend settings, based on given hardware conditions, rather than the driver explicitly. Best regards, Tomasz
