diff mbox series

[v2,3/3] ata: ahci: implement SCSI_SYNC_CACHE

Message ID 20250326-scsi-sync-on-write-v2-3-12ab05bd464b@linaro.org
State New
Headers show
Series scsi: ensure writes are flushed to disk | expand

Commit Message

Caleb Connolly March 26, 2025, 12:24 p.m. UTC
The SCSI layer now issues a SYNC_CACHE command after every write to
ensure there is no data loss due to a board reset after write.

Implement support for this command and remove the same logic from the
ATA write path to be consistent with other SCSI backends.

Ranges are not supported and the whole cache will be flushed in all
cases.

This was done per iteration in ata_scsiop_read_write(), but it's not
clear why this was the case, calling it once for the entire write ought
to achieve the same result.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 drivers/ata/ahci.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

Comments

Neil Armstrong March 26, 2025, 12:33 p.m. UTC | #1
On 26/03/2025 13:24, Caleb Connolly wrote:
> The SCSI layer now issues a SYNC_CACHE command after every write to
> ensure there is no data loss due to a board reset after write.
> 
> Implement support for this command and remove the same logic from the
> ATA write path to be consistent with other SCSI backends.
> 
> Ranges are not supported and the whole cache will be flushed in all
> cases.
> 
> This was done per iteration in ata_scsiop_read_write(), but it's not
> clear why this was the case, calling it once for the entire write ought
> to achieve the same result.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
> ---
>   drivers/ata/ahci.c | 14 +++-----------
>   1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
> index 8058d5ff1c37f92914f250fa9cc78498a3f27331..deb91efd6d2666016f375feabb16ebc8f68fb697 100644
> --- a/drivers/ata/ahci.c
> +++ b/drivers/ata/ahci.c
> @@ -735,19 +735,8 @@ static int ata_scsiop_read_write(struct ahci_uc_priv *uc_priv,
>   			debug("scsi_ahci: SCSI %s10 command failure.\n",
>   			      is_write ? "WRITE" : "READ");
>   			return -EIO;
>   		}
> -
> -		/* If this transaction is a write, do a following flush.
> -		 * Writes in u-boot are so rare, and the logic to know when is
> -		 * the last write and do a flush only there is sufficiently
> -		 * difficult. Just do a flush after every write. This incurs,
> -		 * usually, one extra flush when the rare writes do happen.
> -		 */
> -		if (is_write) {
> -			if (-EIO == ata_io_flush(uc_priv, pccb->target))
> -				return -EIO;
> -		}
>   		user_buffer += transfer_size;
>   		user_buffer_size -= transfer_size;
>   		blocks -= now_blocks;
>   		lba += now_blocks;
> @@ -845,8 +834,11 @@ static int ahci_scsi_exec(struct udevice *dev, struct scsi_cmd *pccb)
>   		break;
>   	case SCSI_INQUIRY:
>   		ret = ata_scsiop_inquiry(uc_priv, pccb);
>   		break;
> +	case SCSI_SYNC_CACHE:
> +		ret = ata_io_flush(uc_priv, pccb->target);
> +		break;
>   	default:
>   		printf("Unsupport SCSI command 0x%02x\n", pccb->cmd[0]);
>   		return -ENOTSUPP;
>   	}
> 

Good catch, much simpler to move it in the scsi layer !

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Can someone test on a SATA enabled device ?

I can have a test run on the Odroid-HC4 with a PCIe-SATA controller.

Thanks,
Neil
diff mbox series

Patch

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index 8058d5ff1c37f92914f250fa9cc78498a3f27331..deb91efd6d2666016f375feabb16ebc8f68fb697 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -735,19 +735,8 @@  static int ata_scsiop_read_write(struct ahci_uc_priv *uc_priv,
 			debug("scsi_ahci: SCSI %s10 command failure.\n",
 			      is_write ? "WRITE" : "READ");
 			return -EIO;
 		}
-
-		/* If this transaction is a write, do a following flush.
-		 * Writes in u-boot are so rare, and the logic to know when is
-		 * the last write and do a flush only there is sufficiently
-		 * difficult. Just do a flush after every write. This incurs,
-		 * usually, one extra flush when the rare writes do happen.
-		 */
-		if (is_write) {
-			if (-EIO == ata_io_flush(uc_priv, pccb->target))
-				return -EIO;
-		}
 		user_buffer += transfer_size;
 		user_buffer_size -= transfer_size;
 		blocks -= now_blocks;
 		lba += now_blocks;
@@ -845,8 +834,11 @@  static int ahci_scsi_exec(struct udevice *dev, struct scsi_cmd *pccb)
 		break;
 	case SCSI_INQUIRY:
 		ret = ata_scsiop_inquiry(uc_priv, pccb);
 		break;
+	case SCSI_SYNC_CACHE:
+		ret = ata_io_flush(uc_priv, pccb->target);
+		break;
 	default:
 		printf("Unsupport SCSI command 0x%02x\n", pccb->cmd[0]);
 		return -ENOTSUPP;
 	}