[4/4] cpufreq: qcom-nvmem: Add MSM8909

Message ID 20230912-msm8909-cpufreq-v1-4-767ce66b544b@kernkonzept.com
State New
Headers
Series cpufreq: Add basic cpufreq scaling for Qualcomm MSM8909 |

Commit Message

Stephan Gerhold Sept. 12, 2023, 9:40 a.m. UTC
  When the MSM8909 SoC is used together with the PM8909 PMIC the primary
power supply for the CPU (VDD_APC) is shared with other components to
the SoC, namely the VDD_CX power domain typically supplied by the PM8909
S1 regulator. This means that all votes for necessary performance states
go via the RPM firmware which collects the requirements from all the
processors in the SoC. The RPM firmware then chooses the actual voltage
based on the performance states ("corners"), depending on calibration
values in the NVMEM and other factors.

The MSM8909 SoC is also sometimes used with the PM8916 or PM660 PMIC.
In that case there is a dedicated regulator connected to VDD_APC and
Linux is responsible to do adaptive voltage scaling using CPR (similar
to the existing code for QCS404).

This difference can be described in the device tree, by either assigning
the CPU a power domain from RPMPD or from the CPR driver.

To describe this in a more generic way, use "apc" as power domain name
instead of "cpr". From the Linux point of view there is no CPR involved
when MSM8909 is used together with PM8909.

Also add a simple function that reads the speedbin from a NVMEM cell
and sets it as-is for opp-supported-hw. The actual bit position can be
described in the device tree without additional driver changes.

Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>
---
 drivers/cpufreq/qcom-cpufreq-nvmem.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)
  

Comments

Konrad Dybcio Sept. 12, 2023, 9:59 a.m. UTC | #1
On 12.09.2023 11:40, Stephan Gerhold wrote:
> When the MSM8909 SoC is used together with the PM8909 PMIC the primary
> power supply for the CPU (VDD_APC) is shared with other components to
> the SoC, namely the VDD_CX power domain typically supplied by the PM8909
> S1 regulator. This means that all votes for necessary performance states
> go via the RPM firmware which collects the requirements from all the
> processors in the SoC. The RPM firmware then chooses the actual voltage
> based on the performance states ("corners"), depending on calibration
> values in the NVMEM and other factors.
> 
> The MSM8909 SoC is also sometimes used with the PM8916 or PM660 PMIC.
> In that case there is a dedicated regulator connected to VDD_APC and
> Linux is responsible to do adaptive voltage scaling using CPR (similar
> to the existing code for QCS404).
> 
> This difference can be described in the device tree, by either assigning
> the CPU a power domain from RPMPD or from the CPR driver.
> 
> To describe this in a more generic way, use "apc" as power domain name
> instead of "cpr". From the Linux point of view there is no CPR involved
> when MSM8909 is used together with PM8909.
Without checking, I have a vague recollection of CPR output also
being called VDD_APCx, so it's a-ok

> 
> Also add a simple function that reads the speedbin from a NVMEM cell
> and sets it as-is for opp-supported-hw. The actual bit position can be
> described in the device tree without additional driver changes.
> 
> Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>
> ---
Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org>

One nit below:

[...]

>  static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
> +	{ .compatible = "qcom,msm8909", .data = &match_data_msm8909 },
>  	{ .compatible = "qcom,apq8096", .data = &match_data_kryo },
msm8909 should come after apq8096 (even if adding apq and not msm was
a mistake)

Konrad
  
Stephan Gerhold Sept. 12, 2023, 11:08 a.m. UTC | #2
On Tue, Sep 12, 2023 at 11:59:47AM +0200, Konrad Dybcio wrote:
> On 12.09.2023 11:40, Stephan Gerhold wrote:
> > When the MSM8909 SoC is used together with the PM8909 PMIC the primary
> > power supply for the CPU (VDD_APC) is shared with other components to
> > the SoC, namely the VDD_CX power domain typically supplied by the PM8909
> > S1 regulator. This means that all votes for necessary performance states
> > go via the RPM firmware which collects the requirements from all the
> > processors in the SoC. The RPM firmware then chooses the actual voltage
> > based on the performance states ("corners"), depending on calibration
> > values in the NVMEM and other factors.
> > 
> > The MSM8909 SoC is also sometimes used with the PM8916 or PM660 PMIC.
> > In that case there is a dedicated regulator connected to VDD_APC and
> > Linux is responsible to do adaptive voltage scaling using CPR (similar
> > to the existing code for QCS404).
> > 
> > This difference can be described in the device tree, by either assigning
> > the CPU a power domain from RPMPD or from the CPR driver.
> > 
> > To describe this in a more generic way, use "apc" as power domain name
> > instead of "cpr". From the Linux point of view there is no CPR involved
> > when MSM8909 is used together with PM8909.
> Without checking, I have a vague recollection of CPR output also
> being called VDD_APCx, so it's a-ok
> 

