| Message ID | 1459789432-13372-1-git-send-email-swarren@wwwdotorg.org |
|---|---|
| State | Accepted |
| Commit | 11b9a4d8d96674a57ff61ce622b0900ebee1392d |
| Delegated to: | Tom Rini |
| Headers | show |
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!
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);