On 9/6/23 12:23, Ulf Hansson wrote:
On Mon, 28 Aug 2023 at 13:42, Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> wrote:
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
This commit introduces a new driver, based on the one for cpr v1,
to enable support for the newer Qualcomm Core Power Reduction
hardware, known downstream as CPR3, CPR4 and CPRh, and support
for MSM8998 and SDM630 CPU power reduction.
In these new versions of the hardware, support for various new
features was introduced, including voltage reduction for the GPU,
security hardening and a new way of controlling CPU DVFS,
consisting in internal communication between microcontrollers,
specifically the CPR-Hardened and the Operating State Manager.
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, SDM630/636/660/845.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxxx>
[Konrad: rebase, apply review comments]
Tested-by: Jeffrey Hugo <quic_jhugo@xxxxxxxxxxx>
Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx>
---
[...]
+
+static void cpr_pd_detach_dev(struct generic_pm_domain *domain,
+ struct device *dev)
+{
+ struct cpr_thread *thread = container_of(domain, struct cpr_thread, pd);
+ struct cpr_drv *drv = thread->drv;
+
+ mutex_lock(&drv->lock);
+
+ dev_dbg(drv->dev, "detach callback for: %s\n", dev_name(dev));
+ thread->attached_cpu_dev = NULL;
+
+ mutex_unlock(&drv->lock);
Don't you need to do some additional cleanup here? Like calling
dev_pm_opp_of_remove_table() for example?
Ouch, right..
[...]
+
+ /* CPR-Hardened performance states are managed in firmware */
+ if (desc->cpr_type == CTRL_TYPE_CPRH)
+ thread->pd.set_performance_state = cprh_dummy_set_performance_state;
The dummy function above always returns 0, without actually doing
anything. I am trying to understand the purpose of this.
Would you mind elaborating on this a bit?
It looks like this was put in place to overcome the
.. && genpd->set_performance_state)
check in of_genpd_add_provider_onecell() that gatekeeps calling
functions that parse OPP tables from DT
[...]
Note that, this was mostly a drive-by-review, looking at the genpd
provider specific parts. In general this looks good to me, other than
the minor comments I had above.
No worries, every time I open this file, I end up fixing more and
more things..
Konrad
