[PATCH] ata: libata-scsi: refactor ata_scsiop_maint_in()

[PATCH] ata: libata-scsi: refactor ata_scsiop_maint_in()

[Niklas Cassel]

ata_scsiop_maint_in() is currently quite confusing to read, because it
currently only implements support for the service action REPORT SUPPORTED
OPERATION CODES.

Thus, when this function is checking for "invalid command format", it is
not very clear if it is an invalid command format for the MAINTENANCE IN
command itself, or an invalid command format for the (currently one and
only) service action/subcommand implemented for this command.

Move the service action to a separate function, so it is more clear that
the "invalid command format" check is actually specific for the REPORT
SUPPORTED OPERATION CODES service action.

This also makes it easier and less confusing to add support for additional
service actions in the future.

Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 drivers/ata/libata-scsi.c | 47 ++++++++++++++++++++++++---------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 00b3ffbfe169..06f3a243f037 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3573,28 +3573,13 @@ static unsigned int ata_scsi_write_same_xlat(struct ata_queued_cmd *qc)
 	return 1;
 }
 
-/**
- *	ata_scsiop_maint_in - Simulate a subset of MAINTENANCE_IN
- *	@dev: Target device.
- *	@cmd: SCSI command of interest.
- *	@rbuf: Response buffer, to which simulated SCSI cmd output is sent.
- *
- *	Yields a subset to satisfy scsi_report_opcode()
- *
- *	LOCKING:
- *	spin_lock_irqsave(host lock)
- */
-static unsigned int ata_scsiop_maint_in(struct ata_device *dev,
-					struct scsi_cmnd *cmd, u8 *rbuf)
+static unsigned int ata_scsi_report_supported_opcodes(struct ata_device *dev,
+						      struct scsi_cmnd *cmd,
+						      u8 *rbuf)
 {
 	u8 *cdb = cmd->cmnd;
 	u8 supported = 0, cdlp = 0, rwcdlp = 0;
 
-	if ((cdb[1] & 0x1f) != MI_REPORT_SUPPORTED_OPERATION_CODES) {
-		ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
-		return 0;
-	}
-
 	if (cdb[2] != 1 && cdb[2] != 3) {
 		ata_dev_warn(dev, "invalid command format %d\n", cdb[2]);
 		ata_scsi_set_invalid_field(dev, cmd, 2, 0xff);
@@ -3674,6 +3659,32 @@ static unsigned int ata_scsiop_maint_in(struct ata_device *dev,
 	return 4;
 }
 
+/**
+ *	ata_scsiop_maint_in - Simulate a subset of MAINTENANCE_IN
+ *	@dev: Target device.
+ *	@cmd: SCSI command of interest.
+ *	@rbuf: Response buffer, to which simulated SCSI cmd output is sent.
+ *
+ *	Yields a subset to satisfy scsi_report_opcode()
+ *
+ *	LOCKING:
+ *	spin_lock_irqsave(host lock)
+ */
+static unsigned int ata_scsiop_maint_in(struct ata_device *dev,
+					struct scsi_cmnd *cmd, u8 *rbuf)
+{
+	u8 *cdb = cmd->cmnd;
+	u8 service_action = cdb[1] & 0x1f;
+
+	switch (service_action) {
+	case MI_REPORT_SUPPORTED_OPERATION_CODES:
+		return ata_scsi_report_supported_opcodes(dev, cmd, rbuf);
+	default:
+		ata_scsi_set_invalid_field(dev, cmd, 1, 0xff);
+		return 0;
+	}
+}
+
 /**
  *	ata_scsi_report_zones_complete - convert ATA output
  *	@qc: command structure returning the data
-- 
2.53.0

Re: [PATCH] ata: libata-scsi: refactor ata_scsiop_maint_in()

[Damien Le Moal]

On 3/20/26 19:59, Niklas Cassel wrote:
> ata_scsiop_maint_in() is currently quite confusing to read, because it
> currently only implements support for the service action REPORT SUPPORTED
> OPERATION CODES.
> 
> Thus, when this function is checking for "invalid command format", it is
> not very clear if it is an invalid command format for the MAINTENANCE IN
> command itself, or an invalid command format for the (currently one and
> only) service action/subcommand implemented for this command.
> 
> Move the service action to a separate function, so it is more clear that
> the "invalid command format" check is actually specific for the REPORT
> SUPPORTED OPERATION CODES service action.
> 
> This also makes it easier and less confusing to add support for additional
> service actions in the future.
> 
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH] ata: libata-scsi: refactor ata_scsiop_maint_in()

[Niklas Cassel]

On Fri, 20 Mar 2026 11:59:51 +0100, Niklas Cassel wrote:
> ata_scsiop_maint_in() is currently quite confusing to read, because it
> currently only implements support for the service action REPORT SUPPORTED
> OPERATION CODES.
> 
> Thus, when this function is checking for "invalid command format", it is
> not very clear if it is an invalid command format for the MAINTENANCE IN
> command itself, or an invalid command format for the (currently one and
> only) service action/subcommand implemented for this command.
> 
> [...]

Applied to libata/linux.git (for-7.1), thanks!

[1/1] ata: libata-scsi: refactor ata_scsiop_maint_in()
      https://git.kernel.org/libata/linux/c/a6ac0af4

Kind regards,
Niklas