On Mon, 2008-12-15 at 15:40 +0200, ext Juha Yrjölä wrote: > On Mon, Dec 15, 2008 at 02:08:16PM +0200, Jani Nikula wrote: > > > Add new function omap_update_gpio_switch() to support dynamically > > changing the GPIO switch notify callback functions and debounce > > timeouts. > > Why do they need to be changed? GPIO switches related to the board schematics, > which do not hopefully change dynamically. The switches themselves don't change (nor does this patch support changing them), but different kinds of external devices may be connected to the GPIO swiches. It would be useful to be able to change the notification callbacks and debounce timeouts according to the device. > > + spin_lock_irqsave(&sw->lock, flags); > > if (state) > > timeout = sw->debounce_rising; > > else > > timeout = sw->debounce_falling; > > + spin_unlock_irqrestore(&sw->lock, flags); > > What's the point of this? The above read can be consider an atomic operation. Umm, I suppose I was more worried that the write might not be an atomic operation, messing up the read as well. But I'll take your word for it if you say the write is okay. :) > > + spin_lock_irqsave(&sw->lock, flags); > > + notify = sw->notify; > > + notify_data = sw->notify_data; > > + spin_unlock_irqrestore(&sw->lock, flags); > > + > > + if (notify != NULL) > > + notify(notify_data, state); > > What if omap_update_gpio_switch() is called just before the check for > notify != NULL from another (hypothetical =)) CPU? Then you end up with the > function completing, but still having the old notify callback called with > old notify_data. I was, of course, making sure a NULL pointer is not called and there's no mismatch between old/new notify/notify_data. Your scenario might theoretically actually occur on a single processor system as well, don't you think? But is it actually a problem or not? And if yes, do you have any suggestions as to handling the case? If there's no need to lock for reading the debounce timeouts in gpio_sw_irq_handler() as you say above, do you think I could switch to a mutex and call notify callback holding that? BR, Jani. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
