[PATCH v2] McBSP: Fix the free variable update at remove.

[PATCH v2] McBSP: Fix the free variable update at remove.

[shubhrajyoti]

From: Shubhrajyoti D <shubhrajyoti@ti.com>

At remove the free variable is wrongly updated.Attempting to solve the same.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
---
-[v2] Instead of using 0/1 use a macro

 arch/arm/plat-omap/include/plat/mcbsp.h |    3 +++
 arch/arm/plat-omap/mcbsp.c              |   10 +++++-----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
index b87d83c..8988733 100644
--- a/arch/arm/plat-omap/include/plat/mcbsp.h
+++ b/arch/arm/plat-omap/include/plat/mcbsp.h
@@ -37,6 +37,9 @@ static struct platform_device omap_mcbsp##port_nr = {	\
 	.id	= OMAP_MCBSP##port_nr,			\
 }
 
+#define TRUE			1
+#define FALSE			0
+
 #define OMAP7XX_MCBSP1_BASE	0xfffb1000
 #define OMAP7XX_MCBSP2_BASE	0xfffb1800
 
diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index eac4b97..438b3c7 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -755,7 +755,7 @@ int omap_mcbsp_request(unsigned int id)
 		goto err_kfree;
 	}
 
-	mcbsp->free = 0;
+	mcbsp->free = FALSE;
 	mcbsp->reg_cache = reg_cache;
 	spin_unlock(&mcbsp->lock);
 
@@ -815,7 +815,7 @@ err_clk_disable:
 	clk_disable(mcbsp->iclk);
 
 	spin_lock(&mcbsp->lock);
-	mcbsp->free = 1;
+	mcbsp->free = TRUE;
 	mcbsp->reg_cache = NULL;
 err_kfree:
 	spin_unlock(&mcbsp->lock);
@@ -858,7 +858,7 @@ void omap_mcbsp_free(unsigned int id)
 	if (mcbsp->free)
 		dev_err(mcbsp->dev, "McBSP%d was not reserved\n", mcbsp->id);
 	else
-		mcbsp->free = 1;
+		mcbsp->free = TRUE;
 	mcbsp->reg_cache = NULL;
 	spin_unlock(&mcbsp->lock);
 
@@ -1771,7 +1771,7 @@ static int __devinit omap_mcbsp_probe(struct platform_device *pdev)
 
 	spin_lock_init(&mcbsp->lock);
 	mcbsp->id = id + 1;
-	mcbsp->free = 1;
+	mcbsp->free = TRUE;
 	mcbsp->dma_tx_lch = -1;
 	mcbsp->dma_rx_lch = -1;
 
@@ -1845,7 +1845,7 @@ static int __devexit omap_mcbsp_remove(struct platform_device *pdev)
 
 		mcbsp->fclk = NULL;
 		mcbsp->iclk = NULL;
-		mcbsp->free = 0;
+		mcbsp->free = TRUE;
 		mcbsp->dev = NULL;
 	}
 
-- 
1.7.0.4

Re: [PATCH v2] McBSP: Fix the free variable update at remove.

[Jarkko Nikula]

On Tue,  2 Nov 2010 15:54:48 +0530
shubhrajyoti@ti.com wrote:

> From: Shubhrajyoti D <shubhrajyoti@ti.com>
> 
> At remove the free variable is wrongly updated.Attempting to solve the same.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> -[v2] Instead of using 0/1 use a macro
> 
>  arch/arm/plat-omap/include/plat/mcbsp.h |    3 +++
>  arch/arm/plat-omap/mcbsp.c              |   10 +++++-----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-omap/include/plat/mcbsp.h
> index b87d83c..8988733 100644
> --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> @@ -37,6 +37,9 @@ static struct platform_device omap_mcbsp##port_nr = {	\
>  	.id	= OMAP_MCBSP##port_nr,			\
>  }
>  
> +#define TRUE			1
> +#define FALSE			0
> +
Use 'true' & 'false' in the code instead and then there is no need to
redefine these. They are defined in include/linux/stddef.h which is
probably included already by some another file.


-- 
Jarkko

RE: [PATCH v2] McBSP: Fix the free variable update at remove.

[Varadarajan, Charulatha]

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Datta, Shubhrajyoti
> Sent: Tuesday, November 02, 2010 3:55 PM
> To: linux-omap@vger.kernel.org
> Cc: Datta, Shubhrajyoti
> Subject: [PATCH v2] McBSP: Fix the free variable update at remove.
> 
> From: Shubhrajyoti D <shubhrajyoti@ti.com>
> 
> At remove the free variable is wrongly updated.Attempting to solve the
> same.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> ---
> -[v2] Instead of using 0/1 use a macro
> 
>  arch/arm/plat-omap/include/plat/mcbsp.h |    3 +++
>  arch/arm/plat-omap/mcbsp.c              |   10 +++++-----
>  2 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-
> omap/include/plat/mcbsp.h
> index b87d83c..8988733 100644
> --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> @@ -37,6 +37,9 @@ static struct platform_device omap_mcbsp##port_nr = {	\
>  	.id	= OMAP_MCBSP##port_nr,			\
>  }
> 
> +#define TRUE			1
> +#define FALSE			0
> +

