diff mbox series

[1/5] mpt3sas: Don't change the dma coherent mask after allocations

Message ID 1581416293-41610-2-git-send-email-suganath-prabu.subramani@broadcom.com (mailing list archive)
State Changes Requested
Headers show
Series mpt3sas: Fix changing coherent mask after allocation. | expand

Commit Message

Suganath Prabu S Feb. 11, 2020, 10:18 a.m. UTC 
From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>

During driver load ioc->dma_mask is set to 0. Since this
flag is not set, in _base_config_dma_addressing() driver
always sets 32 bit DMA and later after allocating memory
for RDPQ's the dma mask is set to 64/63 bit from
_base_change_consistent_dma_mask.

Removed Flag ioc->dma_mask and
_base_change_consistent_dma_mask().

Set coherent dma mask to 64/63/32 bit based on
controller at driver load time in _base_config_dma_addressing().
and If 63/64 bit fails attempt again with 32-bit DMA mask.

Signed-off-by: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
---
 drivers/scsi/mpt3sas/mpt3sas_base.c | 82 ++++++++++++++-----------------------
 drivers/scsi/mpt3sas/mpt3sas_base.h |  2 -
 2 files changed, 30 insertions(+), 54 deletions(-)

Comments

Christoph Hellwig Feb. 25, 2020, 6:37 p.m. UTC | #1 
On Tue, Feb 11, 2020 at 05:18:09AM -0500, suganath-prabu.subramani@broadcom.com wrote:
> From: Suganath Prabu S <suganath-prabu.subramani@broadcom.com>
> 
> During driver load ioc->dma_mask is set to 0. Since this
> flag is not set, in _base_config_dma_addressing() driver
> always sets 32 bit DMA and later after allocating memory
> for RDPQ's the dma mask is set to 64/63 bit from
> _base_change_consistent_dma_mask.

Please use up 74 chars for your commit messages.

> +	u64 consistent_dma_mask = DMA_BIT_MASK(64);
>  	if (ioc->is_mcpu_endpoint)

Please keep an empty line after variable declarations.

> +	if (sizeof(dma_addr_t) > 4) {
> +		const u64 required_mask = dma_get_required_mask(&pdev->dev);
> +		if ((required_mask > DMA_BIT_MASK(32)) &&

No need for the braces.

> +		    !pci_set_dma_mask(pdev, consistent_dma_mask) &&
> +		    !pci_set_consistent_dma_mask(pdev,

Please do not use the pci_* functions in new code, but use the
generic DMA ones.

> +		    consistent_dma_mask)) {
> +			ioc->base_add_sg_single = &_base_add_sg_single_64;
> +			ioc->sge_size = sizeof(Mpi2SGESimple64_t);
> +			goto out;
> +		}
> +	}
> +try_32bit:
> +	if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(32))
> +	    && !pci_set_consistent_dma_mask(pdev,

Wrong coding style.  Also there is no need to fall back to a smaller
mask in modern kernels ever.

In other words: please drop this patch and use the one I submitted
weeks ago, as it gets all these things right.
diff mbox series

Patch

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.c b/drivers/scsi/mpt3sas/mpt3sas_base.c
index 663782b..18c5045 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.c
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.c
@@ -2806,55 +2806,42 @@  _base_build_sg_ieee(struct MPT3SAS_ADAPTER *ioc, void *psge,
 static int
 _base_config_dma_addressing(struct MPT3SAS_ADAPTER *ioc, struct pci_dev *pdev)
 {
-	u64 required_mask, coherent_mask;
 	struct sysinfo s;
-	/* Set 63 bit DMA mask for all SAS3 and SAS35 controllers */
-	int dma_mask = (ioc->hba_mpi_version_belonged > MPI2_VERSION) ? 63 : 64;
-
+	char *desc = "64";
+	u64 consistent_dma_mask = DMA_BIT_MASK(64);
 	if (ioc->is_mcpu_endpoint)
 		goto try_32bit;
 
-	required_mask = dma_get_required_mask(&pdev->dev);
-	if (sizeof(dma_addr_t) == 4 || required_mask == 32)
-		goto try_32bit;
-
-	if (ioc->dma_mask)
-		coherent_mask = DMA_BIT_MASK(dma_mask);
-	else
-		coherent_mask = DMA_BIT_MASK(32);
-
-	if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(dma_mask)) ||
-	    dma_set_coherent_mask(&pdev->dev, coherent_mask))
-		goto try_32bit;
-
-	ioc->base_add_sg_single = &_base_add_sg_single_64;
-	ioc->sge_size = sizeof(Mpi2SGESimple64_t);
-	ioc->dma_mask = dma_mask;
-	goto out;
-
- try_32bit:
-	if (dma_set_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(32)))
+	/* Set 63 bit DMA mask for all SAS3 and SAS35 controllers */
+	if (ioc->hba_mpi_version_belonged > MPI2_VERSION) {
+		consistent_dma_mask = DMA_BIT_MASK(63);
+		desc = "63";
+	}
+	if (sizeof(dma_addr_t) > 4) {
+		const u64 required_mask = dma_get_required_mask(&pdev->dev);
+		if ((required_mask > DMA_BIT_MASK(32)) &&
+		    !pci_set_dma_mask(pdev, consistent_dma_mask) &&
+		    !pci_set_consistent_dma_mask(pdev,
+		    consistent_dma_mask)) {
+			ioc->base_add_sg_single = &_base_add_sg_single_64;
+			ioc->sge_size = sizeof(Mpi2SGESimple64_t);
+			goto out;
+		}
+	}
+try_32bit:
+	if (!pci_set_dma_mask(pdev, DMA_BIT_MASK(32))
+	    && !pci_set_consistent_dma_mask(pdev,
+	    DMA_BIT_MASK(32))) {
+		ioc->base_add_sg_single = &_base_add_sg_single_32;
+		ioc->sge_size = sizeof(Mpi2SGESimple32_t);
+		desc = "32";
+	} else
 		return -ENODEV;
