On Mon, Jul 08, 2024 at 02:22:40PM +0200, Konrad Dybcio wrote:
> From: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
>
> Introduce a new driver (based on qcom-cpr for CPRv1 IP) for the newer
> Qualcomm Core Power Reduction hardware, known downstream as CPR[34h]
> (h for hardened).
>
> In these new CPR versions, support for various new features was introduced.
> That includes:
> * voltage reduction for the GPU
> * security hardening
> * a new way of controlling CPU DVFS, based on internal communication
> between CPRh and Operating State Manager MCUs.
>
> The CPR v3, v4 and CPRh are present in a broad range of SoCs, from the
> mid-range to the high end ones including, but not limited to:
> MSM8953/8996/8998 and SDM630/636/660/845.
>
> Note that to reduce the giant review and testing matrix of the driver, this
> patch (admittedly, somewhat confusingly but for good reasons) omits support
> for CPR*3* specifically, which is otherwise quite straightforward to add.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
> [Konrad: rebase, a whole lot of cleanup/fixes]
> Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
> ---
> drivers/pmdomain/qcom/Kconfig | 22 +
> drivers/pmdomain/qcom/Makefile | 4 +-
> drivers/pmdomain/qcom/cpr-common.h | 2 +
> drivers/pmdomain/qcom/cpr3.c | 2711 ++++++++++++++++++++++++++++++++++++
> include/soc/qcom/cpr.h | 17 +
> 5 files changed, 2755 insertions(+), 1 deletion(-)
>
.
.
.
> +/**
> + * cpr_irq_handler() - Handle CPR3/CPR4 status interrupts
> + * @irq: Number of the interrupt
> + * @dev: Pointer to the cpr_thread structure
> + *
> + * Handle the interrupts coming from non-hardened CPR HW as to get
> + * an ok to scale voltages immediately, or to pass error status to
> + * the hardware (either success/ACK or failure/NACK).
> + *
> + * Return: IRQ_SUCCESS for success, IRQ_NONE if the CPR is disabled.
> + */
> +static irqreturn_t cpr_irq_handler(int irq, void *dev)
> +{
> + struct cpr_thread *thread = dev;
While registering this handler with devm_request_threaded_irq,
'cpr_drv' pointer is used as the cookie. But, here the cookie is
being assumed to be cpr_thread. Is that intentional?
Getting this crash while testing:-
Unable to handle kernel NULL pointer dereference at virtual address 0000000000000010
Mem abort info:
ESR = 0x0000000096000004
EC = 0x25: DABT (current EL), IL = 32 bits
SET = 0, FnV = 0
EA = 0, S1PTW = 0
FSC = 0x04: level 0 translation fault
Data abort info:
ISV = 0, ISS = 0x00000004, ISS2 = 0x00000000
CM = 0, WnR = 0, TnD = 0, TagAccess = 0
GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
user pgtable: 4k pages, 48-bit VAs, pgdp=0000000046efd000
[0000000000000010] pgd=0000000000000000, p4d=0000000000000000
Internal error: Oops: 0000000096000004 [#1] PREEMPT SMP
Modules linked in:
CPU: 0 UID: 0 PID: 72 Comm: irq/26-cpr Not tainted 6.11.0-rc1-next-20240729-00019-g244903f9355b-dirty #28
Hardware name: Qualcomm Technologies, Inc. IPQ9574/AP-AL02-C7 (DT)
pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
pc : mutex_lock+0x20/0x54
lr : cpr_irq_handler+0x28/0x260
sp : ffff80008613bd80
x29: ffff80008613bd80 x28: ffff8000800b267c x27: ffff000006f92080
x26: ffff000002989200 x25: ffff000002b28000 x24: 0000000000000001
x23: ffff0000029892ac x22: ffff000002b28000 x21: 0000000000000000
x20: 0000000000000010 x19: ffff000002ba2880 x18: 0000000000000001
x17: 0000000073babcd7 x16: 0000000000000000 x15: ffff00003fc914c0
x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
x11: 0000000000000000 x10: 00000000000009b0 x9 : ffff80008613bd20
x8 : ffff000002b28a10 x7 : ffff00003fc91700 x6 : ffff00003fc90cc0
x5 : 00000000410fd090 x4 : 0000000000100000 x3 : 0000000000000000
x2 : ffff000002b28000 x1 : ffff000002ba2880 x0 : 0000000000000010
Call trace:
mutex_lock+0x20/0x54
irq_thread_fn+0x2c/0xa8
irq_thread+0x16c/0x274
kthread+0x110/0x114
ret_from_fork+0x10/0x20
Code: b5000141 d65f03c0 d2800003 f9800011 (c85ffc01)
---[ end trace 0000000000000000 ]---
Changing it to cpr_thread pointer in devm_request_threaded_irq
fixes the above crash.
> + struct cpr_drv *drv = thread->drv;
> + irqreturn_t ret = IRQ_HANDLED;
> + int i, rc;
> + enum voltage_change_dir dir = NO_CHANGE;
> + u32 val;
> +
> + guard(mutex)(&drv->lock);
> +
> + val = cpr_read(thread, CPR3_REG_IRQ_STATUS);
> +
> + dev_vdbg(drv->dev, "IRQ_STATUS = 0x%x\n", val);
> +
.
.
.
> +/**
> + * cpr_thread_init() - Initialize CPR thread related parameters
> + * @drv: Main driver structure
> + * @tid: Thread ID
> + *
> + * Return: Zero for success, negative number on error
> + */
> +static int cpr_thread_init(struct cpr_drv *drv, int tid)
> +{
> + const struct cpr_desc *desc = drv->desc;
> + const struct cpr_thread_desc *tdesc = desc->threads[tid];
> + struct cpr_thread *thread = &drv->threads[tid];
> + bool pd_registered = false;
> + int ret, i;
.
.
.
> + /* On CPRhardened, the interrupts are managed in firmware */
> + if (desc->cpr_type != CTRL_TYPE_CPRH) {
> + INIT_WORK(&thread->restart_work, cpr_restart_worker);
> +
> + ret = devm_request_threaded_irq(drv->dev, drv->irq,
> + NULL, cpr_irq_handler,
> + IRQF_ONESHOT |
> + IRQF_TRIGGER_RISING,
> + "cpr", drv);
'drv' or 'thread' ?
Thanks
Varada
> + if (ret)
> + goto fail;
> + }
