[PATCH 3/3] scsi: sync cache on write

[Neil Armstrong]

On 25/03/2025 14:02, Caleb Connolly wrote:
> We don't have a mechanism to safely shutdown block devices prior to a
> baord reset or driver removal. Prevent data loss by synchronizing the
> SCSI cache after every write.
> 
> In particular this solves the issue of capsule updates looping on some
> devices because the board resets immediately after deleting the capsule
> file and this write wouldn't be flushed in time.
> 
> This may impact NAND wear, but should be negligible given the usecases
> for disk write in U-Boot.
> 
> Signed-off-by: Caleb Connolly <caleb.connolly at linaro.org>
> ---
>   drivers/scsi/scsi.c | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
> 
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 1aa7fbdbb5278e387de72a3c3e73d19ea0342fff..1b71e580b89b100395ea132a4976366a713cba8a 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -77,8 +77,25 @@ static void scsi_setup_inquiry(struct scsi_cmd *pccb)
>   	pccb->cmdlen = 6;
>   	pccb->msgout[0] = SCSI_IDENTIFY; /* NOT USED */
>   }
>   
> +static void scsi_setup_sync_cache(struct scsi_cmd *pccb, lbaint_t start,
> +				  unsigned short blocks)
> +{
> +	pccb->cmd[0] = SCSI_SYNC_CACHE;
> +	pccb->cmd[1] = 0;
> +	pccb->cmd[2] = (unsigned char)(start >> 24) & 0xff;
> +	pccb->cmd[3] = (unsigned char)(start >> 16) & 0xff;
> +	pccb->cmd[4] = (unsigned char)(start >> 8) & 0xff;
> +	pccb->cmd[5] = (unsigned char)start & 0xff;
> +	pccb->cmd[6] = 0;
> +	pccb->cmd[7] = (unsigned char)(blocks >> 8) & 0xff;
> +	pccb->cmd[8] = (unsigned char)blocks & 0xff;
> +	pccb->cmd[9] = 0;
> +	pccb->cmdlen = 10;
> +	pccb->msgout[0] = SCSI_IDENTIFY; /* NOT USED */
> +}
> +
>   static void scsi_setup_read_ext(struct scsi_cmd *pccb, lbaint_t start,
>   				unsigned short blocks)
>   {
>   	pccb->cmd[0] = SCSI_READ10;
> @@ -235,8 +252,19 @@ static ulong scsi_write(struct udevice *dev, lbaint_t blknr, lbaint_t blkcnt,
>   			blkcnt -= blks;
>   			break;
>   		}
>   
> +		/*
> +		 * Ensure the writes are flushed to disk so we don't lose data
> +		 * on board reset.
> +		 * FIXME: this ought to be in a SCSI shutdown path instead
> +		 */
> +		scsi_setup_sync_cache(pccb, start, smallblks);

Why can't this be done at the end of scsi_write() ?

> +		if (scsi_exec(bdev, pccb)) {
> +			scsi_print_error(pccb);
> +			break;
> +		}
> +
>   		if (blks > max_blks) {
>   			start += max_blks;
>   			blks -= max_blks;
>   		} else {
> 

And perhaps a sync subcommand of the scsi command would be nice to have !

Neil