On Tue, May 27, 2014 at 03:04:09PM +0300, Grygorii Strashko wrote: > Hi Mika, > > On 05/27/2014 11:46 AM, Mika Westerberg wrote: > > On Tue, May 27, 2014 at 12:24:56PM +0300, Grygorii Strashko wrote: > >>>> + > >>>> + if (retval) { > >>>> + dev_warn(&pdev->dev, "request irq failed: %d\n", retval); > >>>> + goto out; > >>>> + } > >>>> + > >>>> + retval = gpiochip_add(&cg->chip); > >>>> + if (retval) { > >>>> + dev_warn(&pdev->dev, "add gpio chip error: %d\n", retval); > >>>> + goto out_free_irq; > >>>> + } > >> > >> As to my mind, It'll be better to setup IRQ as last probing step and > >> free it as the first step of driver removing. > > > > When gpiochip_add() is called the chip is exported to outside world. At > > that point anyone can start requesting GPIOs and setup GPIO based > > interrupts. How does that work if you setup the IRQ after you call > > gpiochip_add()? > > > > It's difficult for me to imagine case when GPIO will be accessed > until GPIO driver's probe is finished. Once you call gpiochip_add() your driver gets registered to the GPIO subsystem and advertised outside. It doesn't matter whether your probe function is finished or not. > Regarding remove()/suspend() routines, It's like an axiom for me: > - always disable irq > - always stop all works/threads created by driver > - do everything else > (It's proved by dozens hours of debugging). That's true for remove and suspend, yes but I'm not talking about them. > Anyway, above is just my opinion :) > So, It's up to you, because it's your code :) No it's not, it's Lejun's driver :) -- To unsubscribe from this list: send the line "unsubscribe linux-gpio" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html