Why do you redefine them? They are already available.

>  #define OMAP7XX_MCBSP1_BASE	0xfffb1000
>  #define OMAP7XX_MCBSP2_BASE	0xfffb1800
> 
> diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> index eac4b97..438b3c7 100644
> --- a/arch/arm/plat-omap/mcbsp.c
> +++ b/arch/arm/plat-omap/mcbsp.c
> @@ -755,7 +755,7 @@ int omap_mcbsp_request(unsigned int id)
>  		goto err_kfree;
>  	}
> 
> -	mcbsp->free = 0;
> +	mcbsp->free = FALSE;
>  	mcbsp->reg_cache = reg_cache;
>  	spin_unlock(&mcbsp->lock);
> 
> @@ -815,7 +815,7 @@ err_clk_disable:
>  	clk_disable(mcbsp->iclk);
> 
>  	spin_lock(&mcbsp->lock);
> -	mcbsp->free = 1;
> +	mcbsp->free = TRUE;
>  	mcbsp->reg_cache = NULL;
>  err_kfree:
>  	spin_unlock(&mcbsp->lock);
> @@ -858,7 +858,7 @@ void omap_mcbsp_free(unsigned int id)
>  	if (mcbsp->free)
>  		dev_err(mcbsp->dev, "McBSP%d was not reserved\n", mcbsp->id);
>  	else
> -		mcbsp->free = 1;
> +		mcbsp->free = TRUE;
>  	mcbsp->reg_cache = NULL;
>  	spin_unlock(&mcbsp->lock);
> 
> @@ -1771,7 +1771,7 @@ static int __devinit omap_mcbsp_probe(struct
> platform_device *pdev)
> 
>  	spin_lock_init(&mcbsp->lock);
>  	mcbsp->id = id + 1;
> -	mcbsp->free = 1;
> +	mcbsp->free = TRUE;

During probe this should not be TRUE as it becomes TRUE during 
a omap_mcbsp_request().

The changes should actually make this flag a boolean one and then
do these changes. You might also need to consider changing the code
in places where this flag is being used.
example:
if (mcbsp->free == TRUE)
	....

>  	mcbsp->dma_tx_lch = -1;
>  	mcbsp->dma_rx_lch = -1;
> 
> @@ -1845,7 +1845,7 @@ static int __devexit omap_mcbsp_remove(struct
> platform_device *pdev)
> 
>  		mcbsp->fclk = NULL;
>  		mcbsp->iclk = NULL;
> -		mcbsp->free = 0;
> +		mcbsp->free = TRUE;
>  		mcbsp->dev = NULL;
>  	}
> 
> --
> 1.7.0.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 ] McBSP: Fix the free variable update at remove

[shubhrajyoti]

From: Shubhrajyoti D <shubhrajyoti@ti.com>

At remove the free variable is wrongly updated.Attempting to solve the same.

Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
Reported-by: Vikram Pandita <vikram.pandita@ti.com>
---
-[v2] Instead of using 0/1 use true/false
 arch/arm/plat-omap/mcbsp.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index eac4b97..07c0525 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -755,7 +755,7 @@ int omap_mcbsp_request(unsigned int id)
 		goto err_kfree;
 	}
 
-	mcbsp->free = 0;
+	mcbsp->free = false;
 	mcbsp->reg_cache = reg_cache;
 	spin_unlock(&mcbsp->lock);
 
@@ -815,7 +815,7 @@ err_clk_disable:
 	clk_disable(mcbsp->iclk);
 
 	spin_lock(&mcbsp->lock);
-	mcbsp->free = 1;
+	mcbsp->free = true;
 	mcbsp->reg_cache = NULL;
 err_kfree:
 	spin_unlock(&mcbsp->lock);
@@ -858,7 +858,7 @@ void omap_mcbsp_free(unsigned int id)
 	if (mcbsp->free)
 		dev_err(mcbsp->dev, "McBSP%d was not reserved\n", mcbsp->id);
 	else
-		mcbsp->free = 1;
+		mcbsp->free = true;
 	mcbsp->reg_cache = NULL;
 	spin_unlock(&mcbsp->lock);
 
@@ -1771,7 +1771,7 @@ static int __devinit omap_mcbsp_probe(struct platform_device *pdev)
 
 	spin_lock_init(&mcbsp->lock);
 	mcbsp->id = id + 1;
-	mcbsp->free = 1;
+	mcbsp->free = true;
 	mcbsp->dma_tx_lch = -1;
 	mcbsp->dma_rx_lch = -1;
 
@@ -1845,7 +1845,7 @@ static int __devexit omap_mcbsp_remove(struct platform_device *pdev)
 
 		mcbsp->fclk = NULL;
 		mcbsp->iclk = NULL;
-		mcbsp->free = 0;
+		mcbsp->free = true;
 		mcbsp->dev = NULL;
 	}
 
