Re: [PATCH 01/12] omap: McBSP: Fix possible port lockout

2010-01-10 Thread Janusz Krzysztofik
Friday 08 January 2010 18:14:06 Sergei Shtylyov napisaƂ(a):
> Tony Lindgren wrote:
> > From: Janusz Krzysztofik 
> >
> > In its current form, the omap_mcbsp_request() function can return after
> > irq_request() failure without any cleanups, effectively locking out the
> > port forever with clocks left running. Fix it.
> >
> > Signed-off-by: Janusz Krzysztofik 
> > Acked-by: Jarkko Nikula 
> > Acked-by: Peter Ujfalusi 
> > Signed-off-by: Tony Lindgren 
>
> [...]
>
> > diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
> > index 2cc1cc3..f757672 100644
> > --- a/arch/arm/plat-omap/mcbsp.c
> > +++ b/arch/arm/plat-omap/mcbsp.c
> > @@ -436,7 +436,7 @@ int omap_mcbsp_request(unsigned int id)
> > dev_err(mcbsp->dev, "Unable to request TX IRQ %d "
> > "for McBSP%d\n", mcbsp->tx_irq,
> > mcbsp->id);
> > -   return err;
> > +   goto error;
> > }
> >
> > init_completion(&mcbsp->rx_irq_completion);
> > @@ -446,12 +446,26 @@ int omap_mcbsp_request(unsigned int id)
> > dev_err(mcbsp->dev, "Unable to request RX IRQ %d "
> > "for McBSP%d\n", mcbsp->rx_irq,
> > mcbsp->id);
> > -   free_irq(mcbsp->tx_irq, (void *)mcbsp);
> > -   return err;
> > +   goto tx_irq;
> > }
> > }
> >
> > return 0;
> > +tx_irq:
>
> As if this wasn't a label for error cleanup, i.e. labels could be named
> more consistently, both including 'err' or 'error'...
>
> > +   free_irq(mcbsp->tx_irq, (void *)mcbsp);
> > +error:
> > +   if (mcbsp->pdata && mcbsp->pdata->ops && mcbsp->pdata->ops->free)
> > +   mcbsp->pdata->ops->free(id);
>
> This line is overindented.
>

Sergei,
Thanks for pointing these out.

Tony,
Since you have already sent a pull request covering this patch, I propose I 
will address both issues soon when I revisit the source while further 
modifying this function for McBSP register cache support, OK?

Thanks,
Janusz
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 01/12] omap: McBSP: Fix possible port lockout

2010-01-08 Thread Sergei Shtylyov

Tony Lindgren wrote:

From: Janusz Krzysztofik 



In its current form, the omap_mcbsp_request() function can return after
irq_request() failure without any cleanups, effectively locking out the port
forever with clocks left running. Fix it.



Signed-off-by: Janusz Krzysztofik 
Acked-by: Jarkko Nikula 
Acked-by: Peter Ujfalusi 
Signed-off-by: Tony Lindgren 


[...]


diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 2cc1cc3..f757672 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -436,7 +436,7 @@ int omap_mcbsp_request(unsigned int id)
dev_err(mcbsp->dev, "Unable to request TX IRQ %d "
"for McBSP%d\n", mcbsp->tx_irq,
mcbsp->id);
-   return err;
+   goto error;
}
 
 		init_completion(&mcbsp->rx_irq_completion);

@@ -446,12 +446,26 @@ int omap_mcbsp_request(unsigned int id)
dev_err(mcbsp->dev, "Unable to request RX IRQ %d "
"for McBSP%d\n", mcbsp->rx_irq,
mcbsp->id);
-   free_irq(mcbsp->tx_irq, (void *)mcbsp);
-   return err;
+   goto tx_irq;
}
}
 
 	return 0;

+tx_irq:


   As if this wasn't a label for error cleanup, i.e. labels could be named 
more consistently, both including 'err' or 'error'...



+   free_irq(mcbsp->tx_irq, (void *)mcbsp);
+error:
+   if (mcbsp->pdata && mcbsp->pdata->ops && mcbsp->pdata->ops->free)
+   mcbsp->pdata->ops->free(id);


   This line is overindented.

WBR, Sergei
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 01/12] omap: McBSP: Fix possible port lockout

2010-01-06 Thread Tony Lindgren
From: Janusz Krzysztofik 

In its current form, the omap_mcbsp_request() function can return after
irq_request() failure without any cleanups, effectively locking out the port
forever with clocks left running. Fix it.

Signed-off-by: Janusz Krzysztofik 
Acked-by: Jarkko Nikula 
Acked-by: Peter Ujfalusi 
Signed-off-by: Tony Lindgren 
---
 arch/arm/plat-omap/mcbsp.c |   20 +---
 1 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/arch/arm/plat-omap/mcbsp.c b/arch/arm/plat-omap/mcbsp.c
index 2cc1cc3..f757672 100644
--- a/arch/arm/plat-omap/mcbsp.c
+++ b/arch/arm/plat-omap/mcbsp.c
@@ -436,7 +436,7 @@ int omap_mcbsp_request(unsigned int id)
dev_err(mcbsp->dev, "Unable to request TX IRQ %d "
"for McBSP%d\n", mcbsp->tx_irq,
mcbsp->id);
-   return err;
+   goto error;
}
 
init_completion(&mcbsp->rx_irq_completion);
@@ -446,12 +446,26 @@ int omap_mcbsp_request(unsigned int id)
dev_err(mcbsp->dev, "Unable to request RX IRQ %d "
"for McBSP%d\n", mcbsp->rx_irq,
mcbsp->id);
-   free_irq(mcbsp->tx_irq, (void *)mcbsp);
-   return err;
+   goto tx_irq;
}
}
 
return 0;
+tx_irq:
+   free_irq(mcbsp->tx_irq, (void *)mcbsp);
+error:
+   if (mcbsp->pdata && mcbsp->pdata->ops && mcbsp->pdata->ops->free)
+   mcbsp->pdata->ops->free(id);
+
+   /* Do procedure specific to omap34xx arch, if applicable */
+   omap34xx_mcbsp_free(mcbsp);
+
+   clk_disable(mcbsp->fclk);
+   clk_disable(mcbsp->iclk);
+
+   mcbsp->free = 1;
+
+   return err;
 }
 EXPORT_SYMBOL(omap_mcbsp_request);
 

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html