Skip to contentSkip to conversation

net/irda: add clk_prepare/clk_unprepare to pxaficp_ir

PATCH In review Philipp Zabel 2 participants 3 messages Mar 15, 2012 – Mar 17, 2012

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 net/irda: add clk_prepare/clk_unprepare to pxaficp_ir

This patch adds clk_prepare/clk_unprepare calls to the pxaficp_ir driver by using the helper functions clk_prepare_enable and clk_disable_unprepare.

Cc Eric Miao <[email protected]>
Cc Haojian Zhuang <[email protected]>
Cc Samuel Ortiz <[email protected]>
---
 drivers/net/irda/pxaficp_ir.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
drivers/net/irda/pxaficp_ir.c +3 -3
index 81d5275..ff16daf 100644
@@ -128,20 +128,20 @@ struct pxa_irda {
 static inline void pxa_irda_disable_clk(struct pxa_irda *si)
 {
 	if (si->cur_clk)
-		clk_disable(si->cur_clk);
+		clk_disable_unprepare(si->cur_clk);
 	si->cur_clk = NULL;
 }
 
 static inline void pxa_irda_enable_firclk(struct pxa_irda *si)
 {
 	si->cur_clk = si->fir_clk;
-	clk_enable(si->fir_clk);
+	clk_prepare_enable(si->fir_clk);
 }
 
 static inline void pxa_irda_enable_sirclk(struct pxa_irda *si)
 {
 	si->cur_clk = si->sir_clk;
-	clk_enable(si->sir_clk);
+	clk_prepare_enable(si->sir_clk);
 }
 
 
Signature · 1 line
1.7.9.1
maintainer
#1 lore raw

From: Philipp Zabel <[email protected]>
Date: Thu, 15 Mar 2012 19:19:29 +0100

Quoted 5 lines

This patch adds clk_prepare/clk_unprepare calls to the pxaficp_ir driver by using the helper functions clk_prepare_enable and clk_disable_unprepare.

This is terrible.

So the problem was that clk_enable can't be invoked from atomic context, because it wants to take a mutex for some implementation.

Therefore an existing routine with well defined semantics, clk_enable(), was turned into a NOP.

And this silently breaks drivers.

Instead of silently breaking things, make direct clk_enable()/clk_disable() invocations either result in a compile error or a run-time BUG_ON().

There are other ways this could have been handled as well.

But just making clk_{enable,disable}() essentially stop working was not a very good one at all.

I'll apply this patch to net-next, but I just wanted to point out what an absolutely brain-dead way this was handled.

#2 lore raw
Quoted 14 lines · David Miller

From: Philipp Zabel <[email protected]>
Date: Thu, 15 Mar 2012 19:19:29 +0100

Quoted 5 lines

This patch adds clk_prepare/clk_unprepare calls to the pxaficp_ir driver by using the helper functions clk_prepare_enable and clk_disable_unprepare.

This is terrible.

So the problem was that clk_enable can't be invoked from atomic context, because it wants to take a mutex for some implementation.

Quoted 8 lines

Therefore an existing routine with well defined semantics, clk_enable(), was turned into a NOP.

And this silently breaks drivers.

Instead of silently breaking things, make direct clk_enable()/clk_disable() invocations either result in a compile error or a run-time BUG_ON().

I expect that anybody who turns clk_enable/clk_disable into no-ops for their platform will be sensible enough to take care of this. The common struct clk framework already keeps a prepare_count variable and clk_enable does WARN_ON(clk->prepare_count == 0).

In case of PXA, it's the clk_prepare/clk_unprepare calls that are the no-ops anyway, so thanks for applying.

regards
Philipp