Re: [PATCH] mtd: pxa3xx_nand: cleanup wait_for_completion handling

2015-03-30 Thread Brian Norris
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

2015-03-30 Thread Brian Norris
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

2015-02-09 Thread Nicholas Mc Guire
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

2015-02-09 Thread Ezequiel Garcia
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

2015-02-09 Thread Ezequiel Garcia
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

2015-02-09 Thread Nicholas Mc Guire
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

2015-02-01 Thread Nicholas Mc Guire
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

2015-02-01 Thread Nicholas Mc Guire
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/