Hi Thomas, > > > > 1) IRQ_NOTCONNECTED > > > > We do not have this check in drm_irq today and we should avoid spreading > > it all over. We are either carrying it forever or we wil lsee patches > > floating in to drop the check again. > > The current use in the kernel is minimal: > > https://elixir.bootlin.com/linux/latest/A/ident/IRQ_NOTCONNECTED > > > > So as a minimum drop it from atmel_hlcdc and preferably from the rest as > > it is really not used. (Speaking as atmel_hlcdc maintainer) > > I'll drop it from atmel_hlcdc then. > > But saying that it's not used is not correct. My point is the drm_irq do not check this - so adding this check add something there was not needed/done before. > > 2) devm_request_irq() > > > > We are moving towards managed allocation so we do not fail to free > > resources. And an irq has a lifetime equal the device itself - so an > > obvious cnadidate for devm_request_irq. > > If we do not introduce it now we will see a revisit of this later. > > I can be convinced to wait with this as we will have to do much more in > > each driver, but I cannot see any good arguments to avoid the more > > modern way to use devm_request_irq. > > I'll change this in atmel_hdlcd and maybe I can find trivial cases where > devm_request_irq() can be used. But drivers that had an uninstall callback > before should not have the cleanup logic altered by a patch as this one. I > suspect that most of the IRQ cleanup > is actually a vblank cleanup and should be done in response to > drm_vblank_init(). But that's again not something for this patchset here. We > cannot change multiple things at once and still expect any of it to work. > > I welcome the use of devm_ et al. But these changes are better done in a > per-driver patchset that changes all of the driver to managed release. Fair enough, and fine with me. I have yet to read through all patches - will do so in the coming week. Sam
