* [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver
@ 2020-04-16 8:58 Stephan Gerhold
2020-04-20 14:58 ` Lina Iyer
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Stephan Gerhold @ 2020-04-16 8:58 UTC (permalink / raw)
To: linux-pm
Cc: Rafael J. Wysocki, Daniel Lezcano, Andy Gross, Bjorn Andersson,
linux-arm-msm, Lorenzo Pieralisi, Ulf Hansson, Lina Iyer,
~postmarketos/upstreaming, Luca Weiss, Stephan Gerhold
The Qualcomm SPM cpuidle driver seems to be the last driver still
using the generic ARM CPUidle infrastructure.
Converting it actually allows us to simplify the driver,
and we end up being able to remove more lines than adding new ones:
- We can parse the CPUidle states in the device tree directly
with dt_idle_states (and don't need to duplicate that
functionality into the spm driver).
- Each "saw" device managed by the SPM driver now directly
registers its own cpuidle driver, removing the need for
any global (per cpu) state.
The device tree binding is the same, so the driver stays
compatible with all old device trees.
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
---
Related change for the PSCI cpuidle driver:
https://lore.kernel.org/linux-pm/cover.1565348376.git.lorenzo.pieralisi@arm.com/
(converting the QCOM SPM driver was mentioned there)
Changes in v2:
- Rebase on top of linux-next, fix conflicts
(Apparently sending shortly before the end of the merge window
was a bad idea... ;) )
v1: https://lore.kernel.org/linux-arm-msm/20200405162052.53622-1-stephan@gerhold.net/
---
MAINTAINERS | 1 +
drivers/cpuidle/Kconfig.arm | 13 ++
drivers/cpuidle/Makefile | 1 +
.../qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} | 138 +++++++-----------
drivers/soc/qcom/Kconfig | 10 --
drivers/soc/qcom/Makefile | 1 -
6 files changed, 67 insertions(+), 97 deletions(-)
rename drivers/{soc/qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} (75%)
diff --git a/MAINTAINERS b/MAINTAINERS
index e64e5db31497..2fd05a6835a6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2223,6 +2223,7 @@ F: drivers/*/qcom*
F: drivers/*/qcom/
F: drivers/bluetooth/btqcomsmd.c
F: drivers/clocksource/timer-qcom.c
+F: drivers/cpuidle/cpuidle-qcom-spm.c
F: drivers/extcon/extcon-qcom*
F: drivers/i2c/busses/i2c-qcom-geni.c
F: drivers/i2c/busses/i2c-qup.c
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 99a2d72ac02b..51a7e89085c0 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -94,3 +94,16 @@ config ARM_TEGRA_CPUIDLE
select ARM_CPU_SUSPEND
help
Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.
+
+config ARM_QCOM_SPM_CPUIDLE
+ bool "CPU Idle Driver for Qualcomm Subsystem Power Manager (SPM)"
+ depends on (ARCH_QCOM || COMPILE_TEST) && !ARM64
+ select ARM_CPU_SUSPEND
+ select CPU_IDLE_MULTIPLE_DRIVERS
+ select DT_IDLE_STATES
+ select QCOM_SCM
+ help
+ Select this to enable cpuidle for Qualcomm processors.
+ The Subsystem Power Manager (SPM) controls low power modes for the
+ CPU and L2 cores. It interface with various system drivers to put
+ the cores in low power modes.
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 55a464f6a78b..f07800cbb43f 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -25,6 +25,7 @@ obj-$(CONFIG_ARM_PSCI_CPUIDLE) += cpuidle_psci.o
cpuidle_psci-y := cpuidle-psci.o
cpuidle_psci-$(CONFIG_PM_GENERIC_DOMAINS_OF) += cpuidle-psci-domain.o
obj-$(CONFIG_ARM_TEGRA_CPUIDLE) += cpuidle-tegra.o
+obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE) += cpuidle-qcom-spm.o
###############################################################################
# MIPS drivers
diff --git a/drivers/soc/qcom/spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
similarity index 75%
rename from drivers/soc/qcom/spm.c
rename to drivers/cpuidle/cpuidle-qcom-spm.c
index 8e10e02c6aa5..adf91a6e4d7d 100644
--- a/drivers/soc/qcom/spm.c
+++ b/drivers/cpuidle/cpuidle-qcom-spm.c
@@ -19,10 +19,11 @@
#include <linux/cpu_pm.h>
#include <linux/qcom_scm.h>
-#include <asm/cpuidle.h>
#include <asm/proc-fns.h>
#include <asm/suspend.h>
+#include "dt_idle_states.h"
+
#define MAX_PMIC_DATA 2
#define MAX_SEQ_DATA 64
#define SPM_CTL_INDEX 0x7f
@@ -62,6 +63,7 @@ struct spm_reg_data {
};
struct spm_driver_data {
+ struct cpuidle_driver cpuidle_driver;
void __iomem *reg_base;
const struct spm_reg_data *reg_data;
};
@@ -107,11 +109,6 @@ static const struct spm_reg_data spm_reg_8064_cpu = {
.start_index[PM_SLEEP_MODE_SPC] = 2,
};
-static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv);
-
-typedef int (*idle_fn)(void);
-static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
-
static inline void spm_register_write(struct spm_driver_data *drv,
enum spm_reg reg, u32 val)
{
@@ -172,10 +169,9 @@ static int qcom_pm_collapse(unsigned long int unused)
return -1;
}
-static int qcom_cpu_spc(void)
+static int qcom_cpu_spc(struct spm_driver_data *drv)
{
int ret;
- struct spm_driver_data *drv = __this_cpu_read(cpu_spm_drv);
spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC);
ret = cpu_suspend(0, qcom_pm_collapse);
@@ -190,94 +186,49 @@ static int qcom_cpu_spc(void)
return ret;
}
-static int qcom_idle_enter(unsigned long index)
+static int spm_enter_idle_state(struct cpuidle_device *dev,
+ struct cpuidle_driver *drv, int idx)
{
- return __this_cpu_read(qcom_idle_ops)[index]();
+ struct spm_driver_data *data = container_of(drv, struct spm_driver_data,
+ cpuidle_driver);
+
+ return CPU_PM_CPU_IDLE_ENTER_PARAM(qcom_cpu_spc, idx, data);
}
-static const struct of_device_id qcom_idle_state_match[] __initconst = {
- { .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
+static struct cpuidle_driver qcom_spm_idle_driver = {
+ .name = "qcom_spm",
+ .owner = THIS_MODULE,
+ .states[0] = {
+ .enter = spm_enter_idle_state,
+ .exit_latency = 1,
+ .target_residency = 1,
+ .power_usage = UINT_MAX,
+ .name = "WFI",
+ .desc = "ARM WFI",
+ }
+};
+
+static const struct of_device_id qcom_idle_state_match[] = {
+ { .compatible = "qcom,idle-state-spc", .data = spm_enter_idle_state },
{ },
};
-static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu)
+static int spm_cpuidle_init(struct cpuidle_driver *drv, int cpu)
{
- const struct of_device_id *match_id;
- struct device_node *state_node;
- int i;
- int state_count = 1;
- idle_fn idle_fns[CPUIDLE_STATE_MAX];
- idle_fn *fns;
- cpumask_t mask;
- bool use_scm_power_down = false;
-
- if (!qcom_scm_is_available())
- return -EPROBE_DEFER;
-
- for (i = 0; ; i++) {
- state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
- if (!state_node)
- break;
-
- if (!of_device_is_available(state_node))
- continue;
-
- if (i == CPUIDLE_STATE_MAX) {
- pr_warn("%s: cpuidle states reached max possible\n",
- __func__);
- break;
- }
-
- match_id = of_match_node(qcom_idle_state_match, state_node);
- if (!match_id)
- return -ENODEV;
-
- idle_fns[state_count] = match_id->data;
-
- /* Check if any of the states allow power down */
- if (match_id->data == qcom_cpu_spc)
- use_scm_power_down = true;
-
- state_count++;
- }
-
- if (state_count == 1)
- goto check_spm;
-
- fns = devm_kcalloc(get_cpu_device(cpu), state_count, sizeof(*fns),
- GFP_KERNEL);
- if (!fns)
- return -ENOMEM;
-
- for (i = 1; i < state_count; i++)
- fns[i] = idle_fns[i];
+ int ret;
- if (use_scm_power_down) {
- /* We have atleast one power down mode */
- cpumask_clear(&mask);
- cpumask_set_cpu(cpu, &mask);
- qcom_scm_set_warm_boot_addr(cpu_resume_arm, &mask);
- }
+ memcpy(drv, &qcom_spm_idle_driver, sizeof(*drv));
+ drv->cpumask = (struct cpumask *)cpumask_of(cpu);
- per_cpu(qcom_idle_ops, cpu) = fns;
+ /* Parse idle states from device tree */
+ ret = dt_init_idle_driver(drv, qcom_idle_state_match, 1);
+ if (ret <= 0)
+ return ret ? : -ENODEV;
- /*
- * SPM probe for the cpu should have happened by now, if the
- * SPM device does not exist, return -ENXIO to indicate that the
- * cpu does not support idle states.
- */
-check_spm:
- return per_cpu(cpu_spm_drv, cpu) ? 0 : -ENXIO;
+ /* We have atleast one power down mode */
+ return qcom_scm_set_warm_boot_addr(cpu_resume_arm, drv->cpumask);
}
-static const struct cpuidle_ops qcom_cpuidle_ops __initconst = {
- .suspend = qcom_idle_enter,
- .init = qcom_cpuidle_init,
-};
-
-CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops);
-CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops);
-
static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
int *spm_cpu)
{
@@ -323,11 +274,15 @@ static int spm_dev_probe(struct platform_device *pdev)
struct resource *res;
const struct of_device_id *match_id;
void __iomem *addr;
- int cpu;
+ int cpu, ret;
+
+ if (!qcom_scm_is_available())
+ return -EPROBE_DEFER;
drv = spm_get_drv(pdev, &cpu);
if (!drv)
return -EINVAL;
+ platform_set_drvdata(pdev, drv);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
drv->reg_base = devm_ioremap_resource(&pdev->dev, res);
@@ -340,6 +295,10 @@ static int spm_dev_probe(struct platform_device *pdev)
drv->reg_data = match_id->data;
+ ret = spm_cpuidle_init(&drv->cpuidle_driver, cpu);
+ if (ret)
+ return ret;
+
/* Write the SPM sequences first.. */
addr = drv->reg_base + drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
__iowrite32_copy(addr, drv->reg_data->seq,
@@ -362,13 +321,20 @@ static int spm_dev_probe(struct platform_device *pdev)
/* Set up Standby as the default low power mode */
spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY);
- per_cpu(cpu_spm_drv, cpu) = drv;
+ return cpuidle_register(&drv->cpuidle_driver, NULL);
+}
+
+static int spm_dev_remove(struct platform_device *pdev)
+{
+ struct spm_driver_data *drv = platform_get_drvdata(pdev);
+ cpuidle_unregister(&drv->cpuidle_driver);
return 0;
}
static struct platform_driver spm_driver = {
.probe = spm_dev_probe,
+ .remove = spm_dev_remove,
.driver = {
.name = "saw",
.of_match_table = spm_match_table,
diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
index bf42a17a45de..285baa7e474e 100644
--- a/drivers/soc/qcom/Kconfig
+++ b/drivers/soc/qcom/Kconfig
@@ -80,16 +80,6 @@ config QCOM_PDR_HELPERS
tristate
select QCOM_QMI_HELPERS
-config QCOM_PM
- bool "Qualcomm Power Management"
- depends on ARCH_QCOM && !ARM64
- select ARM_CPU_SUSPEND
- select QCOM_SCM
- help
- QCOM Platform specific power driver to manage cores and L2 low power
- modes. It interface with various system drivers to put the cores in
- low power modes.
-
config QCOM_QMI_HELPERS
tristate
depends on NET
diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
index 5d6b83dc58e8..92cc4232d72c 100644
--- a/drivers/soc/qcom/Makefile
+++ b/drivers/soc/qcom/Makefile
@@ -8,7 +8,6 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
-obj-$(CONFIG_QCOM_PM) += spm.o
obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
qmi_helpers-y += qmi_encdec.o qmi_interface.o
obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o
--
2.26.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver
2020-04-16 8:58 [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver Stephan Gerhold
@ 2020-04-20 14:58 ` Lina Iyer
2020-04-20 19:10 ` Ulf Hansson
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Lina Iyer @ 2020-04-20 14:58 UTC (permalink / raw)
To: Stephan Gerhold
Cc: linux-pm, Rafael J. Wysocki, Daniel Lezcano, Andy Gross,
Bjorn Andersson, linux-arm-msm, Lorenzo Pieralisi, Ulf Hansson,
~postmarketos/upstreaming, Luca Weiss
On Thu, Apr 16 2020 at 03:02 -0600, Stephan Gerhold wrote:
>The Qualcomm SPM cpuidle driver seems to be the last driver still
>using the generic ARM CPUidle infrastructure.
>
>Converting it actually allows us to simplify the driver,
>and we end up being able to remove more lines than adding new ones:
>
> - We can parse the CPUidle states in the device tree directly
> with dt_idle_states (and don't need to duplicate that
> functionality into the spm driver).
>
> - Each "saw" device managed by the SPM driver now directly
> registers its own cpuidle driver, removing the need for
> any global (per cpu) state.
>
>The device tree binding is the same, so the driver stays
>compatible with all old device trees.
>
>Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Thanks for doing this. I don't see any obvious issues with the patch.
Reviewed-by: Lina Iyer <ilina@codeaurora.org>
>---
>Related change for the PSCI cpuidle driver:
> https://lore.kernel.org/linux-pm/cover.1565348376.git.lorenzo.pieralisi@arm.com/
>(converting the QCOM SPM driver was mentioned there)
>
>Changes in v2:
> - Rebase on top of linux-next, fix conflicts
> (Apparently sending shortly before the end of the merge window
> was a bad idea... ;) )
>
>v1: https://lore.kernel.org/linux-arm-msm/20200405162052.53622-1-stephan@gerhold.net/
>---
> MAINTAINERS | 1 +
> drivers/cpuidle/Kconfig.arm | 13 ++
> drivers/cpuidle/Makefile | 1 +
> .../qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} | 138 +++++++-----------
> drivers/soc/qcom/Kconfig | 10 --
> drivers/soc/qcom/Makefile | 1 -
> 6 files changed, 67 insertions(+), 97 deletions(-)
> rename drivers/{soc/qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} (75%)
>
>diff --git a/MAINTAINERS b/MAINTAINERS
>index e64e5db31497..2fd05a6835a6 100644
>--- a/MAINTAINERS
>+++ b/MAINTAINERS
>@@ -2223,6 +2223,7 @@ F: drivers/*/qcom*
> F: drivers/*/qcom/
> F: drivers/bluetooth/btqcomsmd.c
> F: drivers/clocksource/timer-qcom.c
>+F: drivers/cpuidle/cpuidle-qcom-spm.c
> F: drivers/extcon/extcon-qcom*
> F: drivers/i2c/busses/i2c-qcom-geni.c
> F: drivers/i2c/busses/i2c-qup.c
>diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
>index 99a2d72ac02b..51a7e89085c0 100644
>--- a/drivers/cpuidle/Kconfig.arm
>+++ b/drivers/cpuidle/Kconfig.arm
>@@ -94,3 +94,16 @@ config ARM_TEGRA_CPUIDLE
> select ARM_CPU_SUSPEND
> help
> Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.
>+
>+config ARM_QCOM_SPM_CPUIDLE
>+ bool "CPU Idle Driver for Qualcomm Subsystem Power Manager (SPM)"
>+ depends on (ARCH_QCOM || COMPILE_TEST) && !ARM64
>+ select ARM_CPU_SUSPEND
>+ select CPU_IDLE_MULTIPLE_DRIVERS
>+ select DT_IDLE_STATES
>+ select QCOM_SCM
>+ help
>+ Select this to enable cpuidle for Qualcomm processors.
>+ The Subsystem Power Manager (SPM) controls low power modes for the
>+ CPU and L2 cores. It interface with various system drivers to put
>+ the cores in low power modes.
>diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
>index 55a464f6a78b..f07800cbb43f 100644
>--- a/drivers/cpuidle/Makefile
>+++ b/drivers/cpuidle/Makefile
>@@ -25,6 +25,7 @@ obj-$(CONFIG_ARM_PSCI_CPUIDLE) += cpuidle_psci.o
> cpuidle_psci-y := cpuidle-psci.o
> cpuidle_psci-$(CONFIG_PM_GENERIC_DOMAINS_OF) += cpuidle-psci-domain.o
> obj-$(CONFIG_ARM_TEGRA_CPUIDLE) += cpuidle-tegra.o
>+obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE) += cpuidle-qcom-spm.o
>
> ###############################################################################
> # MIPS drivers
>diff --git a/drivers/soc/qcom/spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
>similarity index 75%
>rename from drivers/soc/qcom/spm.c
>rename to drivers/cpuidle/cpuidle-qcom-spm.c
>index 8e10e02c6aa5..adf91a6e4d7d 100644
>--- a/drivers/soc/qcom/spm.c
>+++ b/drivers/cpuidle/cpuidle-qcom-spm.c
>@@ -19,10 +19,11 @@
> #include <linux/cpu_pm.h>
> #include <linux/qcom_scm.h>
>
>-#include <asm/cpuidle.h>
> #include <asm/proc-fns.h>
> #include <asm/suspend.h>
>
>+#include "dt_idle_states.h"
>+
> #define MAX_PMIC_DATA 2
> #define MAX_SEQ_DATA 64
> #define SPM_CTL_INDEX 0x7f
>@@ -62,6 +63,7 @@ struct spm_reg_data {
> };
>
> struct spm_driver_data {
>+ struct cpuidle_driver cpuidle_driver;
> void __iomem *reg_base;
> const struct spm_reg_data *reg_data;
> };
>@@ -107,11 +109,6 @@ static const struct spm_reg_data spm_reg_8064_cpu = {
> .start_index[PM_SLEEP_MODE_SPC] = 2,
> };
>
>-static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv);
>-
>-typedef int (*idle_fn)(void);
>-static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
>-
> static inline void spm_register_write(struct spm_driver_data *drv,
> enum spm_reg reg, u32 val)
> {
>@@ -172,10 +169,9 @@ static int qcom_pm_collapse(unsigned long int unused)
> return -1;
> }
>
>-static int qcom_cpu_spc(void)
>+static int qcom_cpu_spc(struct spm_driver_data *drv)
> {
> int ret;
>- struct spm_driver_data *drv = __this_cpu_read(cpu_spm_drv);
>
> spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC);
> ret = cpu_suspend(0, qcom_pm_collapse);
>@@ -190,94 +186,49 @@ static int qcom_cpu_spc(void)
> return ret;
> }
>
>-static int qcom_idle_enter(unsigned long index)
>+static int spm_enter_idle_state(struct cpuidle_device *dev,
>+ struct cpuidle_driver *drv, int idx)
> {
>- return __this_cpu_read(qcom_idle_ops)[index]();
>+ struct spm_driver_data *data = container_of(drv, struct spm_driver_data,
>+ cpuidle_driver);
>+
>+ return CPU_PM_CPU_IDLE_ENTER_PARAM(qcom_cpu_spc, idx, data);
> }
>
>-static const struct of_device_id qcom_idle_state_match[] __initconst = {
>- { .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
>+static struct cpuidle_driver qcom_spm_idle_driver = {
>+ .name = "qcom_spm",
>+ .owner = THIS_MODULE,
>+ .states[0] = {
>+ .enter = spm_enter_idle_state,
>+ .exit_latency = 1,
>+ .target_residency = 1,
>+ .power_usage = UINT_MAX,
>+ .name = "WFI",
>+ .desc = "ARM WFI",
>+ }
>+};
>+
>+static const struct of_device_id qcom_idle_state_match[] = {
>+ { .compatible = "qcom,idle-state-spc", .data = spm_enter_idle_state },
> { },
> };
>
>-static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu)
>+static int spm_cpuidle_init(struct cpuidle_driver *drv, int cpu)
> {
>- const struct of_device_id *match_id;
>- struct device_node *state_node;
>- int i;
>- int state_count = 1;
>- idle_fn idle_fns[CPUIDLE_STATE_MAX];
>- idle_fn *fns;
>- cpumask_t mask;
>- bool use_scm_power_down = false;
>-
>- if (!qcom_scm_is_available())
>- return -EPROBE_DEFER;
>-
>- for (i = 0; ; i++) {
>- state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
>- if (!state_node)
>- break;
>-
>- if (!of_device_is_available(state_node))
>- continue;
>-
>- if (i == CPUIDLE_STATE_MAX) {
>- pr_warn("%s: cpuidle states reached max possible\n",
>- __func__);
>- break;
>- }
>-
>- match_id = of_match_node(qcom_idle_state_match, state_node);
>- if (!match_id)
>- return -ENODEV;
>-
>- idle_fns[state_count] = match_id->data;
>-
>- /* Check if any of the states allow power down */
>- if (match_id->data == qcom_cpu_spc)
>- use_scm_power_down = true;
>-
>- state_count++;
>- }
>-
>- if (state_count == 1)
>- goto check_spm;
>-
>- fns = devm_kcalloc(get_cpu_device(cpu), state_count, sizeof(*fns),
>- GFP_KERNEL);
>- if (!fns)
>- return -ENOMEM;
>-
>- for (i = 1; i < state_count; i++)
>- fns[i] = idle_fns[i];
>+ int ret;
>
>- if (use_scm_power_down) {
>- /* We have atleast one power down mode */
>- cpumask_clear(&mask);
>- cpumask_set_cpu(cpu, &mask);
>- qcom_scm_set_warm_boot_addr(cpu_resume_arm, &mask);
>- }
>+ memcpy(drv, &qcom_spm_idle_driver, sizeof(*drv));
>+ drv->cpumask = (struct cpumask *)cpumask_of(cpu);
>
>- per_cpu(qcom_idle_ops, cpu) = fns;
>+ /* Parse idle states from device tree */
>+ ret = dt_init_idle_driver(drv, qcom_idle_state_match, 1);
>+ if (ret <= 0)
>+ return ret ? : -ENODEV;
>
>- /*
>- * SPM probe for the cpu should have happened by now, if the
>- * SPM device does not exist, return -ENXIO to indicate that the
>- * cpu does not support idle states.
>- */
>-check_spm:
>- return per_cpu(cpu_spm_drv, cpu) ? 0 : -ENXIO;
>+ /* We have atleast one power down mode */
>+ return qcom_scm_set_warm_boot_addr(cpu_resume_arm, drv->cpumask);
> }
>
>-static const struct cpuidle_ops qcom_cpuidle_ops __initconst = {
>- .suspend = qcom_idle_enter,
>- .init = qcom_cpuidle_init,
>-};
>-
>-CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops);
>-CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops);
>-
> static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
> int *spm_cpu)
> {
>@@ -323,11 +274,15 @@ static int spm_dev_probe(struct platform_device *pdev)
> struct resource *res;
> const struct of_device_id *match_id;
> void __iomem *addr;
>- int cpu;
>+ int cpu, ret;
>+
>+ if (!qcom_scm_is_available())
>+ return -EPROBE_DEFER;
>
> drv = spm_get_drv(pdev, &cpu);
> if (!drv)
> return -EINVAL;
>+ platform_set_drvdata(pdev, drv);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> drv->reg_base = devm_ioremap_resource(&pdev->dev, res);
>@@ -340,6 +295,10 @@ static int spm_dev_probe(struct platform_device *pdev)
>
> drv->reg_data = match_id->data;
>
>+ ret = spm_cpuidle_init(&drv->cpuidle_driver, cpu);
>+ if (ret)
>+ return ret;
>+
> /* Write the SPM sequences first.. */
> addr = drv->reg_base + drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
> __iowrite32_copy(addr, drv->reg_data->seq,
>@@ -362,13 +321,20 @@ static int spm_dev_probe(struct platform_device *pdev)
> /* Set up Standby as the default low power mode */
> spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY);
>
>- per_cpu(cpu_spm_drv, cpu) = drv;
>+ return cpuidle_register(&drv->cpuidle_driver, NULL);
>+}
>+
>+static int spm_dev_remove(struct platform_device *pdev)
>+{
>+ struct spm_driver_data *drv = platform_get_drvdata(pdev);
>
>+ cpuidle_unregister(&drv->cpuidle_driver);
> return 0;
> }
>
> static struct platform_driver spm_driver = {
> .probe = spm_dev_probe,
>+ .remove = spm_dev_remove,
> .driver = {
> .name = "saw",
> .of_match_table = spm_match_table,
>diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
>index bf42a17a45de..285baa7e474e 100644
>--- a/drivers/soc/qcom/Kconfig
>+++ b/drivers/soc/qcom/Kconfig
>@@ -80,16 +80,6 @@ config QCOM_PDR_HELPERS
> tristate
> select QCOM_QMI_HELPERS
>
>-config QCOM_PM
>- bool "Qualcomm Power Management"
>- depends on ARCH_QCOM && !ARM64
>- select ARM_CPU_SUSPEND
>- select QCOM_SCM
>- help
>- QCOM Platform specific power driver to manage cores and L2 low power
>- modes. It interface with various system drivers to put the cores in
>- low power modes.
>-
> config QCOM_QMI_HELPERS
> tristate
> depends on NET
>diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
>index 5d6b83dc58e8..92cc4232d72c 100644
>--- a/drivers/soc/qcom/Makefile
>+++ b/drivers/soc/qcom/Makefile
>@@ -8,7 +8,6 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
> obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
> obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
>-obj-$(CONFIG_QCOM_PM) += spm.o
> obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
> qmi_helpers-y += qmi_encdec.o qmi_interface.o
> obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o
>--
>2.26.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver
2020-04-16 8:58 [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver Stephan Gerhold
2020-04-20 14:58 ` Lina Iyer
@ 2020-04-20 19:10 ` Ulf Hansson
2020-04-20 19:26 ` Bjorn Andersson
2020-05-26 8:24 ` Ulf Hansson
3 siblings, 0 replies; 6+ messages in thread
From: Ulf Hansson @ 2020-04-20 19:10 UTC (permalink / raw)
To: Stephan Gerhold
Cc: Linux PM, Rafael J. Wysocki, Daniel Lezcano, Andy Gross,
Bjorn Andersson, linux-arm-msm, Lorenzo Pieralisi, Lina Iyer,
~postmarketos/upstreaming, Luca Weiss
On Thu, 16 Apr 2020 at 10:59, Stephan Gerhold <stephan@gerhold.net> wrote:
>
> The Qualcomm SPM cpuidle driver seems to be the last driver still
> using the generic ARM CPUidle infrastructure.
>
> Converting it actually allows us to simplify the driver,
> and we end up being able to remove more lines than adding new ones:
>
> - We can parse the CPUidle states in the device tree directly
> with dt_idle_states (and don't need to duplicate that
> functionality into the spm driver).
>
> - Each "saw" device managed by the SPM driver now directly
> registers its own cpuidle driver, removing the need for
> any global (per cpu) state.
>
> The device tree binding is the same, so the driver stays
> compatible with all old device trees.
>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Excellent, thanks for doing this!
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> Related change for the PSCI cpuidle driver:
> https://lore.kernel.org/linux-pm/cover.1565348376.git.lorenzo.pieralisi@arm.com/
> (converting the QCOM SPM driver was mentioned there)
>
> Changes in v2:
> - Rebase on top of linux-next, fix conflicts
> (Apparently sending shortly before the end of the merge window
> was a bad idea... ;) )
>
> v1: https://lore.kernel.org/linux-arm-msm/20200405162052.53622-1-stephan@gerhold.net/
> ---
> MAINTAINERS | 1 +
> drivers/cpuidle/Kconfig.arm | 13 ++
> drivers/cpuidle/Makefile | 1 +
> .../qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} | 138 +++++++-----------
> drivers/soc/qcom/Kconfig | 10 --
> drivers/soc/qcom/Makefile | 1 -
> 6 files changed, 67 insertions(+), 97 deletions(-)
> rename drivers/{soc/qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} (75%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e64e5db31497..2fd05a6835a6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2223,6 +2223,7 @@ F: drivers/*/qcom*
> F: drivers/*/qcom/
> F: drivers/bluetooth/btqcomsmd.c
> F: drivers/clocksource/timer-qcom.c
> +F: drivers/cpuidle/cpuidle-qcom-spm.c
> F: drivers/extcon/extcon-qcom*
> F: drivers/i2c/busses/i2c-qcom-geni.c
> F: drivers/i2c/busses/i2c-qup.c
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 99a2d72ac02b..51a7e89085c0 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -94,3 +94,16 @@ config ARM_TEGRA_CPUIDLE
> select ARM_CPU_SUSPEND
> help
> Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.
> +
> +config ARM_QCOM_SPM_CPUIDLE
> + bool "CPU Idle Driver for Qualcomm Subsystem Power Manager (SPM)"
> + depends on (ARCH_QCOM || COMPILE_TEST) && !ARM64
> + select ARM_CPU_SUSPEND
> + select CPU_IDLE_MULTIPLE_DRIVERS
> + select DT_IDLE_STATES
> + select QCOM_SCM
> + help
> + Select this to enable cpuidle for Qualcomm processors.
> + The Subsystem Power Manager (SPM) controls low power modes for the
> + CPU and L2 cores. It interface with various system drivers to put
> + the cores in low power modes.
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 55a464f6a78b..f07800cbb43f 100644
> --- a/drivers/cpuidle/Makefile
> +++ b/drivers/cpuidle/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_ARM_PSCI_CPUIDLE) += cpuidle_psci.o
> cpuidle_psci-y := cpuidle-psci.o
> cpuidle_psci-$(CONFIG_PM_GENERIC_DOMAINS_OF) += cpuidle-psci-domain.o
> obj-$(CONFIG_ARM_TEGRA_CPUIDLE) += cpuidle-tegra.o
> +obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE) += cpuidle-qcom-spm.o
>
> ###############################################################################
> # MIPS drivers
> diff --git a/drivers/soc/qcom/spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> similarity index 75%
> rename from drivers/soc/qcom/spm.c
> rename to drivers/cpuidle/cpuidle-qcom-spm.c
> index 8e10e02c6aa5..adf91a6e4d7d 100644
> --- a/drivers/soc/qcom/spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -19,10 +19,11 @@
> #include <linux/cpu_pm.h>
> #include <linux/qcom_scm.h>
>
> -#include <asm/cpuidle.h>
> #include <asm/proc-fns.h>
> #include <asm/suspend.h>
>
> +#include "dt_idle_states.h"
> +
> #define MAX_PMIC_DATA 2
> #define MAX_SEQ_DATA 64
> #define SPM_CTL_INDEX 0x7f
> @@ -62,6 +63,7 @@ struct spm_reg_data {
> };
>
> struct spm_driver_data {
> + struct cpuidle_driver cpuidle_driver;
> void __iomem *reg_base;
> const struct spm_reg_data *reg_data;
> };
> @@ -107,11 +109,6 @@ static const struct spm_reg_data spm_reg_8064_cpu = {
> .start_index[PM_SLEEP_MODE_SPC] = 2,
> };
>
> -static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv);
> -
> -typedef int (*idle_fn)(void);
> -static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
> -
> static inline void spm_register_write(struct spm_driver_data *drv,
> enum spm_reg reg, u32 val)
> {
> @@ -172,10 +169,9 @@ static int qcom_pm_collapse(unsigned long int unused)
> return -1;
> }
>
> -static int qcom_cpu_spc(void)
> +static int qcom_cpu_spc(struct spm_driver_data *drv)
> {
> int ret;
> - struct spm_driver_data *drv = __this_cpu_read(cpu_spm_drv);
>
> spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC);
> ret = cpu_suspend(0, qcom_pm_collapse);
> @@ -190,94 +186,49 @@ static int qcom_cpu_spc(void)
> return ret;
> }
>
> -static int qcom_idle_enter(unsigned long index)
> +static int spm_enter_idle_state(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int idx)
> {
> - return __this_cpu_read(qcom_idle_ops)[index]();
> + struct spm_driver_data *data = container_of(drv, struct spm_driver_data,
> + cpuidle_driver);
> +
> + return CPU_PM_CPU_IDLE_ENTER_PARAM(qcom_cpu_spc, idx, data);
> }
>
> -static const struct of_device_id qcom_idle_state_match[] __initconst = {
> - { .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
> +static struct cpuidle_driver qcom_spm_idle_driver = {
> + .name = "qcom_spm",
> + .owner = THIS_MODULE,
> + .states[0] = {
> + .enter = spm_enter_idle_state,
> + .exit_latency = 1,
> + .target_residency = 1,
> + .power_usage = UINT_MAX,
> + .name = "WFI",
> + .desc = "ARM WFI",
> + }
> +};
> +
> +static const struct of_device_id qcom_idle_state_match[] = {
> + { .compatible = "qcom,idle-state-spc", .data = spm_enter_idle_state },
> { },
> };
>
> -static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu)
> +static int spm_cpuidle_init(struct cpuidle_driver *drv, int cpu)
> {
> - const struct of_device_id *match_id;
> - struct device_node *state_node;
> - int i;
> - int state_count = 1;
> - idle_fn idle_fns[CPUIDLE_STATE_MAX];
> - idle_fn *fns;
> - cpumask_t mask;
> - bool use_scm_power_down = false;
> -
> - if (!qcom_scm_is_available())
> - return -EPROBE_DEFER;
> -
> - for (i = 0; ; i++) {
> - state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
> - if (!state_node)
> - break;
> -
> - if (!of_device_is_available(state_node))
> - continue;
> -
> - if (i == CPUIDLE_STATE_MAX) {
> - pr_warn("%s: cpuidle states reached max possible\n",
> - __func__);
> - break;
> - }
> -
> - match_id = of_match_node(qcom_idle_state_match, state_node);
> - if (!match_id)
> - return -ENODEV;
> -
> - idle_fns[state_count] = match_id->data;
> -
> - /* Check if any of the states allow power down */
> - if (match_id->data == qcom_cpu_spc)
> - use_scm_power_down = true;
> -
> - state_count++;
> - }
> -
> - if (state_count == 1)
> - goto check_spm;
> -
> - fns = devm_kcalloc(get_cpu_device(cpu), state_count, sizeof(*fns),
> - GFP_KERNEL);
> - if (!fns)
> - return -ENOMEM;
> -
> - for (i = 1; i < state_count; i++)
> - fns[i] = idle_fns[i];
> + int ret;
>
> - if (use_scm_power_down) {
> - /* We have atleast one power down mode */
> - cpumask_clear(&mask);
> - cpumask_set_cpu(cpu, &mask);
> - qcom_scm_set_warm_boot_addr(cpu_resume_arm, &mask);
> - }
> + memcpy(drv, &qcom_spm_idle_driver, sizeof(*drv));
> + drv->cpumask = (struct cpumask *)cpumask_of(cpu);
>
> - per_cpu(qcom_idle_ops, cpu) = fns;
> + /* Parse idle states from device tree */
> + ret = dt_init_idle_driver(drv, qcom_idle_state_match, 1);
> + if (ret <= 0)
> + return ret ? : -ENODEV;
>
> - /*
> - * SPM probe for the cpu should have happened by now, if the
> - * SPM device does not exist, return -ENXIO to indicate that the
> - * cpu does not support idle states.
> - */
> -check_spm:
> - return per_cpu(cpu_spm_drv, cpu) ? 0 : -ENXIO;
> + /* We have atleast one power down mode */
> + return qcom_scm_set_warm_boot_addr(cpu_resume_arm, drv->cpumask);
> }
>
> -static const struct cpuidle_ops qcom_cpuidle_ops __initconst = {
> - .suspend = qcom_idle_enter,
> - .init = qcom_cpuidle_init,
> -};
> -
> -CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops);
> -CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops);
> -
> static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
> int *spm_cpu)
> {
> @@ -323,11 +274,15 @@ static int spm_dev_probe(struct platform_device *pdev)
> struct resource *res;
> const struct of_device_id *match_id;
> void __iomem *addr;
> - int cpu;
> + int cpu, ret;
> +
> + if (!qcom_scm_is_available())
> + return -EPROBE_DEFER;
>
> drv = spm_get_drv(pdev, &cpu);
> if (!drv)
> return -EINVAL;
> + platform_set_drvdata(pdev, drv);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> drv->reg_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -340,6 +295,10 @@ static int spm_dev_probe(struct platform_device *pdev)
>
> drv->reg_data = match_id->data;
>
> + ret = spm_cpuidle_init(&drv->cpuidle_driver, cpu);
> + if (ret)
> + return ret;
> +
> /* Write the SPM sequences first.. */
> addr = drv->reg_base + drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
> __iowrite32_copy(addr, drv->reg_data->seq,
> @@ -362,13 +321,20 @@ static int spm_dev_probe(struct platform_device *pdev)
> /* Set up Standby as the default low power mode */
> spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY);
>
> - per_cpu(cpu_spm_drv, cpu) = drv;
> + return cpuidle_register(&drv->cpuidle_driver, NULL);
> +}
> +
> +static int spm_dev_remove(struct platform_device *pdev)
> +{
> + struct spm_driver_data *drv = platform_get_drvdata(pdev);
>
> + cpuidle_unregister(&drv->cpuidle_driver);
> return 0;
> }
>
> static struct platform_driver spm_driver = {
> .probe = spm_dev_probe,
> + .remove = spm_dev_remove,
> .driver = {
> .name = "saw",
> .of_match_table = spm_match_table,
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index bf42a17a45de..285baa7e474e 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -80,16 +80,6 @@ config QCOM_PDR_HELPERS
> tristate
> select QCOM_QMI_HELPERS
>
> -config QCOM_PM
> - bool "Qualcomm Power Management"
> - depends on ARCH_QCOM && !ARM64
> - select ARM_CPU_SUSPEND
> - select QCOM_SCM
> - help
> - QCOM Platform specific power driver to manage cores and L2 low power
> - modes. It interface with various system drivers to put the cores in
> - low power modes.
> -
> config QCOM_QMI_HELPERS
> tristate
> depends on NET
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 5d6b83dc58e8..92cc4232d72c 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -8,7 +8,6 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
> obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
> obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
> -obj-$(CONFIG_QCOM_PM) += spm.o
> obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
> qmi_helpers-y += qmi_encdec.o qmi_interface.o
> obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o
> --
> 2.26.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver
2020-04-16 8:58 [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver Stephan Gerhold
2020-04-20 14:58 ` Lina Iyer
2020-04-20 19:10 ` Ulf Hansson
@ 2020-04-20 19:26 ` Bjorn Andersson
2020-05-26 8:24 ` Ulf Hansson
3 siblings, 0 replies; 6+ messages in thread
From: Bjorn Andersson @ 2020-04-20 19:26 UTC (permalink / raw)
To: Stephan Gerhold
Cc: linux-pm, Rafael J. Wysocki, Daniel Lezcano, Andy Gross,
linux-arm-msm, Lorenzo Pieralisi, Ulf Hansson, Lina Iyer,
~postmarketos/upstreaming, Luca Weiss
On Thu 16 Apr 01:58 PDT 2020, Stephan Gerhold wrote:
> The Qualcomm SPM cpuidle driver seems to be the last driver still
> using the generic ARM CPUidle infrastructure.
>
> Converting it actually allows us to simplify the driver,
> and we end up being able to remove more lines than adding new ones:
>
> - We can parse the CPUidle states in the device tree directly
> with dt_idle_states (and don't need to duplicate that
> functionality into the spm driver).
>
> - Each "saw" device managed by the SPM driver now directly
> registers its own cpuidle driver, removing the need for
> any global (per cpu) state.
>
> The device tree binding is the same, so the driver stays
> compatible with all old device trees.
>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Acked-by: Bjorn Andersson <bjorn.andersson@linaro.org>
Regards,
Bjorn
> ---
> Related change for the PSCI cpuidle driver:
> https://lore.kernel.org/linux-pm/cover.1565348376.git.lorenzo.pieralisi@arm.com/
> (converting the QCOM SPM driver was mentioned there)
>
> Changes in v2:
> - Rebase on top of linux-next, fix conflicts
> (Apparently sending shortly before the end of the merge window
> was a bad idea... ;) )
>
> v1: https://lore.kernel.org/linux-arm-msm/20200405162052.53622-1-stephan@gerhold.net/
> ---
> MAINTAINERS | 1 +
> drivers/cpuidle/Kconfig.arm | 13 ++
> drivers/cpuidle/Makefile | 1 +
> .../qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} | 138 +++++++-----------
> drivers/soc/qcom/Kconfig | 10 --
> drivers/soc/qcom/Makefile | 1 -
> 6 files changed, 67 insertions(+), 97 deletions(-)
> rename drivers/{soc/qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} (75%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e64e5db31497..2fd05a6835a6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2223,6 +2223,7 @@ F: drivers/*/qcom*
> F: drivers/*/qcom/
> F: drivers/bluetooth/btqcomsmd.c
> F: drivers/clocksource/timer-qcom.c
> +F: drivers/cpuidle/cpuidle-qcom-spm.c
> F: drivers/extcon/extcon-qcom*
> F: drivers/i2c/busses/i2c-qcom-geni.c
> F: drivers/i2c/busses/i2c-qup.c
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 99a2d72ac02b..51a7e89085c0 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -94,3 +94,16 @@ config ARM_TEGRA_CPUIDLE
> select ARM_CPU_SUSPEND
> help
> Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.
> +
> +config ARM_QCOM_SPM_CPUIDLE
> + bool "CPU Idle Driver for Qualcomm Subsystem Power Manager (SPM)"
> + depends on (ARCH_QCOM || COMPILE_TEST) && !ARM64
> + select ARM_CPU_SUSPEND
> + select CPU_IDLE_MULTIPLE_DRIVERS
> + select DT_IDLE_STATES
> + select QCOM_SCM
> + help
> + Select this to enable cpuidle for Qualcomm processors.
> + The Subsystem Power Manager (SPM) controls low power modes for the
> + CPU and L2 cores. It interface with various system drivers to put
> + the cores in low power modes.
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 55a464f6a78b..f07800cbb43f 100644
> --- a/drivers/cpuidle/Makefile
> +++ b/drivers/cpuidle/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_ARM_PSCI_CPUIDLE) += cpuidle_psci.o
> cpuidle_psci-y := cpuidle-psci.o
> cpuidle_psci-$(CONFIG_PM_GENERIC_DOMAINS_OF) += cpuidle-psci-domain.o
> obj-$(CONFIG_ARM_TEGRA_CPUIDLE) += cpuidle-tegra.o
> +obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE) += cpuidle-qcom-spm.o
>
> ###############################################################################
> # MIPS drivers
> diff --git a/drivers/soc/qcom/spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> similarity index 75%
> rename from drivers/soc/qcom/spm.c
> rename to drivers/cpuidle/cpuidle-qcom-spm.c
> index 8e10e02c6aa5..adf91a6e4d7d 100644
> --- a/drivers/soc/qcom/spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -19,10 +19,11 @@
> #include <linux/cpu_pm.h>
> #include <linux/qcom_scm.h>
>
> -#include <asm/cpuidle.h>
> #include <asm/proc-fns.h>
> #include <asm/suspend.h>
>
> +#include "dt_idle_states.h"
> +
> #define MAX_PMIC_DATA 2
> #define MAX_SEQ_DATA 64
> #define SPM_CTL_INDEX 0x7f
> @@ -62,6 +63,7 @@ struct spm_reg_data {
> };
>
> struct spm_driver_data {
> + struct cpuidle_driver cpuidle_driver;
> void __iomem *reg_base;
> const struct spm_reg_data *reg_data;
> };
> @@ -107,11 +109,6 @@ static const struct spm_reg_data spm_reg_8064_cpu = {
> .start_index[PM_SLEEP_MODE_SPC] = 2,
> };
>
> -static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv);
> -
> -typedef int (*idle_fn)(void);
> -static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
> -
> static inline void spm_register_write(struct spm_driver_data *drv,
> enum spm_reg reg, u32 val)
> {
> @@ -172,10 +169,9 @@ static int qcom_pm_collapse(unsigned long int unused)
> return -1;
> }
>
> -static int qcom_cpu_spc(void)
> +static int qcom_cpu_spc(struct spm_driver_data *drv)
> {
> int ret;
> - struct spm_driver_data *drv = __this_cpu_read(cpu_spm_drv);
>
> spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC);
> ret = cpu_suspend(0, qcom_pm_collapse);
> @@ -190,94 +186,49 @@ static int qcom_cpu_spc(void)
> return ret;
> }
>
> -static int qcom_idle_enter(unsigned long index)
> +static int spm_enter_idle_state(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int idx)
> {
> - return __this_cpu_read(qcom_idle_ops)[index]();
> + struct spm_driver_data *data = container_of(drv, struct spm_driver_data,
> + cpuidle_driver);
> +
> + return CPU_PM_CPU_IDLE_ENTER_PARAM(qcom_cpu_spc, idx, data);
> }
>
> -static const struct of_device_id qcom_idle_state_match[] __initconst = {
> - { .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
> +static struct cpuidle_driver qcom_spm_idle_driver = {
> + .name = "qcom_spm",
> + .owner = THIS_MODULE,
> + .states[0] = {
> + .enter = spm_enter_idle_state,
> + .exit_latency = 1,
> + .target_residency = 1,
> + .power_usage = UINT_MAX,
> + .name = "WFI",
> + .desc = "ARM WFI",
> + }
> +};
> +
> +static const struct of_device_id qcom_idle_state_match[] = {
> + { .compatible = "qcom,idle-state-spc", .data = spm_enter_idle_state },
> { },
> };
>
> -static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu)
> +static int spm_cpuidle_init(struct cpuidle_driver *drv, int cpu)
> {
> - const struct of_device_id *match_id;
> - struct device_node *state_node;
> - int i;
> - int state_count = 1;
> - idle_fn idle_fns[CPUIDLE_STATE_MAX];
> - idle_fn *fns;
> - cpumask_t mask;
> - bool use_scm_power_down = false;
> -
> - if (!qcom_scm_is_available())
> - return -EPROBE_DEFER;
> -
> - for (i = 0; ; i++) {
> - state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
> - if (!state_node)
> - break;
> -
> - if (!of_device_is_available(state_node))
> - continue;
> -
> - if (i == CPUIDLE_STATE_MAX) {
> - pr_warn("%s: cpuidle states reached max possible\n",
> - __func__);
> - break;
> - }
> -
> - match_id = of_match_node(qcom_idle_state_match, state_node);
> - if (!match_id)
> - return -ENODEV;
> -
> - idle_fns[state_count] = match_id->data;
> -
> - /* Check if any of the states allow power down */
> - if (match_id->data == qcom_cpu_spc)
> - use_scm_power_down = true;
> -
> - state_count++;
> - }
> -
> - if (state_count == 1)
> - goto check_spm;
> -
> - fns = devm_kcalloc(get_cpu_device(cpu), state_count, sizeof(*fns),
> - GFP_KERNEL);
> - if (!fns)
> - return -ENOMEM;
> -
> - for (i = 1; i < state_count; i++)
> - fns[i] = idle_fns[i];
> + int ret;
>
> - if (use_scm_power_down) {
> - /* We have atleast one power down mode */
> - cpumask_clear(&mask);
> - cpumask_set_cpu(cpu, &mask);
> - qcom_scm_set_warm_boot_addr(cpu_resume_arm, &mask);
> - }
> + memcpy(drv, &qcom_spm_idle_driver, sizeof(*drv));
> + drv->cpumask = (struct cpumask *)cpumask_of(cpu);
>
> - per_cpu(qcom_idle_ops, cpu) = fns;
> + /* Parse idle states from device tree */
> + ret = dt_init_idle_driver(drv, qcom_idle_state_match, 1);
> + if (ret <= 0)
> + return ret ? : -ENODEV;
>
> - /*
> - * SPM probe for the cpu should have happened by now, if the
> - * SPM device does not exist, return -ENXIO to indicate that the
> - * cpu does not support idle states.
> - */
> -check_spm:
> - return per_cpu(cpu_spm_drv, cpu) ? 0 : -ENXIO;
> + /* We have atleast one power down mode */
> + return qcom_scm_set_warm_boot_addr(cpu_resume_arm, drv->cpumask);
> }
>
> -static const struct cpuidle_ops qcom_cpuidle_ops __initconst = {
> - .suspend = qcom_idle_enter,
> - .init = qcom_cpuidle_init,
> -};
> -
> -CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops);
> -CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops);
> -
> static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
> int *spm_cpu)
> {
> @@ -323,11 +274,15 @@ static int spm_dev_probe(struct platform_device *pdev)
> struct resource *res;
> const struct of_device_id *match_id;
> void __iomem *addr;
> - int cpu;
> + int cpu, ret;
> +
> + if (!qcom_scm_is_available())
> + return -EPROBE_DEFER;
>
> drv = spm_get_drv(pdev, &cpu);
> if (!drv)
> return -EINVAL;
> + platform_set_drvdata(pdev, drv);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> drv->reg_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -340,6 +295,10 @@ static int spm_dev_probe(struct platform_device *pdev)
>
> drv->reg_data = match_id->data;
>
> + ret = spm_cpuidle_init(&drv->cpuidle_driver, cpu);
> + if (ret)
> + return ret;
> +
> /* Write the SPM sequences first.. */
> addr = drv->reg_base + drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
> __iowrite32_copy(addr, drv->reg_data->seq,
> @@ -362,13 +321,20 @@ static int spm_dev_probe(struct platform_device *pdev)
> /* Set up Standby as the default low power mode */
> spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY);
>
> - per_cpu(cpu_spm_drv, cpu) = drv;
> + return cpuidle_register(&drv->cpuidle_driver, NULL);
> +}
> +
> +static int spm_dev_remove(struct platform_device *pdev)
> +{
> + struct spm_driver_data *drv = platform_get_drvdata(pdev);
>
> + cpuidle_unregister(&drv->cpuidle_driver);
> return 0;
> }
>
> static struct platform_driver spm_driver = {
> .probe = spm_dev_probe,
> + .remove = spm_dev_remove,
> .driver = {
> .name = "saw",
> .of_match_table = spm_match_table,
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index bf42a17a45de..285baa7e474e 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -80,16 +80,6 @@ config QCOM_PDR_HELPERS
> tristate
> select QCOM_QMI_HELPERS
>
> -config QCOM_PM
> - bool "Qualcomm Power Management"
> - depends on ARCH_QCOM && !ARM64
> - select ARM_CPU_SUSPEND
> - select QCOM_SCM
> - help
> - QCOM Platform specific power driver to manage cores and L2 low power
> - modes. It interface with various system drivers to put the cores in
> - low power modes.
> -
> config QCOM_QMI_HELPERS
> tristate
> depends on NET
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 5d6b83dc58e8..92cc4232d72c 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -8,7 +8,6 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
> obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
> obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
> -obj-$(CONFIG_QCOM_PM) += spm.o
> obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
> qmi_helpers-y += qmi_encdec.o qmi_interface.o
> obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o
> --
> 2.26.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver
2020-04-16 8:58 [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver Stephan Gerhold
` (2 preceding siblings ...)
2020-04-20 19:26 ` Bjorn Andersson
@ 2020-05-26 8:24 ` Ulf Hansson
2020-05-26 8:42 ` Rafael J. Wysocki
3 siblings, 1 reply; 6+ messages in thread
From: Ulf Hansson @ 2020-05-26 8:24 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Linux PM, Stephan Gerhold, Daniel Lezcano, Andy Gross,
Bjorn Andersson, linux-arm-msm, Lorenzo Pieralisi, Lina Iyer,
~postmarketos/upstreaming, Luca Weiss
On Thu, 16 Apr 2020 at 10:59, Stephan Gerhold <stephan@gerhold.net> wrote:
>
> The Qualcomm SPM cpuidle driver seems to be the last driver still
> using the generic ARM CPUidle infrastructure.
>
> Converting it actually allows us to simplify the driver,
> and we end up being able to remove more lines than adding new ones:
>
> - We can parse the CPUidle states in the device tree directly
> with dt_idle_states (and don't need to duplicate that
> functionality into the spm driver).
>
> - Each "saw" device managed by the SPM driver now directly
> registers its own cpuidle driver, removing the need for
> any global (per cpu) state.
>
> The device tree binding is the same, so the driver stays
> compatible with all old device trees.
>
> Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Rafael, I think we all believed that you were intending to pick this
up. Is there anything additional you need to make that happen?
FYI, this allows further cleanup to be made on the ARM cpuidle code.
Both in arch and cpuidle code.
Kind regards
Uffe
> ---
> Related change for the PSCI cpuidle driver:
> https://lore.kernel.org/linux-pm/cover.1565348376.git.lorenzo.pieralisi@arm.com/
> (converting the QCOM SPM driver was mentioned there)
>
> Changes in v2:
> - Rebase on top of linux-next, fix conflicts
> (Apparently sending shortly before the end of the merge window
> was a bad idea... ;) )
>
> v1: https://lore.kernel.org/linux-arm-msm/20200405162052.53622-1-stephan@gerhold.net/
> ---
> MAINTAINERS | 1 +
> drivers/cpuidle/Kconfig.arm | 13 ++
> drivers/cpuidle/Makefile | 1 +
> .../qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} | 138 +++++++-----------
> drivers/soc/qcom/Kconfig | 10 --
> drivers/soc/qcom/Makefile | 1 -
> 6 files changed, 67 insertions(+), 97 deletions(-)
> rename drivers/{soc/qcom/spm.c => cpuidle/cpuidle-qcom-spm.c} (75%)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index e64e5db31497..2fd05a6835a6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2223,6 +2223,7 @@ F: drivers/*/qcom*
> F: drivers/*/qcom/
> F: drivers/bluetooth/btqcomsmd.c
> F: drivers/clocksource/timer-qcom.c
> +F: drivers/cpuidle/cpuidle-qcom-spm.c
> F: drivers/extcon/extcon-qcom*
> F: drivers/i2c/busses/i2c-qcom-geni.c
> F: drivers/i2c/busses/i2c-qup.c
> diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
> index 99a2d72ac02b..51a7e89085c0 100644
> --- a/drivers/cpuidle/Kconfig.arm
> +++ b/drivers/cpuidle/Kconfig.arm
> @@ -94,3 +94,16 @@ config ARM_TEGRA_CPUIDLE
> select ARM_CPU_SUSPEND
> help
> Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.
> +
> +config ARM_QCOM_SPM_CPUIDLE
> + bool "CPU Idle Driver for Qualcomm Subsystem Power Manager (SPM)"
> + depends on (ARCH_QCOM || COMPILE_TEST) && !ARM64
> + select ARM_CPU_SUSPEND
> + select CPU_IDLE_MULTIPLE_DRIVERS
> + select DT_IDLE_STATES
> + select QCOM_SCM
> + help
> + Select this to enable cpuidle for Qualcomm processors.
> + The Subsystem Power Manager (SPM) controls low power modes for the
> + CPU and L2 cores. It interface with various system drivers to put
> + the cores in low power modes.
> diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
> index 55a464f6a78b..f07800cbb43f 100644
> --- a/drivers/cpuidle/Makefile
> +++ b/drivers/cpuidle/Makefile
> @@ -25,6 +25,7 @@ obj-$(CONFIG_ARM_PSCI_CPUIDLE) += cpuidle_psci.o
> cpuidle_psci-y := cpuidle-psci.o
> cpuidle_psci-$(CONFIG_PM_GENERIC_DOMAINS_OF) += cpuidle-psci-domain.o
> obj-$(CONFIG_ARM_TEGRA_CPUIDLE) += cpuidle-tegra.o
> +obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE) += cpuidle-qcom-spm.o
>
> ###############################################################################
> # MIPS drivers
> diff --git a/drivers/soc/qcom/spm.c b/drivers/cpuidle/cpuidle-qcom-spm.c
> similarity index 75%
> rename from drivers/soc/qcom/spm.c
> rename to drivers/cpuidle/cpuidle-qcom-spm.c
> index 8e10e02c6aa5..adf91a6e4d7d 100644
> --- a/drivers/soc/qcom/spm.c
> +++ b/drivers/cpuidle/cpuidle-qcom-spm.c
> @@ -19,10 +19,11 @@
> #include <linux/cpu_pm.h>
> #include <linux/qcom_scm.h>
>
> -#include <asm/cpuidle.h>
> #include <asm/proc-fns.h>
> #include <asm/suspend.h>
>
> +#include "dt_idle_states.h"
> +
> #define MAX_PMIC_DATA 2
> #define MAX_SEQ_DATA 64
> #define SPM_CTL_INDEX 0x7f
> @@ -62,6 +63,7 @@ struct spm_reg_data {
> };
>
> struct spm_driver_data {
> + struct cpuidle_driver cpuidle_driver;
> void __iomem *reg_base;
> const struct spm_reg_data *reg_data;
> };
> @@ -107,11 +109,6 @@ static const struct spm_reg_data spm_reg_8064_cpu = {
> .start_index[PM_SLEEP_MODE_SPC] = 2,
> };
>
> -static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv);
> -
> -typedef int (*idle_fn)(void);
> -static DEFINE_PER_CPU(idle_fn*, qcom_idle_ops);
> -
> static inline void spm_register_write(struct spm_driver_data *drv,
> enum spm_reg reg, u32 val)
> {
> @@ -172,10 +169,9 @@ static int qcom_pm_collapse(unsigned long int unused)
> return -1;
> }
>
> -static int qcom_cpu_spc(void)
> +static int qcom_cpu_spc(struct spm_driver_data *drv)
> {
> int ret;
> - struct spm_driver_data *drv = __this_cpu_read(cpu_spm_drv);
>
> spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC);
> ret = cpu_suspend(0, qcom_pm_collapse);
> @@ -190,94 +186,49 @@ static int qcom_cpu_spc(void)
> return ret;
> }
>
> -static int qcom_idle_enter(unsigned long index)
> +static int spm_enter_idle_state(struct cpuidle_device *dev,
> + struct cpuidle_driver *drv, int idx)
> {
> - return __this_cpu_read(qcom_idle_ops)[index]();
> + struct spm_driver_data *data = container_of(drv, struct spm_driver_data,
> + cpuidle_driver);
> +
> + return CPU_PM_CPU_IDLE_ENTER_PARAM(qcom_cpu_spc, idx, data);
> }
>
> -static const struct of_device_id qcom_idle_state_match[] __initconst = {
> - { .compatible = "qcom,idle-state-spc", .data = qcom_cpu_spc },
> +static struct cpuidle_driver qcom_spm_idle_driver = {
> + .name = "qcom_spm",
> + .owner = THIS_MODULE,
> + .states[0] = {
> + .enter = spm_enter_idle_state,
> + .exit_latency = 1,
> + .target_residency = 1,
> + .power_usage = UINT_MAX,
> + .name = "WFI",
> + .desc = "ARM WFI",
> + }
> +};
> +
> +static const struct of_device_id qcom_idle_state_match[] = {
> + { .compatible = "qcom,idle-state-spc", .data = spm_enter_idle_state },
> { },
> };
>
> -static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu)
> +static int spm_cpuidle_init(struct cpuidle_driver *drv, int cpu)
> {
> - const struct of_device_id *match_id;
> - struct device_node *state_node;
> - int i;
> - int state_count = 1;
> - idle_fn idle_fns[CPUIDLE_STATE_MAX];
> - idle_fn *fns;
> - cpumask_t mask;
> - bool use_scm_power_down = false;
> -
> - if (!qcom_scm_is_available())
> - return -EPROBE_DEFER;
> -
> - for (i = 0; ; i++) {
> - state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);
> - if (!state_node)
> - break;
> -
> - if (!of_device_is_available(state_node))
> - continue;
> -
> - if (i == CPUIDLE_STATE_MAX) {
> - pr_warn("%s: cpuidle states reached max possible\n",
> - __func__);
> - break;
> - }
> -
> - match_id = of_match_node(qcom_idle_state_match, state_node);
> - if (!match_id)
> - return -ENODEV;
> -
> - idle_fns[state_count] = match_id->data;
> -
> - /* Check if any of the states allow power down */
> - if (match_id->data == qcom_cpu_spc)
> - use_scm_power_down = true;
> -
> - state_count++;
> - }
> -
> - if (state_count == 1)
> - goto check_spm;
> -
> - fns = devm_kcalloc(get_cpu_device(cpu), state_count, sizeof(*fns),
> - GFP_KERNEL);
> - if (!fns)
> - return -ENOMEM;
> -
> - for (i = 1; i < state_count; i++)
> - fns[i] = idle_fns[i];
> + int ret;
>
> - if (use_scm_power_down) {
> - /* We have atleast one power down mode */
> - cpumask_clear(&mask);
> - cpumask_set_cpu(cpu, &mask);
> - qcom_scm_set_warm_boot_addr(cpu_resume_arm, &mask);
> - }
> + memcpy(drv, &qcom_spm_idle_driver, sizeof(*drv));
> + drv->cpumask = (struct cpumask *)cpumask_of(cpu);
>
> - per_cpu(qcom_idle_ops, cpu) = fns;
> + /* Parse idle states from device tree */
> + ret = dt_init_idle_driver(drv, qcom_idle_state_match, 1);
> + if (ret <= 0)
> + return ret ? : -ENODEV;
>
> - /*
> - * SPM probe for the cpu should have happened by now, if the
> - * SPM device does not exist, return -ENXIO to indicate that the
> - * cpu does not support idle states.
> - */
> -check_spm:
> - return per_cpu(cpu_spm_drv, cpu) ? 0 : -ENXIO;
> + /* We have atleast one power down mode */
> + return qcom_scm_set_warm_boot_addr(cpu_resume_arm, drv->cpumask);
> }
>
> -static const struct cpuidle_ops qcom_cpuidle_ops __initconst = {
> - .suspend = qcom_idle_enter,
> - .init = qcom_cpuidle_init,
> -};
> -
> -CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v1, "qcom,kpss-acc-v1", &qcom_cpuidle_ops);
> -CPUIDLE_METHOD_OF_DECLARE(qcom_idle_v2, "qcom,kpss-acc-v2", &qcom_cpuidle_ops);
> -
> static struct spm_driver_data *spm_get_drv(struct platform_device *pdev,
> int *spm_cpu)
> {
> @@ -323,11 +274,15 @@ static int spm_dev_probe(struct platform_device *pdev)
> struct resource *res;
> const struct of_device_id *match_id;
> void __iomem *addr;
> - int cpu;
> + int cpu, ret;
> +
> + if (!qcom_scm_is_available())
> + return -EPROBE_DEFER;
>
> drv = spm_get_drv(pdev, &cpu);
> if (!drv)
> return -EINVAL;
> + platform_set_drvdata(pdev, drv);
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> drv->reg_base = devm_ioremap_resource(&pdev->dev, res);
> @@ -340,6 +295,10 @@ static int spm_dev_probe(struct platform_device *pdev)
>
> drv->reg_data = match_id->data;
>
> + ret = spm_cpuidle_init(&drv->cpuidle_driver, cpu);
> + if (ret)
> + return ret;
> +
> /* Write the SPM sequences first.. */
> addr = drv->reg_base + drv->reg_data->reg_offset[SPM_REG_SEQ_ENTRY];
> __iowrite32_copy(addr, drv->reg_data->seq,
> @@ -362,13 +321,20 @@ static int spm_dev_probe(struct platform_device *pdev)
> /* Set up Standby as the default low power mode */
> spm_set_low_power_mode(drv, PM_SLEEP_MODE_STBY);
>
> - per_cpu(cpu_spm_drv, cpu) = drv;
> + return cpuidle_register(&drv->cpuidle_driver, NULL);
> +}
> +
> +static int spm_dev_remove(struct platform_device *pdev)
> +{
> + struct spm_driver_data *drv = platform_get_drvdata(pdev);
>
> + cpuidle_unregister(&drv->cpuidle_driver);
> return 0;
> }
>
> static struct platform_driver spm_driver = {
> .probe = spm_dev_probe,
> + .remove = spm_dev_remove,
> .driver = {
> .name = "saw",
> .of_match_table = spm_match_table,
> diff --git a/drivers/soc/qcom/Kconfig b/drivers/soc/qcom/Kconfig
> index bf42a17a45de..285baa7e474e 100644
> --- a/drivers/soc/qcom/Kconfig
> +++ b/drivers/soc/qcom/Kconfig
> @@ -80,16 +80,6 @@ config QCOM_PDR_HELPERS
> tristate
> select QCOM_QMI_HELPERS
>
> -config QCOM_PM
> - bool "Qualcomm Power Management"
> - depends on ARCH_QCOM && !ARM64
> - select ARM_CPU_SUSPEND
> - select QCOM_SCM
> - help
> - QCOM Platform specific power driver to manage cores and L2 low power
> - modes. It interface with various system drivers to put the cores in
> - low power modes.
> -
> config QCOM_QMI_HELPERS
> tristate
> depends on NET
> diff --git a/drivers/soc/qcom/Makefile b/drivers/soc/qcom/Makefile
> index 5d6b83dc58e8..92cc4232d72c 100644
> --- a/drivers/soc/qcom/Makefile
> +++ b/drivers/soc/qcom/Makefile
> @@ -8,7 +8,6 @@ obj-$(CONFIG_QCOM_GSBI) += qcom_gsbi.o
> obj-$(CONFIG_QCOM_MDT_LOADER) += mdt_loader.o
> obj-$(CONFIG_QCOM_OCMEM) += ocmem.o
> obj-$(CONFIG_QCOM_PDR_HELPERS) += pdr_interface.o
> -obj-$(CONFIG_QCOM_PM) += spm.o
> obj-$(CONFIG_QCOM_QMI_HELPERS) += qmi_helpers.o
> qmi_helpers-y += qmi_encdec.o qmi_interface.o
> obj-$(CONFIG_QCOM_RMTFS_MEM) += rmtfs_mem.o
> --
> 2.26.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver
2020-05-26 8:24 ` Ulf Hansson
@ 2020-05-26 8:42 ` Rafael J. Wysocki
0 siblings, 0 replies; 6+ messages in thread
From: Rafael J. Wysocki @ 2020-05-26 8:42 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rafael J. Wysocki, Linux PM, Stephan Gerhold, Daniel Lezcano,
Andy Gross, Bjorn Andersson, linux-arm-msm, Lorenzo Pieralisi,
Lina Iyer, ~postmarketos/upstreaming, Luca Weiss
On Tue, May 26, 2020 at 10:24 AM Ulf Hansson <ulf.hansson@linaro.org> wrote:
>
> On Thu, 16 Apr 2020 at 10:59, Stephan Gerhold <stephan@gerhold.net> wrote:
> >
> > The Qualcomm SPM cpuidle driver seems to be the last driver still
> > using the generic ARM CPUidle infrastructure.
> >
> > Converting it actually allows us to simplify the driver,
> > and we end up being able to remove more lines than adding new ones:
> >
> > - We can parse the CPUidle states in the device tree directly
> > with dt_idle_states (and don't need to duplicate that
> > functionality into the spm driver).
> >
> > - Each "saw" device managed by the SPM driver now directly
> > registers its own cpuidle driver, removing the need for
> > any global (per cpu) state.
> >
> > The device tree binding is the same, so the driver stays
> > compatible with all old device trees.
> >
> > Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
>
> Rafael, I think we all believed that you were intending to pick this up.
Oh, but nobody let me know that before. :-)
> Is there anything additional you need to make that happen?
Not really, thanks for letting me know that I was expected to take it.
> FYI, this allows further cleanup to be made on the ARM cpuidle code.
> Both in arch and cpuidle code.
Sure.
Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-05-26 8:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-16 8:58 [PATCH v2] cpuidle: Convert Qualcomm SPM driver to a generic CPUidle driver Stephan Gerhold
2020-04-20 14:58 ` Lina Iyer
2020-04-20 19:10 ` Ulf Hansson
2020-04-20 19:26 ` Bjorn Andersson
2020-05-26 8:24 ` Ulf Hansson
2020-05-26 8:42 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).