[U-Boot] [PATCH] sf: fix timebase data type in _wait_ready()
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);
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
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
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.
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!
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?
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?
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.
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!
participants (3)
-
Jagan Teki -
Stephen Warren -
Tom Rini