Re: [PATCH 2/2] ARM: DAVINCI: cpuidle - remove ops

[Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org>]

Hi Daniel,

On 5/10/2012 2:14 PM, Daniel Lezcano wrote:
> This patch removes the ops usage because we have the index
> passed as parameter to the idle function and we can determine
> if we do WFI or memory retention.
> 
> The benefit of this cleanup is the removal of:
>  * the ops
>  * the statedata usage because we want to get rid of it in all the drivers
>  * extra structure definition
>  * extra functions definition
>  * pointless macro definition BIT(0)

This macro was not pointless in the original code. This comment seems to
suggest so.

> 
> It also benefits the readability.
> 
> Signed-off-by: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm/mach-davinci/cpuidle.c |   81 +++++++++++++--------------------------
>  1 files changed, 27 insertions(+), 54 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/cpuidle.c b/arch/arm/mach-davinci/cpuidle.c
> index f0f179c..61f4e52 100644
> --- a/arch/arm/mach-davinci/cpuidle.c
> +++ b/arch/arm/mach-davinci/cpuidle.c
> @@ -25,35 +25,46 @@
>  
>  #define DAVINCI_CPUIDLE_MAX_STATES	2
>  
> -struct davinci_ops {
> -	void (*enter) (u32 flags);
> -	void (*exit) (u32 flags);
> -	u32 flags;
> -};
> +static bool ddr2_pwdn = false;

This zero initialization is not required. In fact this throws a
checkpatch error.

> +
> +static void __iomem *ddr2_reg_base;
> +
> +static void davinci_save_ddr_power(int enter, bool pdown)
> +{
> +	u32 val;
> +
> +	val = __raw_readl(ddr2_reg_base + DDR2_SDRCR_OFFSET);
> +
> +	if (enter) {
> +		if (pdown)
> +			val |= DDR2_SRPD_BIT;
> +		else
> +			val &= ~DDR2_SRPD_BIT;
> +		val |= DDR2_LPMODEN_BIT;
> +	} else {
> +		val &= ~(DDR2_SRPD_BIT | DDR2_LPMODEN_BIT);
> +	}
> +
> +	__raw_writel(val, ddr2_reg_base + DDR2_SDRCR_OFFSET);

Can you please use readl/writel instead of the __raw variants. I know
this is carried from original code, but we need to git rid of it with
cleanups like this.

Thanks,
Sekhar