-
-	ioc->base_add_sg_single = &_base_add_sg_single_32;
-	ioc->sge_size = sizeof(Mpi2SGESimple32_t);
-	ioc->dma_mask = 32;
- out:
+out:
 	si_meminfo(&s);
-	ioc_info(ioc, "%d BIT PCI BUS DMA ADDRESSING SUPPORTED, total mem (%ld kB)\n",
-		 ioc->dma_mask, convert_to_kb(s.totalram));
-
-	return 0;
-}
-
-static int
-_base_change_consistent_dma_mask(struct MPT3SAS_ADAPTER *ioc,
-				      struct pci_dev *pdev)
-{
-	if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(ioc->dma_mask))) {
-		if (pci_set_consistent_dma_mask(pdev, DMA_BIT_MASK(32)))
-			return -ENODEV;
-	}
+	ioc_info(ioc,
+		"%s BIT PCI BUS DMA ADDRESSING SUPPORTED, total mem (%ld kB)\n",
+		desc, convert_to_kb(s.totalram));
 	return 0;
 }
 
@@ -5169,14 +5156,6 @@  _base_allocate_memory_pools(struct MPT3SAS_ADAPTER *ioc)
 		total_sz += sz;
 	} while (ioc->rdpq_array_enable && (++i < ioc->reply_queue_count));
 
-	if (ioc->dma_mask > 32) {
-		if (_base_change_consistent_dma_mask(ioc, ioc->pdev) != 0) {
-			ioc_warn(ioc, "no suitable consistent DMA mask for %s\n",
-				 pci_name(ioc->pdev));
-			goto out;
-		}
-	}
-
 	ioc->scsiio_depth = ioc->hba_queue_depth -
 	    ioc->hi_priority_depth - ioc->internal_depth;
 
@@ -7158,7 +7137,6 @@  mpt3sas_base_attach(struct MPT3SAS_ADAPTER *ioc)
 	ioc->smp_affinity_enable = smp_affinity_enable;
 
 	ioc->rdpq_array_enable_assigned = 0;
-	ioc->dma_mask = 0;
 	if (ioc->is_aero_ioc)
 		ioc->base_readl = &_base_readl_aero;
 	else
diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index e719715..caae040 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -1026,7 +1026,6 @@  typedef void (*MPT3SAS_FLUSH_RUNNING_CMDS)(struct MPT3SAS_ADAPTER *ioc);
  * @ir_firmware: IR firmware present
  * @bars: bitmask of BAR's that must be configured
  * @mask_interrupts: ignore interrupt
- * @dma_mask: used to set the consistent dma mask
  * @pci_access_mutex: Mutex to synchronize ioctl, sysfs show path and
  *			pci resource handling
  * @fault_reset_work_q_name: fw fault work queue
@@ -1205,7 +1204,6 @@  struct MPT3SAS_ADAPTER {
 	u8		ir_firmware;
 	int		bars;
 	u8		mask_interrupts;
-	int		dma_mask;
 
 	/* fw fault handler */
 	char		fault_reset_work_q_name[20];