Hi Benoit, On Fri, Sep 20, 2019 at 11:55:29AM -0500, Benoit Parrot wrote: ... > > > @@ -1400,6 +1440,18 @@ static int ov2659_probe(struct i2c_client *client) > > > ov2659->xvclk_frequency > 27000000) > > > return -EINVAL; > > > > > > + /* Optional gpio don't fail if not present */ > > > + ov2659->pwdn_gpio = devm_gpiod_get_optional(&client->dev, "powerdown", > > > + GPIOD_OUT_LOW); > > > + if (IS_ERR(ov2659->pwdn_gpio)) > > > + return PTR_ERR(ov2659->pwdn_gpio); > > > + > > > + /* Optional gpio don't fail if not present */ > > > + ov2659->resetb_gpio = devm_gpiod_get_optional(&client->dev, "reset", > > > + GPIOD_OUT_HIGH); > > > + if (IS_ERR(ov2659->resetb_gpio)) > > > + return PTR_ERR(ov2659->resetb_gpio); > > > + > > > v4l2_ctrl_handler_init(&ov2659->ctrls, 2); > > > ov2659->link_frequency = > > > v4l2_ctrl_new_std(&ov2659->ctrls, &ov2659_ctrl_ops, > > > @@ -1445,6 +1497,9 @@ static int ov2659_probe(struct i2c_client *client) > > > ov2659->frame_size = &ov2659_framesizes[2]; > > > ov2659->format_ctrl_regs = ov2659_formats[0].format_ctrl_regs; > > > > > > + pm_runtime_enable(&client->dev); > > > + pm_runtime_get_sync(&client->dev); > > > > This makes the driver depend on runtime PM. > > Obviously. > Why? Is that bad? Well, if it is, then it should be listed in driver's dependencies in Kconfig. > > > > > See e.g. the smiapp driver for an example how to make it work without. It > > wasn't trivial. :I You won't need autosuspend. > > I took a look at that driver, but I don't get your reference to being able > to work without runtime pm! The driver didn't need runtime PM, so it'd be nice to continue work without. What smiapp does is that it powers the sensor on first *without* runtime PM, and then proceeds to set up runtime PM if it's available. The sensor will only be powered off when the device is unbound with runtime PM disabled. Regarding the smiapp driver, you can replace pm_runtime_get_noresume() and all the autoidle lines with pm_runtime_idle() call after pm_runtime_enable() in the ov2659 driver. > That driver looks pretty similar to ov7740.c which I used as a reference > for this. I guess in practice many sensor drivers don't work without it on DT-based systems I'm afraid. :-( They should be fixed. -- Kind regards, Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxx
