Date: Thu, 9 Jul 2015 16:43:04 +0800
From: Jisheng Zhang <jszhang@marvell.com>
To: <linux@arm.linux.org.uk>, <will.deacon@arm.com>, <mark.rutland@arm.com>,
        <daniel.lezcano@linaro.org>, <Catalin.Marinas@arm.com>,
        <Lorenzo.Pieralisi@arm.com>, <galak@codeaurora.org>,
        <agross@codeaurora.org>, <davidb@codeaurora.org>
CC: <linux-arm-kernel@lists.infradead.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v3 2/3] ARM: cpuidle: refine cpuidle_ops member's
 parameters.
Message-ID: <20150709164304.26b6c2be@xhacker>
In-Reply-To: <1436430685-2202-3-git-send-email-jszhang@marvell.com>
References: <1436430685-2202-1-git-send-email-jszhang@marvell.com>
	<1436430685-2202-3-git-send-email-jszhang@marvell.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="US-ASCII"
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 5075
Lines: 148

On Thu, 9 Jul 2015 16:31:24 +0800
Jisheng Zhang <jszhang@marvell.com> wrote:

> As for the suspend member function, the to-be-suspended cpu is always
> the calling cpu itself, so the 'cpu' parameter may not be necessary, let
> driver get 'cpu' itself if need.
> 
> As for the init member function, the device_node here may not be
> necessary either, because we can get the node via. of_get_cpu_node().

This patch also changes drivers/soc/qcom/spm.c accordingly, but I have no
qcom platform, so I can't test it, just make sure it can pass kernel building.
Could anyone kindly help me to test?

Thanks a lot,
Jisheng

> 
> Signed-off-by: Jisheng Zhang <jszhang@marvell.com>
> ---
>  arch/arm/include/asm/cpuidle.h |  6 +++---
>  arch/arm/kernel/cpuidle.c      |  8 ++++----
>  drivers/soc/qcom/spm.c         | 17 ++++++++++++-----
>  3 files changed, 19 insertions(+), 12 deletions(-)
> 
> diff --git a/arch/arm/include/asm/cpuidle.h b/arch/arm/include/asm/cpuidle.h
> index 0f84249..ca5dfe6 100644
> --- a/arch/arm/include/asm/cpuidle.h
> +++ b/arch/arm/include/asm/cpuidle.h
> @@ -30,8 +30,8 @@ static inline int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
>  struct device_node;
>  
>  struct cpuidle_ops {
> -	int (*suspend)(int cpu, unsigned long arg);
> -	int (*init)(struct device_node *, int cpu);
> +	int (*suspend)(unsigned long arg);
> +	int (*init)(unsigned int cpu);
>  };
>  
>  struct of_cpuidle_method {
> @@ -46,6 +46,6 @@ struct of_cpuidle_method {
>  
>  extern int arm_cpuidle_suspend(int index);
>  
> -extern int arm_cpuidle_init(int cpu);
> +extern int arm_cpuidle_init(unsigned int cpu);
>  
>  #endif
> diff --git a/arch/arm/kernel/cpuidle.c b/arch/arm/kernel/cpuidle.c
> index 318da33..7bc7bc6 100644
> --- a/arch/arm/kernel/cpuidle.c
> +++ b/arch/arm/kernel/cpuidle.c
> @@ -53,10 +53,10 @@ int arm_cpuidle_simple_enter(struct cpuidle_device *dev,
>  int arm_cpuidle_suspend(int index)
>  {
>  	int ret = -EOPNOTSUPP;
> -	int cpu = smp_processor_id();
> +	unsigned int cpu = smp_processor_id();
>  
>  	if (cpuidle_ops[cpu].suspend)
> -		ret = cpuidle_ops[cpu].suspend(cpu, index);
> +		ret = cpuidle_ops[cpu].suspend(index);
>  
>  	return ret;
>  }
> @@ -134,7 +134,7 @@ static int __init arm_cpuidle_read_ops(struct device_node *dn, int cpu)
>   *  -ENXIO if the HW reports a failure or a misconfiguration,
>   *  -ENOMEM if the HW report an memory allocation failure 
>   */
> -int __init arm_cpuidle_init(int cpu)
> +int __init arm_cpuidle_init(unsigned int cpu)
>  {
>  	struct device_node *cpu_node = of_cpu_device_node_get(cpu);
>  	int ret;
> @@ -144,7 +144,7 @@ int __init arm_cpuidle_init(int cpu)
>  
>  	ret = arm_cpuidle_read_ops(cpu_node, cpu);
>  	if (!ret && cpuidle_ops[cpu].init)
> -		ret = cpuidle_ops[cpu].init(cpu_node, cpu);
> +		ret = cpuidle_ops[cpu].init(cpu);
>  
>  	of_node_put(cpu_node);
>  
> diff --git a/drivers/soc/qcom/spm.c b/drivers/soc/qcom/spm.c
> index b04b05a..1649ec3 100644
> --- a/drivers/soc/qcom/spm.c
> +++ b/drivers/soc/qcom/spm.c
> @@ -116,7 +116,7 @@ static const struct spm_reg_data spm_reg_8064_cpu = {
>  
>  static DEFINE_PER_CPU(struct spm_driver_data *, cpu_spm_drv);
>  
> -typedef int (*idle_fn)(int);
> +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,
> @@ -179,9 +179,10 @@ static int qcom_pm_collapse(unsigned long int unused)
>  	return -1;
>  }
>  
> -static int qcom_cpu_spc(int cpu)
> +static int qcom_cpu_spc(void)
>  {
>  	int ret;
> +	int cpu = smp_processor_id();
>  	struct spm_driver_data *drv = per_cpu(cpu_spm_drv, cpu);
>  
>  	spm_set_low_power_mode(drv, PM_SLEEP_MODE_SPC);
> @@ -197,9 +198,11 @@ static int qcom_cpu_spc(int cpu)
>  	return ret;
>  }
>  
> -static int qcom_idle_enter(int cpu, unsigned long index)
> +static int qcom_idle_enter(unsigned long index)
>  {
> -	return per_cpu(qcom_idle_ops, cpu)[index](cpu);
> +	unsigned int cpu = smp_processor_id();
> +
> +	return per_cpu(qcom_idle_ops, cpu)[index]();
>  }
>  
>  static const struct of_device_id qcom_idle_state_match[] __initconst = {
> @@ -207,7 +210,7 @@ static const struct of_device_id qcom_idle_state_match[] __initconst = {
>  	{ },
>  };
>  
> -static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu)
> +static int __init qcom_cpuidle_init(unsigned int cpu)
>  {
>  	const struct of_device_id *match_id;
>  	struct device_node *state_node;
> @@ -217,6 +220,10 @@ static int __init qcom_cpuidle_init(struct device_node *cpu_node, int cpu)
>  	idle_fn *fns;
>  	cpumask_t mask;
>  	bool use_scm_power_down = false;
> +	struct device_node *cpu_node = of_get_cpu_node(cpu, NULL);
> +
> +	if (!cpu_node)
> +		return -ENODEV;
>  
>  	for (i = 0; ; i++) {
>  		state_node = of_parse_phandle(cpu_node, "cpu-idle-states", i);

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/