irq: support IRQ_NESTED_THREAD with non-threaded interrupt handlers
in [Kernel]
|
Prev: [PATCH 07/11] altera_uart: Simplify altera_uart_console_putc
Next: [PATCH] maintainers: Add git trees for SPI and device tree
From: Thomas Gleixner on 8 Jun 2010 03:00 On Mon, 7 Jun 2010, Esben Haabendal wrote: > On Mon, Jun 7, 2010 at 5:06 PM, Thomas Gleixner <tglx(a)linutronix.de> wrote: > > > Maybe you understand now, why I was pretty sure upfront, that your > > approach was wrong even without knowing all the gory details ? :) > > I understand. There is a better solution, which is to use threaded > interrupts where needed. FWIW, it just occured to me, that the only reason why the disable_irq_nosysnc() trips up on the in_atomic() check is your fiddling with the nested irq dispatcher. If you just would have changed the phy driver to request_irq_any_context() it would have simply worked out of the box, without any need to remove buslock and changing genirq code at all. That would not give you the advantage of getting rid of the two additional I2C transfers, but that's nn optimzation not a functional requirement. Thanks, tglx -- 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: Esben Haabendal on 8 Jun 2010 03:30 On Tue, 2010-06-08 at 08:58 +0200, Thomas Gleixner wrote: > On Mon, 7 Jun 2010, Esben Haabendal wrote: > > > On Mon, Jun 7, 2010 at 5:06 PM, Thomas Gleixner <tglx(a)linutronix.de> wrote: > > > > > Maybe you understand now, why I was pretty sure upfront, that your > > > approach was wrong even without knowing all the gory details ? :) > > > > I understand. There is a better solution, which is to use threaded > > interrupts where needed. > > FWIW, it just occured to me, that the only reason why the > disable_irq_nosysnc() trips up on the in_atomic() check is your > fiddling with the nested irq dispatcher. > > If you just would have changed the phy driver to > request_irq_any_context() it would have simply worked out of the box, > without any need to remove buslock and changing genirq code at all. > > That would not give you the advantage of getting rid of the two > additional I2C transfers, but that's nn optimzation not a functional > requirement. Now, that is good news. Thanks! /Esben -- 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: Esben Haabendal on 8 Jun 2010 10:20
On Tue, 2010-06-08 at 01:18 +0200, Thomas Gleixner wrote: > On Mon, 7 Jun 2010, Esben Haabendal wrote: > > On Mon, Jun 7, 2010 at 5:06 PM, Thomas Gleixner <tglx(a)linutronix.de> wrote: > > > > > Maybe you understand now, why I was pretty sure upfront, that your > > > approach was wrong even without knowing all the gory details ? :) > > > > I understand. There is a better solution, which is to use threaded > > interrupts where needed. > > > > But I must confess that I am disappointed that you still fail to see > > how the pca953x > > patch actually eliminates the need for serialization. But I don't > > think there is much > > point in going on about that. > > I return the favour of being disappointed that you are still failing > to accept that there is a fundamental difference between "it works in > a particular case" and semantical correctness under all > circumstances. No reason to do that. I must have misunderstood you then. I understand that the general case, I just never got the point where you understood what I did that (I believe) makes it actually work (not just for now, but really work), in my particular case. > There is no place for "it works in a particular case" in a kernel > which runs on a gazillion of different devices unless you want to > create a complete unmaintainable mess. The only way to avoid this is > to be strict about semantics even if it seems unsensible for a > particular case. I won't argue against. Not at all. > Also I explained it to you in great length, that your patch violates > the semantical guarantees of existing functions, but you ignore that > completely. And I answered you several times, that I understand your point, but > It's Open Source, go wild and change it for your particular case - but > don't expect that I accept patches to the generic irq code which will > cause _me_ predictable trouble as I have to deal with the > fallout. Neither expect, that I ack patches to users of that code > which are semantically wrong. > > Just for the record, the pca953x driver is broken vs. the local state > protection anyway as the GPIO functions are not serialized against the > interrupt controller functions. There is no magic which prevents that, > neither on your system nor on any other. The GPIO function should take > buslock when modifying local state and that's one of the reasons why > buslock it is there and cannot go away. Ok, so the genirq buslock should be held in fx. pca953x_gpio_direction_input and pca953x_gpio_direction_output ? That makes sense in combination with the currently implemented bus_sync_unlock. But I still fail to see why it is needed, or even desired, that the setting of direction should be related to the genirq code at all. I see it as a sane seperation to let the actual gpio code handle the direction setting, and the irq code to handle the (in software) irq mask. Driver and/or board code should be sane enough to set pca953x gpio's as input if/when interrupts from them are needed. And they should do this in advance. And by doing it this way, there should not be any hardware state to serialize, and there actually is no pca953x driver local state either, as the pca953x_chip members are either exclusively managed by gpio or irq code. > I'm anticipating that you are going to tell me that it does not matter > on your particular powerpc combined with your usage pattern (i.e no > GPIO users). And I tell you right away, that I'm not interested in > this answer for well explained reasons. No, I don't tie it to my particular usage pattern. I tell you that I don't need it (serialization) on powerpc because my pca953x powerpc patch have removed the need for shared local state between gpio and irq code. Of-course, for the pca953x driver to be acceptable, it would be nice (or more likely, required) to unify it for other archs also, I just don't see how to do that. That said, I will proceed with the request_any_context_irq() in the phy driver instead :-) Case closed for now. /Esben -- 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/ |