On 06/11/2023 12:59, Marek Vasut wrote:
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.
I meant chip_9485, not the PCI vendor ID. See how it is used as a lookup
to chip-specific parameters for multiple OCZ and MARVELL SoCs in
mvs_pci_table[] and mvs_chips[]
It does not, see:
$ git grep PCI_VENDOR_ID_OCZ include/
include/linux/pci_ids.h:#define PCI_VENDOR_ID_OCZ 0x1b85
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.
Nope. I only have the one device here.
Checking whether the PCI vendor is PCI_VENDOR_ID_OCZ actually covers
many PCI devices, but they all use chip_9485
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.
Wouldn't it be better to give the legacy IRQ a chance in any case, maybe
those do work on some of the other OCZ devices (or other versions of
firmware) ?
Then according to the change here, we would always call
pci_disable_msi() in removal path for OCZ, regardless of whether the
original pci_enable_msi() call was successful - is that safe and proper?
Thanks,
John
