mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git
synced 2024-08-21 00:10:09 +00:00
4cc47e8add
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 <dmitry.baryshkov@linaro.org>
Cc: Johan Hovold <johan+linaro@kernel.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Taniya Das <quic_tdas@quicinc.com>
Cc: Satya Priya <quic_c_skakit@quicinc.com>
Reviewed-by: Douglas Anderson <dianders@chromium.org>
Tested-by: Douglas Anderson <dianders@chromium.org>
Cc: Matthias Kaehlcke <mka@chromium.org>
Reported-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/CAE-0n52xbZeJ66RaKwggeRB57fUAwjvxGxfFMKOKJMKVyFTe+w@mail.gmail.com [1]
Fixes: 1b771839de ("clk: qcom: gdsc: enable optional power domain support")
Signed-off-by: Stephen Boyd <swboyd@chromium.org>
Link: https://lore.kernel.org/r/20221103183030.3594899-1-swboyd@chromium.org
Tested-by: Johan Hovold <johan+linaro@kernel.org>
Reviewed-by: Johan Hovold <johan+linaro@kernel.org>
Signed-off-by: Stephen Boyd <sboyd@kernel.org>
99 lines
2.8 KiB
C
99 lines
2.8 KiB
C
/* SPDX-License-Identifier: GPL-2.0-only */
|
|
/*
|
|
* Copyright (c) 2015, 2017-2018, 2022, The Linux Foundation. All rights reserved.
|
|
*/
|
|
|
|
#ifndef __QCOM_GDSC_H__
|
|
#define __QCOM_GDSC_H__
|
|
|
|
#include <linux/err.h>
|
|
#include <linux/pm_domain.h>
|
|
|
|
struct regmap;
|
|
struct regulator;
|
|
struct reset_controller_dev;
|
|
|
|
/**
|
|
* struct gdsc - Globally Distributed Switch Controller
|
|
* @pd: generic power domain
|
|
* @regmap: regmap for MMIO accesses
|
|
* @gdscr: gsdc control register
|
|
* @collapse_ctrl: APCS collapse-vote register
|
|
* @collapse_mask: APCS collapse-vote mask
|
|
* @gds_hw_ctrl: gds_hw_ctrl register
|
|
* @cxcs: offsets of branch registers to toggle mem/periph bits in
|
|
* @cxc_count: number of @cxcs
|
|
* @pwrsts: Possible powerdomain power states
|
|
* @en_rest_wait_val: transition delay value for receiving enr ack signal
|
|
* @en_few_wait_val: transition delay value for receiving enf ack signal
|
|
* @clk_dis_wait_val: transition delay value for halting clock
|
|
* @resets: ids of resets associated with this gdsc
|
|
* @reset_count: number of @resets
|
|
* @rcdev: reset controller
|
|
*/
|
|
struct gdsc {
|
|
struct generic_pm_domain pd;
|
|
struct generic_pm_domain *parent;
|
|
struct regmap *regmap;
|
|
unsigned int gdscr;
|
|
unsigned int collapse_ctrl;
|
|
unsigned int collapse_mask;
|
|
unsigned int gds_hw_ctrl;
|
|
unsigned int clamp_io_ctrl;
|
|
unsigned int *cxcs;
|
|
unsigned int cxc_count;
|
|
unsigned int en_rest_wait_val;
|
|
unsigned int en_few_wait_val;
|
|
unsigned int clk_dis_wait_val;
|
|
const u8 pwrsts;
|
|
/* Powerdomain allowable state bitfields */
|
|
#define PWRSTS_OFF BIT(0)
|
|
/*
|
|
* There is no SW control to transition a GDSC into
|
|
* PWRSTS_RET. This happens in HW when the parent
|
|
* domain goes down to a low power state
|
|
*/
|
|
#define PWRSTS_RET BIT(1)
|
|
#define PWRSTS_ON BIT(2)
|
|
#define PWRSTS_OFF_ON (PWRSTS_OFF | PWRSTS_ON)
|
|
#define PWRSTS_RET_ON (PWRSTS_RET | PWRSTS_ON)
|
|
const u16 flags;
|
|
#define VOTABLE BIT(0)
|
|
#define CLAMP_IO BIT(1)
|
|
#define HW_CTRL BIT(2)
|
|
#define SW_RESET BIT(3)
|
|
#define AON_RESET BIT(4)
|
|
#define POLL_CFG_GDSCR BIT(5)
|
|
#define ALWAYS_ON BIT(6)
|
|
#define RETAIN_FF_ENABLE BIT(7)
|
|
#define NO_RET_PERIPH BIT(8)
|
|
struct reset_controller_dev *rcdev;
|
|
unsigned int *resets;
|
|
unsigned int reset_count;
|
|
|
|
const char *supply;
|
|
struct regulator *rsupply;
|
|
};
|
|
|
|
struct gdsc_desc {
|
|
struct device *dev;
|
|
struct gdsc **scs;
|
|
size_t num;
|
|
};
|
|
|
|
#ifdef CONFIG_QCOM_GDSC
|
|
int gdsc_register(struct gdsc_desc *desc, struct reset_controller_dev *,
|
|
struct regmap *);
|
|
void gdsc_unregister(struct gdsc_desc *desc);
|
|
int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain);
|
|
#else
|
|
static inline int gdsc_register(struct gdsc_desc *desc,
|
|
struct reset_controller_dev *rcdev,
|
|
struct regmap *r)
|
|
{
|
|
return -ENOSYS;
|
|
}
|
|
|
|
static inline void gdsc_unregister(struct gdsc_desc *desc) {};
|
|
#endif /* CONFIG_QCOM_GDSC */
|
|
#endif /* __QCOM_GDSC_H__ */
|