On Wed, 9 Jul 2025 at 10:23, Jacopo Mondi <jacopo.mondi@xxxxxxxxxxxxxxxx> wrote: > I recall Sakari in the past had opinions on platform drivers selecting > On Wed, Jul 09, 2025 at 09:13:51AM +0100, Dan Scally wrote: > > On 08/07/2025 16:51, Jacopo Mondi wrote: > > > On Tue, Jul 08, 2025 at 03:57:46PM +0100, Dan Scally wrote: > > > > > > +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. You could just remove them anyway, iff you would use the newer *_PM_OPS() instead of the old SET_*_PM_OPS(). P.S. I already converted all Renesas drivers locally, so no need to start working on the conversion to *_PM_OPS() for existing Renesas drivers. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds