Re: [PATCH v3 4/5] media: platform: Add Renesas Input Video Control block driver

[Dan Scally <dan.scally@ideasonboard.com>]

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@pengutronix.de/
> 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.


Thanks

Dan