Re: [PATCH v2] scsi: mvsas: Try to enable MSI

[John Garry <john.g.garry@oracle.com>]

> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Note that the "PCI-E x0, Bandwidth Usage: UnKnown Gbps" is due to QEMU
>       vfio-pci VT-d passthrough, for some reason this is what it reports.
>       The issue with PCI MSI happens on real hardware too, this vfio/VT-d
>       is just debugging convenience.
> Note that this would be nice to have in stable series, but I'm reluctant
>       to ask for that in order to avoid breaking other peoples' machines.
>       Maybe a default-off kernel parameter for the mvsas module would be
>       acceptable for stable?
> ---
> Cc: "James E.J. Bottomley" <jejb@linux.ibm.com>
> Cc: "Martin K. Petersen" <martin.petersen@oracle.com>
> Cc: Bart Van Assche <bvanassche@acm.org>
> Cc: Damien Le Moal <dlemoal@kernel.org>
> Cc: Jason Yan <yanaijie@huawei.com>
> Cc: John Garry <john.g.garry@oracle.com>
> Cc: linux-scsi@vger.kernel.org
> ---
> V2: - Limit the MSI enablement to OCZ devices only
> ---
>   drivers/scsi/mvsas/mv_init.c | 17 +++++++++++++++++
>   1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/scsi/mvsas/mv_init.c b/drivers/scsi/mvsas/mv_init.c
> index 43ebb331e2167..d3b1cee6b3252 100644
> --- a/drivers/scsi/mvsas/mv_init.c
> +++ b/drivers/scsi/mvsas/mv_init.c
> @@ -571,6 +571,17 @@ static int mvs_pci_init(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	rc = sas_register_ha(SHOST_TO_SAS_HA(shost));
>   	if (rc)
>   		goto err_out_shost;
> +
> +	/* Try to enable MSI, this is needed at least on OCZ RevoDrive 3 X2 */
> +	if (pdev->vendor == PCI_VENDOR_ID_OCZ) {

PCI_VENDOR_ID_OCZ means 9485. So how about enable MSI for all PCI device 
IDs which use that, which is all OCZ and MARVELL_EXT? I could not get my 
hands on a datasheet for that SoC (could you?), but since all previous 
generations supported MSI, I think that it's a safe bet.

Then, if we do that, instead of repeating this same vendor check, how 
about add a new member to mvs_chip_info to flag whether we need to try 
MSI? For example, it could be mvs_chip_info.use_msi .

> +		rc = pci_enable_msi(mvi->pdev);
> +		if (rc) {
> +			dev_err(&mvi->pdev->dev,
> +				"mvsas: Failed to enable MSI for OCZ device, attached drives may not be detected. rc=%d\n",
> +				rc);

We should fail to load the driver in this case.

> +		}
> +	}
> +
>   	rc = request_irq(pdev->irq, irq_handler, IRQF_SHARED,
>   		DRV_NAME, SHOST_TO_SAS_HA(shost));
>   	if (rc)
> @@ -583,6 +594,9 @@ static int mvs_pci_init(struct pci_dev *pdev, const struct pci_device_id *ent)
>   	return 0;
>   
>   err_not_sas:
> +	if (pdev->vendor == PCI_VENDOR_ID_OCZ)
> +		pci_disable_msi(mvi->pdev);
> +
>   	sas_unregister_ha(SHOST_TO_SAS_HA(shost));
>   err_out_shost:
>   	scsi_remove_host(mvi->shost);
> @@ -607,6 +621,9 @@ static void mvs_pci_remove(struct pci_dev *pdev)
>   	tasklet_kill(&((struct mvs_prv_info *)sha->lldd_ha)->mv_tasklet);
>   #endif
>   
> +	if (pdev->vendor == PCI_VENDOR_ID_OCZ)
> +		pci_disable_msi(mvi->pdev);
> +
>   	sas_unregister_ha(sha);
>   	sas_remove_host(mvi->shost);
>