Hi Daniel, On Mon, Oct 11, 2021 at 11:22:10PM +0100, Daniel Scally wrote: > Hi Jacopo > > On 11/10/2021 15:34, Jacopo Mondi wrote: > >>>> +static int ar0521_remove(struct i2c_client *client) > >>>> +{ > >>>> + struct v4l2_subdev *sd = i2c_get_clientdata(client); > >>>> + struct ar0521_dev *sensor = to_ar0521_dev(sd); > >>>> + > >>>> + v4l2_async_unregister_subdev(&sensor->sd); > >>>> + media_entity_cleanup(&sensor->sd.entity); > >>>> + v4l2_ctrl_handler_free(&sensor->ctrls.handler); > >>>> + pm_runtime_disable(&client->dev); > >>>> + pm_runtime_set_suspended(&client->dev); > >>> set_suspended() then disable maybe ? > >> Other drivers seem to do it the above way but I don't know the > >> difference. > > Maybe I'm wrong but calling set_suspend() after pm_runtime() had been > > disabled seems pointless. A minor anyway as it's in the driver's > > remove function. > > > > fwiw, the kernel doc [1] for pm_runtime_set_suspended() does say that > it's not valid to call it for devices where runtime PM is still enabled. > Ah, great! thanks for pointing it out, it was very well visible in the documentation :) Sorry for the noise! Cheers j > > [1] > https://elixir.bootlin.com/linux/latest/source/include/linux/pm_runtime.h#L510 >