FWIW: I would say there isn't really something like a "CPR output" in
the hardware. The power supply pin on the SoC for the cores is called
"VDD_APC" and you just have some regulator hooked up there. CPR then
monitors the environment and gives suggestions to adjust the voltage of
the regulator to optimize power and stability. But it doesn't provide
any power itself. You can use it or not. It's just an "add-in" basically.

> > 
> > Also add a simple function that reads the speedbin from a NVMEM cell
> > and sets it as-is for opp-supported-hw. The actual bit position can be
> > described in the device tree without additional driver changes.
> > 
> > Signed-off-by: Stephan Gerhold <stephan.gerhold@kernkonzept.com>
> > ---
> Acked-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> 
> One nit below:
> 
> [...]
> 
> >  static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
> > +	{ .compatible = "qcom,msm8909", .data = &match_data_msm8909 },
> >  	{ .compatible = "qcom,apq8096", .data = &match_data_kryo },
> msm8909 should come after apq8096 (even if adding apq and not msm was
> a mistake)
> 

Right, the list is currently totally out of order. I can prepend a patch
in v2 to sort it first. Will wait some more for more comments though.

Thanks!
  

Patch

diff --git a/drivers/cpufreq/qcom-cpufreq-nvmem.c b/drivers/cpufreq/qcom-cpufreq-nvmem.c
index 17d6ab14c909..2ea5e5ee9f1c 100644
--- a/drivers/cpufreq/qcom-cpufreq-nvmem.c
+++ b/drivers/cpufreq/qcom-cpufreq-nvmem.c
@@ -49,6 +49,24 @@  struct qcom_cpufreq_drv {
 
 static struct platform_device *cpufreq_dt_pdev, *cpufreq_pdev;
 
+static int qcom_cpufreq_simple_get_version(struct device *cpu_dev,
+					   struct nvmem_cell *speedbin_nvmem,
+					   char **pvs_name,
+					   struct qcom_cpufreq_drv *drv)
+{
+	u8 *speedbin;
+
+	*pvs_name = NULL;
+	speedbin = nvmem_cell_read(speedbin_nvmem, NULL);
+	if (IS_ERR(speedbin))
+		return PTR_ERR(speedbin);
+
+	dev_dbg(cpu_dev, "speedbin: %d\n", *speedbin);
+	drv->versions = 1 << *speedbin;
+	kfree(speedbin);
+	return 0;
+}
+
 static void get_krait_bin_format_a(struct device *cpu_dev,
 					  int *speed, int *pvs, int *pvs_ver,
 					  u8 *buf)
@@ -212,6 +230,13 @@  static const struct qcom_cpufreq_match_data match_data_krait = {
 	.get_version = qcom_cpufreq_krait_name_version,
 };
 
+static const char *msm8909_genpd_names[] = { "apc", NULL };
+
+static const struct qcom_cpufreq_match_data match_data_msm8909 = {
+	.get_version = qcom_cpufreq_simple_get_version,
+	.genpd_names = msm8909_genpd_names,
+};
+
 static const char *qcs404_genpd_names[] = { "cpr", NULL };
 
 static const struct qcom_cpufreq_match_data match_data_qcs404 = {
@@ -375,6 +400,7 @@  static struct platform_driver qcom_cpufreq_driver = {
 };
 
 static const struct of_device_id qcom_cpufreq_match_list[] __initconst = {
+	{ .compatible = "qcom,msm8909", .data = &match_data_msm8909 },
 	{ .compatible = "qcom,apq8096", .data = &match_data_kryo },
 	{ .compatible = "qcom,msm8996", .data = &match_data_kryo },
 	{ .compatible = "qcom,qcs404", .data = &match_data_qcs404 },