Add a common struct clk
in [Kernel]
|
Prev: X86:reboot.c Add some dmi entries to pci_reboot_dmi_table.
Next: ext3: fix non-update ctime when changing the file's permission by setfacl
From: Uwe Kleine-König on 11 Jun 2010 06:00 On Fri, Jun 11, 2010 at 11:23:56AM +0200, Lothar Wa�mann wrote: > Hi, > > > > Using a mutex in clk_enable()/clk_disable() is a bad idea, since that > > > makes it impossible to call those functions in interrupt context. IMHO if a device generates an irq its clock should already be on. This way you don't need to enable or disable a clock in irq context. > > Do we do this at the moment? I know at least one implementation of clk_enable > > uses a mutex for locking. > > > You are probably talking about the Freescale i.MX51 kernel, that won't > even boot, if you enable CONFIG_DEBUG_KERNEL, CONFIG_DEBUG_SPINLOCK, > CONFIG_DEBUG_LOCKDEP and CONFIG_DEBUG_SPINLOCK_SLEEP. > The mutex in the clock implementation is one of the reasons. I will have a look into this later today. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K�nig | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Lothar Waßmann on 11 Jun 2010 06:10 Hi, > > > > Using a mutex in clk_enable()/clk_disable() is a bad idea, since that > > > > makes it impossible to call those functions in interrupt context. > IMHO if a device generates an irq its clock should already be on. This > way you don't need to enable or disable a clock in irq context. > You may want to disable a clock in the IRQ handler. The VPU driver in the Freescale BSP for i.MX51 does exactly this. Anyway I don't see any reason for using a mutex here instead of spin_lock_irq_save() as all other implementations do. Lothar Wa�mann -- ___________________________________________________________ Ka-Ro electronics GmbH | Pascalstra�e 22 | D - 52076 Aachen Phone: +49 2408 1402-0 | Fax: +49 2408 1402-10 Gesch�ftsf�hrer: Matthias Kaussen Handelsregistereintrag: Amtsgericht Aachen, HRB 4996 www.karo-electronics.de | info(a)karo-electronics.de ___________________________________________________________ -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jeremy Kerr on 11 Jun 2010 07:00 Hi Lothar, > You may want to disable a clock in the IRQ handler. The VPU driver in > the Freescale BSP for i.MX51 does exactly this. Is this something we can defer to a sleepable context? > Anyway I don't see any reason for using a mutex here instead of > spin_lock_irq_save() as all other implementations do. Just that enable/disable may take an arbitrarily long time to complete. Sounds like that hasn't been a problem in the past though. Cheers, Jeremy -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Uwe Kleine-König on 11 Jun 2010 10:20 Hello Lothar, On Fri, Jun 11, 2010 at 11:58:39AM +0200, Uwe Kleine-K�nig wrote: > > You are probably talking about the Freescale i.MX51 kernel, that won't > > even boot, if you enable CONFIG_DEBUG_KERNEL, CONFIG_DEBUG_SPINLOCK, > > CONFIG_DEBUG_LOCKDEP and CONFIG_DEBUG_SPINLOCK_SLEEP. > > The mutex in the clock implementation is one of the reasons. > I will have a look into this later today. I take this back as this is not a mainline problem. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-K�nig | Industrial Linux Solutions | http://www.pengutronix.de/ | -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Benjamin Herrenschmidt on 12 Jun 2010 01:20
On Fri, 2010-06-11 at 12:08 +0200, Lothar Waßmann wrote: > Hi, > > > > > > Using a mutex in clk_enable()/clk_disable() is a bad idea, since that > > > > > makes it impossible to call those functions in interrupt context. > > IMHO if a device generates an irq its clock should already be on. This > > way you don't need to enable or disable a clock in irq context. > > > You may want to disable a clock in the IRQ handler. The VPU driver in > the Freescale BSP for i.MX51 does exactly this. > Anyway I don't see any reason for using a mutex here instead of > spin_lock_irq_save() as all other implementations do. Because you suddenly make it impossible to sleep inside enable/disable unless I'm mistaken about the implementation details. Some PLLs can need milliseconds to stabilize (especially if they need to be powered up first). Doing that with a lock held is a BAD IDEA. Cheers, Ben. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |