* [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
@ 2016-04-04 17:03 Stephen Warren
2016-04-05 0:01 ` Tom Rini
2016-04-26 0:16 ` [U-Boot] " Tom Rini
0 siblings, 2 replies; 9+ messages in thread
From: Stephen Warren @ 2016-04-04 17:03 UTC (permalink / raw)
To: u-boot
From: Stephen Warren <swarren@nvidia.com>
get_timer() returns an unsigned 64-bit value, but is currently assigned to
a signed 32-bit variable. Due to sign extension and data truncation, this
causes the timeout loop in spi_flash_cmd_wait_ready() to immediately (and
incorrectly) fire for about 50% of all time values, based on whether bit
31 is set. In sandbox at least, this causes the test to pass or fail based
on system uptime, as opposed to time since the U-Boot binary was started.
Fixes: 4efad20a1751 ("sf: Update status reg check in spi_flash_cmd_wait_ready")
Signed-off-by: Stephen Warren <swarren@nvidia.com>
---
drivers/mtd/spi/spi_flash.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c
index 44d9e9ba0105..545172568949 100644
--- a/drivers/mtd/spi/spi_flash.c
+++ b/drivers/mtd/spi/spi_flash.c
@@ -265,7 +265,8 @@ static int spi_flash_ready(struct spi_flash *flash)
static int spi_flash_cmd_wait_ready(struct spi_flash *flash,
unsigned long timeout)
{
- int timebase, ret;
+ unsigned long timebase;
+ int ret;
timebase = get_timer(0);
--
2.8.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
2016-04-04 17:03 [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready() Stephen Warren
@ 2016-04-05 0:01 ` Tom Rini
2016-04-06 11:22 ` Jagan Teki
2016-04-26 0:16 ` [U-Boot] " Tom Rini
1 sibling, 1 reply; 9+ messages in thread
From: Tom Rini @ 2016-04-05 0:01 UTC (permalink / raw)
To: u-boot
On Mon, Apr 04, 2016 at 11:03:52AM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> get_timer() returns an unsigned 64-bit value, but is currently assigned to
> a signed 32-bit variable. Due to sign extension and data truncation, this
> causes the timeout loop in spi_flash_cmd_wait_ready() to immediately (and
> incorrectly) fire for about 50% of all time values, based on whether bit
> 31 is set. In sandbox at least, this causes the test to pass or fail based
> on system uptime, as opposed to time since the U-Boot binary was started.
>
> Fixes: 4efad20a1751 ("sf: Update status reg check in spi_flash_cmd_wait_ready")
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
Reviewed-by: Tom Rini <trini@konsulko.com>
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160404/3bea6f8f/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
2016-04-05 0:01 ` Tom Rini
@ 2016-04-06 11:22 ` Jagan Teki
2016-04-12 15:43 ` Stephen Warren
2016-04-21 5:49 ` Stephen Warren
0 siblings, 2 replies; 9+ messages in thread
From: Jagan Teki @ 2016-04-06 11:22 UTC (permalink / raw)
To: u-boot
On 5 April 2016 at 05:31, Tom Rini <trini@konsulko.com> wrote:
> On Mon, Apr 04, 2016 at 11:03:52AM -0600, Stephen Warren wrote:
>
>> From: Stephen Warren <swarren@nvidia.com>
>>
>> get_timer() returns an unsigned 64-bit value, but is currently assigned to
>> a signed 32-bit variable. Due to sign extension and data truncation, this
>> causes the timeout loop in spi_flash_cmd_wait_ready() to immediately (and
>> incorrectly) fire for about 50% of all time values, based on whether bit
>> 31 is set. In sandbox at least, this causes the test to pass or fail based
>> on system uptime, as opposed to time since the U-Boot binary was started.
>>
>> Fixes: 4efad20a1751 ("sf: Update status reg check in spi_flash_cmd_wait_ready")
>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
Reviewed-by: Jagan Teki <jteki@openedev.com>
--
Jagan.
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
2016-04-06 11:22 ` Jagan Teki
@ 2016-04-12 15:43 ` Stephen Warren
2016-04-12 16:03 ` Tom Rini
2016-04-21 5:49 ` Stephen Warren
1 sibling, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2016-04-12 15:43 UTC (permalink / raw)
To: u-boot
On 04/06/2016 05:22 AM, Jagan Teki wrote:
> On 5 April 2016 at 05:31, Tom Rini <trini@konsulko.com> wrote:
>> On Mon, Apr 04, 2016 at 11:03:52AM -0600, Stephen Warren wrote:
>>
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> get_timer() returns an unsigned 64-bit value, but is currently assigned to
>>> a signed 32-bit variable. Due to sign extension and data truncation, this
>>> causes the timeout loop in spi_flash_cmd_wait_ready() to immediately (and
>>> incorrectly) fire for about 50% of all time values, based on whether bit
>>> 31 is set. In sandbox at least, this causes the test to pass or fail based
>>> on system uptime, as opposed to time since the U-Boot binary was started.
>>>
>>> Fixes: 4efad20a1751 ("sf: Update status reg check in spi_flash_cmd_wait_ready")
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> Reviewed-by: Jagan Teki <jteki@openedev.com>
It'd be great if this could be applied to fix the final test/py failure
on sandbox.
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
2016-04-12 15:43 ` Stephen Warren
@ 2016-04-12 16:03 ` Tom Rini
0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2016-04-12 16:03 UTC (permalink / raw)
To: u-boot
On Tue, Apr 12, 2016 at 09:43:03AM -0600, Stephen Warren wrote:
> On 04/06/2016 05:22 AM, Jagan Teki wrote:
> >On 5 April 2016 at 05:31, Tom Rini <trini@konsulko.com> wrote:
> >>On Mon, Apr 04, 2016 at 11:03:52AM -0600, Stephen Warren wrote:
> >>
> >>>From: Stephen Warren <swarren@nvidia.com>
> >>>
> >>>get_timer() returns an unsigned 64-bit value, but is currently assigned to
> >>>a signed 32-bit variable. Due to sign extension and data truncation, this
> >>>causes the timeout loop in spi_flash_cmd_wait_ready() to immediately (and
> >>>incorrectly) fire for about 50% of all time values, based on whether bit
> >>>31 is set. In sandbox at least, this causes the test to pass or fail based
> >>>on system uptime, as opposed to time since the U-Boot binary was started.
> >>>
> >>>Fixes: 4efad20a1751 ("sf: Update status reg check in spi_flash_cmd_wait_ready")
> >>>Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >>
> >>Reviewed-by: Tom Rini <trini@konsulko.com>
> >
> >Reviewed-by: Jagan Teki <jteki@openedev.com>
>
> It'd be great if this could be applied to fix the final test/py
> failure on sandbox.
Yes, please, lets get some PRs with fixes in now :) Thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160412/e256fec9/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
2016-04-06 11:22 ` Jagan Teki
2016-04-12 15:43 ` Stephen Warren
@ 2016-04-21 5:49 ` Stephen Warren
2016-04-25 16:44 ` Stephen Warren
1 sibling, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2016-04-21 5:49 UTC (permalink / raw)
To: u-boot
On 04/06/2016 05:22 AM, Jagan Teki wrote:
> On 5 April 2016 at 05:31, Tom Rini <trini@konsulko.com> wrote:
>> On Mon, Apr 04, 2016 at 11:03:52AM -0600, Stephen Warren wrote:
>>
>>> From: Stephen Warren <swarren@nvidia.com>
>>>
>>> get_timer() returns an unsigned 64-bit value, but is currently assigned to
>>> a signed 32-bit variable. Due to sign extension and data truncation, this
>>> causes the timeout loop in spi_flash_cmd_wait_ready() to immediately (and
>>> incorrectly) fire for about 50% of all time values, based on whether bit
>>> 31 is set. In sandbox at least, this causes the test to pass or fail based
>>> on system uptime, as opposed to time since the U-Boot binary was started.
>>>
>>> Fixes: 4efad20a1751 ("sf: Update status reg check in spi_flash_cmd_wait_ready")
>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>
>> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> Reviewed-by: Jagan Teki <jteki@openedev.com>
Jagan, are you going to apply this?
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
2016-04-21 5:49 ` Stephen Warren
@ 2016-04-25 16:44 ` Stephen Warren
2016-04-25 16:46 ` Tom Rini
0 siblings, 1 reply; 9+ messages in thread
From: Stephen Warren @ 2016-04-25 16:44 UTC (permalink / raw)
To: u-boot
On 04/20/2016 11:49 PM, Stephen Warren wrote:
> On 04/06/2016 05:22 AM, Jagan Teki wrote:
>> On 5 April 2016 at 05:31, Tom Rini <trini@konsulko.com> wrote:
>>> On Mon, Apr 04, 2016 at 11:03:52AM -0600, Stephen Warren wrote:
>>>
>>>> From: Stephen Warren <swarren@nvidia.com>
>>>>
>>>> get_timer() returns an unsigned 64-bit value, but is currently
>>>> assigned to
>>>> a signed 32-bit variable. Due to sign extension and data truncation,
>>>> this
>>>> causes the timeout loop in spi_flash_cmd_wait_ready() to immediately
>>>> (and
>>>> incorrectly) fire for about 50% of all time values, based on whether
>>>> bit
>>>> 31 is set. In sandbox at least, this causes the test to pass or fail
>>>> based
>>>> on system uptime, as opposed to time since the U-Boot binary was
>>>> started.
>>>>
>>>> Fixes: 4efad20a1751 ("sf: Update status reg check in
>>>> spi_flash_cmd_wait_ready")
>>>> Signed-off-by: Stephen Warren <swarren@nvidia.com>
>>>
>>> Reviewed-by: Tom Rini <trini@konsulko.com>
>>
>> Reviewed-by: Jagan Teki <jteki@openedev.com>
>
> Jagan, are you going to apply this?
Tom, does it make sense for you to apply this?
^ permalink raw reply [flat|nested] 9+ messages in thread* [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
2016-04-25 16:44 ` Stephen Warren
@ 2016-04-25 16:46 ` Tom Rini
0 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2016-04-25 16:46 UTC (permalink / raw)
To: u-boot
On Mon, Apr 25, 2016 at 10:44:02AM -0600, Stephen Warren wrote:
> On 04/20/2016 11:49 PM, Stephen Warren wrote:
> >On 04/06/2016 05:22 AM, Jagan Teki wrote:
> >>On 5 April 2016 at 05:31, Tom Rini <trini@konsulko.com> wrote:
> >>>On Mon, Apr 04, 2016 at 11:03:52AM -0600, Stephen Warren wrote:
> >>>
> >>>>From: Stephen Warren <swarren@nvidia.com>
> >>>>
> >>>>get_timer() returns an unsigned 64-bit value, but is currently
> >>>>assigned to
> >>>>a signed 32-bit variable. Due to sign extension and data truncation,
> >>>>this
> >>>>causes the timeout loop in spi_flash_cmd_wait_ready() to immediately
> >>>>(and
> >>>>incorrectly) fire for about 50% of all time values, based on whether
> >>>>bit
> >>>>31 is set. In sandbox at least, this causes the test to pass or fail
> >>>>based
> >>>>on system uptime, as opposed to time since the U-Boot binary was
> >>>>started.
> >>>>
> >>>>Fixes: 4efad20a1751 ("sf: Update status reg check in
> >>>>spi_flash_cmd_wait_ready")
> >>>>Signed-off-by: Stephen Warren <swarren@nvidia.com>
> >>>
> >>>Reviewed-by: Tom Rini <trini@konsulko.com>
> >>
> >>Reviewed-by: Jagan Teki <jteki@openedev.com>
> >
> >Jagan, are you going to apply this?
>
> Tom, does it make sense for you to apply this?
If Jagan doesn't want to make up a small PR for this, yes. And since I
should do an rc3 today, yes, I should pick this up myself. Thanks for
the reminder.
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160425/3137f755/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread
* [U-Boot] sf: fix timebase data type in _wait_ready()
2016-04-04 17:03 [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready() Stephen Warren
2016-04-05 0:01 ` Tom Rini
@ 2016-04-26 0:16 ` Tom Rini
1 sibling, 0 replies; 9+ messages in thread
From: Tom Rini @ 2016-04-26 0:16 UTC (permalink / raw)
To: u-boot
On Mon, Apr 04, 2016 at 11:03:52AM -0600, Stephen Warren wrote:
> From: Stephen Warren <swarren@nvidia.com>
>
> get_timer() returns an unsigned 64-bit value, but is currently assigned to
> a signed 32-bit variable. Due to sign extension and data truncation, this
> causes the timeout loop in spi_flash_cmd_wait_ready() to immediately (and
> incorrectly) fire for about 50% of all time values, based on whether bit
> 31 is set. In sandbox at least, this causes the test to pass or fail based
> on system uptime, as opposed to time since the U-Boot binary was started.
>
> Fixes: 4efad20a1751 ("sf: Update status reg check in spi_flash_cmd_wait_ready")
> Signed-off-by: Stephen Warren <swarren@nvidia.com>
> Reviewed-by: Tom Rini <trini@konsulko.com>
> Reviewed-by: Jagan Teki <jteki@openedev.com>
Applied to u-boot/master, thanks!
--
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20160425/3da6bf8a/attachment.sig>
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2016-04-26 0:16 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-04 17:03 [U-Boot] [PATCH] sf: fix timebase data type in _wait_ready() Stephen Warren
2016-04-05 0:01 ` Tom Rini
2016-04-06 11:22 ` Jagan Teki
2016-04-12 15:43 ` Stephen Warren
2016-04-12 16:03 ` Tom Rini
2016-04-21 5:49 ` Stephen Warren
2016-04-25 16:44 ` Stephen Warren
2016-04-25 16:46 ` Tom Rini
2016-04-26 0:16 ` [U-Boot] " Tom Rini
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.