Re: [PATCH 00/14] drm: Make DRM's IRQ helpers legacy

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux