return type of wait_for_completion_timeout is unsigned long not int, this
patch uses the return value of wait_for_completion_timeout in the condition
directly rather than assigning it to an incorrect type variable.
The timeout declaration cleanup is just for readability
Signed-off-by: Nicholas Mc Guire <[email protected]>
---
The variable used for handling the return of wait_for_cmpletion_timeout
was int but should be unsigned long, where it was not in use for anything
else and the return value in case of completion (>0) is not used it was
removed and wait_for_completion_timeout() used directly in the if condition.
To make the timeout values a bit simpler to read and also handle all of
the corner cases correctly the declarations are moved to msecs_to_jiffies().
This patch was only compile tested for pxa3xx_defconfig
(implies CONFIG_MTD_NAND_PXA3xx=y)
Patch is against 3.0.19-rc6 -next-20150130
drivers/mtd/nand/pxa3xx_nand.c | 24 ++++++++++--------------
1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/drivers/mtd/nand/pxa3xx_nand.c b/drivers/mtd/nand/pxa3xx_nand.c
index 96b0b1d..ef6545b 100644
--- a/drivers/mtd/nand/pxa3xx_nand.c
+++ b/drivers/mtd/nand/pxa3xx_nand.c
@@ -38,8 +38,8 @@
#include <linux/platform_data/mtd-nand-pxa3xx.h>
-#define CHIP_DELAY_TIMEOUT (2 * HZ/10)
-#define NAND_STOP_DELAY (2 * HZ/50)
+#define CHIP_DELAY_TIMEOUT msecs_to_jiffies(200)
+#define NAND_STOP_DELAY msecs_to_jiffies(40)
#define PAGE_CHUNK_SIZE (2048)
/*
@@ -915,7 +915,7 @@ static void nand_cmdfunc(struct mtd_info *mtd, unsigned command,
{
struct pxa3xx_nand_host *host = mtd->priv;
struct pxa3xx_nand_info *info = host->info_data;
- int ret, exec_cmd;
+ int exec_cmd;
/*
* if this is a x16 device ,then convert the input
@@ -947,9 +947,8 @@ static void nand_cmdfunc(struct mtd_info *mtd, unsigned command,
info->need_wait = 1;
pxa3xx_nand_start(info);
- ret = wait_for_completion_timeout(&info->cmd_complete,
- CHIP_DELAY_TIMEOUT);
- if (!ret) {
+ if (!wait_for_completion_timeout(&info->cmd_complete,
+ CHIP_DELAY_TIMEOUT)) {
dev_err(&info->pdev->dev, "Wait time out!!!\n");
/* Stop State Machine for next command cycle */
pxa3xx_nand_stop(info);
@@ -964,7 +963,7 @@ static void nand_cmdfunc_extended(struct mtd_info *mtd,
{
struct pxa3xx_nand_host *host = mtd->priv;
struct pxa3xx_nand_info *info = host->info_data;
- int ret, exec_cmd, ext_cmd_type;
+ int exec_cmd, ext_cmd_type;
/*
* if this is a x16 device then convert the input
@@ -1027,9 +1026,8 @@ static void nand_cmdfunc_extended(struct mtd_info *mtd,
init_completion(&info->cmd_complete);
pxa3xx_nand_start(info);
- ret = wait_for_completion_timeout(&info->cmd_complete,
- CHIP_DELAY_TIMEOUT);
- if (!ret) {
+ if (!wait_for_completion_timeout(&info->cmd_complete,
+ CHIP_DELAY_TIMEOUT)) {
dev_err(&info->pdev->dev, "Wait time out!!!\n");
/* Stop State Machine for next command cycle */
pxa3xx_nand_stop(info);
@@ -1162,13 +1160,11 @@ static int pxa3xx_nand_waitfunc(struct mtd_info *mtd, struct nand_chip *this)
{
struct pxa3xx_nand_host *host = mtd->priv;
struct pxa3xx_nand_info *info = host->info_data;
- int ret;
if (info->need_wait) {
- ret = wait_for_completion_timeout(&info->dev_ready,
- CHIP_DELAY_TIMEOUT);
info->need_wait = 0;
- if (!ret) {
+ if (!wait_for_completion_timeout(&info->dev_ready,
+ CHIP_DELAY_TIMEOUT)) {
dev_err(&info->pdev->dev, "Ready time out!!!\n");
return NAND_STATUS_FAIL;
}
--
1.7.10.4
On 02/01/2015 01:55 PM, Nicholas Mc Guire wrote:
> return type of wait_for_completion_timeout is unsigned long not int, this
> patch uses the return value of wait_for_completion_timeout in the condition
> directly rather than assigning it to an incorrect type variable.
>
> The timeout declaration cleanup is just for readability
>
> Signed-off-by: Nicholas Mc Guire <[email protected]>
> ---
>
> The variable used for handling the return of wait_for_cmpletion_timeout
> was int but should be unsigned long, where it was not in use for anything
> else and the return value in case of completion (>0) is not used it was
> removed and wait_for_completion_timeout() used directly in the if condition.
>
> To make the timeout values a bit simpler to read and also handle all of
> the corner cases correctly the declarations are moved to msecs_to_jiffies().
>
Not sure why you decided to put this explanation outside of the commit log.
It looks useful so I'd move it up.
> This patch was only compile tested for pxa3xx_defconfig
> (implies CONFIG_MTD_NAND_PXA3xx=y)
>
The change looks good, but I would like someone to test it on real hardware.
--
Ezequiel Garc?a, Free Electrons
Embedded Linux, Kernel and Android Engineering
http://free-electrons.com
On Mon, 09 Feb 2015, Ezequiel Garcia wrote:
> On 02/01/2015 01:55 PM, Nicholas Mc Guire wrote:
> > return type of wait_for_completion_timeout is unsigned long not int, this
> > patch uses the return value of wait_for_completion_timeout in the condition
> > directly rather than assigning it to an incorrect type variable.
> >
> > The timeout declaration cleanup is just for readability
> >
> > Signed-off-by: Nicholas Mc Guire <[email protected]>
> > ---
> >
> > The variable used for handling the return of wait_for_cmpletion_timeout
> > was int but should be unsigned long, where it was not in use for anything
> > else and the return value in case of completion (>0) is not used it was
> > removed and wait_for_completion_timeout() used directly in the if condition.
> >
> > To make the timeout values a bit simpler to read and also handle all of
> > the corner cases correctly the declarations are moved to msecs_to_jiffies().
> >
>
> Not sure why you decided to put this explanation outside of the commit log.
> It looks useful so I'd move it up.
>
simply because I was not sure about what should go into the log and what not
but this has been clarified by Dan Carpenter - should have it correct in
future patches.
> > This patch was only compile tested for pxa3xx_defconfig
> > (implies CONFIG_MTD_NAND_PXA3xx=y)
> >
>
> The change looks good, but I would like someone to test it on real hardware.
thx!
hofrat