Re: [PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling
On Sun, Feb 01, 2015 at 11:55:37AM -0500, 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 > --- > > 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 Reworked the commit message a bit and pushed to l2-mtd.git. Brian -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling
On Sun, Feb 01, 2015 at 11:55:37AM -0500, 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 hof...@osadl.org --- 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 Reworked the commit message a bit and pushed to l2-mtd.git. Brian -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling
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 > > --- > > > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling
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 > --- > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling
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 hof...@osadl.org --- 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling
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 hof...@osadl.org --- 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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling
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 --- 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 -#defineCHIP_DELAY_TIMEOUT (2 * HZ/10) -#define NAND_STOP_DELAY(2 * HZ/50) +#defineCHIP_DELAY_TIMEOUT msecs_to_jiffies(200) +#define NAND_STOP_DELAYmsecs_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(>cmd_complete, - CHIP_DELAY_TIMEOUT); - if (!ret) { + if (!wait_for_completion_timeout(>cmd_complete, + CHIP_DELAY_TIMEOUT)) { dev_err(>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(>cmd_complete); pxa3xx_nand_start(info); - ret = wait_for_completion_timeout(>cmd_complete, - CHIP_DELAY_TIMEOUT); - if (!ret) { + if (!wait_for_completion_timeout(>cmd_complete, + CHIP_DELAY_TIMEOUT)) { dev_err(>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(>dev_ready, - CHIP_DELAY_TIMEOUT); info->need_wait = 0; - if (!ret) { + if (!wait_for_completion_timeout(>dev_ready, + CHIP_DELAY_TIMEOUT)) { dev_err(>pdev->dev, "Ready time out!!!\n"); return NAND_STATUS_FAIL; } -- 1.7.10.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
[PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling
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 hof...@osadl.org --- 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 -#defineCHIP_DELAY_TIMEOUT (2 * HZ/10) -#define NAND_STOP_DELAY(2 * HZ/50) +#defineCHIP_DELAY_TIMEOUT msecs_to_jiffies(200) +#define NAND_STOP_DELAYmsecs_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 -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/