scsi: lpfc: use memcpy_toio instead of writeq

Message ID 20180223153700.2186058-1-arnd@arndb.de
State New
Headers show
Series
  • scsi: lpfc: use memcpy_toio instead of writeq
Related show

Commit Message

Arnd Bergmann Feb. 23, 2018, 3:36 p.m.
32-bit architectures generally cannot use writeq(), so we now get a build
failure for the lpfc driver:

drivers/scsi/lpfc/lpfc_sli.c: In function 'lpfc_sli4_wq_put':
drivers/scsi/lpfc/lpfc_sli.c:145:4: error: implicit declaration of function 'writeq'; did you mean 'writeb'? [-Werror=implicit-function-declaration]

Another problem here is that writing out actual data (unlike accessing
mmio registers) means we must write the data with the same endianess
that we have read from memory, but writeq() will perform byte swaps
and add barriers inbetween accesses as we do for registers.

Using memcpy_toio() should do the right thing here, using register
sized stores with correct endianess conversion and barriers (i.e. none),
but on some architectures might fall back to byte-size access.

Side note: shouldn't the driver use ioremap_wc() instead of ioremap()
to get a write-combining mapping on all architectures that support this?

Fixes: 1351e69fc6db ("scsi: lpfc: Add push-to-adapter support to sli4")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/scsi/lpfc/lpfc_sli.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

-- 
2.9.0

Comments

Arnd Bergmann Feb. 23, 2018, 9:02 p.m. | #1
On Fri, Feb 23, 2018 at 4:36 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> @@ -138,12 +137,10 @@ lpfc_sli4_wq_put(struct lpfc_queue *q, union lpfc_wqe *wqe)

>         if (q->phba->sli3_options & LPFC_SLI4_PHWQ_ENABLED)

>                 bf_set(wqe_wqid, &wqe->generic.wqe_com, q->queue_id);

>         lpfc_sli_pcimem_bcopy(wqe, temp_wqe, q->entry_size);

> -       if (q->dpp_enable && q->phba->cfg_enable_dpp) {

> +       if (q->dpp_enable && q->phba->cfg_enable_dpp)

>                 /* write to DPP aperture taking advatage of Combined Writes */

> -               tmp = (uint8_t *)wqe;

> -               for (i = 0; i < q->entry_size; i += sizeof(uint64_t))

> -                       writeq(*((uint64_t *)(tmp + i)), q->dpp_regaddr + i);

> -       }

> +               memcpy_toio(tmp, q->dpp_regaddr, q->entry_size);

> +

>         /* ensure WQE bcopy and DPP flushed before doorbell write */

>         wmb();

>


Not sure where we are with the question of whether memcpy_toio
is a good replacement or not, but further build testing showed that
my patch was completely broken in more than one way:

I mixed up the source and destination arguments, and I used
the uninitialized 'tmp' instead of 'wqe'. Don't try this patch.

       Arnd

Patch

diff --git a/drivers/scsi/lpfc/lpfc_sli.c b/drivers/scsi/lpfc/lpfc_sli.c
index 4ce3ca6f4b79..6749d41753b4 100644
--- a/drivers/scsi/lpfc/lpfc_sli.c
+++ b/drivers/scsi/lpfc/lpfc_sli.c
@@ -115,7 +115,6 @@  lpfc_sli4_wq_put(struct lpfc_queue *q, union lpfc_wqe *wqe)
 	struct lpfc_register doorbell;
 	uint32_t host_index;
 	uint32_t idx;
-	uint32_t i = 0;
 	uint8_t *tmp;
 
 	/* sanity check on queue memory */
@@ -138,12 +137,10 @@  lpfc_sli4_wq_put(struct lpfc_queue *q, union lpfc_wqe *wqe)
 	if (q->phba->sli3_options & LPFC_SLI4_PHWQ_ENABLED)
 		bf_set(wqe_wqid, &wqe->generic.wqe_com, q->queue_id);
 	lpfc_sli_pcimem_bcopy(wqe, temp_wqe, q->entry_size);
-	if (q->dpp_enable && q->phba->cfg_enable_dpp) {
+	if (q->dpp_enable && q->phba->cfg_enable_dpp)
 		/* write to DPP aperture taking advatage of Combined Writes */
-		tmp = (uint8_t *)wqe;
-		for (i = 0; i < q->entry_size; i += sizeof(uint64_t))
-			writeq(*((uint64_t *)(tmp + i)), q->dpp_regaddr + i);
-	}
+		memcpy_toio(tmp, q->dpp_regaddr, q->entry_size);
+
 	/* ensure WQE bcopy and DPP flushed before doorbell write */
 	wmb();