Re: [PATCH] lpfc: add cfg_use_msi test before pci_msi_disable() calls

[James Smart <James.Smart@Emulex.Com>]

Richard,

When we put this in, the kernel code that we inspected allowed for
the call if msi was not enabled (check on dev->msi_enabled), and did
nothing. Thus, we believed it was in the scope of the interface.
kfree does the same kind of thing. Testing on 2.6.21-rc4 on a machine
w/o MSI also results in no issues/messages.

Also - cfg_use_msi doesn't actually mean that a successful pci_msi_enable
had been performed. To accurately track it, we'll need a flag bit, which
we thought we could avoid based on the interface. I'd prefer to leave it
as is, unless we truly are mistaken about the interface or coding style.
Is your platform replacing the pci functions ?

-- james s


Richard Lary wrote:
> From: Richard Lary <rlary@us.ibm.com>
> 
> This patch adds test for phba->cfg_use_msi
> before calls to pci_msi_disable() to prevent
> calls to this function when pci_msi_enable()
> has not been called.
> 
> Signed-off-by: Richard Lary <rlary@us.ibm.com>
> ---
> 
> Calling pci_msi_disable() when pci_msi_enable()
> has not been previously called results in
> console error message when removing lpfc.
> 
> $ modprobe -r lpfc
> error[-3]: getting the number of MSI interrupts for fibre-channel
> 
> Applies to: 2.6.21-rc4-git5
> 
> Index: b/drivers/scsi/lpfc/lpfc_init.c
> ===================================================================
> --- a/drivers/scsi/lpfc/lpfc_init.c
> +++ b/drivers/scsi/lpfc/lpfc_init.c
> @@ -1704,7 +1704,8 @@ out_free_irq:
>  	lpfc_stop_timer(phba);
>  	phba->work_hba_events = 0;
>  	free_irq(phba->pcidev->irq, phba);
> -	pci_disable_msi(phba->pcidev);
> +	if (phba->cfg_use_msi)
> +		pci_disable_msi(phba->pcidev);
>  out_free_sysfs_attr:
>  	lpfc_free_sysfs_attr(phba);
>  out_remove_host:
> @@ -1771,7 +1772,8 @@ lpfc_pci_remove_one(struct pci_dev *pdev
>  
>  	/* Release the irq reservation */
>  	free_irq(phba->pcidev->irq, phba);
> -	pci_disable_msi(phba->pcidev);
> +	if (phba->cfg_use_msi)
> +		pci_disable_msi(phba->pcidev);
>  
>  	lpfc_cleanup(phba, 0);
>  	lpfc_stop_timer(phba);
>