Gentle ping...
On 2025/2/20 17:00, Yihang Li wrote:
> From: Xingui Yang <yangxingui@xxxxxxxxxx>
>
> At present, we determine the protocol through the cmd type, but other
> cmd types, such as vendor-specific commands, default to the pio protocol.
> This strategy often causes the execution of different vendor-specific
> commands to fail. In fact, for these commands, a better way is to use the
> protocol configured by the command's tf to determine its protocol.
>
> Fixes: 6f2ff1a1311e ("hisi_sas: add v2 path to send ATA command")
> Signed-off-by: Xingui Yang <yangxingui@xxxxxxxxxx>
> Reviewed-by: Yihang Li <liyihang9@xxxxxxxxxx>
> ---
> drivers/scsi/hisi_sas/hisi_sas.h | 3 +--
> drivers/scsi/hisi_sas/hisi_sas_main.c | 28 ++++++++++++++++++++++++--
> drivers/scsi/hisi_sas/hisi_sas_v2_hw.c | 4 +---
> drivers/scsi/hisi_sas/hisi_sas_v3_hw.c | 4 +---
> 4 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas.h b/drivers/scsi/hisi_sas/hisi_sas.h
> index 2d438d722d0b..e17f5d8226bf 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas.h
> +++ b/drivers/scsi/hisi_sas/hisi_sas.h
> @@ -633,8 +633,7 @@ extern struct dentry *hisi_sas_debugfs_dir;
> extern void hisi_sas_stop_phys(struct hisi_hba *hisi_hba);
> extern int hisi_sas_alloc(struct hisi_hba *hisi_hba);
> extern void hisi_sas_free(struct hisi_hba *hisi_hba);
> -extern u8 hisi_sas_get_ata_protocol(struct host_to_dev_fis *fis,
> - int direction);
> +extern u8 hisi_sas_get_ata_protocol(struct sas_task *task);
> extern struct hisi_sas_port *to_hisi_sas_port(struct asd_sas_port *sas_port);
> extern void hisi_sas_sata_done(struct sas_task *task,
> struct hisi_sas_slot *slot);
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index da4a2ed8ee86..3596414d970b 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -21,8 +21,32 @@ struct hisi_sas_internal_abort_data {
> bool rst_ha_timeout; /* reset the HA for timeout */
> };
>
> -u8 hisi_sas_get_ata_protocol(struct host_to_dev_fis *fis, int direction)
> +static u8 hisi_sas_get_ata_protocol_from_tf(struct ata_queued_cmd *qc)
> {
> + if (!qc)
> + return HISI_SAS_SATA_PROTOCOL_PIO;
> +
> + switch (qc->tf.protocol) {
> + case ATA_PROT_NODATA:
> + return HISI_SAS_SATA_PROTOCOL_NONDATA;
> + case ATA_PROT_PIO:
> + return HISI_SAS_SATA_PROTOCOL_PIO;
> + case ATA_PROT_DMA:
> + return HISI_SAS_SATA_PROTOCOL_DMA;
> + case ATA_PROT_NCQ_NODATA:
> + case ATA_PROT_NCQ:
> + return HISI_SAS_SATA_PROTOCOL_FPDMA;
> + default:
> + return HISI_SAS_SATA_PROTOCOL_PIO;
> + }
> +}
> +
> +u8 hisi_sas_get_ata_protocol(struct sas_task *task)
> +{
> + struct host_to_dev_fis *fis = &task->ata_task.fis;
> + struct ata_queued_cmd *qc = task->uldd_task;
> + int direction = task->data_dir;
> +
> switch (fis->command) {
> case ATA_CMD_FPDMA_WRITE:
> case ATA_CMD_FPDMA_READ:
> @@ -93,7 +117,7 @@ u8 hisi_sas_get_ata_protocol(struct host_to_dev_fis *fis, int direction)
> {
> if (direction == DMA_NONE)
> return HISI_SAS_SATA_PROTOCOL_NONDATA;
> - return HISI_SAS_SATA_PROTOCOL_PIO;
> + return hisi_sas_get_ata_protocol_from_tf(qc);
> }
> }
> }
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> index 71cd5b4450c2..6e7f99fcc824 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v2_hw.c
> @@ -2538,9 +2538,7 @@ static void prep_ata_v2_hw(struct hisi_hba *hisi_hba,
> (task->ata_task.fis.control & ATA_SRST))
> dw1 |= 1 << CMD_HDR_RESET_OFF;
>
> - dw1 |= (hisi_sas_get_ata_protocol(
> - &task->ata_task.fis, task->data_dir))
> - << CMD_HDR_FRAME_TYPE_OFF;
> + dw1 |= (hisi_sas_get_ata_protocol(task)) << CMD_HDR_FRAME_TYPE_OFF;
> dw1 |= sas_dev->device_id << CMD_HDR_DEV_ID_OFF;
> hdr->dw1 = cpu_to_le32(dw1);
>
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> index 48b95d9a7927..095bbf80c34e 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_v3_hw.c
> @@ -1456,9 +1456,7 @@ static void prep_ata_v3_hw(struct hisi_hba *hisi_hba,
> (task->ata_task.fis.control & ATA_SRST))
> dw1 |= 1 << CMD_HDR_RESET_OFF;
>
> - dw1 |= (hisi_sas_get_ata_protocol(
> - &task->ata_task.fis, task->data_dir))
> - << CMD_HDR_FRAME_TYPE_OFF;
> + dw1 |= (hisi_sas_get_ata_protocol(task)) << CMD_HDR_FRAME_TYPE_OFF;
> dw1 |= sas_dev->device_id << CMD_HDR_DEV_ID_OFF;
>
> if (FIS_CMD_IS_UNCONSTRAINED(task->ata_task.fis))
>
