[PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[Christian Pellegrin]

Option CONFIG_MTD_NAND_S3C2410_CLKSTOP was broken by higher MTD layers
calling select_chip with -1 twice in a row. This patch fixes the problem
by making sure that clk_disable is not called too many times.

Signed-off-by: Christian Pellegrin <chripell@fsfe.org>
---
 drivers/mtd/nand/s3c2410.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
index fa6e9c7..9adecc6 100644
--- a/drivers/mtd/nand/s3c2410.c
+++ b/drivers/mtd/nand/s3c2410.c
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/cpufreq.h>
+#include <linux/bitops.h>
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/nand.h>
@@ -135,6 +136,9 @@ struct s3c2410_nand_info {
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block	freq_transition;
 #endif
+#ifdef CONFIG_MTD_NAND_S3C2410_CLKSTOP
+	unsigned long                   clk_running;
+#endif
 };
 
 /* conversion functions */
@@ -333,7 +337,8 @@ static void s3c2410_nand_select_chip(struct mtd_info *mtd, int chip)
 	nmtd = this->priv;
 	info = nmtd->info;
 
-	if (chip != -1 && allow_clk_stop(info))
+	if (chip != -1 && allow_clk_stop(info) &&
+	    !test_and_set_bit(0, &info->clk_running))
 		clk_enable(info->clk);
 
 	cur = readl(info->sel_reg);
@@ -356,7 +361,8 @@ static void s3c2410_nand_select_chip(struct mtd_info *mtd, int chip)
 
 	writel(cur, info->sel_reg);
 
-	if (chip == -1 && allow_clk_stop(info))
+	if (chip == -1 && allow_clk_stop(info) &&
+	    test_and_clear_bit(0, &info->clk_running))
 		clk_disable(info->clk);
 }
 
-- 
1.5.6.5

[PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[Christian Pellegrin]

Option CONFIG_MTD_NAND_S3C2410_CLKSTOP was broken by higher MTD layers
calling select_chip with -1 twice in a row. This patch fixes the problem
by making sure that clk_disable is not called too many times.

Signed-off-by: Christian Pellegrin <chripell@fsfe.org>
---
 drivers/mtd/nand/s3c2410.c |   10 ++++++++--
 1 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
index fa6e9c7..9adecc6 100644
--- a/drivers/mtd/nand/s3c2410.c
+++ b/drivers/mtd/nand/s3c2410.c
@@ -37,6 +37,7 @@
 #include <linux/slab.h>
 #include <linux/clk.h>
 #include <linux/cpufreq.h>
+#include <linux/bitops.h>
 
 #include <linux/mtd/mtd.h>
 #include <linux/mtd/nand.h>
@@ -135,6 +136,9 @@ struct s3c2410_nand_info {
 #ifdef CONFIG_CPU_FREQ
 	struct notifier_block	freq_transition;
 #endif
+#ifdef CONFIG_MTD_NAND_S3C2410_CLKSTOP
+	unsigned long                   clk_running;
+#endif
 };
 
 /* conversion functions */
@@ -333,7 +337,8 @@ static void s3c2410_nand_select_chip(struct mtd_info *mtd, int chip)
 	nmtd = this->priv;
 	info = nmtd->info;
 
-	if (chip != -1 && allow_clk_stop(info))
+	if (chip != -1 && allow_clk_stop(info) &&
+	    !test_and_set_bit(0, &info->clk_running))
 		clk_enable(info->clk);
 
 	cur = readl(info->sel_reg);
@@ -356,7 +361,8 @@ static void s3c2410_nand_select_chip(struct mtd_info *mtd, int chip)
 
 	writel(cur, info->sel_reg);
 
-	if (chip == -1 && allow_clk_stop(info))
+	if (chip == -1 && allow_clk_stop(info) &&
+	    test_and_clear_bit(0, &info->clk_running))
 		clk_disable(info->clk);
 }
 
-- 
1.5.6.5

