On 09/24/2013 09:39 AM, Sricharan R wrote: > Hi, > On Monday 23 September 2013 10:37 PM, Tony Lindgren wrote: >> * Javier Martinez Canillas <javier.martinez@xxxxxxxxxxxxxxx> [130923 10:09]: >>> On 09/23/2013 06:45 PM, Tony Lindgren wrote: >>>> Hmm does this still work for legacy platform data based >>>> drivers that are doing gpio_request() first? >>>> >>> Yes it still work when booting using board files. I tested on my OMAP3 board and >>> it worked in both DT and legacy booting mode. >> OK great. >> >>>> And what's the path for clearing things for PM when free_irq() >>>> gets called? It seems that this would leave the GPIO bank >>>> enabled causing a PM regression? >>>> >>> Indeed, I did set bank->mod_usage |= 1 << offset so the bank is enabled if the >>> device goes to suspended and then resumed but I completely forget about the >>> clearing path when the IRQ is freed. >>> >>> Which makes me think that we should probably maintain two usage variables, one >>> for GPIO and another one for IRQ and check both of them on the suspend/resume pm >>> functions. >> Yes that it seems that they should be treated separately. > To understand, why cant the flag be cleared in gpio_irq_shutdown ? Hi Sricharan, Without this patch today drivers do this: gpio_request(gpio, "foo IRQ"); // bank->mod_usage |= 1 << offset gpio_direction_input(gpio); and then request a IRQ with: irq = gpio_to_irq(gpio); request_irq(irq, ...); later on its cleanup path: free_irq(irq, dev); gpio_free(gpio) // bank->mod_usage &= ~(1 << offset); So if you clear the flag on gpio_irq_shutdown then bank module won't be enabled after a suspend making drivers using the GPIO after freeing the IRQ to fail. So the idea is to have something like this: a) Drivers that request both the GPIO and IRQ gpio_request(gpio, "foo IRQ"); // bank->mod_usage |= 1 << offset gpio_direction_input(gpio); irq = gpio_to_irq(gpio); request_irq(irq, ...); // bank->irq_usage |= 1 << offset free_irq(irq, dev); // bank->irq_usage &= ~(1 << offset); gpio_free(gpio) // bank->mod_usage &= ~(1 << offset); b) Drivers that just request the IRQ: irq = gpio_to_irq(gpio); request_irq(irq, ...); // bank->irq_usage |= 1 << offset free_irq(irq, dev); // bank->irq_usage &= ~(1 << offset); So irq_usage or mod_usage is set means that the bank has to be enabled and if both are not set means that the bank module can be disabled. > > Regards, > Sricharan > Best regards, Javier -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
