On Thu, 2024-10-10 at 11:43 -0600, Alex Williamson wrote:

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

quoted

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.

Yup, also my personal preference. But we can mark those functions as
deprecated via docstring-comment. That should fullfill Damien's goal.

  Would it be sufficient if pci_intx()
triggered a WARN_ON if called for a pci_is_managed() device?

No, I don't think that's a good idea; reason being that
pci_is_managed() just checks that global boolean which we inherited
from the old implementation and which should not be necessary with
proper devres.
The boolean is used for making functions such as pci_intx() and
__pci_request_region() hybrid. So with our non-hybrid version we never
need it.

P.

  Thanks,

Alex