Hi Alexey, On 2026-09-18T06:35:41, Alexey Charkov <[email protected]> wrote: > ufs: hand sense data to the SCSI layer > > A logical unit exposes a CHECK CONDITION through its sense data, and > without it a caller cannot tell a transient condition from a permanent > one. struct scsi_cmd has carried a sense buffer all along that no driver > ever filled, so fill it, and clear the length in scsi_exec() so that one > command cannot be read as another's sense.
Not quite. common/usb_storage.c fills sense_buf from its REQUEST SENSE, although it never sets sensedatalen and does not go through scsi_exec(). Please can you reword this, e.g. to say that no driver in the SCSI uclass fills it? The patch also starts setting pccb->status, which is not mentioned here. > > Signed-off-by: Alexey Charkov <[email protected]> > > drivers/scsi/scsi-uclass.c | 3 +++ > drivers/ufs/ufs-uclass.c | 23 ++++++++++++++++++++++- > 2 files changed, 25 insertions(+), 1 deletion(-) > diff --git a/drivers/scsi/scsi-uclass.c b/drivers/scsi/scsi-uclass.c > @@ -22,6 +22,9 @@ int scsi_exec(struct udevice *dev, struct scsi_cmd *pccb) > + /* Whatever a previous command left behind is not this command's sense > */ > + pccb->sensedatalen = 0; > + This changes the uclass for every SCSI driver, under a 'ufs:' subject. I suspect it would be better as its own 'scsi:' patch, ahead of this one. What do you think? > diff --git a/drivers/ufs/ufs-uclass.c b/drivers/ufs/ufs-uclass.c > @@ -1683,8 +1701,11 @@ static int ufs_scsi_exec(struct udevice *scsi_dev, > struct scsi_cmd *pccb) > - if (scsi_status) > + if (scsi_status) { > + ufshcd_copy_sense_data(hba, pccb); > + pccb->status = scsi_status; > return -EINVAL; The status is only written on failure and scsi.c uses a single static tempccb for everything, so a later command that succeeds still shows the old status. That is the same problem you are solving for sensedatalen, so please can you clear the status in scsi_exec() as well, or set it unconditionally here? Also, as far as I can see nothing in the series reads pccb->status, since scsi_unit_attention() in the next patch only looks at the sense data. Either check for CHECK CONDITION there before trusting the sense buffer, or drop this line. Regards, Simon
