Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
clk: qcom: gdsc: Remove direct runtime PM calls
We shouldn't be calling runtime PM APIs from within the genpd enable/disable path for a couple reasons. First, this causes an AA lockdep splat[1] because genpd can call into genpd code again while holding the genpd lock. WARNING: possible recursive locking detected 5.19.0-rc2-lockdep+ #7 Not tainted -------------------------------------------- kworker/2:1/49 is trying to acquire lock: ffffffeea0370788 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x24/0x30 but task is already holding lock: ffffffeea03710a8 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x24/0x30 other info that might help us debug this: Possible unsafe locking scenario: CPU0 ---- lock(&genpd->mlock); lock(&genpd->mlock); *** DEADLOCK *** May be due to missing lock nesting notation 3 locks held by kworker/2:1/49: #0: 74ffff80811a5748 ((wq_completion)pm){+.+.}-{0:0}, at: process_one_work+0x320/0x5fc #1: ffffffc008537cf8 ((work_completion)(&genpd->power_off_work)){+.+.}-{0:0}, at: process_one_work+0x354/0x5fc #2: ffffffeea03710a8 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x24/0x30 stack backtrace: CPU: 2 PID: 49 Comm: kworker/2:1 Not tainted 5.19.0-rc2-lockdep+ #7 Hardware name: Google Lazor (rev3 - 8) with KB Backlight (DT) Workqueue: pm genpd_power_off_work_fn Call trace: dump_backtrace+0x1a0/0x200 show_stack+0x24/0x30 dump_stack_lvl+0x7c/0xa0 dump_stack+0x18/0x44 __lock_acquire+0xb38/0x3634 lock_acquire+0x180/0x2d4 __mutex_lock_common+0x118/0xe30 mutex_lock_nested+0x70/0x7c genpd_lock_mtx+0x24/0x30 genpd_runtime_suspend+0x2f0/0x414 __rpm_callback+0xdc/0x1b8 rpm_callback+0x4c/0xcc rpm_suspend+0x21c/0x5f0 rpm_idle+0x17c/0x1e0 __pm_runtime_idle+0x78/0xcc gdsc_disable+0x24c/0x26c _genpd_power_off+0xd4/0x1c4 genpd_power_off+0x2d8/0x41c genpd_power_off_work_fn+0x60/0x94 process_one_work+0x398/0x5fc worker_thread+0x42c/0x6c4 kthread+0x194/0x1b4 ret_from_fork+0x10/0x20 Second, this confuses runtime PM on CoachZ for the camera devices by causing the camera clock controller's runtime PM usage_count to go negative after resuming from suspend. This is because runtime PM is being used on the clock controller while runtime PM is disabled for the device. The reason for the negative count is because a GDSC is represented as a genpd and each genpd that is attached to a device is resumed during the noirq phase of system wide suspend/resume (see the noirq suspend ops assignment in pm_genpd_init() for more details). The camera GDSCs are attached to camera devices with the 'power-domains' property in DT. Every device has runtime PM disabled in the late system suspend phase via __device_suspend_late(). Runtime PM is not usable until runtime PM is enabled in device_resume_early(). The noirq phases run after the 'late' and before the 'early' phase of suspend/resume. When the genpds are resumed in genpd_resume_noirq(), we call down into gdsc_enable() that calls pm_runtime_resume_and_get() and that returns -EACCES to indicate failure to resume because runtime PM is disabled for all devices. Upon closer inspection, calling runtime PM APIs like this in the GDSC driver doesn't make sense. It was intended to make sure the GDSC for the clock controller providing other GDSCs was enabled, specifically the MMCX GDSC for the display clk controller on SM8250 (sm8250-dispcc), so that GDSC register accesses succeeded. That will already happen because we make the 'dev->pm_domain' a parent domain of each GDSC we register in gdsc_register() via pm_genpd_add_subdomain(). When any of these GDSCs are accessed, we'll enable the parent domain (in this specific case MMCX). We also remove any getting of runtime PM during registration, because when a genpd is registered it increments the count on the parent if the genpd itself is already enabled. Cc: Dmitry Baryshkov <[email protected]> Cc: Johan Hovold <[email protected]> Cc: Ulf Hansson <[email protected]> Cc: Taniya Das <[email protected]> Cc: Satya Priya <[email protected]> Reviewed-by: Douglas Anderson <[email protected]> Tested-by: Douglas Anderson <[email protected]> Cc: Matthias Kaehlcke <[email protected]> Reported-by: Stephen Boyd <[email protected]> Link: https://lore.kernel.org/r/CAE-0n52xbZeJ66RaKwggeRB57fUAwjvxGxfFMKOKJMKVyFTe+w@mail.gmail.com [1] Fixes: 1b77183 ("clk: qcom: gdsc: enable optional power domain support") Signed-off-by: Stephen Boyd <[email protected]> Link: https://lore.kernel.org/r/[email protected] Tested-by: Johan Hovold <[email protected]> Reviewed-by: Johan Hovold <[email protected]> Signed-off-by: Stephen Boyd <[email protected]>
- Loading branch information
