Re: [RFC PATCH 13/13] Remove devres from pci_intx()

On Thu, 10 Oct 2024 11:11:36 +0200
Philipp Stanner [off-list ref] wrote:

On Thu, 2024-10-10 at 11:50 +0300, Dan Carpenter wrote:

quoted

On Wed, Oct 09, 2024 at 10:35:19AM +0200, Philipp Stanner wrote:  

quoted

pci_intx() is a hybrid function which can sometimes be managed
through
devres. This hybrid nature is undesirable.

Since all users of pci_intx() have by now been ported either to
always-managed pcim_intx() or never-managed pci_intx_unmanaged(),
the
devres functionality can be removed from pci_intx().

Consequently, pci_intx_unmanaged() is now redundant, because
pci_intx()
itself is now unmanaged.

Remove the devres functionality from pci_intx(). Remove
pci_intx_unmanaged().
Have all users of pci_intx_unmanaged() call pci_intx().

Signed-off-by: Philipp Stanner <redacted>  

I don't like when we change a function like this but it still
compiles fine.
If someone is working on a driver and hasn't pushed it yet, then it's
probably
supposed to be using the new pcim_intx() but they won't discover that
until they
detect the leaks at runtime.  

There wouldn't be any *leaks*, it's just that the INTx state would not
automatically be restored. BTW the official documentation in its
current state does not hint at pci_intx() doing anything automatically,
but rather actively marks it as deprecated.

But you are right that a hypothetical new driver and OOT drivers could
experience bugs through this change.

quoted

Why not leave the pci_intx_unmanaged() name.  It's ugly and that will
discorage
people from introducing new uses.  

I'd be OK with that. Then we'd have to remove pci_intx() as it has new
users anymore.

Either way should be fine and keep the behavior for existing drivers
identical.

I think Bjorn should express a preference

FWIW, I think pcim_intx() and pci_intx() align better to our naming
convention for devres interfaces.  Would it be sufficient if pci_intx()
triggered a WARN_ON if called for a pci_is_managed() device?  Thanks,

Alex