Re: [PATCH 09/12] rtc: rzn1: Use temporary variable for struct device

[Wolfram Sang <wsa+renesas@sang-engineering.com>]

[-- Attachment #1: Type: text/plain, Size: 1657 bytes --]

Hi Prabhakar,

> > > Could userspace still interact with the RTC during this window via ioctls
> > > or sysfs, potentially causing synchronous bus faults since the hardware is
> > > already suspended? Should teardown be bound using
> > > devm_add_action_or_reset() to guarantee correct reverse execution order?
> >
> > [wsa] Or maybe not use devm then?
> >
> Maybe just add two `devm_add_action_or_reset()` calls: one to clear
> interrupts and another to call pm_runtime_put()?

Why two? I wouldn't think it matters much if the two are separated. Main
problem seems to me the disabled clocks because of pm_runtime_put()? But
maybe I am overlooking sth?

> > > [Severity: High]
> > > This is a pre-existing issue, but does using pm_runtime_put() here (and in
> > > rzn1_rtc_remove) leave the device powered on indefinitely?
> > >
> > > Since devm_pm_runtime_enable(dev) schedules pm_runtime_disable() to
> > > execute during devres cleanup, calling pm_runtime_put() only queues an
> > > asynchronous idle check. The immediate return triggers devres cleanup,
> > > which executes a barrier that explicitly cancels pending async operations.
> > >
> > > Should this use pm_runtime_put_sync() instead to ensure the device is
> > > synchronously suspended before teardown?
> > >
> Although there were some patches accepted for similar kind of issue
> reported by Sashiko, do you think I should switch to
> pm_runtime_put_sync() (Ive not seen any issues)

I am not a PM expert, so I can't guide you. The report from Sashiko
sounds reasonable to me. But you'd have to look up the code path to
verify the reasoning. Or ask an PM expert.

Happy hacking,

   Wolfram


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]