From: Santosh Shilimkar <santosh.shilimkar@ti.com>
To: Lokesh Vutla <lokeshvutla@ti.com>
Cc: linux-kernel@vger.kernel.org, gregkh@linuxfoundation.org,
linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/8] memory: emif: Correct the lpmode timeout calculation
Date: Mon, 11 Mar 2013 11:04:02 +0530 [thread overview]
Message-ID: <513D6CCA.4000906@ti.com> (raw)
In-Reply-To: <1362978365-5593-2-git-send-email-lokeshvutla@ti.com>
minor nit.
$subject
s/Correct/Fix
On Monday 11 March 2013 10:35 AM, Lokesh Vutla wrote:
> The driver tries to round up the specified timeout cycles to
> the next power of 2 value. But this is done wrongly.
> Correcting this here.
>
Change needs to be improved here. See below.
> Reported-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> drivers/memory/emif.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
> index df08736..622638c 100644
> --- a/drivers/memory/emif.c
> +++ b/drivers/memory/emif.c
> @@ -732,9 +732,9 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
> if (timeout < 16) {
> timeout = 0;
> } else {
> - timeout = __fls(timeout) - 3;
> if (timeout & (timeout - 1))
So from the change, it appears that, the timeout
check for power of 2 should be done before updating
the variable which seems to be the right fix.
> - timeout++;
> + timeout <<= 1;
> + timeout = __fls(timeout) - 3;
> }
>
> switch (lpmode) {
>
So just make changelog verbose as well as add a
comment about the calculation in the code.
Otherwise, patch looks fine to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
WARNING: multiple messages have this Message-ID (diff)
From: santosh.shilimkar@ti.com (Santosh Shilimkar)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/8] memory: emif: Correct the lpmode timeout calculation
Date: Mon, 11 Mar 2013 11:04:02 +0530 [thread overview]
Message-ID: <513D6CCA.4000906@ti.com> (raw)
In-Reply-To: <1362978365-5593-2-git-send-email-lokeshvutla@ti.com>
minor nit.
$subject
s/Correct/Fix
On Monday 11 March 2013 10:35 AM, Lokesh Vutla wrote:
> The driver tries to round up the specified timeout cycles to
> the next power of 2 value. But this is done wrongly.
> Correcting this here.
>
Change needs to be improved here. See below.
> Reported-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> drivers/memory/emif.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
> index df08736..622638c 100644
> --- a/drivers/memory/emif.c
> +++ b/drivers/memory/emif.c
> @@ -732,9 +732,9 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
> if (timeout < 16) {
> timeout = 0;
> } else {
> - timeout = __fls(timeout) - 3;
> if (timeout & (timeout - 1))
So from the change, it appears that, the timeout
check for power of 2 should be done before updating
the variable which seems to be the right fix.
> - timeout++;
> + timeout <<= 1;
> + timeout = __fls(timeout) - 3;
> }
>
> switch (lpmode) {
>
So just make changelog verbose as well as add a
comment about the calculation in the code.
Otherwise, patch looks fine to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
WARNING: multiple messages have this Message-ID (diff)
From: Santosh Shilimkar <santosh.shilimkar@ti.com>
To: Lokesh Vutla <lokeshvutla@ti.com>
Cc: <linux-kernel@vger.kernel.org>, <gregkh@linuxfoundation.org>,
<linux-omap@vger.kernel.org>,
<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/8] memory: emif: Correct the lpmode timeout calculation
Date: Mon, 11 Mar 2013 11:04:02 +0530 [thread overview]
Message-ID: <513D6CCA.4000906@ti.com> (raw)
In-Reply-To: <1362978365-5593-2-git-send-email-lokeshvutla@ti.com>
minor nit.
$subject
s/Correct/Fix
On Monday 11 March 2013 10:35 AM, Lokesh Vutla wrote:
> The driver tries to round up the specified timeout cycles to
> the next power of 2 value. But this is done wrongly.
> Correcting this here.
>
Change needs to be improved here. See below.
> Reported-by: Nishanth Menon <nm@ti.com>
> Signed-off-by: Lokesh Vutla <lokeshvutla@ti.com>
> ---
> drivers/memory/emif.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/memory/emif.c b/drivers/memory/emif.c
> index df08736..622638c 100644
> --- a/drivers/memory/emif.c
> +++ b/drivers/memory/emif.c
> @@ -732,9 +732,9 @@ static u32 get_pwr_mgmt_ctrl(u32 freq, struct emif_data *emif, u32 ip_rev)
> if (timeout < 16) {
> timeout = 0;
> } else {
> - timeout = __fls(timeout) - 3;
> if (timeout & (timeout - 1))
So from the change, it appears that, the timeout
check for power of 2 should be done before updating
the variable which seems to be the right fix.
> - timeout++;
> + timeout <<= 1;
> + timeout = __fls(timeout) - 3;
> }
>
> switch (lpmode) {
>
So just make changelog verbose as well as add a
comment about the calculation in the code.
Otherwise, patch looks fine to me.
Acked-by: Santosh Shilimkar <santosh.shilimkar@ti.com>
next prev parent reply other threads:[~2013-03-11 5:34 UTC|newest]
Thread overview: 59+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-03-11 5:05 [PATCH 0/8] memory: emif: miscellaneous bug fixes for EMIF driver Lokesh Vutla
2013-03-11 5:05 ` Lokesh Vutla
2013-03-11 5:05 ` Lokesh Vutla
2013-03-11 5:05 ` [PATCH 1/8] memory: emif: Correct the lpmode timeout calculation Lokesh Vutla
2013-03-11 5:05 ` Lokesh Vutla
2013-03-11 5:05 ` Lokesh Vutla
2013-03-11 5:34 ` Santosh Shilimkar [this message]
2013-03-11 5:34 ` Santosh Shilimkar
2013-03-11 5:34 ` Santosh Shilimkar
2013-03-11 5:05 ` [PATCH 2/8] memory: emif: setup LP settings on freq update Lokesh Vutla
2013-03-11 5:05 ` Lokesh Vutla
2013-03-11 5:05 ` Lokesh Vutla
2013-03-11 5:34 ` Santosh Shilimkar
2013-03-11 5:34 ` Santosh Shilimkar
2013-03-11 5:34 ` Santosh Shilimkar
2013-03-11 5:06 ` [PATCH 3/8] memory: emif: handle overflow for timing for LP mode Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:35 ` Santosh Shilimkar
2013-03-11 5:35 ` Santosh Shilimkar
2013-03-11 5:35 ` Santosh Shilimkar
2013-03-11 5:06 ` [PATCH 4/8] memory: emif: Handle devices which are not rated for >85C Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:38 ` Santosh Shilimkar
2013-03-11 5:38 ` Santosh Shilimkar
2013-03-11 5:38 ` Santosh Shilimkar
2013-03-11 5:06 ` [PATCH 5/8] memory: emif: use restart if power_off not present when out of spec Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:39 ` Santosh Shilimkar
2013-03-11 5:39 ` Santosh Shilimkar
2013-03-11 5:39 ` Santosh Shilimkar
2013-03-11 5:06 ` [PATCH 6/8] memory: emif: fix timings initialization issue Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:44 ` Santosh Shilimkar
2013-03-11 5:44 ` Santosh Shilimkar
2013-03-11 5:44 ` Santosh Shilimkar
2013-03-11 5:06 ` [PATCH 7/8] memory: emif: errata i743: Prohibit usage of Power-Down mode Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:50 ` Santosh Shilimkar
2013-03-11 5:50 ` Santosh Shilimkar
2013-03-11 5:50 ` Santosh Shilimkar
2013-03-11 11:10 ` Lokesh Vutla
2013-03-11 11:10 ` Lokesh Vutla
2013-03-11 11:10 ` Lokesh Vutla
2013-03-11 5:06 ` [PATCH 8/8] memory: emif: Load the correct custom config values from dt Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:06 ` Lokesh Vutla
2013-03-11 5:51 ` Santosh Shilimkar
2013-03-11 5:51 ` Santosh Shilimkar
2013-03-11 5:51 ` Santosh Shilimkar
2013-03-15 18:08 ` [PATCH 0/8] memory: emif: miscellaneous bug fixes for EMIF driver Greg KH
2013-03-15 18:08 ` Greg KH
2013-03-16 6:13 ` Lokesh Vutla
2013-03-16 6:13 ` Lokesh Vutla
2013-03-16 6:13 ` Lokesh Vutla
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=513D6CCA.4000906@ti.com \
--to=santosh.shilimkar@ti.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-omap@vger.kernel.org \
--cc=lokeshvutla@ti.com \
/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.