Re: [PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[Ben Dooks]

On Tue, Mar 30, 2010 at 10:22:03AM +0200, Christian Pellegrin wrote:
> Option CONFIG_MTD_NAND_S3C2410_CLKSTOP was broken by higher MTD layers
> calling select_chip with -1 twice in a row. This patch fixes the problem
> by making sure that clk_disable is not called too many times.
> 
> Signed-off-by: Christian Pellegrin <chripell@fsfe.org>
> ---
>  drivers/mtd/nand/s3c2410.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
> index fa6e9c7..9adecc6 100644
> --- a/drivers/mtd/nand/s3c2410.c
> +++ b/drivers/mtd/nand/s3c2410.c
> @@ -37,6 +37,7 @@
>  #include <linux/slab.h>
>  #include <linux/clk.h>
>  #include <linux/cpufreq.h>
> +#include <linux/bitops.h>
>  
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/nand.h>
> @@ -135,6 +136,9 @@ struct s3c2410_nand_info {
>  #ifdef CONFIG_CPU_FREQ
>  	struct notifier_block	freq_transition;
>  #endif
> +#ifdef CONFIG_MTD_NAND_S3C2410_CLKSTOP
> +	unsigned long                   clk_running;
> +#endif
>  };
>  
>  /* conversion functions */
> @@ -333,7 +337,8 @@ static void s3c2410_nand_select_chip(struct mtd_info *mtd, int chip)
>  	nmtd = this->priv;
>  	info = nmtd->info;
>  
> -	if (chip != -1 && allow_clk_stop(info))
> +	if (chip != -1 && allow_clk_stop(info) &&
> +	    !test_and_set_bit(0, &info->clk_running))
>  		clk_enable(info->clk);

please let me know why you feel the need to use test_and_set_bit on
a variable that is single use.
  
>  	cur = readl(info->sel_reg);
> @@ -356,7 +361,8 @@ static void s3c2410_nand_select_chip(struct mtd_info *mtd, int chip)
>  
>  	writel(cur, info->sel_reg);
>  
> -	if (chip == -1 && allow_clk_stop(info))
> +	if (chip == -1 && allow_clk_stop(info) &&
> +	    test_and_clear_bit(0, &info->clk_running))
>  		clk_disable(info->clk);
>  }
>  
> -- 
> 1.5.6.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

[PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[Ben Dooks]

On Tue, Mar 30, 2010 at 10:22:03AM +0200, Christian Pellegrin wrote:
> Option CONFIG_MTD_NAND_S3C2410_CLKSTOP was broken by higher MTD layers
> calling select_chip with -1 twice in a row. This patch fixes the problem
> by making sure that clk_disable is not called too many times.
> 
> Signed-off-by: Christian Pellegrin <chripell@fsfe.org>
> ---
>  drivers/mtd/nand/s3c2410.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/mtd/nand/s3c2410.c b/drivers/mtd/nand/s3c2410.c
> index fa6e9c7..9adecc6 100644
> --- a/drivers/mtd/nand/s3c2410.c
> +++ b/drivers/mtd/nand/s3c2410.c
> @@ -37,6 +37,7 @@
>  #include <linux/slab.h>
>  #include <linux/clk.h>
>  #include <linux/cpufreq.h>
> +#include <linux/bitops.h>
>  
>  #include <linux/mtd/mtd.h>
>  #include <linux/mtd/nand.h>
> @@ -135,6 +136,9 @@ struct s3c2410_nand_info {
>  #ifdef CONFIG_CPU_FREQ
>  	struct notifier_block	freq_transition;
>  #endif
> +#ifdef CONFIG_MTD_NAND_S3C2410_CLKSTOP
> +	unsigned long                   clk_running;
> +#endif
>  };
>  
>  /* conversion functions */
> @@ -333,7 +337,8 @@ static void s3c2410_nand_select_chip(struct mtd_info *mtd, int chip)
>  	nmtd = this->priv;
>  	info = nmtd->info;
>  
> -	if (chip != -1 && allow_clk_stop(info))
> +	if (chip != -1 && allow_clk_stop(info) &&
> +	    !test_and_set_bit(0, &info->clk_running))
>  		clk_enable(info->clk);

please let me know why you feel the need to use test_and_set_bit on
a variable that is single use.
  
>  	cur = readl(info->sel_reg);
> @@ -356,7 +361,8 @@ static void s3c2410_nand_select_chip(struct mtd_info *mtd, int chip)
>  
>  	writel(cur, info->sel_reg);
>  
> -	if (chip == -1 && allow_clk_stop(info))
> +	if (chip == -1 && allow_clk_stop(info) &&
> +	    test_and_clear_bit(0, &info->clk_running))
>  		clk_disable(info->clk);
>  }
>  
> -- 
> 1.5.6.5
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

