Re: [PATCH V2 06/10] i3c: mipi-i3c-hci-pci: Enable MSI support

[Adrian Hunter <adrian.hunter@intel.com>]

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:

commit 5982a539cdcedcf1a87709c01125e1596eee2c9a
Author: Salah Triki <salah.triki@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@linux.intel.com>
    Signed-off-by: Salah Triki <salah.triki@gmail.com>
    Signed-off-by: Lizhi Hou <lizhi.hou@amd.com>
    Link: https://lore.kernel.org/r/aHs8QAfUlFeNp7qL@pc

commit 604f7e7777d663033063886b6a5362d0e6092e3a
Author: Michał Winiarski <michal.winiarski@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).
    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@intel.com>
    Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
    Link: https://lore.kernel.org/r/20231129214509.1174116-4-michal.winiarski@intel.com
    Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

commit a264cee31f13ae3b8d32b3e53774759afa55361e
Author: Srinivas Pandruvada <srinivas.pandruvada@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@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@intel.com>

commit 5e74a4b3ec1816e3bbfd715d46ae29d2508079cb
Author: Christophe JAILLET <christophe.jaillet@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@wanadoo.fr>
    Reviewed-by: Wong Vee Khee <vee.khee.wong@linux.intel.com>
    Link: https://lore.kernel.org/r/1ac9b6787b0db83b0095711882c55c77c8ea8da0.1654462241.git.christophe.jaillet@wanadoo.fr
    Signed-off-by: Paolo Abeni <pabeni@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@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@lists.infradead.org
>>>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>>>
>>>>
>>>> --
>>>> linux-i3c mailing list
>>>> linux-i3c@lists.infradead.org
>>>> http://lists.infradead.org/mailman/listinfo/linux-i3c
>>
>>
>> --
>> linux-i3c mailing list
>> linux-i3c@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-i3c


-- 
linux-i3c mailing list
linux-i3c@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-i3c