mtd: spi-nor: improve wait-till-ready timeout loop
Subsystems (2)
PATCH 1/1 mtd: spi-nor: improve wait-till-ready timeout loop
There are a few small issues with the timeout loop in spi_nor_wait_till_ready():
* The first operation should not be a reschedule; we should check the status register at least once to see if we're complete!
* We should check the status register one last time after declaring the deadline has passed, to prevent a premature timeout erro (this is theoretically possible if we sleep for a long time after the previous status register check).
* Add an error message, so it's obvious if we ever hit a timeout.
--- drivers/mtd/spi-nor/spi-nor.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
drivers/mtd/spi-nor/spi-nor.c +9 -4
@@ -202,19 +202,24 @@ static int spi_nor_ready(struct spi_nor *nor)static int spi_nor_wait_till_ready (struct spi_nor *nor) {unsigned long deadline; -int ret; +int timeout =0 , ret; deadline = jiffies +MAX_READY_WAIT_JIFFIES ; -do { -cond_resched (); +while (!timeout) { +if (time_after_eq (jiffies, deadline)) + timeout =1 ; ret =spi_nor_ready (nor);if (ret <0 )return ret;if (ret)return 0 ; - }while (!time_after_eq (jiffies, deadline)); + +cond_resched (); + } + +dev_err (nor->dev,"flash operation timed out\n" );return -ETIMEDOUT ; }
Signature · 1 line
1.9.1
Quoted 41 lines · Brian Norris
There are a few small issues with the timeout loop in
spi_nor_wait_till_ready():* The first operation should not be a reschedule; we should check the status register at least once to see if we're complete!
* We should check the status register one last time after declaring the deadline has passed, to prevent a premature timeout erro (this is theoretically possible if we sleep for a long time after the previous status register check).
* Add an error message, so it's obvious if we ever hit a timeout.
Signed-off-by Brian Norris <[email protected]>--- drivers/mtd/spi-nor/spi-nor.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)drivers/mtd/spi-nor/spi-nor.c +4
-3@@ -202,19 +202,24 @@ static int spi_nor_ready(struct spi_nor *nor)static int spi_nor_wait_till_ready (struct spi_nor *nor) {unsigned long deadline; -int ret; +int timeout =0 , ret; deadline = jiffies +MAX_READY_WAIT_JIFFIES ; - do { - cond_resched(); +while (!timeout) { +if (time_after_eq (jiffies, deadline)) + timeout =1 ; ret =spi_nor_ready (nor);if (ret <0 )return ret;if (ret)
drivers/mtd/spi-nor/spi-nor.c static int spi_nor_ready(struct spi_nor *nor) · line 225
Quoted 10 lines
drivers/mtd/spi-nor/spi-nor.c +5
-1return 0 ; - }while (!time_after_eq (jiffies, deadline)); + +cond_resched (); + } + +dev_err (nor->dev,"flash operation timed out\n" );return -ETIMEDOUT ; }
look good.
Quoted 42 lines · Brian Norris
There are a few small issues with the timeout loop in
spi_nor_wait_till_ready():* The first operation should not be a reschedule; we should check the status register at least once to see if we're complete!
* We should check the status register one last time after declaring the deadline has passed, to prevent a premature timeout erro (this is theoretically possible if we sleep for a long time after the previous status register check).
* Add an error message, so it's obvious if we ever hit a timeout.
Signed-off-by Brian Norris <[email protected]>--- drivers/mtd/spi-nor/spi-nor.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)drivers/mtd/spi-nor/spi-nor.c +4
-3@@ -202,19 +202,24 @@ static int spi_nor_ready(struct spi_nor *nor)static int spi_nor_wait_till_ready (struct spi_nor *nor) {unsigned long deadline; -int ret; +int timeout =0 , ret; deadline = jiffies +MAX_READY_WAIT_JIFFIES ; - do { - cond_resched(); +while (!timeout) { +if (time_after_eq (jiffies, deadline)) + timeout =1 ; ret =spi_nor_ready (nor);if (ret <0 )return ret;if (ret)
drivers/mtd/spi-nor/spi-nor.c static int spi_nor_ready(struct spi_nor *nor) · line 225
Quoted 10 lines
drivers/mtd/spi-nor/spi-nor.c +5
-1return 0 ; - }while (!time_after_eq (jiffies, deadline)); + +cond_resched (); + } + +dev_err (nor->dev,"flash operation timed out\n" );return -ETIMEDOUT ; }
Looks good to me.
Signature · 2 lines
Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar
Quoted 50 lines · Ezequiel Garcia
Quoted 49 lines · Brian Norris
There are a few small issues with the timeout loop in
spi_nor_wait_till_ready():* The first operation should not be a reschedule; we should check the status register at least once to see if we're complete!
* We should check the status register one last time after declaring the deadline has passed, to prevent a premature timeout erro (this is theoretically possible if we sleep for a long time after the previous status register check).
* Add an error message, so it's obvious if we ever hit a timeout.
Signed-off-by Brian Norris <[email protected]>--- drivers/mtd/spi-nor/spi-nor.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)drivers/mtd/spi-nor/spi-nor.c +9
-4@@ -202,19 +202,24 @@ static int spi_nor_ready(struct spi_nor *nor)static int spi_nor_wait_till_ready (struct spi_nor *nor) {unsigned long deadline; -int ret; +int timeout =0 , ret; deadline = jiffies +MAX_READY_WAIT_JIFFIES ; -do { -cond_resched (); +while (!timeout) { +if (time_after_eq (jiffies, deadline)) + timeout =1 ; ret =spi_nor_ready (nor);if (ret <0 )return ret;if (ret)return 0 ; - }while (!time_after_eq (jiffies, deadline)); + +cond_resched (); + } + +dev_err (nor->dev,"flash operation timed out\n" );
Quoted 8 lines
drivers/mtd/spi-nor/spi-nor.c static int spi_nor_ready(struct spi_nor *nor) · line 225
Quoted 3 lines
drivers/mtd/spi-nor/spi-nor.c +0
-0return -ETIMEDOUT ; }Looks good to me.
Reviewed-by Ezequiel Garcia <[email protected]>
On a second look... I find more readable to use a bool type for booleans, but it's just a silly nitpick.
Signature · 2 lines
Ezequiel Garcia, VanguardiaSur www.vanguardiasur.com.ar
Brian Norris
AppliedQuoted 6 lines · Brian Norris
There are a few small issues with the timeout loop in
spi_nor_wait_till_ready():* The first operation should not be a reschedule; we should check the status register at least once to see if we're complete!
Quoted 8 lines
* We should check the status register one last time after declaring the deadline has passed, to prevent a premature timeout erro (this is theoretically possible if we sleep for a long time after the previous status register check).
* Add an error message, so it's obvious if we ever hit a timeout.
Signed-off-by Brian Norris <[email protected]>
Applied to l2-mtd.git.
Brian