| Message ID | 20260903034201.112211-1-dlemoal@kernel.org |
|---|---|
| Headers | show |
| Series | Use defined 16-bits ASC/ASCQ combinations | expand |
> Refactor the EMC SCSI device handler driver to use the 16-bits sense_code > field of struct scsi_sense_hdr and replace all hard-coded additional sense > codes and additional sense code qualifiers with the enum values defined in > include/scsi/scsi_sense.h. This helps with code clarity as the sense codes > being processed are easier to test and self-documented. > > No functional change intended. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Sashiko has reviewed this patch and found no issues. It looks great!
> Refactor the megaraid driver to use the scsi_set_sense() function and > replace all hard-coded additional sense codes and additional sense code > qualifiers with the enum values defined in include/scsi/scsi_sense.h. This > helps with code clarity as the sense codes being processed are easier to > test and self-documented. > > No functional change intended. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Sashiko has reviewed this patch and found no issues. It looks great!
> Refactor the hpsa driver to replace all hard-coded additional sense codes > and additional sense code qualifiers with the enum values defined in > include/scsi/scsi_sense.h. This helps with code clarity as the sense codes > being processed are easier to test and self-documented. > > The function decode_sense_data() is also modified to take a pointer to a > 16-bits sense_code variable in place of the two pointers to the additional > sense code and its code qualifier. The local definitions of ASCs and ASCQs > are deleted too. > > No functional change intended, but the function hpsa_volume_offline() was > checking only the addditional sense code qualifier with checking the > additional sense code. This change assumes that the intended additional > sense code to check is ASC_LU_NOT_READY. > > Signed-off-by: Damien Le Moal <dlemoal@kernel.org> Sashiko has reviewed this patch and found no issues. It looks great!
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Incomplete refactoring leaves hard-coded ASCQ values in `zfcp_fsf.c`, contradicting the commit message.
--
commit a120aaf764276a0a8205d59d479d4eea2ff29d24
Author: Damien Le Moal <dlemoal@kernel.org>
s390: scsi: use 16-bits defined sense codes
Use the 16-bits sense codes and replace all hard-coded additional sense
codes and additional sense code qualifiers with the enum values defined
in include/scsi/scsi_sense.h. This helps with code clarity as the sense
codes being processed are easier to test and self-documented.
> diff --git a/drivers/s390/scsi/zfcp_scsi.c b/drivers/s390/scsi/zfcp_scsi.c
> index 3590113c2b744..0994fa328a4fa 100644
> --- a/drivers/s390/scsi/zfcp_scsi.c
> +++ b/drivers/s390/scsi/zfcp_scsi.c
> @@ -856,7 +856,8 @@ void zfcp_scsi_set_prot(struct zfcp_adapter *adapter)
> */
> void zfcp_scsi_dif_sense_error(struct scsi_cmnd *scmd, int ascq)
> {
> - scsi_build_sense(scmd, 1, ILLEGAL_REQUEST, 0x10, ascq);
> + scsi_set_sense(scmd, 1, ILLEGAL_REQUEST,
> + scsi_sense_code(ASC_ID_CRC_OR_ECC_ERROR, ascq));
[Severity: Medium]
Does leaving the ascq parameter as an integer leave hard-coded additional
sense code qualifiers in the codebase, contradicting the commit message?
Looking at drivers/s390/scsi/zfcp_fsf.c:zfcp_fsf_fcp_cmnd_handler():
case FSF_BLOCK_GUARD_CHECK_FAILURE:
zfcp_scsi_dif_sense_error(scpnt, 0x1);
goto skip_fsfstatus;
case FSF_APP_TAG_CHECK_FAILURE:
zfcp_scsi_dif_sense_error(scpnt, 0x2);
It appears the hard-coded literals (0x1, 0x2) are still being used here
instead of the new 16-bit enum values.
Could the function signature and callers be updated to use the full 16-bit
enums (e.g., LOGICAL_BLOCK_GUARD_CHECK_FAILED) to completely remove these
hard-coded values?