-- 
1.7.0.4

RE: [PATCH v2] McBSP: Fix the free variable update at remove.

[Datta, Shubhrajyoti]

> -----Original Message-----
> From: Jarkko Nikula [mailto:jhnikula@gmail.com]
> Sent: Tuesday, November 02, 2010 4:03 PM
> To: Datta, Shubhrajyoti
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH v2] McBSP: Fix the free variable update at remove.
> 
> On Tue,  2 Nov 2010 15:54:48 +0530
> shubhrajyoti@ti.com wrote:
> 
> > From: Shubhrajyoti D <shubhrajyoti@ti.com>
> >
> > At remove the free variable is wrongly updated.Attempting to solve the
> same.
> >
> > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> > ---
> > -[v2] Instead of using 0/1 use a macro
> >
> >  arch/arm/plat-omap/include/plat/mcbsp.h |    3 +++
> >  arch/arm/plat-omap/mcbsp.c              |   10 +++++-----
> >  2 files changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/arm/plat-omap/include/plat/mcbsp.h b/arch/arm/plat-
> omap/include/plat/mcbsp.h
> > index b87d83c..8988733 100644
> > --- a/arch/arm/plat-omap/include/plat/mcbsp.h
> > +++ b/arch/arm/plat-omap/include/plat/mcbsp.h
> > @@ -37,6 +37,9 @@ static struct platform_device omap_mcbsp##port_nr = {
> 	\
> >  	.id	= OMAP_MCBSP##port_nr,			\
> >  }
> >
> > +#define TRUE			1
> > +#define FALSE			0
> > +
> Use 'true' & 'false' in the code instead and then there is no need to
> redefine these. They are defined in include/linux/stddef.h which is
> probably included already by some another file.
> 
Yes missed it. Will fix that.
> 
> --
> Jarkko

Re: [PATCH v2 ] McBSP: Fix the free variable update at remove

[Jarkko Nikula]

On Tue,  2 Nov 2010 16:33:34 +0530
shubhrajyoti@ti.com wrote:

> From: Shubhrajyoti D <shubhrajyoti@ti.com>
> 
> At remove the free variable is wrongly updated.Attempting to solve the same.
> 
> Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> Reported-by: Vikram Pandita <vikram.pandita@ti.com>
> ---
> -[v2] Instead of using 0/1 use true/false
>  arch/arm/plat-omap/mcbsp.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
...
> @@ -1845,7 +1845,7 @@ static int __devexit omap_mcbsp_remove(struct
platform_device *pdev)
>  
>  		mcbsp->fclk = NULL;
>  		mcbsp->iclk = NULL;
> -		mcbsp->free = 0;
> +		mcbsp->free = true;
>  		mcbsp->dev = NULL;
>  	}
>  
While reviewing this I noticed that mcbsp->free is not problem at all
but the memory leak as there is no kfree(mcbsp) here and clk_disables
looked suspicious too.

I sent a patch fixing these and your patch changing to use true/false
flags with mcbsp->free looks worth to do as well. Care to send an
updated patch on top of mine (i.e no changes to omap_mcbsp_remove
reguired)?


-- 
Jarkko

RE: [PATCH v2 ] McBSP: Fix the free variable update at remove

[Datta, Shubhrajyoti]

> -----Original Message-----
> From: Jarkko Nikula [mailto:jhnikula@gmail.com]
> Sent: Wednesday, November 03, 2010 2:58 PM
> To: Datta, Shubhrajyoti
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH v2 ] McBSP: Fix the free variable update at remove
> 
> On Tue,  2 Nov 2010 16:33:34 +0530
> shubhrajyoti@ti.com wrote:
> 
> > From: Shubhrajyoti D <shubhrajyoti@ti.com>
> >
> > At remove the free variable is wrongly updated.Attempting to solve the
> same.
> >
> > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com>
> > Reported-by: Vikram Pandita <vikram.pandita@ti.com>
> > ---
> > -[v2] Instead of using 0/1 use true/false
> >  arch/arm/plat-omap/mcbsp.c |   10 +++++-----
> >  1 files changed, 5 insertions(+), 5 deletions(-)
> ...
> > @@ -1845,7 +1845,7 @@ static int __devexit omap_mcbsp_remove(struct
> platform_device *pdev)
> >
> >  		mcbsp->fclk = NULL;
> >  		mcbsp->iclk = NULL;
> > -		mcbsp->free = 0;
> > +		mcbsp->free = true;
> >  		mcbsp->dev = NULL;
> >  	}
> >
> While reviewing this I noticed that mcbsp->free is not problem at all
> but the memory leak as there is no kfree(mcbsp) here and clk_disables
> looked suspicious too.
> 
> I sent a patch fixing these and your patch changing to use true/false
> flags with mcbsp->free looks worth to do as well. Care to send an
> updated patch on top of mine (i.e no changes to omap_mcbsp_remove
> reguired)?

Makes sense. I agree will send a fresh patch.
> 
> 
> --
> Jarkko