[PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support
Frank Li
Frank.li at nxp.com
Mon Dec 15 11:17:41 PST 2025
On Mon, Dec 15, 2025 at 08:19:10PM +0200, Adrian Hunter wrote:
> On 15/12/2025 19:44, Frank Li wrote:
> > On Mon, Dec 15, 2025 at 07:26:55PM +0200, Adrian Hunter wrote:
> >> On 12/12/2025 19:38, Frank Li wrote:
> >>> On Thu, Dec 11, 2025 at 06:19:56PM +0200, Adrian Hunter wrote:
> >>>> On 11/12/2025 17:40, Frank Li wrote:
> >>>>> On Thu, Dec 11, 2025 at 03:48:05PM +0200, Adrian Hunter wrote:
> >>>>>> Use pci_alloc_irq_vectors() to support all IRQ types. Do not call
> >>>>>> pci_free_irq_vectors() because it is unnecessary when the device is
> >>>>>> managed due to the use of pcim_enable_device().
> >>>>>
> >>>>> pcim_enable_device() add action pcim_disable_device(), I have not see
> >>>>> pci_free_irq_vectors() in pcim_disable_device().
> >>>>>
> >>>>> Can you tell me where call pci_free_irq_vectors?
> >>>>
> >>>> pci_alloc_irq_vectors()
> >>>> pci_alloc_irq_vectors_affinity()
> >>>> __pci_enable_msix_range()
> >>>> pci_setup_msi_context()
> >>>> pcim_setup_msi_release()
> >>>> devm_add_action(pcim_msi_release)
> >>>> :
> >>>> pci_free_irq_vectors()
> >>>> __pci_enable_msi_range()
> >>>> pci_setup_msi_context()
> >>>
> >>> Okay, it should be that pci_alloc_irq_vectors() provide auto free. not
> >>> because pcim_enable_device()?
> >>
> >> No, it is conditional on pci_is_managed(dev) which is true if
> >> pcim_enable_device() has been used.
> >
> > I have not found any document said that,
> >
> > * Managed pci_enable_device(). Device will automatically be disabled on
> > * driver detach.
> >
> > which have not mentioned that it also controller free irqs.
>
> It is not a secret:
Agree, it may well known by someones. But it is not obvious from
description and function name.
Ideally create patch to update document, of course, it should separate
works.
>
> commit 5982a539cdcedcf1a87709c01125e1596eee2c9a
> Author: Salah Triki <salah.triki at gmail.com>
> Date: Sat Jul 19 07:33:36 2025 +0100
>
> accel/amdxdna: Delete pci_free_irq_vectors()
>
> The device is managed so pci_free_irq_vectors() is called automatically
> no need to do it manually.
>
> Reviewed-by: Jacek Lawrynowicz <jacek.lawrynowicz at linux.intel.com>
> Signed-off-by: Salah Triki <salah.triki at gmail.com>
> Signed-off-by: Lizhi Hou <lizhi.hou at amd.com>
> Link: https://lore.kernel.org/r/aHs8QAfUlFeNp7qL@pc
>
> commit 604f7e7777d663033063886b6a5362d0e6092e3a
> Author: Michał Winiarski <michal.winiarski at intel.com>
> Date: Wed Nov 29 22:44:59 2023 +0100
>
> drm/xe/irq: Don't call pci_free_irq_vectors
>
> For devres managed devices, pci_alloc_irq_vectors is also managed (see
> pci_setup_msi_context for reference).
I prefer use this sentence at your commit message.
Frank
> PCI device used by Xe is devres managed (it was enabled with
> pcim_enable_device), which means that calls to pci_free_irq_vectors are
> redundant and can be safely removed.
>
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> Reviewed-by: Matt Roper <matthew.d.roper at intel.com>
> Link: https://lore.kernel.org/r/20231129214509.1174116-4-michal.winiarski@intel.com
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> commit a264cee31f13ae3b8d32b3e53774759afa55361e
> Author: Srinivas Pandruvada <srinivas.pandruvada at linux.intel.com>
> Date: Wed Jun 19 10:21:07 2024 -0700
>
> thermal: intel: int340x: Remove unnecessary calls to free irq
>
> Remove calls to devm_free_irq() and pci_free_irq_vectors().
> They will be called on driver release anyway.
>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada at linux.intel.com>
> Link: https://patch.msgid.link/20240619172109.497639-2-srinivas.pandruvada@linux.intel.com
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki at intel.com>
>
> commit 5e74a4b3ec1816e3bbfd715d46ae29d2508079cb
> Author: Christophe JAILLET <christophe.jaillet at wanadoo.fr>
> Date: Sun Jun 5 22:50:48 2022 +0200
>
> stmmac: intel: Fix an error handling path in intel_eth_pci_probe()
>
> When the managed API is used, there is no need to explicitly call
> pci_free_irq_vectors().
>
> This looks to be a left-over from the commit in the Fixes tag. Only the
> .remove() function had been updated.
>
> So remove this unused function call and update goto label accordingly.
>
> Fixes: 8accc467758e ("stmmac: intel: use managed PCI function on probe and resume")
> Signed-off-by: Christophe JAILLET <christophe.jaillet at wanadoo.fr>
> Reviewed-by: Wong Vee Khee <vee.khee.wong at linux.intel.com>
> Link: https://lore.kernel.org/r/1ac9b6787b0db83b0095711882c55c77c8ea8da0.1654462241.git.christophe.jaillet@wanadoo.fr
> Signed-off-by: Paolo Abeni <pabeni at redhat.com>
>
>
> >
> > I suggest commit message descript whole story or change PCI's document
> > to descript it clearly. I think other person should have similar question
> > if he have not closely follow PCI's change and go through PCI codes.
> >
> > Frank
> >
> >>
> >>>
> >>> The below statement should be wrong.
> >>>
> >>> "Do not call pci_free_irq_vectors() because it is unnecessary when the device is
> >>> managed due to the use of pcim_enable_device()."
> >>>
> >>> Frank
> >>>>
> >>>>>
> >>>>> And move these small patch ahead in serise.
> >>>>>
> >>>>> Frank
> >>>>>>
> >>>>>> Signed-off-by: Adrian Hunter <adrian.hunter at intel.com>
> >>>>>> ---
> >>>>>>
> >>>>>>
> >>>>>> Changes in V2:
> >>>>>> New patch split from "i3c: mipi-i3c-hci-pci: Add support for
> >>>>>> Multi-Bus Instances"
> >>>>>>
> >>>>>>
> >>>>>> drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c | 8 ++++++--
> >>>>>> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>>>>>
> >>>>>> diff --git a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>>>> index 8ade911e3835..0fd3587671e1 100644
> >>>>>> --- a/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>>>> +++ b/drivers/i3c/master/mipi-i3c-hci/mipi-i3c-hci-pci.c
> >>>>>> @@ -203,6 +203,10 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>>>
> >>>>>> pci_set_master(pci);
> >>>>>>
> >>>>>> + ret = pci_alloc_irq_vectors(pci, 1, 1, PCI_IRQ_ALL_TYPES);
> >>>>>> + if (ret < 0)
> >>>>>> + return ret;
> >>>>>> +
> >>>>>> memset(&res, 0, sizeof(res));
> >>>>>>
> >>>>>> res[0].flags = IORESOURCE_MEM;
> >>>>>> @@ -210,8 +214,8 @@ static int mipi_i3c_hci_pci_probe(struct pci_dev *pci,
> >>>>>> res[0].end = pci_resource_end(pci, 0);
> >>>>>>
> >>>>>> res[1].flags = IORESOURCE_IRQ;
> >>>>>> - res[1].start = pci->irq;
> >>>>>> - res[1].end = pci->irq;
> >>>>>> + res[1].start = pci_irq_vector(hci->pci, 0);
> >>>>>> + res[1].end = res[1].start;
> >>>>>>
> >>>>>> dev_id = ida_alloc(&mipi_i3c_hci_pci_ida, GFP_KERNEL);
> >>>>>> if (dev_id < 0)
> >>>>>> --
> >>>>>> 2.51.0
> >>>>>>
> >>>>>>
> >>>>>> --
> >>>>>> linux-i3c mailing list
> >>>>>> linux-i3c at lists.infradead.org
> >>>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>>>
> >>>>
> >>>> --
> >>>> linux-i3c mailing list
> >>>> linux-i3c at lists.infradead.org
> >>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
> >>
> >>
> >> --
> >> linux-i3c mailing list
> >> linux-i3c at lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-i3c
>
>
> --
> linux-i3c mailing list
> linux-i3c at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-i3c
More information about the linux-i3c
mailing list