Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, Jul 22, 2009 at 12:44:01PM +0200, Thomas Gleixner wrote: On Tue, 21 Jul 2009, Mark Brown wrote: I'll need to have a more detailed look at that but it's not immediately clear to me how a driver (or even machine) should use that code - it looks more like it's intended to be called from within the IRQ infrastructure than from random driver code. All it needs is to set handle_level_oneshot_irq for the interrupt line of your I2C or whatever devices. set_irq_handler(irq, handle_level_oneshot_irq); Yeah, I know - the issue I was having was that the use of set_irq_handler() seemed rather rude for driver code. Grepping around I see that there are actually a small number of drivers using it already but at first glance most of them appear to be implementing interrupt controllers. It was setting off alarm bells about abstraction layer violation like set_irq_type() does. Nothing if the above works, though I guess more documentation wouldn't hurt (and possibly a more friendly wrapper). From the name and Wrapper for what ? Something to package up the set_irq_handler() and request_threaded_irq() (possibly a flag for request_threaded_irq()). This is such a common thing and request_threaded_irq() looks so much like it should Just Work that I'd expect it'll help usability a lot to have a single function which says this is the idiomatic way to implement this. The irq thread finds out which interrupt(s) are active in the device. So it raises the interrupt handlers for those from the thread which will wake up the relevant interrupt threads for those devices. Once all the thread handlers have finished you return from the main thread and the interrupt line gets unmasked again. Yes, this bit of it isn't too much of a problem, it's what's going on in all the non-genirq infrastructures, but... The driver which controls the interrupt device has to expose the demultiplexed interrupts via its own irq_chip implementation. Of course the chip functions like mask/ack/unmask cannot run in atomic context as they require bus access again. ...as you say this is the tricky bit. Here in deed we need to put some thought into common infrastructure as it seems that such excellent hardware designs are becoming more popular :( This isn't a new issue - it's more that you're seeing a greater degree of mainline activity from embedded systems. FWIW the hardware tradeoff here is in a large part down to the fact that many of these devices are heavily size (and therefore pin count) constrained, often on both the device and CPU end of things. Adding more pins would either mean a bigger device or a device that's more expensive to manufacture and use. This pushes to minimal wire, low speed control interfaces and for the sort of low volume control these things need there's not much operational problem there. These things are *normally* either pushing events that either won't happen very often (eg, RTC alarm expiry) or shouldn't happen at all (eg, power failures) and so aren't performance critical. After that bus_sync_unlock() is called outside the atomic context. Now the chip implementation issues the bus commands, waits for completion and unlocks the interrupt controller bus. I'll try to find time to implement some use of it and give it a spin - it looks good at first glance but I'll need to convert one of my drivers to genirq in order to test. Someone working on a chip that already uses genirq might get there first. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
Mark, On Wed, 22 Jul 2009, Mark Brown wrote: On Wed, Jul 22, 2009 at 12:44:01PM +0200, Thomas Gleixner wrote: On Tue, 21 Jul 2009, Mark Brown wrote: I'll need to have a more detailed look at that but it's not immediately clear to me how a driver (or even machine) should use that code - it looks more like it's intended to be called from within the IRQ infrastructure than from random driver code. All it needs is to set handle_level_oneshot_irq for the interrupt line of your I2C or whatever devices. set_irq_handler(irq, handle_level_oneshot_irq); Yeah, I know - the issue I was having was that the use of set_irq_handler() seemed rather rude for driver code. Grepping around I see that there are actually a small number of drivers using it already but at first glance most of them appear to be implementing interrupt controllers. It was setting off alarm bells about abstraction layer violation like set_irq_type() does. I don't think it belongs into the driver code. It belongs into the platform code which sets up the system and knows what's on which interrupt line. Nothing if the above works, though I guess more documentation wouldn't hurt (and possibly a more friendly wrapper). From the name and Wrapper for what ? Something to package up the set_irq_handler() and request_threaded_irq() (possibly a flag for request_threaded_irq()). This is such a common thing and request_threaded_irq() looks so much like it should Just Work that I'd expect it'll help usability a lot to have a single function which says this is the idiomatic way to implement this. Yeah, we might do with a flag, but I'd prefer that the platform code aka arch/xxx/yourmachine/setup.c does this. That's the canonical place. After that bus_sync_unlock() is called outside the atomic context. Now the chip implementation issues the bus commands, waits for completion and unlocks the interrupt controller bus. I'll try to find time to implement some use of it and give it a spin - it looks good at first glance but I'll need to convert one of my drivers to genirq in order to test. Someone working on a chip that already uses genirq might get there first. That'd be great to get some feedback. Both patches need some more thought, but I think they are a good start to do some testing. One thing which I definitely want to change is the thread_eoi function. We probably want to have it customizable for the following reason: main interrupt hardirq handler wakes main thread handler main thread handler bus magic subdevice1 hardirq handler wakes subdevice1 irq thread subdevice2 hardirq handler wakes subdevice2 irq thread main thread handler waits for subdevice1/2 handlers to complete subdevice1 thread handler bus magic thread_fn returns signal main thread handler via completion subdevice2 thread handler bus magic thread_fn returns signal main thread handler via completion main thread handler resumes bus magic main thread handler returns from thread_fn unmask main interrupt So the thread_eoi function is useful for both the main and the subdevice handlers. The main handler probably just issues the unmask while the subdevice handlers probably want to call a completion to notify the main thread handler that they are done. That would be a fairly proper design I think and would encourage driver writers to follow the common scheme instead of doing their own black magic. Thinking further we should even provide some infrastructure for that in the common code which would handle the completion and attach it to the interrupts in question, so the driver authors would not have to deal with that at all. They just would return from thread_fn and let the generic code deal with the notification. The notification has to be set up by the interrupt controller code. That way you can use the same device driver code w/o knowledge of the interrupt controller implementation it is attached to. Thoughts ? Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, 2009-07-22 at 14:58 +0200, Thomas Gleixner wrote: One thing which I definitely want to change is the thread_eoi function. We probably want to have it customizable for the following reason: main interrupt hardirq handler wakes main thread handler main thread handler bus magic subdevice1 hardirq handler wakes subdevice1 irq thread subdevice2 hardirq handler wakes subdevice2 irq thread main thread handler waits for subdevice1/2 handlers to complete subdevice1 thread handler bus magic thread_fn returns signal main thread handler via completion subdevice2 thread handler bus magic thread_fn returns signal main thread handler via completion main thread handler resumes bus magic main thread handler returns from thread_fn unmask main interrupt So the thread_eoi function is useful for both the main and the subdevice handlers. The main handler probably just issues the unmask while the subdevice handlers probably want to call a completion to notify the main thread handler that they are done. That would be a fairly proper design I think and would encourage driver writers to follow the common scheme instead of doing their own black magic. Thinking further we should even provide some infrastructure for that in the common code which would handle the completion and attach it to the interrupts in question, so the driver authors would not have to deal with that at all. They just would return from thread_fn and let the generic code deal with the notification. The notification has to be set up by the interrupt controller code. That way you can use the same device driver code w/o knowledge of the interrupt controller implementation it is attached to. Wouldn't it be better if we could express the nesting property from within genirq, so that we can do things like: register_chip_nested(parent_chip, parent_irq, slave_chip); And let genirq set-up the needed magic to make the nesting work. Also, how important is it that subhandler1..n run in their own thread? That is, can't we let them run from the thread that is otherwise waiting for the completino anyway? -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, Jul 22, 2009 at 02:58:24PM +0200, Thomas Gleixner wrote: On Wed, 22 Jul 2009, Mark Brown wrote: On Wed, Jul 22, 2009 at 12:44:01PM +0200, Thomas Gleixner wrote: set_irq_handler(irq, handle_level_oneshot_irq); Yeah, I know - the issue I was having was that the use of set_irq_handler() seemed rather rude for driver code. Grepping around I see that there I don't think it belongs into the driver code. It belongs into the platform code which sets up the system and knows what's on which interrupt line. Yeah, that is doable - but it'd be good for usability if it could be handled transparently by the driver. Even in board code it's fairly rare to need to explicitly set the handler if you're not actually implementing an interrupt controller. Most of the board-specific code doing this is doing so for a CPLD, FPGA or similar on the board rather than configuring a preexisting IRQ for use by a leaf device. Half the trouble is that if you're not implementing interrupt controller code then calling set_irq_*() is normally a sign that you're doing something wrong (for example, for set_irq_type() you should be using flags on request_irq()). Thinking further we should even provide some infrastructure for that in the common code which would handle the completion and attach it to the interrupts in question, so the driver authors would not have to deal with that at all. They just would return from thread_fn and let the generic code deal with the notification. The notification has to be set up by the interrupt controller code. That way you can use the same device driver code w/o knowledge of the interrupt controller implementation it is attached to. Yes, I think that'll be required for users like gpiolib. If someone's done something like have a button implemented by attaching switch to a GPIO input on for something like jack detect on an audio CODEC or a wake source for a PMIC then we've got generic code that expects to just take a gpio/irq and interact with it. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, 22 Jul 2009, Peter Zijlstra wrote: On Wed, 2009-07-22 at 14:58 +0200, Thomas Gleixner wrote: Thinking further we should even provide some infrastructure for that in the common code which would handle the completion and attach it to the interrupts in question, so the driver authors would not have to deal with that at all. They just would return from thread_fn and let the generic code deal with the notification. The notification has to be set up by the interrupt controller code. That way you can use the same device driver code w/o knowledge of the interrupt controller implementation it is attached to. Wouldn't it be better if we could express the nesting property from within genirq, so that we can do things like: register_chip_nested(parent_chip, parent_irq, slave_chip); And let genirq set-up the needed magic to make the nesting work. Good point. Also, how important is it that subhandler1..n run in their own thread? That is, can't we let them run from the thread that is otherwise waiting for the completino anyway? In those cases I suspect we can do that. I guess there can be async handling as well: the main handler queries the pending interrupts, masks them, wakes the handlers and returns. No wait for all threads to finish necessary before unmasking the main interrupt line. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, 22 Jul 2009, Mark Brown wrote: On Wed, Jul 22, 2009 at 04:18:26PM +0200, Thomas Gleixner wrote: On Wed, 22 Jul 2009, Peter Zijlstra wrote: Also, how important is it that subhandler1..n run in their own thread? That is, can't we let them run from the thread that is otherwise waiting for the completino anyway? In those cases I suspect we can do that. I guess there can be async handling as well: the main handler queries the pending interrupts, masks them, wakes the handlers and returns. No wait for all threads to finish necessary before unmasking the main interrupt line. The chips I'm deling with can certainly support doing that but I'm not sure there'd be enormous advantages - a lot of the interrupt handlers will either be trivial (eg, RTC ticks or alarms) or be serialised by a need to interact with the chip anyway. I'd expect this to be generally true, though ICBW. So in your case it would be possible to run the various subdevice thread_fn handlers from your main interrupt thread one after each other ? Well, that indeed makes it a candidate for a nested structure handling, where your main thread calls handle_nested_irq(irq) which simply runs the thread function of that nested interrupt while keeping the semantics vs. disable/enable ... intact. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
Hi Thomas, On Wed, Jul 22, 2009 at 02:58:24PM +0200, Thomas Gleixner wrote: Mark, On Wed, 22 Jul 2009, Mark Brown wrote: On Wed, Jul 22, 2009 at 12:44:01PM +0200, Thomas Gleixner wrote: On Tue, 21 Jul 2009, Mark Brown wrote: I'll need to have a more detailed look at that but it's not immediately clear to me how a driver (or even machine) should use that code - it looks more like it's intended to be called from within the IRQ infrastructure than from random driver code. All it needs is to set handle_level_oneshot_irq for the interrupt line of your I2C or whatever devices. set_irq_handler(irq, handle_level_oneshot_irq); Yeah, I know - the issue I was having was that the use of set_irq_handler() seemed rather rude for driver code. Grepping around I see that there are actually a small number of drivers using it already but at first glance most of them appear to be implementing interrupt controllers. It was setting off alarm bells about abstraction layer violation like set_irq_type() does. I don't think it belongs into the driver code. It belongs into the platform code which sets up the system and knows what's on which interrupt line. I do think this should be set up by the driver - the platform/arch code can't be 100% certain what model of servicing interrupts driver will chose, nor the driver can know whether arch code set things up for threaded or classic interrupt handling. Since handle_level_oneshot_irq requires drivers to use threaded IRQ model (in absence of thread interrupt will never be unmasked) it would be better if we did set it up automatically, right there in request_threaded_irq(). This would reduce maintenance issues between platform and driver code. Thanks. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, 22 Jul 2009, Dmitry Torokhov wrote: I don't think it belongs into the driver code. It belongs into the platform code which sets up the system and knows what's on which interrupt line. I do think this should be set up by the driver - the platform/arch code can't be 100% certain what model of servicing interrupts driver will chose, nor the driver can know whether arch code set things up for threaded or classic interrupt handling. Since handle_level_oneshot_irq requires drivers to use threaded IRQ model (in absence of thread interrupt will never be unmasked) it would be better if we did set it up automatically, right there in request_threaded_irq(). This would reduce maintenance issues between platform and driver code. No, it's the wrong way round. The platform code sets up the platform devices. So there is no real good reason that the platform code does not know about the details. If it conveys the wrong irq number then it wont work, if it sets the wrong handler it wont work either. So what ? If the driver is implemented to use a threaded handler, which it better is no matter what as it uses a bus, and the interrupt controller logic is proper implemented as well then the driver does not care about those details at all. It will magically work with any interrupt controller you put in front of it. If the platform maintainer sets the wrong handler or the wrong platform data then it's not the driver writers problem. Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tuesday 21 July 2009, Thomas Gleixner wrote: See http://lkml.org/lkml/2009/7/17/174 Interesting. The model is then to switch over to __set_irq_handler(), or maybe set_irq_chip_and_handler(), to replace the toplevel dispatch code for will-be-threaded IRQs which happen to be hooked up to inputs that don't sense edges. (Agree, that seems like a goof. But hardware designers sometimes have any choices there.) The normal model is that boards don't get involved with that level of logic ... all IRQs get set up once on generic code, and flow handlers don't change. Or if they do change, it's done when changing the IRQ's trigger mode from edge to level, or vice versa. Can that be cleaned up a bit, so that the handle_level_oneshot_irq() and unmask_oneshot_irq() stuff kicks in automatically when needed, instead of requiring board-specific (or driver-specific) code to get that stuff right? - Dave -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wednesday 22 July 2009, Peter Zijlstra wrote: Wouldn't it be better if we could express the nesting property from within genirq, so that we can do things like: register_chip_nested(parent_chip, parent_irq, slave_chip); And let genirq set-up the needed magic to make the nesting work. I've been requesting such IRQ chaining support for some time now ... if the ears are now listening, that kind of direction should be pursued. Also, how important is it that subhandler1..n run in their own thread? Completely unimportant in a practical sense. Undesirable, even; wasteful to allocate all those stack pages and keep them idle most of the time. There might be an argument that the design isn't technicaly done until that model *can* be supported. On the flip side, last time this came up there was no customer demand for that ... it was all supplier push. That is, can't we let them run from the thread that is otherwise waiting for the completino anyway? That would be far preferable, yes. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, Jul 22, 2009 at 06:40:21PM +0200, Thomas Gleixner wrote: If the platform maintainer sets the wrong handler or the wrong platform data then it's not the driver writers problem. It becomes the driver writer's problem as soon as the people writing the machine integration start e-mailing saying that the driver is buggy because they've not set this stuff up for the driver, it not being something that they'd expect to have to do for any other interrupt in their system. This isn't such an issue in the PC world there's relatively few people doing the platform development but in the embedded space that's not the case - there are vastly more people doing platform development since essentially everyone building a new device has to do some. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wednesday 22 July 2009, Thomas Gleixner wrote: main interrupt hardirq handler wakes main thread handler main thread handler bus magic subdevice1 hardirq handler wakes subdevice1 irq thread subdevice2 hardirq handler wakes subdevice2 irq thread Why force this wasteful/undesired all subdevices must have their own IRQ handling thread policy? As previously noted, a single thread typically suffices. There's no need to waste a dozen (or so) pages of memory for such purposes ... these events are (as noted most recently by Mark) infrequent/rare and performance isn't a functionality gatekeeper. Plus ... main thread handler waits for subdevice1/2 handlers to complete ... sharing that thread can eliminate that synchronization problem, and simplify the whole process. subdevice1 thread handler bus magic thread_fn returns signal main thread handler via completion subdevice2 thread handler bus magic thread_fn returns signal main thread handler via completion main thread handler resumes bus magic main thread handler returns from thread_fn unmask main interrupt -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wednesday 22 July 2009, Thomas Gleixner wrote: I do think this should be set up by the driver - the platform/arch code can't be 100% certain what model of servicing interrupts driver will chose, nor the driver can know whether arch code set things up for threaded or classic interrupt handling. Since handle_level_oneshot_irq requires drivers to use threaded IRQ model (in absence of thread interrupt will never be unmasked) it would be better if we did set it up automatically, right there in request_threaded_irq(). This would reduce maintenance issues between platform and driver code. No, it's the wrong way round. The platform code sets up the platform devices. So there is no real good reason that the platform code does not know about the details. Except for the development board family of exceptions to such rules ... or the Processor-on-Card model, where the same platform/card gets used in a variety of different chassis configurations, with different peripherals. It may not be possible to know *which* configuration is being used at board setup time. However it most certainly is known by the time a driver is configuring. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tuesday 21 July 2009, Thomas Gleixner wrote: - Multi-function devices like the twl4030 which have an interrupt controller on them and would like to expose that interrupt controller via the generic IRQ subsystem. This was a large part of the discussion in the thread above is a much trickier problem. Why ? Because the current threaded IRQ framework stops short of supporting the relevant IRQ chaining mechanisms. It handles the first level IRQ. Not the second or third level which needs to be dispatched from that one. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, 22 Jul 2009, David Brownell wrote: On Wednesday 22 July 2009, Peter Zijlstra wrote: Wouldn't it be better if we could express the nesting property from within genirq, so that we can do things like: register_chip_nested(parent_chip, parent_irq, slave_chip); And let genirq set-up the needed magic to make the nesting work. I've been requesting such IRQ chaining support for some time now ... if the ears are now listening, that kind of direction should be pursued. Well, I was all ear back then, but the disconnect between embedded only needs X be happy and my repsonsibility to keep that all working for everyone was way larger. :) Also, how important is it that subhandler1..n run in their own thread? Completely unimportant in a practical sense. Undesirable, even; wasteful to allocate all those stack pages and keep them idle most of the time. There might be an argument that the design isn't technicaly done until that model *can* be supported. On the flip side, last time this came up there was no customer demand for that ... it was all supplier push. That is, can't we let them run from the thread that is otherwise waiting for the completino anyway? That would be far preferable, yes. Ok, so let me summarize what we came up with so far. 1) handle_level_oneshot_irq is the correct answer to the problem of those I'm behind a slow bus interrupt controllers. 2) Some mechanism to request ONESHOT from the driver level is required. Preferrably via a flag on request_threaded_irq 3) a function which allows to express the nested thread irq nature of the interrupt controller and its subdevices. 4) a generic serializing mechanism which is implemented via irq_chip functions to solve the chip-mask/unmask issue for the demultiplexed interrupts. Something like the bus_lock/bus_sync_unlock patch I posted earlier. 5) a common function which allows to call the thread handler of the subdevice interrupts in the context of the main thread which takes care of serialization against disable/enable/request/free irq et al. Any more ? Thanks, tglx
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wed, Jul 22, 2009 at 2:04 PM, Thomas Gleixnert...@linutronix.de wrote: On Wed, 22 Jul 2009, David Brownell wrote: On Wednesday 22 July 2009, Peter Zijlstra wrote: Wouldn't it be better if we could express the nesting property from within genirq, so that we can do things like: register_chip_nested(parent_chip, parent_irq, slave_chip); And let genirq set-up the needed magic to make the nesting work. I've been requesting such IRQ chaining support for some time now ... if the ears are now listening, that kind of direction should be pursued. Well, I was all ear back then, but the disconnect between embedded only needs X be happy and my repsonsibility to keep that all working for everyone was way larger. :) Also, how important is it that subhandler1..n run in their own thread? Completely unimportant in a practical sense. Undesirable, even; wasteful to allocate all those stack pages and keep them idle most of the time. There might be an argument that the design isn't technicaly done until that model *can* be supported. On the flip side, last time this came up there was no customer demand for that ... it was all supplier push. That is, can't we let them run from the thread that is otherwise waiting for the completino anyway? That would be far preferable, yes. Ok, so let me summarize what we came up with so far. 1) handle_level_oneshot_irq is the correct answer to the problem of those I'm behind a slow bus interrupt controllers. 2) Some mechanism to request ONESHOT from the driver level is required. Preferrably via a flag on request_threaded_irq 3) a function which allows to express the nested thread irq nature of the interrupt controller and its subdevices. 4) a generic serializing mechanism which is implemented via irq_chip functions to solve the chip-mask/unmask issue for the demultiplexed interrupts. Something like the bus_lock/bus_sync_unlock patch I posted earlier. 5) a common function which allows to call the thread handler of the subdevice interrupts in the context of the main thread which takes care of serialization against disable/enable/request/free irq et al. Any more ? It would also be useful to mask an edge triggered interrupt until the thread handler has finished. The touchscreen on the G1 is connected to an edge triggered interrupt, and the touchscreen may toggle the interrupt line while reading its registers. -- Arve Hjønnevåg -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wednesday 22 July 2009, Thomas Gleixner wrote: Ok, so let me summarize what we came up with so far. 1) handle_level_oneshot_irq is the correct answer to the problem of those I'm behind a slow bus interrupt controllers. Where slow means access needs to sleep ... preventing register access from hardirq contexts. I think you must mean IRQ source not controller; in the examples so far on this thread, the irq_chip in these cases has been a typical SoC/ASIC thing, but the device issuing the IRQ is over I2C/etc. (When the irq_chip itself is across I2C/etc, #3 applies.) 2) Some mechanism to request ONESHOT from the driver level is required. Preferrably via a flag on request_threaded_irq Preferably explicit; a flag implementation suffices. Yes. 3) a function which allows to express the nested thread irq nature of the interrupt controller and its subdevices. That's one possible implementation. Basically, irq chaining should work for threaded IRQs; some irq_chip devices will be across sleeping/slow busses. Some will even chain to another level of irq_chip across such a bus. 4) a generic serializing mechanism which is implemented via irq_chip functions to solve the chip-mask/unmask issue for the demultiplexed interrupts. Something like the bus_lock/bus_sync_unlock patch I posted earlier. In general, all irq_chip methods would need to use the sleeping/slow bus ... like set_type(), and more. That patch somewhat resembles the twl4030_sih_irq_chip stuff. 5) a common function which allows to call the thread handler of the subdevice interrupts in the context of the main thread which takes care of serialization against disable/enable/request/free irq et al. A mechanism like that, yes. ISTR sending a patch a while back with a handle_threaded_irq() flow handler which you'd suggested. I can dig that up if you like, but I suspect you've had more thoughts about it since that time. Any more ? Not that comes quickly to mind. If genirq can do all that, then a lot of drivers/mfd/twl4030-irq.c can vanish ... I mention that as probably the strongest acceptance test that's handy. If you like to work with concrete use cases, that's one. Also, a simpler slow irq_chip device is the mcp23s08 GPIO expander. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Wednesday 22 July 2009, Arve Hjønnevåg wrote: It would also be useful to mask an edge triggered interrupt until the thread handler has finished. The touchscreen on the G1 is connected to an edge triggered interrupt, and the touchscreen may toggle the interrupt line while reading its registers. To clarify: if it does toggle, do you want that event to be queued up so the IRQ is re-issued -- or not? That oneshot mechanism does some of that, though it's for level triggers. Parts of that might be appropriate to handle in the threaded IRQ handler itself. It seems a bit device-specific. - Dave -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
2009/7/22 David Brownell davi...@pacbell.net: On Wednesday 22 July 2009, Arve Hjønnevåg wrote: It would also be useful to mask an edge triggered interrupt until the thread handler has finished. The touchscreen on the G1 is connected to an edge triggered interrupt, and the touchscreen may toggle the interrupt line while reading its registers. To clarify: if it does toggle, do you want that event to be queued up so the IRQ is re-issued -- or not? That oneshot mechanism does some of that, though it's for level triggers. For this driver yes the event should normally be queued up. If there is an error detected though, there is no need to queue up interrupts that are generated during the reinit. Also, if you implement a keypad matrix driver using a threaded interrupt handler, you will not want the interrupt to be re-issues unless it occured after the last key was released since the scan itself always generated more interrupts. This is not specific to threaded interrupt handlers though, and could be handled by adding a clear-interrupt function. Parts of that might be appropriate to handle in the threaded IRQ handler itself. It seems a bit device-specific. Yes, the driver could request a threaded one-shot interrupt so that it will work when it is connected to a level triggered interrupt, and also call disable_interrupt/enable_interrrupt in the handler to avoid extra interrupts when it is connected to an edge triggered source. But, it would be nicer if requesting a one-shot interrupt always works. Also, waiting until the threaded handler runs before masking the interrupt would not work well if the interrupt line needs software debouncing. -- Arve Hjønnevåg -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tue, Jul 21, 2009 at 12:59:25PM +0200, Pavel Machek wrote:
This looks like an unrelated (but useful) change:
*
+ * http://www.synaptics.com/sites/default/files/511_99_01F.pdf
*/
This too:
static void decode_report(struct synaptics_ts_data *ts, u8 *buf)
{
+/*
+ * This sensor sends two 6-byte absolute finger reports, an optional
+ * 2-byte relative report followed by a status byte. This function
+ * reads the two finger reports and transforms the coordinates
Worth splitting them out?
+static irqreturn_t synaptics_ts_hardirq(int irq, void *dev_id)
+{
+ disable_irq_nosync(irq);
+ return IRQ_WAKE_THREAD;
Are you sure that this is going to work? The IRQ thread code will not
call the thread function if the IRQ is marked as disabled so the thread
won't actually get called and the interrupt will just stay disabled (see
irq_thread() in kernel/irq/manage.c). As far as I can see the threaded
IRQ support can't be used for devices on interrupt driven buses that
can't interact with the hardware in hardirq context but I might be
missing something here.
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
Hi Mark,
On Tue, Jul 21, 2009 at 5:06 PM, Mark
Brownbroo...@opensource.wolfsonmicro.com wrote:
On Tue, Jul 21, 2009 at 12:59:25PM +0200, Pavel Machek wrote:
This looks like an unrelated (but useful) change:
*
+ * http://www.synaptics.com/sites/default/files/511_99_01F.pdf
*/
This too:
static void decode_report(struct synaptics_ts_data *ts, u8 *buf)
{
+/*
+ * This sensor sends two 6-byte absolute finger reports, an optional
+ * 2-byte relative report followed by a status byte. This function
+ * reads the two finger reports and transforms the coordinates
Worth splitting them out?
+static irqreturn_t synaptics_ts_hardirq(int irq, void *dev_id)
+{
+ disable_irq_nosync(irq);
+ return IRQ_WAKE_THREAD;
Are you sure that this is going to work? The IRQ thread code will not
call the thread function if the IRQ is marked as disabled so the thread
won't actually get called and the interrupt will just stay disabled (see
irq_thread() in kernel/irq/manage.c). As far as I can see the threaded
IRQ support can't be used for devices on interrupt driven buses that
can't interact with the hardware in hardirq context but I might be
missing something here.
I think threaded irqs are used in USB drivers too, I can't locate the
patches link for that.
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
Hi Mark,
On Tue, Jul 21, 2009 at 5:48 PM, Trilok Sonisoni.tri...@gmail.com wrote:
Hi Mark,
On Tue, Jul 21, 2009 at 5:06 PM, Mark
Brownbroo...@opensource.wolfsonmicro.com wrote:
On Tue, Jul 21, 2009 at 12:59:25PM +0200, Pavel Machek wrote:
This looks like an unrelated (but useful) change:
*
+ * http://www.synaptics.com/sites/default/files/511_99_01F.pdf
*/
This too:
static void decode_report(struct synaptics_ts_data *ts, u8 *buf)
{
+/*
+ * This sensor sends two 6-byte absolute finger reports, an optional
+ * 2-byte relative report followed by a status byte. This function
+ * reads the two finger reports and transforms the coordinates
Worth splitting them out?
+static irqreturn_t synaptics_ts_hardirq(int irq, void *dev_id)
+{
+ disable_irq_nosync(irq);
+ return IRQ_WAKE_THREAD;
Are you sure that this is going to work? The IRQ thread code will not
call the thread function if the IRQ is marked as disabled so the thread
won't actually get called and the interrupt will just stay disabled (see
irq_thread() in kernel/irq/manage.c). As far as I can see the threaded
IRQ support can't be used for devices on interrupt driven buses that
can't interact with the hardware in hardirq context but I might be
missing something here.
I think threaded irqs are used in USB drivers too, I can't locate the
patches link for that.
Hopefully, this thread can give all details about threaded irq discussion.
http://lkml.org/lkml/2009/2/27/255
--
---Trilok Soni
http://triloksoni.wordpress.com
http://www.linkedin.com/in/triloksoni
--
To unsubscribe from this list: send the line unsubscribe linux-i2c in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tue, Jul 21, 2009 at 05:48:40PM +0530, Trilok Soni wrote: I think threaded irqs are used in USB drivers too, I can't locate the patches link for that. The only user of any kind I can see in -next is dm355evm_keys which relies on the IRQ it uses being configured edge triggered so that it doesn't need to disable the interrupt to silence the hard IRQ. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tue, Jul 21, 2009 at 06:00:07PM +0530, Trilok Soni wrote: Hopefully, this thread can give all details about threaded irq discussion. http://lkml.org/lkml/2009/2/27/255 Yes, I'm aware of that - I read it at the time. It seemed to peter out without any satisfactory solution, unfortunately. There's two separate issues here: - Ordinary devices on interrupt driven or slow buses like I2C. These need something along the lines of request_threaded_irq() that's allows them to schedule the main IRQ handler outside hardirq context so that they can interact with the device. They need to do something in hardirq context to disable the interrupt if it's level triggered but most of the time the only option they've got is to disable the IRQ and reenable it when the worker thread is done. This is the issue here. My immediate thought when I noticed this was that we should probably fix request_threaded_irq() so that it's useful for them; I'd been intending to do some digging and try to understand why it is currently implemented as it is. - Multi-function devices like the twl4030 which have an interrupt controller on them and would like to expose that interrupt controller via the generic IRQ subsystem. This was a large part of the discussion in the thread above is a much trickier problem. I've added the folks from Samsung posting the MELFAS MCS-5000 driver to the thread since they're running into the same issue. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tue, Jul 21, 2009 at 01:49:33PM +0100, Mark Brown wrote: On Tue, Jul 21, 2009 at 06:00:07PM +0530, Trilok Soni wrote: Hopefully, this thread can give all details about threaded irq discussion. http://lkml.org/lkml/2009/2/27/255 Yes, I'm aware of that - I read it at the time. It seemed to peter out without any satisfactory solution, unfortunately. There's two separate issues here: - Ordinary devices on interrupt driven or slow buses like I2C. These need something along the lines of request_threaded_irq() that's allows them to schedule the main IRQ handler outside hardirq context so that they can interact with the device. They need to do something in hardirq context to disable the interrupt if it's level triggered but most of the time the only option they've got is to disable the IRQ and reenable it when the worker thread is done. This is the issue here. My immediate thought when I noticed this was that we should probably fix request_threaded_irq() so that it's useful for them; I'd been intending to do some digging and try to understand why it is currently implemented as it is. - Multi-function devices like the twl4030 which have an interrupt controller on them and would like to expose that interrupt controller via the generic IRQ subsystem. This was a large part of the discussion in the thread above is a much trickier problem. I've added the folks from Samsung posting the MELFAS MCS-5000 driver to the thread since they're running into the same issue. Let's also add Thomas and David since I believe they worked on the feature. From my part I would like to have the threaded IRQ available to all drivers since it seens to be hanlding driver shutdown cleanly and without races which is a big plus for me since very few drivers get it right. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tue, 21 Jul 2009, Dmitry Torokhov wrote: On Tue, Jul 21, 2009 at 01:49:33PM +0100, Mark Brown wrote: On Tue, Jul 21, 2009 at 06:00:07PM +0530, Trilok Soni wrote: Hopefully, this thread can give all details about threaded irq discussion. http://lkml.org/lkml/2009/2/27/255 Yes, I'm aware of that - I read it at the time. It seemed to peter out without any satisfactory solution, unfortunately. There's two separate issues here: - Ordinary devices on interrupt driven or slow buses like I2C. These need something along the lines of request_threaded_irq() that's allows them to schedule the main IRQ handler outside hardirq context so that they can interact with the device. They need to do something in hardirq context to disable the interrupt if it's level triggered but most of the time the only option they've got is to disable the IRQ and reenable it when the worker thread is done. This is the issue here. There is already a sane solution to the problem: See http://lkml.org/lkml/2009/7/17/174 My immediate thought when I noticed this was that we should probably fix request_threaded_irq() so that it's useful for them; I'd been intending to do some digging and try to understand why it is currently implemented as it is. What's to fix there ? - Multi-function devices like the twl4030 which have an interrupt controller on them and would like to expose that interrupt controller via the generic IRQ subsystem. This was a large part of the discussion in the thread above is a much trickier problem. Why ? I've added the folks from Samsung posting the MELFAS MCS-5000 driver to the thread since they're running into the same issue. Let's also add Thomas and David since I believe they worked on the feature. From my part I would like to have the threaded IRQ available to all drivers since it seens to be hanlding driver shutdown cleanly and without races which is a big plus for me since very few drivers get it right. :) Thanks, tglx -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
Em Ter, 2009-07-21 às 09:04 -0700, Dmitry Torokhov escreveu: On Tue, Jul 21, 2009 at 01:49:33PM +0100, Mark Brown wrote: - Multi-function devices like the twl4030 which have an interrupt controller on them and would like to expose that interrupt controller via the generic IRQ subsystem. This was a large part of the discussion in the thread above is a much trickier problem. I've added the folks from Samsung posting the MELFAS MCS-5000 driver to the thread since they're running into the same issue. Let's also add Thomas and David since I believe they worked on the feature. Please, keep me on CC too, pcap has an interrupt controller, is connected to SPI bus and exposes its interrupts via generic IRQ subsystem. -- Daniel Ribeiro signature.asc Description: Esta é uma parte de mensagem assinada digitalmente
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tue, Jul 21, 2009 at 10:30:26PM +0200, Thomas Gleixner wrote: On Tue, 21 Jul 2009, Dmitry Torokhov wrote: On Tue, Jul 21, 2009 at 01:49:33PM +0100, Mark Brown wrote: On Tue, Jul 21, 2009 at 06:00:07PM +0530, Trilok Soni wrote: Hopefully, this thread can give all details about threaded irq discussion. http://lkml.org/lkml/2009/2/27/255 Yes, I'm aware of that - I read it at the time. It seemed to peter out without any satisfactory solution, unfortunately. There's two separate issues here: - Ordinary devices on interrupt driven or slow buses like I2C. These need something along the lines of request_threaded_irq() that's allows them to schedule the main IRQ handler outside hardirq context so that they can interact with the device. They need to do something in hardirq context to disable the interrupt if it's level triggered but most of the time the only option they've got is to disable the IRQ and reenable it when the worker thread is done. This is the issue here. There is already a sane solution to the problem: See http://lkml.org/lkml/2009/7/17/174 My immediate thought when I noticed this was that we should probably fix request_threaded_irq() so that it's useful for them; I'd been intending to do some digging and try to understand why it is currently implemented as it is. What's to fix there ? duisable_irq_nosync() in the hard interrupt handler stops the thread handler from running. Unfortunately there are devices where it is the only thing we can do in the hard interrupt. -- Dmitry -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Threaded interrupts for synaptic touchscreen in HTC dream
On Tue, Jul 21, 2009 at 10:30:26PM +0200, Thomas Gleixner wrote: On Tue, 21 Jul 2009, Dmitry Torokhov wrote: On Tue, Jul 21, 2009 at 01:49:33PM +0100, Mark Brown wrote: - Ordinary devices on interrupt driven or slow buses like I2C. These need something along the lines of request_threaded_irq() that's allows them to schedule the main IRQ handler outside hardirq context so that they can interact with the device. They need to do something in There is already a sane solution to the problem: See http://lkml.org/lkml/2009/7/17/174 I'll need to have a more detailed look at that but it's not immediately clear to me how a driver (or even machine) should use that code - it looks more like it's intended to be called from within the IRQ infrastructure than from random driver code. My immediate thought when I noticed this was that we should probably fix request_threaded_irq() so that it's useful for them; I'd been intending to do some digging and try to understand why it is currently implemented as it is. What's to fix there ? Nothing if the above works, though I guess more documentation wouldn't hurt (and possibly a more friendly wrapper). From the name and documentation request_threaded_irq() looks like it should be exactly what's needed. - Multi-function devices like the twl4030 which have an interrupt controller on them and would like to expose that interrupt controller via the generic IRQ subsystem. This was a large part of the discussion in the thread above is a much trickier problem. Why ? Partly just because it's idiomatic for the devices - these things are from a software point of view essentially a small stack of devices glued together and one of the devices on them is an interrupt controller so the natural thing is to want to represent that interrupt controller in the way Linux normally represents interrupt controllers and be able to reuse all the core code rather than having to implement a clone of it. The other part of it is that it gets you all the interfaces for interrupts that the rest of the kernel expects which is needed when the device interacts with others. The biggest issue here is that these devices often have GPIOs on them (especially PMICs and audio CODECs). These have all the facilities one expects of GPIOs, including being used as interrupt sources. If we need to use chip-specific APIs to interact with the interrupts they raise then the drivers for anything using them need to know about those APIs and have special cases to work with them which obviously doesn't scale. From my part I would like to have the threaded IRQ available to all drivers since it seens to be hanlding driver shutdown cleanly and without races which is a big plus for me since very few drivers get it right. :) It's also wasteful having to write the code in every single driver that needs to handle interrupts on interrupt driven buses. -- To unsubscribe from this list: send the line unsubscribe linux-i2c in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