Re: [PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[christian pellegrin]

On Tue, May 4, 2010 at 8:57 AM, Ben Dooks <ben-linux@fluff.org> wrote:
>
> please let me know why you feel the need to use test_and_set_bit on
> a variable that is single use.
>

As I mentioned in the description the select function is not called in
a balanced way by upper layers, so there is a need to count it in the
driver (otherwise you end by calling clk_disable too many times if I
remember well). Perhaps the atomic operations are not needed but I'm
not sure about this so I preferred to take the cautionary approach (I
haven't studied if calls to select are serialized in the upper MTD
layer or even the block layer). Feel free to use a simple integer, but
I confirm that something like this patch is needed in 2.6.33 and
2.6.34 if you want clock stopping enabled.

Thanks,

-- 
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."

[PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[christian pellegrin]

On Tue, May 4, 2010 at 8:57 AM, Ben Dooks <ben-linux@fluff.org> wrote:
>
> please let me know why you feel the need to use test_and_set_bit on
> a variable that is single use.
>

As I mentioned in the description the select function is not called in
a balanced way by upper layers, so there is a need to count it in the
driver (otherwise you end by calling clk_disable too many times if I
remember well). Perhaps the atomic operations are not needed but I'm
not sure about this so I preferred to take the cautionary approach (I
haven't studied if calls to select are serialized in the upper MTD
layer or even the block layer). Feel free to use a simple integer, but
I confirm that something like this patch is needed in 2.6.33 and
2.6.34 if you want clock stopping enabled.

Thanks,

-- 
Christian Pellegrin, see http://www.evolware.org/chri/
"Real Programmers don't play tennis, or any other sport which requires
you to change clothes. Mountain climbing is OK, and Real Programmers
wear their climbing boots to work in case a mountain should suddenly
spring up in the middle of the computer room."

Re: [PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[Ben Dooks]

On Tue, May 04, 2010 at 09:13:41AM +0200, christian pellegrin wrote:
> On Tue, May 4, 2010 at 8:57 AM, Ben Dooks <ben-linux@fluff.org> wrote:
> >
> > please let me know why you feel the need to use test_and_set_bit on
> > a variable that is single use.
> >
> 
> As I mentioned in the description the select function is not called in
> a balanced way by upper layers, so there is a need to count it in the
> driver (otherwise you end by calling clk_disable too many times if I
> remember well). Perhaps the atomic operations are not needed but I'm
> not sure about this so I preferred to take the cautionary approach (I
> haven't studied if calls to select are serialized in the upper MTD
> layer or even the block layer). Feel free to use a simple integer, but
> I confirm that something like this patch is needed in 2.6.33 and
> 2.6.34 if you want clock stopping enabled.

I don't belive that atomic ops are needed, IIRC the mtd layer should
sort that out. As such test_and_set_bit is hardly a count?
 
> Thanks,
> 
> -- 
> Christian Pellegrin, see http://www.evolware.org/chri/
> "Real Programmers don't play tennis, or any other sport which requires
> you to change clothes. Mountain climbing is OK, and Real Programmers
> wear their climbing boots to work in case a mountain should suddenly
> spring up in the middle of the computer room."
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.

[PATCH v2.6.34-rc2] s3c: Fix CONFIG_MTD_NAND_S3C2410_CLKSTOP

[Ben Dooks]

On Tue, May 04, 2010 at 09:13:41AM +0200, christian pellegrin wrote:
> On Tue, May 4, 2010 at 8:57 AM, Ben Dooks <ben-linux@fluff.org> wrote:
> >
> > please let me know why you feel the need to use test_and_set_bit on
> > a variable that is single use.
> >
> 
> As I mentioned in the description the select function is not called in
> a balanced way by upper layers, so there is a need to count it in the
> driver (otherwise you end by calling clk_disable too many times if I
> remember well). Perhaps the atomic operations are not needed but I'm
> not sure about this so I preferred to take the cautionary approach (I
> haven't studied if calls to select are serialized in the upper MTD
> layer or even the block layer). Feel free to use a simple integer, but
> I confirm that something like this patch is needed in 2.6.33 and
> 2.6.34 if you want clock stopping enabled.

I don't belive that atomic ops are needed, IIRC the mtd layer should
sort that out. As such test_and_set_bit is hardly a count?
 
> Thanks,
> 
> -- 
> Christian Pellegrin, see http://www.evolware.org/chri/
> "Real Programmers don't play tennis, or any other sport which requires
> you to change clothes. Mountain climbing is OK, and Real Programmers
> wear their climbing boots to work in case a mountain should suddenly
> spring up in the middle of the computer room."
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.