| Message ID | 20180223153700.2186058-1-arnd@arndb.de |
|---|---|
| State | New |
| Headers | show |
| Series | scsi: lpfc: use memcpy_toio instead of writeq | expand |
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
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();
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