Hi Dan I recall Sakari in the past had opinions on platform drivers selecting CONFIG_PM, let's cc him On Wed, Jul 09, 2025 at 09:13:51AM +0100, Dan Scally wrote: > Morning > > On 08/07/2025 16:51, Jacopo Mondi wrote: > > Hi Dan > > > > On Tue, Jul 08, 2025 at 03:57:46PM +0100, Dan Scally wrote: > > > Hi Jacopo > > > > > [snip] > > > > > > > + > > > > > +static int __maybe_unused rzv2h_ivc_runtime_resume(struct device *dev) > > > > The driver doesn't depend or select CONFIG_PM, so this is rightfully > > > > marked as __maybe_unused. > > > > > > > > However, it doesn't seem to me that the probe() routine manually > > > > enable the peripheral, so in case of !CONFIG_PM am I wrong or the > > > > device won't operate at all ? > > > > > > > > I would select CONFIG_PM, or otherwise call this function from the probe() > > > > routine and then call pm_runtime_set_active() to inform runtime_pm > > > > that the peripheral is active, and at the end of the probe routine > > > > call pm_runtime_put_autosuspend(): in case of CONFIG_PM the peripheral > > > > will suspend, in case of !CONFIG_PM the pm_runtime_put_autosuspend() > > > > reduces to a nop leaving the peripheral enabled. > > > Ack > > > > I would just select CONFIG_PM tbh > > > I dropped it on Philipp's suggestion in the last review; I have no strong > > I only see a comment from Philipp here > > https://lore.kernel.org/all/8301d2862546507303e2dba1dd61906b848552c2.camel@xxxxxxxxxxxxxx/ > > about the RESET_CONTROLLER. Have I missed other comments maybe ? > Oh no you're right; I misremembered. Sorry for the noise! > > > > > feelings to be honest, I would expect it to be enabled in any configuration > > > that was intending to use this...but I suppose there's no harm accounting > > > for the possibility that it won't be > > no harm no, but a bit more complex handling of the device power up > > sequences. > > No problem; I'll just select CONFIG_PM. You can then remove __maybe_unused from the function declaration. > > > Thanks > > Dan >
