All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Lezcano <daniel.lezcano@linaro.org>
To: Sekhar Nori <nsekhar@ti.com>
Cc: khilman@ti.com, davinci-linux-open-source@linux.davincidsp.com,
	linaro-dev@lists.linaro.org, patches@linaro.org,
	linux-pm@lists.linux-foundation.org
Subject: Re: [PATCH 2/2] ARM: DAVINCI: cpuidle - remove ops
Date: Sun, 20 May 2012 12:37:02 +0200	[thread overview]
Message-ID: <4FB8C94E.4080806@linaro.org> (raw)
In-Reply-To: <4FB8C53C.1020301@ti.com>

On 05/20/2012 12:19 PM, Sekhar Nori wrote:
> 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@linaro.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.

Sure, I will resend a version without the initialization and the 
_raw_write variant.

Thanks
   -- Daniel


-- 
  <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

_______________________________________________
linux-pm mailing list
linux-pm@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-pm

  reply	other threads:[~2012-05-20 10:37 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-10  8:44 [PATCH 0/2] ARM: DAVINCI: cpuidle - cleanups Daniel Lezcano
2012-05-10  8:44 ` [PATCH 1/2] ARM: DAVINCI: cpuidle - remove useless state count initialization Daniel Lezcano
     [not found]   ` <1336639485-26955-2-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2012-05-20  9:24     ` Sekhar Nori
     [not found] ` <1336639485-26955-1-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2012-05-10  8:44   ` [PATCH 2/2] ARM: DAVINCI: cpuidle - remove ops Daniel Lezcano
     [not found]     ` <1336639485-26955-3-git-send-email-daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2012-05-20 10:19       ` Sekhar Nori
2012-05-20 10:37         ` Daniel Lezcano [this message]
2012-05-19  9:26 ` [PATCH 0/2] ARM: DAVINCI: cpuidle - cleanups Daniel Lezcano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4FB8C94E.4080806@linaro.org \
    --to=daniel.lezcano@linaro.org \
    --cc=davinci-linux-open-source@linux.davincidsp.com \
    --cc=khilman@ti.com \
    --cc=linaro-dev@lists.linaro.org \
    --cc=linux-pm@lists.linux-foundation.org \
    --cc=nsekhar@ti.com \
    --cc=patches@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.