| Message ID | 20220217132956.484818-15-damien.lemoal@opensource.wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libsas and pm8001 fixes | expand |
On Thu, Feb 17, 2022 at 2:30 PM Damien Le Moal <damien.lemoal@opensource.wdc.com> wrote: > > In the pm8001_chip_sata_req() and pm80xx_chip_sata_req() functions, all > tasks with a DMA direction of DMA_NONE (no data transfer) are > initialized using the ATAP value 0x04. However, NCQ NON DATA commands, > while being DMA_NONE commands are NCQ commands and need to be > initialized using the value 0x07 for ATAP, similarly to other NCQ > commands. > > Make sure that NCQ NON DATA command tasks are initialized similarly to > other NCQ commands by also testing the task "use_ncq" field in addition > to the DMA direction. While at it, reorganize the code into a chain of > if - else if - else to avoid useless affectations and debug messages. > > Fixes: dbf9bfe61571 ("[SCSI] pm8001: add SAS/SATA HBA driver") > Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Reviewed-by: Jack Wang <jinpu.wang@ionos.com> thx! > --- > drivers/scsi/pm8001/pm8001_hwi.c | 14 +++++++------- > drivers/scsi/pm8001/pm80xx_hwi.c | 13 ++++++------- > 2 files changed, 13 insertions(+), 14 deletions(-) > > diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c > index e20a1d4db026..c0215a35a7b5 100644 > --- a/drivers/scsi/pm8001/pm8001_hwi.c > +++ b/drivers/scsi/pm8001/pm8001_hwi.c > @@ -4265,22 +4265,22 @@ static int pm8001_chip_sata_req(struct pm8001_hba_info *pm8001_ha, > u32 opc = OPC_INB_SATA_HOST_OPSTART; > memset(&sata_cmd, 0, sizeof(sata_cmd)); > circularQ = &pm8001_ha->inbnd_q_tbl[0]; > - if (task->data_dir == DMA_NONE) { > + > + if (task->data_dir == DMA_NONE && !task->ata_task.use_ncq) { > ATAP = 0x04; /* no data*/ > pm8001_dbg(pm8001_ha, IO, "no data\n"); > } else if (likely(!task->ata_task.device_control_reg_update)) { > - if (task->ata_task.dma_xfer) { > + if (task->ata_task.use_ncq && > + dev->sata_dev.class != ATA_DEV_ATAPI) { > + ATAP = 0x07; /* FPDMA */ > + pm8001_dbg(pm8001_ha, IO, "FPDMA\n"); > + } else if (task->ata_task.dma_xfer) { > ATAP = 0x06; /* DMA */ > pm8001_dbg(pm8001_ha, IO, "DMA\n"); > } else { > ATAP = 0x05; /* PIO*/ > pm8001_dbg(pm8001_ha, IO, "PIO\n"); > } > - if (task->ata_task.use_ncq && > - dev->sata_dev.class != ATA_DEV_ATAPI) { > - ATAP = 0x07; /* FPDMA */ > - pm8001_dbg(pm8001_ha, IO, "FPDMA\n"); > - } > } > if (task->ata_task.use_ncq && pm8001_get_ncq_tag(task, &hdr_tag)) { > task->ata_task.fis.sector_count |= (u8) (hdr_tag << 3); > diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c > index 60c305f987b5..3deb89b11d2f 100644 > --- a/drivers/scsi/pm8001/pm80xx_hwi.c > +++ b/drivers/scsi/pm8001/pm80xx_hwi.c > @@ -4546,22 +4546,21 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha, > q_index = (u32) (cpu_id) % (pm8001_ha->max_q_num); > circularQ = &pm8001_ha->inbnd_q_tbl[q_index]; > > - if (task->data_dir == DMA_NONE) { > + if (task->data_dir == DMA_NONE && !task->ata_task.use_ncq) { > ATAP = 0x04; /* no data*/ > pm8001_dbg(pm8001_ha, IO, "no data\n"); > } else if (likely(!task->ata_task.device_control_reg_update)) { > - if (task->ata_task.dma_xfer) { > + if (task->ata_task.use_ncq && > + dev->sata_dev.class != ATA_DEV_ATAPI) { > + ATAP = 0x07; /* FPDMA */ > + pm8001_dbg(pm8001_ha, IO, "FPDMA\n"); > + } else if (task->ata_task.dma_xfer) { > ATAP = 0x06; /* DMA */ > pm8001_dbg(pm8001_ha, IO, "DMA\n"); > } else { > ATAP = 0x05; /* PIO*/ > pm8001_dbg(pm8001_ha, IO, "PIO\n"); > } > - if (task->ata_task.use_ncq && > - dev->sata_dev.class != ATA_DEV_ATAPI) { > - ATAP = 0x07; /* FPDMA */ > - pm8001_dbg(pm8001_ha, IO, "FPDMA\n"); > - } > } > if (task->ata_task.use_ncq && pm8001_get_ncq_tag(task, &hdr_tag)) { > task->ata_task.fis.sector_count |= (u8) (hdr_tag << 3); > -- > 2.34.1 >
diff --git a/drivers/scsi/pm8001/pm8001_hwi.c b/drivers/scsi/pm8001/pm8001_hwi.c index e20a1d4db026..c0215a35a7b5 100644 --- a/drivers/scsi/pm8001/pm8001_hwi.c +++ b/drivers/scsi/pm8001/pm8001_hwi.c @@ -4265,22 +4265,22 @@ static int pm8001_chip_sata_req(struct pm8001_hba_info *pm8001_ha, u32 opc = OPC_INB_SATA_HOST_OPSTART; memset(&sata_cmd, 0, sizeof(sata_cmd)); circularQ = &pm8001_ha->inbnd_q_tbl[0]; - if (task->data_dir == DMA_NONE) { + + if (task->data_dir == DMA_NONE && !task->ata_task.use_ncq) { ATAP = 0x04; /* no data*/ pm8001_dbg(pm8001_ha, IO, "no data\n"); } else if (likely(!task->ata_task.device_control_reg_update)) { - if (task->ata_task.dma_xfer) { + if (task->ata_task.use_ncq && + dev->sata_dev.class != ATA_DEV_ATAPI) { + ATAP = 0x07; /* FPDMA */ + pm8001_dbg(pm8001_ha, IO, "FPDMA\n"); + } else if (task->ata_task.dma_xfer) { ATAP = 0x06; /* DMA */ pm8001_dbg(pm8001_ha, IO, "DMA\n"); } else { ATAP = 0x05; /* PIO*/ pm8001_dbg(pm8001_ha, IO, "PIO\n"); } - if (task->ata_task.use_ncq && - dev->sata_dev.class != ATA_DEV_ATAPI) { - ATAP = 0x07; /* FPDMA */ - pm8001_dbg(pm8001_ha, IO, "FPDMA\n"); - } } if (task->ata_task.use_ncq && pm8001_get_ncq_tag(task, &hdr_tag)) { task->ata_task.fis.sector_count |= (u8) (hdr_tag << 3); diff --git a/drivers/scsi/pm8001/pm80xx_hwi.c b/drivers/scsi/pm8001/pm80xx_hwi.c index 60c305f987b5..3deb89b11d2f 100644 --- a/drivers/scsi/pm8001/pm80xx_hwi.c +++ b/drivers/scsi/pm8001/pm80xx_hwi.c @@ -4546,22 +4546,21 @@ static int pm80xx_chip_sata_req(struct pm8001_hba_info *pm8001_ha, q_index = (u32) (cpu_id) % (pm8001_ha->max_q_num); circularQ = &pm8001_ha->inbnd_q_tbl[q_index]; - if (task->data_dir == DMA_NONE) { + if (task->data_dir == DMA_NONE && !task->ata_task.use_ncq) { ATAP = 0x04; /* no data*/ pm8001_dbg(pm8001_ha, IO, "no data\n"); } else if (likely(!task->ata_task.device_control_reg_update)) { - if (task->ata_task.dma_xfer) { + if (task->ata_task.use_ncq && + dev->sata_dev.class != ATA_DEV_ATAPI) { + ATAP = 0x07; /* FPDMA */ + pm8001_dbg(pm8001_ha, IO, "FPDMA\n"); + } else if (task->ata_task.dma_xfer) { ATAP = 0x06; /* DMA */ pm8001_dbg(pm8001_ha, IO, "DMA\n"); } else { ATAP = 0x05; /* PIO*/ pm8001_dbg(pm8001_ha, IO, "PIO\n"); } - if (task->ata_task.use_ncq && - dev->sata_dev.class != ATA_DEV_ATAPI) { - ATAP = 0x07; /* FPDMA */ - pm8001_dbg(pm8001_ha, IO, "FPDMA\n"); - } } if (task->ata_task.use_ncq && pm8001_get_ncq_tag(task, &hdr_tag)) { task->ata_task.fis.sector_count |= (u8) (hdr_tag << 3);
In the pm8001_chip_sata_req() and pm80xx_chip_sata_req() functions, all tasks with a DMA direction of DMA_NONE (no data transfer) are initialized using the ATAP value 0x04. However, NCQ NON DATA commands, while being DMA_NONE commands are NCQ commands and need to be initialized using the value 0x07 for ATAP, similarly to other NCQ commands. Make sure that NCQ NON DATA command tasks are initialized similarly to other NCQ commands by also testing the task "use_ncq" field in addition to the DMA direction. While at it, reorganize the code into a chain of if - else if - else to avoid useless affectations and debug messages. Fixes: dbf9bfe61571 ("[SCSI] pm8001: add SAS/SATA HBA driver") Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> --- drivers/scsi/pm8001/pm8001_hwi.c | 14 +++++++------- drivers/scsi/pm8001/pm80xx_hwi.c | 13 ++++++------- 2 files changed, 13 insertions(+), 14 deletions(-)