Il 27/02/23 03:55, Dmitry Baryshkov ha scritto:
On 17/02/2023 13:08, Konrad Dybcio 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] Signed-off-by: Konrad Dybcio <konrad.dybcio@xxxxxxxxxx> --- drivers/soc/qcom/Kconfig | 22 + drivers/soc/qcom/Makefile | 4 +- drivers/soc/qcom/cpr-common.h | 2 + drivers/soc/qcom/cpr3.c | 2923 +++++++++++++++++++++++++++++++++++++++++ include/soc/qcom/cpr.h | 17 + 5 files changed, 2967 insertions(+), 1 deletion(-) diff --git a/include/soc/qcom/cpr.h b/include/soc/qcom/cpr.h new file mode 100644 index 000000000000..2ba4324d18f6 --- /dev/null +++ b/include/soc/qcom/cpr.h @@ -0,0 +1,17 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (c) 2013-2020, The Linux Foundation. All rights reserved. + * Copyright (c) 2019 Linaro Limited + * Copyright (c) 2021, AngeloGioacchino Del Regno + * <angelogioacchino.delregno@xxxxxxxxxxxxxx> + */ + +#ifndef __CPR_H__ +#define __CPR_H__ + +struct cpr_ext_data { + int mem_acc_threshold_uV; + int apm_threshold_uV; +};Who is going to use this? Is it the cpufreq driver or some other driver? We are adding an API without a clean user, can we drop it for now?
This is mandatory: qcom-cpufreq-hw is supposed to program the OSM before starting. From SDM845 onwards, the OSM is programmed by the bootloader before booting Linux; In MSM8996/98, SDM630/636/660, others, the bootloader does not program the OSM uC, so this has to be done in Linux - specifically, in the CPUFREQ driver (qcom-cpufreq-hw), otherwise this driver is completely pointless to have. CPU DVFS requires three uC to be correctly programmed in order to work: - SAW (for sleep states) - CPR-Hardened (voltage control, mandatory for stability) - OSM (for cpufreq-hw frequency steps [1..N]) Failing to *correctly* program either of the three will render CPU DVFS unusable. That clarified, my opinion is: No, you can't drop this. It's an essential piece for functionality. I agree in that this commit introduces a header that has only an internal (as in cpr3.c) user and no external ones, but I think that Konrad didn't want to include the qcom-cpufreq-hw.c commits in this series because it's already huge and pretty difficult to review; adding the cpufreq-hw commits would make the situation worse. Konrad, perhaps you can send the cpufreq-hw commits in a separate series, in which cover letter you mention a dependency on this one? That would *clearly* show the full picture to reviewers. I remember that when I sent the cpufreq-hw series along with this one (~2 years ago, I think?) that code had positive reviews from Bjorn, so it should be OK. It wasn't picked just-only-because of the cpr3 dependency. Regards, Angelo
+ +#endif /* __CPR_H__ */
