Re: [PATCH v2] media: ov5640: Use runtime PM

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Sakari,

On Mon, Mar 14, 2022 at 11:11:18PM +0200, Sakari Ailus wrote:
> On Mon, Mar 14, 2022 at 10:05:37PM +0200, Laurent Pinchart wrote:
> ...
> > > > Yes, after reading the version register (or doing any other harware
> > > > access). Actually the full code would be
> > > > 
> > > > 
> > > >  	pm_runtime_enable(dev);
> > > >  	pm_runtime_resume_and_get(dev);
> > > > 
> > > > 	/* Hardware access */
> > > > 
> > > > 	pm_runtime_set_autosuspend_delay(dev, 1000);
> > > > 	pm_runtime_use_autosuspend(dev);
> > > > 	pm_runtime_put_autosuspend(dev);
> > > > 
> > > > (plus error handling).
> > > > 
> > > > If the probe function doesn't need to access the hardware, then
> > > > the above becomes
> > > > 
> > > > 	pm_runtime_enable(dev);
> > > > 	pm_runtime_set_autosuspend_delay(dev, 1000);
> > > > 	pm_runtime_use_autosuspend(dev);
> > > > 
> > > > instead of having to power up the device just in case !PM.
> > > > 
> > > > > Also the latter only works on DT-based systems so it's not an option for
> > > > > most of the drivers.

Does the former work on ACPI systems ?

> > > > How so, what's wrong with the above for ACPI-based system ?
> > > 
> > > I²C devices are already powered on for probe on ACPI based systems.
> > 
> > Not through RPM I suppose ?
> 
> Runtime PM isn't involved, this takes place in the ACPI framework (via
> dev_pm_domain_attach() called in i2c_device_probe()).

How can we fix this ? It may have made sense a long time ago, but it's
making RPM handling way too difficult in I2C drivers now. We need
something better instead of continuing to rely on cargo-cult for probe
functions. Most drivers are broken.

-- 
Regards,

Laurent Pinchart