* [PATCH] scsi: hisi_sas: Fixed failure to issue vendor specific commands
@ 2025-02-20 9:00 Yihang Li
2025-03-13 1:08 ` Yihang Li
2025-03-21 0:36 ` Martin K. Petersen
0 siblings, 2 replies; 4+ messages in thread
From: Yihang Li @ 2025-02-20 9:00 UTC (permalink / raw)
To: James.Bottomley, martin.petersen
Cc: linux-scsi, linuxarm, liyihang9, liuyonglong, prime.zeng
From: Xingui Yang <yangxingui@huawei.com>
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@huawei.com>
Reviewed-by: Yihang Li <liyihang9@huawei.com>
---
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))
--
2.33.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: hisi_sas: Fixed failure to issue vendor specific commands
2025-02-20 9:00 [PATCH] scsi: hisi_sas: Fixed failure to issue vendor specific commands Yihang Li
@ 2025-03-13 1:08 ` Yihang Li
2025-03-18 1:28 ` Martin K. Petersen
2025-03-21 0:36 ` Martin K. Petersen
1 sibling, 1 reply; 4+ messages in thread
From: Yihang Li @ 2025-03-13 1:08 UTC (permalink / raw)
To: James.Bottomley, martin.petersen
Cc: linux-scsi, linuxarm, liuyonglong, prime.zeng
Gentle ping...
On 2025/2/20 17:00, Yihang Li wrote:
> From: Xingui Yang <yangxingui@huawei.com>
>
> 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@huawei.com>
> Reviewed-by: Yihang Li <liyihang9@huawei.com>
> ---
> 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))
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: hisi_sas: Fixed failure to issue vendor specific commands
2025-03-13 1:08 ` Yihang Li
@ 2025-03-18 1:28 ` Martin K. Petersen
0 siblings, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2025-03-18 1:28 UTC (permalink / raw)
To: Yihang Li
Cc: James.Bottomley, martin.petersen, linux-scsi, linuxarm,
liuyonglong, prime.zeng
Yihang,
>> 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.
Applied to 6.15/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] scsi: hisi_sas: Fixed failure to issue vendor specific commands
2025-02-20 9:00 [PATCH] scsi: hisi_sas: Fixed failure to issue vendor specific commands Yihang Li
2025-03-13 1:08 ` Yihang Li
@ 2025-03-21 0:36 ` Martin K. Petersen
1 sibling, 0 replies; 4+ messages in thread
From: Martin K. Petersen @ 2025-03-21 0:36 UTC (permalink / raw)
To: James.Bottomley, Yihang Li
Cc: Martin K . Petersen, linux-scsi, linuxarm, liuyonglong,
prime.zeng
On Thu, 20 Feb 2025 17:00:11 +0800, Yihang Li wrote:
> 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.
>
>
> [...]
Applied to 6.15/scsi-queue, thanks!
[1/1] scsi: hisi_sas: Fixed failure to issue vendor specific commands
https://git.kernel.org/mkp/scsi/c/750d4fbe2c20
--
Martin K. Petersen Oracle Linux Engineering
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-03-21 0:37 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-20 9:00 [PATCH] scsi: hisi_sas: Fixed failure to issue vendor specific commands Yihang Li
2025-03-13 1:08 ` Yihang Li
2025-03-18 1:28 ` Martin K. Petersen
2025-03-21 0:36 ` Martin K. Petersen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox