[PATCH] remoteproc: mediatek: Break lock dependency to `prepare_lock`

[Tzung-Bi Shih]

On Tue, Jan 06, 2026 at 10:10:27AM -0700, Mathieu Poirier wrote:
> On Tue, Jan 06, 2026 at 03:13:22AM +0000, Tzung-Bi Shih wrote:
> > On Mon, Jan 05, 2026 at 03:16:33PM -0700, Mathieu Poirier wrote:
> > > On Mon, Dec 29, 2025 at 04:31:46AM +0000, Tzung-Bi Shih wrote:
> > > > `scp_ipi_send` acquires `prepare_lock` via `clk_prepare_enable` while
> > > > the caller often holds `ec_dev->lock` (e.g., `cros_ec_cmd_xfer`).  The
> > > > reverse dependency exists where `clk_prepare` can trigger operations
> > > > that eventually take `ec_dev->lock` (e.g., via sysfs/regulator/genpd).
> > > 
> > > What operation would that be?  Please be specific so that I can trace the code.
> > 
> > The chain is discovered by lockdep: &ec_dev->lock -> prepare_lock ->
> > &genpd->mlock -> ... -> kn->active#2 -> &ec_dev->lock.
> > 
> > -> #6 (&ec_dev->lock){+.+.}-{3:3}:
> >        __mutex_lock_common
> >        mutex_lock_nested
> >        cros_ec_cmd_xfer
> >        cros_ec_cmd_xfer_status
> >        cros_usbpd_charger_get_port_status
> >        cros_usbpd_charger_get_prop
> >        power_supply_get_property
> >        power_supply_show_property
> >        power_supply_uevent
> >        dev_uevent
> >        uevent_show
> >        dev_attr_show
> >        sysfs_kf_seq_show
> >        kernfs_seq_show
> >        seq_read_iter
> >        kernfs_fop_read_iter
> >        vfs_read
> > -> #5 (kn->active#2){++++}-{0:0}:
> >        kernfs_drain
> >        __kernfs_remove
> >        kernfs_remove_by_name_ns
> >        sysfs_remove_file_ns
> >        device_del
> >        __device_link_del
> >        device_links_driver_bound
> >        driver_bound
> > -> #4 (device_links_lock){+.+.}-{3:3}:
> >        __mutex_lock_common
> >        mutex_lock_nested
> >        device_link_remove
> >        _regulator_put
> >        regulator_put
> >        devm_regulator_release
> > ...
> > -> #1 (&genpd->mlock){+.+.}-{3:3}:
> >        __mutex_lock_common
> >        mutex_lock_nested
> >        genpd_lock_mtx
> >        genpd_runtime_resume
> >        __rpm_callback
> >        rpm_callback
> >        rpm_resume
> >        __pm_runtime_resume
> >        clk_core_prepare
> >        clk_prepare
> > -> #0 (prepare_lock){+.+.}-{3:3}:
> >        __lock_acquire
> >        lock_acquire
> >        __mutex_lock_common
> >        mutex_lock_nested
> >        clk_prepare
> >        scp_ipi_send
> >        scp_send_ipi
> >        mtk_rpmsg_send
> >        rpmsg_send
> >        cros_ec_pkt_xfer_rpmsg
> >
> 
> From what I understand, cros_ec_cmd_xfer() gets called and takes @ec_dev->lock.
> From there scp_ipi_send() and clk_prepare_enable() are eventually called.  The
> latter takes @prepare_lock and proceeds to enable the mechanic that will get the
> clock prepared.  The process to enable the clock mechanic, which may happen on
> a different CPU, involves calling cros_ec_cmd_xfer() and lockdep complains
> because @ec_dev->lock is already held. 
>  
> > > > Move clock prepare / unprepare operations to remoteproc prepare() /
> > > > unprepare() callbacks to break the lock dependency from `ec_dev->lock`
> > > > to `prepare_lock`.
> > > 
> > > With the information presented to me, I don't see how doing that changes
> > > anything.  @prepare_lock is simply held for a longer period of time.  
> > 
> > In prepare() callback, the clock becomes prepared and prepare_lock won't be
> > held after that.
> 
> If my theory (above) is correct, you are proposing to avoid the condition by
> preparing the clock ahead of time before any IPI can take place.  Is this
> correct?

Correct, so that it doesn't need to prepare the clock (i.e., acquire the
@prepare_lock) when @ec_dev->lock is held.