Hi Laurent, On Tue, Sep 10, 2024 at 01:11:37PM +0300, Laurent Pinchart wrote: > On Tue, Sep 10, 2024 at 12:56:38PM +0300, Tomi Valkeinen wrote: > > On 10/09/2024 12:19, Jacopo Mondi wrote: > > > > > However, I think this current patch is correct (assuming the above > > > reasoning on i2c sensor drivers is correct) and doesn't require > > > CONFIG_PM, so I would be tempted to keep this version. > > > > I think the existence of this discussion alone proves my point that we > > should only support PM-case, unless !PM is a requirement =). > > For me it proves there's a dire need to document the runtime PM API in a > way that a human could understand :-) > > > But if you do want to keep !PM: > > > > Is there a reason why not mark the device as active with > > pm_runtime_set_active() after calling pispbe_runtime_resume and before > > accessing the device? That feels like the most logical way to use the > > function, and it would be right regardless whether the core will enable > > the parents before probe() or not. > > Does pm_runtime_set_active() resume the parent ? No, it simply sets the device's runtime PM status. > > > And not related to the BE or CFE drivers, but it strikes me odd that to > > support PM and !PM we need to play with these tricks. I think the core > > should just do the right thing if the driver does pm_runtime_get_sync() > > even with !PM (although maybe the time has passed to be able to do that). > > The runtime PM concepts are nice, but the API is wrong in my opinion. > Instead of being designed to expose the internals of runtime PM, it > should focus on usability from a driver point of view first. It's been moving a little bit to that direction, largely with new helper functions. I²C devices have been powered on for probe since commit a76e9bd89ae70 . Relation to runtime PM wasn't considered at the time, apparently. -- Kind regards, Sakari Ailus