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 ? > 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.
