Re: [PATCH net-next v3 2/3] net: stmmac: Add glue driver for Motorcomm YT6801 ethernet controller

On Mon, Dec 08, 2025 at 10:54:36AM +0100, Philipp Stanner wrote:

On Fri, 2025-12-05 at 16:16 -0600, Bjorn Helgaas wrote:

quoted

[+to Philipp, Thomas for MSI devres question]

On Fri, Dec 05, 2025 at 09:34:54AM +0000, Russell King (Oracle) wrote:

quoted

On Fri, Dec 05, 2025 at 05:31:34AM +0000, Yao Zi wrote:

quoted

On Mon, Nov 24, 2025 at 07:06:12PM +0000, Russell King (Oracle) wrote:

quoted

On Mon, Nov 24, 2025 at 04:32:10PM +0000, Yao Zi wrote:

...

quoted

quoted

This looks very non-intuitive, and the documentation for
pci_alloc_irq_vectors() doesn't help:

 * Upon a successful allocation, the caller should use pci_irq_vector()
 * to get the Linux IRQ number to be passed to request_threaded_irq().
 * The driver must call pci_free_irq_vectors() on cleanup.
   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

because if what you say is correct (and it looks like it is) then this
line is blatently incorrect.

True, this line is false. It should probably state "If you didn't
enable your PCI device with pcim_enable_device(), you must call
pci_free_irq_vectors() on cleanup."

If it's not a bug, one could keep the docu that way or at least phrase
it in a way so that no additional users start relying on that hybrid
mechanism.

Thanks for the clarification, would you mind me sending a patch to fix
the description, and also mention the automatic clean-up behavior
shouldn't be relied anymore in new code?

...

The good news is that it's the last remainder of PCI hybrid devres and
getting rid of it would allow for removal of some additional code, too
(e.g., is_enabled bit and pcim_pin_device()).

The bad news is that it's not super trivial to remove. I looked into it
about two times and decided I can't invest that time currently. You
need to go over all drivers again to see who uses pcim_enable_device(),
then add free_irq_vecs() for them all and so on…

Do you think adding an implementation of pcim_alloc_irq_vectors(), that
always call pci_free_irq_vectors() regardless whether the PCI device is
managed, will help the conversion?

This will make it more trival to rewrite drivers depending on the
automatic clean-up behavior: since calling pci_free_irq_vectors()
several times is okay, we could simply change pci_alloc_irq_vectors() to
pcim_alloc_irq_vectors(), without considering where to call
pci_free_irq_vectors().

Introducing pcim_alloc_irq_vectors() will also help newly-introduced
drivers to reduce duplicated code to handle resource clean-up.

If you give me a pointer I can provide a TODO entry. In any case, feel
free to set me as a reviewer!

Regards
Philipp

Regards,
Yao Zi